[U-Boot] [PATCH] tools: gen_eth_addr: remove getpid() operation for the random seed

[Josh Wu]

Hi, Andreas

On 9/16/2015 4:23 PM, Andreas Bießmann wrote:
> Hi Josh,
>
> On 09/16/2015 05:18 AM, Josh Wu wrote:
>> As 'time(0) | getpid()' sometimes get same value. That depends on the
>> value of getpid().
>> So that is not a expected behavior. We expect different value for the
>> seed when when run it in many times.
> I don't think your change made it better. Here is a snippet from a run
> of time(NULL) and getpid():
>
> ---8<---
> time: 1442389450; pid: 11632; time | pid: 1442397690;
> time: 1442389450; pid: 11633; time | pid: 1442397691;
> time: 1442389450; pid: 11634; time | pid: 1442397690;
> time: 1442389450; pid: 11635; time | pid: 1442397691;
> time: 1442389450; pid: 11636; time | pid: 1442397694;
> time: 1442389450; pid: 11637; time | pid: 1442397695;
> time: 1442389450; pid: 11638; time | pid: 1442397694;
> time: 1442389450; pid: 11639; time | pid: 1442397695;
> time: 1442389450; pid: 11640; time | pid: 1442397690;
> time: 1442389450; pid: 11641; time | pid: 1442397691;
> time: 1442389450; pid: 11642; time | pid: 1442397690;
> time: 1442389450; pid: 11643; time | pid: 1442397691;
> --->8---
>
> While time(NULL) is stable, getpid() is incrementing by one. As you may
> expect the OR'ed value is oscillating and the values almost the same. So
> calling gen_eth_addr three times within the same second you will get two
> time the same MAC.
>
>> So this patch remove the getpid(), just use the time(0) as the seed.
> So let's see the effect of your change ...
>
> The output of gen_eth_addr at the current ToT:
>
> % RUN=0; while [ $RUN -lt 10000 ]; do
> /tmp/build-uboot-test/tools/gen_eth_addr; RUN=$(($RUN+1)); done | sort |
> uniq | wc -l
> 254
>
> With your change applied:
>
> % RUN=0; while [ $RUN -lt 10000 ]; do
> /tmp/build-uboot-test/tools/gen_eth_addr; RUN=$(($RUN+1)); done | sort |
> uniq | wc -l
> 10
>
> Another approach would be to change the algorithm (OR the values) here.
> A short test showed that using XOR could be a solution:
>
> ---8<---
> time: 1442389450; pid: 11632; time ^ pid: 1442394298;
> time: 1442389450; pid: 11633; time ^ pid: 1442394299;
> time: 1442389450; pid: 11634; time ^ pid: 1442394296;
> time: 1442389450; pid: 11635; time ^ pid: 1442394297;
> time: 1442389450; pid: 11636; time ^ pid: 1442394302;
> time: 1442389450; pid: 11637; time ^ pid: 1442394303;
> time: 1442389450; pid: 11638; time ^ pid: 1442394300;
> time: 1442389450; pid: 11639; time ^ pid: 1442394301;
> time: 1442389450; pid: 11640; time ^ pid: 1442394290;
> time: 1442389450; pid: 11641; time ^ pid: 1442394291;
> time: 1442389450; pid: 11642; time ^ pid: 1442394288;
> time: 1442389450; pid: 11643; time ^ pid: 1442394289;
> --->8---
>
> It is the same input but none of the outputs is the same value.
>
> The XOR approach applied to gen_eth_addr:
>
> % RUN=0; while [ $RUN -lt 10000 ]; do
> /tmp/build-uboot-test/tools/gen_eth_addr; RUN=$(($RUN+1)); done | sort |
> uniq | wc -l
> 9988

How about using ADD ?
When I change to 'time + (pid << 8)', the result is better. And the pid 
change will has more impact in time range.

➜  tools  time RUN=0; while [ $RUN -lt 10000 ]; do
./gen_eth_addr_add_shift 1; RUN=$(($RUN+1)); done | sort | uniq | wc -l
10000

The disadvantage is it might cost more time.

Best Regards,
Josh Wu

> Andreas
>
>> Signed-off-by: Josh Wu <josh.wu at atmel.com>
>> ---
>>
>>   tools/gen_eth_addr.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tools/gen_eth_addr.c b/tools/gen_eth_addr.c
>> index bf9d935..53b023a 100644
>> --- a/tools/gen_eth_addr.c
>> +++ b/tools/gen_eth_addr.c
>> @@ -15,7 +15,7 @@ main(int argc, char *argv[])
>>   {
>>       unsigned long ethaddr_low, ethaddr_high;
>>   
>> -    srand(time(0) | getpid());
>> +    srand(time(0));
>>   
>>       /*
>>        * setting the 2nd LSB in the most significant byte of
>>