diff options
author | Yu Watanabe <watanabe.yu+github@gmail.com> | 2024-01-16 14:36:29 +0100 |
---|---|---|
committer | Yu Watanabe <watanabe.yu+github@gmail.com> | 2024-01-19 11:20:29 +0100 |
commit | 5a18697d61047e8c189bd280b6748f1159dcc385 (patch) | |
tree | f549016d01f0e2704c812682f1d47ea683100e20 | |
parent | network/route-nexthop: fix route_nexthop_copy() (diff) | |
download | systemd-5a18697d61047e8c189bd280b6748f1159dcc385.tar.xz systemd-5a18697d61047e8c189bd280b6748f1159dcc385.zip |
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.
-rw-r--r-- | src/network/networkd-dhcp-prefix-delegation.c | 8 | ||||
-rw-r--r-- | src/network/networkd-dhcp4.c | 3 | ||||
-rw-r--r-- | src/network/networkd-ndisc.c | 3 | ||||
-rw-r--r-- | src/network/networkd-queue.c | 4 | ||||
-rw-r--r-- | src/network/networkd-route-nexthop.c | 66 | ||||
-rw-r--r-- | src/network/networkd-route.c | 636 | ||||
-rw-r--r-- | src/network/networkd-route.h | 6 |
7 files changed, 283 insertions, 443 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; - - assert(in); +int route_get(Manager *manager, Link *link, const Route *route, Route **ret) { + Route *existing; - if (route_type_is_reject(in)) { - if (!manager) - return -ENOENT; - - route = set_get(manager->routes, in); - } else { - if (!link) - return -ENOENT; + if (!manager) + manager = ASSERT_PTR(ASSERT_PTR(link)->manager); + assert(route); - 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; - - 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; - } + if (route->family == AF_INET || ordered_set_isempty(route->nexthops)) + (void) link_unmark_route(link, route, NULL); - for (size_t i = 0; i < converted->n; i++) - if (route_get(manager, NULL, converted->routes[i], &existing) >= 0) - route_unmark(existing); + else { + RouteNextHop *nh; + ORDERED_SET_FOREACH(nh, route->nexthops) + (void) link_unmark_route(link, route, nh); + } } } } @@ -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); - - if (route->lifetime_usec == 0) { - if (consume_object) - route_free(route); - /* The requested route is outdated. Let's remove it. */ - return route_remove_and_drop(existing); - } - - 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_dup(route, nh, &tmp); + if (r < 0) + return r; - r = route_add(link->manager, link, tmp); - if (r < 0) - return r; + r = route_adjust_nexthops(tmp, link); + 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); - } + if (route_get(link->manager, link, tmp, &existing) >= 0) + /* Copy state for logging below. */ + tmp->state = existing->state; - 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,37 +1442,21 @@ 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 (!route_needs_convert(tmp)) - return process_route_one(m, link, type, TAKE_PTR(tmp), has_cacheinfo ? &cacheinfo : NULL); + if (tmp->family == AF_INET || ordered_set_isempty(tmp->nexthops)) + return process_route_one(m, 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; - } + RouteNextHop *nh; + ORDERED_SET_FOREACH(nh, tmp->nexthops) { + _cleanup_(route_freep) Route *dup = NULL; - assert(r > 0); - assert(converted); + r = route_dup(tmp, nh, &dup); + if (r < 0) + return log_oom(); - 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); + r = process_route_one(m, type, TAKE_PTR(dup), has_cacheinfo ? &cacheinfo : NULL); + if (r < 0) + return r; + } 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); |