[PATCH RFC] cmd: fix net list command

[PATCH RFC] cmd: fix net list command

[Michael Walle]

Don't get the MAC address by the environment, but by the platform data
of the udevice. This will fix "net list" if the MAC address is randomly
generated and won't change behavior when linux is booted.

Signed-off-by: Michael Walle <michael@walle.cc>
---

Hi,

this is a proposal to fix the "net list" in a way that linux still can have
its own fallback mechanism of MAC addresses even if the bootloader has
CONFIG_NET_RANDOM_ETHADDR enabled.

The intention is to replace [1], where if I understood correctly, the
intention was to fix "net list".

[1] https://lore.kernel.org/u-boot/3996ba2ee4e6ac136c0802dc0df4ef9b1750157c.1635506067.git.michal.simek@xilinx.com/

 cmd/net.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/cmd/net.c b/cmd/net.c
index 651c1411f4..1df0f4eb99 100644
--- a/cmd/net.c
+++ b/cmd/net.c
@@ -486,14 +486,14 @@ U_BOOT_CMD(
 static int do_net_list(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
 {
 	const struct udevice *current = eth_get_dev();
-	unsigned char env_enetaddr[ARP_HLEN];
 	const struct udevice *dev;
+	struct eth_pdata *pdata;
 	struct uclass *uc;
 
 	uclass_id_foreach_dev(UCLASS_ETH, dev, uc) {
-		eth_env_get_enetaddr_by_index("eth", dev_seq(dev), env_enetaddr);
-		printf("eth%d : %s %pM %s\n", dev_seq(dev), dev->name, env_enetaddr,
-		       current == dev ? "active" : "");
+		pdata = dev_get_plat(dev);
+		printf("eth%d : %s %pM %s\n", dev_seq(dev), dev->name,
+		       pdata->enetaddr, current == dev ? "active" : "");
 	}
 	return CMD_RET_SUCCESS;
 }
-- 
2.30.2

Re: [PATCH RFC] cmd: fix net list command

[Marek Vasut]

On 11/15/21 1:11 PM, Michael Walle wrote:
> Don't get the MAC address by the environment, but by the platform data
> of the udevice. This will fix "net list" if the MAC address is randomly
> generated and won't change behavior when linux is booted.
> 
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---
> 
> Hi,
> 
> this is a proposal to fix the "net list" in a way that linux still can have
> its own fallback mechanism of MAC addresses even if the bootloader has
> CONFIG_NET_RANDOM_ETHADDR enabled.
> 
> The intention is to replace [1], where if I understood correctly, the
> intention was to fix "net list".
> 
> [1] https://lore.kernel.org/u-boot/3996ba2ee4e6ac136c0802dc0df4ef9b1750157c.1635506067.git.michal.simek@xilinx.com/

Does that mean U-Boot and Linux will possibly each have different MAC 
address on the same device ? That is clearly wrong and not how it is 
supposed to work, U-Boot passes its ethaddr/ethNaddr settings to Linux 
so that they would have the same MAC address on the same device.

Re: [PATCH RFC] cmd: fix net list command

[Michael Walle]

Am 2021-11-15 13:15, schrieb Marek Vasut:
> On 11/15/21 1:11 PM, Michael Walle wrote:
>> Don't get the MAC address by the environment, but by the platform data
>> of the udevice. This will fix "net list" if the MAC address is 
>> randomly
>> generated and won't change behavior when linux is booted.
>> 
>> Signed-off-by: Michael Walle <michael@walle.cc>
>> ---
>> 
>> Hi,
>> 
>> this is a proposal to fix the "net list" in a way that linux still can 
>> have
>> its own fallback mechanism of MAC addresses even if the bootloader has
>> CONFIG_NET_RANDOM_ETHADDR enabled.
>> 
>> The intention is to replace [1], where if I understood correctly, the
>> intention was to fix "net list".
>> 
>> [1] 
>> https://lore.kernel.org/u-boot/3996ba2ee4e6ac136c0802dc0df4ef9b1750157c.1635506067.git.michal.simek@xilinx.com/
> 
> Does that mean U-Boot and Linux will possibly each have different MAC
> address on the same device ? That is clearly wrong and not how it is
> supposed to work, U-Boot passes its ethaddr/ethNaddr settings to Linux
> so that they would have the same MAC address on the same device.

Yes they have different ones, but having a random mac address is  wrong
in the first place. So IMHO it is more useful to dectect that in linux,
(and there was a good point that usually a user will look at dmesg and
not some u-boot output) than having two different random MAC addresses,
one for u-boot and one for linux.

Initially that random MAC address thing was supposed to get networking
running in u-boot if there is no ethaddr at all, eg. for network 
console.
Now we are passing this made up network address to linux, too. Why? Just
because it is wrong to have two different mac addresses? Some drivers
doesn't even probe without one. And we are just masking away this error.
You get this "two different mac addresses" between reboots. Which is 
also
wrong. This actually strenghten my opinion that this "feature" should
be an u-boot only thing.

-michael

Re: [PATCH RFC] cmd: fix net list command

[Marek Vasut]

On 11/15/21 1:24 PM, Michael Walle wrote:
> Am 2021-11-15 13:15, schrieb Marek Vasut:
>> On 11/15/21 1:11 PM, Michael Walle wrote:
>>> Don't get the MAC address by the environment, but by the platform data
>>> of the udevice. This will fix "net list" if the MAC address is randomly
>>> generated and won't change behavior when linux is booted.
>>>
>>> Signed-off-by: Michael Walle <michael@walle.cc>
>>> ---
>>>
>>> Hi,
>>>
>>> this is a proposal to fix the "net list" in a way that linux still 
>>> can have
>>> its own fallback mechanism of MAC addresses even if the bootloader has
>>> CONFIG_NET_RANDOM_ETHADDR enabled.
>>>
>>> The intention is to replace [1], where if I understood correctly, the
>>> intention was to fix "net list".
>>>
>>> [1] 
>>> https://lore.kernel.org/u-boot/3996ba2ee4e6ac136c0802dc0df4ef9b1750157c.1635506067.git.michal.simek@xilinx.com/ 
>>>
>>
>> Does that mean U-Boot and Linux will possibly each have different MAC
>> address on the same device ? That is clearly wrong and not how it is
>> supposed to work, U-Boot passes its ethaddr/ethNaddr settings to Linux
>> so that they would have the same MAC address on the same device.
> 
> Yes they have different ones, but having a random mac address is  wrong
> in the first place. So IMHO it is more useful to dectect that in linux,
> (and there was a good point that usually a user will look at dmesg and
> not some u-boot output) than having two different random MAC addresses,
> one for u-boot and one for linux.

Currently U-Boot passes MAC address stored in ethaddr/ethNaddr variable 
to Linux by patching it into DT, so Linux has the same MAC address, that 
is the correct behavior.

If U-Boot generates random MAC address for some reason, then the same 
random MAC address is also passed to Linux.

The argument that user cannot be bothered to look at U-Boot console when 
debugging a problem with random MAC address being passed to Linux is 
bogus, sorry. This is exactly where the user should look and fix the 
problem.

Are you by any chance trying to work around some bug on your system, 
where U-Boot is unable to obtain the correct MAC address from OTP and 
pass that to Linux, so by ignoring the "ethaddr" variable here, you in 
fact avoid passing that U-Boot MAC address to Linux and Linux uses some 
internal fallback and picks MAC address from OTP itself ?

> Initially that random MAC address thing was supposed to get networking
> running in u-boot if there is no ethaddr at all, eg. for network console.
> Now we are passing this made up network address to linux, too. Why?

We are passing the MAC address used by U-Boot to Linux, it doesn't 
matter whether it is random or not.

> Just
> because it is wrong to have two different mac addresses? Some drivers
> doesn't even probe without one. And we are just masking away this error.
> You get this "two different mac addresses" between reboots. Which is also
> wrong. This actually strenghten my opinion that this "feature" should
> be an u-boot only thing.

Maybe it would make sense to have U-Boot obtain the correct MAC address 
on your machine first, and then pass that same correct MAC address to 
Linux ?

Re: [PATCH RFC] cmd: fix net list command

[Michael Walle]

Am 2021-11-15 14:17, schrieb Marek Vasut:
> On 11/15/21 1:24 PM, Michael Walle wrote:
>> Am 2021-11-15 13:15, schrieb Marek Vasut:
>>> On 11/15/21 1:11 PM, Michael Walle wrote:
>>>> Don't get the MAC address by the environment, but by the platform 
>>>> data
>>>> of the udevice. This will fix "net list" if the MAC address is 
>>>> randomly
>>>> generated and won't change behavior when linux is booted.
>>>> 
>>>> Signed-off-by: Michael Walle <michael@walle.cc>
>>>> ---
>>>> 
>>>> Hi,
>>>> 
>>>> this is a proposal to fix the "net list" in a way that linux still 
>>>> can have
>>>> its own fallback mechanism of MAC addresses even if the bootloader 
>>>> has
>>>> CONFIG_NET_RANDOM_ETHADDR enabled.
>>>> 
>>>> The intention is to replace [1], where if I understood correctly, 
>>>> the
>>>> intention was to fix "net list".
>>>> 
>>>> [1] 
>>>> https://lore.kernel.org/u-boot/3996ba2ee4e6ac136c0802dc0df4ef9b1750157c.1635506067.git.michal.simek@xilinx.com/
>>> 
>>> Does that mean U-Boot and Linux will possibly each have different MAC
>>> address on the same device ? That is clearly wrong and not how it is
>>> supposed to work, U-Boot passes its ethaddr/ethNaddr settings to 
>>> Linux
>>> so that they would have the same MAC address on the same device.
>> 
>> Yes they have different ones, but having a random mac address is  
>> wrong
>> in the first place. So IMHO it is more useful to dectect that in 
>> linux,
>> (and there was a good point that usually a user will look at dmesg and
>> not some u-boot output) than having two different random MAC 
>> addresses,
>> one for u-boot and one for linux.
> 
> Currently U-Boot passes MAC address stored in ethaddr/ethNaddr
> variable to Linux by patching it into DT, so Linux has the same MAC
> address, that is the correct behavior.
> 
> If U-Boot generates random MAC address for some reason, then the same
> random MAC address is also passed to Linux.

So, then why does linux have some other means to fix that? Which are
now not available anymore (or doesn't get triggered).

> The argument that user cannot be bothered to look at U-Boot console
> when debugging a problem with random MAC address being passed to Linux
> is bogus, sorry. This is exactly where the user should look and fix
> the problem.
> 
> Are you by any chance trying to work around some bug on your system,
> where U-Boot is unable to obtain the correct MAC address from OTP and
> pass that to Linux, so by ignoring the "ethaddr" variable here, you in
> fact avoid passing that U-Boot MAC address to Linux and Linux uses
> some internal fallback and picks MAC address from OTP itself ?

No I'm not, I try to preserve the initial intention of that random
ethernet address. But anyway. Take this patch or leave it. I'll turn
this option off and open code it into my board file (or just disable
it because I don't actually need it).

>> Initially that random MAC address thing was supposed to get networking
>> running in u-boot if there is no ethaddr at all, eg. for network 
>> console.
>> Now we are passing this made up network address to linux, too. Why?
> 
> We are passing the MAC address used by U-Boot to Linux, it doesn't
> matter whether it is random or not.

And again you're masking the error and possible fixes by linux itself.
Seems like this isn't an argument.

>> Just
>> because it is wrong to have two different mac addresses? Some drivers
>> doesn't even probe without one. And we are just masking away this 
>> error.
>> You get this "two different mac addresses" between reboots. Which is 
>> also
>> wrong. This actually strenghten my opinion that this "feature" should
>> be an u-boot only thing.
> 
> Maybe it would make sense to have U-Boot obtain the correct MAC
> address on your machine first, and then pass that same correct MAC
> address to Linux ?

Eh? I don't get it. That is the only reason to have this option.
Again, this has nothing to do with my board which doesn't have a
MAC address and cannot read it for now (because for now, the user
is expected to set it via the environment). It's about the correct
behavior of the said config option.

Sorry, if the original intention was to make "net list" work
correctly, then why is there now such a huge fuzz around that
CONFIG_NET_RANDOM_ETHADDR config option and why can't we just
fix the error with "net list" and leave the behavior like it
was for years.

-michael

Re: [PATCH RFC] cmd: fix net list command

[Wolfgang Denk]

Dear Michael,

In message <c0282bc070180d74e055f82b5ecab5b0@walle.cc> you wrote:
>
> And again you're masking the error and possible fixes by linux itself.
> Seems like this isn't an argument.

Respecting the explicit will of the user (i. e using what he
configured in U-Boot and passed to the kernel) is not an error.  it
is intended and documented behaviour.

> Sorry, if the original intention was to make "net list" work
> correctly, then why is there now such a huge fuzz around that
> CONFIG_NET_RANDOM_ETHADDR config option and why can't we just
> fix the error with "net list" and leave the behavior like it
> was for years.

Maybe you explain what exactly the ``error with "net list"'' is,
considering the fact that it is U-Boot which determins the
"correct" MAC address and passes it to Linux.  And if the user
configures U-Boot such that a random MAC address is used, then this
_is_ the correct MAC address, as normally (*) U-Boot is just a tool
that does what the user requests.

(*) Exceptions are situations where U-Boot's user interfaces are
locked down for example for security reasons.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
A modem is a baudy house.

Re: [PATCH RFC] cmd: fix net list command

[Michael Walle]

Hi,

Am 2021-11-15 15:31, schrieb Wolfgang Denk:
> In message <c0282bc070180d74e055f82b5ecab5b0@walle.cc> you wrote:
>> 
>> And again you're masking the error and possible fixes by linux itself.
>> Seems like this isn't an argument.
> 
> Respecting the explicit will of the user (i. e using what he
> configured in U-Boot and passed to the kernel) is not an error.  it
> is intended and documented behaviour.

What is the will of the user in this case? It is the will of the
developer to make the board more robust. That is, if there is
for whatever reason no valid ethernet address found, then there
will be one generated. That is the sole purpose of the config
option in question (or maybe I used it completely wrong). So from
a user perspective, this shouldn't even happen and I doubt he is
even aware that there will be a random one. (I saw Tom's mail and
I'm not talking about the USB adapters where this might be the
normal case.) I might come from a different perspective, but
users ususally don't look at the serial output. Instead they
look at the kernel log. And there will be not the slightest
error, because u-boot will happily fix the missing MAC address
with a random one.

>> Sorry, if the original intention was to make "net list" work
>> correctly, then why is there now such a huge fuzz around that
>> CONFIG_NET_RANDOM_ETHADDR config option and why can't we just
>> fix the error with "net list" and leave the behavior like it
>> was for years.
> 
> Maybe you explain what exactly the ``error with "net list"'' is,

Michal mentioned it here [1].

> considering the fact that it is U-Boot which determins the
> "correct" MAC address and passes it to Linux.  And if the user
> configures U-Boot such that a random MAC address is used, then this
> _is_ the correct MAC address, as normally (*) U-Boot is just a tool
> that does what the user requests.

Only that the user doesn't do it but the board developer/OEM. I.e.
there is no valid MAC address to pass to linux. It is really just
for having netconsole running (or maybe you'll need networking for
to rescue your failed EEPROM or whatever).

I think, we have to distiguish between two use cases here:
  (1) make networking in u-boot work "somehow", to have a last
      resort recovery, esp. required for netconsole where
      the user cannot set the mac address by hand.
  (2) normal use case, where drivers simply doesn't have any
      other source for the mac address and need to fall back
      to a random one.

I agree, for (2) the mac address should be passed to linux. But for
(1) it shouldn't because it will just mask the error and any fall
back mechanisms in linux.

-michael

[1] 
https://lore.kernel.org/u-boot/07b688d8-f05f-d11e-5e3e-e5454e1c9af0@xilinx.com/

Re: [PATCH RFC] cmd: fix net list command

[Michal Simek]

On 11/15/21 15:52, Michael Walle wrote:
> Hi,
> 
> Am 2021-11-15 15:31, schrieb Wolfgang Denk:
>> In message <c0282bc070180d74e055f82b5ecab5b0@walle.cc> you wrote:
>>>
>>> And again you're masking the error and possible fixes by linux itself.
>>> Seems like this isn't an argument.
>>
>> Respecting the explicit will of the user (i. e using what he
>> configured in U-Boot and passed to the kernel) is not an error.  it
>> is intended and documented behaviour.
> 
> What is the will of the user in this case? It is the will of the
> developer to make the board more robust. That is, if there is
> for whatever reason no valid ethernet address found, then there
> will be one generated. That is the sole purpose of the config
> option in question (or maybe I used it completely wrong). So from
> a user perspective, this shouldn't even happen and I doubt he is
> even aware that there will be a random one. (I saw Tom's mail and
> I'm not talking about the USB adapters where this might be the
> normal case.) I might come from a different perspective, but
> users ususally don't look at the serial output. Instead they
> look at the kernel log. And there will be not the slightest
> error, because u-boot will happily fix the missing MAC address
> with a random one.
> 
>>> Sorry, if the original intention was to make "net list" work
>>> correctly, then why is there now such a huge fuzz around that
>>> CONFIG_NET_RANDOM_ETHADDR config option and why can't we just
>>> fix the error with "net list" and leave the behavior like it
>>> was for years.
>>
>> Maybe you explain what exactly the ``error with "net list"'' is,
> 
> Michal mentioned it here [1].
> 
>> considering the fact that it is U-Boot which determins the
>> "correct" MAC address and passes it to Linux.  And if the user
>> configures U-Boot such that a random MAC address is used, then this
>> _is_ the correct MAC address, as normally (*) U-Boot is just a tool
>> that does what the user requests.
> 
> Only that the user doesn't do it but the board developer/OEM. I.e.
> there is no valid MAC address to pass to linux. It is really just
> for having netconsole running (or maybe you'll need networking for
> to rescue your failed EEPROM or whatever).
> 
> I think, we have to distiguish between two use cases here:
>   (1) make networking in u-boot work "somehow", to have a last
>       resort recovery, esp. required for netconsole where
>       the user cannot set the mac address by hand.
>   (2) normal use case, where drivers simply doesn't have any
>       other source for the mac address and need to fall back
>       to a random one.
> 
> I agree, for (2) the mac address should be passed to linux. But for
> (1) it shouldn't because it will just mask the error and any fall
> back mechanisms in linux.
 >
> -michael
> 
> [1] 
> https://lore.kernel.org/u-boot/07b688d8-f05f-d11e-5e3e-e5454e1c9af0@xilinx.com/ 


As we discussed in previous thread. I think there shouldn't be a problem 
when u-boot passes random mac address (in whatever way) but if Linux 
driver know how to get correct one it should be tried first. If that 
pass you will get new one. If not you will use the one passed from u-boot.
And it is up to board designer to decide when it is the right time. I 
can imagine that it can be the part of kernel driver or can be purely 
done in user space.

Thanks,
Michal

Re: [PATCH RFC] cmd: fix net list command

[Tom Rini]

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

On Mon, Nov 15, 2021 at 03:57:51PM +0100, Michal Simek wrote:
> 
> 
> On 11/15/21 15:52, Michael Walle wrote:
> > Hi,
> > 
> > Am 2021-11-15 15:31, schrieb Wolfgang Denk:
> > > In message <c0282bc070180d74e055f82b5ecab5b0@walle.cc> you wrote:
> > > > 
> > > > And again you're masking the error and possible fixes by linux itself.
> > > > Seems like this isn't an argument.
> > > 
> > > Respecting the explicit will of the user (i. e using what he
> > > configured in U-Boot and passed to the kernel) is not an error.  it
> > > is intended and documented behaviour.
> > 
> > What is the will of the user in this case? It is the will of the
> > developer to make the board more robust. That is, if there is
> > for whatever reason no valid ethernet address found, then there
> > will be one generated. That is the sole purpose of the config
> > option in question (or maybe I used it completely wrong). So from
> > a user perspective, this shouldn't even happen and I doubt he is
> > even aware that there will be a random one. (I saw Tom's mail and
> > I'm not talking about the USB adapters where this might be the
> > normal case.) I might come from a different perspective, but
> > users ususally don't look at the serial output. Instead they
> > look at the kernel log. And there will be not the slightest
> > error, because u-boot will happily fix the missing MAC address
> > with a random one.
> > 
> > > > Sorry, if the original intention was to make "net list" work
> > > > correctly, then why is there now such a huge fuzz around that
> > > > CONFIG_NET_RANDOM_ETHADDR config option and why can't we just
> > > > fix the error with "net list" and leave the behavior like it
> > > > was for years.
> > > 
> > > Maybe you explain what exactly the ``error with "net list"'' is,
> > 
> > Michal mentioned it here [1].
> > 
> > > considering the fact that it is U-Boot which determins the
> > > "correct" MAC address and passes it to Linux.  And if the user
> > > configures U-Boot such that a random MAC address is used, then this
> > > _is_ the correct MAC address, as normally (*) U-Boot is just a tool
> > > that does what the user requests.
> > 
> > Only that the user doesn't do it but the board developer/OEM. I.e.
> > there is no valid MAC address to pass to linux. It is really just
> > for having netconsole running (or maybe you'll need networking for
> > to rescue your failed EEPROM or whatever).
> > 
> > I think, we have to distiguish between two use cases here:
> >   (1) make networking in u-boot work "somehow", to have a last
> >       resort recovery, esp. required for netconsole where
> >       the user cannot set the mac address by hand.
> >   (2) normal use case, where drivers simply doesn't have any
> >       other source for the mac address and need to fall back
> >       to a random one.
> > 
> > I agree, for (2) the mac address should be passed to linux. But for
> > (1) it shouldn't because it will just mask the error and any fall
> > back mechanisms in linux.
> >
> > -michael
> > 
> > [1] https://lore.kernel.org/u-boot/07b688d8-f05f-d11e-5e3e-e5454e1c9af0@xilinx.com/
> 
> As we discussed in previous thread. I think there shouldn't be a problem
> when u-boot passes random mac address (in whatever way) but if Linux driver
> know how to get correct one it should be tried first. If that pass you will
> get new one. If not you will use the one passed from u-boot.

I think part of the problem here is that Linux can be inconsistent with
how it behaves here?  I remember there being some headaches early on
with the TI CPSW driver in Linux being allowed to, or not allowed to,
figure out where the MAC was.  But that was a long time ago now and
things are perhaps different now.

> And it is up to board designer to decide when it is the right time. I can
> imagine that it can be the part of kernel driver or can be purely done in
> user space.

It might also be the case that in U-Boot some systems that are using
NET_RANDOM_ETHADDR that really probably shouldn't?  A quick grep shows
253 platforms enabling it today, which feels high for the number of
boards in the "we'll never have a valid MAC, need a random one"
category.  Thinking out loud, perhaps we need an option for "no possible
valid MAC on board" and a different option for "random MAC for fall-back
only" ?

-- 
Tom

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

Re: [PATCH RFC] cmd: fix net list command

[Wolfgang Denk]

Dear Tom,

In message <20211115150753.GL24579@bill-the-cat> you wrote:
> 
> It might also be the case that in U-Boot some systems that are using
> NET_RANDOM_ETHADDR that really probably shouldn't?  A quick grep shows
> 253 platforms enabling it today, which feels high for the number of
> boards in the "we'll never have a valid MAC, need a random one"
> category.  Thinking out loud, perhaps we need an option for "no possible
> valid MAC on board" and a different option for "random MAC for fall-back
> only" ?

This is indeed obvious abuse of a feature which has been designed
for a few specific use cases.  It shpould be made clear that this is
nothing to select without thinking.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
If you fail to plan, plan to fail.

Re: [PATCH RFC] cmd: fix net list command

[Tom Rini]

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

On Mon, Nov 15, 2021 at 04:22:39PM +0100, Wolfgang Denk wrote:
> Dear Tom,
> 
> In message <20211115150753.GL24579@bill-the-cat> you wrote:
> > 
> > It might also be the case that in U-Boot some systems that are using
> > NET_RANDOM_ETHADDR that really probably shouldn't?  A quick grep shows
> > 253 platforms enabling it today, which feels high for the number of
> > boards in the "we'll never have a valid MAC, need a random one"
> > category.  Thinking out loud, perhaps we need an option for "no possible
> > valid MAC on board" and a different option for "random MAC for fall-back
> > only" ?
> 
> This is indeed obvious abuse of a feature which has been designed
> for a few specific use cases.  It shpould be made clear that this is
> nothing to select without thinking.

Well, it's not 100% obvious.  In cost-cutting terms I think there's a
fairly large segment of cheap SBCs that lack a persistent MAC due to
lack of storage space for it.  And there is also usb gadget ethernet.
But I really do want to know what's going on with a good number of SoCs.
I'm going to start a thread for some specific boards I do have and can
poke to start seeing what's up.

-- 
Tom

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

Re: [PATCH RFC] cmd: fix net list command

[Wolfgang Denk]

Dear Michal,

In message <2a51974b-41cf-56e4-c9c9-e6b699f27f5c@xilinx.com> you wrote:
> 
> As we discussed in previous thread. I think there shouldn't be a problem 
> when u-boot passes random mac address (in whatever way) but if Linux 
> driver know how to get correct one it should be tried first.

No, I strongly disagree here.  If I set a MAC address in U-Boot and
pass it to the kernel, I want that the kernel uses this address.
there may be very good reasons to chose some other address - even if
the "correct" one is known in U-Boot (say, by accident there are
several boards using the same MAC address).

> And it is up to board designer to decide when it is the right time. I 
> can imagine that it can be the part of kernel driver or can be purely 
> done in user space.

Again, no.  It must not be done automatically in the kernel driver,
as this would restrict the user's capabilities to decide what
happens.  If the user want's to change the address in Linux, he can
do this in user space - but again, it is then a (hopufully concious)
decision of the user, and not some automagic code in a driver which
ruins all my attempts to use the address I want to have used.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
Perfection is reached, not when there is no longer anything  to  add,
but when there is no longer anything to take away.
                                           - Antoine de Saint-Exupery

Re: [PATCH RFC] cmd: fix net list command

[Michal Simek]

On 11/15/21 16:20, Wolfgang Denk wrote:
> Dear Michal,
> 
> In message <2a51974b-41cf-56e4-c9c9-e6b699f27f5c@xilinx.com> you wrote:
>>
>> As we discussed in previous thread. I think there shouldn't be a problem
>> when u-boot passes random mac address (in whatever way) but if Linux
>> driver know how to get correct one it should be tried first.
> 
> No, I strongly disagree here.  If I set a MAC address in U-Boot and
> pass it to the kernel, I want that the kernel uses this address.
> there may be very good reasons to chose some other address - even if
> the "correct" one is known in U-Boot (say, by accident there are
> several boards using the same MAC address).

MAC addresses can be provided in OTP location of qspi. Or on eeprom 
which is accessible only when PL is loaded. Also information on 1wire, 
or PL gpio pins, etc.
It means mac address is on the board but at u-boot stage doesn't need to 
be accessible (or it is not useful to waste time/money on writing driver 
for it).

In perfect world u-boot will just get it and use. And if manufacturer 
doesn't do it properly but want to enable ethernet in u-boot random 
address could be a solution for them.
And then in Linux where drivers are available you will use the right one.

But back to this patch. I think it is good that net list will show all 
mac addresses even they are not saved in environment.

And I will let you guys to discuss if makes sense to share random 
address up which is what the whole discussion is about. And I have 
created a patch which is exactly aligned what it is described above. If 
u-boot starts to use random mac I want to see it in Linux too.

Thanks,
Michal

Re: [PATCH RFC] cmd: fix net list command

[Wolfgang Denk]

Dear Michal,

In message <d5ebea06-a896-a4ef-97ae-e9496605ff55@xilinx.com> you wrote:
> 
> In perfect world u-boot will just get it and use. And if manufacturer 
> doesn't do it properly but want to enable ethernet in u-boot random 
> address could be a solution for them.
> And then in Linux where drivers are available you will use the right one.

You miss the point.  The whole argument is about who is control of
the system - can I as user do what I want, or are there some stupid
limitations in the system that prevent me from doing it.

But if the user of the system decides to use a specific MAC address
(for whatever reason ever), the software should just do what he
requests, and use this MAC address on no other one.  It is up to the
user to decide which is "the right one".

> But back to this patch. I think it is good that net list will show all 
> mac addresses even they are not saved in environment.

Agreed.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
A girl with a future avoids the man with a past.
                                   -- Evan Esar, "The Humor of Humor"

Re: [PATCH RFC] cmd: fix net list command

[Tom Rini]

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

On Mon, Nov 15, 2021 at 03:52:51PM +0100, Michael Walle wrote:
> Hi,
> 
> Am 2021-11-15 15:31, schrieb Wolfgang Denk:
> > In message <c0282bc070180d74e055f82b5ecab5b0@walle.cc> you wrote:
> > > 
> > > And again you're masking the error and possible fixes by linux itself.
> > > Seems like this isn't an argument.
> > 
> > Respecting the explicit will of the user (i. e using what he
> > configured in U-Boot and passed to the kernel) is not an error.  it
> > is intended and documented behaviour.
> 
> What is the will of the user in this case? It is the will of the
> developer to make the board more robust. That is, if there is
> for whatever reason no valid ethernet address found, then there
> will be one generated. That is the sole purpose of the config
> option in question (or maybe I used it completely wrong). So from
> a user perspective, this shouldn't even happen and I doubt he is
> even aware that there will be a random one. (I saw Tom's mail and
> I'm not talking about the USB adapters where this might be the
> normal case.) I might come from a different perspective, but
> users ususally don't look at the serial output. Instead they
> look at the kernel log. And there will be not the slightest
> error, because u-boot will happily fix the missing MAC address
> with a random one.

So one thing I don't know is what's the situation on (I assume) the
kontron boards?  I see both NXP reference platforms for layerscape and
imx8 enabling NET_RANDOM_ETHADDR but I don't know what the expectation
is for on-device MAC addresses, and I recall other threads talking
about how complex the layerscape physical layout can get.  Thanks!

-- 
Tom

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

Re: [PATCH RFC] cmd: fix net list command

[Michael Walle]

Am 2021-11-15 16:12, schrieb Tom Rini:
> On Mon, Nov 15, 2021 at 03:52:51PM +0100, Michael Walle wrote:
>> Hi,
>> 
>> Am 2021-11-15 15:31, schrieb Wolfgang Denk:
>> > In message <c0282bc070180d74e055f82b5ecab5b0@walle.cc> you wrote:
>> > >
>> > > And again you're masking the error and possible fixes by linux itself.
>> > > Seems like this isn't an argument.
>> >
>> > Respecting the explicit will of the user (i. e using what he
>> > configured in U-Boot and passed to the kernel) is not an error.  it
>> > is intended and documented behaviour.
>> 
>> What is the will of the user in this case? It is the will of the
>> developer to make the board more robust. That is, if there is
>> for whatever reason no valid ethernet address found, then there
>> will be one generated. That is the sole purpose of the config
>> option in question (or maybe I used it completely wrong). So from
>> a user perspective, this shouldn't even happen and I doubt he is
>> even aware that there will be a random one. (I saw Tom's mail and
>> I'm not talking about the USB adapters where this might be the
>> normal case.) I might come from a different perspective, but
>> users ususally don't look at the serial output. Instead they
>> look at the kernel log. And there will be not the slightest
>> error, because u-boot will happily fix the missing MAC address
>> with a random one.
> 
> So one thing I don't know is what's the situation on (I assume) the
> kontron boards?  I see both NXP reference platforms for layerscape and
> imx8 enabling NET_RANDOM_ETHADDR but I don't know what the expectation
> is for on-device MAC addresses, and I recall other threads talking
> about how complex the layerscape physical layout can get.  Thanks!

Kontron boards are fine, well at least if I disable the random mac
address config option, usually there is a serial console.

board/buffalo/lsxl.c needs it definetly (and was actually the first
user).

-michael

Re: [PATCH RFC] cmd: fix net list command

[Tom Rini]

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

On Mon, Nov 15, 2021 at 04:15:31PM +0100, Michael Walle wrote:
> Am 2021-11-15 16:12, schrieb Tom Rini:
> > On Mon, Nov 15, 2021 at 03:52:51PM +0100, Michael Walle wrote:
> > > Hi,
> > > 
> > > Am 2021-11-15 15:31, schrieb Wolfgang Denk:
> > > > In message <c0282bc070180d74e055f82b5ecab5b0@walle.cc> you wrote:
> > > > >
> > > > > And again you're masking the error and possible fixes by linux itself.
> > > > > Seems like this isn't an argument.
> > > >
> > > > Respecting the explicit will of the user (i. e using what he
> > > > configured in U-Boot and passed to the kernel) is not an error.  it
> > > > is intended and documented behaviour.
> > > 
> > > What is the will of the user in this case? It is the will of the
> > > developer to make the board more robust. That is, if there is
> > > for whatever reason no valid ethernet address found, then there
> > > will be one generated. That is the sole purpose of the config
> > > option in question (or maybe I used it completely wrong). So from
> > > a user perspective, this shouldn't even happen and I doubt he is
> > > even aware that there will be a random one. (I saw Tom's mail and
> > > I'm not talking about the USB adapters where this might be the
> > > normal case.) I might come from a different perspective, but
> > > users ususally don't look at the serial output. Instead they
> > > look at the kernel log. And there will be not the slightest
> > > error, because u-boot will happily fix the missing MAC address
> > > with a random one.
> > 
> > So one thing I don't know is what's the situation on (I assume) the
> > kontron boards?  I see both NXP reference platforms for layerscape and
> > imx8 enabling NET_RANDOM_ETHADDR but I don't know what the expectation
> > is for on-device MAC addresses, and I recall other threads talking
> > about how complex the layerscape physical layout can get.  Thanks!
> 
> Kontron boards are fine, well at least if I disable the random mac
> address config option, usually there is a serial console.

Well, what I mean is, where are the real MAC addresses?  What is the
rationale for setting NET_RANDOM_ETHADDR?

> board/buffalo/lsxl.c needs it definetly (and was actually the first
> user).

OK, there's a big handy comment there to read, thanks.

> 
> -michael

-- 
Tom

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

Re: [PATCH RFC] cmd: fix net list command

[Wolfgang Denk]

Dear Tom,

In message <20211115151956.GN24579@bill-the-cat> you wrote:
> 
> Well, what I mean is, where are the real MAC addresses?  What is the
> rationale for setting NET_RANDOM_ETHADDR?

One practical use case is board provisioning in the factory, which
includes setting up valid produt data like serial number, MAC
address etc. Yes, this can be done over serial consone as well, but
1) this is slower than network and 2) many commecial products don't
have the connector etc. populated.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
How many Unix hacks does it take to change a light bulb?  Let's  see,
   can you use a shell script for that or does it need a C program?

Re: [PATCH RFC] cmd: fix net list command

[Tom Rini]

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

On Mon, Nov 15, 2021 at 04:25:55PM +0100, Wolfgang Denk wrote:
> Dear Tom,
> 
> In message <20211115151956.GN24579@bill-the-cat> you wrote:
> > 
> > Well, what I mean is, where are the real MAC addresses?  What is the
> > rationale for setting NET_RANDOM_ETHADDR?
> 
> One practical use case is board provisioning in the factory, which
> includes setting up valid produt data like serial number, MAC
> address etc. Yes, this can be done over serial consone as well, but
> 1) this is slower than network and 2) many commecial products don't
> have the connector etc. populated.

