[PATCH] smsc75xx: only set mac address once on bind

This patch changes when we decide what the device's MAC address
is from per ifconfig up to once when the device is connected.

Without this patch, a manually forced device MAC is overwritten
on ifconfig down/up.  Also devices that have no EEPROM are
assigned a new random address on ifconfig down/up instead of
persisting the same one.

Signed-off-by: Steve Glendinning <steve.glendinning@shawell.net>
Reported-by: Robert Cunningham <redacted>
Cc: Bjorn Mork <bjorn@mork.no>
Cc: Dan Williams <redacted>
---
 drivers/net/usb/smsc75xx.c |   10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/net/usb/smsc75xx.c b/drivers/net/usb/smsc75xx.c
index 1cbd936..251a335 100644
--- a/drivers/net/usb/smsc75xx.c
+++ b/drivers/net/usb/smsc75xx.c

@@ -1054,8 +1054,6 @@ static int smsc75xx_reset(struct usbnet *dev)
 
 	netif_dbg(dev, ifup, dev->net, "PHY reset complete\n");
 
-	smsc75xx_init_mac_address(dev);
-
 	ret = smsc75xx_set_mac_address(dev);
 	if (ret < 0) {
 		netdev_warn(dev->net, "Failed to set mac address\n");

@@ -1422,6 +1420,14 @@ static int smsc75xx_bind(struct usbnet *dev, struct usb_interface *intf)
 	dev->net->hw_features = NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
 		NETIF_F_SG | NETIF_F_TSO | NETIF_F_TSO6 | NETIF_F_RXCSUM;
 
+	ret = smsc75xx_wait_ready(dev, 0);
+	if (ret < 0) {
+		netdev_warn(dev->net, "device not ready in smsc75xx_bind\n");
+		return ret;
+	}
+
+	smsc75xx_init_mac_address(dev);
+
 	/* Init all registers */
 	ret = smsc75xx_reset(dev);
 	if (ret < 0) {

-- 
1.7.10.4

From: Steve Glendinning <steve.glendinning@shawell.net>
Date: Mon, 10 Dec 2012 11:01:19 +0000

This patch changes when we decide what the device's MAC address
is from per ifconfig up to once when the device is connected.

Without this patch, a manually forced device MAC is overwritten
on ifconfig down/up.  Also devices that have no EEPROM are
assigned a new random address on ifconfig down/up instead of
persisting the same one.

Signed-off-by: Steve Glendinning <steve.glendinning@shawell.net>
Reported-by: Robert Cunningham <redacted>

Applied.

On Mon, 2012-12-10 at 11:01 +0000, Steve Glendinning wrote:

This patch changes when we decide what the device's MAC address
is from per ifconfig up to once when the device is connected.

Without this patch, a manually forced device MAC is overwritten
on ifconfig down/up.  Also devices that have no EEPROM are
assigned a new random address on ifconfig down/up instead of
persisting the same one.

Does this mean that on devices without EEPROM, ifconfig XXX
down/ifconfig XXX up will generate a *new* random address?  That seems a
bit odd; why wouldn't the first random address generated for the device
persist until either (a) changed by ifconfig or (b) device was
disconnected?

Dan

Hi Dan,

On 10 December 2012 23:00, Dan Williams [off-list ref] wrote:

On Mon, 2012-12-10 at 11:01 +0000, Steve Glendinning wrote:

quoted

This patch changes when we decide what the device's MAC address
is from per ifconfig up to once when the device is connected.

Without this patch, a manually forced device MAC is overwritten
on ifconfig down/up.  Also devices that have no EEPROM are
assigned a new random address on ifconfig down/up instead of
persisting the same one.

Does this mean that on devices without EEPROM, ifconfig XXX
down/ifconfig XXX up will generate a *new* random address?  That seems a
bit odd; why wouldn't the first random address generated for the device
persist until either (a) changed by ifconfig or (b) device was
disconnected?

Sorry if my commit message wasn't clear enough.  That is indeed a bit
odd, and it describes the (buggy) behaviour BEFORE this patch was
applied.

With this patch applied, devices without EEPROM (and without a
manually specified MAC address) will get a randomly generated address
assigned once at bind time, so this MAC will persist for the lifetime
the USB device is connected.  We also now won't trash a manually
specified MAC, for cases where userland sets the MAC before bringing
up the interface.

-- 
Steve Glendinning

On Tue, 2012-12-11 at 14:35 +0000, Steve Glendinning wrote:

Hi Dan,

On 10 December 2012 23:00, Dan Williams [off-list ref] wrote:

quoted

On Mon, 2012-12-10 at 11:01 +0000, Steve Glendinning wrote:

quoted

This patch changes when we decide what the device's MAC address
is from per ifconfig up to once when the device is connected.

Without this patch, a manually forced device MAC is overwritten
on ifconfig down/up.  Also devices that have no EEPROM are
assigned a new random address on ifconfig down/up instead of
persisting the same one.

Does this mean that on devices without EEPROM, ifconfig XXX
down/ifconfig XXX up will generate a *new* random address?  That seems a
bit odd; why wouldn't the first random address generated for the device
persist until either (a) changed by ifconfig or (b) device was
disconnected?

Sorry if my commit message wasn't clear enough.  That is indeed a bit
odd, and it describes the (buggy) behaviour BEFORE this patch was
applied.

With this patch applied, devices without EEPROM (and without a
manually specified MAC address) will get a randomly generated address
assigned once at bind time, so this MAC will persist for the lifetime
the USB device is connected.  We also now won't trash a manually
specified MAC, for cases where userland sets the MAC before bringing
up the interface.

Excellent, thanks for the clarification.

Dan