netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: 'Dominique MARTINET' <dominique.martinet@atmark-techno.com>
To: Jakub Kicinski <kuba@kernel.org>
Cc: David Laight <David.Laight@aculab.com>,
	Oliver Neukum <oneukum@suse.com>,
	"edumazet@google.com" <edumazet@google.com>,
	"pabeni@redhat.com" <pabeni@redhat.com>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	Greg Thelen <gthelen@google.com>,
	John Sperbeck <jsperbeck@google.com>,
	"stable@vger.kernel.org" <stable@vger.kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Subject: Re: [PATCH net] net: usb: usbnet: fix name regression
Date: Tue, 3 Dec 2024 08:39:47 +0900	[thread overview]
Message-ID: <Z05FQ-Z6yv16lSnY@atmark-techno.com> (raw)
In-Reply-To: <20241202065600.4d98a3fe@kernel.org>

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.

-- 
Dominique

  reply	other threads:[~2024-12-02 23:40 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-10-17  7:18 [PATCH net] net: usb: usbnet: fix name regression Oliver Neukum
2024-10-17 16:11 ` Simon Horman
2024-10-22 11:23   ` Paolo Abeni
2024-10-22 11:30 ` patchwork-bot+netdevbpf
2024-12-02  3:50 ` Dominique MARTINET
2024-12-02  6:29   ` Greg Kroah-Hartman
2024-12-02  8:24     ` Dominique MARTINET
2024-12-02  8:17   ` David Laight
2024-12-02  8:36     ` 'Dominique MARTINET'
2024-12-02 14:56       ` Jakub Kicinski
2024-12-02 23:39         ` 'Dominique MARTINET' [this message]
2024-12-03  0:26           ` Jakub Kicinski
2024-12-03  1:18             ` 'Dominique MARTINET'
2024-12-03  2:29               ` Jakub Kicinski
2024-12-03 13:14                 ` 'Dominique MARTINET'

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Z05FQ-Z6yv16lSnY@atmark-techno.com \
    --to=dominique.martinet@atmark-techno.com \
    --cc=David.Laight@aculab.com \
    --cc=edumazet@google.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=gthelen@google.com \
    --cc=jsperbeck@google.com \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=oneukum@suse.com \
    --cc=pabeni@redhat.com \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).