Is that the case for the layerscape or imx8 families?  I know on the TI
side these values are derived from chip-specific fuses that are blown at
chip production so we don't need an initial random one.  I'd have to
pull up some NXP manuals to see what's the case there, but I think some
other people on the thread know the answers here already.

-- 
Tom

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

Re: [PATCH RFC] cmd: fix net list command

[Wolfgang Denk]

Dear Tom,

In message <20211115154516.GR24579@bill-the-cat> you wrote:
> 
> > One practical use case is board provisioning in the factory, which
> > includes setting up valid produt data like serial number, MAC
> > address etc. Yes, this can be done over serial consone as well, but
> > 1) this is slower than network and 2) many commecial products don't
> > have the connector etc. populated.
> 
> Is that the case for the layerscape or imx8 families?  I know on the TI
> side these values are derived from chip-specific fuses that are blown at
> chip production so we don't need an initial random one.  I'd have to
> pull up some NXP manuals to see what's the case there, but I think some
> other people on the thread know the answers here already.

This is actually independent of the hardware, I think.

It is more or less a management decision how board bringup / inital
commissioning is organized.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
In a business, marketroids, salespukes, and  lawyers  have  different
goals from those who actually do work and produce something. Usually,
is  is the former who triumph over the latter, due to the simple rule
that those who print the money make the rules.
         -- Tom Christiansen in <5jdcls$b04$2@csnews.cs.colorado.edu>

