config: drop slaves-order config option

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
This commit is contained in:
Fernando Fernandez Mancera 2023-12-06 10:16:55 +01:00
parent d256831510
commit 6576ddc532
7 changed files with 22 additions and 54 deletions

View file

@ -460,20 +460,6 @@ no-auto-default=*
</listitem>
</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>

View file

@ -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, ),
},
{

View file

@ -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 =

View file

@ -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);
}
/*****************************************************************************/

View file

@ -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"

View file

@ -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++) {

View file

@ -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,