core: change order/priority of static IPv6 addresses relative to autoconf6/DHCPv6

The order of addresses can matter for source address selection.
This is described in RFC 6724 section 5, but if the rules don't
determine a clear winner, the order matters.

Change the relative order of IPv6 addresses. Previously, we would prefer
autoconf6, over DHCPv6, over manual addresses. Now that got reverted
to make more sense and be consistent with IPv4.
Also, if we had multiple autoconf6 addresses (received at different
moments in time), then previously a newly received address would be
added with highest priority. Now, the older address will be preferred
and that order will be enforced (this can be a problem, see (*) below).

For IPv4, it's all simple and sensible. When we add addresses in kernel
via netlink, the first address (of a subnet) becomes the primary.
Note that we only control the order of addresses of the same subnet.
The addresses in ipv4.addresses" are sorted with primary address first.
In the same way is the order for addresses in NML3ConfigData and for
@known_addresses in nm_platform_ip_address_sync(), all primary-first.
Also, manual addresses are sorted with higher priority compared to DHCPv4
addresses (at least since NetworkManager 1.36). That means the way how we
merge NML3ConfigData makes sense (nm_l3_config_data_merge()) because we first
merge the static configuration, then the DHCPv4 configuration, where we just
append the lower priority DHCPv4 addresses.

For IPv6, the address priority is messed up. On netlink/kernel, the last added
address becomes the preferred one (we thus need to add them in the order of
lowest priority first). Consequently and historically, the IPv6 addresses in
@known_addresses parameter to nm_platform_ip_address_sync() were
lowest priority first. And so they were tracked in NML3ConfigData
and in the profile ("ipv6.addresses"). That is confusing.
Also, we usually want to merge NML3ConfigData with different priorities
(e.g. static configuration from the profile before autoconf6/DHCPv6),
as we do with IPv4. However, since internally IPv6 addresses are tracked in
reverse order, it means later NML3ConfigData would be appended and get effectively
a higher priority. That means, autoconf6 addresses were preferred over DHCPv6 and
over manual "ipv6.addresses", respectively. That seems undesirable and inconsistent
with IPv4. Change that. This is a change in behavior.

Note that changing the order of addresses means to remove and re-add
them in the right (inverse) order, with lease important first. This
means, when we add a new address with lower priority, we need to remove
all higher priority addresses temporarily, before readding them. That
is a problem(*).

Note that in the profile, "ipv6.addresses" is still tracked in reverse
order. This did not change, but might change later.

3 files changed, 26 insertions, 11 deletions

diff --git a/NEWS b/NEWS
index 14aea8d9f7..76a056bf51 100644
--- a/NEWS
+++ b/NEWS
@@ -13,6 +13,10 @@ USE AT YOUR OWN RISK.  NOT RECOMMENDED FOR PRODUCTION USE!
 * The nmcli command now supports --offline argument with "add" and
   "modify" commands, allowing operation on keyfile-formatted connection
   profiles without the service running (e.g. during system provisioning).
+* Static IPv6 addresses from "ipv6.addresses" are now preferred over
+  addresses from DHCPv6, which are preferred over addresses from autoconf.
+  This affects IPv6 source address selection, if the rules from
+  RFC 6724, section 5 don't give a exhaustive match.
 
 =============================================
 NetworkManager-1.38
