diff mbox series

defconfig: espressobin: enable NET_RANDOM_ETHADDR

Message ID 20200908063500.480897-1-a.heider@gmail.com
State Accepted
Commit 073ccfab5ffd522a35bef7b424a1ac824315b3c1
Delegated to: Stefan Roese
Headers show
Series defconfig: espressobin: enable NET_RANDOM_ETHADDR | expand

Commit Message

Andre Heider Sept. 8, 2020, 6:35 a.m. UTC
The hardware does not provide a MAC address. Enable this so that
network access works with just the default environment.

Signed-off-by: Andre Heider <a.heider@gmail.com>
---
 configs/mvebu_espressobin-88f3720_defconfig | 1 +
 1 file changed, 1 insertion(+)

Comments

Pali Rohár Sept. 8, 2020, 7:42 a.m. UTC | #1
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.

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.

> Signed-off-by: Andre Heider <a.heider@gmail.com>
> ---
>  configs/mvebu_espressobin-88f3720_defconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/configs/mvebu_espressobin-88f3720_defconfig b/configs/mvebu_espressobin-88f3720_defconfig
> index 7aabbba59f..5e9fcd1f26 100644
> --- a/configs/mvebu_espressobin-88f3720_defconfig
> +++ b/configs/mvebu_espressobin-88f3720_defconfig
> @@ -84,3 +84,4 @@ CONFIG_USB_ETHER_RTL8152=y
>  CONFIG_USB_ETHER_SMSC95XX=y
>  CONFIG_SHA1=y
>  CONFIG_SHA256=y
> +CONFIG_NET_RANDOM_ETHADDR=y
> -- 
> 2.28.0
>
Andre Heider Sept. 8, 2020, 8:14 a.m. UTC | #2
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.

> 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.

Out of the box working network access is probably what most users care 
about.

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 ;)

Thanks,
Andre

> 
>> Signed-off-by: Andre Heider <a.heider@gmail.com>
>> ---
>>   configs/mvebu_espressobin-88f3720_defconfig | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/configs/mvebu_espressobin-88f3720_defconfig b/configs/mvebu_espressobin-88f3720_defconfig
>> index 7aabbba59f..5e9fcd1f26 100644
>> --- a/configs/mvebu_espressobin-88f3720_defconfig
>> +++ b/configs/mvebu_espressobin-88f3720_defconfig
>> @@ -84,3 +84,4 @@ CONFIG_USB_ETHER_RTL8152=y
>>   CONFIG_USB_ETHER_SMSC95XX=y
>>   CONFIG_SHA1=y
>>   CONFIG_SHA256=y
>> +CONFIG_NET_RANDOM_ETHADDR=y
>> -- 
>> 2.28.0
>>
Tom Rini Sept. 8, 2020, 12:52 p.m. UTC | #3
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.

> > 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.
> 
> Out of the box working network access is probably what most users care
> about.
> 
> 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 ;)

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.  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.
Pali Rohár Sept. 8, 2020, 10:38 p.m. UTC | #4
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.
Marek Behún Sept. 11, 2020, 11:55 a.m. UTC | #5
On Wed, 9 Sep 2020 00:38:31 +0200
Pali Rohár <pali@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?
Andre Heider Sept. 11, 2020, 3:52 p.m. UTC | #6
Hi Marek,

On 11/09/2020 13:55, Marek Behún wrote:
> On Wed, 9 Sep 2020 00:38:31 +0200 Pali Rohár <pali@kernel.org> wrote:
>> On Tuesday 08 September 2020 08:52:56 Tom Rini wrote:
>>> 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?

if there's a mac stored in otp during manufacturing, that's of course 
the best solution. There's no need for CONFIG_NET_RANDOM_ETHADDR then. 
But globalscale does not do that.

Doing it afterwards, so u-boot claiming some otp space for itself, and 
instructing the user how to write to it sounds too dangerous/error-prone.

For me CONFIG_NET_RANDOM_ETHADDR is a knob that should be enabled if 
there's no mac address stored in a sane way (where saving it just to an 
u-boot env during manufacturing doesn't count as sane, especially if the 
vendor moves the spi env offset around in a firmware update).

