config: drop slaves-order config optionff/drop_slaves_order

This option was only introduced only to allow keeping the old behavior
in RHEL7, while the default order was changed from 'ifindex' to 'name'
in RHEL8. The usefulness of this option is questionable, as 'name'
together with predictable interface names should give predictable order.
When not using predictable interface names, the name is unpredictable
but so is the ifindex.

https://issues.redhat.com/browse/NMT-926

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

7 files changed, 22 insertions, 54 deletions

diff --git a/man/NetworkManager.conf.xml b/man/NetworkManager.conf.xml
index b331beb9dc..5a7f0d83d5 100644
--- a/man/NetworkManager.conf.xml
+++ b/man/NetworkManager.conf.xml
@@ -461,20 +461,6 @@ no-auto-default=*
       </varlistentry>
 
       <varlistentry>
-        <term><varname>slaves-order</varname></term>
-        <listitem>
-          <para>
-            This key specifies in which order slave connections are
-            auto-activated on boot or when the master activates
-            them. Allowed values are <literal>name</literal> (order
-            connection by interface name, the default), or
-            <literal>index</literal> (order slaves by their kernel
-            index).
-          </para>
-        </listitem>
-      </varlistentry>
-
-      <varlistentry>
         <term><varname>firewall-backend</varname></term>
         <listitem>
           <para>
