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 <rcunningham@nsmsurveillance.com>
Cc: Bjorn Mork <bjorn@mork.no>
Cc: Dan Williams <dcbw@redhat.com>
---
 drivers/net/usb/smsc75xx.c |   10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

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 <rcunningham@nsmsurveillance.com>

Applied.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

> Signed-off-by: Steve Glendinning <steve.glendinning@shawell.net>
> Reported-by: Robert Cunningham <rcunningham@nsmsurveillance.com>
> Cc: Bjorn Mork <bjorn@mork.no>
> Cc: Dan Williams <dcbw@redhat.com>
> ---
>  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) {


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Hi Dan,

On 10 December 2012 23:00, Dan Williams <dcbw@redhat.com> wrote:
> 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?

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.

On Tue, 2012-12-11 at 14:35 +0000, Steve Glendinning wrote:
> Hi Dan,
> 
> On 10 December 2012 23:00, Dan Williams <dcbw@redhat.com> wrote:
> > 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?
> 
> 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

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html