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

[Andreas Bießmann]

Hi Josh,

On 09/16/2015 11:12 AM, Josh Wu wrote:
> On 9/16/2015 4:23 PM, Andreas Bießmann wrote:
>> 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

I'm fine with that too. All I'd like to say with my answer was that
adding getpid() was for some reason and removing it is worse than having
it OR'ed.

> The disadvantage is it might cost more time.

I don't think this count's here. This tool would be used to generate a
temporary MAC (or more than one) for testing purposes. So getting a
bunch of MAC's can last some time, but should provide unique ones.

Andreas