So I think CONFIG_NET_RANDOM_ETHADDR is enough.

But it would be nice if e.g. a board could set specific env vars as 
indestructible/unwipeable/precious/whatever, which instructs `env 
default -a` to keep those (which is common after updating the 
bootloader). Maybe that's an idea worth pursuing?

Thanks,
Andre
Marek Behún Sept. 11, 2020, 4:22 p.m. UTC | #7
On Fri, 11 Sep 2020 17:52:26 +0200
Andre Heider <a.heider@gmail.com> wrote:

> Hi Marek,
> 
> On 11/09/2020 13:55, Marek Behún wrote:
> > On Wed, 9 Sep 2020 00:38:31 +0200 Pali Rohár <pali@kernel.org>
> > wrote:  
> >> On Tuesday 08 September 2020 08:52:56 Tom Rini wrote:  
> >>> 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?  
> 
> if there's a mac stored in otp during manufacturing, that's of course 
> the best solution. There's no need for CONFIG_NET_RANDOM_ETHADDR
> then. But globalscale does not do that.
> 
> Doing it afterwards, so u-boot claiming some otp space for itself,
> and instructing the user how to write to it sounds too
> dangerous/error-prone.
> 
> For me CONFIG_NET_RANDOM_ETHADDR is a knob that should be enabled if 
> there's no mac address stored in a sane way (where saving it just to
> an u-boot env during manufacturing doesn't count as sane, especially
> if the vendor moves the spi env offset around in a firmware update).
> 
> So I think CONFIG_NET_RANDOM_ETHADDR is enough.
> 

I understand Pali's concerns, though.

The thing is that if we enable CONFIG_NET_RANDOM_ETHADDR by default,
many users that have managed to wipe their env won't care about that
they are using randomly generated MAC address and won't solve the issue,
which is again the spirit of correctly configure networks.

If we do not enable CONFIG_NET_RANDOM_ETHADDR, the worst that can
happen is that the device won't boot from network. This will force the
users to solve the issue, which is not that hard
  setenv ethaddr aa:bb:cc:dd:ee:ff (address from the sticker)
  saveenv
If the users boots from SD/eMMC/SATA/USB, Linux won't have problem with
network, since it will generate random MAC address anyway.

The worst case scenario does not look that bad to me if we don't enable
CONFIG_NET_RANDOM_ETHADDR by default.

I think the option CONFIG_NET_RANDOM_ETHADDR should be used only by
developers when they are developing on devices that have not yet burned
the MAC addresses, or on some embedded devices that don't have space
for saving a MAC address (no storage for env or anything else, do such
devices exist?).

But it shouldn't be used as a default setting for a device that has
storage where it can store the address.

> But it would be nice if e.g. a board could set specific env vars as 
> indestructible/unwipeable/precious/whatever, which instructs `env 
> default -a` to keep those (which is common after updating the 
> bootloader). Maybe that's an idea worth pursuing?
> 

Yes, that would be nice :)

> Thanks,
> Andre

Marek
Andre Heider Sept. 11, 2020, 4:47 p.m. UTC | #8
On 11/09/2020 18:22, Marek Behún wrote:
> On Fri, 11 Sep 2020 17:52:26 +0200
> Andre Heider <a.heider@gmail.com> wrote:
> 
>> Hi Marek,
>>
>> On 11/09/2020 13:55, Marek Behún wrote:
>>> On Wed, 9 Sep 2020 00:38:31 +0200 Pali Rohár <pali@kernel.org>
>>> wrote:
>>>> On Tuesday 08 September 2020 08:52:56 Tom Rini wrote:
>>>>> 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?
>>
>> if there's a mac stored in otp during manufacturing, that's of course
>> the best solution. There's no need for CONFIG_NET_RANDOM_ETHADDR
>> then. But globalscale does not do that.
>>
>> Doing it afterwards, so u-boot claiming some otp space for itself,
>> and instructing the user how to write to it sounds too
>> dangerous/error-prone.
>>
>> For me CONFIG_NET_RANDOM_ETHADDR is a knob that should be enabled if
>> there's no mac address stored in a sane way (where saving it just to
>> an u-boot env during manufacturing doesn't count as sane, especially
>> if the vendor moves the spi env offset around in a firmware update).
>>
>> So I think CONFIG_NET_RANDOM_ETHADDR is enough.
>>
> 
> I understand Pali's concerns, though.
> 
> The thing is that if we enable CONFIG_NET_RANDOM_ETHADDR by default,
> many users that have managed to wipe their env won't care about that
> they are using randomly generated MAC address and won't solve the issue,
> which is again the spirit of correctly configure networks.
> 
> If we do not enable CONFIG_NET_RANDOM_ETHADDR, the worst that can
> happen is that the device won't boot from network. This will force the
> users to solve the issue, which is not that hard
>    setenv ethaddr aa:bb:cc:dd:ee:ff (address from the sticker)
>    saveenv
> If the users boots from SD/eMMC/SATA/USB, Linux won't have problem with
> network, since it will generate random MAC address anyway.

