From 30d31eea64e4d50019029849515ecb7adfb18f37 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Thu, 20 Feb 2025 13:43:50 +0100 Subject: [PATCH 1/3] core: split nm_netns_watcher_remove_all() The name suggests that the function always removes all the watchers with the given tag; instead it removes only "dirty" ones when the "all" parameter is FALSE. Split the function in two variants. (cherry picked from commit b6e67c6abc8cf2907fb4f25af291710b41d2577d) (cherry picked from commit a301c259f20fcf6f90c33a7c94b77ab401d6ec56) --- src/core/nm-l3cfg.c | 10 +++------- src/core/nm-netns.c | 19 +++++++++++++++++-- src/core/nm-netns.h | 4 ++-- 3 files changed, 22 insertions(+), 11 deletions(-) diff --git a/src/core/nm-l3cfg.c b/src/core/nm-l3cfg.c index a7189d3e60..ae2a7938e7 100644 --- a/src/core/nm-l3cfg.c +++ b/src/core/nm-l3cfg.c @@ -4764,7 +4764,7 @@ next: } out: - nm_netns_watcher_remove_all(self->priv.netns, TAG, FALSE); + nm_netns_watcher_remove_dirty(self->priv.netns, TAG); } /*****************************************************************************/ @@ -5636,12 +5636,8 @@ finalize(GObject *object) gboolean changed; if (self->priv.netns) { - nm_netns_watcher_remove_all(self->priv.netns, - _NETNS_WATCHER_IP_ADDR_TAG(self, AF_INET), - TRUE); - nm_netns_watcher_remove_all(self->priv.netns, - _NETNS_WATCHER_IP_ADDR_TAG(self, AF_INET6), - TRUE); + nm_netns_watcher_remove_all(self->priv.netns, _NETNS_WATCHER_IP_ADDR_TAG(self, AF_INET)); + nm_netns_watcher_remove_all(self->priv.netns, _NETNS_WATCHER_IP_ADDR_TAG(self, AF_INET6)); } nm_prioq_destroy(&self->priv.p->failedobj_prioq); diff --git a/src/core/nm-netns.c b/src/core/nm-netns.c index ed33d33676..4d28656142 100644 --- a/src/core/nm-netns.c +++ b/src/core/nm-netns.c @@ -1369,8 +1369,8 @@ nm_netns_watcher_remove_handle(NMNetns *self, NMNetnsWatcherHandle *handle) g_object_unref(self); } -void -nm_netns_watcher_remove_all(NMNetns *self, gconstpointer tag, gboolean all) +static void +watcher_remove(NMNetns *self, gconstpointer tag, gboolean all) { NMNetnsPrivate *priv; WatcherByTag *watcher_by_tag; @@ -1420,6 +1420,21 @@ nm_netns_watcher_remove_all(NMNetns *self, gconstpointer tag, gboolean all) } } +void +nm_netns_watcher_remove_all(NMNetns *self, gconstpointer tag) +{ + watcher_remove(self, tag, TRUE); +} + +/* Similar to nm_netns_watcher_remove_all(), but removes only watchers + * that were marked as "dirty" in a previous call of this function and were + * not added back via nm_netns_watcher_add() in the meantime. */ +void +nm_netns_watcher_remove_dirty(NMNetns *self, gconstpointer tag) +{ + watcher_remove(self, tag, FALSE); +} + /*****************************************************************************/ static void diff --git a/src/core/nm-netns.h b/src/core/nm-netns.h index 7725ae79b4..43e9c7818a 100644 --- a/src/core/nm-netns.h +++ b/src/core/nm-netns.h @@ -98,7 +98,7 @@ void nm_netns_watcher_add(NMNetns *self, NMNetnsWatcherCallback callback, gpointer user_data); -void -nm_netns_watcher_remove_all(NMNetns *self, gconstpointer tag, gboolean all /* or only dirty */); +void nm_netns_watcher_remove_all(NMNetns *self, gconstpointer tag); +void nm_netns_watcher_remove_dirty(NMNetns *self, gconstpointer tag); #endif /* __NM_NETNS_H__ */ From 9622c04e6aa2989abd4ac6ac6c52762d818a4c71 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Mon, 17 Feb 2025 11:17:14 +0100 Subject: [PATCH 2/3] l3cfg: wait for the address before configuring an MPTCP endpoint Skip the configuration of the MPTCP endpoint when the address is in the l3cd but is not yet configured in the platform. This typically happens when IPv4 DAD is enabled and the address is being probed. If we configure the endpoint without the address set, the kernel will try to use the endpoint immediately but it will fail. Then, the endpoint will not be used ever again after the address is added. (cherry picked from commit 6bf859af792b14fb9afab7675de1fd6d40e23f78) (cherry picked from commit 2c5a51201d80cd3aa7287144ec46af0ffac70311) --- src/core/nm-l3cfg.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/core/nm-l3cfg.c b/src/core/nm-l3cfg.c index ae2a7938e7..2fb8d0a14f 100644 --- a/src/core/nm-l3cfg.c +++ b/src/core/nm-l3cfg.c @@ -4855,6 +4855,8 @@ _l3_commit_mptcp_af(NML3Cfg *self, self->priv.p->combined_l3cd_commited, addr_family, (const NMPlatformIPAddress **) &addr) { + const NMPObject *obj; + /* We want to evaluate the with-{loopback,link_local}-{4,6} flags based on the actual * ifa_scope that the address will have once we configure it. * "addr" is an address we want to configure, we expect that it will @@ -4881,6 +4883,16 @@ _l3_commit_mptcp_af(NML3Cfg *self, break; } + obj = nm_platform_ip_address_get(self->priv.platform, + addr_family, + self->priv.ifindex, + addr); + if (!obj) { + /* The address is not yet configured in platform, typically due to + * IPv4 DAD; skip it for now otherwise the kernel will try to use + * the endpoint, it will fail, and it will never try it again. */ + goto skip_addr; + } a.addr = nm_ip_addr_init(addr_family, addr->ax.address_ptr); /* We track the address with different priorities, that depends From cda3fc918114f3236a726af025b1a4de2ae234be Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Mon, 17 Feb 2025 13:25:01 +0100 Subject: [PATCH 3/3] l3cfg: only add MPTCP endpoints for non-tentative IPv6 addresses An IPv6 endpoint is not usable until the address is non-tentative. Add a mechanism to wait until the address is ready. (cherry picked from commit 227cd6307b3c82df76c277a610a38b081f674eee) (cherry picked from commit ceef38d9a5f5916b577ee88075dd49f9f1f43136) --- src/core/nm-l3cfg.c | 47 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/src/core/nm-l3cfg.c b/src/core/nm-l3cfg.c index 2fb8d0a14f..0d20bf44a5 100644 --- a/src/core/nm-l3cfg.c +++ b/src/core/nm-l3cfg.c @@ -370,6 +370,8 @@ G_DEFINE_TYPE(NML3Cfg, nm_l3cfg, G_TYPE_OBJECT) #define _NETNS_WATCHER_IP_ADDR_TAG(self, addr_family) \ ((gconstpointer) & (((char *) self)[1 + NM_IS_IPv4(addr_family)])) +#define _NETNS_WATCHER_MPTCP_IPV6_TAG(self) ((gconstpointer) & (((char *) self)[3])) + /*****************************************************************************/ #define _NMLOG_DOMAIN LOGD_CORE @@ -4777,6 +4779,30 @@ _global_tracker_mptcp_untrack(NML3Cfg *self, int addr_family) TRUE); } +static void +mptcp_ipv6_addr_cb(NMNetns *netns, + NMNetnsWatcherType watcher_type, + const NMNetnsWatcherData *watcher_data, + gconstpointer tag, + const NMNetnsWatcherEventData *event_data, + gpointer user_data) +{ + NML3Cfg *self = user_data; + + if (event_data->ip_addr.change_type == NM_PLATFORM_SIGNAL_REMOVED) + return; + + nm_assert(NMP_OBJECT_GET_TYPE(event_data->ip_addr.obj) == NMP_OBJECT_TYPE_IP6_ADDRESS); + + if (event_data->ip_addr.obj->ip6_address.n_ifa_flags & IFA_F_TENTATIVE) + return; + + /* We are inside the handler for a platform event, we should not + * perform other operations on platform synchronously. Schedule a + * commit in a idle handler. */ + nm_l3cfg_commit_on_idle_schedule(self, NM_L3_CFG_COMMIT_TYPE_AUTO); +} + static gboolean _l3_commit_mptcp_af(NML3Cfg *self, NML3CfgCommitType commit_type, @@ -4893,6 +4919,24 @@ _l3_commit_mptcp_af(NML3Cfg *self, * the endpoint, it will fail, and it will never try it again. */ goto skip_addr; } + if (!IS_IPv4 && (obj->ip6_address.n_ifa_flags & IFA_F_TENTATIVE)) { + NMNetnsWatcherData watcher_data = {}; + + /* The endpoint is not usable when the address is tentative. + * Watch the address until it becomes non-tentative and then + * schedule a new commit. */ + watcher_data.ip_addr.addr.addr_family = AF_INET6; + watcher_data.ip_addr.addr.addr.addr6 = addr->a6.address; + + nm_netns_watcher_add(self->priv.netns, + NM_NETNS_WATCHER_TYPE_IP_ADDR, + &watcher_data, + _NETNS_WATCHER_MPTCP_IPV6_TAG(self), + mptcp_ipv6_addr_cb, + self); + goto skip_addr; + } + a.addr = nm_ip_addr_init(addr_family, addr->ax.address_ptr); /* We track the address with different priorities, that depends @@ -4921,6 +4965,8 @@ skip_addr: } } + nm_netns_watcher_remove_dirty(self->priv.netns, _NETNS_WATCHER_MPTCP_IPV6_TAG(self)); + if (!any_tracked) { /* We need to make it known that this ifindex is used. Track a dummy object. */ if (nmp_global_tracker_track( @@ -5650,6 +5696,7 @@ finalize(GObject *object) if (self->priv.netns) { nm_netns_watcher_remove_all(self->priv.netns, _NETNS_WATCHER_IP_ADDR_TAG(self, AF_INET)); nm_netns_watcher_remove_all(self->priv.netns, _NETNS_WATCHER_IP_ADDR_TAG(self, AF_INET6)); + nm_netns_watcher_remove_all(self->priv.netns, _NETNS_WATCHER_MPTCP_IPV6_TAG(self)); } nm_prioq_destroy(&self->priv.p->failedobj_prioq);