core: merge branch 'th/ethtool-reset-autoneg'

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/356
https://bugzilla.redhat.com/show_bug.cgi?id=1807171

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

4 files changed, 108 insertions, 98 deletions

diff --git a/src/devices/nm-device-ethernet.c b/src/devices/nm-device-ethernet.c
index 7556bb27ba..9e7601f005 100644
--- a/src/devices/nm-device-ethernet.c
+++ b/src/devices/nm-device-ethernet.c
@@ -100,7 +100,15 @@ typedef struct _NMDeviceEthernetPrivate {
 	DcbWait       dcb_wait;
 	guint         dcb_timeout_id;
 
+	guint32       ethtool_prev_speed;
+
+	NMPlatformLinkDuplexType ethtool_prev_duplex:3;
+
 	bool          dcb_handle_carrier_changes:1;
+
+	bool          ethtool_prev_set:1;
+	bool          ethtool_prev_autoneg:1;
+
 } NMDeviceEthernetPrivate;
 
 NM_GOBJECT_PROPERTIES_DEFINE (NMDeviceEthernet,
@@ -882,6 +890,7 @@ static void
 link_negotiation_set (NMDevice *device)
 {
 	NMDeviceEthernet *self = NM_DEVICE_ETHERNET (device);
+	NMDeviceEthernetPrivate *priv = NM_DEVICE_ETHERNET_GET_PRIVATE (self);
 	NMSettingWired *s_wired;
 	gboolean autoneg = TRUE;
 	gboolean link_autoneg;
@@ -925,7 +934,7 @@ link_negotiation_set (NMDevice *device)
 	    && !duplex)
 		_LOGD (LOGD_DEVICE, "set-link: configure auto-negotiation");
 	else {
-		_LOGD (LOGD_DEVICE, "set-link: configure %snegotiation (%u Mbit%s - %s duplex%s)",
+		_LOGD (LOGD_DEVICE, "set-link: configure %snegotiation (%u Mbit%s, %s duplex%s)",
 		       autoneg ? "auto-" : "static ",
 		       speed ?: link_speed,
 		       speed ? "" : "*",
@@ -935,6 +944,14 @@ link_negotiation_set (NMDevice *device)
 		       duplex ? "" : "*");
 	}
 
+	if (!priv->ethtool_prev_set) {
+		/* remember the values we had before setting it. */
+		priv->ethtool_prev_autoneg = link_autoneg;
+		priv->ethtool_prev_speed = link_speed;
+		priv->ethtool_prev_duplex = link_duplex;
+		priv->ethtool_prev_set = TRUE;
+	}
+
 	if (!nm_platform_ethtool_set_link_settings (nm_device_get_platform (device),
 	                                            nm_device_get_ifindex (device),
 	                                            autoneg,
@@ -964,6 +981,28 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason)
 	NMDeviceEthernet *self = NM_DEVICE_ETHERNET (device);
 	NMDeviceEthernetPrivate *priv = NM_DEVICE_ETHERNET_GET_PRIVATE (self);
 
+	if (nm_device_sys_iface_state_is_external_or_assume (device)) {
+		if (   !priv->ethtool_prev_set
+		    && !nm_device_sys_iface_state_is_external (device)) {
+			NMSettingWired *s_wired;
+
+			/* During restart of NetworkManager service we forget the original auto
+			 * negotiation settings. When taking over a device, remember to reset
+			 * the "default" during deactivate. */
+			s_wired = nm_device_get_applied_setting (device, NM_TYPE_SETTING_WIRED);
+			if (   s_wired
+			    && (   nm_setting_wired_get_auto_negotiate (s_wired)
+			        || nm_setting_wired_get_speed (s_wired)
+			        || nm_setting_wired_get_duplex (s_wired))) {
+				priv->ethtool_prev_set = TRUE;
+				priv->ethtool_prev_autoneg = TRUE;
+				priv->ethtool_prev_speed = 0;
+				priv->ethtool_prev_duplex = NM_PLATFORM_LINK_DUPLEX_UNKNOWN;
+			}
+		}
+		return NM_ACT_STAGE_RETURN_SUCCESS;
+	}
+
 	link_negotiation_set (device);
 
 	/* If we're re-activating a PPPoE connection a short while after
@@ -1525,6 +1564,23 @@ deactivate (NMDevice *device)
 	/* Set last PPPoE connection time */
 	if (nm_device_get_applied_setting (device, NM_TYPE_SETTING_PPPOE))
 		priv->last_pppoe_time = nm_utils_get_monotonic_timestamp_sec ();
+
+	if (priv->ethtool_prev_set) {
+		priv->ethtool_prev_set = FALSE;
+
+		_LOGD (LOGD_DEVICE, "set-link: reset %snegotiation (%u Mbit, %s duplex)",
+		       priv->ethtool_prev_autoneg ? "auto-" : "static ",
+		       priv->ethtool_prev_speed,
+		       nm_platform_link_duplex_type_to_string (priv->ethtool_prev_duplex));
+		if (!nm_platform_ethtool_set_link_settings (nm_device_get_platform (device),
+		                                            nm_device_get_ifindex (device),
+		                                            priv->ethtool_prev_autoneg,
+		                                            priv->ethtool_prev_speed,
+		                                            priv->ethtool_prev_duplex)) {
+			_LOGW (LOGD_DEVICE, "set-link: failure to reset link negotiation");
+			return;
+		}
+	}
 }
 
 static gboolean
@@ -1914,6 +1970,7 @@ nm_device_ethernet_class_init (NMDeviceEthernetClass *klass)
 	device_class->complete_connection = complete_connection;
 	device_class->new_default_connection = new_default_connection;
 
+	device_class->act_stage1_prepare_also_for_external_or_assume = TRUE;
 	device_class->act_stage1_prepare = act_stage1_prepare;
 	device_class->act_stage1_prepare_set_hwaddr_ethernet = TRUE;
 	device_class->act_stage2_config = act_stage2_config;
diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c
index 9b018ccc8c..327df52c6c 100644
--- a/src/devices/nm-device.c
+++ b/src/devices/nm-device.c
@@ -950,50 +950,6 @@ _ethtool_coalesce_set (NMDevice *self,
 	_LOGD (LOGD_DEVICE, "ethtool: coalesce settings successfully set");
 }
 
-static gboolean
-_ethtool_init_ring (NMDevice *self,
-                    NMPlatform *platform,
-                    NMSettingEthtool *s_ethtool,
-                    NMEthtoolRingState *ring)
-{
-	GHashTable *hash;
-	GHashTableIter iter;
-	const char *name;
-	GVariant *variant;
-	gsize n_ring_set = 0;
-
-	nm_assert (self);
-	nm_assert (platform);
-	nm_assert (ring);
-	nm_assert (NM_IS_SETTING_ETHTOOL (s_ethtool));
-
-	hash = _nm_setting_option_hash (NM_SETTING (s_ethtool), FALSE);
-	if (!hash)
-		return FALSE;
-
-	g_hash_table_iter_init (&iter, hash);
-	while (g_hash_table_iter_next (&iter, (gpointer *) &name, (gpointer *) &variant)) {
-		if (!g_variant_is_of_type (variant, G_VARIANT_TYPE_UINT32))
-			continue;
-		if (!nm_ethtool_optname_is_ring (name))
-			continue;
-
-		if (!nm_platform_ethtool_init_ring (platform,
-		                                    ring,
-		                                    name,
-		                                    g_variant_get_uint32(variant))) {
-			_LOGW (LOGD_DEVICE, "ethtool: invalid ring setting %s", name);
-		    return FALSE;
-		}
-		++n_ring_set;
-
-	}
-
-	return !!n_ring_set;
-}
-
-
-
 static void
 _ethtool_ring_reset (NMDevice *self,
                      NMPlatform *platform,
@@ -1025,25 +981,64 @@ _ethtool_ring_set (NMDevice *self,
 {
 	NMEthtoolRingState ring_old;
 	NMEthtoolRingState ring_new;
+	GHashTable *hash;
+	GHashTableIter iter;
+	const char *name;
+	GVariant *variant;
+	gboolean has_old = FALSE;
 
-	nm_assert (ethtool_state);
 	nm_assert (NM_IS_DEVICE (self));
 	nm_assert (NM_IS_PLATFORM (platform));
 	nm_assert (NM_IS_SETTING_ETHTOOL (s_ethtool));
+	nm_assert (ethtool_state);
+	nm_assert (!ethtool_state->ring);
 
-	if (!nm_platform_ethtool_get_link_ring (platform,
-	                                        ethtool_state->ifindex,
-	                                        &ring_old)) {
-		_LOGW (LOGD_DEVICE, "ethtool: failure getting ring settings (cannot read)");
+	hash = _nm_setting_option_hash (NM_SETTING (s_ethtool), FALSE);
+	if (!hash)
 		return;
-	}
 
-	ring_new = ring_old;
+	g_hash_table_iter_init (&iter, hash);
+	while (g_hash_table_iter_next (&iter, (gpointer *) &name, (gpointer *) &variant)) {
+		NMEthtoolID ethtool_id = nm_ethtool_id_get_by_name (name);
+		guint32 u32;
+
+		if (!nm_ethtool_id_is_ring (ethtool_id))
+			continue;
+
+		nm_assert (g_variant_is_of_type (variant, G_VARIANT_TYPE_UINT32));
 
-	if (!_ethtool_init_ring (self,
-	                         platform,
-	                         s_ethtool,
-	                         &ring_new))
+		if (!has_old) {
+			if (!nm_platform_ethtool_get_link_ring (platform,
+			                                        ethtool_state->ifindex,
+			                                        &ring_old)) {
+				_LOGW (LOGD_DEVICE, "ethtool: failure setting ring options (cannot read existing setting)");
+				return;
+			}
+			has_old = TRUE;
+			ring_new = ring_old;
+		}
+
+		u32 = g_variant_get_uint32 (variant);
+
+		switch (ethtool_id) {
+		case NM_ETHTOOL_ID_RING_RX:
+			ring_new.rx_pending = u32;
+			break;
+		case NM_ETHTOOL_ID_RING_RX_JUMBO:
+			ring_new.rx_jumbo_pending = u32;
+			break;
+		case NM_ETHTOOL_ID_RING_RX_MINI:
+			ring_new.rx_mini_pending = u32;
+			break;
+		case NM_ETHTOOL_ID_RING_TX:
+			ring_new.tx_pending = u32;
+			break;
+		default:
+			nm_assert_not_reached ();
+		}
+	}
+
+	if (!has_old)
 		return;
 
 	ethtool_state->ring = nm_memdup (&ring_old, sizeof (ring_old));
@@ -1076,8 +1071,6 @@ _ethtool_state_reset (NMDevice *self)
 		_ethtool_ring_reset (self, platform, ethtool_state);
 }
 
-
-
 static void
 _ethtool_state_set (NMDevice *self)
 {
diff --git a/src/platform/nm-platform.c b/src/platform/nm-platform.c
index b7a43a0ea1..2f2f99f00a 100644
--- a/src/platform/nm-platform.c
+++ b/src/platform/nm-platform.c
@@ -3253,41 +3253,6 @@ nm_platform_ethtool_get_link_ring (NMPlatform *self,
 }
 
 gboolean
-nm_platform_ethtool_init_ring (NMPlatform *self,
-                               NMEthtoolRingState *ring,
-                               const char *option_name,
-                               guint32 value)
-{
-	NMEthtoolID ethtool_id;
-
-	g_return_val_if_fail (ring, FALSE);
-	g_return_val_if_fail (option_name, FALSE);
-
-	ethtool_id = nm_ethtool_id_get_by_name (option_name);
-
-	g_return_val_if_fail (nm_ethtool_id_is_ring (ethtool_id), FALSE);
-
-	switch (ethtool_id) {
-		case NM_ETHTOOL_ID_RING_RX:
-			ring->rx_pending = value;
-			break;
-		case NM_ETHTOOL_ID_RING_RX_JUMBO:
-			ring->rx_jumbo_pending = value;
-			break;
-		case NM_ETHTOOL_ID_RING_RX_MINI:
-			ring->rx_mini_pending = value;
-			break;
-		case NM_ETHTOOL_ID_RING_TX:
-			ring->tx_pending = value;
-			break;
-		default:
-			g_return_val_if_reached (FALSE);
-	}
-
-	return TRUE;
-}
-
-gboolean
 nm_platform_ethtool_set_ring (NMPlatform *self,
                               int ifindex,
                               const NMEthtoolRingState *ring)
diff --git a/src/platform/nm-platform.h b/src/platform/nm-platform.h
index fb82262979..c4eda16cc4 100644
--- a/src/platform/nm-platform.h
+++ b/src/platform/nm-platform.h
@@ -1975,11 +1975,6 @@ gboolean nm_platform_ethtool_get_link_ring (NMPlatform *self,
                                             int ifindex,
                                             NMEthtoolRingState *ring);
 
-gboolean nm_platform_ethtool_init_ring (NMPlatform *self,
-                                        NMEthtoolRingState *ring,
-                                        const char *option_name,
-                                        guint32 value);
-
 gboolean nm_platform_ethtool_set_ring (NMPlatform *self,
                                        int ifindex,
                                        const NMEthtoolRingState *ring);