Re: [PATCH RFC] cmd: fix net list command

[Wolfgang Denk]

Dear Michael,

In message <bf1393770cc04a3fb2b271d25fa6a46c@walle.cc> you wrote:
>
> What is the will of the user in this case?

In which case?  When the user does not bother to set a specific MAC
address and let the system gernerate a random one?  Well it is his
(maybe concious, maybe not) decision...

> It is the will of the
> developer to make the board more robust.

Maybe, maybe not.  Often this is just a convenient method for the
board manufacturer to provision his boards with valid data. If it
makes sense to ship boards in such a state to the end user is
another question.

> > Maybe you explain what exactly the ``error with "net list"'' is,
>
> Michal mentioned it here [1].

Yes, and he also provided a valuable and correct comment:

| And if you don't want to use this feature just don't enable it via
| CONFIG_NET_RANDOM_ETHADDR.

This say all, no patches needed.

> > considering the fact that it is U-Boot which determins the
> > "correct" MAC address and passes it to Linux.  And if the user
> > configures U-Boot such that a random MAC address is used, then this
> > _is_ the correct MAC address, as normally (*) U-Boot is just a tool
> > that does what the user requests.
>
> Only that the user doesn't do it but the board developer/OEM. I.e.
> there is no valid MAC address to pass to linux.

