[PATCH] net: usb: usbnet: restore usb%d name exception for local mac addresses

[PATCH] net: usb: usbnet: restore usb%d name exception for local mac addresses

[Dominique Martinet]

From: Dominique Martinet <dominique.martinet@atmark-techno.com>

The previous commit assumed that local addresses always came from the
kernel, but some devices hand out local mac addresses so we ended up
with point-to-point devices with a mac set by the driver, renaming to
eth%d when they used to be named usb%d.

Userspace should not rely on device name, but for the sake of stability
restore the local mac address check portion of the naming exception:
point to point devices which either have no mac set by the driver or
have a local mac handed out by the driver will keep the usb%d name.

Fixes: 8a7d12d674ac ("net: usb: usbnet: fix name regression")
Signed-off-by: Dominique Martinet <dominique.martinet@atmark-techno.com>
---
 drivers/net/usb/usbnet.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 44179f4e807f..d044dc7b7622 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -178,6 +178,17 @@ int usbnet_get_ethernet_addr(struct usbnet *dev, int iMACAddress)
 }
 EXPORT_SYMBOL_GPL(usbnet_get_ethernet_addr);
 
+static bool usbnet_needs_usb_name_format(struct usbnet *dev, struct net_device *net)
+{
+	/* 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_zero_ether_addr(net->dev_addr) ||
+		 is_local_ether_addr(net->dev_addr));
+}
+
 static void intr_complete (struct urb *urb)
 {
 	struct usbnet	*dev = urb->context;
@@ -1762,13 +1773,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_needs_usb_name_format(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)
-- 
2.47.0

Re: [PATCH] net: usb: usbnet: restore usb%d name exception for local mac addresses

[Greg KH]

On Tue, Dec 03, 2024 at 10:04:55PM +0900, Dominique Martinet wrote:
> From: Dominique Martinet <dominique.martinet@atmark-techno.com>
> 
> The previous commit assumed that local addresses always came from the
> kernel, but some devices hand out local mac addresses so we ended up
> with point-to-point devices with a mac set by the driver, renaming to
> eth%d when they used to be named usb%d.
> 
> Userspace should not rely on device name, but for the sake of stability
> restore the local mac address check portion of the naming exception:
> point to point devices which either have no mac set by the driver or
> have a local mac handed out by the driver will keep the usb%d name.
> 
> Fixes: 8a7d12d674ac ("net: usb: usbnet: fix name regression")
> Signed-off-by: Dominique Martinet <dominique.martinet@atmark-techno.com>
> ---
>  drivers/net/usb/usbnet.c | 20 ++++++++++++++------
>  1 file changed, 14 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
> index 44179f4e807f..d044dc7b7622 100644
> --- a/drivers/net/usb/usbnet.c
> +++ b/drivers/net/usb/usbnet.c
> @@ -178,6 +178,17 @@ int usbnet_get_ethernet_addr(struct usbnet *dev, int iMACAddress)
>  }
>  EXPORT_SYMBOL_GPL(usbnet_get_ethernet_addr);
>  
> +static bool usbnet_needs_usb_name_format(struct usbnet *dev, struct net_device *net)
> +{
> +	/* 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_zero_ether_addr(net->dev_addr) ||
> +		 is_local_ether_addr(net->dev_addr));
> +}
> +
>  static void intr_complete (struct urb *urb)
>  {
>  	struct usbnet	*dev = urb->context;
> @@ -1762,13 +1773,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_needs_usb_name_format(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)
> -- 
> 2.47.0
> 
> 

Hi,

This is the friendly patch-bot of Greg Kroah-Hartman.  You have sent him
a patch that has triggered this response.  He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created.  Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- You have marked a patch with a "Fixes:" tag for a commit that is in an
  older released kernel, yet you do not have a cc: stable line in the
  signed-off-by area at all, which means that the patch will not be
  applied to any older kernel releases.  To properly fix this, please
  follow the documented rules in the
  Documentation/process/stable-kernel-rules.rst file for how to resolve
  this.

If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot

Re: [PATCH] net: usb: usbnet: restore usb%d name exception for local mac addresses

[Andrew Lunn]

On Tue, Dec 03, 2024 at 10:04:55PM +0900, Dominique Martinet wrote:
> From: Dominique Martinet <dominique.martinet@atmark-techno.com>
> 
> The previous commit assumed that local addresses always came from the
> kernel, but some devices hand out local mac addresses so we ended up
> with point-to-point devices with a mac set by the driver, renaming to
> eth%d when they used to be named usb%d.
> 
> Userspace should not rely on device name, but for the sake of stability
> restore the local mac address check portion of the naming exception:
> point to point devices which either have no mac set by the driver or
> have a local mac handed out by the driver will keep the usb%d name.

Are you saying the OTP or NVMEM has a locally administered MAC address
stored in it? Is there a mechanism to change it? The point about
locally administered MAC addresses is that they are locally
administered.

	Andrew

Re: [PATCH] net: usb: usbnet: restore usb%d name exception for local mac addresses

[Jakub Kicinski]

On Tue, 3 Dec 2024 21:47:57 +0100 Andrew Lunn wrote:
> Are you saying the OTP or NVMEM has a locally administered MAC address
> stored in it? Is there a mechanism to change it? The point about
> locally administered MAC addresses is that they are locally
> administered.

You may also enjoy reading this :)

commit bfe9b9d2df669a57a95d641ed46eb018e204c6ce
Author: Kristian Evensen <kristian.evensen@gmail.com>
Date:   Thu Jul 21 11:10:06 2016 +0200

    cdc_ether: Improve ZTE MF823/831/910 handling
    
    The firmware in several ZTE devices (at least the MF823/831/910
    modems/mifis) use OS fingerprinting to determine which type of device to
    export. In addition, these devices export a REST API which can be used to
    control the type of device. So far, on Linux, the devices have been seen as
    RNDIS or CDC Ether.
    
    When CDC Ether is used, devices of the same type are, as with RNDIS,
    exported with the same, bogus random MAC address. In addition, the devices
    (at least on all firmware revisions I have found) use the bogus MAC when
    sending traffic routed from external networks. And as a final feature, the
    devices sometimes export the link state incorrectly. There are also
    references online to several other ZTE devices displaying this behavior,
    with several different PIDs and MAC addresses.

Re: [PATCH] net: usb: usbnet: restore usb%d name exception for local mac addresses

[Dominique Martinet]

Andrew Lunn wrote on Tue, Dec 03, 2024 at 09:47:57PM +0100:
> On Tue, Dec 03, 2024 at 10:04:55PM +0900, Dominique Martinet wrote:
> > From: Dominique Martinet <dominique.martinet@atmark-techno.com>
> > 
> > The previous commit assumed that local addresses always came from the
> > kernel, but some devices hand out local mac addresses so we ended up
> > with point-to-point devices with a mac set by the driver, renaming to
> > eth%d when they used to be named usb%d.
> > 
> > Userspace should not rely on device name, but for the sake of stability
> > restore the local mac address check portion of the naming exception:
> > point to point devices which either have no mac set by the driver or
> > have a local mac handed out by the driver will keep the usb%d name.
> 
> Are you saying the OTP or NVMEM has a locally administered MAC address
> stored in it?

I'm afraid so... (At least the Gemalto^W Cinterion^W Thales^W Telit
ELS31-J we use on a couple of boards seem to do that, it's soldered on
our boards so I can't swap it out easily to confirm but the mac address
is stable accross reboots)

The good news is that after having been sold at least 4 times it's been
made EOL now, so in another 12-ish years I'll probably be able to ignore
this particular problem :)


> Is there a mechanism to change it?

Looking at some confidential documentation I found on our file server
there seems to be an usb function that contains the mac address and
various ethernet statistics, but it's not clear to me if it's actually
writable or even how to actually use it in practice and it was certainly
not designed with being modified in mind.

(I suspect there should be some vendor AT command that would allow
overriding the setting somewhere but I can't find that either)

OTOH, just changing the mac locally (ip link set usb0 addr
02:12:34:56:78:90) works and dhcp gets me a new IP, so it's not like
overriding it is a problem either.
(interestingly putting the old mac back gets me the old IP back, so
there's a real dhcp server with leases behind this and I suspect I
could just bridge this out and it'd work as expected...) 


> The point about locally administered MAC addresses is that they are
> locally administered.

Honest question here our of curiosity, my reading of a few random pages
on the internet is that it would be acceptable for the modem to randomly
generate it?
(under the assumption that e.g. a reset would clear it and get me a new
mac)

Or does it have to be assigned as late as possible, e.g. we'd want linux
to be generating it in this case?


Thanks,
-- 
Dominique

Re: [PATCH] net: usb: usbnet: restore usb%d name exception for local mac addresses

[Andrew Lunn]

> > The point about locally administered MAC addresses is that they are
> > locally administered.
> 
> Honest question here our of curiosity, my reading of a few random pages
> on the internet is that it would be acceptable for the modem to randomly
> generate it?
> (under the assumption that e.g. a reset would clear it and get me a new
> mac)
> 
> Or does it have to be assigned as late as possible, e.g. we'd want linux
> to be generating it in this case?

Honest, answer is, i've never read what IEEE says about locally
administered MAC addresses.

The general pattern in linux is, if there is no alternative, generate
a random MAC address in the locally administered range. The sysadmin
should be able to change it via ip link set address, and more likely
making it 'permanent' by setting it in /etc/network/interfaces or
whatever the distribution uses. I would not be too surprised if some
MAC drivers are broken and that fails.

Since you can change the MAC address at runtime, and you say they do
appear to be somewhat random, i don't see why we cannot live with
this. It can still be locally administrated if need be. But it is no
something we want to see OEMs do, they really should get an OUI.

	Andrew

Re: [PATCH] net: usb: usbnet: restore usb%d name exception for local mac addresses

[Ahmed Naseef]

Hello,  

I have tested this patch and can confirm that it works as expected with at least three models
of Quectel LTE modems.  


Tested-by: Ahmed Naseef <naseefkm@gmail.com> 

This issue affects many users of OpenWrt, where USB LTE modems are widely used. The device
name change has caused significant inconvenience, and as a result, this patch has already been
accepted in OpenWrt:

https://github.com/openwrt/openwrt/commit/ecd609f509f29ed1f75db5c7a623f359c64efb72  

Restoring the previous naming convention at the kernel level would greatly help in maintaining 
consistency and avoiding unnecessary workarounds in userspace which is not straightforward in openwrt.  

I hope this feedback helps in reconsidering the patch for mainline inclusion.  

Best regards,  
Ahmed Naseef  

Re: [PATCH] net: usb: usbnet: restore usb%d name exception for local mac addresses

[Jakub Kicinski]

On Wed, 26 Mar 2025 11:27:26 +0400 Ahmed Naseef wrote:
> Hello,  
> 
> I have tested this patch and can confirm that it works as expected with at least three models
> of Quectel LTE modems.  
> 
> 
> Tested-by: Ahmed Naseef <naseefkm@gmail.com> 
> 
> This issue affects many users of OpenWrt, where USB LTE modems are widely used. The device
> name change has caused significant inconvenience, and as a result, this patch has already been
> accepted in OpenWrt:
> 
> https://github.com/openwrt/openwrt/commit/ecd609f509f29ed1f75db5c7a623f359c64efb72  
> 
> Restoring the previous naming convention at the kernel level would greatly help in maintaining 
> consistency and avoiding unnecessary workarounds in userspace which is not straightforward in openwrt.  
> 
> I hope this feedback helps in reconsidering the patch for mainline inclusion.  

It needs to be reposted to be reconsidered, FWIW

Re: [PATCH] net: usb: usbnet: restore usb%d name exception for local mac addresses

[Dominique Martinet]

Jakub Kicinski wrote on Wed, Mar 26, 2025 at 04:11:58AM -0700:
> On Wed, 26 Mar 2025 11:27:26 +0400 Ahmed Naseef wrote:
> > I hope this feedback helps in reconsidering the patch for mainline inclusion.  
> 
> It needs to be reposted to be reconsidered, FWIW

I just reposted it here after this reminder:
https://lkml.kernel.org/r/20250326-usbnet_rename-v2-1-57eb21fcff26@atmark-techno.com

I've just remembered the timing might not be great though with the merge
window that just started, and now I'm (re)reading through
Documentation/procss/maintainer-netdev.rst I pobably should have added
net-next? to the subject... If it weren't closed.

I'll re-repost for net-next after -rc1, unless something happens to the
patch earlier.

Thanks,
-- 
Dominique

Re: [PATCH] net: usb: usbnet: restore usb%d name exception for local mac addresses

[Jakub Kicinski]

On Wed, 26 Mar 2025 20:34:33 +0900 Dominique Martinet wrote:
> Jakub Kicinski wrote on Wed, Mar 26, 2025 at 04:11:58AM -0700:
> > On Wed, 26 Mar 2025 11:27:26 +0400 Ahmed Naseef wrote:  
> > > I hope this feedback helps in reconsidering the patch for mainline inclusion.    
> > 
> > It needs to be reposted to be reconsidered, FWIW  
> 
> I just reposted it here after this reminder:
> https://lkml.kernel.org/r/20250326-usbnet_rename-v2-1-57eb21fcff26@atmark-techno.com

Thanks!

> I've just remembered the timing might not be great though with the merge
> window that just started, and now I'm (re)reading through
> Documentation/procss/maintainer-netdev.rst I pobably should have added
> net-next? to the subject... If it weren't closed.
> 
> I'll re-repost for net-next after -rc1, unless something happens to the
> patch earlier.

It's in a gray zone between a fix and an improvement, we can take it
via net at this stage.