platform: merge branch 'th/platform-address-order' (part 1)

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1168

(cherry picked from commit 8b95693985f52df631b822fda754b81007c3ff5d)

(cherry picked from commit 14052c847c53c79549172f126a8cfb7eaba1049f)

2 files changed, 202 insertions, 119 deletions

diff --git a/src/libnm-platform/nm-platform.c b/src/libnm-platform/nm-platform.c
index 922f412df7..00a3fb2a0a 100644
--- a/src/libnm-platform/nm-platform.c
+++ b/src/libnm-platform/nm-platform.c
@@ -3698,8 +3698,8 @@ clear_and_next:
 static gboolean
 ip4_addr_subnets_is_plain_address(const GPtrArray *addresses, gconstpointer needle)
 {
-    return needle >= (gconstpointer) &addresses->pdata[0]
-           && needle < (gconstpointer) &addresses->pdata[addresses->len];
+    return nm_ptr_to_uintptr(needle) >= nm_ptr_to_uintptr(&addresses->pdata[0])
+           && nm_ptr_to_uintptr(needle) < nm_ptr_to_uintptr(&addresses->pdata[addresses->len]);
 }
 
 static const NMPObject **
@@ -3733,6 +3733,30 @@ ip4_addr_subnets_destroy_index(GHashTable *subnets, const GPtrArray *addresses)
     g_hash_table_unref(subnets);
 }
 