Good point, so let's assume the user doesn't have a mac stored.

Without CONFIG_NET_RANDOM_ETHADDR:
* u-boot refuses to do anything network related
* Linux generates a random mac, network works

With CONFIG_NET_RANDOM_ETHADDR:
* u-boot generates a random mac, network works
* Linux uses the same random mac, passed on from u-boot throught the 
device-tree, network works

It's still random, but at least it's consistent ;)

> The worst case scenario does not look that bad to me if we don't enable
> CONFIG_NET_RANDOM_ETHADDR by default.
> 
> I think the option CONFIG_NET_RANDOM_ETHADDR should be used only by
> developers when they are developing on devices that have not yet burned
> the MAC addresses, or on some embedded devices that don't have space
> for saving a MAC address (no storage for env or anything else, do such
> devices exist?).

It looks like it's used for more than that though:

$ git ls-files configs|wc -l
1362

$ git grep CONFIG_NET_RANDOM_ETHADDR configs|wc -l
209

Some popular sbc's are part of that list.

> But it shouldn't be used as a default setting for a device that has
> storage where it can store the address.
> 
>> But it would be nice if e.g. a board could set specific env vars as
>> indestructible/unwipeable/precious/whatever, which instructs `env
>> default -a` to keep those (which is common after updating the
>> bootloader). Maybe that's an idea worth pursuing?
>>
> 
> Yes, that would be nice :)
> 
>> Thanks,
>> Andre
> 
> Marek
>
Tom Rini Sept. 11, 2020, 5:10 p.m. UTC | #9
On Fri, Sep 11, 2020 at 06:47:02PM +0200, Andre Heider wrote:
> On 11/09/2020 18:22, Marek Behún wrote:
> > On Fri, 11 Sep 2020 17:52:26 +0200
> > Andre Heider <a.heider@gmail.com> wrote:
> > 
> > > Hi Marek,
> > > 
> > > On 11/09/2020 13:55, Marek Behún wrote:
> > > > On Wed, 9 Sep 2020 00:38:31 +0200 Pali Rohár <pali@kernel.org>
> > > > wrote:
> > > > > On Tuesday 08 September 2020 08:52:56 Tom Rini wrote:
> > > > > > 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?
> > > 
> > > if there's a mac stored in otp during manufacturing, that's of course
> > > the best solution. There's no need for CONFIG_NET_RANDOM_ETHADDR
> > > then. But globalscale does not do that.
> > > 
> > > Doing it afterwards, so u-boot claiming some otp space for itself,
> > > and instructing the user how to write to it sounds too
> > > dangerous/error-prone.
> > > 
> > > For me CONFIG_NET_RANDOM_ETHADDR is a knob that should be enabled if
> > > there's no mac address stored in a sane way (where saving it just to
> > > an u-boot env during manufacturing doesn't count as sane, especially
> > > if the vendor moves the spi env offset around in a firmware update).
> > > 
> > > So I think CONFIG_NET_RANDOM_ETHADDR is enough.
> > > 
> > 
> > I understand Pali's concerns, though.
> > 
> > The thing is that if we enable CONFIG_NET_RANDOM_ETHADDR by default,
> > many users that have managed to wipe their env won't care about that
> > they are using randomly generated MAC address and won't solve the issue,
> > which is again the spirit of correctly configure networks.
> > 
> > If we do not enable CONFIG_NET_RANDOM_ETHADDR, the worst that can
> > happen is that the device won't boot from network. This will force the
> > users to solve the issue, which is not that hard
> >    setenv ethaddr aa:bb:cc:dd:ee:ff (address from the sticker)
> >    saveenv
> > If the users boots from SD/eMMC/SATA/USB, Linux won't have problem with
> > network, since it will generate random MAC address anyway.
> 
> Good point, so let's assume the user doesn't have a mac stored.
> 
> Without CONFIG_NET_RANDOM_ETHADDR:
> * u-boot refuses to do anything network related
> * Linux generates a random mac, network works
> 
> With CONFIG_NET_RANDOM_ETHADDR:
> * u-boot generates a random mac, network works
> * Linux uses the same random mac, passed on from u-boot throught the
> device-tree, network works
> 
> It's still random, but at least it's consistent ;)