diff --git a/src/core/nm-config.c b/src/core/nm-config.c
index 31d758f882..5db4a92a4f 100644
--- a/src/core/nm-config.c
+++ b/src/core/nm-config.c
@@ -858,7 +858,6 @@ static const ConfigGroup config_groups[] = {
                              NM_CONFIG_KEYFILE_KEY_MAIN_NO_AUTO_DEFAULT,
                              NM_CONFIG_KEYFILE_KEY_MAIN_PLUGINS,
                              NM_CONFIG_KEYFILE_KEY_MAIN_RC_MANAGER,
-                             NM_CONFIG_KEYFILE_KEY_MAIN_SLAVES_ORDER,
                              NM_CONFIG_KEYFILE_KEY_MAIN_SYSTEMD_RESOLVED, ),
     },
     {
diff --git a/src/core/nm-manager.c b/src/core/nm-manager.c
index bd2a49b53a..f550cca17f 100644
--- a/src/core/nm-manager.c
+++ b/src/core/nm-manager.c
@@ -4424,11 +4424,7 @@ platform_query_devices(NMManager *self)
     gs_free char                *order = NULL;
 
     guess_assume = nm_config_get_first_start(nm_config_get());
-    order        = nm_config_data_get_value(NM_CONFIG_GET_DATA,
-                                     NM_CONFIG_KEYFILE_GROUP_MAIN,
-                                     NM_CONFIG_KEYFILE_KEY_MAIN_SLAVES_ORDER,
-                                     NM_CONFIG_GET_VALUE_STRIP);
-    links        = nm_platform_link_get_all(priv->platform, !nm_streq0(order, "index"));
+    links        = nm_platform_link_get_all(priv->platform);
     if (!links)
         return;
     for (i = 0; i < links->len; i++) {
@@ -5364,7 +5360,7 @@ out:
 }
 
 static int
-compare_slaves(gconstpointer a, gconstpointer b, gpointer sort_by_name)
+compare_slaves(gconstpointer a, gconstpointer b)
 {
     const SlaveConnectionInfo *a_info = a;
     const SlaveConnectionInfo *b_info = b;
@@ -5375,11 +5371,7 @@ compare_slaves(gconstpointer a, gconstpointer b, gpointer sort_by_name)
     if (!b_info->device)
         return -1;
 
-    if (GPOINTER_TO_INT(sort_by_name)) {
-        return nm_strcmp0(nm_device_get_iface(a_info->device), nm_device_get_iface(b_info->device));
-    }
-
-    return nm_device_get_ifindex(a_info->device) - nm_device_get_ifindex(b_info->device);
+    return nm_strcmp0(nm_device_get_iface(a_info->device), nm_device_get_iface(b_info->device));
 }
 
 static void
@@ -5399,17 +5391,7 @@ autoconnect_slaves(NMManager            *self,
 
         slaves = find_slaves(self, master_connection, master_device, &n_slaves, for_user_request);
         if (n_slaves > 1) {
-            gs_free char *value = NULL;
-
-            value = nm_config_data_get_value(NM_CONFIG_GET_DATA,
-                                             NM_CONFIG_KEYFILE_GROUP_MAIN,
-                                             NM_CONFIG_KEYFILE_KEY_MAIN_SLAVES_ORDER,
-                                             NM_CONFIG_GET_VALUE_STRIP);
-            g_qsort_with_data(slaves,
-                              n_slaves,
-                              sizeof(slaves[0]),
-                              compare_slaves,
-                              GINT_TO_POINTER(!nm_streq0(value, "index")));
+            qsort(slaves, n_slaves, sizeof(slaves[0]), compare_slaves);
         }
 
         bind_lifetime_to_profile_visibility =
diff --git a/src/core/platform/tests/test-platform-general.c b/src/core/platform/tests/test-platform-general.c
index cebd44f65b..fd6aae8265 100644
--- a/src/core/platform/tests/test-platform-general.c
+++ b/src/core/platform/tests/test-platform-general.c
@@ -44,7 +44,7 @@ test_link_get_all(void)
 
     platform = nm_linux_platform_new(NULL, TRUE, NM_PLATFORM_NETNS_SUPPORT_DEFAULT, TRUE);
 
-    links = nm_platform_link_get_all(platform, TRUE);
+    links = nm_platform_link_get_all(platform);
 }
 
 /*****************************************************************************/
diff --git a/src/libnm-base/nm-config-base.h b/src/libnm-base/nm-config-base.h
index c90efc1b7c..6d050ae176 100644
--- a/src/libnm-base/nm-config-base.h
+++ b/src/libnm-base/nm-config-base.h
@@ -35,7 +35,6 @@
 #define NM_CONFIG_KEYFILE_KEY_MAIN_NO_AUTO_DEFAULT             "no-auto-default"
 #define NM_CONFIG_KEYFILE_KEY_MAIN_PLUGINS                     "plugins"
 #define NM_CONFIG_KEYFILE_KEY_MAIN_RC_MANAGER                  "rc-manager"
-#define NM_CONFIG_KEYFILE_KEY_MAIN_SLAVES_ORDER                "slaves-order"
 #define NM_CONFIG_KEYFILE_KEY_MAIN_SYSTEMD_RESOLVED            "systemd-resolved"
 
 #define NM_CONFIG_KEYFILE_KEY_LOGGING_AUDIT   "audit"
diff --git a/src/libnm-platform/nm-platform.c b/src/libnm-platform/nm-platform.c
index 5a12d92da1..531c98e8dd 100644
--- a/src/libnm-platform/nm-platform.c
+++ b/src/libnm-platform/nm-platform.c
@@ -991,7 +991,7 @@ nm_platform_sysctl_ip_conf_get_rp_filter_ipv4(NMPlatform *self,
 /*****************************************************************************/
 
 static int
-_link_get_all_presort(gconstpointer p_a, gconstpointer p_b, gpointer sort_by_name)
+_link_get_all_presort(gconstpointer p_a, gconstpointer p_b)
 {
     const NMPlatformLink *a = NMP_OBJECT_CAST_LINK(*((const NMPObject **) p_a));
     const NMPlatformLink *b = NMP_OBJECT_CAST_LINK(*((const NMPObject **) p_b));
@@ -1002,28 +1002,30 @@ _link_get_all_presort(gconstpointer p_a, gconstpointer p_b, gpointer sort_by_nam
     if (b->ifindex == NM_LOOPBACK_IFINDEX)
         return 1;
 
-    if (GPOINTER_TO_INT(sort_by_name)) {
-        /* Initialized links first */
-        if (a->initialized > b->initialized)
-            return -1;
-        if (a->initialized < b->initialized)
-            return 1;
+    /* Initialized links first */
+    if (a->initialized > b->initialized)
+        return -1;
+    if (a->initialized < b->initialized)
+        return 1;
 
-        return strcmp(a->name, b->name);
-    } else
-        return a->ifindex - b->ifindex;
+    NM_CMP_DIRECT_STRCMP(a->name, b->name);
+    /* Fallback to ifindex */
+    NM_CMP_DIRECT(a->ifindex, b->ifindex);
+    /* Fallback to pointer comparison */
+    NM_CMP_DIRECT_PTR(a, b);
+
+    return 0;
 }
 
 /**
  * nm_platform_link_get_all:
  * @self: platform instance
- * @sort_by_name: whether to sort by name or ifindex.
  *
  * Retrieve a snapshot of configuration for all links at once. The result is
  * owned by the caller and should be freed with g_ptr_array_unref().
  */
 GPtrArray *
-nm_platform_link_get_all(NMPlatform *self, gboolean sort_by_name)
+nm_platform_link_get_all(NMPlatform *self)
 {
     gs_unref_ptrarray GPtrArray   *links = NULL;
     GPtrArray                     *result;
@@ -1049,9 +1051,9 @@ nm_platform_link_get_all(NMPlatform *self, gboolean sort_by_name)
     if (links->len == 0)
         return NULL;
 
-    /* first sort the links by their ifindex or name. Below we will sort
+    /* first sort the links by their name. Below we will sort
      * further by moving children/slaves to the end. */
-    g_ptr_array_sort_with_data(links, _link_get_all_presort, GINT_TO_POINTER(sort_by_name));
+    g_ptr_array_sort(links, _link_get_all_presort);
 
     unseen = g_hash_table_new(nm_direct_hash, NULL);
     for (i = 0; i < links->len; i++) {
diff --git a/src/libnm-platform/nm-platform.h b/src/libnm-platform/nm-platform.h
index 810a59133d..996f84b8bd 100644
--- a/src/libnm-platform/nm-platform.h
+++ b/src/libnm-platform/nm-platform.h
@@ -1651,7 +1651,7 @@ const NMPlatformLink *nm_platform_link_get_by_address(NMPlatform   *self,
                                                       gconstpointer address,
                                                       size_t        length);
 
-GPtrArray *nm_platform_link_get_all(NMPlatform *self, gboolean sort_by_name);
+GPtrArray *nm_platform_link_get_all(NMPlatform *self);
 
 int nm_platform_link_add(NMPlatform            *self,
                          NMLinkType             type,