+static guint
+_ip4_addr_subnets_hash(gconstpointer ptr)
+{
+    const NMPlatformIP4Address *addr = NMP_OBJECT_CAST_IP4_ADDRESS(ptr);
+    NMHashState                 h;
+
+    nm_hash_init(&h, 3282159733);
+    nm_hash_update_vals(&h,
+                        addr->plen,
+                        nm_utils_ip4_address_clear_host_address(addr->address, addr->plen));
+    return nm_hash_complete(&h);
+}
+
+static gboolean
+_ip4_addr_subnets_equal(gconstpointer p_a, gconstpointer p_b)
+{
+    const NMPlatformIP4Address *a = NMP_OBJECT_CAST_IP4_ADDRESS(p_a);
+    const NMPlatformIP4Address *b = NMP_OBJECT_CAST_IP4_ADDRESS(p_b);
+
+    return a->plen == b->plen
+           && (nm_utils_ip4_address_clear_host_address(a->address, a->plen)
+               == nm_utils_ip4_address_clear_host_address(b->address, b->plen));
+}
+
 static GHashTable *
 ip4_addr_subnets_build_index(const GPtrArray *addresses,
                              gboolean         consider_flags,
@@ -3743,34 +3767,35 @@ ip4_addr_subnets_build_index(const GPtrArray *addresses,
 
     nm_assert(addresses && addresses->len);
 
-    subnets = g_hash_table_new(nm_direct_hash, NULL);
+    subnets = g_hash_table_new(_ip4_addr_subnets_hash, _ip4_addr_subnets_equal);
 
     /* Build a hash table of all addresses per subnet */
     for (i = 0; i < addresses->len; i++) {
+        const NMPObject           **p_obj;
+        const NMPObject            *obj;
         const NMPlatformIP4Address *address;
-        gpointer                    p_address;
         GPtrArray                  *addr_list;
-        guint32                     net;
         int                         position;
         gpointer                    p;
 
         if (!addresses->pdata[i])
             continue;
 
-        p_address = &addresses->pdata[i];
-        address   = NMP_OBJECT_CAST_IP4_ADDRESS(addresses->pdata[i]);
+        p_obj = (const NMPObject **) &addresses->pdata[i];
+        obj   = *p_obj;
 
-        net = address->address & _nm_utils_ip4_prefix_to_netmask(address->plen);
-        if (!g_hash_table_lookup_extended(subnets, GUINT_TO_POINTER(net), NULL, &p)) {
-            g_hash_table_insert(subnets, GUINT_TO_POINTER(net), p_address);
+        if (!g_hash_table_lookup_extended(subnets, obj, NULL, &p)) {
+            g_hash_table_insert(subnets, (gpointer) obj, p_obj);
             continue;
         }
         nm_assert(p);
 
+        address = NMP_OBJECT_CAST_IP4_ADDRESS(obj);
+
         if (full_index) {
             if (ip4_addr_subnets_is_plain_address(addresses, p)) {
                 addr_list = g_ptr_array_new();
-                g_hash_table_insert(subnets, GUINT_TO_POINTER(net), addr_list);
+                g_hash_table_insert(subnets, (gpointer) obj, addr_list);
                 g_ptr_array_add(addr_list, p);
             } else
                 addr_list = p;
@@ -3779,13 +3804,13 @@ ip4_addr_subnets_build_index(const GPtrArray *addresses,
                 position = -1; /* append */
             else
                 position = 0; /* prepend */
-            g_ptr_array_insert(addr_list, position, p_address);
+            g_ptr_array_insert(addr_list, position, p_obj);
         } else {
             /* we only care about the primary. No need to track the secondaries
              * as a GPtrArray. */
             nm_assert(ip4_addr_subnets_is_plain_address(addresses, p));
             if (consider_flags && !NM_FLAGS_HAS(address->n_ifa_flags, IFA_F_SECONDARY)) {
-                g_hash_table_insert(subnets, GUINT_TO_POINTER(net), p_address);
+                g_hash_table_insert(subnets, (gpointer) obj, p_obj);
             }
         }
     }
@@ -3811,16 +3836,11 @@ ip4_addr_subnets_is_secondary(const NMPObject  *address,
                               const GPtrArray  *addresses,
                               const GPtrArray **out_addr_list)
 {
-    const NMPlatformIP4Address *a;
-    const GPtrArray            *addr_list;
-    gconstpointer               p;
-    guint32                     net;
-    const NMPObject           **o;
-
-    a = NMP_OBJECT_CAST_IP4_ADDRESS(address);
+    const GPtrArray  *addr_list;
+    gconstpointer     p;
+    const NMPObject **o;
 
-    net = a->address & _nm_utils_ip4_prefix_to_netmask(a->plen);
-    p   = g_hash_table_lookup(subnets, GUINT_TO_POINTER(net));
+    p = g_hash_table_lookup(subnets, address);
     nm_assert(p);
     if (!ip4_addr_subnets_is_plain_address(addresses, p)) {
         addr_list = p;
@@ -3877,21 +3897,15 @@ ip6_address_scope_cmp(gconstpointer p_a, gconstpointer p_b, gpointer increasing)
  * @self: platform instance
  * @addr_family: the address family AF_INET or AF_INET6.
  * @ifindex: Interface index
- * @known_addresses: List of addresses. The list will be modified and only
- *   addresses that were successfully added will be kept in the list.
- *   That means, expired addresses and addresses that could not be added
- *   will be dropped.
- *   Hence, the input argument @known_addresses is also an output argument
- *   telling which addresses were successfully added.
- *   Addresses are removed by unrefing the instance via nmp_object_unref()
- *   and leaving a NULL tombstone.
+ * @known_addresses: List of addresses. The list will be modified and
+ *   expired addresses will be cleared (by calling nmp_object_unref()
+ *   on the array element).
  * @addresses_prune: (allow-none): the list of addresses to delete.
  *   If platform has such an address configured, it will be deleted
  *   at the beginning of the sync. Note that the array will be modified
  *   by the function.
- *   Note that the addresses must be properly sorted, by their priority.
- *   Create this list with nm_platform_ip_address_get_prune_list() which
- *   gets the sorting right.
+ *   Addresses that are both contained in @known_addresses and @addresses_prune
+ *   will be configured.
  *
  * A convenience function to synchronize addresses for a specific interface
  * with the least possible disturbance. It simply removes addresses that are
@@ -3906,11 +3920,12 @@ nm_platform_ip_address_sync(NMPlatform *self,
                             GPtrArray  *known_addresses,
                             GPtrArray  *addresses_prune)
 {
-    const gint32                   now                 = nm_utils_get_monotonic_timestamp_sec();
-    const int                      IS_IPv4             = NM_IS_IPv4(addr_family);
+    const gint32                   now     = nm_utils_get_monotonic_timestamp_sec();
+    const int                      IS_IPv4 = NM_IS_IPv4(addr_family);
+    NMPLookup                      lookup;
     gs_unref_hashtable GHashTable *known_addresses_idx = NULL;
-    GPtrArray                     *plat_addresses;
-    GHashTable                    *known_subnets = NULL;
+    gs_unref_ptrarray GPtrArray   *plat_addresses      = NULL;
+    gboolean                       success;
     guint                          i_plat;
     guint                          i_know;
     guint                          i;
@@ -3918,6 +3933,9 @@ nm_platform_ip_address_sync(NMPlatform *self,
 
     _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
      * scope, as the kernel keeps addresses sorted by scope. Therefore,
      * apply the same sorting to known addresses, so that we don't try to
@@ -3936,50 +3954,92 @@ nm_platform_ip_address_sync(NMPlatform *self,
                                    &known_addresses_idx))
         known_addresses = NULL;
 
-    /* @plat_addresses must be sorted in decreasing priority order (highest priority addresses first), contrary to
-     * @known_addresses which is in increasing priority order (lowest priority addresses first). */
-    plat_addresses = addresses_prune;
+    if (nm_g_ptr_array_len(addresses_prune) > 0) {
+        /* First delete addresses that we should prune (and which are no longer tracked
+         * 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) {
-        /* Delete unknown addresses */
+        /* Delete addresses that interfere with our intended order. */
         if (IS_IPv4) {
-            GHashTable *plat_subnets;
+            GHashTable   *known_subnets = NULL;
+            GHashTable   *plat_subnets;
+            gs_free bool *plat_handled_to_free = NULL;
+            bool         *plat_handled         = NULL;
+
+            /* 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);
 
             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 GPtrArray            *addr_list;
+                gboolean                    secondary;
 
-                plat_obj = plat_addresses->pdata[i];
-                if (!plat_obj) {
-                    /* Already deleted */
+                if (plat_handled && plat_handled[i])
                     continue;
-                }
 
-                plat_address = NMP_OBJECT_CAST_IP4_ADDRESS(plat_obj);
+                known_obj = nm_g_hash_table_lookup(known_addresses_idx, plat_obj);
 
-                if (known_addresses) {
-                    const NMPObject *o;
+                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;
+                }
 
-                    o = g_hash_table_lookup(known_addresses_idx, plat_obj);
-                    if (o) {
-                        gboolean secondary;
+                if (!known_subnets)
+                    known_subnets = ip4_addr_subnets_build_index(known_addresses, FALSE, FALSE);
 
-                        if (!known_subnets)
-                            known_subnets =
-                                ip4_addr_subnets_build_index(known_addresses, FALSE, FALSE);
+                plat_address = NMP_OBJECT_CAST_IP4_ADDRESS(plat_obj);
 
-                        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;
-                        }
-                    }
+                secondary =
+                    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)) {
+                    /* 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,
                                                ifindex,
@@ -3999,74 +4059,76 @@ nm_platform_ip_address_sync(NMPlatform *self,
                      * addresses are deleted, so that we can start with a clean
                      * slate and add addresses in the right order. */
                     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);
-                        nm_assert(o);
+                        o_idx = (o - ((const NMPObject **) &plat_addresses->pdata[0]));
 
-                        if (*o) {
-                            const NMPlatformIP4Address *a;
+                        nm_assert(o_idx < plat_addresses->len);
+                        nm_assert(o == ((const NMPObject **) &plat_addresses->pdata[o_idx]));
 
-                            a = NMP_OBJECT_CAST_IP4_ADDRESS(*o);
-                            nm_platform_ip4_address_delete(self,
-                                                           ifindex,
-                                                           a->address,
-                                                           a->plen,
-                                                           a->peer_address);
-                            nmp_object_unref(*o);
-                            *o = NULL;
+                        if (plat_handled[o_idx])
+                            continue;
+
+                        plat_handled[o_idx] = TRUE;
+
+                        if (!nm_g_hash_table_contains(known_addresses_idx, *o)) {
+                            /* Again, this is an external address. We cannot delete
+                             * it to fix the address order. Pass. */
+                        } else {
+                            nm_platform_ip_address_delete(self,
+                                                          AF_INET,
+                                                          ifindex,
+                                                          NMP_OBJECT_CAST_IP4_ADDRESS(*o));
                         }
                     }
                 }
             }
             ip4_addr_subnets_destroy_index(plat_subnets, plat_addresses);
+            ip4_addr_subnets_destroy_index(known_subnets, known_addresses);
         } else {
             guint        known_addresses_len;
             IP6AddrScope cur_scope;
             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,
                                        ip6_address_scope_cmp,
                                        GINT_TO_POINTER(FALSE));
 
-            known_addresses_len = known_addresses ? known_addresses->len : 0;
+            known_addresses_len = nm_g_ptr_array_len(known_addresses);
 
-            /* First, compare every address whether it is still a "known address", that is, whether
-             * to keep it or to delete it.
-             *
-             * 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. */
+            /* First, check that existing addresses have a matching plen as the ones
+             * we are about to configure (@known_addresses). If not, delete them. */
             for (i_plat = 0; i_plat < plat_addresses->len; i_plat++) {
-                const NMPObject            *plat_obj = plat_addresses->pdata[i_plat];
-                const NMPObject            *know_obj;
-                const NMPlatformIP6Address *plat_addr = NMP_OBJECT_CAST_IP6_ADDRESS(plat_obj);
-
-                if (known_addresses_idx) {
-                    know_obj = g_hash_table_lookup(known_addresses_idx, plat_obj);
-                    if (know_obj
-                        && plat_addr->plen == NMP_OBJECT_CAST_IP6_ADDRESS(know_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 (regrading
-                         * its identity, not its other attributes).
-                         * 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;
-                    }
+                const NMPObject *plat_obj = plat_addresses->pdata[i_plat];
+                const NMPObject *known_obj;
+
+                known_obj = nm_g_hash_table_lookup(known_addresses_idx, plat_obj);
+                if (!known_obj) {
+                    /* We don't know this address. It was added externally. Keep it configured.
+                     * We also don't want to delete the address below, so mark it as handled
+                     * by clearing the pointer. */
+                    nm_clear_pointer(&plat_addresses->pdata[i_plat], nmp_object_unref);
+                    continue;
                 }
 
-                nm_platform_ip6_address_delete(self, ifindex, plat_addr->address, plat_addr->plen);
-                nmp_object_unref(g_steal_pointer(&plat_addresses->pdata[i_plat]));
+                if (NMP_OBJECT_CAST_IP6_ADDRESS(plat_obj)->plen
+                    != 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
@@ -4077,7 +4139,7 @@ nm_platform_ip_address_sync(NMPlatform *self,
              * @known_addresses (which has lowest priority first).
              *
              * 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. */
             cur_scope              = IP6_ADDR_SCOPE_LOOPBACK;
             delete_remaining_addrs = FALSE;
@@ -4134,8 +4196,7 @@ next_plat:;
     if (!known_addresses)
         return TRUE;
 
-    if (IS_IPv4)
-        ip4_addr_subnets_destroy_index(known_subnets, known_addresses);
+    success = TRUE;
 
     /* Add missing addresses. New addresses are added by kernel with top
      * priority.
@@ -4172,9 +4233,8 @@ next_plat:;
                     lifetime,
                     preferred,
                     IFA_F_NOPREFIXROUTE,
-                    known_address->a4.label)) {
-                /* ignore error, for unclear reasons. */
-            }
+                    known_address->a4.label))
+                success = FALSE;
         } else {
             if (!nm_platform_ip6_address_add(self,
                                              ifindex,
@@ -4184,11 +4244,11 @@ next_plat:;
                                              lifetime,
                                              preferred,
                                              IFA_F_NOPREFIXROUTE | known_address->a6.n_ifa_flags))
-                return FALSE;
+                success = FALSE;
         }
     }
 
-    return TRUE;
+    return success;
 }
 
 gboolean
diff --git a/src/libnm-platform/nm-platform.h b/src/libnm-platform/nm-platform.h
index 6f5cd5248f..baa3967db0 100644
--- a/src/libnm-platform/nm-platform.h
+++ b/src/libnm-platform/nm-platform.h
@@ -2097,6 +2097,29 @@ gboolean nm_platform_ip4_address_delete(NMPlatform *self,
 gboolean
 nm_platform_ip6_address_delete(NMPlatform *self, int ifindex, struct in6_addr address, guint8 plen);
 
+static inline gboolean
+nm_platform_ip_address_delete(NMPlatform                                       *self,
+                              int                                               addr_family,
+                              int                                               ifindex,
+                              gconstpointer /* (const NMPlatformIPAddress *) */ addr)
+{
+    if (NM_IS_IPv4(addr_family)) {
+        const NMPlatformIP4Address *a = addr;
+
+        if (ifindex <= 0)
+            ifindex = a->ifindex;
+
+        return nm_platform_ip4_address_delete(self, ifindex, a->address, a->plen, a->peer_address);
+    } else {
+        const NMPlatformIP6Address *a = addr;
+
+        if (ifindex <= 0)
+            ifindex = a->ifindex;
+
+        return nm_platform_ip6_address_delete(self, ifindex, a->address, a->plen);
+    }
+}
+
 gboolean nm_platform_ip_address_sync(NMPlatform *self,
                                      int         addr_family,
                                      int         ifindex,