From 5ff08fbbea0acbc17bcb9c69901902456547515c Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 5 May 2022 11:18:27 +0200 Subject: [PATCH 1/3] glib-aux: add nm_g_array_data() helper It's annoying to do (arr ? arr->data : NULL) Especially, because usually you'd need to cast the above (which would have type (char *)). --- src/libnm-glib-aux/nm-shared-utils.h | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/libnm-glib-aux/nm-shared-utils.h b/src/libnm-glib-aux/nm-shared-utils.h index ec57190e5..80f6bcddb 100644 --- a/src/libnm-glib-aux/nm-shared-utils.h +++ b/src/libnm-glib-aux/nm-shared-utils.h @@ -2165,6 +2165,12 @@ char *nm_utils_g_slist_strlist_join(const GSList *a, const char *separator); /*****************************************************************************/ +static inline gpointer +nm_g_array_data(const GArray *arr) +{ + return arr ? arr->data : NULL; +} + static inline guint nm_g_array_len(const GArray *arr) { From 518f6124c6476e5f91b30b7d5583f494e84fd936 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 5 May 2022 10:22:20 +0200 Subject: [PATCH 2/3] l3cfg: fix clearing IPv6 temporary addresses to avoid stale addresses IPv6 temporary addresses are configured by kernel, with the "ipv6.ip6-privacy" setting ("use_tempaddr" sysctl) and the IFA_F_MANAGETEMPADDR flag. As such, the idea was that during reapply we would not remove them. However, that is wrong. The only case when we want to keep those addresses, is if during reapply we are going to configure the same primary address (with mngtmpaddr flag) again. Otherwise, theses addresses must always go away. This is quite serious. This not only affects Reapply. Also during disconnect we clear IP configuration via l3cfg. Have an ethernet profile active with "ipv6.ip6-privacy". Unplug the cable, the device disconnects but the temporary IPv6 address is not cleared. As such, nm_device_generate_connection() will now generate an external profile (with "ipv6.method=disabled" and no manual IP addresses). The result is, that the device cannot properly autoconnect again, once you replug the cable. This is serious for disconnect. But I could not actually reproduce the problem using reapply. That is, because during reapply we usually toggle ipv6_disable sysctl, which drops all IPv6 addresses. I still went through the effort of trying to preserve addresses that we still want to have, because I am not sure whether there are cases where we don't toggle ipv6_disable. Also, doing ipv6_disable during reapply is bad anyway, and we might want to avoid that in the future. Fixes: 58287cbcc0c8 ('core: rework IP configuration in NetworkManager using layer 3 configuration') --- src/core/nm-l3cfg.c | 33 ++++++++++-- src/libnm-platform/nm-platform.c | 91 +++++++++++++++++++++++++------- src/libnm-platform/nm-platform.h | 42 +++------------ 3 files changed, 107 insertions(+), 59 deletions(-) diff --git a/src/core/nm-l3cfg.c b/src/core/nm-l3cfg.c index 6b7e6f4bc..6fdf5b80a 100644 --- a/src/core/nm-l3cfg.c +++ b/src/core/nm-l3cfg.c @@ -4186,6 +4186,7 @@ _l3_commit_one(NML3Cfg *self, gboolean final_failure_for_temporary_not_available = FALSE; char sbuf_commit_type[50]; gboolean success = TRUE; + guint i; nm_assert(NM_IS_L3CFG(self)); nm_assert(NM_IN_SET(commit_type, @@ -4218,11 +4219,33 @@ _l3_commit_one(NML3Cfg *self, route_table_sync = NM_IP_ROUTE_TABLE_SYNC_MODE_MAIN; if (commit_type == NM_L3_CFG_COMMIT_TYPE_REAPPLY) { - addresses_prune = nm_platform_ip_address_get_prune_list(self->priv.platform, - addr_family, - self->priv.ifindex, - TRUE); - routes_prune = nm_platform_ip_route_get_prune_list(self->priv.platform, + gs_unref_array GArray *ipv6_temp_addrs_keep = NULL; + + if (!IS_IPv4 && addresses) { + for (i = 0; i < addresses->len; i++) { + const NMPlatformIP6Address *addr = NMP_OBJECT_CAST_IP6_ADDRESS(addresses->pdata[i]); + + if (!NM_FLAGS_HAS(addr->n_ifa_flags, IFA_F_MANAGETEMPADDR)) + continue; + + nm_assert(addr->plen == 64); + + /* Construct a list of all IPv6 prefixes for which we (still) set + * IFA_F_MANAGETEMPADDR (that is, for which we will have temporary addresses). + * Those should not be pruned during reapply. */ + if (!ipv6_temp_addrs_keep) + ipv6_temp_addrs_keep = g_array_new(FALSE, FALSE, sizeof(struct in6_addr)); + g_array_append_val(ipv6_temp_addrs_keep, addr->address); + } + } + addresses_prune = + nm_platform_ip_address_get_prune_list(self->priv.platform, + addr_family, + self->priv.ifindex, + nm_g_array_data(ipv6_temp_addrs_keep), + nm_g_array_len(ipv6_temp_addrs_keep)); + + routes_prune = nm_platform_ip_route_get_prune_list(self->priv.platform, addr_family, self->priv.ifindex, route_table_sync); diff --git a/src/libnm-platform/nm-platform.c b/src/libnm-platform/nm-platform.c index 8866a4367..090af26dc 100644 --- a/src/libnm-platform/nm-platform.c +++ b/src/libnm-platform/nm-platform.c @@ -4459,15 +4459,25 @@ gboolean nm_platform_ip_address_flush(NMPlatform *self, int addr_family, int ifindex) { gboolean success = TRUE; + int IS_IPv4; _CHECK_SELF(self, klass, FALSE); - nm_assert(NM_IN_SET(addr_family, AF_UNSPEC, AF_INET, AF_INET6)); + nm_assert_addr_family_or_unspec(addr_family); - if (NM_IN_SET(addr_family, AF_UNSPEC, AF_INET)) - success &= nm_platform_ip4_address_sync(self, ifindex, NULL); - if (NM_IN_SET(addr_family, AF_UNSPEC, AF_INET6)) - success &= nm_platform_ip6_address_sync(self, ifindex, NULL, TRUE); + for (IS_IPv4 = 1; IS_IPv4 >= 0; IS_IPv4--) { + gs_unref_ptrarray GPtrArray *addresses_prune = NULL; + const int addr_family2 = IS_IPv4 ? AF_INET : AF_INET6; + + if (!NM_IN_SET(addr_family, AF_UNSPEC, addr_family2)) + continue; + + addresses_prune = + nm_platform_ip_address_get_prune_list(self, addr_family2, ifindex, NULL, 0); + + if (!nm_platform_ip_address_sync(self, addr_family2, ifindex, NULL, addresses_prune)) + success = FALSE; + } return success; } @@ -4509,17 +4519,31 @@ _err_inval_due_to_ipv6_tentative_pref_src(NMPlatform *self, const NMPObject *obj return TRUE; } -GPtrArray * -nm_platform_ip_address_get_prune_list(NMPlatform *self, - int addr_family, - int ifindex, - gboolean exclude_ipv6_temporary_addrs) +static guint +_ipv6_temporary_addr_prefixes_keep_hash(gconstpointer ptr) { - const int IS_IPv4 = NM_IS_IPv4(addr_family); - const NMDedupMultiHeadEntry *head_entry; - NMPLookup lookup; - GPtrArray *result = NULL; - CList *iter; + return nm_hash_mem(1161670183u, ptr, 8); +} + +static gboolean +_ipv6_temporary_addr_prefixes_keep_equal(gconstpointer ptr_a, gconstpointer ptr_b) +{ + return !memcmp(ptr_a, ptr_b, 8); +} + +GPtrArray * +nm_platform_ip_address_get_prune_list(NMPlatform *self, + int addr_family, + int ifindex, + const struct in6_addr *ipv6_temporary_addr_prefixes_keep, + guint ipv6_temporary_addr_prefixes_keep_len) +{ + gs_unref_hashtable GHashTable *ipv6_temporary_addr_prefixes_keep_idx = NULL; + const int IS_IPv4 = NM_IS_IPv4(addr_family); + const NMDedupMultiHeadEntry *head_entry; + NMPLookup lookup; + GPtrArray *result = NULL; + CList *iter; nmp_lookup_init_object(&lookup, NMP_OBJECT_TYPE_IP_ADDRESS(NM_IS_IPv4(addr_family)), ifindex); @@ -4532,9 +4556,40 @@ nm_platform_ip_address_get_prune_list(NMPlatform *self, const NMPObject *obj = c_list_entry(iter, NMDedupMultiEntry, lst_entries)->obj; if (!IS_IPv4) { - if (exclude_ipv6_temporary_addrs - && NM_FLAGS_HAS(NMP_OBJECT_CAST_IP_ADDRESS(obj)->n_ifa_flags, IFA_F_SECONDARY)) - continue; + const NMPlatformIP6Address *a6 = NMP_OBJECT_CAST_IP6_ADDRESS(obj); + + if (NM_FLAGS_HAS(a6->n_ifa_flags, IFA_F_SECONDARY) + && ipv6_temporary_addr_prefixes_keep_len > 0 && a6->plen == 64) { + gboolean keep = FALSE; + guint i; + + if (ipv6_temporary_addr_prefixes_keep_len < 10) { + for (i = 0; i < ipv6_temporary_addr_prefixes_keep_len; i++) { + if (memcmp(&ipv6_temporary_addr_prefixes_keep[i], &a6->address, 8) == 0) { + keep = TRUE; + break; + } + } + } else { + /* We have a larger number of addresses. We want that our functions are O(n), + * so build a lookup index. */ + if (!ipv6_temporary_addr_prefixes_keep_idx) { + ipv6_temporary_addr_prefixes_keep_idx = + g_hash_table_new(_ipv6_temporary_addr_prefixes_keep_hash, + _ipv6_temporary_addr_prefixes_keep_equal); + for (i = 0; i < ipv6_temporary_addr_prefixes_keep_len; i++) { + g_hash_table_add(ipv6_temporary_addr_prefixes_keep_idx, + (gpointer) &ipv6_temporary_addr_prefixes_keep[i]); + } + } + if (g_hash_table_contains(ipv6_temporary_addr_prefixes_keep_idx, &a6->address)) + keep = TRUE; + } + if (keep) { + /* This IPv6 temporary address has a prefix that we want to keep. */ + continue; + } + } } if (!result) diff --git a/src/libnm-platform/nm-platform.h b/src/libnm-platform/nm-platform.h index e83efd57a..7113607e8 100644 --- a/src/libnm-platform/nm-platform.h +++ b/src/libnm-platform/nm-platform.h @@ -2187,42 +2187,12 @@ gboolean nm_platform_ip_address_sync(NMPlatform *self, GPtrArray *known_addresses, GPtrArray *addresses_prune); -GPtrArray *nm_platform_ip_address_get_prune_list(NMPlatform *self, - int addr_family, - int ifindex, - gboolean exclude_ipv6_temporary_addrs); - -static inline gboolean -_nm_platform_ip_address_sync(NMPlatform *self, - int addr_family, - int ifindex, - GPtrArray *known_addresses, - gboolean full_sync) -{ - gs_unref_ptrarray GPtrArray *addresses_prune = NULL; - - addresses_prune = nm_platform_ip_address_get_prune_list(self, addr_family, ifindex, !full_sync); - return nm_platform_ip_address_sync(self, - addr_family, - ifindex, - known_addresses, - addresses_prune); -} - -static inline gboolean -nm_platform_ip4_address_sync(NMPlatform *self, int ifindex, GPtrArray *known_addresses) -{ - return _nm_platform_ip_address_sync(self, AF_INET, ifindex, known_addresses, TRUE); -} - -static inline gboolean -nm_platform_ip6_address_sync(NMPlatform *self, - int ifindex, - GPtrArray *known_addresses, - gboolean full_sync) -{ - return _nm_platform_ip_address_sync(self, AF_INET6, ifindex, known_addresses, full_sync); -} +GPtrArray * +nm_platform_ip_address_get_prune_list(NMPlatform *self, + int addr_family, + int ifindex, + const struct in6_addr *ipv6_temporary_addr_prefixes_keep, + guint ipv6_temporary_addr_prefixes_keep_len); gboolean nm_platform_ip_address_flush(NMPlatform *self, int addr_family, int ifindex); From 9a69bc8d84fc9f9d4c28123dbbb37570008697df Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 5 May 2022 17:24:48 +0200 Subject: [PATCH 3/3] l3cfg: refresh platform cache before creating prune list during L3Cfg commit It seems, we should make decisions based on the latest state. Make sure to process all pending netlink events. --- src/core/nm-l3cfg.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/core/nm-l3cfg.c b/src/core/nm-l3cfg.c index 6fdf5b80a..38b9d8224 100644 --- a/src/core/nm-l3cfg.c +++ b/src/core/nm-l3cfg.c @@ -4221,6 +4221,8 @@ _l3_commit_one(NML3Cfg *self, if (commit_type == NM_L3_CFG_COMMIT_TYPE_REAPPLY) { gs_unref_array GArray *ipv6_temp_addrs_keep = NULL; + nm_platform_process_events(self->priv.platform); + if (!IS_IPv4 && addresses) { for (i = 0; i < addresses->len; i++) { const NMPlatformIP6Address *addr = NMP_OBJECT_CAST_IP6_ADDRESS(addresses->pdata[i]);