It's also only used when we cannot find the MAC.  At the end of the day,
the final decision here is Konstantin's (as the listed maintainer).  I
personally think enabling the option is good given the apparent number
of ways the real MAC will have been lost from SW, but if the maintainer
wants to instead opt for a verbose failure message, that's fine too.
Dennis Gilmore Sept. 11, 2020, 5:16 p.m. UTC | #10
I agree with Tom here, I have a few different systems that use this
feature, it is useful. I think most if not all of the systems I have
using it have Marvell SoC's

Dennis

On Fri, Sep 11, 2020 at 12:11 PM Tom Rini <trini@konsulko.com> wrote:
>
> On Fri, Sep 11, 2020 at 06:47:02PM +0200, Andre Heider wrote:
> > On 11/09/2020 18:22, Marek Behún wrote:
> > > On Fri, 11 Sep 2020 17:52:26 +0200
> > > Andre Heider <a.heider@gmail.com> wrote:
> > >
> > > > Hi Marek,
> > > >
> > > > On 11/09/2020 13:55, Marek Behún wrote:
> > > > > On Wed, 9 Sep 2020 00:38:31 +0200 Pali Rohár <pali@kernel.org>
> > > > > wrote:
> > > > > > On Tuesday 08 September 2020 08:52:56 Tom Rini wrote:
> > > > > > > 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?
> > > >
> > > > if there's a mac stored in otp during manufacturing, that's of course
> > > > the best solution. There's no need for CONFIG_NET_RANDOM_ETHADDR
> > > > then. But globalscale does not do that.
> > > >
> > > > Doing it afterwards, so u-boot claiming some otp space for itself,
> > > > and instructing the user how to write to it sounds too
> > > > dangerous/error-prone.
> > > >
> > > > For me CONFIG_NET_RANDOM_ETHADDR is a knob that should be enabled if
> > > > there's no mac address stored in a sane way (where saving it just to
> > > > an u-boot env during manufacturing doesn't count as sane, especially
> > > > if the vendor moves the spi env offset around in a firmware update).
> > > >
> > > > So I think CONFIG_NET_RANDOM_ETHADDR is enough.
> > > >
> > >
> > > I understand Pali's concerns, though.
> > >
> > > The thing is that if we enable CONFIG_NET_RANDOM_ETHADDR by default,
> > > many users that have managed to wipe their env won't care about that
> > > they are using randomly generated MAC address and won't solve the issue,
> > > which is again the spirit of correctly configure networks.
> > >
> > > If we do not enable CONFIG_NET_RANDOM_ETHADDR, the worst that can
> > > happen is that the device won't boot from network. This will force the
> > > users to solve the issue, which is not that hard
> > >    setenv ethaddr aa:bb:cc:dd:ee:ff (address from the sticker)
> > >    saveenv
> > > If the users boots from SD/eMMC/SATA/USB, Linux won't have problem with
> > > network, since it will generate random MAC address anyway.
> >
> > Good point, so let's assume the user doesn't have a mac stored.
> >
> > Without CONFIG_NET_RANDOM_ETHADDR:
> > * u-boot refuses to do anything network related
> > * Linux generates a random mac, network works
> >
> > With CONFIG_NET_RANDOM_ETHADDR:
> > * u-boot generates a random mac, network works
> > * Linux uses the same random mac, passed on from u-boot throught the
> > device-tree, network works
> >
> > It's still random, but at least it's consistent ;)
>
> It's also only used when we cannot find the MAC.  At the end of the day,
> the final decision here is Konstantin's (as the listed maintainer).  I
> personally think enabling the option is good given the apparent number
> of ways the real MAC will have been lost from SW, but if the maintainer
> wants to instead opt for a verbose failure message, that's fine too.
>
> --
> Tom
Kostya Porotchkin Sept. 13, 2020, 9:21 a.m. UTC | #11
Hello,

