| Message ID | 20101111142435.GA4950@evatp |
|---|---|
| State | Accepted |
| Delegated to: | Tim Gardner |
| Headers | show |
On Thu, 2010-11-11 at 12:24 -0200, Ricardo Salveti de Araujo wrote: > SRU Justification: > > Impact: The smsc95xx driver generates a new random MAC address on every > ifdown/up instead of only one during driver's init, and doesn't let the user to > set up the mac address using known methods like ifconfig usb0 hw ether <mac>. > > Fix: Just move the init_mac_address function to bind instead of reset. > > Testcase: The interface should keep the same generated mac address even when > giving ifdown/up, and the user should be able to set up the mac address using > 'ifconfig usb0 hw ether <mac>. > > Patch is already upstream as f4e8ab7, and is tested already with a compiled > kernel deb with the fix included. > > BugLink: https://bugs.launchpad.net/bugs/673504 and https://bugs.launchpad.net/bugs/673509 Looks reasonable to me with positive test confirmation in the bug reports. Acked-by: Leann Ogasawara <leann.ogasawara@canonical.com> > From: Bernard Blackham <bernard@largestprime.net> > Date: Tue, 19 Oct 2010 10:16:39 +1100 > Subject: [PATCH] smsc95xx: generate random MAC address once, not every ifup > > The smsc95xx driver currently generates a new random MAC address > every time the interface is brought up. This makes it impossible to > override using the standard `ifconfig hw ether` approach. > > Past patches tried to make the MAC address a module parameter or > base it off the die ID, but it seems to me much simpler (and > hopefully less controversial) to stick with the current random > generation scheme, but allow the user to change the address. > > This patch does exactly that - it moves the random address > generation from smsc95xx_reset() into smsc95xx_bind(), so that it is > done once on module load, not on every ifup. The user can then > override this using the standard mechanisms. > > Applies against 2.6.35 and linux-2.6 head. > > Signed-off-by: Bernard Blackham <b-omap@largestprime.net> > Signed-off-by: Ricardo Salveti de Araujo <ricardo.salveti@canonical.com> > --- > drivers/net/usb/smsc95xx.c | 4 ++-- > 1 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c > index 12a3c88..65cb1ab 100644 > --- a/drivers/net/usb/smsc95xx.c > +++ b/drivers/net/usb/smsc95xx.c > @@ -805,8 +805,6 @@ static int smsc95xx_reset(struct usbnet *dev) > return ret; > } > > - smsc95xx_init_mac_address(dev); > - > ret = smsc95xx_set_mac_address(dev); > if (ret < 0) > return ret; > @@ -1047,6 +1045,8 @@ static int smsc95xx_bind(struct usbnet *dev, struct usb_interface *intf) > pdata->use_tx_csum = DEFAULT_TX_CSUM_ENABLE; > pdata->use_rx_csum = DEFAULT_RX_CSUM_ENABLE; > > + smsc95xx_init_mac_address(dev); > + > /* Init all registers */ > ret = smsc95xx_reset(dev); > > -- > 1.7.1 >
On 11/11/2010 03:24 PM, Ricardo Salveti de Araujo wrote: > SRU Justification: > > Impact: The smsc95xx driver generates a new random MAC address on every > ifdown/up instead of only one during driver's init, and doesn't let the user to > set up the mac address using known methods like ifconfig usb0 hw ether <mac>. > > Fix: Just move the init_mac_address function to bind instead of reset. > > Testcase: The interface should keep the same generated mac address even when > giving ifdown/up, and the user should be able to set up the mac address using > 'ifconfig usb0 hw ether <mac>. > > Patch is already upstream as f4e8ab7, and is tested already with a compiled > kernel deb with the fix included. > > BugLink: https://bugs.launchpad.net/bugs/673504 and https://bugs.launchpad.net/bugs/673509 > Just minor formatting issues. The BugLink should be two lines and go into the patch itself. > From: Bernard Blackham <bernard@largestprime.net> > Date: Tue, 19 Oct 2010 10:16:39 +1100 > Subject: [PATCH] smsc95xx: generate random MAC address once, not every ifup > BugLink: https://bugs.launchpad.net/bugs/673504 BugLink: https://bugs.launchpad.net/bugs/673509 > The smsc95xx driver currently generates a new random MAC address > every time the interface is brought up. This makes it impossible to > override using the standard `ifconfig hw ether` approach. > > Past patches tried to make the MAC address a module parameter or > base it off the die ID, but it seems to me much simpler (and > hopefully less controversial) to stick with the current random > generation scheme, but allow the user to change the address. > > This patch does exactly that - it moves the random address > generation from smsc95xx_reset() into smsc95xx_bind(), so that it is > done once on module load, not on every ifup. The user can then > override this using the standard mechanisms. > > Applies against 2.6.35 and linux-2.6 head. > > Signed-off-by: Bernard Blackham <b-omap@largestprime.net> > Signed-off-by: Ricardo Salveti de Araujo <ricardo.salveti@canonical.com> Acked-by: Stefan Bader <stefan.bader@canonical.com> > --- > drivers/net/usb/smsc95xx.c | 4 ++-- > 1 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c > index 12a3c88..65cb1ab 100644 > --- a/drivers/net/usb/smsc95xx.c > +++ b/drivers/net/usb/smsc95xx.c > @@ -805,8 +805,6 @@ static int smsc95xx_reset(struct usbnet *dev) > return ret; > } > > - smsc95xx_init_mac_address(dev); > - > ret = smsc95xx_set_mac_address(dev); > if (ret < 0) > return ret; > @@ -1047,6 +1045,8 @@ static int smsc95xx_bind(struct usbnet *dev, struct usb_interface *intf) > pdata->use_tx_csum = DEFAULT_TX_CSUM_ENABLE; > pdata->use_rx_csum = DEFAULT_RX_CSUM_ENABLE; > > + smsc95xx_init_mac_address(dev); > + > /* Init all registers */ > ret = smsc95xx_reset(dev); >
On 11/11/2010 07:24 AM, Ricardo Salveti de Araujo wrote: > SRU Justification: > > Impact: The smsc95xx driver generates a new random MAC address on every > ifdown/up instead of only one during driver's init, and doesn't let the user to > set up the mac address using known methods like ifconfig usb0 hw ether<mac>. > > Fix: Just move the init_mac_address function to bind instead of reset. > > Testcase: The interface should keep the same generated mac address even when > giving ifdown/up, and the user should be able to set up the mac address using > 'ifconfig usb0 hw ether<mac>. > > Patch is already upstream as f4e8ab7, and is tested already with a compiled > kernel deb with the fix included. applied, packaged, and uploaded
On Tue, Nov 16, 2010 at 11:51 PM, Tim Gardner <tim.gardner@canonical.com> wrote: > On 11/11/2010 07:24 AM, Ricardo Salveti de Araujo wrote: >> >> SRU Justification: >> >> Impact: The smsc95xx driver generates a new random MAC address on every >> ifdown/up instead of only one during driver's init, and doesn't let the >> user to >> set up the mac address using known methods like ifconfig usb0 hw >> ether<mac>. >> >> Fix: Just move the init_mac_address function to bind instead of reset. >> >> Testcase: The interface should keep the same generated mac address even >> when >> giving ifdown/up, and the user should be able to set up the mac address >> using >> 'ifconfig usb0 hw ether<mac>. >> >> Patch is already upstream as f4e8ab7, and is tested already with a >> compiled >> kernel deb with the fix included. > > applied, packaged, and uploaded Did you also apply it at the master branch? You closed the xM bug as a duplicate of the Omap 4 one, but I don't see this commit at the master branch yet. Thanks,
On 11/16/2010 06:15 PM, Ricardo Salveti wrote: > On Tue, Nov 16, 2010 at 11:51 PM, Tim Gardner<tim.gardner@canonical.com> wrote: >> On 11/11/2010 07:24 AM, Ricardo Salveti de Araujo wrote: >>> >>> SRU Justification: >>> >>> Impact: The smsc95xx driver generates a new random MAC address on every >>> ifdown/up instead of only one during driver's init, and doesn't let the >>> user to >>> set up the mac address using known methods like ifconfig usb0 hw >>> ether<mac>. >>> >>> Fix: Just move the init_mac_address function to bind instead of reset. >>> >>> Testcase: The interface should keep the same generated mac address even >>> when >>> giving ifdown/up, and the user should be able to set up the mac address >>> using >>> 'ifconfig usb0 hw ether<mac>. >>> >>> Patch is already upstream as f4e8ab7, and is tested already with a >>> compiled >>> kernel deb with the fix included. >> >> applied, packaged, and uploaded > > Did you also apply it at the master branch? You closed the xM bug as a > duplicate of the Omap 4 one, but I don't see this commit at the master > branch yet. > > Thanks, I'll take care of that. Brad
On 11/16/2010 07:16 PM, Brad Figg wrote: > On 11/16/2010 06:15 PM, Ricardo Salveti wrote: >> On Tue, Nov 16, 2010 at 11:51 PM, Tim Gardner<tim.gardner@canonical.com> wrote: >>> On 11/11/2010 07:24 AM, Ricardo Salveti de Araujo wrote: >>>> >>>> SRU Justification: >>>> >>>> Impact: The smsc95xx driver generates a new random MAC address on every >>>> ifdown/up instead of only one during driver's init, and doesn't let the >>>> user to >>>> set up the mac address using known methods like ifconfig usb0 hw >>>> ether<mac>. >>>> >>>> Fix: Just move the init_mac_address function to bind instead of reset. >>>> >>>> Testcase: The interface should keep the same generated mac address even >>>> when >>>> giving ifdown/up, and the user should be able to set up the mac address >>>> using >>>> 'ifconfig usb0 hw ether<mac>. >>>> >>>> Patch is already upstream as f4e8ab7, and is tested already with a >>>> compiled >>>> kernel deb with the fix included. >>> >>> applied, packaged, and uploaded >> >> Did you also apply it at the master branch? You closed the xM bug as a >> duplicate of the Omap 4 one, but I don't see this commit at the master >> branch yet. >> >> Thanks, > > I'll take care of that. > > Brad Oops, good catch. rtg
On 11/11/2010 06:24 AM, Ricardo Salveti de Araujo wrote: > SRU Justification: > > Impact: The smsc95xx driver generates a new random MAC address on every > ifdown/up instead of only one during driver's init, and doesn't let the user to > set up the mac address using known methods like ifconfig usb0 hw ether<mac>. > > Fix: Just move the init_mac_address function to bind instead of reset. > > Testcase: The interface should keep the same generated mac address even when > giving ifdown/up, and the user should be able to set up the mac address using > 'ifconfig usb0 hw ether<mac>. > > Patch is already upstream as f4e8ab7, and is tested already with a compiled > kernel deb with the fix included. > > BugLink: https://bugs.launchpad.net/bugs/673504 and https://bugs.launchpad.net/bugs/673509 > > From: Bernard Blackham<bernard@largestprime.net> > Date: Tue, 19 Oct 2010 10:16:39 +1100 > Subject: [PATCH] smsc95xx: generate random MAC address once, not every ifup > > The smsc95xx driver currently generates a new random MAC address > every time the interface is brought up. This makes it impossible to > override using the standard `ifconfig hw ether` approach. > > Past patches tried to make the MAC address a module parameter or > base it off the die ID, but it seems to me much simpler (and > hopefully less controversial) to stick with the current random > generation scheme, but allow the user to change the address. > > This patch does exactly that - it moves the random address > generation from smsc95xx_reset() into smsc95xx_bind(), so that it is > done once on module load, not on every ifup. The user can then > override this using the standard mechanisms. > > Applies against 2.6.35 and linux-2.6 head. > > Signed-off-by: Bernard Blackham<b-omap@largestprime.net> > Signed-off-by: Ricardo Salveti de Araujo<ricardo.salveti@canonical.com> > --- > drivers/net/usb/smsc95xx.c | 4 ++-- > 1 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c > index 12a3c88..65cb1ab 100644 > --- a/drivers/net/usb/smsc95xx.c > +++ b/drivers/net/usb/smsc95xx.c > @@ -805,8 +805,6 @@ static int smsc95xx_reset(struct usbnet *dev) > return ret; > } > > - smsc95xx_init_mac_address(dev); > - > ret = smsc95xx_set_mac_address(dev); > if (ret< 0) > return ret; > @@ -1047,6 +1045,8 @@ static int smsc95xx_bind(struct usbnet *dev, struct usb_interface *intf) > pdata->use_tx_csum = DEFAULT_TX_CSUM_ENABLE; > pdata->use_rx_csum = DEFAULT_RX_CSUM_ENABLE; > > + smsc95xx_init_mac_address(dev); > + > /* Init all registers */ > ret = smsc95xx_reset(dev); > Applied to Maverick master branch.
diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c index 12a3c88..65cb1ab 100644 --- a/drivers/net/usb/smsc95xx.c +++ b/drivers/net/usb/smsc95xx.c @@ -805,8 +805,6 @@ static int smsc95xx_reset(struct usbnet *dev) return ret; } - smsc95xx_init_mac_address(dev); - ret = smsc95xx_set_mac_address(dev); if (ret < 0) return ret; @@ -1047,6 +1045,8 @@ static int smsc95xx_bind(struct usbnet *dev, struct usb_interface *intf) pdata->use_tx_csum = DEFAULT_TX_CSUM_ENABLE; pdata->use_rx_csum = DEFAULT_RX_CSUM_ENABLE; + smsc95xx_init_mac_address(dev); + /* Init all registers */ ret = smsc95xx_reset(dev);