diff --git a/src/core/nm-l3-config-data.c b/src/core/nm-l3-config-data.c
index c732840e31..7512e1c9a7 100644
--- a/src/core/nm-l3-config-data.c
+++ b/src/core/nm-l3-config-data.c
@@ -2769,7 +2769,7 @@ _init_from_connection_ip(NML3ConfigData *self, int addr_family, NMConnection *co
 
     nroutes = nm_setting_ip_config_get_num_routes(s_ip);
     for (i = 0; i < nroutes; i++) {
-        NMIPRoute         *s_route = nm_setting_ip_config_get_route(s_ip, i);
+        NMIPRoute         *s_route;
         NMPlatformIPXRoute r;
         NMIPAddr           network_bin;
         NMIPAddr           next_hop_bin;
@@ -2777,6 +2777,12 @@ _init_from_connection_ip(NML3ConfigData *self, int addr_family, NMConnection *co
         guint32            metric;
         gboolean           metric_any;
         guint              plen;
+        guint              i_idx;
+
+        /* Routes in ipv6.addresses are in reversed priority order. */
+        i_idx = IS_IPv4 ? i : (nroutes - i - 1);
+
+        s_route = nm_setting_ip_config_get_route(s_ip, i_idx);
 
         nm_assert(nm_ip_route_get_family(s_route) == addr_family);
 
diff --git a/src/libnm-platform/nm-platform.c b/src/libnm-platform/nm-platform.c
index 184ca3fde1..f584540d8e 100644
--- a/src/libnm-platform/nm-platform.c
+++ b/src/libnm-platform/nm-platform.c
@@ -4073,8 +4073,7 @@ nm_platform_ip_address_sync(NMPlatform *self,
         }
     }
 
-    /* @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). */
+    /* @known_addresses are in decreasing priority order (highest priority addresses first). */
 
     /* The order we want to enforce is only among addresses with the same
      * scope, as the kernel keeps addresses sorted by scope. Therefore,
@@ -4082,7 +4081,7 @@ nm_platform_ip_address_sync(NMPlatform *self,
      * unnecessary change the order of addresses with different scopes. */
     if (!IS_IPv4) {
         if (known_addresses)
-            g_ptr_array_sort_with_data(known_addresses, ip6_address_scope_cmp_ascending, NULL);
+            g_ptr_array_sort_with_data(known_addresses, ip6_address_scope_cmp_descending, NULL);
     }
 
     if (!_addr_array_clean_expired(addr_family,
@@ -4247,7 +4246,6 @@ nm_platform_ip_address_sync(NMPlatform *self,
             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;
 
@@ -4256,8 +4254,6 @@ nm_platform_ip_address_sync(NMPlatform *self,
 
             g_ptr_array_sort_with_data(plat_addresses, ip6_address_scope_cmp_descending, NULL);
 
-            known_addresses_len = nm_g_ptr_array_len(known_addresses);
-
             /* 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++) {
@@ -4302,7 +4298,8 @@ nm_platform_ip_address_sync(NMPlatform *self,
             cur_scope              = IP6_ADDR_SCOPE_LOOPBACK;
             delete_remaining_addrs = FALSE;
             i_plat                 = plat_addresses->len;
-            i_know                 = 0;
+            i_know                 = nm_g_ptr_array_len(known_addresses);
+
             while (i_plat > 0) {
                 const NMPlatformIP6Address *plat_addr =
                     NMP_OBJECT_CAST_IP6_ADDRESS(plat_addresses->pdata[--i_plat]);
@@ -4320,9 +4317,9 @@ nm_platform_ip_address_sync(NMPlatform *self,
 
                 if (!delete_remaining_addrs) {
                     delete_remaining_addrs = TRUE;
-                    for (; i_know < known_addresses_len; i_know++) {
+                    while (i_know > 0) {
                         const NMPlatformIP6Address *know_addr =
-                            NMP_OBJECT_CAST_IP6_ADDRESS(known_addresses->pdata[i_know]);
+                            NMP_OBJECT_CAST_IP6_ADDRESS(known_addresses->pdata[--i_know]);
                         IP6AddrScope know_scope;
 
                         if (!know_addr)
@@ -4359,13 +4356,21 @@ next_plat:;
     /* Add missing addresses. New addresses are added by kernel with top
      * priority.
      */
-    for (i_know = 0; i_know < known_addresses->len; i_know++) {
+    for (i = 0; i < known_addresses->len; i++) {
         const NMPObject            *plat_obj;
         const NMPObject            *known_obj;
         const NMPlatformIPXAddress *known_address;
         guint32                     lifetime;
         guint32                     preferred;
 
+        /* IPv4 addresses we need to add in the order most important first.
+         * IPv6 addresses we need to add in the reverse order with least
+         *   important first. Kernel will interpret the last address as most
+         *   important.
+         *
+         * @known_addresses is always in the order most-important-first. */
+        i_know = IS_IPv4 ? i : (known_addresses->len - i - 1u);
+
         known_obj = known_addresses->pdata[i_know];
         if (!known_obj)
             continue;