l3cfg: merge branch 'th/device-manual-l3cd-prio'

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

4 files changed, 41 insertions, 10 deletions

diff --git a/src/core/devices/nm-device.c b/src/core/devices/nm-device.c
index bc23ef0198..a39aec16fb 100644
--- a/src/core/devices/nm-device.c
+++ b/src/core/devices/nm-device.c
@@ -137,9 +137,8 @@ typedef struct {
 typedef enum {
     /* The various NML3ConfigData types that we track explicitly. Note that
      * their relative order matters: higher numbers in this enum means more
-     * important (and during merge overwrites other settings). */
-
-    L3_CONFIG_DATA_TYPE_MANUALIP,
+     * important (and during merge overwrites other settings). This is passed
+     * as priority to nm_l3cfg_add_config(). */
 
     L3_CONFIG_DATA_TYPE_LL_4,
     L3_CONFIG_DATA_TYPE_LL_6,
@@ -183,12 +182,16 @@ typedef enum {
         _t;                                        \
     })
 
+    L3_CONFIG_DATA_TYPE_MANUALIP,
+
     _L3_CONFIG_DATA_TYPE_NUM,
     _L3_CONFIG_DATA_TYPE_NONE,
     _L3_CONFIG_DATA_TYPE_ACD_ONLY,
 } L3ConfigDataType;
 
 G_STATIC_ASSERT(NM_L3CFG_CONFIG_PRIORITY_IPV4LL == L3_CONFIG_DATA_TYPE_LL_4);
+G_STATIC_ASSERT(NM_L3CFG_CONFIG_PRIORITY_IPV6LL == L3_CONFIG_DATA_TYPE_LL_6);
+G_STATIC_ASSERT(NM_L3CFG_CONFIG_PRIORITY_VPN == L3_CONFIG_DATA_TYPE_DEVIP_6);
 
 typedef enum {
     HW_ADDR_TYPE_UNSET = 0,
diff --git a/src/core/nm-l3cfg.c b/src/core/nm-l3cfg.c
index f36d798d2c..b42ff8cb75 100644
--- a/src/core/nm-l3cfg.c
+++ b/src/core/nm-l3cfg.c
@@ -3055,9 +3055,9 @@ _l3_config_datas_get_sorted_cmp(gconstpointer p_a, gconstpointer p_b, gpointer u
     nm_assert(b);
     nm_assert(nm_l3_config_data_get_ifindex(a->l3cd) == nm_l3_config_data_get_ifindex(b->l3cd));
 
-    /* we sort the entries with higher priority (more important, lower numerical value)
+    /* we sort the entries with higher priority (higher numerical value, more important)
      * first. */
-    NM_CMP_FIELD(a, b, priority_confdata);
+    NM_CMP_FIELD(b, a, priority_confdata);
 
     /* if the priority is not unique, we sort them in the order they were added,
      * with the oldest first (lower numerical value). */
@@ -3105,6 +3105,14 @@ nm_l3cfg_mark_config_dirty(NML3Cfg *self, gconstpointer tag, gboolean dirty)
     }
 }
 
