device: delay capturing permanent MAC address until UDEV is settled

The permanent MAC address of an NMDevice shall not change as
long as the device is realized. That is, we read it only once
and don't change it afterwards.

There are two issues that this commit tries to mitigate:

(1) users are advised to use UDEV to rename interfaces. As we lookup
  the permenent MAC address using ethtool (which uses the interface
  name), there is a race where we could read the permanent MAC
  address using the wrong interface name. We should wait until
  UDEV finished initializing the device and until the interface
  name is stable (see rh#1388286).
  This commit still cannot avoid the race of ethtool entirely. It only
  tries to avoid ethtool until UDEV has done its work. That is, until we
  expect the interface name no longer to change.

(2) some device types, don't have a permanent MAC address so we fall
  back to use the currently set address (fake). Again, users are advised
  to use UDEV to configure the MAC addresses on such software devices.
  Thus, we should not get the fake MAC address until UDEV initialized
  the device.

This patch actually doesn't solve the problem at all yet.
The reason is that a regular caller of nm_device_get_permanent_hw_address() can
not afford to wait until UDEV settled. Thus, any user who requests the
permanent MAC address before the link is initialized, runs into the
problems above.

In a next step, we shall revisit such calls to nm_device_get_permanent_hw_address()
and delay them until the link is initialized.

(cherry picked from commit 7b7c653c4f812f0ede676e8e0a08aa750e9e30b5)

Conflicts:
        src/devices/nm-device.c
        src/nm-manager.c

3 files changed, 69 insertions, 38 deletions

diff --git a/src/devices/nm-device-ethernet.c b/src/devices/nm-device-ethernet.c
index 5e980ed9c5..51515c090a 100644
--- a/src/devices/nm-device-ethernet.c
+++ b/src/devices/nm-device-ethernet.c
@@ -1374,7 +1374,7 @@ complete_connection (NMDevice *device,
 		nm_connection_add_setting (connection, NM_SETTING (s_wired));
 	}
 
-	perm_hw_addr = nm_device_get_permanent_hw_address_full (device, &perm_hw_addr_is_fake);
+	perm_hw_addr = nm_device_get_permanent_hw_address_full (device, TRUE, &perm_hw_addr_is_fake);
 	if (perm_hw_addr && !perm_hw_addr_is_fake) {
 		setting_mac = nm_setting_wired_get_mac_address (s_wired);
 		if (setting_mac) {
@@ -1491,7 +1491,7 @@ update_connection (NMDevice *device, NMConnection *connection)
 	/* If the device reports a permanent address, use that for the MAC address
 	 * and the current MAC, if different, is the cloned MAC.
 	 */
-	perm_hw_addr = nm_device_get_permanent_hw_address_full (device, &perm_hw_addr_is_fake);
+	perm_hw_addr = nm_device_get_permanent_hw_address_full (device, TRUE, &perm_hw_addr_is_fake);
 	if (perm_hw_addr && !perm_hw_addr_is_fake) {
 		g_object_set (s_wired, NM_SETTING_WIRED_MAC_ADDRESS, perm_hw_addr, NULL);
 
diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c
index ca84cdfa47..0aee6fbf65 100644
--- a/src/devices/nm-device.c
+++ b/src/devices/nm-device.c
@@ -1821,7 +1821,7 @@ device_link_changed (NMDevice *self)
 	had_hw_addr = (priv->hw_addr != NULL);
 	nm_device_update_hw_address (self);
 	got_hw_addr = (!had_hw_addr && priv->hw_addr);
-	nm_device_update_permanent_hw_address (self);
+	nm_device_update_permanent_hw_address (self, FALSE);
 
 	if (info.name[0] && strcmp (priv->iface, info.name) != 0) {
 		_LOGI (LOGD_DEVICE, "interface index %d renamed iface from '%s' to '%s'",
@@ -2287,7 +2287,7 @@ realize_start_setup (NMDevice *self, const NMPlatformLink *plink)
 
 	nm_device_update_hw_address (self);
 	nm_device_update_initial_hw_address (self);
-	nm_device_update_permanent_hw_address (self);
+	nm_device_update_permanent_hw_address (self, FALSE);
 
 	/* Note: initial hardware address must be read before calling get_ignore_carrier() */
 	config = nm_config_get ();
@@ -11605,12 +11605,14 @@ nm_device_update_initial_hw_address (NMDevice *self)
 }
 
 void
-nm_device_update_permanent_hw_address (NMDevice *self)
+nm_device_update_permanent_hw_address (NMDevice *self, gboolean force_freeze)
 {
 	NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self);
 	guint8 buf[NM_UTILS_HWADDR_LEN_MAX];
 	size_t len = 0;
 	gboolean success_read;
+	int ifindex;
+	const NMPlatformLink *pllink;
 
 	if (priv->hw_addr_perm) {
 		/* the permanent hardware address is only read once and not
@@ -11621,39 +11623,60 @@ nm_device_update_permanent_hw_address (NMDevice *self)
 		return;
 	}
 
-	if (priv->ifindex <= 0)
+	ifindex = priv->ifindex;
+	if (ifindex <= 0)
 		return;
 
-	if (!priv->hw_addr_len) {
-		nm_device_update_hw_address (self);
-		if (!priv->hw_addr_len)
+	/* the user is advised to configure stable MAC addresses for software devices via
+	 * UDEV. Thus, check whether the link is fully initialized. */
+	pllink = nm_platform_link_get (NM_PLATFORM_GET, ifindex);
+	if (   !pllink
+	    || !pllink->initialized) {
+		if (!force_freeze) {
+			/* we can afford to wait. Back off and leave the permanent MAC address
+			 * undecided for now. */
 			return;
-	}
-
-	success_read = nm_platform_link_get_permanent_address (NM_PLATFORM_GET, priv->ifindex, buf, &len);
-	if (!success_read || len != priv->hw_addr_len) {
-		priv->hw_addr_perm_fake = TRUE;
+		}
+		/* try to refresh the link just to give UDEV a bit more time... */
+		nm_platform_link_refresh (NM_PLATFORM_GET, ifindex);
+		/* maybe the MAC address changed... */
+		nm_device_update_hw_address (self);
+	} else if (!priv->hw_addr_len)
+		nm_device_update_hw_address (self);
 
-		/* we failed to read a permanent MAC address, thus we use a fake address,
-		 * that is the current MAC address of the device.
+	if (!priv->hw_addr_len) {
+		/* we need the current MAC address because we require the permanent MAC address
+		 * to have the same length as the current address.
 		 *
-		 * In some cases it might be necessary to know whether this is a "real" or
-		 * a temporary address (fake). */
-		_LOGD (LOGD_PLATFORM | LOGD_ETHER, "hw-addr: %s (use current: %s)",
-		       success_read
-		           ? "read HW addr length of permanent MAC address differs"
-		           : "unable to read permanent MAC address",
-		       priv->hw_addr);
-		priv->hw_addr_perm = g_strdup (priv->hw_addr);
-		goto out;
-	}
-
-	priv->hw_addr_perm_fake = FALSE;
-	priv->hw_addr_perm = nm_utils_hwaddr_ntoa (buf, len);
-	_LOGD (LOGD_DEVICE, "hw-addr: read permanent MAC address '%s'",
-	       priv->hw_addr_perm);
+		 * Abort if there is no current MAC address. */
+		return;
+	}
 
-out:
+	success_read = nm_platform_link_get_permanent_address (NM_PLATFORM_GET, ifindex, buf, &len);
+	if (success_read && priv->hw_addr_len == len) {
+		priv->hw_addr_perm_fake = FALSE;
+		priv->hw_addr_perm = nm_utils_hwaddr_ntoa (buf, len);
+		_LOGD (LOGD_DEVICE, "hw-addr: read permanent MAC address '%s'",
+		       priv->hw_addr_perm);
+		goto notify_and_out;
+	}
+
+	/* we failed to read a permanent MAC address, thus we use a fake address,
+	 * that is the current MAC address of the device.
+	 *
+	 * Note that the permanet MAC address of a NMDevice instance does not change
+	 * after being set once. Thus, we use now a fake address and stick to that
+	 * (until we unrealize the device). */
+	priv->hw_addr_perm_fake = TRUE;
+
+	_LOGD (LOGD_PLATFORM | LOGD_ETHER, "hw-addr: %s (use current: %s)",
+	       success_read
+	           ? "read HW addr length of permanent MAC address differs"
+	           : "unable to read permanent MAC address",
+	       priv->hw_addr);
+	priv->hw_addr_perm = g_strdup (priv->hw_addr);
+
+notify_and_out:
 	_notify (self, PROP_PERM_HW_ADDRESS);
 }
 
@@ -11988,13 +12011,22 @@ nm_device_hw_addr_reset (NMDevice *self, const char *detail)
 }
 
 const char *
-nm_device_get_permanent_hw_address_full (NMDevice *self, gboolean *out_is_fake)
+nm_device_get_permanent_hw_address_full (NMDevice *self, gboolean force_freeze, gboolean *out_is_fake)
 {
 	NMDevicePrivate *priv;
 
 	g_return_val_if_fail (NM_IS_DEVICE (self), NULL);
 
 	priv = NM_DEVICE_GET_PRIVATE (self);
+
+	if (   !priv->hw_addr_perm
+	    && force_freeze) {
+		/* somebody requests a permanent MAC address, but we don't have it set
+		 * yet. We cannot delay it any longer and try to get it without waiting
+		 * for UDEV. */
+		nm_device_update_permanent_hw_address (self, TRUE);
+	}
+
 	NM_SET_OUT (out_is_fake, priv->hw_addr_perm && priv->hw_addr_perm_fake);
 	return priv->hw_addr_perm;
 }
@@ -12002,9 +12034,7 @@ nm_device_get_permanent_hw_address_full (NMDevice *self, gboolean *out_is_fake)
 const char *
 nm_device_get_permanent_hw_address (NMDevice *self)
 {
-	g_return_val_if_fail (NM_IS_DEVICE (self), NULL);
-
-	return NM_DEVICE_GET_PRIVATE (self)->hw_addr_perm;
+	return nm_device_get_permanent_hw_address_full (self, TRUE, NULL);
 }
 
 const char *
@@ -12539,7 +12569,7 @@ get_property (GObject *object, guint prop_id,
 		const char *perm_hw_addr;
 		gboolean perm_hw_addr_is_fake;
 
-		perm_hw_addr = nm_device_get_permanent_hw_address_full (self, &perm_hw_addr_is_fake);
+		perm_hw_addr = nm_device_get_permanent_hw_address_full (self, FALSE, &perm_hw_addr_is_fake);
 		/* this property is exposed on D-Bus for NMDeviceEthernet and NMDeviceWifi. */
 		g_value_set_string (value, perm_hw_addr && !perm_hw_addr_is_fake ? perm_hw_addr : NULL);
 		break;
diff --git a/src/devices/nm-device.h b/src/devices/nm-device.h
index 476a42bc0f..90443749ec 100644
--- a/src/devices/nm-device.h
+++ b/src/devices/nm-device.h
@@ -366,6 +366,7 @@ guint32         nm_device_get_ip6_route_metric  (NMDevice *dev);
 const char *    nm_device_get_hw_address        (NMDevice *dev);
 const char *    nm_device_get_permanent_hw_address (NMDevice *self);
 const char *    nm_device_get_permanent_hw_address_full (NMDevice *self,
+                                                         gboolean force_freeze,
                                                          gboolean *out_is_fake);
 const char *    nm_device_get_initial_hw_address (NMDevice *dev);
 
@@ -591,7 +592,7 @@ void nm_device_reactivate_ip6_config (NMDevice *device,
 
 gboolean nm_device_update_hw_address (NMDevice *self);
 void nm_device_update_initial_hw_address (NMDevice *self);
-void nm_device_update_permanent_hw_address (NMDevice *self);
+void nm_device_update_permanent_hw_address (NMDevice *self, gboolean force_freeze);
 void nm_device_update_dynamic_ip_setup (NMDevice *self);
 
 #endif /* __NETWORKMANAGER_DEVICE_H__ */