Why do you continue to claim that the MAC address used in U-Boot is
not valid?  Of course it is.


This is similar to the situation where appliances sip with default
passwords like ADMIN/ADMIN.  The user can ignore the documentation
which has bright red warning notes in CAPITAL LETTRRS that he *must*
configure secure passwords...

> It is really just
> for having netconsole running (or maybe you'll need networking for
> to rescue your failed EEPROM or whatever).

This is one of many possible use cases. Board provisioning is
another one.

> I think, we have to distiguish between two use cases here:
>   (1) make networking in u-boot work "somehow", to have a last
>       resort recovery, esp. required for netconsole where
>       the user cannot set the mac address by hand.
>   (2) normal use case, where drivers simply doesn't have any
>       other source for the mac address and need to fall back
>       to a random one.

There are many more use cases.  And it may be intentional to use a
random MAC address.

There is this old rule:

"UNIX was not designed to stop you from doing stupid things,  because
that would also stop you from doing clever things."       - Doug Gwyn


If you don't like it, then disable the feature. It is nonstandard
anyway, i. e. only intended for special cases / qualified users.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
The day-to-day travails of the IBM programmer are so amusing to  most
of us who are fortunate enough never to have been one - like watching
Charlie Chaplin trying to cook a shoe.

Re: [PATCH RFC] cmd: fix net list command

