[PATCH] defconfig: espressobin: enable NET_RANDOM_ETHADDR

[Marek Behún]

On Wed, 9 Sep 2020 00:38:31 +0200
Pali Rohár <pali at kernel.org> wrote:

> On Tuesday 08 September 2020 08:52:56 Tom Rini wrote:
> > On Tue, Sep 08, 2020 at 10:14:15AM +0200, Andre Heider wrote:  
> > > On 08/09/2020 09:42, Pali Rohár wrote:  
> > > > On Tuesday 08 September 2020 08:35:00 Andre Heider wrote:  
> > > > > The hardware does not provide a MAC address. Enable this so
> > > > > that network access works with just the default environment.  
> > > > 
> > > > Well, this is not fully truth as MAC address is stored in SPI,
> > > > just in non-standard format, in U-Boot env stored in env
> > > > partition and it is hard to use outside of U-Boot, plus easy to
> > > > erase / overwrite / lost.  
> > > 
> > > True, but updating the bootloader usually implies wiping the env,
> > > so it's very easy to lose it.  
> > 
> > It most certainly should NOT wipe out the existing environment, it
> > should be using the same environment location as before.  
> 
> The main issue is that downstream distributions are doing it. Plus
> they probably have custom U-Boot patches. Nothing which mainline
> U-Boot can forbid or change.
> 
> Also if somebody is updating from old Marvell factory U-Boot to
> mainline U-Boot, SPI env offset may be different and therefore it
> wipes env.
> 
> And also, if U-Boot is upgrading from ancient version to new, it is
> suggested to reset env to have e.g. new distroboot support.
> 
> So any of above situation can lead to erasing env and therefore it is
> a very good idea to create backup of MAC address(es).
> 
> > > > I'm not a big fan of this change. This looks like a workaround
> > > > / hack for boards where MAC address was erased (e.g. by broken
> > > > U-Boot distro scripts) or for early boards where MAC address
> > > > was not written at all (as I was told).
> > > > 
> > > > And on these boards this patch would cause that U-Boot would
> > > > see on every boot different MAC address. This would cause
> > > > another mess in network for U-Boot netboot as DHCP/TFTP server
> > > > would see for one board every time different MAC address.
> > > > 
> > > > Is not really better to instruct user how to fix board where
> > > > e.g. broken distro scripts erased MAC address? We have already
> > > > paragraph in README.marvell about it.
> > > > 
> > > > Also this change affects "default" defconfig value. And based
> > > > on above arguments I do not think that this change should be
> > > > enabled by default.
> > > > 
> > > > I understand that for some situations it may be useful (e.g.
> > > > mass board reparation process via netboot), but as this is
> > > > config option, users in such situation can enable this option
> > > > manually.
> > > > 
> > > > I think that for default behavior is not provide network access
> > > > in U-Boot if for some reasons factory permanent MAC address was
> > > > removed. User can easier and faster detect this issue and fix
> > > > it.  
> > > 
> > > It can be argued both ways I guess. If this option wouldn't make
> > > sense it wouldn't exist.  
> 
> As I wrote, I understand that this option is useful in some
> situations.
> 
> > > Out of the box working network access is probably what most users
> > > care about.  
> 
> Of course! But what does "working network access" means? Somebody
> wants to have DHCP, even semi/half-working and does not care about RFC
> compliance. Somebody does not want to have mess in the network and so
> every device must use only assigned MAC / IP (v4/v6) address and not
> any different. And somebody just to want working TFTP, does not care
> about DHCP, IP or MAC address.
> 
> And for some of these situation is CONFIG_NET_RANDOM_ETHADDR great,
> for some harmless and for other is harmful (in context that all these
> devices got assigned MAC address in factory).
> 
> Also I know that in some networks is forbidden to use other than
> factory MAC address (users are not allowed to connect device with
> changed MAC address).
> 
> > > Changing this default means that you need to build the whole
> > > firmware yourself, and let's face it: building it for this board
> > > is a total clusterfuck. Most users will just download a binary. I
> > > don't care too much for this patch, but I would consider what
> > > fits most users.
> > > 
> > > Right now most users will probably run the downstream binaries
> > > provided by armbian, and as you know, that even has single
> > > hardcoded MAC, used for all boards. So this would already be an
> > > improvement ;)  
> 
> I think that we do not have to care about people who use downstream
> patched U-Boot. In most case default U-Boot defconfig may be changed
> or be different.
> 
> Change is in this patch is only about default mainline U-Boot
> configuration. I think it should be sane default and for me sane
> default is the least surprise. So if configuration is missing either
> throw error so can immediately fix it or make it predicable /
> deterministic.
> 
> > Note that when CONFIG_NET_RANDOM_ETHADDR is set, we only use a
> > random MAC address when we haven't found one either on the hardware
> > or environment.  
> 
> I know.
> 
> > It also prints a warning that you are using a random MAC,
> > so if it's documented on how to recover the real MAC a user should
> > see that warning and fix it.  
> 
> In case you did backup of MAC address or you have MAC address printed
> on sticker, you can restore it. If you loaded distribution U-Boot
> which erase MAC address and you have not did any backup, then your
> MAC address is forever lost.

On Turris MOX we write the MAC address into OTP of the SOC during
manufacturing.

It is possible to write code that burns the MAC address into OTP, I
consider this a better option than enabling random MAC address.

Maybe we can enable random MAC address, and if MAC address is not found
in environment nor OTP, issue a warning, something like
  "WARNING: MAC address lost, please burn the MAC address of your device
            to OTP with command xyz"

What do you think?