device: rework mtu priority handlingbg/mtu-rh1586191

If commit_mtu() is called multiple times and dev->get_configured_mtu()
returns @is_user_config=FALSE, only the first call changes the
MTU. So, for example, when the parent MTU of a VLAN changes, we apply
the new MTU only the first time.

Reworking the handling of MTU in NMDevice, and store the source of the
configured MTU. When commit_mtu() is called again, we ask the subclass
a MTU to configure and apply it only if the source has higher
priority, or when the parent MTU changed.

3 files changed, 48 insertions, 25 deletions

diff --git a/src/devices/nm-device-vlan.c b/src/devices/nm-device-vlan.c
index d57c266064..b0e746a822 100644
--- a/src/devices/nm-device-vlan.c
+++ b/src/devices/nm-device-vlan.c
@@ -545,8 +545,10 @@ get_configured_mtu (NMDevice *self, NMDeviceMtuSource *out_source)
 
 	/* Inherit the MTU from parent device, if any */
 	ifindex = nm_device_parent_get_ifindex (self);
-	if (ifindex > 0)
+	if (ifindex > 0) {
 		mtu = nm_platform_link_get_mtu (nm_device_get_platform (NM_DEVICE (self)), ifindex);
+		*out_source = NM_DEVICE_MTU_SOURCE_PARENT;
+	}
 
 	return mtu;
 }
diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c
index 5de879cb3f..8fcee6baab 100644
--- a/src/devices/nm-device.c
+++ b/src/devices/nm-device.c
@@ -349,8 +349,9 @@ typedef struct _NMDevicePrivate {
 	gulong          config_changed_id;
 	guint32         mtu;
 	guint32         ip6_mtu;
-	guint32 mtu_initial;
-	guint32 ip6_mtu_initial;
+	guint32         mtu_initial;
+	guint32         ip6_mtu_initial;
+	NMDeviceMtuSource mtu_source;
 
 	guint32         v4_route_table;
 	guint32         v6_route_table;
@@ -366,8 +367,6 @@ typedef struct _NMDevicePrivate {
 	bool            carrier:1;
 	bool            ignore_carrier:1;
 
-	bool mtu_initialized:1;
-
 	bool            up:1;   /* IFF_UP */
 
 	bool            v4_commit_first_time:1;
@@ -735,6 +734,14 @@ NM_UTILS_LOOKUP_STR_DEFINE (nm_device_state_reason_to_str, NMDeviceStateReason,
 #define reason_to_string(reason) \
 	NM_UTILS_LOOKUP_STR (nm_device_state_reason_to_str, reason)
 
+NM_UTILS_LOOKUP_STR_DEFINE_STATIC (mtu_source_to_str, NMDeviceMtuSource,
+	NM_UTILS_LOOKUP_DEFAULT_NM_ASSERT ("unknown"),
+	NM_UTILS_LOOKUP_STR_ITEM (NM_DEVICE_MTU_SOURCE_NONE,       "none"),
+	NM_UTILS_LOOKUP_STR_ITEM (NM_DEVICE_MTU_SOURCE_PARENT,     "parent"),
+	NM_UTILS_LOOKUP_STR_ITEM (NM_DEVICE_MTU_SOURCE_IP_CONFIG,  "ip-config"),
+	NM_UTILS_LOOKUP_STR_ITEM (NM_DEVICE_MTU_SOURCE_CONNECTION, "connection"),
+);
+
 /*****************************************************************************/
 
 NMSettings *
@@ -4001,7 +4008,7 @@ realize_start_setup (NMDevice *self,
 	/* Balanced by a thaw in nm_device_realize_finish() */
 	g_object_freeze_notify (G_OBJECT (self));
 
-	priv->mtu_initialized = FALSE;
+	priv->mtu_source = NM_DEVICE_MTU_SOURCE_NONE;
 	priv->mtu_initial = 0;
 	priv->ip6_mtu_initial = 0;
 	priv->ip6_mtu = 0;
@@ -8456,6 +8463,7 @@ static void
 _commit_mtu (NMDevice *self, const NMIP4Config *config)
 {
 	NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self);
+	NMDeviceMtuSource source = NM_DEVICE_MTU_SOURCE_NONE;
 	guint32 ip6_mtu, ip6_mtu_orig;
 	guint32 mtu_desired, mtu_desired_orig;
 	guint32 mtu_plat;
@@ -8465,6 +8473,7 @@ _commit_mtu (NMDevice *self, const NMIP4Config *config)
 	} ip6_mtu_sysctl = { 0, };
 	int ifindex;
 	char sbuf[64], sbuf1[64], sbuf2[64];
+	gboolean success = TRUE;
 
 	ifindex = nm_device_get_ip_ifindex (self);
 	if (ifindex <= 0)
@@ -8478,7 +8487,6 @@ _commit_mtu (NMDevice *self, const NMIP4Config *config)
 	}
 
 	{
-		NMDeviceMtuSource mtu_source = NM_DEVICE_MTU_SOURCE_NONE;
 		guint32 mtu = 0;
 
 		/* preferably, get the MTU from explict user-configuration.
@@ -8486,23 +8494,30 @@ _commit_mtu (NMDevice *self, const NMIP4Config *config)
 		 * MTUs from DHCP/PPP) or maybe fallback to a device-specific MTU. */
 
 		if (NM_DEVICE_GET_CLASS (self)->get_configured_mtu)
-			mtu = NM_DEVICE_GET_CLASS (self)->get_configured_mtu (self, &mtu_source);
+			mtu = NM_DEVICE_GET_CLASS (self)->get_configured_mtu (self, &source);
+
+		if (   config
+		    && source < NM_DEVICE_MTU_SOURCE_IP_CONFIG
+		    && nm_ip4_config_get_mtu (config)) {
+			mtu = nm_ip4_config_get_mtu (config);
+			source = NM_DEVICE_MTU_SOURCE_IP_CONFIG;
+		}
+
+		if (mtu != 0) {
+			_LOGT (LOGD_DEVICE,
+			       "mtu: value %u from source '%s' (%u), current source '%s' (%u)",
+			       (guint) mtu,
+			       mtu_source_to_str (source), (guint) source,
+			       mtu_source_to_str (priv->mtu_source), (guint) priv->mtu_source);
+		}
 
-		if (mtu_source == NM_DEVICE_MTU_SOURCE_CONNECTION)
+		if (   mtu != 0
+		    && (   source > priv->mtu_source
+		        || (priv->mtu_source == NM_DEVICE_MTU_SOURCE_PARENT && source == priv->mtu_source)))
 			mtu_desired = mtu;
 		else {
-			if (config)
-				mtu_desired = nm_ip4_config_get_mtu (config);
-			else
-				mtu_desired = 0;
-			if (!mtu_desired && !priv->mtu_initialized) {
-				/* there is no MTU specified, and this is the first commit of the MTU.
-				 * Reset a per-device MTU default, as returned from get_configured_mtu().
-				 *
-				 * The device might choose not to return a default MTU via get_configured_mtu()
-				 * to suppress this behavior. */
-				mtu_desired = mtu;
-			}
+			mtu_desired = 0;
+			source = NM_DEVICE_MTU_SOURCE_NONE;
 		}
 	}
 
@@ -8524,7 +8539,7 @@ _commit_mtu (NMDevice *self, const NMIP4Config *config)
 	}
 
 	ip6_mtu = priv->ip6_mtu;
-	if (!ip6_mtu && !priv->mtu_initialized) {
+	if (!ip6_mtu && priv->mtu_source == NM_DEVICE_MTU_SOURCE_NONE) {
 		/* initially, if the IPv6 MTU is not specified, grow it as large as the
 		 * link MTU @mtu_desired. Only exception is, if @mtu_desired is so small
 		 * to disable IPv6. */
@@ -8532,8 +8547,6 @@ _commit_mtu (NMDevice *self, const NMIP4Config *config)
 			ip6_mtu = mtu_desired;
 	}
 
-	priv->mtu_initialized = TRUE;
-
 	if (!ip6_mtu && !mtu_desired)
 		return;
 
@@ -8585,6 +8598,7 @@ _commit_mtu (NMDevice *self, const NMIP4Config *config)
 		if (mtu_desired && mtu_desired != mtu_plat) {
 			if (nm_platform_link_set_mtu (nm_device_get_platform (self), ifindex, mtu_desired) == NM_PLATFORM_ERROR_CANT_SET_MTU) {
 				anticipated_failure = TRUE;
+				success = FALSE;
 				_LOGW (LOGD_DEVICE, "mtu: failure to set MTU. %s",
 				       NM_IS_DEVICE_VLAN (self)
 				         ? "Is the parent's MTU size large enough?"
@@ -8606,10 +8620,15 @@ _commit_mtu (NMDevice *self, const NMIP4Config *config)
 				        anticipated_failure && errsv == EINVAL
 				           ? ": Is the underlying MTU value successfully set?"
 				           : "");
+				success = FALSE;
 			}
 			priv->carrier_wait_until_ms = nm_utils_get_monotonic_timestamp_ms () + CARRIER_WAIT_TIME_AFTER_MTU_MS;
 		}
 	}
+
+	if (success && source != NM_DEVICE_MTU_SOURCE_NONE)
+		priv->mtu_source = source;
+
 #undef _IP6_MTU_SYS
 }
 
@@ -13648,7 +13667,7 @@ nm_device_cleanup (NMDevice *self, NMDeviceStateReason reason, CleanupType clean
 		NM_DEVICE_GET_CLASS (self)->deactivate_reset_hw_addr (self);
 	}
 
-	priv->mtu_initialized = FALSE;
+	priv->mtu_source = NM_DEVICE_MTU_SOURCE_NONE;
 	if (priv->mtu_initial || priv->ip6_mtu_initial) {
 		ifindex = nm_device_get_ip_ifindex (self);
 
diff --git a/src/devices/nm-device.h b/src/devices/nm-device.h
index 9332476a61..010a0521c8 100644
--- a/src/devices/nm-device.h
+++ b/src/devices/nm-device.h
@@ -46,6 +46,8 @@ typedef enum {
 
 typedef enum {
 	NM_DEVICE_MTU_SOURCE_NONE,
+	NM_DEVICE_MTU_SOURCE_PARENT,
+	NM_DEVICE_MTU_SOURCE_IP_CONFIG,
 	NM_DEVICE_MTU_SOURCE_CONNECTION,
 } NMDeviceMtuSource;