[Michael Walle]

Hi,

Am 2021-11-15 16:15, schrieb Wolfgang Denk:

> | And if you don't want to use this feature just don't enable it via
> | CONFIG_NET_RANDOM_ETHADDR.
> 
> This say all, no patches needed.

Yes, I'm giving up. I'll just deactivate that option then and open code
it in my board.

-michael

Re: [PATCH RFC] cmd: fix net list command

[Marek Vasut]

On 11/15/21 4:21 PM, Michael Walle wrote:
> Hi,
> 
> Am 2021-11-15 16:15, schrieb Wolfgang Denk:
> 
>> | And if you don't want to use this feature just don't enable it via
>> | CONFIG_NET_RANDOM_ETHADDR.
>>
>> This say all, no patches needed.
> 
> Yes, I'm giving up. I'll just deactivate that option then and open code
> it in my board.

Why don't you simply implement the SPI NOR OTP readout in U-Boot and get 
valid MAC address from there, which is the proper fix here ?

Re: [PATCH RFC] cmd: fix net list command

[Ramon Fried]

On Mon, Nov 15, 2021 at 6:45 PM Marek Vasut <marex@denx.de> wrote:
>
> On 11/15/21 4:21 PM, Michael Walle wrote:
> > Hi,
> >
> > Am 2021-11-15 16:15, schrieb Wolfgang Denk:
> >
> >> | And if you don't want to use this feature just don't enable it via
> >> | CONFIG_NET_RANDOM_ETHADDR.
> >>
> >> This say all, no patches needed.
> >
> > Yes, I'm giving up. I'll just deactivate that option then and open code
> > it in my board.
>
> Why don't you simply implement the SPI NOR OTP readout in U-Boot and get
> valid MAC address from there, which is the proper fix here ?

