Hi,
Oliver Neukum wrote on Thu, Oct 17, 2024 at 09:18:37AM +0200:
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.
So we hit the exact inverse problem with this patch: our device ships an LTE modem which exposes a cdc-ethernet interface that had always been named usb0, and with this patch it started being named eth1, breaking too many hardcoded things expecting the name to be usb0 and making our devices unable to connect to the internet after updating the kernel.
Long term we'll probably add an udev rule or something to make the name explicit in userspace and not risk this happening again, but perhaps there's a better way to keep the old behavior?
(In particular this hit all stable kernels last month so I'm sure we won't be the only ones getting annoyed with this... Perhaps reverting both patches for stable branches might make sense if no better way forward is found -- I've added stable@ in cc for heads up/opinions)
+++ 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)))
... or actually now I'm looking at it again, perhaps is the check just backwards, or am I getting this wrong? previous check was rename if (mac[0] & 0x2 == 0), which reads to me as "nobody set the 2nd bit" new check now renames if !is_zero, so renames if it was set, which is the opposite?...
strscpy(net->name, "eth%d", sizeof(net->name)); /* WLAN devices should always be named "wlan%d" */
Thanks,
On Mon, Dec 02, 2024 at 12:50:15PM +0900, Dominique MARTINET wrote:
Hi,
Oliver Neukum wrote on Thu, Oct 17, 2024 at 09:18:37AM +0200:
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.
So we hit the exact inverse problem with this patch: our device ships an LTE modem which exposes a cdc-ethernet interface that had always been named usb0, and with this patch it started being named eth1, breaking too many hardcoded things expecting the name to be usb0 and making our devices unable to connect to the internet after updating the kernel.
Long term we'll probably add an udev rule or something to make the name explicit in userspace and not risk this happening again, but perhaps there's a better way to keep the old behavior?
(In particular this hit all stable kernels last month so I'm sure we won't be the only ones getting annoyed with this... Perhaps reverting both patches for stable branches might make sense if no better way forward is found -- I've added stable@ in cc for heads up/opinions)
Device names have NEVER been stable. They move around and can change on every boot, let alone almost always changing between kernel versions. That's why we created udev, to solve this issue.
But how is changing the MAC address changing the device naming here? How are they linked to suddenly causing the names to switch around?
thanks,
greg k-h
Greg Kroah-Hartman wrote on Mon, Dec 02, 2024 at 07:29:52AM +0100:
On Mon, Dec 02, 2024 at 12:50:15PM +0900, Dominique MARTINET wrote:
So we hit the exact inverse problem with this patch: our device ships an LTE modem which exposes a cdc-ethernet interface that had always been named usb0, and with this patch it started being named eth1, breaking too many hardcoded things expecting the name to be usb0 and making our devices unable to connect to the internet after updating the kernel.
Long term we'll probably add an udev rule or something to make the name explicit in userspace and not risk this happening again, but perhaps there's a better way to keep the old behavior?
(In particular this hit all stable kernels last month so I'm sure we won't be the only ones getting annoyed with this... Perhaps reverting both patches for stable branches might make sense if no better way forward is found -- I've added stable@ in cc for heads up/opinions)
Device names have NEVER been stable. They move around and can change on every boot, let alone almost always changing between kernel versions. That's why we created udev, to solve this issue.
Yes, I agree and we will add an udev rule to enforce the name for later updates (I really am just a messenger here as "the kernel guy", after having been asked why did this change), and I have no problem with this behavior changing on master whatever the direction this takes (... assuming the check was written as intended)
Now you're saying this I guess the main reason we were affected is that alpine never made the "stable network interface name" systemd-udev switch, so for most people that interface will just be named enx028072781510 anyway and most people will probably not notice this... (But then again these don't use the "new" name either, so they just don't care)
With that said, and while I agree the names can change, I still think there's some hierarchy here -- the X part of ethX/usbX can change on every boot and I have zero problem with that, but I wouldn't expect the "type" part to change so easily, and I would have assume stable kernels would want to at least try to preserve these unless there is a good reason not to. The two commits here (8a7d12d674ac ("net: usb: usbnet: fix name regression") and bab8eb0dd4cb ("usbnet: modern method to get random MAC") before it) are just cleanup I see no problem reverting them for stable kernels if they cause any sort of issue, regardless of how userspace "should" work.
But how is changing the MAC address changing the device naming here? How are they linked to suddenly causing the names to switch around?
That's also something I'd like to understand :)
Apparently, usbnet had a rule that said that if a device is ethernet, and either (it's not point-to-point) or (mac[0] & 0x2 == 0) then we would rename it to ethX instad of the usbX name.
commit 8a7d12d674ac ("net: usb: usbnet: fix name regression") made it so the last part of the check would rename it to ethX if the mac has been set by any driver, so my understanding is that all drivers that used to set the mac to something that avoided the 0x2 would now get renamed?... But as you can see above from the stable device name I gave, the mac address does start with 02, so I don't understand why it hadn't been renamed before or what this rules mean and why it's here...?
The commit message mentions commit bab8eb0dd4cb ("usbnet: modern method to get random MAC") which changed the timing usbnet would set a random mac, but in my case the mac does come from the hardware (it's stable across reboots), so I guess I wasn't affected by that commit but the new one trying to make it better for people with a random mac made it worse for my case?
Anyway, as said above we'll try to figure something for udev, and this will hopefully be a heads up for anyone else falling here doing a web search. (Our users are rather adverse to changes so I don't see myself enabling static interface names anytime soon, but time will tell how that turns out...)
Cheers,
From: Dominique MARTINET
Sent: 02 December 2024 03:50
Hi,
Oliver Neukum wrote on Thu, Oct 17, 2024 at 09:18:37AM +0200:
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.
So we hit the exact inverse problem with this patch: our device ships an LTE modem which exposes a cdc-ethernet interface that had always been named usb0, and with this patch it started being named eth1, breaking too many hardcoded things expecting the name to be usb0 and making our devices unable to connect to the internet after updating the kernel.
Erm does that mean your modem has a locally administered MAC address? It really shouldn't.
Long term we'll probably add an udev rule or something to make the name explicit in userspace and not risk this happening again, but perhaps there's a better way to keep the old behavior?
(In particular this hit all stable kernels last month so I'm sure we won't be the only ones getting annoyed with this... Perhaps reverting both patches for stable branches might make sense if no better way forward is found -- I've added stable@ in cc for heads up/opinions)
+++ 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)))
... or actually now I'm looking at it again, perhaps is the check just backwards, or am I getting this wrong? previous check was rename if (mac[0] & 0x2 == 0), which reads to me as "nobody set the 2nd bit" new check now renames if !is_zero, so renames if it was set, which is the opposite?...
The 2nd bit (aka mac[0] & 2) is the 'locally administered' bit. The intention of the standard was that all manufacturers would get a valid 14-bit OUI and the EEPROM (or equivalent) would contain an addresses with that bit clear, such addresses should be globally unique. Alternatively the local network administrator could assign an address with that bit set, required by protocols like DECnet.
This has never actually been strictly true, a few manufacturers used 'locally administered addresses' (02:cf:1f:xx:xx:xx comes to mind) and systems typically allow any (non-broadcast) be set.
So basing any decision on whether a MAC address is local or global is always going to be confusing.
Linux will allocate a random (locally administered) address if none is found - usually due to a corrupt eeprom.
David
strscpy(net->name, "eth%d", sizeof(net->name)); /* WLAN devices should always be named "wlan%d" */
Thanks,
Dominique Martinet
- Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
(Sorry for back-to-back replies, hadn't noticed this when I wrote the previous mail)
David Laight wrote on Mon, Dec 02, 2024 at 08:17:59AM +0000:
So we hit the exact inverse problem with this patch: our device ships an LTE modem which exposes a cdc-ethernet interface that had always been named usb0, and with this patch it started being named eth1, breaking too many hardcoded things expecting the name to be usb0 and making our devices unable to connect to the internet after updating the kernel.
Erm does that mean your modem has a locally administered MAC address? It really shouldn't.
Unfortunately, that's what it gives us: # ip a show dev eth1 4: eth1: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN qlen 1000 link/ether 02:80:72:78:15:10 brd ff:ff:ff:ff:ff:ff
# udevadm info /sys/class/net/eth1 P: /devices/platform/soc/2100000.bus/2184200.usb/ci_hdrc.1/usb2/2-1/2-1:1.0/net/eth1 E: DEVPATH=/devices/platform/soc/2100000.bus/2184200.usb/ci_hdrc.1/usb2/2-1/2-1:1.0/net/eth1 E: ID_BUS=usb E: ID_MM_CANDIDATE=1 E: ID_MODEL=GTO E: ID_MODEL_ENC=GTO E: ID_MODEL_ID=00a0 E: ID_NET_NAME_MAC=enx028072781510 E: ID_REVISION=0307 E: ID_SERIAL=Gemalto_GTO_101578728002 E: ID_SERIAL_SHORT=101578728002 E: ID_TYPE=generic E: ID_USB_DRIVER=cdc_ether E: ID_USB_INTERFACES=:020600:0a0000:020201: E: ID_USB_INTERFACE_NUM=00 E: ID_VENDOR=Gemalto E: ID_VENDOR_ENC=Gemalto E: ID_VENDOR_ID=1e2d E: IFINDEX=4 E: INTERFACE=eth1 E: NM_UNMANAGED=1 E: SUBSYSTEM=net E: USEC_INITIALIZED=23339186
(that mac is stable accross reboots)
afaiu, this modem generates a pointtopoint ethernet device and then routes packets sent there to LTE.
I've just checked what my phone does when I do USB tethering and it's pretty similar, the mac is also a local 02:xx mac (02:36:05) which is also not in OUI lookup tables. (the only difference is, I plugged it on my laptop which runs systemd, so it got named enx023605xxxxxx and none of this matters..)
+++ 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)))
... or actually now I'm looking at it again, perhaps is the check just backwards, or am I getting this wrong? previous check was rename if (mac[0] & 0x2 == 0), which reads to me as "nobody set the 2nd bit" new check now renames if !is_zero, so renames if it was set, which is the opposite?...
The 2nd bit (aka mac[0] & 2) is the 'locally administered' bit. The intention of the standard was that all manufacturers would get a valid 14-bit OUI and the EEPROM (or equivalent) would contain an addresses with that bit clear, such addresses should be globally unique. Alternatively the local network administrator could assign an address with that bit set, required by protocols like DECnet.
This has never actually been strictly true, a few manufacturers used 'locally administered addresses' (02:cf:1f:xx:xx:xx comes to mind) and systems typically allow any (non-broadcast) be set.
So basing any decision on whether a MAC address is local or global is always going to be confusing.
Thank you for the explanation, I now understand the old check better -- point to point devices that are a local MAC addresses (0x2 set) would get to keep the usb0 name.
The new check however no longer cares about the address globality, and just basically always renames the interface if the driver provided a mac ?
If that is what was intended, I am fine with this, but I think these local ppp usb interfaces are rather common in the cheap modem world.
Linux will allocate a random (locally administered) address if none is found - usually due to a corrupt eeprom.
Thanks,
On Mon, 2 Dec 2024 17:36:51 +0900 'Dominique MARTINET' wrote:
The new check however no longer cares about the address globality, and just basically always renames the interface if the driver provided a mac ?
Any way we can identify those devices and not read the address from the device? Reading a locally administered address from the device seems rather pointless, we can generate one ourselves.
If that is what was intended, I am fine with this, but I think these local ppp usb interfaces are rather common in the cheap modem world.
Which will work, as long as they are marked appropriately; that is marked with FLAG_POINTTOPOINT.
Jakub Kicinski wrote on Mon, Dec 02, 2024 at 06:56:00AM -0800:
On Mon, 2 Dec 2024 17:36:51 +0900 'Dominique MARTINET' wrote:
The new check however no longer cares about the address globality, and just basically always renames the interface if the driver provided a mac ?
Any way we can identify those devices and not read the address from the device? Reading a locally administered address from the device seems rather pointless, we can generate one ourselves.
Would you want to regenerate a local address on every boot?
This might not have a properly allocated mac address range but this at least has a constant mac, so the devices can be easily identified (without looking at serial properties) .. I guess the generation might be made to be a hash from ID_USB_SERIAL or something like that, but if the device already provides something stable I don't see any reason not to use it?
(With that said, I don't see anything in `udevadm info` that'd easily point at this being a modem point to point device under the wraps..)
If that is what was intended, I am fine with this, but I think these local ppp usb interfaces are rather common in the cheap modem world.
Which will work, as long as they are marked appropriately; that is marked with FLAG_POINTTOPOINT.
Hmm, but the check here was either FLAG_POINTTOPOINT being unset or not locally administered address, so to keep the usb0 name we need both?
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));
i.e., something that didn't have FLAG_POINTTOPOINT in the first place would not get into this mac consideration, so it must be set.
My problematic device here has FLAG_POINTTOPOINT and a (locally admistered) mac address set, so it was not renamed up till now, but the new check makes the locally admistered mac address being set mean that it is no longer eligible to keep the usbX name.
... Would this check just be fine without any mac check at all? e.g. anything that's not flagged as point to point will be renamed ethX, and all non ethernet or point to point keep usbX.
On Tue, 3 Dec 2024 08:39:47 +0900 'Dominique MARTINET' wrote:
If that is what was intended, I am fine with this, but I think these local ppp usb interfaces are rather common in the cheap modem world.
Which will work, as long as they are marked appropriately; that is marked with FLAG_POINTTOPOINT.
Hmm, but the check here was either FLAG_POINTTOPOINT being unset or not locally administered address, so to keep the usb0 name we need both?
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));
i.e., something that didn't have FLAG_POINTTOPOINT in the first place would not get into this mac consideration, so it must be set.
Right! I missed the && plus ||
My problematic device here has FLAG_POINTTOPOINT and a (locally admistered) mac address set, so it was not renamed up till now, but the new check makes the locally admistered mac address being set mean that it is no longer eligible to keep the usbX name.
Ideally, udev would be the best option, like Greg said. This driver is already a fragile pile of workarounds.
If you really really want the old behavior tho, let's convert the zero check to !is_zero_ether_addr() && !is_local_ether_addr(). Maybe factor out the P2P + address validation to a helper because the && vs || is getting complicated.
Jakub Kicinski wrote on Mon, Dec 02, 2024 at 04:26:53PM -0800:
My problematic device here has FLAG_POINTTOPOINT and a (locally admistered) mac address set, so it was not renamed up till now, but the new check makes the locally admistered mac address being set mean that it is no longer eligible to keep the usbX name.
Ideally, udev would be the best option, like Greg said. This driver is already a fragile pile of workarounds.
Right, as I replied to Greg I'm fine with this as long as it's what was intended.
Half of the reason I sent the mail in the first place is I don't understand what commit 8a7d12d674ac ("net: usb: usbnet: fix name regression") actually fixes: the commit message desribes something about mac address not being set before bind() but the code does not change what address is looked at (net->dev_addr), just which bits of the address is checked; and I don't see what which bytes are being looked at changing has anything to do with the "fixed" commit bab8eb0dd4cb9 ("usbnet: modern method to get random MAC") ... And now we've started discussing this and I understand the check better, I also don't see what having a mac set by the previous driver has to do with the link not being P2P either.
(The other half was I was wondering what kind of policy stable would have for this kind of things, but that was made clear enough)
If you really really want the old behavior tho, let's convert the zero check to !is_zero_ether_addr() && !is_local_ether_addr().
As far as I understand, !is_local_ether_addr (mac[0] & 0x2) implies !is_zero_ether_addr (all bits of mac or'd), so that'd get us back to exactly the old check.
Maybe factor out the P2P + address validation to a helper because the && vs || is getting complicated.
... And I can definitely relate to this part :)
So: - final behavior wise I have no strong feeling, we'll fix our userspace (... and documentation) whatever is decided here - let's improve the comment and factor the check anyway. As said above I don't understand why having a mac set matters, if that can be explained I'll be happy to send a patch. Or if we go with the local address version, something like the following?
---- diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c index 44179f4e807f..240ae86adf08 100644 --- a/drivers/net/usb/usbnet.c +++ b/drivers/net/usb/usbnet.c @@ -178,6 +178,13 @@ int usbnet_get_ethernet_addr(struct usbnet *dev, int iMACAddress) } EXPORT_SYMBOL_GPL(usbnet_get_ethernet_addr);
+static bool usbnet_dev_is_two_host (struct usbnet *dev, struct net_device *net) +{ + /* device is marked point-to-point with a local mac address */ + return (dev->driver_info->flags & FLAG_POINTTOPOINT) != 0 && + is_local_ether_addr(net->dev_addr); +} + static void intr_complete (struct urb *urb) { struct usbnet *dev = urb->context; @@ -1762,13 +1769,10 @@ usbnet_probe (struct usb_interface *udev, const struct usb_device_id *prod) if (status < 0) goto out1;
- // 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. + /* heuristic: rename to "eth%d" if we are not sure this link + * is two-host (these links keep "usb%d") */ if ((dev->driver_info->flags & FLAG_ETHER) != 0 && - ((dev->driver_info->flags & FLAG_POINTTOPOINT) == 0 || - /* somebody touched it*/ - !is_zero_ether_addr(net->dev_addr))) + !usbnet_dev_is_two_host(dev, net)) strscpy(net->name, "eth%d", sizeof(net->name)); /* WLAN devices should always be named "wlan%d" */ if ((dev->driver_info->flags & FLAG_WLAN) != 0) ----
Thanks,
On Tue, 3 Dec 2024 10:18:44 +0900 'Dominique MARTINET' wrote:
Jakub Kicinski wrote on Mon, Dec 02, 2024 at 04:26:53PM -0800:
My problematic device here has FLAG_POINTTOPOINT and a (locally admistered) mac address set, so it was not renamed up till now, but the new check makes the locally admistered mac address being set mean that it is no longer eligible to keep the usbX name.
Ideally, udev would be the best option, like Greg said. This driver is already a fragile pile of workarounds.
Right, as I replied to Greg I'm fine with this as long as it's what was intended.
In theory unintentional != bug, but yes, its very likely unintentional.
Half of the reason I sent the mail in the first place is I don't understand what commit 8a7d12d674ac ("net: usb: usbnet: fix name regression") actually fixes: the commit message desribes something about mac address not being set before bind() but the code does not change what address is looked at (net->dev_addr), just which bits of the address is checked; and I don't see what which bytes are being looked at changing has anything to do with the "fixed" commit bab8eb0dd4cb9 ("usbnet: modern method to get random MAC")
We moved where the random address is assigned, we used to assign random (local) addr at init, now we assign it after calling ->bind().
Previously we checked "if local" as a shorthand for checking "if driver updated". This check should really have been "if addr == node_id".
... And now we've started discussing this and I understand the check better, I also don't see what having a mac set by the previous driver has to do with the link not being P2P either.
(The other half was I was wondering what kind of policy stable would have for this kind of things, but that was made clear enough)
If you really really want the old behavior tho, let's convert the zero check to !is_zero_ether_addr() && !is_local_ether_addr().
As far as I understand, !is_local_ether_addr (mac[0] & 0x2) implies !is_zero_ether_addr (all bits of mac or'd), so that'd get us back to exactly the old check.
Let the compiler discover that, this is control path code, so write it for the human reader... The condition we want is "driver did not initialize the MAC address, or it initialized it to a local MAC address".
Maybe factor out the P2P + address validation to a helper because the && vs || is getting complicated.
... And I can definitely relate to this part :)
So:
- final behavior wise I have no strong feeling, we'll fix our userspace
(... and documentation) whatever is decided here
- let's improve the comment and factor the check anyway.
As said above I don't understand why having a mac set matters, if that can be explained I'll be happy to send a patch. Or if we go with the local address version, something like the following?
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c index 44179f4e807f..240ae86adf08 100644 --- a/drivers/net/usb/usbnet.c +++ b/drivers/net/usb/usbnet.c @@ -178,6 +178,13 @@ int usbnet_get_ethernet_addr(struct usbnet *dev, int iMACAddress) } EXPORT_SYMBOL_GPL(usbnet_get_ethernet_addr); +static bool usbnet_dev_is_two_host (struct usbnet *dev, struct net_device *net)
static bool usenet_needs_usb_name_format(...
+{
- /* device is marked point-to-point with a local mac address */
/* Point to point devices which don't have a real MAC address * (or report a fake local one) have historically used the usb%d * naming. Preserve this.. */
- return (dev->driver_info->flags & FLAG_POINTTOPOINT) != 0 &&
is_local_ether_addr(net->dev_addr);
if ((dev->driver_info->flags & FLAG_POINTTOPOINT) && (is_zero_ether_addr(net->dev_addr) || is_local_ether_addr(net->dev_addr));
+}
Up to you if you want to send this.
Jakub Kicinski wrote on Mon, Dec 02, 2024 at 06:29:35PM -0800:
Half of the reason I sent the mail in the first place is I don't understand what commit 8a7d12d674ac ("net: usb: usbnet: fix name regression") actually fixes: the commit message desribes something about mac address not being set before bind() but the code does not change what address is looked at (net->dev_addr), just which bits of the address is checked; and I don't see what which bytes are being looked at changing has anything to do with the "fixed" commit bab8eb0dd4cb9 ("usbnet: modern method to get random MAC")
We moved where the random address is assigned, we used to assign random (local) addr at init, now we assign it after calling ->bind().
Previously we checked "if local" as a shorthand for checking "if driver updated". This check should really have been "if addr == node_id".
Ok, so a zero address here means a driver didn't set it, because the ex-"node_id" address was no longer set at this point, and these would fail the 0x2 check that worked previously...
The third time's a charm, the ordering part of the message just clicked for me, thank you for putting up with me.
As far as I understand, !is_local_ether_addr (mac[0] & 0x2) implies !is_zero_ether_addr (all bits of mac or'd), so that'd get us back to exactly the old check.
Let the compiler discover that, this is control path code, so write it for the human reader... The condition we want is "driver did not initialize the MAC address, or it initialized it to a local MAC address".
(I was reading that '&& !' wrong here, having the check negated in the helper is much more clear and it's required to keep the two anyway...)
Or if we go with the local address version, something like the following?
[...]
Up to you if you want to send this.
Thank you; after thinking it through today I think it won't hurt further to send so I did. Almost everyone involved is in Cc, but for follow-up it is here: https://lore.kernel.org/r/20241203130457.904325-1-asmadeus@codewreck.org
Thanks,
linux-stable-mirror@lists.linaro.org