| Message ID | 20241015140442.247752-1-oneukum@suse.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [RFC] net: usb: usbnet: fix name regression | expand |
On Tue, Oct 15, 2024 at 04:03:32PM +0200, Oliver Neukum wrote: > The fix for MAC addresses broke detection of the naming convention > because it gave network devices no random MAC before bind() > was called. This means that the check for the local assignment bit > was always negative as the address was zeroed from allocation, > instead of from overwriting the MAC with a unique hardware address. > > The correct check for whether bind() has altered the MAC is > done with is_zero_ether_addr > > Signed-off-by: Oliver Neukum <oneukum@suse.com> > Reported-by: Greg Thelen <gthelen@google.com> > Diagnosed-by: John Sperbeck <jsperbeck@google.com> > Fixes: bab8eb0dd4cb9 ("usbnet: modern method to get random MAC") > --- > drivers/net/usb/usbnet.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c > index ee1b5fd7b491..44179f4e807f 100644 > --- a/drivers/net/usb/usbnet.c > +++ b/drivers/net/usb/usbnet.c > @@ -1767,7 +1767,8 @@ usbnet_probe (struct usb_interface *udev, const struct usb_device_id *prod) > // can rename the link if it knows better. > if ((dev->driver_info->flags & FLAG_ETHER) != 0 && > ((dev->driver_info->flags & FLAG_POINTTOPOINT) == 0 || > - (net->dev_addr [0] & 0x02) == 0)) > + /* somebody touched it*/ > + !is_zero_ether_addr(net->dev_addr))) Hi Oliver, I think works for the case where a random address will be assigned as per the cited commit. But I'm unsure that is correct wrt to the case where ->bind assigns an address with 0x2 set in the 0th octet. Can that occur in practice? Perhaps not because the driver would rely on usbnet_probe() to set a random address. But if so then it would previously have hit the "eth%d" logic, but does not anymore. > strscpy(net->name, "eth%d", sizeof(net->name)); > /* WLAN devices should always be named "wlan%d" */ > if ((dev->driver_info->flags & FLAG_WLAN) != 0) > -- > 2.47.0 > >
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c index ee1b5fd7b491..44179f4e807f 100644 --- a/drivers/net/usb/usbnet.c +++ b/drivers/net/usb/usbnet.c @@ -1767,7 +1767,8 @@ usbnet_probe (struct usb_interface *udev, const struct usb_device_id *prod) // can rename the link if it knows better. if ((dev->driver_info->flags & FLAG_ETHER) != 0 && ((dev->driver_info->flags & FLAG_POINTTOPOINT) == 0 || - (net->dev_addr [0] & 0x02) == 0)) + /* somebody touched it*/ + !is_zero_ether_addr(net->dev_addr))) strscpy(net->name, "eth%d", sizeof(net->name)); /* WLAN devices should always be named "wlan%d" */ if ((dev->driver_info->flags & FLAG_WLAN) != 0)
The fix for MAC addresses broke detection of the naming convention because it gave network devices no random MAC before bind() was called. This means that the check for the local assignment bit was always negative as the address was zeroed from allocation, instead of from overwriting the MAC with a unique hardware address. The correct check for whether bind() has altered the MAC is done with is_zero_ether_addr Signed-off-by: Oliver Neukum <oneukum@suse.com> Reported-by: Greg Thelen <gthelen@google.com> Diagnosed-by: John Sperbeck <jsperbeck@google.com> Fixes: bab8eb0dd4cb9 ("usbnet: modern method to get random MAC") --- drivers/net/usb/usbnet.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)