[U-Boot] [PATCH v3 02/21] net: Move MAC-seeded rand out of bootp.c

[Joe Hershberger]

Hi Michael,

On Mon, May 28, 2012 at 5:03 PM, Michael Walle <michael at walle.cc> wrote:
> [sorry for my first mail.. wasn't intented to be send]
>
>
> Sorry for being too late on this.
>
> Am Mittwoch 23 Mai 2012, 19:57:58 schrieb Joe Hershberger:
>> Make the MAC-seeded random number generator available to /net in
>> general.  MAC-seeded rand will be needed by link-local as well, so
>> give it an interface.
>>
>> Signed-off-by: Joe Hershberger <joe.hershberger at ni.com>
>> Cc: Joe Hershberger <joe.hershberger at gmail.com>
>> ---
>> Changes for v3:
>>   - Lowercase hex
>>   - Add blank line
>>   - Always include header, even when it isn't needed
>>   - Add function comments
>>
>>  net/Makefile   |    1 +
>>  net/bootp.c    |   67
>> ++++++++++--------------------------------------------- net/bootp.h    |
>>  3 --
>>  net/net_rand.c |   68
>> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ net/net_rand.h |
>>  26 +++++++++++++++++++++
>>  5 files changed, 107 insertions(+), 58 deletions(-)
>>  create mode 100644 net/net_rand.c
>>  create mode 100644 net/net_rand.h
>>
> [..]
>
>> diff --git a/net/net_rand.c b/net/net_rand.c
>> new file mode 100644
>> index 0000000..5387aba
>> --- /dev/null
>> +++ b/net/net_rand.c
>> @@ -0,0 +1,68 @@
>> +/*
>> + *   Based on LiMon - BOOTP.
>> + *
>> + *   Copyright 1994, 1995, 2000 Neil Russell.
>> + *   (See License)
>> + *   Copyright 2000 Roland Borde
>> + *   Copyright 2000 Paolo Scaffardi
>> + *   Copyright 2000-2004 Wolfgang Denk, wd at denx.de
>> + */
>> +
>> +#include <common.h>
>> +#include <net.h>
>> +#include "net_rand.h"
>> +
>> +static ulong seed1, seed2;
>> +
>> +void srand_mac(void)
>> +{
>> +     ulong tst1, tst2, m_mask;
>> +     ulong m_value = 0;
>> +     int reg;
>> +     unsigned char bi_enetaddr[6];
>> +
>> +     /* get our mac */
>> +     eth_getenv_enetaddr("ethaddr", bi_enetaddr);
>> +
>> +     debug("BootpRequest => Our Mac: ");
> mh? this shouldn't be here anymore should it?

Yes... It's not very appropriate any longer.

>> +     for (reg = 0; reg < 6; reg++)
>> +             debug("%x%c", bi_enetaddr[reg], reg == 5 ? '\n' : ':');
>> +
>> +     /* Mac-Manipulation 2 get seed1 */
>> +     tst1 = 0;
>> +     tst2 = 0;
>> +     for (reg = 2; reg < 6; reg++) {
>> +             tst1 = tst1 << 8;
>> +             tst1 = tst1 | bi_enetaddr[reg];
>> +     }
>> +     for (reg = 0; reg < 2; reg++) {
>> +             tst2 = tst2 | bi_enetaddr[reg];
>> +             tst2 = tst2 << 8;
>> +     }
>> +
>> +     seed1 = tst1^tst2;
>> +
>> +     /* Mirror seed1*/
>> +     m_mask = 0x1;
>> +     for (reg = 1; reg <= 32; reg++) {
>> +             m_value |= (m_mask & seed1);
>> +             seed1 = seed1 >> 1;
>> +             m_value = m_value << 1;
>> +     }
>> +     seed1 = m_value;
>> +     seed2 = 0xb78d0945;
>> +}
>> +
>> +unsigned long rand(void)
>> +{
>> +     ulong sum;
>> +
>> +     /* Random Number Generator */
>> +     sum = seed1 + seed2;
>> +     if (sum < seed1 || sum < seed2)
>> +             sum++;
>> +     seed2 = seed1;
>> +     seed1 = sum;
>> +
>> +     return sum;
>> +}
>
> is this really random?

It was the implementation used for bootp delays.  It is some form of
PRNG... just not the same as what you've implemented.

> this conflicts with my patch (lib/rand.c) i'm trying to
> put into upstream.

True.

>> diff --git a/net/net_rand.h b/net/net_rand.h
>> new file mode 100644
>> index 0000000..c98db64
>> --- /dev/null
>> +++ b/net/net_rand.h
>> @@ -0,0 +1,26 @@
>> +/*
>> + *   Copied from LiMon - BOOTP.
>> + *
>> + *   Copyright 1994, 1995, 2000 Neil Russell.
>> + *   (See License)
>> + *   Copyright 2000 Paolo Scaffardi
>> + */
>> +
>> +#ifndef __NET_RAND_H__
>> +#define __NET_RAND_H__
>> +
>> +#define RAND_MAX 0xffffffff
>> +
>> +/*
>> + * Seed the random number generator using the eth0 MAC address
>> + */
>> +void srand_mac(void);
>> +
>> +/*
>> + * Get a random number (after seeding with MAC address)
>> + *
>> + * @return random number
>> + */
>> +unsigned long rand(void);
>> +
>> +#endif /* __NET_RAND_H__ */
>
> if this is unconditionally included, my patch fails because there are multiple
> declarations of rand().
>
> Can we unify this with my patch, which is based on a paper about PRNGs? Eg.
> use the rand()/srand() from my patch and create an srand_mac() inline function
> which uses srand() from lib/rand.c ?

If you can verify that the functionality of the
CONFIG_BOOTP_RANDOM_DELAY and CONFIG_CMD_LINK_LOCAL are uneffected,
then I'm OK with it.

One thing to note is that the link-local implementation needs to use a
MAC seeded random number.  That means we can't have other things
coming in and seeding it with a time.  It is nice that it is separate
for that reason.  Can you solve that and integrate it with your PRNG?

> Joe, if that is ok, i'll put that into the next version of my "lsxl: add
> support for lschlv2 and lsxhl" patch series.

I'm certainly willing to look at what you come up with.

Thanks,
-Joe