[PATCH] Re: NetworkManager reset MAC address

[Jirka Klimes <jklimes redhat com>]

On Tuesday 21 of September 2010 00:47:42 Dan Williams wrote:
> On Wed, 2010-09-01 at 16:31 +0200, Jirka Klimes wrote:
> > On Tuesday 31 of August 2010 12:26:05 mmsim6 gmail com wrote:
> > > I wrote a udev rule to change the MAC address of my wireless card and
> > > it worked correctly until I upgraded to
> > > NetworkManager-0.8.1-4.git20100817.fc13.x86_64
> > > today. Now I find the MAC is reset back to the original address by
> > > NetworkManager.
> > > How to stop this new feature?
> > 
> > The new NetworkManager has implemented MAC spoofing feature just for this
> > purpose.
> > In connection editor,  on 'Wireless' tab there is a new edit box 'Cloned
> > MAC address'. If you put your desired MAC here, it will be set on an
> > interface when the connection is activated. And you don't need to change
> > your MAC in udev or any other way.
> > See https://bugzilla.redhat.com/show_bug.cgi?id=447827
> 
> Yeah, though I think if something has set the MAC before NM starts, we
> probably want to make NM read that MAC address in as a spoofed MAC when
> NM starts.  I'm not sure we do that yet?  Basically respect the
> configuration that exists for both permanent MAC and spoofed MAC when NM
> starts up if we can.
> 
> Dan
> 

The problem is that we currently reset the MAC to permanent MAC address and 
thus ignoring the MAC that has been changed before NM starts.
The attached patch reads the MAC set on interface when NM starts and when uses 
that when resetting MAC. Thus, NM won't interfere with other settings and 
won't annoyingly keep putting permanent MAC to the interface.

Jirka

diff --git a/src/nm-device-ethernet.c b/src/nm-device-ethernet.c
index 393b921..08bbc50 100644
--- a/src/nm-device-ethernet.c
+++ b/src/nm-device-ethernet.c
@@ -107,8 +107,9 @@ typedef struct Supplicant {
 typedef struct {
 	gboolean            disposed;
 
-	guint8              hw_addr[ETH_ALEN];      /* Currently set MAC address */
-	guint8              perm_hw_addr[ETH_ALEN]; /* Currently set MAC address */
+	guint8              hw_addr[ETH_ALEN];         /* Currently set MAC address */
+	guint8              perm_hw_addr[ETH_ALEN];    /* Permanent MAC address */
+	guint8              initial_hw_addr[ETH_ALEN]; /* Initial MAC address (as seen when NM starts) */
 	gboolean            carrier;
 
 	NMNetlinkMonitor *  monitor;
@@ -739,6 +740,33 @@ real_update_permanent_hw_address (NMDevice *dev)
 	close (fd);
 }
 
+static void
+real_update_initial_hw_address (NMDevice *dev)
+{
+	NMDeviceEthernet *self = NM_DEVICE_ETHERNET (dev);
+	NMDeviceEthernetPrivate *priv = NM_DEVICE_ETHERNET_GET_PRIVATE (self);
+	char *mac_str = NULL;
+	guint8 *addr = priv->initial_hw_addr;
+	guint8 zero[ETH_ALEN] = {0,0,0,0,0,0};
+
+	/* This sets initial MAC address from current MAC address. It should only
+	 * be called from NMDevice constructor() to really get the initial address.
+	 */
+	if (!memcmp (&priv->hw_addr, &zero, ETH_ALEN))
+		real_update_hw_address (dev);
+
+	if (memcmp (&priv->initial_hw_addr, &priv->hw_addr, ETH_ALEN))
+		memcpy (&priv->initial_hw_addr, &priv->hw_addr, ETH_ALEN);
+
+	mac_str = g_strdup_printf ("%02X:%02X:%02X:%02X:%02X:%02X",
+	                           addr[0], addr[1], addr[2], addr[3], addr[4], addr[5]);
+
+	nm_log_dbg (LOGD_DEVICE | LOGD_ETHER, "(%s): read initial MAC address %s",
+	            nm_device_get_iface (dev), mac_str);
+
+	g_free (mac_str);
+}
+
 static guint32
 real_get_generic_capabilities (NMDevice *dev)
 {
@@ -1735,8 +1763,8 @@ real_deactivate_quickly (NMDevice *device)
 
 	supplicant_interface_release (self);
 
-	/* Reset MAC address back to permanent address */
-	_set_hw_addr (self, priv->perm_hw_addr, "reset");
+	/* Reset MAC address back to initial address */
+	_set_hw_addr (self, priv->initial_hw_addr, "reset");
 }
 
 static gboolean
@@ -2110,6 +2138,7 @@ nm_device_ethernet_class_init (NMDeviceEthernetClass *klass)
 	parent_class->can_interrupt_activation = real_can_interrupt_activation;
 	parent_class->update_hw_address = real_update_hw_address;
 	parent_class->update_permanent_hw_address = real_update_permanent_hw_address;
+	parent_class->update_initial_hw_address = real_update_initial_hw_address;
 	parent_class->get_best_auto_connection = real_get_best_auto_connection;
 	parent_class->is_available = real_is_available;
 	parent_class->connection_secrets_updated = real_connection_secrets_updated;
diff --git a/src/nm-device-wifi.c b/src/nm-device-wifi.c
index 3973fe7..b09d112 100644
--- a/src/nm-device-wifi.c
+++ b/src/nm-device-wifi.c
@@ -146,8 +146,9 @@ typedef struct Supplicant {
 struct _NMDeviceWifiPrivate {
 	gboolean          disposed;
 
-	guint8            hw_addr[ETH_ALEN];      /* Currently set MAC address */
-	guint8            perm_hw_addr[ETH_ALEN]; /* Permanent MAC address */
+	guint8            hw_addr[ETH_ALEN];         /* Currently set MAC address */
+	guint8            perm_hw_addr[ETH_ALEN];    /* Permanent MAC address */
+	guint8            initial_hw_addr[ETH_ALEN]; /* Initial MAC address (as seen when NM starts) */
 
 	/* Legacy rfkill for ipw2x00; will be fixed with 2.6.33 kernel */
 	char *            ipw_rfkill_path;
@@ -1250,7 +1251,8 @@ real_deactivate_quickly (NMDevice *dev)
 		g_object_unref (orig_ap);
 	}
 
-	_set_hw_addr (self, priv->perm_hw_addr, "reset");
+	/* Reset MAC address back to initial address */
+	_set_hw_addr (self, priv->initial_hw_addr, "reset");
 }
 
 static void
@@ -3106,6 +3108,32 @@ real_update_permanent_hw_address (NMDevice *dev)
 	close (fd);
 }
 
