[U-Boot] net: use random ethernet address if invalid and not zero

From: Siva Durga Prasad Paladugu <siva.durga.paladugu@xilinx.com>

Use random ethernet address if the ethernet address found
is invalid, not zero and config for random address
is defined.

Signed-off-by: Siva Durga Prasad Paladugu <sivadur@xilinx.com>
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

 net/eth-uclass.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Hi,

Regarding "invalid" Ethernet address.

Is there a reliable way to set the default environment that will prevent 
Ethernet communications from being attempted.

That is, when an Ethernet capable system is brand new and before an 
Ethernet MAC address has been assigned to that system during 
manufacturing with the "setenv ethaddr" command, how can Ethernet comms 
be disabled?

This would be a fail-safe to make sure that part of the manufacturing 
process is done correctly and that an Ethernet address is intentionally 
assigned.

Thanks,
Jim

On 11/02/2016 04:52 AM, Michal Simek wrote:
> From: Siva Durga Prasad Paladugu <siva.durga.paladugu@xilinx.com>
>
> Use random ethernet address if the ethernet address found
> is invalid, not zero and config for random address
> is defined.
>
> Signed-off-by: Siva Durga Prasad Paladugu <sivadur@xilinx.com>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---
>
>  net/eth-uclass.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/net/eth-uclass.c b/net/eth-uclass.c
> index a32961e6ceaa..1d130110890e 100644
> --- a/net/eth-uclass.c
> +++ b/net/eth-uclass.c
> @@ -512,7 +512,8 @@ static int eth_post_probe(struct udevice *dev)
>  		eth_setenv_enetaddr_by_index("eth", dev->seq, pdata->enetaddr);
>  		printf("\nWarning: %s using MAC address from ROM\n",
>  		       dev->name);
> -	} else if (is_zero_ethaddr(pdata->enetaddr)) {
> +	} else if (is_zero_ethaddr(pdata->enetaddr) ||
> +		   !is_valid_ethaddr(pdata->enetaddr)) {
>  #ifdef CONFIG_NET_RANDOM_ETHADDR
>  		net_random_ethaddr(pdata->enetaddr);
>  		printf("\nWarning: %s (eth%d) using random MAC address - %pM\n",
>

On Wed, Nov 2, 2016 at 6:52 AM, Michal Simek <michal.simek@xilinx.com> wrote:
> From: Siva Durga Prasad Paladugu <siva.durga.paladugu@xilinx.com>
>
> Use random ethernet address if the ethernet address found
> is invalid, not zero and config for random address
> is defined.
>
> Signed-off-by: Siva Durga Prasad Paladugu <sivadur@xilinx.com>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>

Acked-by: Joe Hershberger <joe.hershberger@ni.com>

Hi Michal,

https://patchwork.ozlabs.org/patch/690374/ was applied to u-boot-net.git.

Thanks!
-Joe

Hi Jim,

On Wed, Nov 2, 2016 at 8:21 AM, James Chargin <jimccrown@gmail.com> wrote:
> Hi,
>
> Regarding "invalid" Ethernet address.
>
> Is there a reliable way to set the default environment that will prevent
> Ethernet communications from being attempted.
>
> That is, when an Ethernet capable system is brand new and before an Ethernet
> MAC address has been assigned to that system during manufacturing with the
> "setenv ethaddr" command, how can Ethernet comms be disabled?
>
> This would be a fail-safe to make sure that part of the manufacturing
> process is done correctly and that an Ethernet address is intentionally
> assigned.

There is no feature like that as far as I know. Can you not just read
back the ethaddr variable and compare it to what you intended to
program?

-Joe

Hi Joe,

Thanks for your reply.

On 11/07/2016 10:20 AM, Joe Hershberger wrote:
> Hi Jim,
>
> On Wed, Nov 2, 2016 at 8:21 AM, James Chargin <jimccrown@gmail.com> wrote:
>> Hi,
>>
>> Regarding "invalid" Ethernet address.
>>
>> Is there a reliable way to set the default environment that will prevent
>> Ethernet communications from being attempted.
>>
>> That is, when an Ethernet capable system is brand new and before an Ethernet
>> MAC address has been assigned to that system during manufacturing with the
>> "setenv ethaddr" command, how can Ethernet comms be disabled?
>>
>> This would be a fail-safe to make sure that part of the manufacturing
>> process is done correctly and that an Ethernet address is intentionally
>> assigned.
>
> There is no feature like that as far as I know. Can you not just read
> back the ethaddr variable and compare it to what you intended to
> program?

I don't understand what you are suggesting, sorry.

A long time ago I wrote code

/*
  * This is an invalid MAC address.
  * Having it be invalid prevents connection to
  * the network until the U-Boot "setenv ethaddr"
  * command is run, which selects a valid MAC
  * Refer to the section titled "Null values" in
  * http://standards.ieee.org/regauth/oui/tutorials/UseOfEUI.html.
  */
#define CONFIG_ETHADDR      ff:ff:ff:ff:ff:ff

The referenced section says, "... The all-zeros and all-ones EUI-64 
values, 00-00-00-00-00-00-00-00hex and FF-FF-FF-FF-FF-FF-FF-FFhex, 
respectively, are owned by the IEEE Registration Authority and will 
never be assigned, and are invalid for use as identifiers. ..." (which 
doesn't really apply to my case, but I was hopeful that I could 
generalize to my case)

At the time I wrote this code, I was trying to cope with a mfg process 
that was about to ship systems without setting correct MAC addresses. I 
was new to U-Boot at that time but found that U-Boot would not build 
without CONFIG_ETHADDR defined.

I subsequently checked again and found that U-Boot connects to the 
network regardless of the value of CONFIG_ETHADDR and even when it is 
not defined at compile-time or run-time.

I left it there, instead I made changes to the mfg process so that 
"setenv ethaddr" was more likely to always get done.

I would have liked a way to configure the default U-Boot environment so 
that no network connection could be made until a MAC address had been 
set, but I wasn't willing to spend any more time figuring out how to 
make that happen (possibly by submitting a patch upstream)

Jim

On 7.11.2016 18:31, Joe Hershberger wrote:
> Hi Michal,
> 
> https://patchwork.ozlabs.org/patch/690374/ was applied to u-boot-net.git.

Thanks,
Michal

On Mon, Nov 7, 2016 at 4:25 PM, James Chargin <jimccrown@gmail.com> wrote:
> Hi Joe,
>
> Thanks for your reply.
>
> On 11/07/2016 10:20 AM, Joe Hershberger wrote:
>>
>> Hi Jim,
>>
>> On Wed, Nov 2, 2016 at 8:21 AM, James Chargin <jimccrown@gmail.com> wrote:
>>>
>>> Hi,
>>>
>>> Regarding "invalid" Ethernet address.
>>>
>>> Is there a reliable way to set the default environment that will prevent
>>> Ethernet communications from being attempted.
>>>
>>> That is, when an Ethernet capable system is brand new and before an
>>> Ethernet
>>> MAC address has been assigned to that system during manufacturing with
>>> the
>>> "setenv ethaddr" command, how can Ethernet comms be disabled?
>>>
>>> This would be a fail-safe to make sure that part of the manufacturing
>>> process is done correctly and that an Ethernet address is intentionally
>>> assigned.
>>
>>
>> There is no feature like that as far as I know. Can you not just read
>> back the ethaddr variable and compare it to what you intended to
>> program?
>
>
> I don't understand what you are suggesting, sorry.
>
> A long time ago I wrote code
>
> /*
>  * This is an invalid MAC address.
>  * Having it be invalid prevents connection to
>  * the network until the U-Boot "setenv ethaddr"
>  * command is run, which selects a valid MAC
>  * Refer to the section titled "Null values" in
>  * http://standards.ieee.org/regauth/oui/tutorials/UseOfEUI.html.
>  */
> #define CONFIG_ETHADDR      ff:ff:ff:ff:ff:ff
>
> The referenced section says, "... The all-zeros and all-ones EUI-64 values,
> 00-00-00-00-00-00-00-00hex and FF-FF-FF-FF-FF-FF-FF-FFhex, respectively, are
> owned by the IEEE Registration Authority and will never be assigned, and are
> invalid for use as identifiers. ..." (which doesn't really apply to my case,
> but I was hopeful that I could generalize to my case)
>
> At the time I wrote this code, I was trying to cope with a mfg process that
> was about to ship systems without setting correct MAC addresses. I was new
> to U-Boot at that time but found that U-Boot would not build without
> CONFIG_ETHADDR defined.
>
> I subsequently checked again and found that U-Boot connects to the network
> regardless of the value of CONFIG_ETHADDR and even when it is not defined at
> compile-time or run-time.
>
> I left it there, instead I made changes to the mfg process so that "setenv
> ethaddr" was more likely to always get done.

I was simply suggesting that you add a step to your MFG that reads
back the ethaddr after rebooting the target to ensure that the MAC was
programmed.

> I would have liked a way to configure the default U-Boot environment so that
> no network connection could be made until a MAC address had been set, but I
> wasn't willing to spend any more time figuring out how to make that happen
> (possibly by submitting a patch upstream)

You can get that behavior simply by not defining CONFIG_NET_RANDOM_ETHADDR.

#ifdef CONFIG_NET_RANDOM_ETHADDR
                net_random_ethaddr(pdata->enetaddr);
                printf("\nWarning: %s (eth%d) using random MAC address - %pM\n",
                       dev->name, dev->seq, pdata->enetaddr);
#else
                printf("\nError: %s address not set.\n",
                       dev->name);
                return -EINVAL;
#endif

Cheers,
-Joe