+/*
+ * nm_l3cfg_add_config:
+ * @priority: all l3cd get merged/combined. This merging requires that some
+ *   l3cd are more important than others. For example, coming from static IP
+ *   configuration needs to take precedence over DHCP. The @priority determines
+ *   the order in which l3cds get merged (and thus the outcome). Higher numbers
+ *   mean more important!!
+ */
 gboolean
 nm_l3cfg_add_config(NML3Cfg              *self,
                     gconstpointer         tag,
@@ -3470,6 +3478,20 @@ _l3cfg_update_combined_config(NML3Cfg               *self,
         l3_config_datas_arr[i] = _l3_config_datas_at(self->priv.p->l3_config_datas, i);
 
     if (l3_config_datas_len > 1) {
+        /* We are about to merge the l3cds. The order in which we do that matters.
+         *
+         * Below, we iterate over the l3cds and merge them into a new one. nm_l3_config_data_merge()
+         * uses "NM_L3_CONFIG_ADD_FLAGS_EXCLUSIVE" flag, which means to keep the first entry.
+         *
+         * Consider for example addresses/routes, which have a set of ID attributes  (based on
+         * which no duplicates can be accepted) and additional attributes. For example, trying
+         * to add the same address twice ("same" according to their ID), only one can be added.
+         * If they differ in their lifetimes, we need to make a choice.
+         * We could merge the attributes in a sensible way. Instead, NM_L3_CONFIG_ADD_FLAGS_EXCLUSIVE
+         * takes care to only take the first one.
+         *
+         * So we want to sort the more important entries *first*, and this is based on
+         * the priority_confdata.  */
         g_qsort_with_data(l3_config_datas_arr,
                           l3_config_datas_len,
                           sizeof(l3_config_datas_arr[0]),
@@ -3499,6 +3521,14 @@ _l3cfg_update_combined_config(NML3Cfg               *self,
         for (i = 0; i < l3_config_datas_len; i++) {
             const L3ConfigData *l3cd_data = l3_config_datas_arr[i];
 
+            /* more important entries must be sorted *first*. */
+            nm_assert(
+                i == 0
+                || (l3_config_datas_arr[i - 1]->priority_confdata > l3cd_data->priority_confdata)
+                || (l3_config_datas_arr[i - 1]->priority_confdata == l3cd_data->priority_confdata
+                    && l3_config_datas_arr[i - 1]->pseudo_timestamp_confdata
+                           < l3cd_data->pseudo_timestamp_confdata));
+
             if (NM_FLAGS_HAS(l3cd_data->config_flags, NM_L3CFG_CONFIG_FLAGS_ONLY_FOR_ACD))
                 continue;
 
diff --git a/src/core/nm-l3cfg.h b/src/core/nm-l3cfg.h
index d994efde32..6fd8f9de80 100644
--- a/src/core/nm-l3cfg.h
+++ b/src/core/nm-l3cfg.h
@@ -6,8 +6,9 @@
 #include "libnm-platform/nmp-object.h"
 #include "nm-l3-config-data.h"
 
-#define NM_L3CFG_CONFIG_PRIORITY_IPV4LL 1
+#define NM_L3CFG_CONFIG_PRIORITY_IPV4LL 0
 #define NM_L3CFG_CONFIG_PRIORITY_IPV6LL 1
+#define NM_L3CFG_CONFIG_PRIORITY_VPN    9
 #define NM_ACD_TIMEOUT_RFC5227_MSEC     9000u
 
 #define NM_TYPE_L3CFG            (nm_l3cfg_get_type())
diff --git a/src/core/vpn/nm-vpn-connection.c b/src/core/vpn/nm-vpn-connection.c
index 14ef1c4cac..bbb7355016 100644
--- a/src/core/vpn/nm-vpn-connection.c
+++ b/src/core/vpn/nm-vpn-connection.c
@@ -716,7 +716,6 @@ _l3cfg_l3cd_update(NMVpnConnection *self, L3CDType l3cd_type)
 {
     NMVpnConnectionPrivate      *priv = NM_VPN_CONNECTION_GET_PRIVATE(self);
     NML3Cfg                     *l3cfg;
-    int                          priority;
     const NML3ConfigData *const *p_l3cd;
 
     if (NM_IN_SET(l3cd_type, L3CD_TYPE_IP_4, L3CD_TYPE_IP_6, L3CD_TYPE_GENERIC, L3CD_TYPE_STATIC)) {
@@ -743,13 +742,11 @@ _l3cfg_l3cd_update(NMVpnConnection *self, L3CDType l3cd_type)
         goto handle_changed;
     }
 
-    priority = 0;
-
     if (!nm_l3cfg_add_config(l3cfg,
                              p_l3cd,
                              TRUE,
                              *p_l3cd,
-                             priority,
+                             NM_L3CFG_CONFIG_PRIORITY_VPN,
                              get_route_table(self, AF_INET, TRUE),
                              get_route_table(self, AF_INET6, TRUE),
                              nm_vpn_connection_get_ip_route_metric(self, AF_INET),