From 7a748ae556ba34a7e6c35ddde1fe06eedeb5b94b Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 28 Nov 2023 15:20:33 +0100 Subject: [PATCH] core: fix nm_netns_ip_route_ecmp_commit() to return regular single-hop route nm_netns_ip_route_ecmp_commit() returns the ECMP routes that didn't find a merge-partner. It expects NML3Cfg to configure such routes in platform. Note that these routes have a positive "weight", which was used for tracking the ECMP information in NML3Cfg and NMNetns. But in kernel, single-hop routes don't have a weight. All routes in NMPlatform will have a zero weight. While it somewhat works to call nm_platform_ip_route_add() with a single-hop route of non-zero weight, the result will be a different(!) route. This means for example that nm_platform_ip_route_sync() will search the NMPlatform cache for whether the routes exist, but in the cache single-hop routes with positive weight never exist. Previously this happened and nm_platform_ip_route_sync() would not delete those routes when it should. It's also important because NML3Cfg tracks the object states of routes that it wants to configure, vs routes in kernel (NMPlatform cache). The route in kernel has a weight of zero. The route we want to configure must also have a weight of zero. The solution is that nm_netns_ip_route_ecmp_commit() does not tell NML3Cfg to add a route, that cannot be added. Instead, it must first normalize the weight to zero, to have a "regular" single-hop route. Fixes: 5b5ce4268211 ('nm-netns: track ECMP routes') --- src/core/nm-netns.c | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/src/core/nm-netns.c b/src/core/nm-netns.c index ad156f99ed..9cb441531b 100644 --- a/src/core/nm-netns.c +++ b/src/core/nm-netns.c @@ -910,6 +910,8 @@ nm_netns_ip_route_ecmp_commit(NMNetns *self, } if (route->n_nexthops <= 1) { + NMPObject *route_clone; + /* This is a single hop route. Return it to the caller. */ if (!*out_singlehop_routes) { /* Note that the returned array does not own a reference. This @@ -918,7 +920,28 @@ nm_netns_ip_route_ecmp_commit(NMNetns *self, *out_singlehop_routes = g_ptr_array_new_with_free_func((GDestroyNotify) nmp_object_unref); } - g_ptr_array_add(*out_singlehop_routes, (gpointer) nmp_object_ref(route_obj)); + + /* We have here a IPv4 single-hop route. For internal tracking purposes, + * this route has a positive "weight" (which was used to mark it as a candidate + * for ECMP merging). Now we want to return this route to NML3Cfg and add it + * as regular single-hop routes. + * + * A single-hop route in kernel always has a "weight" of zero. This route + * cannot be added as-is. Well, if we would, then the result would be + * a different(!) route (with a zero "weight"). + * + * Anticipate that and normalize the route now to be a regular single-hop + * route (with weight zero). nm_platform_ip_route_normalize() does that. + * We really want to return a regular route here, not the route with a positive + * weight that exists for internal tracking purposes. + */ + nm_assert(NMP_OBJECT_GET_TYPE(route_obj) == NMP_OBJECT_TYPE_IP4_ROUTE); + nm_assert(route_obj->ip4_route.weight > 0u); + + route_clone = nmp_object_clone(route_obj, FALSE); + nm_platform_ip_route_normalize(AF_INET, NMP_OBJECT_CAST_IP_ROUTE(route_clone)); + g_ptr_array_add(*out_singlehop_routes, route_clone); + if (changed) { _LOGT("ecmp-route: single-hop %s", nmp_object_to_string(route_obj,