From mboxrd@z Thu Jan  1 00:00:00 1970
From: Christian Riesch <christian.riesch@omicron.at>
Date: Wed, 09 Jan 2013 09:34:12 +0100
Subject: [U-Boot] [PATCH 2/2] calimain: Generate random MAC address for
 factory tests
In-Reply-To: <20130108173947.5B123202B69@gemini.denx.de>
References: <1357653279-29635-1-git-send-email-christian.riesch@omicron.at>
	<6cc4810c-1e2e-4ebf-912a-96936f035c0e@mary.at.omicron.at>
	<20130108173947.5B123202B69@gemini.denx.de>
Message-ID: <50ED2B84.2080905@omicron.at>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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
>