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

[Michael Walle]

Hi Joe,

Am Dienstag 29 Mai 2012, 20:08:26 schrieb 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?

I'm in a hurry, short answer for now:
I thought of sth like this:

static inline void srand_mac(void)
{
        unsigned char enetaddr[6];
        unsigned int seed;

        /* get our mac */
        eth_getenv_enetaddr("ethaddr", enetaddr);

        seed = enetaddr[5];
        seed |= enetaddr[4] << 8;
        seed |= enetaddr[3] << 16;
        seed |= enetaddr[2] << 24;

        srand(seed);
}

-- 
Michael