diff mbox

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

Message ID 1337795897-19231-3-git-send-email-joe.hershberger@ni.com
State Accepted
Commit eafc8db0e35275330f43a4cf7b7ae8aba71c9728
Delegated to: Joe Hershberger
Headers show

Commit Message

Joe Hershberger May 23, 2012, 5:57 p.m. UTC 
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@ni.com>
Cc: Joe Hershberger <joe.hershberger@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

Comments

Michael Walle May 28, 2012, 9:53 p.m. UTC | #1 
Am Mittwoch 23 Mai 2012, 19:57:58 schrieb Joe Hershberger:
> t_ran
Michael Walle May 28, 2012, 10:03 p.m. UTC | #2 
[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@ni.com>
> Cc: Joe Hershberger <joe.hershberger@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/bootp.h b/net/bootp.h
> index ce73734..bf4e875 100644
> --- a/net/bootp.h
> +++ b/net/bootp.h
> @@ -63,9 +63,6 @@ struct Bootp_t {
>  extern ulong	BootpID;		/* ID of cur BOOTP request	*/
>  extern char	BootFile[128];		/* Boot file name		*/
>  extern int	BootpTry;
> -#ifdef CONFIG_BOOTP_RANDOM_DELAY
> -extern ulong	seed1, seed2;		/* seed for random BOOTP delay	*/
> -#endif
> 
> 
>  /* Send a BOOTP request */
> 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@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?

> +	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? this conflicts with my patch (lib/rand.c) i'm trying to 
put into upstream.

> 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 ?

Joe, if that is ok, i'll put that into the next version of my "lsxl: add 
support for lschlv2 and lsxhl" patch series.
Joe Hershberger May 29, 2012, 6:08 p.m. UTC | #3 
Hi Michael,

On Mon, May 28, 2012 at 5:03 PM, Michael Walle <michael@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@ni.com>
>> Cc: Joe Hershberger <joe.hershberger@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@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
Michael Walle May 29, 2012, 6:23 p.m. UTC | #4 
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@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@ni.com>
> >> Cc: Joe Hershberger <joe.hershberger@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@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);
}
Joe Hershberger May 29, 2012, 9:08 p.m. UTC | #5 
Hi Michael,

On Tue, May 29, 2012 at 1:23 PM, Michael Walle <michael@walle.cc> wrote:
> Hi Joe,
>
> Am Dienstag 29 Mai 2012, 20:08:26 schrieb Joe Hershberger:
>> 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);
> }

I'm not sure why you are only seeding with the last 4 bytes of the
MAC.  The original algorithm used all 6 (in its way).  You also
haven't addressed the issue of isolating the link-local algorithm from
other non-MAC-seeded random numbers.  The most naive way around it
would be to rename the rand() in net to rand_mac() or something like
that and not attempt to combine them.

-Joe
Michael Walle May 29, 2012, 10:06 p.m. UTC | #6 
Hi Joe,

Am Dienstag 29 Mai 2012, 23:08:23 schrieb Joe Hershberger:
> Hi Michael,
> 
> On Tue, May 29, 2012 at 1:23 PM, Michael Walle <michael@walle.cc> wrote:
> > Hi Joe,
> > 
> > Am Dienstag 29 Mai 2012, 20:08:26 schrieb Joe Hershberger:
> >> 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);
> > }
> 
> I'm not sure why you are only seeding with the last 4 bytes of the
> MAC.  The original algorithm used all 6 (in its way).
yeah i'll fix that.


> You also haven't addressed the issue of isolating the link-local
> algorithm from other non-MAC-seeded random numbers.
Ah now i get it. But which other code may run and seed the NG with the timer 
only, while we are in the link local netloop? Shouldn't it be safe to first 
call srand(mac+timer) and then rand() multiple times? No other code can be run 
unless NetLoop(LINKLOCAL) returns, right?


> The most naive way around it
> would be to rename the rand() in net to rand_mac() or something like
> that and not attempt to combine them.
i think we should focus on combining not reinventing the wheel multiple times.
Joe Hershberger May 29, 2012, 10:40 p.m. UTC | #7 
Hi Michael,

On Tue, May 29, 2012 at 5:06 PM, Michael Walle <michael@walle.cc> wrote:
>
> Hi Joe,
>
> Am Dienstag 29 Mai 2012, 23:08:23 schrieb Joe Hershberger:
>> Hi Michael,
>>
>> On Tue, May 29, 2012 at 1:23 PM, Michael Walle <michael@walle.cc> wrote:
>> > Hi Joe,
>> >
>> > Am Dienstag 29 Mai 2012, 20:08:26 schrieb Joe Hershberger:
>> >> 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);
>> > }
>>
>> I'm not sure why you are only seeding with the last 4 bytes of the
>> MAC.  The original algorithm used all 6 (in its way).
> yeah i'll fix that.
>
>
>> You also haven't addressed the issue of isolating the link-local
>> algorithm from other non-MAC-seeded random numbers.
> Ah now i get it. But which other code may run and seed the NG with the timer
> only, while we are in the link local netloop? Shouldn't it be safe to first
> call srand(mac+timer) and then rand() multiple times? No other code can be run
> unless NetLoop(LINKLOCAL) returns, right?

It needs to run multiple times, yes.  But it will also process any
packets that are received when executing any other net command later.
The state machine will keep running.  There can obviously be other
commands that run in between.

Perhaps we can separate the state for the PRNG between the 2 cases but
share the code.

>> The most naive way around it
>> would be to rename the rand() in net to rand_mac() or something like
>> that and not attempt to combine them.
> i think we should focus on combining not reinventing the wheel multiple times.

