fdo-mirrors/NetworkManager
1
0
Fork 0
mirror of https://gitlab.freedesktop.org/NetworkManager/NetworkManager.git synced 2026-02-27 17:30:42 +01:00
Code Issues Projects Releases Packages Wiki Activity Actions

platform: fix address order in nm_platform_ip_address_sync()

Browse source 
In the past, nm_platform_ip_address_sync() only had the @known_addresses
argument. We would figure out which addresses to delete and which to preserve,
based on what addresses were known. That means, @known_addresses must have contained
all the addresses we wanted to preserve, even the external ones. That approach
was inherently racy.

Instead, nowadays we have the addresses we want to configure (@known_addresses)
and the addresses we want to delete (@prune_addresses). This started to change in
commit dadfc3abd5 ('platform: allow injecting the list of addresses to prune'),
but only commit 58287cbcc0 ('core: rework IP configuration in NetworkManager using
layer 3 configuration') actually changed to pass separate @prune_addresses argument.

However, the order of IP addresses matters and there is no sensible kernel API
to configure the order (short of adding them in the right order), we still need
to look at all the addresses, check their order, and possibly delete some.
That is, we need to handle addresses we want to delete (@prune_addresses)
but still look at all addresses in platform (@plat_addresses) to check
their order.

Now, first handle @prune_addresses. That's simple. These are just the
addresses we want to delete. Second, get the list of all addresses in
platform (@plat_addresses) and check the order.

Note that if there is an external address that interferes with our
desired order, we will leave it untouched. Thus, such external addresses
might prevent us from getting the order as desired. But that's just
how it is. Don't add addresses outside of NetworkManager to avoid that.

Fixes: 58287cbcc0 ('core: rework IP configuration in NetworkManager using layer 3 configuration')
(cherry picked from commit 80f8e23992)
(cherry picked from commit 4c3197b377)
This commit is contained in:
Thomas Haller 2022-03-28 21:13:09 +02:00 • committed by Beniamino Galvani
parent 264296868b
commit cd4601802d
1 changed files with 126 additions and 81 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

207
src/libnm-platform/nm-platform.c
View file

@ -3889,9 +3889,8 @@ ip6_address_scope_cmp(gconstpointer p_a, gconstpointer p_b, gpointer increasing)
* If platform has such an address configured, it will be deleted * If platform has such an address configured, it will be deleted
* at the beginning of the sync. Note that the array will be modified * at the beginning of the sync. Note that the array will be modified
* by the function. * by the function.
* Note that the addresses must be properly sorted, by their priority. * Addresses that are both contained in @known_addresses and @addresses_prune
* Create this list with nm_platform_ip_address_get_prune_list() which * will be configured.
* gets the sorting right.
* *
* A convenience function to synchronize addresses for a specific interface * A convenience function to synchronize addresses for a specific interface
* with the least possible disturbance. It simply removes addresses that are * with the least possible disturbance. It simply removes addresses that are
@ -3906,11 +3905,12 @@ nm_platform_ip_address_sync(NMPlatform *self,
GPtrArray *known_addresses, GPtrArray *known_addresses,
GPtrArray *addresses_prune) GPtrArray *addresses_prune)
{ {
const gint32 now = nm_utils_get_monotonic_timestamp_sec(); const gint32 now = nm_utils_get_monotonic_timestamp_sec();
const int IS_IPv4 = NM_IS_IPv4(addr_family); const int IS_IPv4 = NM_IS_IPv4(addr_family);
NMPLookup lookup;
gs_unref_hashtable GHashTable *known_addresses_idx = NULL; gs_unref_hashtable GHashTable *known_addresses_idx = NULL;
GPtrArray *plat_addresses; gs_unref_ptrarray GPtrArray *plat_addresses = NULL;
GHashTable *known_subnets = NULL; GHashTable *known_subnets = NULL;
guint i_plat; guint i_plat;
guint i_know; guint i_know;
guint i; guint i;
@ -3918,6 +3918,9 @@ nm_platform_ip_address_sync(NMPlatform *self,
_CHECK_SELF(self, klass, FALSE); _CHECK_SELF(self, klass, FALSE);
/* @known_addresses (IPv4) are in decreasing priority order (highest priority addresses first).
* @known_addresses (IPv6) are in increasing priority order (highest priority addresses last) (we will sort them by scope next). */
/* The order we want to enforce is only among addresses with the same /* The order we want to enforce is only among addresses with the same
* scope, as the kernel keeps addresses sorted by scope. Therefore, * scope, as the kernel keeps addresses sorted by scope. Therefore,
* apply the same sorting to known addresses, so that we don't try to * apply the same sorting to known addresses, so that we don't try to
@ -3936,51 +3939,92 @@ nm_platform_ip_address_sync(NMPlatform *self,
&known_addresses_idx)) &known_addresses_idx))
known_addresses = NULL; known_addresses = NULL;
/* @plat_addresses must be sorted in decreasing priority order (highest priority addresses first), contrary to if (nm_g_ptr_array_len(addresses_prune) > 0) {
* @known_addresses which is in increasing priority order (lowest priority addresses first). */ /* First delete addresses that we should prune (and which are no longer tracked
plat_addresses = addresses_prune; * as @known_addresses. */
for (i = 0; i < addresses_prune->len; i++) {
const NMPObject *prune_obj = addresses_prune->pdata[i];
nm_assert(NM_IN_SET(NMP_OBJECT_GET_TYPE(prune_obj),
NMP_OBJECT_TYPE_IP4_ADDRESS,
NMP_OBJECT_TYPE_IP6_ADDRESS));
if (nm_g_hash_table_contains(known_addresses_idx, prune_obj))
continue;
nm_platform_ip_address_delete(self,
addr_family,
ifindex,
NMP_OBJECT_CAST_IP_ADDRESS(prune_obj));
}
}
/* @plat_addresses for IPv6 must be sorted in decreasing priority order (highest priority addresses first).
* IPv4 are probably unsorted or sorted with lowest priority first, but their order doesn't matter because
* we check the "secondary" flag. */
plat_addresses = nm_platform_lookup_clone(
self,
nmp_lookup_init_object(&lookup, NMP_OBJECT_TYPE_IP_ADDRESS(IS_IPv4), ifindex),
NULL,
NULL);
if (nm_g_ptr_array_len(plat_addresses) > 0) { if (nm_g_ptr_array_len(plat_addresses) > 0) {
/* Delete unknown addresses */ /* Delete addresses that interfere with our intended order. */
if (IS_IPv4) { if (IS_IPv4) {
GHashTable *plat_subnets; gs_free bool *plat_handled_to_free = NULL;
bool *plat_handled = NULL;
GHashTable *plat_subnets;
/* For IPv4, we only consider it a conflict for addresses in the same
* subnet. That's where kernel will assign a primary/secondary flag.
* For different subnets, we don't define the order. */
plat_subnets = ip4_addr_subnets_build_index(plat_addresses, TRUE, TRUE); plat_subnets = ip4_addr_subnets_build_index(plat_addresses, TRUE, TRUE);
for (i = 0; i < plat_addresses->len; i++) { for (i = 0; i < plat_addresses->len; i++) {
const NMPObject *plat_obj; const NMPObject *plat_obj = plat_addresses->pdata[i];
const NMPObject *known_obj;
const NMPlatformIP4Address *plat_address; const NMPlatformIP4Address *plat_address;
const GPtrArray *addr_list; const GPtrArray *addr_list;
gboolean secondary;
plat_obj = plat_addresses->pdata[i]; if (plat_handled && plat_handled[i])
if (!plat_obj) { continue;
/* Already deleted */
known_obj = nm_g_hash_table_lookup(known_addresses_idx, plat_obj);
if (!known_obj) {
/* this address is added externally. Even if it's presence would mess
* with our desired order, we cannot delete it. Skip it. */
if (!plat_handled) {
plat_handled = nm_malloc0_maybe_a(300,
sizeof(bool) * plat_addresses->len,
&plat_handled_to_free);
}
plat_handled[i] = TRUE;
continue; continue;
} }
if (!known_subnets)
known_subnets = ip4_addr_subnets_build_index(known_addresses, FALSE, FALSE);
plat_address = NMP_OBJECT_CAST_IP4_ADDRESS(plat_obj); plat_address = NMP_OBJECT_CAST_IP4_ADDRESS(plat_obj);
if (known_addresses) { secondary =
const NMPObject *o; ip4_addr_subnets_is_secondary(known_obj, known_subnets, known_addresses, NULL);
if (secondary == NM_FLAGS_HAS(plat_address->n_ifa_flags, IFA_F_SECONDARY)) {
o = g_hash_table_lookup(known_addresses_idx, plat_obj); /* if we have an existing known-address, with matching secondary role,
if (o) { * do not delete the platform-address. */
gboolean secondary; continue;
if (!known_subnets)
known_subnets =
ip4_addr_subnets_build_index(known_addresses, FALSE, FALSE);
secondary =
ip4_addr_subnets_is_secondary(o, known_subnets, known_addresses, NULL);
if (secondary == NM_FLAGS_HAS(plat_address->n_ifa_flags, IFA_F_SECONDARY)) {
/* if we have an existing known-address, with matching secondary role,
* do not delete the platform-address. */
continue;
}
}
} }
if (!plat_handled) {
plat_handled = nm_malloc0_maybe_a(300,
sizeof(bool) * plat_addresses->len,
&plat_handled_to_free);
}
plat_handled[i] = TRUE;
nm_platform_ip4_address_delete(self, nm_platform_ip4_address_delete(self,
ifindex, ifindex,
plat_address->address, plat_address->address,
@ -3999,22 +4043,27 @@ nm_platform_ip_address_sync(NMPlatform *self,
* addresses are deleted, so that we can start with a clean * addresses are deleted, so that we can start with a clean
* slate and add addresses in the right order. */ * slate and add addresses in the right order. */
for (j = 1; j < addr_list->len; j++) { for (j = 1; j < addr_list->len; j++) {
const NMPObject **o; const NMPObject **o = ip4_addr_subnets_addr_list_get(addr_list, j);
guint o_idx;
o = ip4_addr_subnets_addr_list_get(addr_list, j); o_idx = (o - ((const NMPObject **) &plat_addresses->pdata[0]));
nm_assert(o);
if (*o) { nm_assert(o_idx < plat_addresses->len);
const NMPlatformIP4Address *a; nm_assert(o == ((const NMPObject **) &plat_addresses->pdata[o_idx]));
a = NMP_OBJECT_CAST_IP4_ADDRESS(*o); if (plat_handled[o_idx])
nm_platform_ip4_address_delete(self, continue;
ifindex,
a->address, plat_handled[o_idx] = TRUE;
a->plen,
a->peer_address); if (!nm_g_hash_table_contains(known_addresses_idx, *o)) {
nmp_object_unref(*o); /* Again, this is an external address. We cannot delete
*o = NULL; * it to fix the address order. Pass. */
} else {
nm_platform_ip_address_delete(self,
AF_INET,
ifindex,
NMP_OBJECT_CAST_IP4_ADDRESS(*o));
} }
} }
} }
@ -4025,48 +4074,44 @@ nm_platform_ip_address_sync(NMPlatform *self,
IP6AddrScope cur_scope; IP6AddrScope cur_scope;
gboolean delete_remaining_addrs; gboolean delete_remaining_addrs;
/* For IPv6, we only compare addresses per-scope. Addresses in different
* scopes don't have a defined order. */
g_ptr_array_sort_with_data(plat_addresses, g_ptr_array_sort_with_data(plat_addresses,
ip6_address_scope_cmp, ip6_address_scope_cmp,
GINT_TO_POINTER(FALSE)); GINT_TO_POINTER(FALSE));
known_addresses_len = known_addresses ? known_addresses->len : 0; known_addresses_len = known_addresses ? known_addresses->len : 0;
/* First, compare every address whether it is still a "known address", that is, whether /* First, check that existing addresses have a matching plen as the ones
* to keep it or to delete it. * we are about to configure (@known_addresses). If not, delete them. */
*
* If we don't find a matching valid address in @known_addresses, we will delete
* plat_addr.
*
* Certain addresses, like temporary addresses, are ignored by this function
* if not run with full_sync. These addresses are usually not managed by NetworkManager
* directly, or at least, they are not managed via nm_platform_ip6_address_sync().
* Only in full_sync mode, we really want to get rid of them (usually, when we take
* the interface down).
*
* Note that we mark handled addresses by setting it to %NULL in @plat_addresses array. */
for (i_plat = 0; i_plat < plat_addresses->len; i_plat++) { for (i_plat = 0; i_plat < plat_addresses->len; i_plat++) {
const NMPObject *plat_obj = plat_addresses->pdata[i_plat]; const NMPObject *plat_obj = plat_addresses->pdata[i_plat];
const NMPObject *know_obj; const NMPObject *known_obj;
const NMPlatformIP6Address *plat_addr = NMP_OBJECT_CAST_IP6_ADDRESS(plat_obj);
if (known_addresses_idx) { known_obj = nm_g_hash_table_lookup(known_addresses_idx, plat_obj);
know_obj = g_hash_table_lookup(known_addresses_idx, plat_obj); if (!known_obj) {
if (know_obj /* We don't know this address. It was added externally. Keep it configured.
&& plat_addr->plen == NMP_OBJECT_CAST_IP6_ADDRESS(know_obj)->plen) { * We also don't want to delete the address below, so mark it as handled
/* technically, plen is not part of the ID for IPv6 addresses and thus * by clearing the pointer. */
* @plat_addr is essentially the same address as @know_addr (regrading nm_clear_pointer(&plat_addresses->pdata[i_plat], nmp_object_unref);
* its identity, not its other attributes). continue;
* However, we cannot modify an existing addresses' plen without
* removing and readding it. Thus, only keep plat_addr, if the plen
* matches.
*
* keep this one, and continue */
continue;
}
} }
nm_platform_ip6_address_delete(self, ifindex, plat_addr->address, plat_addr->plen); if (NMP_OBJECT_CAST_IP6_ADDRESS(plat_obj)->plen
nmp_object_unref(g_steal_pointer(&plat_addresses->pdata[i_plat])); != NMP_OBJECT_CAST_IP6_ADDRESS(known_obj)->plen) {
/* technically, plen is not part of the ID for IPv6 addresses and thus
* @plat_addr is essentially the same address as @know_addr (w.r.t.
* its identity, not its other attributes).
* However, we cannot modify an existing addresses' plen without
* removing and readding it. Thus, we need to delete plat_addr. */
nm_platform_ip_address_delete(self,
AF_INET6,
ifindex,
NMP_OBJECT_CAST_IP6_ADDRESS(plat_obj));
/* Mark address as handled. */
nm_clear_pointer(&plat_addresses->pdata[i_plat], nmp_object_unref);
}
} }
/* Next, we must preserve the priority of the routes. That is, source address /* Next, we must preserve the priority of the routes. That is, source address
@ -4077,7 +4122,7 @@ nm_platform_ip_address_sync(NMPlatform *self,
* @known_addresses (which has lowest priority first). * @known_addresses (which has lowest priority first).
* *
* If we find a first discrepancy, we need to delete all remaining addresses * If we find a first discrepancy, we need to delete all remaining addresses
* with same scope from that point on, because below we must re-add all the * for same scope from that point on, because below we must re-add all the
* addresses in the right order to get their priority right. */ * addresses in the right order to get their priority right. */
cur_scope = IP6_ADDR_SCOPE_LOOPBACK; cur_scope = IP6_ADDR_SCOPE_LOOPBACK;
delete_remaining_addrs = FALSE; delete_remaining_addrs = FALSE;