[PATCH] usbnet: modern method to get random MAC

[PATCH] usbnet: modern method to get random MAC

[Oliver Neukum]

The old method had the bug of issuing the same
random MAC over and over even to two devices at the
same time. Instead of fixing that, this does as
Jakub suggested and moves to the modern method.

This bug is as old as the driver.

Signed-off-by: Oliver Neukum <oneukum@suse.com>
---
 drivers/net/usb/usbnet.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index c70def1a87ca..715cc8b66776 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -61,9 +61,6 @@
 
 /*-------------------------------------------------------------------------*/
 
-// randomly generated ethernet address
-static u8	node_id [ETH_ALEN];
-
 /* use ethtool to change the level for any given device */
 static int msg_level = -1;
 module_param (msg_level, int, 0);
@@ -1725,7 +1722,6 @@ usbnet_probe (struct usb_interface *udev, const struct usb_device_id *prod)
 
 	dev->net = net;
 	strscpy(net->name, "usb%d", sizeof(net->name));
-	eth_hw_addr_set(net, node_id);
 
 	/* rx and tx sides can use different message sizes;
 	 * bind() should set rx_urb_size in that case.
@@ -1793,9 +1789,13 @@ usbnet_probe (struct usb_interface *udev, const struct usb_device_id *prod)
 		goto out4;
 	}
 
-	/* let userspace know we have a random address */
-	if (ether_addr_equal(net->dev_addr, node_id))
-		net->addr_assign_type = NET_ADDR_RANDOM;
+	/*
+	 * if the device does not come with a MAC
+	 * we ask the network core to generate us one
+	 * and flag the device accordingly
+	 */
+	if (!is_valid_ether_addr(net->dev_addr))
+		eth_hw_addr_random(net);
 
 	if ((dev->driver_info->flags & FLAG_WLAN) != 0)
 		SET_NETDEV_DEVTYPE(net, &wlan_type);
@@ -2205,7 +2205,6 @@ static int __init usbnet_init(void)
 	BUILD_BUG_ON(
 		sizeof_field(struct sk_buff, cb) < sizeof(struct skb_data));
 
-	eth_random_addr(node_id);
 	return 0;
 }
 module_init(usbnet_init);
-- 
2.35.3

Re: [PATCH] usbnet: modern method to get random MAC

[Paolo Abeni]

On Wed, 2022-07-06 at 11:15 +0200, Oliver Neukum wrote:
> The old method had the bug of issuing the same
> random MAC over and over even to two devices at the
> same time. Instead of fixing that, this does as
> Jakub suggested and moves to the modern method.
> 
> This bug is as old as the driver.
> 
> Signed-off-by: Oliver Neukum <oneukum@suse.com>
> ---
>  drivers/net/usb/usbnet.c | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
> index c70def1a87ca..715cc8b66776 100644
> --- a/drivers/net/usb/usbnet.c
> +++ b/drivers/net/usb/usbnet.c
> @@ -61,9 +61,6 @@
>  
>  /*-------------------------------------------------------------------------*/
>  
> -// randomly generated ethernet address
> -static u8	node_id [ETH_ALEN];
> -
>  /* use ethtool to change the level for any given device */
>  static int msg_level = -1;
>  module_param (msg_level, int, 0);
> @@ -1725,7 +1722,6 @@ usbnet_probe (struct usb_interface *udev, const struct usb_device_id *prod)
>  
>  	dev->net = net;
>  	strscpy(net->name, "usb%d", sizeof(net->name));
> -	eth_hw_addr_set(net, node_id);
>  
>  	/* rx and tx sides can use different message sizes;
>  	 * bind() should set rx_urb_size in that case.
> @@ -1793,9 +1789,13 @@ usbnet_probe (struct usb_interface *udev, const struct usb_device_id *prod)
>  		goto out4;
>  	}
>  
> -	/* let userspace know we have a random address */
> -	if (ether_addr_equal(net->dev_addr, node_id))
> -		net->addr_assign_type = NET_ADDR_RANDOM;
> +	/*
> +	 * if the device does not come with a MAC
> +	 * we ask the network core to generate us one
> +	 * and flag the device accordingly
> +	 */

Minir nit: even if usbnet uses a mixed style for comments, I think it's
better to stick to netdev format for new code.

	/* if the device does not come with a MAC
...
	 */

Thanks!

/P

> +	if (!is_valid_ether_addr(net->dev_addr))
> +		eth_hw_addr_random(net);
>  
>  	if ((dev->driver_info->flags & FLAG_WLAN) != 0)
>  		SET_NETDEV_DEVTYPE(net, &wlan_type);
> @@ -2205,7 +2205,6 @@ static int __init usbnet_init(void)
>  	BUILD_BUG_ON(
>  		sizeof_field(struct sk_buff, cb) < sizeof(struct skb_data));
>  
> -	eth_random_addr(node_id);
>  	return 0;
>  }
>  module_init(usbnet_init);