Re: [PATCH net] net: usb: usbnet: fix name regression

['Dominique MARTINET' <dominique.martinet@atmark-techno.com>]

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,
-- 
Dominique