From 151b2bed361af5209f605c39dc074caffb72f2bf Mon Sep 17 00:00:00 2001 From: Fernando Fernandez Mancera Date: Tue, 18 Oct 2022 18:53:46 +0200 Subject: [PATCH] platform: pass extra_hops to ip_route_add function When adding a new route we need to consider it contains extra nexthops i.e it is a ECMP route. As we cannot modify the NMPObject once created, we need to pass the extra nexthops as an argument. We cannot use the original NMPObject because normalization is happening during when adding the route. --- src/core/platform/nm-fake-platform.c | 23 ++++---- src/core/platform/tests/test-common.c | 4 +- src/core/platform/tests/test-route.c | 8 +-- src/libnm-platform/nm-linux-platform.c | 16 ++---- src/libnm-platform/nm-platform.c | 73 ++++++++++++++++++-------- src/libnm-platform/nm-platform.h | 10 ++-- 6 files changed, 78 insertions(+), 56 deletions(-) diff --git a/src/core/platform/nm-fake-platform.c b/src/core/platform/nm-fake-platform.c index 8740dbb908..f85a02e159 100644 --- a/src/core/platform/nm-fake-platform.c +++ b/src/core/platform/nm-fake-platform.c @@ -1092,10 +1092,7 @@ object_delete(NMPlatform *platform, const NMPObject *obj) } static int -ip_route_add(NMPlatform *platform, - NMPNlmFlags flags, - int addr_family, - const NMPlatformIPRoute *route) +ip_route_add(NMPlatform *platform, NMPNlmFlags flags, NMPObject *obj_stack) { NMDedupMultiIter iter; nm_auto_nmpobj NMPObject *obj = NULL; @@ -1109,23 +1106,29 @@ ip_route_add(NMPlatform *platform, NMPlatformIPRoute *r = NULL; NMPlatformIP4Route *r4 = NULL; NMPlatformIP6Route *r6 = NULL; + int addr_family; gboolean has_same_weak_id; gboolean only_dirty; guint16 nlmsgflags; - g_assert(NM_IN_SET(addr_family, AF_INET, AF_INET6)); + g_assert(NM_IN_SET(NMP_OBJECT_GET_TYPE(obj_stack), + NMP_OBJECT_TYPE_IP4_ROUTE, + NMP_OBJECT_TYPE_IP6_ROUTE)); + + addr_family = NMP_OBJECT_GET_ADDR_FAMILY(obj_stack); flags = NM_FLAGS_UNSET(flags, NMP_NLM_FLAG_SUPPRESS_NETLINK_FAILURE); /* currently, only replace is implemented. */ g_assert(flags == NMP_NLM_FLAG_REPLACE); - obj = nmp_object_new(addr_family == AF_INET ? NMP_OBJECT_TYPE_IP4_ROUTE - : NMP_OBJECT_TYPE_IP6_ROUTE, - (const NMPlatformObject *) route); - r = NMP_OBJECT_CAST_IP_ROUTE(obj); + if (NMP_OBJECT_GET_TYPE(obj_stack) == NMP_OBJECT_TYPE_IP4_ROUTE + && obj_stack->ip4_route.n_nexthops == 0 && obj_stack->ip4_route.ifindex > 0) + obj_stack->ip4_route.n_nexthops = 1; - nm_platform_ip_route_normalize(addr_family, r); + obj = nmp_object_clone(obj_stack, FALSE); + + r = NMP_OBJECT_CAST_IP_ROUTE(obj); switch (addr_family) { case AF_INET: diff --git a/src/core/platform/tests/test-common.c b/src/core/platform/tests/test-common.c index 09e7bd6c9a..5049bc5189 100644 --- a/src/core/platform/tests/test-common.c +++ b/src/core/platform/tests/test-common.c @@ -1250,8 +1250,8 @@ nmtstp_ip4_route_add(NMPlatform *platform, route.metric = metric; route.mss = mss; - g_assert( - NMTST_NM_ERR_SUCCESS(nm_platform_ip4_route_add(platform, NMP_NLM_FLAG_REPLACE, &route))); + g_assert(NMTST_NM_ERR_SUCCESS( + nm_platform_ip4_route_add(platform, NMP_NLM_FLAG_REPLACE, &route, NULL))); } void diff --git a/src/core/platform/tests/test-route.c b/src/core/platform/tests/test-route.c index c441f58410..1bf6eb5162 100644 --- a/src/core/platform/tests/test-route.c +++ b/src/core/platform/tests/test-route.c @@ -713,7 +713,7 @@ test_ip4_route_options(gconstpointer test_data) for (i = 0; i < rts_n; i++) g_assert(NMTST_NM_ERR_SUCCESS( - nm_platform_ip4_route_add(NM_PLATFORM_GET, NMP_NLM_FLAG_REPLACE, &rts_add[i]))); + nm_platform_ip4_route_add(NM_PLATFORM_GET, NMP_NLM_FLAG_REPLACE, &rts_add[i], NULL))); for (i = 0; i < rts_n; i++) { rts_cmp[i] = rts_add[i]; @@ -988,8 +988,8 @@ again_find_idx: order_idx[order_len++] = idx; r->ifindex = iface_data[idx].ifindex; - g_assert( - NMTST_NM_ERR_SUCCESS(nm_platform_ip4_route_add(platform, NMP_NLM_FLAG_APPEND, r))); + g_assert(NMTST_NM_ERR_SUCCESS( + nm_platform_ip4_route_add(platform, NMP_NLM_FLAG_APPEND, r, NULL))); } else { i = nmtst_get_rand_uint32() % order_len; idx = order_idx[i]; @@ -1936,7 +1936,7 @@ test_blackhole(gconstpointer test_data) nm_platform_ip_route_normalize(addr_family, &rr.rx); if (IS_IPv4) - r = nm_platform_ip4_route_add(NM_PLATFORM_GET, NMP_NLM_FLAG_APPEND, &rr.r4); + r = nm_platform_ip4_route_add(NM_PLATFORM_GET, NMP_NLM_FLAG_APPEND, &rr.r4, NULL); else r = nm_platform_ip6_route_add(NM_PLATFORM_GET, NMP_NLM_FLAG_APPEND, &rr.r6); diff --git a/src/libnm-platform/nm-linux-platform.c b/src/libnm-platform/nm-linux-platform.c index 4ea67c1791..a7442e4acc 100644 --- a/src/libnm-platform/nm-linux-platform.c +++ b/src/libnm-platform/nm-linux-platform.c @@ -9427,25 +9427,15 @@ ip6_address_delete(NMPlatform *platform, int ifindex, struct in6_addr addr, guin /*****************************************************************************/ static int -ip_route_add(NMPlatform *platform, - NMPNlmFlags flags, - int addr_family, - const NMPlatformIPRoute *route) +ip_route_add(NMPlatform *platform, NMPNlmFlags flags, NMPObject *obj_stack) { nm_auto_nlmsg struct nl_msg *nlmsg = NULL; - NMPObject obj; - nmp_object_stackinit(&obj, - NMP_OBJECT_TYPE_IP_ROUTE(NM_IS_IPv4(addr_family)), - (const NMPlatformObject *) route); - - nm_platform_ip_route_normalize(addr_family, NMP_OBJECT_CAST_IP_ROUTE(&obj)); - - nlmsg = _nl_msg_new_route(RTM_NEWROUTE, flags & NMP_NLM_FLAG_FMASK, &obj); + nlmsg = _nl_msg_new_route(RTM_NEWROUTE, flags & NMP_NLM_FLAG_FMASK, obj_stack); if (!nlmsg) g_return_val_if_reached(-NME_BUG); return do_add_addrroute(platform, - &obj, + obj_stack, nlmsg, NM_FLAGS_HAS(flags, NMP_NLM_FLAG_SUPPRESS_NETLINK_FAILURE)); } diff --git a/src/libnm-platform/nm-platform.c b/src/libnm-platform/nm-platform.c index 188db58ed0..590c4c8ecd 100644 --- a/src/libnm-platform/nm-platform.c +++ b/src/libnm-platform/nm-platform.c @@ -5164,47 +5164,73 @@ nm_platform_ip_route_normalize(int addr_family, NMPlatformIPRoute *route) } static int -_ip_route_add(NMPlatform *self, NMPNlmFlags flags, int addr_family, gconstpointer route) +_ip_route_add(NMPlatform *self, NMPNlmFlags flags, NMPObject *obj_stack) { char sbuf[NM_UTILS_TO_STRING_BUFFER_SIZE]; int ifindex; _CHECK_SELF(self, klass, FALSE); - nm_assert(route); - nm_assert(NM_IN_SET(addr_family, AF_INET, AF_INET6)); + /* The caller already ensures that this is a stack allocated copy, that + * - stays alive for the duration of the call. + * - that the ip_route_add() implementation is allowed to modify. + */ + nm_assert(obj_stack); + nm_assert(NMP_OBJECT_IS_STACKINIT(obj_stack)); + nm_assert(NM_IN_SET(NMP_OBJECT_GET_TYPE(obj_stack), + NMP_OBJECT_TYPE_IP4_ROUTE, + NMP_OBJECT_TYPE_IP6_ROUTE)); + + nm_assert(NMP_OBJECT_GET_TYPE(obj_stack) != NMP_OBJECT_TYPE_IP4_ROUTE + || obj_stack->ip4_route.n_nexthops <= 1u || obj_stack->_ip4_route.extra_nexthops); + + nm_platform_ip_route_normalize(NMP_OBJECT_GET_ADDR_FAMILY((obj_stack)), + NMP_OBJECT_CAST_IP_ROUTE(obj_stack)); + + ifindex = obj_stack->ip_route.ifindex; - ifindex = ((const NMPlatformIPRoute *) route)->ifindex; _LOG3D("route: %-10s IPv%c route: %s", _nmp_nlm_flag_to_string(flags & NMP_NLM_FLAG_FMASK), - nm_utils_addr_family_to_char(addr_family), - NM_IS_IPv4(addr_family) ? nm_platform_ip4_route_to_string(route, sbuf, sizeof(sbuf)) - : nm_platform_ip6_route_to_string(route, sbuf, sizeof(sbuf))); + nm_utils_addr_family_to_char(NMP_OBJECT_GET_ADDR_FAMILY(obj_stack)), + nmp_object_to_string(obj_stack, NMP_OBJECT_TO_STRING_PUBLIC, sbuf, sizeof(sbuf))); - return klass->ip_route_add(self, flags, addr_family, route); + /* At this point, we pass "obj_stack" to the klass->ip_route_add() implementation. + * The callee can rely on: + * - the object being normalized and validated. + * - staying fully alive until the function returns. In this case it + * is stack allocated (and the potential "extra_nexthops" array is + * guaranteed to stay alive too). + */ + return klass->ip_route_add(self, flags, obj_stack); } int -nm_platform_ip_route_add(NMPlatform *self, NMPNlmFlags flags, const NMPObject *route) +nm_platform_ip_route_add(NMPlatform *self, NMPNlmFlags flags, const NMPObject *obj) { - int addr_family; + nm_auto_nmpobj const NMPObject *obj_keep_alive = NULL; + NMPObject obj_stack; - switch (NMP_OBJECT_GET_TYPE(route)) { - case NMP_OBJECT_TYPE_IP4_ROUTE: - addr_family = AF_INET; - break; - case NMP_OBJECT_TYPE_IP6_ROUTE: - addr_family = AF_INET6; - break; - default: - g_return_val_if_reached(FALSE); + nm_assert( + NM_IN_SET(NMP_OBJECT_GET_TYPE(obj), NMP_OBJECT_TYPE_IP4_ROUTE, NMP_OBJECT_TYPE_IP6_ROUTE)); + + nmp_object_stackinit(&obj_stack, NMP_OBJECT_GET_TYPE(obj), &obj->ip_route); + + if (NMP_OBJECT_GET_TYPE(obj) == NMP_OBJECT_TYPE_IP4_ROUTE && obj->ip4_route.n_nexthops > 1u) { + /* Ensure @obj stays alive, so we can alias extra_nexthops from the stackallocated + * @obj_stack. */ + nm_assert(obj->_ip4_route.extra_nexthops); + obj_keep_alive = nmp_object_ref(obj); + obj_stack._ip4_route.extra_nexthops = obj->_ip4_route.extra_nexthops; } - return _ip_route_add(self, flags, addr_family, NMP_OBJECT_CAST_IP_ROUTE(route)); + return _ip_route_add(self, flags, &obj_stack); } int -nm_platform_ip4_route_add(NMPlatform *self, NMPNlmFlags flags, const NMPlatformIP4Route *route) +nm_platform_ip4_route_add(NMPlatform *self, + NMPNlmFlags flags, + const NMPlatformIP4Route *route, + const NMPlatformIP4RtNextHop *extra_nexthops) { gs_free NMPlatformIP4RtNextHop *extra_nexthops_free = NULL; NMPObject obj; @@ -5235,7 +5261,10 @@ nm_platform_ip4_route_add(NMPlatform *self, NMPNlmFlags flags, const NMPlatformI int nm_platform_ip6_route_add(NMPlatform *self, NMPNlmFlags flags, const NMPlatformIP6Route *route) { - return _ip_route_add(self, flags, AF_INET6, route); + NMPObject obj; + + nmp_object_stackinit(&obj, NMP_OBJECT_TYPE_IP6_ROUTE, (const NMPlatformObject *) route); + return _ip_route_add(self, flags, &obj); } gboolean diff --git a/src/libnm-platform/nm-platform.h b/src/libnm-platform/nm-platform.h index 6429e1d923..3a6f9a9f2c 100644 --- a/src/libnm-platform/nm-platform.h +++ b/src/libnm-platform/nm-platform.h @@ -1206,10 +1206,7 @@ typedef struct { struct in6_addr address, guint8 plen); - int (*ip_route_add)(NMPlatform *self, - NMPNlmFlags flags, - int addr_family, - const NMPlatformIPRoute *route); + int (*ip_route_add)(NMPlatform *self, NMPNlmFlags flags, NMPObject *obj_stack); int (*ip_route_get)(NMPlatform *self, int addr_family, gconstpointer address, @@ -2220,7 +2217,10 @@ nm_platform_ip_route_get_gateway(int addr_family, const NMPlatformIPRoute *route } int nm_platform_ip_route_add(NMPlatform *self, NMPNlmFlags flags, const NMPObject *route); -int nm_platform_ip4_route_add(NMPlatform *self, NMPNlmFlags flags, const NMPlatformIP4Route *route); +int nm_platform_ip4_route_add(NMPlatform *self, + NMPNlmFlags flags, + const NMPlatformIP4Route *route, + const NMPlatformIP4RtNextHop *extra_nexthops); int nm_platform_ip6_route_add(NMPlatform *self, NMPNlmFlags flags, const NMPlatformIP6Route *route); GPtrArray *nm_platform_ip_route_get_prune_list(NMPlatform *self,