[U-Boot,2/2] calimain: Generate random MAC address for factory tests

Signed-off-by: Christian Riesch <christian.riesch@omicron.at>
---
 board/omicron/calimain/calimain.c |   31 ++++++++++++++++++++++++++++++-
 include/configs/calimain.h        |    2 ++
 2 files changed, 32 insertions(+), 1 deletion(-)

Dear Christian Riesch,

In message <6cc4810c-1e2e-4ebf-912a-96936f035c0e@mary.at.omicron.at> you wrote:
> Signed-off-by: Christian Riesch <christian.riesch@omicron.at>
> ---
>  board/omicron/calimain/calimain.c |   31 ++++++++++++++++++++++++++++++-
>  include/configs/calimain.h        |    2 ++
>  2 files changed, 32 insertions(+), 1 deletion(-)
> 
> diff --git a/board/omicron/calimain/calimain.c b/board/omicron/calimain/calimain.c
> index 1060a1f..80e3893 100644
> --- a/board/omicron/calimain/calimain.c
> +++ b/board/omicron/calimain/calimain.c
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright (C) 2011 OMICRON electronics GmbH
> + * Copyright (C) 2011-2013 OMICRON electronics GmbH
>   *
>   * Based on da850evm.c. Original Copyrights follow:
>   *
> @@ -136,6 +136,35 @@ int board_init(void)
>  	return 0;
>  }
>  
> +/* seed random number generator with uninitialized SRAM content */
> +static void srand_sram(void)
> +{
> +	int *p;
> +	int seed = 0;
> +
> +	for (p = (int *) 0x80000000; p < (int *) 0x8001ffff; p++)
> +		seed ^= *p;
> +
> +	srand(seed);
> +}

Note that your "uninitialized" SRAM content is probably not so much
random at all - I guess, it is much less random than the originally
used timer value.

What exactly is your justification for such a change?  Please
elucidate...

> +int board_late_init(void)
> +{
> +	uchar enetaddr[6];
> +
> +	if (!eth_getenv_enetaddr("ethaddr", enetaddr)) {
> +		srand_sram();
> +		eth_random_enetaddr(enetaddr);
> +		if (eth_setenv_enetaddr("ethaddr", enetaddr)) {
> +			printf("Failed to set random ethernet address\n");
> +		} else {
> +			printf("Setting random ethernet address %pM.\n",
> +			       enetaddr);
> +		}
> +	}
> +	return 0;
> +}

NAK! You are but duplicating the code already present in net/eth.c

This makes no sense.

Best regards,

Wolfgang Denk

Hello Wolfgang,
Thank you again for your comments.

On 2013-01-08 18:39, Wolfgang Denk wrote:
> Dear Christian Riesch,
>
> In message <6cc4810c-1e2e-4ebf-912a-96936f035c0e@mary.at.omicron.at> you wrote:
>> Signed-off-by: Christian Riesch <christian.riesch@omicron.at>
>> ---
>>   board/omicron/calimain/calimain.c |   31 ++++++++++++++++++++++++++++++-
>>   include/configs/calimain.h        |    2 ++
>>   2 files changed, 32 insertions(+), 1 deletion(-)
>>
>> diff --git a/board/omicron/calimain/calimain.c b/board/omicron/calimain/calimain.c
>> index 1060a1f..80e3893 100644
>> --- a/board/omicron/calimain/calimain.c
>> +++ b/board/omicron/calimain/calimain.c
>> @@ -1,5 +1,5 @@
>>   /*
>> - * Copyright (C) 2011 OMICRON electronics GmbH
>> + * Copyright (C) 2011-2013 OMICRON electronics GmbH
>>    *
>>    * Based on da850evm.c. Original Copyrights follow:
>>    *
>> @@ -136,6 +136,35 @@ int board_init(void)
>>   	return 0;
>>   }
>>
>> +/* seed random number generator with uninitialized SRAM content */
>> +static void srand_sram(void)
>> +{
>> +	int *p;
>> +	int seed = 0;
>> +
>> +	for (p = (int *) 0x80000000; p < (int *) 0x8001ffff; p++)
>> +		seed ^= *p;
>> +
>> +	srand(seed);
>> +}
>
> Note that your "uninitialized" SRAM content is probably not so much
> random at all -

There are several papers around describing the use of initial SRAM value 
after power up for the generation of random numbers. This is why I gave 
it a try, and it works pretty well for me. I get a different seed for 
each power-up cycle. I guess that the randomness is limited and that 
part of the generated seed is more a fingerprint for the chip, therefore 
it may not be good enough for security related stuff, but for my purpose 
it's ok.

> I guess, it is much less random than the originally
> used timer value.

In my case the timer value is not random at all since it is reset to 
zero at power-up. Since seeding the random number generator is always 
done at the same time after power-up in the current code, the seed will 
always be the same for my devices. Therefore the generated MAC address 
will always be the same for all devices.

> What exactly is your justification for such a change?  Please
> elucidate...

Actually I do not change anything ;-)

For the lsxl board that is currently the only user of eth_random_enet(), 
nothing changes. get_timer(0) remains the source of the randomness for 
this board. My patches only allow other boards to use a sources of 
randomness that is available to them instead of forcing everyone to use 
get_timer(0).

>
>> +int board_late_init(void)
>> +{
>> +	uchar enetaddr[6];
>> +
>> +	if (!eth_getenv_enetaddr("ethaddr", enetaddr)) {
>> +		srand_sram();
>> +		eth_random_enetaddr(enetaddr);
>> +		if (eth_setenv_enetaddr("ethaddr", enetaddr)) {
>> +			printf("Failed to set random ethernet address\n");
>> +		} else {
>> +			printf("Setting random ethernet address %pM.\n",
>> +			       enetaddr);
>> +		}
>> +	}
>> +	return 0;
>> +}
>
> NAK! You are but duplicating the code already present in net/eth.c

Apparently I am missing something here. I do not see a call of 
eth_random_enetaddr() in net/eth.c. To which part of net/eth.c are you 
referring?

Regards, Christian

> This makes no sense.
>
> Best regards,
>
> Wolfgang Denk
>

Dear Christian Riesch,

In message <50ED2B84.2080905@omicron.at> you wrote:
> 
> > What exactly is your justification for such a change?  Please
> > elucidate...
> 
> Actually I do not change anything ;-)

Why do you need a patch then? :-P

> > NAK! You are but duplicating the code already present in net/eth.c
> 
> Apparently I am missing something here. I do not see a call of 
> eth_random_enetaddr() in net/eth.c. To which part of net/eth.c are you 
> referring?

Hm... I can't find it now, either. Dunno what I have seen then.
eventually I was looking at your patch in two windows :-(

Sorry - but still: very similar code exists in rescue_mode() in
"board/buffalo/lsxl/lsxl.c"; this should be factored out.

Best regards,

Wolfgang Denk