platform: re-configure one address at a time in nm_platform_ip_address_sync()

Try to do one change at a time when reconfiguring addresses, to not
remove several/all addresses at once.

For IP addresses, kernel cares about the order in which they were added.
This mostly affects source address selection, and the "secondary" flag
for IPv4 addresses. The order is thus related to the priority of an
address.

There is no direct kernel API to change the order. Instead, we have to
add them in the correct order. During a sync, if an address already
exists in the wrong order, we need to remove it, and re-add it.
Btw, with IPv4 addresses added first via netlink are the primary
address, while with IPv6 it's reverse.

Previously, we would first iterate over all addresses and remove those
that had a conflicting order. This means, that we would potentially
remove all addresses for a short while, before readding them. That seems
problematic.

Instead, first track all addresses that are in the wrong order. And in
the step when we add/update the address, remove it. We now only remove
and address shortly before re-adding it. This way the time for which the
address on the interface is missing is shorter. More importantly, we will
never remove all addresses at the same time.
This commit is contained in:
Thomas Haller
2022-04-27 16:25:01 +02:00
parent ee7240783a
commit a6fd641634

View File

@@ -4031,6 +4031,7 @@ nm_platform_ip_address_sync(NMPlatform *self,
NMPLookup lookup; NMPLookup lookup;
const gboolean EXTRA_LOGGING = FALSE; const gboolean EXTRA_LOGGING = FALSE;
gs_unref_hashtable GHashTable *known_addresses_idx = NULL; gs_unref_hashtable GHashTable *known_addresses_idx = NULL;
gs_unref_hashtable GHashTable *plat_addrs_to_delete = NULL;
gs_unref_ptrarray GPtrArray *plat_addresses = NULL; gs_unref_ptrarray GPtrArray *plat_addresses = NULL;
gboolean success; gboolean success;
guint i_plat; guint i_plat;
@@ -4040,6 +4041,19 @@ nm_platform_ip_address_sync(NMPlatform *self,
_CHECK_SELF(self, klass, FALSE); _CHECK_SELF(self, klass, FALSE);
#define _plat_addrs_to_delete_ensure(ptr) \
({ \
GHashTable **_ptr = (ptr); \
\
if (!*_ptr) { \
*_ptr = g_hash_table_new_full((GHashFunc) nmp_object_id_hash, \
(GEqualFunc) nmp_object_id_equal, \
(GDestroyNotify) nmp_object_unref, \
NULL); \
} \
*_ptr; \
})
/* Disabled. Enable this for printf debugging. */ /* Disabled. Enable this for printf debugging. */
if (EXTRA_LOGGING) { if (EXTRA_LOGGING) {
char sbuf[NM_UTILS_TO_STRING_BUFFER_SIZE]; char sbuf[NM_UTILS_TO_STRING_BUFFER_SIZE];
@@ -4200,11 +4214,8 @@ nm_platform_ip_address_sync(NMPlatform *self,
} }
plat_handled[i] = TRUE; plat_handled[i] = TRUE;
nm_platform_ip4_address_delete(self, g_hash_table_add(_plat_addrs_to_delete_ensure(&plat_addrs_to_delete),
ifindex, (gpointer) nmp_object_ref(plat_obj));
plat_address->address,
plat_address->plen,
plat_address->peer_address);
if (!ip4_addr_subnets_is_secondary(plat_obj, if (!ip4_addr_subnets_is_secondary(plat_obj,
plat_subnets, plat_subnets,
@@ -4234,12 +4245,11 @@ nm_platform_ip_address_sync(NMPlatform *self,
if (!nm_g_hash_table_contains(known_addresses_idx, *o)) { if (!nm_g_hash_table_contains(known_addresses_idx, *o)) {
/* Again, this is an external address. We cannot delete /* Again, this is an external address. We cannot delete
* it to fix the address order. Pass. */ * it to fix the address order. Pass. */
} else { continue;
nm_platform_ip_address_delete(self,
AF_INET,
ifindex,
NMP_OBJECT_CAST_IP4_ADDRESS(*o));
} }
g_hash_table_add(_plat_addrs_to_delete_ensure(&plat_addrs_to_delete),
(gpointer) nmp_object_ref(*o));
} }
} }
} }
@@ -4275,7 +4285,10 @@ nm_platform_ip_address_sync(NMPlatform *self,
* @plat_addr is essentially the same address as @know_addr (w.r.t. * @plat_addr is essentially the same address as @know_addr (w.r.t.
* its identity, not its other attributes). * its identity, not its other attributes).
* However, we cannot modify an existing addresses' plen without * However, we cannot modify an existing addresses' plen without
* removing and readding it. Thus, we need to delete plat_addr. */ * removing and readding it. Thus, we need to delete plat_addr.
*
* We don't just add this address to @plat_addrs_to_delete, because
* it's too different. Instead, delete and re-add below. */
nm_platform_ip_address_delete(self, nm_platform_ip_address_delete(self,
AF_INET6, AF_INET6,
ifindex, ifindex,
@@ -4301,8 +4314,8 @@ nm_platform_ip_address_sync(NMPlatform *self,
i_know = nm_g_ptr_array_len(known_addresses); i_know = nm_g_ptr_array_len(known_addresses);
while (i_plat > 0) { while (i_plat > 0) {
const NMPlatformIP6Address *plat_addr = const NMPObject *plat_obj = plat_addresses->pdata[--i_plat];
NMP_OBJECT_CAST_IP6_ADDRESS(plat_addresses->pdata[--i_plat]); const NMPlatformIP6Address *plat_addr = NMP_OBJECT_CAST_IP6_ADDRESS(plat_obj);
IP6AddrScope plat_scope; IP6AddrScope plat_scope;
if (!plat_addr) if (!plat_addr)
@@ -4342,7 +4355,8 @@ nm_platform_ip_address_sync(NMPlatform *self,
} }
} }
nm_platform_ip6_address_delete(self, ifindex, plat_addr->address, plat_addr->plen); g_hash_table_add(_plat_addrs_to_delete_ensure(&plat_addrs_to_delete),
(gpointer) nmp_object_ref(plat_obj));
next_plat:; next_plat:;
} }
} }
@@ -4387,6 +4401,17 @@ next_plat:;
nm_assert(lifetime > 0); nm_assert(lifetime > 0);
plat_obj = nm_platform_ip_address_get(self, addr_family, ifindex, known_address); plat_obj = nm_platform_ip_address_get(self, addr_family, ifindex, known_address);
if (plat_obj && nm_g_hash_table_contains(plat_addrs_to_delete, plat_obj)) {
/* This address exists, but it had the wrong priority earlier. We
* cannot just update it, we need to remove it first. */
nm_platform_ip_address_delete(self,
addr_family,
ifindex,
NMP_OBJECT_CAST_IP_ADDRESS(plat_obj));
plat_obj = NULL;
}
if (plat_obj if (plat_obj
&& nm_platform_vtable_address.vx[IS_IPv4].address_cmp( && nm_platform_vtable_address.vx[IS_IPv4].address_cmp(
known_address, known_address,