Dear Tom,
In message 20211117161545.GA24579@bill-the-cat you wrote:
Yes, you're changing behavior by requiring this change, and fwiw I suggested a slightly different fix-up here of deleting the device tree properties if it's a random MAC, in Michael's patch just disabling the feature on the platforms he cares about.
Of course fixing a bug will change behaviour; that's the intention of the fix.
Technically there is no difference between the user setting "ethaddr" manually to a locally administered MAC address or doing this automatically in some code or script. In all cases setting "ethaddr" means: hey, this is my MAC address, please use it.
I've asked Neil to chime in on the amlogic cases after talking on IRC, but the short answer was for prior to fuse/serial# reading support for a non-random MAC. Possibly other SoCs in a similar situation.
It is perfectly OK for U-Boot to start with a random MAC address, use this for a while, and change it so something else later. This is what may happen at production: say the MAC address is stored in some EEPROM or fuses, which are initially empty, so U-Boot will use a random MAC address, download it's board specific date (serial#, MAC address, ...) over network, programm it into the respective storage devices, and switch to using the new "official" MAC address.
I don't mean this in a super snarky way, but I'm more concerned about the implications of changing our documented albeit slightly inconsistent behavior than I am about some of the myriad of technically feasible environment variable names we've also in theory supported. For about half the time we've supported device trees, the solution to "I need to use a random MAC" was "run tools/gen_eth_addr, setenv, saveenv".
This is indeed what seems a straightforward approach to me. The problem here is that this needs to be done manually. CONFIG_NET_RANDOM_ETHADDR is the attempt to do the same automatically, except that it falls short of setting the "ethaddr" environment variable. I consider this a bug.
For the second half of the time, it was the same, but with a side of "or note the random MAC from console logs and use that".
Yes, and it should be clear that this is not a reasonable approach. It thwarts the original intent of the NET_RANDOM_ETHADDR patch to do thing in an automated, scriptable way. I see this actually as a manifestation of the bug that ethaddr does not get set. At this point the problem was recognized, but instead of properly fixing it, a poor workaround was documented.
We're about to introduce the 3rd variant. I'd feel a whole lot better about taking in a v2 of this patch that correct the help (and maybe updates doc/README.enetaddr!) if someone could report back on what's going on with the layerscape, imx* and socfpga platforms. There's in fact a number of platforms enabling it that I'm pretty sure DENX has or had the hardware on, so can we get some spot checking done there?
I will check and provide an update later, but from the best of my knowledge none of the boards we ported actually need or use this feature. This is just a copy&paste mess.
If we can drop from 250 platforms to 50 platforms with some level of confidence we understand why are setting NET_RANDOM_ETHADDR, I'd be a lot less worried we're about to introduce another change that we won't found out messed up a bunch of people on until some new work-around is accepted in to Linux or something.
Well, this work-around in Linux is because there have been incnsistent ways how to store the MAC address in the device tree. This has nothing to do with our problem here - Linux has no way to know whether a locally administered MAC address has been assigned by the user for a specific purpose, or if it has been generated randomly. Actually it does not even care.
Best regards,
Wolfgang Denk