+static void
+real_update_initial_hw_address (NMDevice *dev)
+{
+	NMDeviceWifi *self = NM_DEVICE_WIFI (dev);
+	NMDeviceWifiPrivate *priv = NM_DEVICE_WIFI_GET_PRIVATE (self);
+	char *mac_str = NULL;
+	guint8 *addr = priv->initial_hw_addr;
+	guint8 zero[ETH_ALEN] = {0,0,0,0,0,0};
+
+	/* This sets initial MAC address from current MAC address. It should only
+	 * be called from NMDevice constructor() to really get the initial address.
+	 */
+	if (!memcmp (&priv->hw_addr, &zero, ETH_ALEN))
+		real_update_hw_address (dev);
+
+	if (memcmp (&priv->initial_hw_addr, &priv->hw_addr, ETH_ALEN))
+		memcpy (&priv->initial_hw_addr, &priv->hw_addr, ETH_ALEN);
+
+	mac_str = g_strdup_printf ("%02X:%02X:%02X:%02X:%02X:%02X",
+	                           addr[0], addr[1], addr[2], addr[3], addr[4], addr[5]);
+
+	nm_log_dbg (LOGD_DEVICE | LOGD_ETHER, "(%s): read initial MAC address %s",
+	            nm_device_get_iface (dev), mac_str);
+
+	g_free (mac_str);
+}
 
 static NMActStageReturn
 real_act_stage1_prepare (NMDevice *dev, NMDeviceStateReason *reason)
@@ -3935,6 +3963,7 @@ nm_device_wifi_class_init (NMDeviceWifiClass *klass)
 	parent_class->take_down = real_take_down;
 	parent_class->update_hw_address = real_update_hw_address;
 	parent_class->update_permanent_hw_address = real_update_permanent_hw_address;
+	parent_class->update_initial_hw_address = real_update_initial_hw_address;
 	parent_class->get_best_auto_connection = real_get_best_auto_connection;
 	parent_class->is_available = real_is_available;
 	parent_class->connection_secrets_updated = real_connection_secrets_updated;
diff --git a/src/nm-device.c b/src/nm-device.c
index ad1976b..31e4fb1 100644
--- a/src/nm-device.c
+++ b/src/nm-device.c
@@ -288,6 +288,9 @@ constructor (GType type,
 	if (NM_DEVICE_GET_CLASS (dev)->update_permanent_hw_address)
 		NM_DEVICE_GET_CLASS (dev)->update_permanent_hw_address (dev);
 
+	if (NM_DEVICE_GET_CLASS (dev)->update_initial_hw_address)
+		NM_DEVICE_GET_CLASS (dev)->update_initial_hw_address (dev);
+
 	priv->dhcp_manager = nm_dhcp_manager_get ();
 
 	update_accept_ra_save (dev);
diff --git a/src/nm-device.h b/src/nm-device.h
index ca067f4..db2b1b7 100644
--- a/src/nm-device.h
+++ b/src/nm-device.h
@@ -73,6 +73,7 @@ typedef struct {
 
 	void        (* update_hw_address) (NMDevice *self);
 	void        (* update_permanent_hw_address) (NMDevice *self);
+	void        (* update_initial_hw_address) (NMDevice *self);
 
 	guint32		(* get_type_capabilities)	(NMDevice *self);
 	guint32		(* get_generic_capabilities)	(NMDevice *self);