Re: [PATCHv2 net] usbnet: modern method to get random MAC
From: Eric Dumazet <edumazet@google.com>
Date: 2024-10-14 19:59:49
Also in:
linux-usb, lkml
On Thu, Aug 29, 2024 at 7:52 PM Oliver Neukum [off-list ref] wrote:
quoted hunk ↗ jump to hunk
The driver generates a random MAC once on load and uses it over and over, including on two devices needing a random MAC at the same time. Jakub suggested revamping the driver to the modern API for setting a random MAC rather than fixing the old stuff. The bug is as old as the driver. Signed-off-by: Oliver Neukum <oneukum@suse.com> --- v2: Correct commentary style drivers/net/usb/usbnet.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-)diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c index dfc37016690b..40536e1cb4df 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);@@ -1743,7 +1740,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.@@ -1819,9 +1815,9 @@ 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; + /* this flags the device for user space */ + 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);@@ -2229,7 +2225,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.45.2
As diagnosed by John Sperbeck :
This patch implies all ->bind() method took care of populating net->dev_addr ?
Otherwise the following existing heuristic is no longer working
// heuristic: "usb%d" for links we know are two-host,
// else "eth%d" when there's reasonable doubt. userspace
// 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))
strscpy(net->name, "eth%d", sizeof(net->name));