Re: [PATCH] net: uclass: Save ethernet MAC address when generated

[Tom Rini <trini@konsulko.com>]

[-- Attachment #1: Type: text/plain, Size: 2762 bytes --]

On Wed, Nov 17, 2021 at 08:44:52AM +0100, Wolfgang Denk wrote:
> Dear Tom,
> 
> In message <20211116184146.GF24579@bill-the-cat> you wrote:
> > 
> > Because honestly, the more I read this, the more I think
> > https://patchwork.ozlabs.org/project/uboot/patch/20211115121152.3470910-1-m=
> > ichael@walle.cc/
> > is essentially the right direction.  There's no reason for 'net list' to
> > be using the environment here when ->enetaddr is what's being used by
> > the stack.  The use case of "I want to make my locally administered MAC
> > persist because my USB ethernet adapter lacks a MAC address" is solved
> > via the environment already.
> 
> If the MAC address is not placed in the environment, then how can a
> user query the currently used MAC address?  All documentation says
> basically: run "printenv ethaddr".

Update the documentation and say to use "net list" or read the previous
part of console log that says we're using a random mac in this case?
The more I look here the more I see we're changing part of the API with
the OS, and it's not something that should be done without consulting
the consumers too.

I should note first that I got how the "USB ethernet that lacks HW
assigned MAC" case has worked since forever (the beagleboard xM is the
first case that springs to mind and that's over 10 years old).  That's
"run gen_eth_addr, setenv ethaddr $X, saveenv" as yes, our randomly
generated MAC within U-Boot has never been super easy to have persist,
possibly intentionally.  Possibly not!  But it's an API we've had for
over 10 years now.

Next, pulling up
https://www.kernel.org/doc/Documentation/devicetree/bindings/net/ethernet-controller.yaml
there's two important things.  First, there's no way to flag "this is a
random MAC, do not use" (if after all, that's what the user wants, such
as in Michael's case).  And second, yes we probably ought to have
enforced "mac-address" being the random one we had been using, all
along.  Then we pull up
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/net/core/of_net.c#n98
and see that oh the kernel's already made some assumptions about what
might be set where and why because of seemingly arbitrary things we were
doing that may or may not have matched up with the at the time binding.

So no, in sum, I'm not convinced that the best path forward right here
and now is to put the random MAC in the environment, not correct the now
incorrect help text and not even poke the binding maintainer nor
relevant lists to see how exactly they think it would be best to handle
a locally administered MAC being used as there are both valid use cases
for "use this in the kernel" and "disregard this in the kernel".

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]