> -----Original Message-----
> From: Dennis Gilmore <dgilmore@fedoraproject.org>
> Sent: Friday, September 11, 2020 20:16
> To: Tom Rini <trini@konsulko.com>
> Cc: Andre Heider <a.heider@gmail.com>; Marek Behún
> <marek.behun@nic.cz>; Pali Rohár <pali@kernel.org>; Stefan Roese
> <sr@denx.de>; Kostya Porotchkin <kostap@marvell.com>; U-Boot Mailing
> List <u-boot@lists.denx.de>
> Subject: [EXT] Re: [PATCH] defconfig: espressobin: enable
> NET_RANDOM_ETHADDR
> 
> External Email
> 
> ----------------------------------------------------------------------
> I agree with Tom here, I have a few different systems that use this feature, it
> is useful. I think most if not all of the systems I have using it have Marvell
> SoC's
> 
> Dennis
> 
> On Fri, Sep 11, 2020 at 12:11 PM Tom Rini <trini@konsulko.com> wrote:
> >
> > On Fri, Sep 11, 2020 at 06:47:02PM +0200, Andre Heider wrote:
> > > On 11/09/2020 18:22, Marek Behún wrote:
> > > > On Fri, 11 Sep 2020 17:52:26 +0200 Andre Heider
> > > > <a.heider@gmail.com> wrote:
> > > >
> > > > > Hi Marek,
> > > > >
> > > > > On 11/09/2020 13:55, Marek Behún wrote:
> > > > > > On Wed, 9 Sep 2020 00:38:31 +0200 Pali Rohár <pali@kernel.org>
> > > > > > wrote:
> > > > > > > On Tuesday 08 September 2020 08:52:56 Tom Rini wrote:
> > > > > > > > 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?
> > > > >
> > > > > if there's a mac stored in otp during manufacturing, that's of
> > > > > course the best solution. There's no need for
> > > > > CONFIG_NET_RANDOM_ETHADDR then. But globalscale does not do
> that.
> > > > >
> > > > > Doing it afterwards, so u-boot claiming some otp space for
> > > > > itself, and instructing the user how to write to it sounds too
> > > > > dangerous/error-prone.
> > > > >
> > > > > For me CONFIG_NET_RANDOM_ETHADDR is a knob that should be
> > > > > enabled if there's no mac address stored in a sane way (where
> > > > > saving it just to an u-boot env during manufacturing doesn't
> > > > > count as sane, especially if the vendor moves the spi env offset
> around in a firmware update).
> > > > >
> > > > > So I think CONFIG_NET_RANDOM_ETHADDR is enough.
> > > > >
> > > >
> > > > I understand Pali's concerns, though.
> > > >
> > > > The thing is that if we enable CONFIG_NET_RANDOM_ETHADDR by
> > > > default, many users that have managed to wipe their env won't care
> > > > about that they are using randomly generated MAC address and won't
> > > > solve the issue, which is again the spirit of correctly configure networks.
> > > >
> > > > If we do not enable CONFIG_NET_RANDOM_ETHADDR, the worst that
> can
> > > > happen is that the device won't boot from network. This will force
> > > > the users to solve the issue, which is not that hard
> > > >    setenv ethaddr aa:bb:cc:dd:ee:ff (address from the sticker)
> > > >    saveenv
> > > > If the users boots from SD/eMMC/SATA/USB, Linux won't have
> problem
> > > > with network, since it will generate random MAC address anyway.
> > >
> > > Good point, so let's assume the user doesn't have a mac stored.
> > >
> > > Without CONFIG_NET_RANDOM_ETHADDR:
> > > * u-boot refuses to do anything network related
> > > * Linux generates a random mac, network works
> > >
> > > With CONFIG_NET_RANDOM_ETHADDR:
> > > * u-boot generates a random mac, network works
> > > * Linux uses the same random mac, passed on from u-boot throught the
> > > device-tree, network works
> > >
> > > It's still random, but at least it's consistent ;)
> >
> > It's also only used when we cannot find the MAC.  At the end of the
> > day, the final decision here is Konstantin's (as the listed
> > maintainer).  I personally think enabling the option is good given the
> > apparent number of ways the real MAC will have been lost from SW, but
> > if the maintainer wants to instead opt for a verbose failure message, that's
> fine too.
I have no objections about this configuration option. 
It is better to have a random MAC address rather then no MAC at all.

