From 5a18697d61047e8c189bd280b6748f1159dcc385 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 16 Jan 2024 22:36:29 +0900 Subject: 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-route-nexthop.c | 66 ++++++++++++++++++++++++++---------- 1 file changed, 48 insertions(+), 18 deletions(-) (limited to 'src/network/networkd-route-nexthop.c') 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; } -- cgit v1.2.3