Joining late to the party, which started earlier in IRC.
net list has a problem, it won't display random MAC addresses if those
were generated with the CONFIG_NET_RANDOM_ETHADDR.
It appears that the reason for that, is that the random mac address
didn't reach the environment variable, where it should have been.
That's a bug, and it was fixed by Michal. and I approved that patch.
already waiting to be pulled by Tom.

It appears that Michael has some board and a scenario that *this* bug
was working for his best interest, where he can have two distinct MAC
addresses, one in U-boot and one in Linux.
Mark and I already provided work around for Michael to delete the
environment variable before he boots. or implement the necessary OTP
driver in U-Boot.
From my end, I think we can conclude the discussion.
Thanks,
Ramon.

Re: [PATCH RFC] cmd: fix net list command

[Wolfgang Denk]

Dear Ramon,

In message <CAGi-RUJqhd=dAo7cJqtTmv3G=VfDGFuyn0=YgHZGVGhTkH0t-g@mail.gmail.com> you wrote:
>
> It appears that Michael has some board and a scenario that *this* bug
> was working for his best interest, where he can have two distinct MAC
> addresses, one in U-boot and one in Linux.

If the Linux environment is supposed to be a specific MAC address
(which may be different from the one passed by U-Boot), then there
are standard tools available to set the address.  I don't see a
problem here.