Regards
Kosta
> >
> > --
> > Tom
Stefan Roese Sept. 21, 2020, 7:50 a.m. UTC | #12
Hi Kosta,

On 13.09.20 11:21, Kostya Porotchkin wrote:

<snip>

>>>> Good point, so let's assume the user doesn't have a mac stored.
>>>>
>>>> Without CONFIG_NET_RANDOM_ETHADDR:
>>>> * u-boot refuses to do anything network related
>>>> * Linux generates a random mac, network works
>>>>
>>>> With CONFIG_NET_RANDOM_ETHADDR:
>>>> * u-boot generates a random mac, network works
>>>> * Linux uses the same random mac, passed on from u-boot throught the
>>>> device-tree, network works
>>>>
>>>> It's still random, but at least it's consistent ;)
>>>
>>> It's also only used when we cannot find the MAC.  At the end of the
>>> day, the final decision here is Konstantin's (as the listed
>>> maintainer).  I personally think enabling the option is good given the
>>> apparent number of ways the real MAC will have been lost from SW, but
>>> if the maintainer wants to instead opt for a verbose failure message, that's
>> fine too.
> I have no objections about this configuration option.
> It is better to have a random MAC address rather then no MAC at all.

This is also my feeling.

Kosta, can you please send a Reviewed-by or Acked-by tag then?

Thanks,
Stefan
Pali Rohár Sept. 21, 2020, 1:05 p.m. UTC | #13
On Friday 11 September 2020 18:22:04 Marek Behún wrote:
> On Fri, 11 Sep 2020 17:52:26 +0200
> Andre Heider <a.heider@gmail.com> wrote:
> 
> > Hi Marek,
> > 
> > On 11/09/2020 13:55, Marek Behún wrote:
> > > On Wed, 9 Sep 2020 00:38:31 +0200 Pali Rohár <pali@kernel.org>
> > > wrote:  
> > >> On Tuesday 08 September 2020 08:52:56 Tom Rini wrote:  
> > >>> 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?  
> > 
> > if there's a mac stored in otp during manufacturing, that's of course 
> > the best solution. There's no need for CONFIG_NET_RANDOM_ETHADDR
> > then. But globalscale does not do that.
> > 
> > Doing it afterwards, so u-boot claiming some otp space for itself,
> > and instructing the user how to write to it sounds too
> > dangerous/error-prone.
> > 
> > For me CONFIG_NET_RANDOM_ETHADDR is a knob that should be enabled if 
> > there's no mac address stored in a sane way (where saving it just to
> > an u-boot env during manufacturing doesn't count as sane, especially
> > if the vendor moves the spi env offset around in a firmware update).
> > 
> > So I think CONFIG_NET_RANDOM_ETHADDR is enough.
> > 
> 
> I understand Pali's concerns, though.
> 
> The thing is that if we enable CONFIG_NET_RANDOM_ETHADDR by default,
> many users that have managed to wipe their env won't care about that
> they are using randomly generated MAC address and won't solve the issue,
> which is again the spirit of correctly configure networks.

