From 5aa50d7c87c85591f2dada6dd0af8efdfb1cf837 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 7 Aug 2019 12:51:23 +0200 Subject: [PATCH 1/3] core: fix wrongly generating "Wired connection 1" (auto-default) for ethernet with MAC If a profile has only "ethernet.mac-address" set, but "connection.interface-name" not, then the previous check iface = nm_setting_connection_get_interface_name (s_con); if (!nm_streq0 (iface, nm_device_get_iface (device))) continue; would wrongly consider the profile not matching for the device. As a result, we would wrongly create a auto-default connection. Fix that. We already call nm_device_check_connection_compatible() above. That is fully suitable to compare the interface name and the MAC address. We don't need to duplicate this check (wrongly). See also commit 77d01c909470 ('settings: ignore incompatible connections when looking for existing ones') for how this code changed. https://bugzilla.redhat.com/show_bug.cgi?id=1727909 --- src/settings/nm-settings.c | 33 +++------------------------------ 1 file changed, 3 insertions(+), 30 deletions(-) diff --git a/src/settings/nm-settings.c b/src/settings/nm-settings.c index f474324c20..45cfbd0683 100644 --- a/src/settings/nm-settings.c +++ b/src/settings/nm-settings.c @@ -3355,23 +3355,16 @@ static gboolean have_connection_for_device (NMSettings *self, NMDevice *device) { NMSettingsPrivate *priv = NM_SETTINGS_GET_PRIVATE (self); - NMSettingWired *s_wired; - const char *setting_hwaddr; - const char *perm_hw_addr; NMSettingsConnection *sett_conn; g_return_val_if_fail (NM_IS_SETTINGS (self), FALSE); - perm_hw_addr = nm_device_get_permanent_hw_address (device); - - /* Find a wired connection locked to the given MAC address, if any */ + /* Find a wired connection matching for the device, if any */ c_list_for_each_entry (sett_conn, &priv->connections_lst_head, _connections_lst) { NMConnection *connection = nm_settings_connection_get_connection (sett_conn); - NMSettingConnection *s_con = nm_connection_get_setting_connection (connection); const char *ctype; - const char *iface; - ctype = nm_setting_connection_get_connection_type (s_con); + ctype = nm_connection_get_connection_type (connection); if (!NM_IN_STRSET (ctype, NM_SETTING_WIRED_SETTING_NAME, NM_SETTING_PPPOE_SETTING_NAME)) continue; @@ -3386,27 +3379,7 @@ have_connection_for_device (NMSettings *self, NMDevice *device) NM_SETTINGS_CONNECTION_INT_FLAGS_VOLATILE)) continue; - iface = nm_setting_connection_get_interface_name (s_con); - if (!nm_streq0 (iface, nm_device_get_iface (device))) - continue; - - s_wired = nm_connection_get_setting_wired (connection); - if ( !s_wired - && nm_streq (ctype, NM_SETTING_PPPOE_SETTING_NAME)) { - /* No wired setting; therefore the PPPoE connection applies to any device */ - return TRUE; - } - - setting_hwaddr = nm_setting_wired_get_mac_address (s_wired); - if (setting_hwaddr) { - /* A connection mac-locked to this device */ - if ( perm_hw_addr - && nm_utils_hwaddr_matches (setting_hwaddr, -1, perm_hw_addr, -1)) - return TRUE; - } else { - /* A connection that applies to any wired device */ - return TRUE; - } + return TRUE; } /* See if there's a known non-NetworkManager configuration for the device */ From 3e39d2a5861a162cd6afb92ff60bee71fb0b2517 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 7 Aug 2019 13:01:45 +0200 Subject: [PATCH 2/3] settings: shortcut check for whether to create auto-default wired connection This check is only useful for devices that implement new_default_connection. We can shortcut the possibly expensive checks like have_connection_for_device(), which need to iterate all profiles. --- src/settings/nm-settings.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/settings/nm-settings.c b/src/settings/nm-settings.c index 45cfbd0683..09de8de8ed 100644 --- a/src/settings/nm-settings.c +++ b/src/settings/nm-settings.c @@ -3435,7 +3435,8 @@ device_realized (NMDevice *device, GParamSpec *pspec, NMSettings *self) /* If the device isn't managed or it already has a default wired connection, * ignore it. */ - if ( !nm_device_get_managed (device, FALSE) + if ( !NM_DEVICE_GET_CLASS (self)->new_default_connection + || !nm_device_get_managed (device, FALSE) || g_object_get_qdata (G_OBJECT (device), _default_wired_connection_quark ()) || have_connection_for_device (self, device) || nm_config_get_no_auto_default_for_device (priv->config, device)) From 2a506d8a0964424a0df04319a629082969e4fe63 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 7 Aug 2019 13:02:48 +0200 Subject: [PATCH 3/3] settings: drop redundant check from have_connection_for_device() have_connection_for_device() really should just call nm_device_check_connection_compatible(). Note that nm_device_check_connection_compatible() of course checks the connection type already, so this is redundant. --- src/settings/nm-settings.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/settings/nm-settings.c b/src/settings/nm-settings.c index 09de8de8ed..fdd5835c59 100644 --- a/src/settings/nm-settings.c +++ b/src/settings/nm-settings.c @@ -3362,12 +3362,6 @@ have_connection_for_device (NMSettings *self, NMDevice *device) /* Find a wired connection matching for the device, if any */ c_list_for_each_entry (sett_conn, &priv->connections_lst_head, _connections_lst) { NMConnection *connection = nm_settings_connection_get_connection (sett_conn); - const char *ctype; - - ctype = nm_connection_get_connection_type (connection); - if (!NM_IN_STRSET (ctype, NM_SETTING_WIRED_SETTING_NAME, - NM_SETTING_PPPOE_SETTING_NAME)) - continue; if (!nm_device_check_connection_compatible (device, connection, NULL)) continue;