From ed6a7148f409bce0d24ba52a82a4da8b64ee7d08 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 19 Jan 2024 19:19:01 +0900 Subject: [PATCH 1/5] network/route-nexthop: fix route_nexthop_copy() Follow-up for 413ea20ab3d86b5bccf775da21a945327b3880c9. --- src/network/networkd-route-nexthop.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/network/networkd-route-nexthop.c b/src/network/networkd-route-nexthop.c index 9e14c695be..639302db71 100644 --- a/src/network/networkd-route-nexthop.c +++ b/src/network/networkd-route-nexthop.c @@ -179,7 +179,7 @@ static int route_nexthop_copy(const RouteNextHop *src, RouteNextHop *dest) { /* unset pointer copied in the above. */ dest->ifname = NULL; - return strdup_or_null(src->ifindex == 0 ? NULL : src->ifname, &dest->ifname); + return strdup_or_null(src->ifindex > 0 ? NULL : src->ifname, &dest->ifname); } int route_nexthops_copy(const Route *src, const RouteNextHop *nh, Route *dest) { From 5a18697d61047e8c189bd280b6748f1159dcc385 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 16 Jan 2024 22:36:29 +0900 Subject: [PATCH 2/5] network/route: convert route before requesting Previously, 1. use the passed Route object as is when a route is requested, 2. when the route becomes ready to configure, convert the Route object if necessary, to resolve outgoing interface name, and split multipath routes, and save them to the associated interfaces, 3. configure the route with the passed Route object. However, there are several inconsistencies with what kernel does: - The kernel does not merge nor split IPv4 multipath routes. However, we unconditionally split multipath routes to manage. - The kernel does not set gateway or so to a route if it has nexthop ID. Fortunately, I do not find any issues caused by the inconsistencies. But for safety, let's manage routes in a consistent way with the kernel. This makes, 1. when a route is requested, split IPv6 multipath routes, but keep IPv4 multipath routes as is, and queue (possibly multiple) requests for the route. 2. when the route becomes ready to configure, resolve nexthop and interface name, and requeue request if necessary. 3. configure the (possibly split) route. By using the logic, - Now we manage routes in a mostly consistent way with the kernel. - We can drop ConvertedRoutes object. - Hopefully the code becomes much simpler. --- src/network/networkd-dhcp-prefix-delegation.c | 8 +- src/network/networkd-dhcp4.c | 3 +- src/network/networkd-ndisc.c | 3 +- src/network/networkd-queue.c | 4 +- src/network/networkd-route-nexthop.c | 66 +- src/network/networkd-route.c | 638 +++++++----------- src/network/networkd-route.h | 6 +- 7 files changed, 284 insertions(+), 444 deletions(-) diff --git a/src/network/networkd-dhcp-prefix-delegation.c b/src/network/networkd-dhcp-prefix-delegation.c index aecdaa5b25..b0e4afdc45 100644 --- a/src/network/networkd-dhcp-prefix-delegation.c +++ b/src/network/networkd-dhcp-prefix-delegation.c @@ -309,8 +309,7 @@ static int dhcp_pd_request_route(Link *link, const struct in6_addr *prefix, usec else route_unmark(existing); - r = link_request_route(link, TAKE_PTR(route), true, &link->dhcp_pd_messages, - dhcp_pd_route_handler, NULL); + r = link_request_route(link, route, &link->dhcp_pd_messages, dhcp_pd_route_handler); if (r < 0) return log_link_error_errno(link, r, "Failed to request DHCP-PD prefix route: %m"); @@ -708,7 +707,7 @@ static int dhcp_request_unreachable_route( else route_unmark(existing); - r = link_request_route(link, TAKE_PTR(route), true, counter, callback, NULL); + r = link_request_route(link, route, counter, callback); if (r < 0) return log_link_error_errno(link, r, "Failed to request unreachable route for DHCP delegated prefix %s: %m", IN6_ADDR_PREFIX_TO_STRING(addr, prefixlen)); @@ -806,8 +805,7 @@ static int dhcp4_pd_request_default_gateway_on_6rd_tunnel(Link *link, const stru else route_unmark(existing); - r = link_request_route(link, TAKE_PTR(route), true, &link->dhcp_pd_messages, - dhcp_pd_route_handler, NULL); + r = link_request_route(link, route, &link->dhcp_pd_messages, dhcp_pd_route_handler); if (r < 0) return log_link_debug_errno(link, r, "Failed to request default gateway for DHCP delegated prefix: %m"); diff --git a/src/network/networkd-dhcp4.c b/src/network/networkd-dhcp4.c index 2afda41ab5..6d8dfcfe29 100644 --- a/src/network/networkd-dhcp4.c +++ b/src/network/networkd-dhcp4.c @@ -399,8 +399,7 @@ static int dhcp4_request_route(Route *in, Link *link) { else route_unmark(existing); - return link_request_route(link, TAKE_PTR(route), true, &link->dhcp4_messages, - dhcp4_route_handler, NULL); + return link_request_route(link, route, &link->dhcp4_messages, dhcp4_route_handler); } static bool link_prefixroute(Link *link) { diff --git a/src/network/networkd-ndisc.c b/src/network/networkd-ndisc.c index 74bb549dac..3885bde180 100644 --- a/src/network/networkd-ndisc.c +++ b/src/network/networkd-ndisc.c @@ -224,8 +224,7 @@ static int ndisc_request_route(Route *in, Link *link, sd_ndisc_router *rt) { is_new = route_get(NULL, link, route, NULL) < 0; - r = link_request_route(link, TAKE_PTR(route), true, &link->ndisc_messages, - ndisc_route_handler, NULL); + r = link_request_route(link, route, &link->ndisc_messages, ndisc_route_handler); if (r < 0) return r; if (r > 0 && is_new) diff --git a/src/network/networkd-queue.c b/src/network/networkd-queue.c index 383f259fa7..f1dd6b44d2 100644 --- a/src/network/networkd-queue.c +++ b/src/network/networkd-queue.c @@ -58,7 +58,7 @@ static void request_hash_func(const Request *req, struct siphash *state) { siphash24_compress_typesafe(req->type, state); - if (req->type != REQUEST_TYPE_NEXTHOP) { + if (!IN_SET(req->type, REQUEST_TYPE_NEXTHOP, REQUEST_TYPE_ROUTE)) { siphash24_compress_boolean(req->link, state); if (req->link) siphash24_compress_typesafe(req->link->ifindex, state); @@ -81,7 +81,7 @@ static int request_compare_func(const struct Request *a, const struct Request *b if (r != 0) return r; - if (a->type != REQUEST_TYPE_NEXTHOP) { + if (!IN_SET(a->type, REQUEST_TYPE_NEXTHOP, REQUEST_TYPE_ROUTE)) { r = CMP(!!a->link, !!b->link); if (r != 0) return r; diff --git a/src/network/networkd-route-nexthop.c b/src/network/networkd-route-nexthop.c index 639302db71..eb90e79f9d 100644 --- a/src/network/networkd-route-nexthop.c +++ b/src/network/networkd-route-nexthop.c @@ -35,7 +35,7 @@ void route_nexthops_done(Route *route) { ordered_set_free(route->nexthops); } -static void route_nexthop_hash_func_full(const RouteNextHop *nh, struct siphash *state, bool hash_all_parameters) { +static void route_nexthop_hash_func_full(const RouteNextHop *nh, struct siphash *state, bool with_weight) { assert(nh); assert(state); @@ -46,15 +46,14 @@ static void route_nexthop_hash_func_full(const RouteNextHop *nh, struct siphash return; in_addr_hash_func(&nh->gw, nh->family, state); - if (!hash_all_parameters) - return; - siphash24_compress_typesafe(nh->weight, state); + if (with_weight) + siphash24_compress_typesafe(nh->weight, state); siphash24_compress_typesafe(nh->ifindex, state); if (nh->ifindex == 0) siphash24_compress_string(nh->ifname, state); /* For Network or Request object. */ } -static int route_nexthop_compare_func_full(const RouteNextHop *a, const RouteNextHop *b, bool hash_all_parameters) { +static int route_nexthop_compare_func_full(const RouteNextHop *a, const RouteNextHop *b, bool with_weight) { int r; assert(a); @@ -71,12 +70,11 @@ static int route_nexthop_compare_func_full(const RouteNextHop *a, const RouteNex if (r != 0) return r; - if (!hash_all_parameters) - return 0; - - r = CMP(a->weight, b->weight); - if (r != 0) - return r; + if (with_weight) { + r = CMP(a->weight, b->weight); + if (r != 0) + return r; + } r = CMP(a->ifindex, b->ifindex); if (r != 0) @@ -92,11 +90,11 @@ static int route_nexthop_compare_func_full(const RouteNextHop *a, const RouteNex } static void route_nexthop_hash_func(const RouteNextHop *nh, struct siphash *state) { - route_nexthop_hash_func_full(nh, state, /* hash_all_parameters = */ true); + route_nexthop_hash_func_full(nh, state, /* with_weight = */ true); } static int route_nexthop_compare_func(const RouteNextHop *a, const RouteNextHop *b) { - return route_nexthop_compare_func_full(a, b, /* hash_all_parameters = */ true); + return route_nexthop_compare_func_full(a, b, /* with_weight = */ true); } DEFINE_PRIVATE_HASH_OPS_WITH_KEY_DESTRUCTOR( @@ -107,8 +105,6 @@ DEFINE_PRIVATE_HASH_OPS_WITH_KEY_DESTRUCTOR( route_nexthop_free); static size_t route_n_nexthops(const Route *route) { - assert(route); - if (route->nexthop_id != 0 || route_type_is_reject(route)) return 0; @@ -130,7 +126,7 @@ void route_nexthops_hash_func(const Route *route, struct siphash *state) { return; case 1: - route_nexthop_hash_func_full(&route->nexthop, state, /* hash_all_parameters = */ false); + route_nexthop_hash_func_full(&route->nexthop, state, /* with_weight = */ false); return; default: { @@ -157,7 +153,7 @@ int route_nexthops_compare_func(const Route *a, const Route *b) { return CMP(a->nexthop_id, b->nexthop_id); case 1: - return route_nexthop_compare_func_full(&a->nexthop, &b->nexthop, /* hash_all_parameters = */ false); + return route_nexthop_compare_func_full(&a->nexthop, &b->nexthop, /* with_weight = */ false); default: { RouteNextHop *nh; @@ -182,7 +178,28 @@ static int route_nexthop_copy(const RouteNextHop *src, RouteNextHop *dest) { return strdup_or_null(src->ifindex > 0 ? NULL : src->ifname, &dest->ifname); } +static int route_nexthop_dup(const RouteNextHop *src, RouteNextHop **ret) { + _cleanup_(route_nexthop_freep) RouteNextHop *dest = NULL; + int r; + + assert(src); + assert(ret); + + dest = new(RouteNextHop, 1); + if (!dest) + return -ENOMEM; + + r = route_nexthop_copy(src, dest); + if (r < 0) + return r; + + *ret = TAKE_PTR(dest); + return 0; +} + int route_nexthops_copy(const Route *src, const RouteNextHop *nh, Route *dest) { + int r; + assert(src); assert(dest); @@ -195,7 +212,20 @@ int route_nexthops_copy(const Route *src, const RouteNextHop *nh, Route *dest) { if (ordered_set_isempty(src->nexthops)) return route_nexthop_copy(&src->nexthop, &dest->nexthop); - /* Currently, this does not copy multipath routes. */ + ORDERED_SET_FOREACH(nh, src->nexthops) { + _cleanup_(route_nexthop_freep) RouteNextHop *nh_dup = NULL; + + r = route_nexthop_dup(nh, &nh_dup); + if (r < 0) + return r; + + r = ordered_set_ensure_put(&dest->nexthops, &route_nexthop_hash_ops, nh_dup); + if (r < 0) + return r; + assert(r > 0); + + TAKE_PTR(nh_dup); + } return 0; } diff --git a/src/network/networkd-route.c b/src/network/networkd-route.c index f877702efc..c3a0d30cca 100644 --- a/src/network/networkd-route.c +++ b/src/network/networkd-route.c @@ -262,24 +262,16 @@ int route_new_static(Network *network, const char *filename, unsigned section_li return 0; } -static int route_add(Manager *manager, Link *link, Route *route) { +static int route_add(Manager *manager, Route *route) { int r; + assert(manager); assert(route); + assert(!route->network); + assert(!route->wireguard); - if (route_type_is_reject(route)) { - assert(manager); - - r = set_ensure_put(&manager->routes, &route_hash_ops, route); - if (r < 0) - return r; - if (r == 0) - return -EEXIST; - - route->manager = manager; - } else { - assert(link); - + Link *link; + if (route_nexthop_get_link(manager, NULL, &route->nexthop, &link) >= 0) { r = set_ensure_put(&link->routes, &route_hash_ops, route); if (r < 0) return r; @@ -287,32 +279,35 @@ static int route_add(Manager *manager, Link *link, Route *route) { return -EEXIST; route->link = link; + return 0; } + r = set_ensure_put(&manager->routes, &route_hash_ops, route); + if (r < 0) + return r; + if (r == 0) + return -EEXIST; + + route->manager = manager; return 0; } -int route_get(Manager *manager, Link *link, const Route *in, Route **ret) { - Route *route; +int route_get(Manager *manager, Link *link, const Route *route, Route **ret) { + Route *existing; - assert(in); + if (!manager) + manager = ASSERT_PTR(ASSERT_PTR(link)->manager); + assert(route); - if (route_type_is_reject(in)) { - if (!manager) - return -ENOENT; - - route = set_get(manager->routes, in); - } else { - if (!link) - return -ENOENT; - - route = set_get(link->routes, in); - } - if (!route) + if (route_nexthop_get_link(manager, NULL, &route->nexthop, &link) >= 0) + existing = set_get(link->routes, route); + else + existing = set_get(manager->routes, route); + if (!existing) return -ENOENT; if (ret) - *ret = route; + *ret = existing; return 0; } @@ -336,16 +331,14 @@ static int route_get_link(Manager *manager, const Route *route, Link **ret) { return route_nexthop_get_link(manager, NULL, &route->nexthop, ret); } -static int route_get_request(Link *link, const Route *route, Request **ret) { +static int route_get_request(Manager *manager, const Route *route, Request **ret) { Request *req; - assert(link); - assert(link->manager); + assert(manager); assert(route); - req = ordered_set_get(link->manager->request_queue, + req = ordered_set_get(manager->request_queue, &(const Request) { - .link = link, .type = REQUEST_TYPE_ROUTE, .userdata = (void*) route, .hash_func = (hash_func_t) route_hash_func, @@ -393,179 +386,6 @@ int route_dup(const Route *src, const RouteNextHop *nh, Route **ret) { return 0; } -static void route_apply_nexthop(Route *route, const NextHop *nh, uint8_t nh_weight) { - assert(route); - assert(nh); - assert(hashmap_isempty(nh->group)); - - route->nexthop.family = nh->family; - route->nexthop.gw = nh->gw; - - if (nh_weight != UINT8_MAX) - route->nexthop.weight = nh_weight; - - if (nh->blackhole) - route->type = RTN_BLACKHOLE; -} - -static void route_apply_route_nexthop(Route *route, const RouteNextHop *nh) { - assert(route); - assert(nh); - - route->nexthop.family = nh->family; - route->nexthop.gw = nh->gw; - route->nexthop.weight = nh->weight; -} - -typedef struct ConvertedRoutes { - size_t n; - Route **routes; - Link **links; -} ConvertedRoutes; - -static ConvertedRoutes *converted_routes_free(ConvertedRoutes *c) { - if (!c) - return NULL; - - for (size_t i = 0; i < c->n; i++) - route_free(c->routes[i]); - - free(c->routes); - free(c->links); - - return mfree(c); -} - -DEFINE_TRIVIAL_CLEANUP_FUNC(ConvertedRoutes*, converted_routes_free); - -static int converted_routes_new(size_t n, ConvertedRoutes **ret) { - _cleanup_(converted_routes_freep) ConvertedRoutes *c = NULL; - _cleanup_free_ Route **routes = NULL; - _cleanup_free_ Link **links = NULL; - - assert(n > 0); - assert(ret); - - routes = new0(Route*, n); - if (!routes) - return -ENOMEM; - - links = new0(Link*, n); - if (!links) - return -ENOMEM; - - c = new(ConvertedRoutes, 1); - if (!c) - return -ENOMEM; - - *c = (ConvertedRoutes) { - .n = n, - .routes = TAKE_PTR(routes), - .links = TAKE_PTR(links), - }; - - *ret = TAKE_PTR(c); - return 0; -} - -static bool route_needs_convert(const Route *route) { - assert(route); - - return route->nexthop_id > 0 || !ordered_set_isempty(route->nexthops); -} - -static int route_convert(Manager *manager, Link *link, const Route *route, ConvertedRoutes **ret) { - _cleanup_(converted_routes_freep) ConvertedRoutes *c = NULL; - int r; - - assert(manager); - assert(route); - assert(ret); - - /* link may be NULL */ - - if (!route_needs_convert(route)) { - *ret = NULL; - return 0; - } - - if (route->nexthop_id > 0) { - struct nexthop_grp *nhg; - NextHop *nh; - - r = nexthop_get_by_id(manager, route->nexthop_id, &nh); - if (r < 0) - return r; - - if (hashmap_isempty(nh->group)) { - r = converted_routes_new(1, &c); - if (r < 0) - return r; - - r = route_dup(route, NULL, &c->routes[0]); - if (r < 0) - return r; - - route_apply_nexthop(c->routes[0], nh, UINT8_MAX); - (void) link_get_by_index(manager, nh->ifindex, c->links); - - *ret = TAKE_PTR(c); - return 1; - } - - r = converted_routes_new(hashmap_size(nh->group), &c); - if (r < 0) - return r; - - size_t i = 0; - HASHMAP_FOREACH(nhg, nh->group) { - NextHop *h; - - r = nexthop_get_by_id(manager, nhg->id, &h); - if (r < 0) - return r; - - r = route_dup(route, NULL, &c->routes[i]); - if (r < 0) - return r; - - route_apply_nexthop(c->routes[i], h, nhg->weight); - (void) link_get_by_index(manager, h->ifindex, c->links + i); - - i++; - } - - *ret = TAKE_PTR(c); - return 1; - - } - - assert(!ordered_set_isempty(route->nexthops)); - - r = converted_routes_new(ordered_set_size(route->nexthops), &c); - if (r < 0) - return r; - - size_t i = 0; - RouteNextHop *nh; - ORDERED_SET_FOREACH(nh, route->nexthops) { - r = route_dup(route, NULL, &c->routes[i]); - if (r < 0) - return r; - - route_apply_route_nexthop(c->routes[i], nh); - - r = route_nexthop_get_link(manager, link, nh, &c->links[i]); - if (r < 0) - return r; - - i++; - } - - *ret = TAKE_PTR(c); - return 1; -} - void link_mark_routes(Link *link, NetworkConfigSource source) { Route *route; @@ -727,14 +547,12 @@ static int route_remove_handler(sd_netlink *rtnl, sd_netlink_message *m, Link *l int route_remove(Route *route) { _cleanup_(sd_netlink_message_unrefp) sd_netlink_message *m = NULL; - unsigned char type; Manager *manager; Link *link; int r; assert(route); assert(route->manager || (route->link && route->link->manager)); - assert(IN_SET(route->family, AF_INET, AF_INET6)); link = route->link; manager = route->manager ?: link->manager; @@ -749,21 +567,6 @@ int route_remove(Route *route) { if (r < 0) return log_link_warning_errno(link, r, "Could not fill netlink message: %m"); - if (route->family == AF_INET && route->nexthop_id > 0 && route->type == RTN_BLACKHOLE) - /* When IPv4 route has nexthop id and the nexthop type is blackhole, even though kernel - * sends RTM_NEWROUTE netlink message with blackhole type, kernel's internal route type - * fib_rt_info::type may not be blackhole. Thus, we cannot know the internal value. - * Moreover, on route removal, the matching is done with the hidden value if we set - * non-zero type in RTM_DELROUTE message. Note, sd_rtnl_message_new_route() sets - * RTN_UNICAST by default. So, we need to clear the type here. */ - type = RTN_UNSPEC; - else - type = route->type; - - r = sd_rtnl_message_route_set_type(m, type); - if (r < 0) - return log_link_error_errno(link, r, "Could not set route type: %m"); - r = netlink_call_async(manager->rtnl, NULL, m, route_remove_handler, link ? link_netlink_destroy_callback : NULL, link); if (r < 0) @@ -790,10 +593,32 @@ int route_remove_and_drop(Route *route) { return 0; } +static int link_unmark_route(Link *link, const Route *route, const RouteNextHop *nh) { + _cleanup_(route_freep) Route *tmp = NULL; + Route *existing; + int r; + + assert(link); + assert(route); + + r = route_dup(route, nh, &tmp); + if (r < 0) + return r; + + r = route_adjust_nexthops(tmp, link); + if (r < 0) + return r; + + if (route_get(link->manager, link, tmp, &existing) < 0) + return 0; + + route_unmark(existing); + return 1; +} + static void manager_mark_routes(Manager *manager, bool foreign, const Link *except) { Route *route; Link *link; - int r; assert(manager); @@ -830,21 +655,14 @@ static void manager_mark_routes(Manager *manager, bool foreign, const Link *exce continue; HASHMAP_FOREACH(route, link->network->routes_by_section) { - _cleanup_(converted_routes_freep) ConvertedRoutes *converted = NULL; - Route *existing; + if (route->family == AF_INET || ordered_set_isempty(route->nexthops)) + (void) link_unmark_route(link, route, NULL); - r = route_convert(manager, link, route, &converted); - if (r < 0) - continue; - if (r == 0) { - if (route_get(manager, NULL, route, &existing) >= 0) - route_unmark(existing); - continue; + else { + RouteNextHop *nh; + ORDERED_SET_FOREACH(nh, route->nexthops) + (void) link_unmark_route(link, route, nh); } - - for (size_t i = 0; i < converted->n; i++) - if (route_get(manager, NULL, converted->routes[i], &existing) >= 0) - route_unmark(existing); } } } @@ -888,12 +706,11 @@ static void link_unmark_wireguard_routes(Link *link) { if (!link->netdev || link->netdev->kind != NETDEV_KIND_WIREGUARD) return; - Route *route, *existing; + Route *route; Wireguard *w = WIREGUARD(link->netdev); SET_FOREACH(route, w->routes) - if (route_get(NULL, link, route, &existing) >= 0) - route_unmark(existing); + (void) link_unmark_route(link, route, NULL); } int link_drop_foreign_routes(Link *link) { @@ -929,21 +746,14 @@ int link_drop_foreign_routes(Link *link) { } HASHMAP_FOREACH(route, link->network->routes_by_section) { - _cleanup_(converted_routes_freep) ConvertedRoutes *converted = NULL; - Route *existing; + if (route->family == AF_INET || ordered_set_isempty(route->nexthops)) + (void) link_unmark_route(link, route, NULL); - r = route_convert(link->manager, link, route, &converted); - if (r < 0) - continue; - if (r == 0) { - if (route_get(NULL, link, route, &existing) >= 0) - route_unmark(existing); - continue; + else { + RouteNextHop *nh; + ORDERED_SET_FOREACH(nh, route->nexthops) + (void) link_unmark_route(link, route, nh); } - - for (size_t i = 0; i < converted->n; i++) - if (route_get(NULL, link, converted->routes[i], &existing) >= 0) - route_unmark(existing); } link_unmark_wireguard_routes(link); @@ -1105,6 +915,64 @@ static int route_configure(const Route *route, uint32_t lifetime_sec, Link *link return request_call_netlink_async(link->manager->rtnl, m, req); } +static int route_requeue_request(Request *req, Link *link, const Route *route) { + _unused_ _cleanup_(request_unrefp) Request *req_unref = NULL; + _cleanup_(route_freep) Route *tmp = NULL; + int r; + + assert(req); + assert(link); + assert(link->manager); + assert(route); + + /* It is not possible to adjust the Route object owned by Request, as it is used as a key to manage + * Request objects in the queue. Hence, we need to re-request with the updated Route object. */ + + if (!route_nexthops_needs_adjust(route)) + return 0; /* The Route object does not need the adjustment. Continue with it. */ + + r = route_dup(route, NULL, &tmp); + if (r < 0) + return r; + + r = route_adjust_nexthops(tmp, link); + if (r <= 0) + return r; + + if (route_compare_func(route, tmp) == 0 && route->type == tmp->type) + return 0; /* No effective change?? That's OK. */ + + /* Avoid the request to be freed by request_detach(). */ + req_unref = request_ref(req); + + /* Detach the request from the queue, to make not the new request is deduped. + * Why this is necessary? IPv6 routes with different type may be handled as the same, + * As commented in route_adjust_nexthops(), we need to configure the adjusted type, + * otherwise we cannot remove the route on reconfigure or so. If we request the new Route object + * without detaching the current request, the new request is deduped, and the route is configured + * with unmodified type. */ + request_detach(req); + + /* Request the route with the adjusted Route object combined with the same other parameters. */ + r = link_queue_request_full(link, + req->type, + tmp, + req->free_func, + req->hash_func, + req->compare_func, + req->process, + req->counter, + req->netlink_handler, + NULL); + if (r < 0) + return r; + if (r == 0) + return 1; /* Already queued?? That's OK. Maybe, [Route] section is effectively duplicated. */ + + TAKE_PTR(tmp); + return 1; /* New request is queued. Finish to process this request. */ +} + static int route_is_ready_to_configure(const Route *route, Link *link) { int r; @@ -1127,7 +995,7 @@ static int route_is_ready_to_configure(const Route *route, Link *link) { } static int route_process_request(Request *req, Link *link, Route *route) { - _cleanup_(converted_routes_freep) ConvertedRoutes *converted = NULL; + Route *existing; int r; assert(req); @@ -1141,36 +1009,6 @@ static int route_process_request(Request *req, Link *link, Route *route) { if (r == 0) return 0; - if (route_needs_convert(route)) { - r = route_convert(link->manager, link, route, &converted); - if (r < 0) - return log_link_warning_errno(link, r, "Failed to convert route: %m"); - - assert(r > 0); - assert(converted); - - for (size_t i = 0; i < converted->n; i++) { - Route *existing; - - if (route_get(link->manager, converted->links[i] ?: link, converted->routes[i], &existing) < 0) { - _cleanup_(route_freep) Route *tmp = NULL; - - r = route_dup(converted->routes[i], NULL, &tmp); - if (r < 0) - return log_oom(); - - r = route_add(link->manager, converted->links[i] ?: link, tmp); - if (r < 0) - return log_link_warning_errno(link, r, "Failed to add route: %m"); - - TAKE_PTR(tmp); - } else { - existing->source = converted->routes[i]->source; - existing->provider = converted->routes[i]->provider; - } - } - } - usec_t now_usec; assert_se(sd_event_now(link->manager->event, CLOCK_BOOTTIME, &now_usec) >= 0); uint32_t sec = usec_to_sec(route->lifetime_usec, now_usec); @@ -1178,101 +1016,100 @@ static int route_process_request(Request *req, Link *link, Route *route) { log_link_debug(link, "Refuse to configure %s route with zero lifetime.", network_config_source_to_string(route->source)); - if (converted) - for (size_t i = 0; i < converted->n; i++) { - Route *existing; - - assert_se(route_get(link->manager, converted->links[i] ?: link, converted->routes[i], &existing) >= 0); - route_cancel_requesting(existing); - } - else - route_cancel_requesting(route); - + route_cancel_requesting(route); + if (route_get(link->manager, link, route, &existing) >= 0) + route_cancel_requesting(existing); return 1; } + r = route_requeue_request(req, link, route); + if (r != 0) + return r; + r = route_configure(route, sec, link, req); if (r < 0) return log_link_warning_errno(link, r, "Failed to configure route: %m"); - if (converted) - for (size_t i = 0; i < converted->n; i++) { - Route *existing; - - assert_se(route_get(link->manager, converted->links[i] ?: link, converted->routes[i], &existing) >= 0); - route_enter_configuring(existing); - } - else - route_enter_configuring(route); - + route_enter_configuring(route); + if (route_get(link->manager, link, route, &existing) >= 0) + route_enter_configuring(existing); return 1; } -int link_request_route( +static int link_request_route_one( Link *link, - Route *route, - bool consume_object, + const Route *route, + const RouteNextHop *nh, unsigned *message_counter, - route_netlink_handler_t netlink_handler, - Request **ret) { + route_netlink_handler_t netlink_handler) { + _cleanup_(route_freep) Route *tmp = NULL; Route *existing = NULL; int r; assert(link); assert(link->manager); assert(route); - assert(route->source != NETWORK_CONFIG_SOURCE_FOREIGN); - assert(!route_needs_convert(route)); - (void) route_get(link->manager, link, route, &existing); + r = route_dup(route, nh, &tmp); + if (r < 0) + return r; - if (route->lifetime_usec == 0) { - if (consume_object) - route_free(route); + r = route_adjust_nexthops(tmp, link); + if (r < 0) + return r; - /* The requested route is outdated. Let's remove it. */ - return route_remove_and_drop(existing); - } + if (route_get(link->manager, link, tmp, &existing) >= 0) + /* Copy state for logging below. */ + tmp->state = existing->state; - if (!existing) { - _cleanup_(route_freep) Route *tmp = NULL; - - if (consume_object) - tmp = route; - else { - r = route_dup(route, NULL, &tmp); - if (r < 0) - return r; - } - - r = route_add(link->manager, link, tmp); - if (r < 0) - return r; - - existing = TAKE_PTR(tmp); - } else { - existing->source = route->source; - existing->provider = route->provider; - existing->lifetime_usec = route->lifetime_usec; - if (consume_object) - route_free(route); - } - - log_route_debug(existing, "Requesting", link->manager); + log_route_debug(tmp, "Requesting", link->manager); r = link_queue_request_safe(link, REQUEST_TYPE_ROUTE, - existing, NULL, + tmp, + route_free, route_hash_func, route_compare_func, route_process_request, - message_counter, netlink_handler, ret); + message_counter, + netlink_handler, + NULL); if (r <= 0) return r; - route_enter_requesting(existing); + route_enter_requesting(tmp); + if (existing) + route_enter_requesting(existing); + + TAKE_PTR(tmp); return 1; } +int link_request_route( + Link *link, + const Route *route, + unsigned *message_counter, + route_netlink_handler_t netlink_handler) { + + int r; + + assert(link); + assert(link->manager); + assert(route); + assert(route->source != NETWORK_CONFIG_SOURCE_FOREIGN); + + if (route->family == AF_INET || route_type_is_reject(route) || ordered_set_isempty(route->nexthops)) + return link_request_route_one(link, route, NULL, message_counter, netlink_handler); + + RouteNextHop *nh; + ORDERED_SET_FOREACH(nh, route->nexthops) { + r = link_request_route_one(link, route, nh, message_counter, netlink_handler); + if (r < 0) + return r; + } + + return 0; +} + static int static_route_handler(sd_netlink *rtnl, sd_netlink_message *m, Request *req, Link *link, Route *route) { int r; @@ -1291,22 +1128,6 @@ static int static_route_handler(sd_netlink *rtnl, sd_netlink_message *m, Request return 1; } -static int link_request_static_route(Link *link, Route *route) { - assert(link); - assert(link->manager); - assert(route); - - if (!route_needs_convert(route)) - return link_request_route(link, route, false, &link->static_route_messages, - static_route_handler, NULL); - - log_route_debug(route, "Requesting", link->manager); - return link_queue_request_safe(link, REQUEST_TYPE_ROUTE, - route, NULL, route_hash_func, route_compare_func, - route_process_request, - &link->static_route_messages, static_route_handler, NULL); -} - static int link_request_wireguard_routes(Link *link, bool only_ipv4) { NetDev *netdev; Route *route; @@ -1326,7 +1147,7 @@ static int link_request_wireguard_routes(Link *link, bool only_ipv4) { if (only_ipv4 && route->family != AF_INET) continue; - r = link_request_static_route(link, route); + r = link_request_route(link, route, &link->static_route_messages, static_route_handler); if (r < 0) return r; } @@ -1350,7 +1171,7 @@ int link_request_static_routes(Link *link, bool only_ipv4) { if (only_ipv4 && route->family != AF_INET) continue; - r = link_request_static_route(link, route); + r = link_request_route(link, route, &link->static_route_messages, static_route_handler); if (r < 0) return r; } @@ -1372,14 +1193,15 @@ int link_request_static_routes(Link *link, bool only_ipv4) { void route_cancel_request(Route *route, Link *link) { assert(route); - - link = ASSERT_PTR(route->link ?: link); + Manager *manager = ASSERT_PTR(route->manager ?: + route->link ? route->link->manager : + ASSERT_PTR(link)->manager); if (!route_is_requesting(route)) return; Request *req; - if (route_get_request(link, route, &req) >= 0) + if (route_get_request(manager, route, &req) >= 0) request_detach(req); route_cancel_requesting(route); @@ -1387,13 +1209,14 @@ void route_cancel_request(Route *route, Link *link) { static int process_route_one( Manager *manager, - Link *link, uint16_t type, Route *in, const struct rta_cacheinfo *cacheinfo) { _cleanup_(route_freep) Route *tmp = in; + Request *req = NULL; Route *route = NULL; + Link *link = NULL; bool is_new = false, update_dhcp4; int r; @@ -1401,23 +1224,23 @@ static int process_route_one( assert(tmp); assert(IN_SET(type, RTM_NEWROUTE, RTM_DELROUTE)); - /* link may be NULL. This consumes 'in'. */ + (void) route_get(manager, NULL, tmp, &route); + (void) route_get_request(manager, tmp, &req); + (void) route_get_link(manager, tmp, &link); update_dhcp4 = link && tmp->family == AF_INET6 && tmp->dst_prefixlen == 0; - (void) route_get(manager, link, tmp, &route); - switch (type) { case RTM_NEWROUTE: if (!route) { - if (!manager->manage_foreign_routes) { + if (!manager->manage_foreign_routes && !(req && req->waiting_reply)) { route_enter_configured(tmp); log_route_debug(tmp, "Ignoring received", manager); return 0; } /* If we do not know the route, then save it. */ - r = route_add(manager, link, tmp); + r = route_add(manager, tmp); if (r < 0) { log_link_warning_errno(link, r, "Failed to remember foreign route, ignoring: %m"); return 0; @@ -1428,7 +1251,16 @@ static int process_route_one( } else /* Update remembered route with the received notification. */ - route->flags = tmp->flags; + route->nexthop.weight = tmp->nexthop.weight; + + /* Also update information that cannot be obtained through netlink notification. */ + if (req && req->waiting_reply) { + Route *rt = ASSERT_PTR(req->userdata); + + route->source = rt->source; + route->provider = rt->provider; + route->lifetime_usec = rt->lifetime_usec; + } route_enter_configured(route); log_route_debug(route, is_new ? "Received new" : "Received remembered", manager); @@ -1440,16 +1272,16 @@ static int process_route_one( case RTM_DELROUTE: if (route) { route_enter_removed(route); - if (route->state == 0) { - log_route_debug(route, "Forgetting", manager); - route_free(route); - } else - log_route_debug(route, "Removed", manager); + log_route_debug(route, "Forgetting removed", manager); + route_free(route); } else log_route_debug(tmp, manager->manage_foreign_routes ? "Kernel removed unknown" : "Ignoring received", manager); + if (req) + route_enter_removed(req->userdata); + break; default: @@ -1610,38 +1442,22 @@ int manager_rtnl_process_route(sd_netlink *rtnl, sd_netlink_message *message, Ma } has_cacheinfo = r >= 0; - Link *link = NULL; - if (tmp->nexthop.ifindex > 0) { - r = link_get_by_index(m, tmp->nexthop.ifindex, &link); - if (r < 0) { - /* when enumerating we might be out of sync, but we will - * get the route again, so just ignore it */ - if (!m->enumerating) - log_warning("rtnl: received route message for link (%i) we do not know about, ignoring", tmp->nexthop.ifindex); - return 0; - } + if (tmp->family == AF_INET || ordered_set_isempty(tmp->nexthops)) + return process_route_one(m, type, TAKE_PTR(tmp), has_cacheinfo ? &cacheinfo : NULL); + + RouteNextHop *nh; + ORDERED_SET_FOREACH(nh, tmp->nexthops) { + _cleanup_(route_freep) Route *dup = NULL; + + r = route_dup(tmp, nh, &dup); + if (r < 0) + return log_oom(); + + r = process_route_one(m, type, TAKE_PTR(dup), has_cacheinfo ? &cacheinfo : NULL); + if (r < 0) + return r; } - if (!route_needs_convert(tmp)) - return process_route_one(m, link, type, TAKE_PTR(tmp), has_cacheinfo ? &cacheinfo : NULL); - - _cleanup_(converted_routes_freep) ConvertedRoutes *converted = NULL; - r = route_convert(m, link, tmp, &converted); - if (r < 0) { - log_link_warning_errno(link, r, "rtnl: failed to convert received route, ignoring: %m"); - return 0; - } - - assert(r > 0); - assert(converted); - - for (size_t i = 0; i < converted->n; i++) - (void) process_route_one(m, - converted->links[i] ?: link, - type, - TAKE_PTR(converted->routes[i]), - has_cacheinfo ? &cacheinfo : NULL); - return 1; } diff --git a/src/network/networkd-route.h b/src/network/networkd-route.h index 49e78e9777..e86693e315 100644 --- a/src/network/networkd-route.h +++ b/src/network/networkd-route.h @@ -101,11 +101,9 @@ void link_foreignize_routes(Link *link); void route_cancel_request(Route *route, Link *link); int link_request_route( Link *link, - Route *route, - bool consume_object, + const Route *route, unsigned *message_counter, - route_netlink_handler_t netlink_handler, - Request **ret); + route_netlink_handler_t netlink_handler); int link_request_static_routes(Link *link, bool only_ipv4); int manager_rtnl_process_route(sd_netlink *rtnl, sd_netlink_message *message, Manager *m); From 10b08b6790f300fc4f7e9d4bfab038b13b153373 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 13 Jan 2024 16:02:44 +0900 Subject: [PATCH 3/5] network/route-nexthop: drop unnecessary link arguments These arguments are unused now. --- src/network/networkd-route-nexthop.c | 54 +++++++++++----------------- src/network/networkd-route-nexthop.h | 6 ++-- src/network/networkd-route.c | 18 +++++----- 3 files changed, 31 insertions(+), 47 deletions(-) diff --git a/src/network/networkd-route-nexthop.c b/src/network/networkd-route-nexthop.c index eb90e79f9d..1524b23188 100644 --- a/src/network/networkd-route-nexthop.c +++ b/src/network/networkd-route-nexthop.c @@ -341,7 +341,7 @@ int route_adjust_nexthops(Route *route, Link *link) { return true; /* updated */ } -int route_nexthop_get_link(Manager *manager, Link *link, const RouteNextHop *nh, Link **ret) { +int route_nexthop_get_link(Manager *manager, const RouteNextHop *nh, Link **ret) { assert(manager); assert(nh); @@ -350,20 +350,16 @@ int route_nexthop_get_link(Manager *manager, Link *link, const RouteNextHop *nh, if (nh->ifname) return link_get_by_name(manager, nh->ifname, ret); - if (link) { - if (ret) - *ret = link; - return 0; - } - return -ENOENT; } -static bool route_nexthop_is_ready_to_configure(const RouteNextHop *nh, Link *link, bool onlink) { - assert(nh); - assert(link); +static bool route_nexthop_is_ready_to_configure(const RouteNextHop *nh, Manager *manager, bool onlink) { + Link *link; - if (route_nexthop_get_link(link->manager, link, nh, &link)) + assert(nh); + assert(manager); + + if (route_nexthop_get_link(manager, nh, &link) < 0) return false; if (!link_is_ready_to_configure(link, /* allow_unmanaged = */ true)) @@ -377,13 +373,11 @@ static bool route_nexthop_is_ready_to_configure(const RouteNextHop *nh, Link *li return gateway_is_ready(link, onlink, nh->family, &nh->gw); } -int route_nexthops_is_ready_to_configure(const Route *route, Link *link) { +int route_nexthops_is_ready_to_configure(const Route *route, Manager *manager) { int r; assert(route); - assert(link); - - Manager *manager = ASSERT_PTR(link->manager); + assert(manager); if (route->nexthop_id != 0) { struct nexthop_grp *nhg; @@ -406,11 +400,11 @@ int route_nexthops_is_ready_to_configure(const Route *route, Link *link) { return true; if (ordered_set_isempty(route->nexthops)) - return route_nexthop_is_ready_to_configure(&route->nexthop, link, FLAGS_SET(route->flags, RTNH_F_ONLINK)); + return route_nexthop_is_ready_to_configure(&route->nexthop, manager, FLAGS_SET(route->flags, RTNH_F_ONLINK)); RouteNextHop *nh; ORDERED_SET_FOREACH(nh, route->nexthops) - if (!route_nexthop_is_ready_to_configure(nh, link, FLAGS_SET(route->flags, RTNH_F_ONLINK))) + if (!route_nexthop_is_ready_to_configure(nh, manager, FLAGS_SET(route->flags, RTNH_F_ONLINK))) return false; return true; @@ -481,7 +475,7 @@ int route_nexthops_to_string(const Route *route, char **ret) { return 0; } -static int append_nexthop_one(Link *link, const Route *route, const RouteNextHop *nh, struct rtattr **rta, size_t offset) { +static int append_nexthop_one(const Route *route, const RouteNextHop *nh, struct rtattr **rta, size_t offset) { struct rtnexthop *rtnh; struct rtattr *new_rta; int r; @@ -492,15 +486,6 @@ static int append_nexthop_one(Link *link, const Route *route, const RouteNextHop assert(rta); assert(*rta); - if (nh->ifindex <= 0) { - assert(link); - assert(link->manager); - - r = route_nexthop_get_link(link->manager, link, nh, &link); - if (r < 0) - return r; - } - new_rta = realloc(*rta, RTA_ALIGN((*rta)->rta_len) + RTA_SPACE(sizeof(struct rtnexthop))); if (!new_rta) return -ENOMEM; @@ -509,7 +494,7 @@ static int append_nexthop_one(Link *link, const Route *route, const RouteNextHop rtnh = (struct rtnexthop *)((uint8_t *) *rta + offset); *rtnh = (struct rtnexthop) { .rtnh_len = sizeof(*rtnh), - .rtnh_ifindex = nh->ifindex > 0 ? nh->ifindex : link->ifindex, + .rtnh_ifindex = nh->ifindex, .rtnh_hops = nh->weight, }; @@ -547,7 +532,7 @@ clear: return r; } -static int netlink_message_append_multipath_route(Link *link, const Route *route, sd_netlink_message *message) { +static int netlink_message_append_multipath_route(const Route *route, sd_netlink_message *message) { _cleanup_free_ struct rtattr *rta = NULL; size_t offset; int r; @@ -566,7 +551,7 @@ static int netlink_message_append_multipath_route(Link *link, const Route *route offset = (uint8_t *) RTA_DATA(rta) - (uint8_t *) rta; if (ordered_set_isempty(route->nexthops)) { - r = append_nexthop_one(link, route, &route->nexthop, &rta, offset); + r = append_nexthop_one(route, &route->nexthop, &rta, offset); if (r < 0) return r; @@ -575,7 +560,7 @@ static int netlink_message_append_multipath_route(Link *link, const Route *route ORDERED_SET_FOREACH(nh, route->nexthops) { struct rtnexthop *rtnh; - r = append_nexthop_one(link, route, nh, &rta, offset); + r = append_nexthop_one(route, nh, &rta, offset); if (r < 0) return r; @@ -587,7 +572,7 @@ static int netlink_message_append_multipath_route(Link *link, const Route *route return sd_netlink_message_append_data(message, RTA_MULTIPATH, RTA_DATA(rta), RTA_PAYLOAD(rta)); } -int route_nexthops_set_netlink_message(Link *link, const Route *route, sd_netlink_message *message) { +int route_nexthops_set_netlink_message(const Route *route, sd_netlink_message *message) { int r; assert(route); @@ -619,10 +604,11 @@ int route_nexthops_set_netlink_message(Link *link, const Route *route, sd_netlin return r; } - return sd_netlink_message_append_u32(message, RTA_OIF, route->nexthop.ifindex > 0 ? route->nexthop.ifindex : ASSERT_PTR(link)->ifindex); + assert(route->nexthop.ifindex > 0); + return sd_netlink_message_append_u32(message, RTA_OIF, route->nexthop.ifindex); } - return netlink_message_append_multipath_route(link, route, message); + return netlink_message_append_multipath_route(route, message); } static int route_parse_nexthops(Route *route, const struct rtnexthop *rtnh, size_t size) { diff --git a/src/network/networkd-route-nexthop.h b/src/network/networkd-route-nexthop.h index a63409c9e2..5e4602d3cd 100644 --- a/src/network/networkd-route-nexthop.h +++ b/src/network/networkd-route-nexthop.h @@ -38,12 +38,12 @@ int route_nexthops_copy(const Route *src, const RouteNextHop *nh, Route *dest); bool route_nexthops_needs_adjust(const Route *route); int route_adjust_nexthops(Route *route, Link *link); -int route_nexthop_get_link(Manager *manager, Link *link, const RouteNextHop *nh, Link **ret); -int route_nexthops_is_ready_to_configure(const Route *route, Link *link); +int route_nexthop_get_link(Manager *manager, const RouteNextHop *nh, Link **ret); +int route_nexthops_is_ready_to_configure(const Route *route, Manager *manager); int route_nexthops_to_string(const Route *route, char **ret); -int route_nexthops_set_netlink_message(Link *link, const Route *route, sd_netlink_message *message); +int route_nexthops_set_netlink_message(const Route *route, sd_netlink_message *message); int route_nexthops_read_netlink_message(Route *route, sd_netlink_message *message); int route_section_verify_nexthops(Route *route); diff --git a/src/network/networkd-route.c b/src/network/networkd-route.c index c3a0d30cca..a142719fb7 100644 --- a/src/network/networkd-route.c +++ b/src/network/networkd-route.c @@ -271,7 +271,7 @@ static int route_add(Manager *manager, Route *route) { assert(!route->wireguard); Link *link; - if (route_nexthop_get_link(manager, NULL, &route->nexthop, &link) >= 0) { + if (route_nexthop_get_link(manager, &route->nexthop, &link) >= 0) { r = set_ensure_put(&link->routes, &route_hash_ops, route); if (r < 0) return r; @@ -299,7 +299,7 @@ int route_get(Manager *manager, Link *link, const Route *route, Route **ret) { manager = ASSERT_PTR(ASSERT_PTR(link)->manager); assert(route); - if (route_nexthop_get_link(manager, NULL, &route->nexthop, &link) >= 0) + if (route_nexthop_get_link(manager, &route->nexthop, &link) >= 0) existing = set_get(link->routes, route); else existing = set_get(manager->routes, route); @@ -328,7 +328,7 @@ static int route_get_link(Manager *manager, const Route *route, Link **ret) { return link_get_by_index(manager, nh->ifindex, ret); } - return route_nexthop_get_link(manager, NULL, &route->nexthop, ret); + return route_nexthop_get_link(manager, &route->nexthop, ret); } static int route_get_request(Manager *manager, const Route *route, Request **ret) { @@ -440,14 +440,12 @@ static void log_route_debug(const Route *route, const char *str, Manager *manage strna(proto), strna(scope), strna(route_type_to_string(route->type)), strna(flags)); } -static int route_set_netlink_message(const Route *route, sd_netlink_message *m, Link *link) { +static int route_set_netlink_message(const Route *route, sd_netlink_message *m) { int r; assert(route); assert(m); - /* link may be NULL */ - /* rtmsg header (and relevant attributes) */ if (route->dst_prefixlen > 0) { r = netlink_message_append_in_addr_union(m, RTA_DST, route->family, &route->dst); @@ -516,7 +514,7 @@ static int route_set_netlink_message(const Route *route, sd_netlink_message *m, return r; /* nexthops */ - r = route_nexthops_set_netlink_message(link, route, m); + r = route_nexthops_set_netlink_message(route, m); if (r < 0) return r; @@ -563,7 +561,7 @@ int route_remove(Route *route) { if (r < 0) return log_link_warning_errno(link, r, "Could not create netlink message: %m"); - r = route_set_netlink_message(route, m, link); + r = route_set_netlink_message(route, m); if (r < 0) return log_link_warning_errno(link, r, "Could not fill netlink message: %m"); @@ -902,7 +900,7 @@ static int route_configure(const Route *route, uint32_t lifetime_sec, Link *link if (r < 0) return r; - r = route_set_netlink_message(route, m, link); + r = route_set_netlink_message(route, m); if (r < 0) return r; @@ -991,7 +989,7 @@ static int route_is_ready_to_configure(const Route *route, Link *link) { return r; } - return route_nexthops_is_ready_to_configure(route, link); + return route_nexthops_is_ready_to_configure(route, link->manager); } static int route_process_request(Request *req, Link *link, Route *route) { From e217da13803518fb9592a18b0ec7dade513878a0 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 13 Jan 2024 20:18:24 +0900 Subject: [PATCH 4/5] network/route: drop unnecessary ownership transfer Let's free Route objects on the caller side. --- src/network/networkd-dhcp4.c | 20 +++++++++----------- src/network/networkd-ndisc.c | 11 +++++------ 2 files changed, 14 insertions(+), 17 deletions(-) diff --git a/src/network/networkd-dhcp4.c b/src/network/networkd-dhcp4.c index 6d8dfcfe29..28702ff446 100644 --- a/src/network/networkd-dhcp4.c +++ b/src/network/networkd-dhcp4.c @@ -353,8 +353,7 @@ static int dhcp4_route_handler(sd_netlink *rtnl, sd_netlink_message *m, Request return 1; } -static int dhcp4_request_route(Route *in, Link *link) { - _cleanup_(route_freep) Route *route = in; +static int dhcp4_request_route(Route *route, Link *link) { struct in_addr server; Route *existing; int r; @@ -432,7 +431,7 @@ static int dhcp4_request_prefix_route(Link *link) { if (r < 0) return r; - return dhcp4_request_route(TAKE_PTR(route), link); + return dhcp4_request_route(route, link); } static int dhcp4_request_route_to_gateway(Link *link, const struct in_addr *gw) { @@ -457,15 +456,14 @@ static int dhcp4_request_route_to_gateway(Link *link, const struct in_addr *gw) route->prefsrc.in = address; route->scope = RT_SCOPE_LINK; - return dhcp4_request_route(TAKE_PTR(route), link); + return dhcp4_request_route(route, link); } static int dhcp4_request_route_auto( - Route *in, + Route *route, Link *link, const struct in_addr *gw) { - _cleanup_(route_freep) Route *route = in; struct in_addr address; int r; @@ -534,7 +532,7 @@ static int dhcp4_request_route_auto( route->prefsrc.in = address; } - return dhcp4_request_route(TAKE_PTR(route), link); + return dhcp4_request_route(route, link); } static int dhcp4_request_classless_static_or_static_routes(Link *link) { @@ -574,7 +572,7 @@ static int dhcp4_request_classless_static_or_static_routes(Link *link) { if (r < 0) return r; - r = dhcp4_request_route_auto(TAKE_PTR(route), link, &gw); + r = dhcp4_request_route_auto(route, link, &gw); if (r < 0) return r; } @@ -626,7 +624,7 @@ static int dhcp4_request_default_gateway(Link *link) { route->nexthop.gw.in = router; route->prefsrc.in = address; - return dhcp4_request_route(TAKE_PTR(route), link); + return dhcp4_request_route(route, link); } static int dhcp4_request_semi_static_routes(Link *link) { @@ -668,7 +666,7 @@ static int dhcp4_request_semi_static_routes(Link *link) { route->nexthop.gw.in = gw; - r = dhcp4_request_route(TAKE_PTR(route), link); + r = dhcp4_request_route(route, link); if (r < 0) return r; } @@ -711,7 +709,7 @@ static int dhcp4_request_routes_to_servers( route->dst.in = *dst; route->dst_prefixlen = 32; - r = dhcp4_request_route_auto(TAKE_PTR(route), link, &gw); + r = dhcp4_request_route_auto(route, link, &gw); if (r < 0) return r; } diff --git a/src/network/networkd-ndisc.c b/src/network/networkd-ndisc.c index 3885bde180..9f56fd23d4 100644 --- a/src/network/networkd-ndisc.c +++ b/src/network/networkd-ndisc.c @@ -172,8 +172,7 @@ static void ndisc_set_route_priority(Link *link, Route *route) { } } -static int ndisc_request_route(Route *in, Link *link, sd_ndisc_router *rt) { - _cleanup_(route_freep) Route *route = in; +static int ndisc_request_route(Route *route, Link *link, sd_ndisc_router *rt) { struct in6_addr router; uint8_t hop_limit = 0; uint32_t mtu = 0; @@ -328,7 +327,7 @@ static int ndisc_router_process_default(Link *link, sd_ndisc_router *rt) { route->nexthop.gw.in6 = gateway; route->lifetime_usec = lifetime_usec; - r = ndisc_request_route(TAKE_PTR(route), link, rt); + r = ndisc_request_route(route, link, rt); if (r < 0) return log_link_warning_errno(link, r, "Could not request default route: %m"); } @@ -352,7 +351,7 @@ static int ndisc_router_process_default(Link *link, sd_ndisc_router *rt) { route->pref = preference; route->lifetime_usec = lifetime_usec; - r = ndisc_request_route(TAKE_PTR(route), link, rt); + r = ndisc_request_route(route, link, rt); if (r < 0) return log_link_warning_errno(link, r, "Could not request gateway: %m"); } @@ -502,7 +501,7 @@ static int ndisc_router_process_onlink_prefix(Link *link, sd_ndisc_router *rt) { route->pref = preference; route->lifetime_usec = lifetime_usec; - r = ndisc_request_route(TAKE_PTR(route), link, rt); + r = ndisc_request_route(route, link, rt); if (r < 0) return log_link_warning_errno(link, r, "Could not request prefix route: %m"); @@ -635,7 +634,7 @@ static int ndisc_router_process_route(Link *link, sd_ndisc_router *rt) { route->dst_prefixlen = prefixlen; route->lifetime_usec = lifetime_usec; - r = ndisc_request_route(TAKE_PTR(route), link, rt); + r = ndisc_request_route(route, link, rt); if (r < 0) return log_link_warning_errno(link, r, "Could not request additional route: %m"); From cfe8975a3797312fcf86365f6495594dd8a7be8a Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 13 Jan 2024 13:32:27 +0900 Subject: [PATCH 5/5] network/route-nexthop: configure as a simple gateway when MultipathRoute= is specified only once The kernel handles such configuration completely equivalent to the case that Gateway= is specified. --- src/network/networkd-route-nexthop.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/network/networkd-route-nexthop.c b/src/network/networkd-route-nexthop.c index 1524b23188..2228c6280d 100644 --- a/src/network/networkd-route-nexthop.c +++ b/src/network/networkd-route-nexthop.c @@ -873,6 +873,16 @@ int route_section_verify_nexthops(Route *route) { "Ignoring [Route] section from line %u.", route->section->filename, route->section->line); + if (ordered_set_size(route->nexthops) == 1) { + _cleanup_(route_nexthop_freep) RouteNextHop *nh = ordered_set_steal_first(route->nexthops); + + route_nexthop_done(&route->nexthop); + route->nexthop = TAKE_STRUCT(*nh); + + assert(ordered_set_isempty(route->nexthops)); + route->nexthops = ordered_set_free(route->nexthops); + } + return 0; }