Yes, CONFIG_NET_RANDOM_ETHADDR does not solve the issue. It is just a
workaround which hides real issue.

> If we do not enable CONFIG_NET_RANDOM_ETHADDR, the worst that can
> happen is that the device won't boot from network. This will force the
> users to solve the issue, which is not that hard
>   setenv ethaddr aa:bb:cc:dd:ee:ff (address from the sticker)
>   saveenv
> If the users boots from SD/eMMC/SATA/USB, Linux won't have problem with
> network, since it will generate random MAC address anyway.
> 
> The worst case scenario does not look that bad to me if we don't enable
> CONFIG_NET_RANDOM_ETHADDR by default.

I see it in the same way.

CONFIG_NET_RANDOM_ETHADDR does not affect Linux kernel as it always
generates a random mac address when it does not have correct one.

Moreover CONFIG_NET_RANDOM_ETHADDR introduce another issue. In case
there is no valid mac address, U-Boot generates one. But it does not
pass this generates mac address to kernel and therefore kernel generates
another new random mac address.

I just do not see a benefit from having CONFIG_NET_RANDOM_ETHADDR
enabled by default for Espressobin. Real issue is hidden, people would
be confused (why it still have different mac address) and distributions,
like armbian would continue "workarounding" it by hardcoding one static
mac address for all devices into boot scripts or other places.

> I think the option CONFIG_NET_RANDOM_ETHADDR should be used only by
> developers when they are developing on devices that have not yet burned
> the MAC addresses, or on some embedded devices that don't have space
> for saving a MAC address (no storage for env or anything else, do such
> devices exist?).
> 
> But it shouldn't be used as a default setting for a device that has
> storage where it can store the address.
> 
> > But it would be nice if e.g. a board could set specific env vars as 
> > indestructible/unwipeable/precious/whatever, which instructs `env 
> > default -a` to keep those (which is common after updating the 
> > bootloader). Maybe that's an idea worth pursuing?
> > 
> 
> Yes, that would be nice :)
> 
> > Thanks,
> > Andre
> 
> Marek
Marek Behún Sept. 21, 2020, 1:21 p.m. UTC | #14
On Mon, 21 Sep 2020 15:05:22 +0200
Pali Rohár <pali@kernel.org> wrote:

> Moreover CONFIG_NET_RANDOM_ETHADDR introduce another issue. In case
> there is no valid mac address, U-Boot generates one. But it does not
> pass this generates mac address to kernel and therefore kernel generates
> another new random mac address.

I think it does :) At least on A3720, if the net controller has MAC
address set by U-Boot, I think the same one will be used by kernel.

Marek
Pali Rohár Sept. 21, 2020, 1:31 p.m. UTC | #15
On Monday 21 September 2020 15:21:44 Marek Behun wrote:
> On Mon, 21 Sep 2020 15:05:22 +0200
> Pali Rohár <pali@kernel.org> wrote:
> 
> > Moreover CONFIG_NET_RANDOM_ETHADDR introduce another issue. In case
> > there is no valid mac address, U-Boot generates one. But it does not
> > pass this generates mac address to kernel and therefore kernel generates
> > another new random mac address.
> 
> I think it does :) At least on A3720, if the net controller has MAC
> address set by U-Boot, I think the same one will be used by kernel.

I mean the case when mac address is not already set in env. The case
when CONFIG_NET_RANDOM_ETHADDR code configure mac address, when env
variable is not set.

U-Boot set mac address into FDT file only from env.