> Mark and I already provided work around for Michael to delete the
> environment variable before he boots. or implement the necessary OTP
> driver in U-Boot.

As Marek pointed out, in this specific case it would be best to
provide such a driver.

> From my end, I think we can conclude the discussion.

ACK.

Thanks.

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
My challenge to the goto-less programmer  is  to  recode  tcp_input()
without any gotos ... without any loss of efficiency (there has to be
a catch).                                             - W. R. Stevens

Re: [PATCH RFC] cmd: fix net list command

[Tom Rini]

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

On Tue, Nov 16, 2021 at 02:33:01PM +0100, Wolfgang Denk wrote:
> Dear Ramon,
> 
> In message <CAGi-RUJqhd=dAo7cJqtTmv3G=VfDGFuyn0=YgHZGVGhTkH0t-g@mail.gmail.com> you wrote:
> >
> > It appears that Michael has some board and a scenario that *this* bug
> > was working for his best interest, where he can have two distinct MAC
> > addresses, one in U-boot and one in Linux.
> 
> If the Linux environment is supposed to be a specific MAC address
> (which may be different from the one passed by U-Boot), then there
> are standard tools available to set the address.  I don't see a
> problem here.

One thing I'm not totally sure on yet is, looking at the README I see:
"If Ethernet drivers implement the 'write_hwaddr' function, valid MAC
addresses will be programmed into hardware as part of the initialization
process.  This may be skipped by setting the appropriate 'ethmacskip'
environment variable.  The naming convention is as follows: "ethmacskip"
(=>eth0), "eth1macskip" (=>eth1) etc."

As I'm sure that predates device trees being used to the extent they are
now, should 'ethmacskip' be involved in the "fixup the device tree"
logic, and appropriate rST / Kconfig help text updated?  My first
reaction is that in spirit, this is how to solve Michael's use case and
README / doc/README.enetaddr do not specify when/why the "also fixup the
device tree if it exists" is done.

-- 
Tom

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

Re: [PATCH RFC] cmd: fix net list command

[Wolfgang Denk]

Dear Tom,

In message <20211116141030.GC24579@bill-the-cat> you wrote:
> 
> One thing I'm not totally sure on yet is, looking at the README I see:
> "If Ethernet drivers implement the 'write_hwaddr' function, valid MAC
> addresses will be programmed into hardware as part of the initialization
> process.  This may be skipped by setting the appropriate 'ethmacskip'
> environment variable.  The naming convention is as follows: "ethmacskip"
> (=>eth0), "eth1macskip" (=>eth1) etc."

I have to admit that until now I was not even aware of these
variables...

> As I'm sure that predates device trees being used to the extent they are
> now, should 'ethmacskip' be involved in the "fixup the device tree"
> logic, and appropriate rST / Kconfig help text updated?  My first
> reaction is that in spirit, this is how to solve Michael's use case and
> README / doc/README.enetaddr do not specify when/why the "also fixup the
> device tree if it exists" is done.

Apparently this logic was added more than 10 years ago in commit
ecee9324d "Program net device MAC addresses after initializing".