Agreed, but we have to solve the problem above.

Thanks,
-Joe
Michael Walle May 29, 2012, 10:54 p.m. UTC | #8 
Hi Joe,

Am Mittwoch 30 Mai 2012, 00:40:18 schrieb Joe Hershberger:
> > Ah now i get it. But which other code may run and seed the NG with the
> > timer only, while we are in the link local netloop? Shouldn't it be safe
> > to first call srand(mac+timer) and then rand() multiple times? No other
> > code can be run unless NetLoop(LINKLOCAL) returns, right?
> 
> It needs to run multiple times, yes.  But it will also process any
> packets that are received when executing any other net command later.
> The state machine will keep running.  There can obviously be other
> commands that run in between.
i guess i have to study the netloop a bit more ;)

> 
> Perhaps we can separate the state for the PRNG between the 2 cases but
> share the code.
yeah i was thinking of rand_r().

> >> The most naive way around it
> >> would be to rename the rand() in net to rand_mac() or something like
> >> that and not attempt to combine them.
> > 
> > i think we should focus on combining not reinventing the wheel multiple
> > times.
> 
> Agreed, but we have to solve the problem above.
fully argreeing ;)
diff mbox

Patch

diff --git a/net/Makefile b/net/Makefile
index 0544f6b..5901046 100644
--- a/net/Makefile
+++ b/net/Makefile
@@ -31,6 +31,7 @@  COBJS-$(CONFIG_CMD_NET)  += bootp.o
 COBJS-$(CONFIG_CMD_DNS)  += dns.o
 COBJS-$(CONFIG_CMD_NET)  += eth.o
 COBJS-$(CONFIG_CMD_NET)  += net.o
+COBJS-$(CONFIG_BOOTP_RANDOM_DELAY) += net_rand.o
 COBJS-$(CONFIG_CMD_NFS)  += nfs.o
 COBJS-$(CONFIG_CMD_RARP) += rarp.o
 COBJS-$(CONFIG_CMD_SNTP) += sntp.o
diff --git a/net/bootp.c b/net/bootp.c
index d0a7da2..0185e56 100644
--- a/net/bootp.c
+++ b/net/bootp.c
@@ -12,6 +12,7 @@ 
 #include <command.h>
 #include <net.h>
 #include "bootp.h"
+#include "net_rand.h"
 #include "tftp.h"
 #include "nfs.h"
 #ifdef CONFIG_STATUS_LED
@@ -37,9 +38,6 @@ 
 
 ulong		BootpID;
 int		BootpTry;
-#ifdef CONFIG_BOOTP_RANDOM_DELAY
-ulong		seed1, seed2;
-#endif
 
 #if defined(CONFIG_CMD_DHCP)
 dhcp_state_t dhcp_state = INIT;
@@ -584,6 +582,9 @@  BootpRequest(void)
 	uchar *pkt, *iphdr;
 	struct Bootp_t *bp;
 	int ext_len, pktlen, iplen;
+#ifdef CONFIG_BOOTP_RANDOM_DELAY
+	ulong i, rand_ms;
+#endif
 
 	bootstage_mark_name(BOOTSTAGE_ID_BOOTP_START, "bootp_start");
 #if defined(CONFIG_CMD_DHCP)
@@ -591,60 +592,16 @@  BootpRequest(void)
 #endif
 
 #ifdef CONFIG_BOOTP_RANDOM_DELAY		/* Random BOOTP delay */
-	unsigned char bi_enetaddr[6];
-	int   reg;
-	ulong tst1, tst2, sum, m_mask, m_value = 0;
-
-	if (BootpTry == 0) {
-		/* get our mac */
-		eth_getenv_enetaddr("ethaddr", bi_enetaddr);
-
-		debug("BootpRequest => Our Mac: ");
-		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;
-		}
+	if (BootpTry == 0)
+		srand_mac();
 
-		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;
-	}
-
-	/* Random Number Generator */
-	for (reg = 0; reg <= 0; reg++) {
-		sum = seed1 + seed2;
-		if (sum < seed1 || sum < seed2)
-			sum++;
-		seed2 = seed1;
-		seed1 = sum;
-
-		if (BootpTry <= 2) {	/* Start with max 1024 * 1ms */
-			sum = sum >> (22-BootpTry);
-		} else {	/*After 3rd BOOTP request max 8192 * 1ms */
-			sum = sum >> 19;
-		}
-	}
+	if (BootpTry <= 2)	/* Start with max 1024 * 1ms */
+		rand_ms = rand() >> (22 - BootpTry);
+	else		/* After 3rd BOOTP request max 8192 * 1ms */
+		rand_ms = rand() >> 19;
 
-	printf("Random delay: %ld ms...\n", sum);
-	for (reg = 0; reg < sum; reg++)
+	printf("Random delay: %ld ms...\n", rand_ms);
+	for (i = 0; i < rand_ms; i++)
 		udelay(1000); /*Wait 1ms*/
 
 #endif	/* CONFIG_BOOTP_RANDOM_DELAY */
diff --git a/net/bootp.h b/net/bootp.h
index ce73734..bf4e875 100644
--- a/net/bootp.h
+++ b/net/bootp.h
@@ -63,9 +63,6 @@  struct Bootp_t {
 extern ulong	BootpID;		/* ID of cur BOOTP request	*/
 extern char	BootFile[128];		/* Boot file name		*/
 extern int	BootpTry;
-#ifdef CONFIG_BOOTP_RANDOM_DELAY
-extern ulong	seed1, seed2;		/* seed for random BOOTP delay	*/
-#endif
 
 
 /* Send a BOOTP request */
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@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: ");
+	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;
+}
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__ */