turris_mox.c code calls eth_env_set_enetaddr() to set env with correct
mac address.
Tom Rini Sept. 21, 2020, 3:03 p.m. UTC | #16
On Mon, Sep 21, 2020 at 03:31:14PM +0200, Pali Rohár wrote:
> On Monday 21 September 2020 15:21:44 Marek Behun wrote:
> > On Mon, 21 Sep 2020 15:05:22 +0200
> > Pali Rohár <pali@kernel.org> wrote:
> > 
> > > Moreover CONFIG_NET_RANDOM_ETHADDR introduce another issue. In case
> > > there is no valid mac address, U-Boot generates one. But it does not
> > > pass this generates mac address to kernel and therefore kernel generates
> > > another new random mac address.
> > 
> > I think it does :) At least on A3720, if the net controller has MAC
> > address set by U-Boot, I think the same one will be used by kernel.
> 
> I mean the case when mac address is not already set in env. The case
> when CONFIG_NET_RANDOM_ETHADDR code configure mac address, when env
> variable is not set.
> 
> U-Boot set mac address into FDT file only from env.
> 
> turris_mox.c code calls eth_env_set_enetaddr() to set env with correct
> mac address.

If the random address that U-Boot generates is NOT being sent to the
kernel, that's a bug to go and fix.  That's not usually the case for
random MAC addresses generated in U-Boot.
Kostya Porotchkin Sept. 21, 2020, 3:10 p.m. UTC | #17
> -----Original Message-----
> From: Andre Heider <a.heider@gmail.com>
> Sent: Tuesday, September 8, 2020 09:35
> To: Stefan Roese <sr@denx.de>; Kostya Porotchkin <kostap@marvell.com>
> Cc: Pali Rohár <pali@kernel.org>; u-boot@lists.denx.de
> Subject: [EXT] [PATCH] defconfig: espressobin: enable
> NET_RANDOM_ETHADDR
> 
> External Email
> 
> ----------------------------------------------------------------------
> The hardware does not provide a MAC address. Enable this so that network
> access works with just the default environment.
> 
> Signed-off-by: Andre Heider <a.heider@gmail.com>
Acked-by: Konstantin Porotchkin <kostap@marvell.com>

> ---
>  configs/mvebu_espressobin-88f3720_defconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/configs/mvebu_espressobin-88f3720_defconfig
> b/configs/mvebu_espressobin-88f3720_defconfig
> index 7aabbba59f..5e9fcd1f26 100644
> --- a/configs/mvebu_espressobin-88f3720_defconfig
> +++ b/configs/mvebu_espressobin-88f3720_defconfig
> @@ -84,3 +84,4 @@ CONFIG_USB_ETHER_RTL8152=y
> CONFIG_USB_ETHER_SMSC95XX=y  CONFIG_SHA1=y  CONFIG_SHA256=y
> +CONFIG_NET_RANDOM_ETHADDR=y
> --
> 2.28.0
Stefan Roese Sept. 24, 2020, 10:36 a.m. UTC | #18
On 08.09.20 08:35, Andre Heider wrote:
> The hardware does not provide a MAC address. Enable this so that
> network access works with just the default environment.
> 
> Signed-off-by: Andre Heider <a.heider@gmail.com>
> ---
>   configs/mvebu_espressobin-88f3720_defconfig | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/configs/mvebu_espressobin-88f3720_defconfig b/configs/mvebu_espressobin-88f3720_defconfig
> index 7aabbba59f..5e9fcd1f26 100644
> --- a/configs/mvebu_espressobin-88f3720_defconfig
> +++ b/configs/mvebu_espressobin-88f3720_defconfig
> @@ -84,3 +84,4 @@ CONFIG_USB_ETHER_RTL8152=y
>   CONFIG_USB_ETHER_SMSC95XX=y
>   CONFIG_SHA1=y
>   CONFIG_SHA256=y
> +CONFIG_NET_RANDOM_ETHADDR=y
> 

Applied to u-boot-marvell/master

Thanks,
Stefan
diff mbox series

Patch

diff --git a/configs/mvebu_espressobin-88f3720_defconfig b/configs/mvebu_espressobin-88f3720_defconfig
index 7aabbba59f..5e9fcd1f26 100644
--- a/configs/mvebu_espressobin-88f3720_defconfig
+++ b/configs/mvebu_espressobin-88f3720_defconfig
@@ -84,3 +84,4 @@  CONFIG_USB_ETHER_RTL8152=y
 CONFIG_USB_ETHER_SMSC95XX=y
 CONFIG_SHA1=y
 CONFIG_SHA256=y
+CONFIG_NET_RANDOM_ETHADDR=y