I think it does not change anything to the logic that Linux uses the
MAC address passed by U-Boot. Whether U-Boot also writes some MAC
address to the device's persistent storage is an independent act.
If there already was an addressed stored there befoire it would not
have been read by Linux, so the same should happen here.

[Here it is even less problematic as U-Boot has the very same MAC
address in it's environment.]

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
Everyting looks interesting until you do it. Then you find it's  just
another job.                     - Terry Pratchett, _Moving Pictures_

Re: [PATCH RFC] cmd: fix net list command

[Tom Rini]

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

On Mon, Nov 15, 2021 at 01:24:34PM +0100, Michael Walle wrote:
> Am 2021-11-15 13:15, schrieb Marek Vasut:
> > On 11/15/21 1:11 PM, Michael Walle wrote:
> > > Don't get the MAC address by the environment, but by the platform data
> > > of the udevice. This will fix "net list" if the MAC address is
> > > randomly
> > > generated and won't change behavior when linux is booted.
> > > 
> > > Signed-off-by: Michael Walle <michael@walle.cc>
> > > ---
> > > 
> > > Hi,
> > > 
> > > this is a proposal to fix the "net list" in a way that linux still
> > > can have
> > > its own fallback mechanism of MAC addresses even if the bootloader has
> > > CONFIG_NET_RANDOM_ETHADDR enabled.
> > > 
> > > The intention is to replace [1], where if I understood correctly, the
> > > intention was to fix "net list".
> > > 
> > > [1] https://lore.kernel.org/u-boot/3996ba2ee4e6ac136c0802dc0df4ef9b1750157c.1635506067.git.michal.simek@xilinx.com/
> > 
> > Does that mean U-Boot and Linux will possibly each have different MAC
> > address on the same device ? That is clearly wrong and not how it is
> > supposed to work, U-Boot passes its ethaddr/ethNaddr settings to Linux
> > so that they would have the same MAC address on the same device.
> 
> Yes they have different ones, but having a random mac address is  wrong
> in the first place. So IMHO it is more useful to dectect that in linux,
> (and there was a good point that usually a user will look at dmesg and
> not some u-boot output) than having two different random MAC addresses,
> one for u-boot and one for linux.
> 
> Initially that random MAC address thing was supposed to get networking
> running in u-boot if there is no ethaddr at all, eg. for network console.
> Now we are passing this made up network address to linux, too. Why? Just
> because it is wrong to have two different mac addresses? Some drivers
> doesn't even probe without one. And we are just masking away this error.
> You get this "two different mac addresses" between reboots. Which is also
> wrong. This actually strenghten my opinion that this "feature" should
> be an u-boot only thing.

Do you have some link explaining this use case more clearly, ie why
U-Boot is using a random MAC being wrong?  We have a number of long
standing use cases today where U-Boot generates and uses the random MAC
and passes it to Linux and Linux would generate another random MAC, so
we avoid that mismatch by making sure Linux does get the random MAC we
generate.  The BeagleBoard xM is the first one that pops to mind for me,
but I believe it's typical on some classes of USB ethernet adapters.

-- 
Tom

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

Re: [PATCH RFC] cmd: fix net list command

[Michael Walle]

Am 2021-11-15 14:57, schrieb Tom Rini:
> On Mon, Nov 15, 2021 at 01:24:34PM +0100, Michael Walle wrote:
>> Am 2021-11-15 13:15, schrieb Marek Vasut:
>> > On 11/15/21 1:11 PM, Michael Walle wrote:
>> > > Don't get the MAC address by the environment, but by the platform data
>> > > of the udevice. This will fix "net list" if the MAC address is
>> > > randomly
>> > > generated and won't change behavior when linux is booted.
>> > >
>> > > Signed-off-by: Michael Walle <michael@walle.cc>
>> > > ---
>> > >
>> > > Hi,
>> > >
>> > > this is a proposal to fix the "net list" in a way that linux still
>> > > can have
>> > > its own fallback mechanism of MAC addresses even if the bootloader has
>> > > CONFIG_NET_RANDOM_ETHADDR enabled.
>> > >
>> > > The intention is to replace [1], where if I understood correctly, the
>> > > intention was to fix "net list".
>> > >
>> > > [1] https://lore.kernel.org/u-boot/3996ba2ee4e6ac136c0802dc0df4ef9b1750157c.1635506067.git.michal.simek@xilinx.com/
>> >
>> > Does that mean U-Boot and Linux will possibly each have different MAC
>> > address on the same device ? That is clearly wrong and not how it is
>> > supposed to work, U-Boot passes its ethaddr/ethNaddr settings to Linux
>> > so that they would have the same MAC address on the same device.
>> 
>> Yes they have different ones, but having a random mac address is  
>> wrong
>> in the first place. So IMHO it is more useful to dectect that in 
>> linux,
>> (and there was a good point that usually a user will look at dmesg and
>> not some u-boot output) than having two different random MAC 
>> addresses,
>> one for u-boot and one for linux.
>> 
>> Initially that random MAC address thing was supposed to get networking
>> running in u-boot if there is no ethaddr at all, eg. for network 
>> console.
>> Now we are passing this made up network address to linux, too. Why? 
>> Just
>> because it is wrong to have two different mac addresses? Some drivers
>> doesn't even probe without one. And we are just masking away this 
>> error.
>> You get this "two different mac addresses" between reboots. Which is 
>> also
>> wrong. This actually strenghten my opinion that this "feature" should
>> be an u-boot only thing.
> 
> Do you have some link explaining this use case more clearly, ie why
> U-Boot is using a random MAC being wrong?  We have a number of long
> standing use cases today where U-Boot generates and uses the random MAC
> and passes it to Linux and Linux would generate another random MAC, so
> we avoid that mismatch by making sure Linux does get the random MAC we
> generate.  The BeagleBoard xM is the first one that pops to mind for 
> me,
> but I believe it's typical on some classes of USB ethernet adapters.

Please see my previous response to Wolfgang.

-michael

Re: [PATCH RFC] cmd: fix net list command

[Michal Simek]

On 11/15/21 13:11, Michael Walle wrote:
> Don't get the MAC address by the environment, but by the platform data
> of the udevice. This will fix "net list" if the MAC address is randomly
> generated and won't change behavior when linux is booted.
> 
> Signed-off-by: Michael Walle <michael@walle.cc>

thanks for looking at this one. I was OOF last week that's why didn't 
react on it.

I think patch itself is good but only the last sentence should be 
updated to describe current state which is.
When random MAC address is generated it is not saved now to environment 
variable that's why net list command can't display it. ...

And if make sense to save random mac to variable should be discussed in 
that second thread.

Thanks,
Michal