[U-Boot] [PATCH v7 2/4] net: use common rand()/srand() functions

[Michael Walle]

Hi Joe,

Am Donnerstag 31 Mai 2012, 21:04:03 schrieb Joe Hershberger:
> On Thu, May 31, 2012 at 1:12 PM, Michael Walle <michael at walle.cc> wrote:
> > Replace rand() with the functions from lib/. The link-local network code
> > stores its own seed, derived from the MAC address. Thus making it
> > independent from calls to srand() in other modules.
> > 
> > Signed-off-by: Michael Walle <michael at walle.cc>
> > Cc: Joe Hershberger <joe.hershberger at ni.com>
> > ---
> >  include/configs/ETX094.h          |    1 +
> >  include/configs/MERGERBOX.h       |    1 +
> >  include/configs/MVBC_P.h          |    1 +
> >  include/configs/MVBLM7.h          |    1 +
> >  include/configs/MVSMR.h           |    1 +
> >  include/configs/bfin_adi_common.h |    1 +
> >  include/configs/sacsng.h          |    1 +
> >  net/Makefile                      |    2 -
> >  net/link_local.c                  |    7 ++--
> >  net/net_rand.c                    |   68
> > ------------------------------------- net/net_rand.h                  
> >  |   32 +++++++++++++----
> >  11 files changed, 36 insertions(+), 80 deletions(-)
> >  delete mode 100644 net/net_rand.c
> > 
> > diff --git a/include/configs/ETX094.h b/include/configs/ETX094.h
> > index c427093..aee9915 100644
> > --- a/include/configs/ETX094.h
> > +++ b/include/configs/ETX094.h
> > @@ -54,6 +54,7 @@
> > 
> >  #define        CONFIG_FLASH_16BIT              /* for board with 16bit
> > wide flash      */ #undef SB_ETX094                       /* only for
> > SB-Board with 16MB SDRAM    */ +#define CONFIG_RAND
> >  #define        CONFIG_BOOTP_RANDOM_DELAY       /* graceful BOOTP
> > recovery mode         */
> 
> It would be great if this could be implied instead of explicit... see
> below.
> 
> >  #define CONFIG_ETHADDR 08:00:06:00:00:00
> > diff --git a/net/Makefile b/net/Makefile
> > index 5264687..e7764ce 100644
> > --- a/net/Makefile
> > +++ b/net/Makefile
> > @@ -34,8 +34,6 @@ COBJS-$(CONFIG_CMD_DNS)  += dns.o
> >  COBJS-$(CONFIG_CMD_NET)  += eth.o
> >  COBJS-$(CONFIG_CMD_LINK_LOCAL) += link_local.o
> >  COBJS-$(CONFIG_CMD_NET)  += net.o
> > -COBJS-$(CONFIG_BOOTP_RANDOM_DELAY) += net_rand.o
> > -COBJS-$(CONFIG_CMD_LINK_LOCAL) += net_rand.o
> 
> In the Makefile for lib/, mimic the implementation that you are
> removing here.  This way each user of CMD_LINK_LOCAL and
> BOOTP_RANDOM_DELAY aren't forced to also define RAND.  You can still
> also keep CONFIG_RAND for cases like your board where all you want is
> RAND.  Don't forget that you need to add a COBJS := $(sort $(COBJS-y))
> to the Makefile like this one in case more than one of the options is
> enabled for the same board.

Yeah i saw that. But I don't know what to make of it. Eg. (1) no module in 
lib/ does it that way, (2) if more and more features use a module, the 
makefile will be more and more cluttered, (3) that applies for the guards in 
the header, too and (4) you have to take care of the features in two places 
(header+makefile).

Nevertheless, i'll do it you're way.

> >  COBJS-$(CONFIG_CMD_NFS)  += nfs.o
> >  COBJS-$(CONFIG_CMD_PING) += ping.o
> >  COBJS-$(CONFIG_CMD_RARP) += rarp.o
> > diff --git a/net/net_rand.h b/net/net_rand.h
> > index c98db64..62de375 100644
> > --- a/net/net_rand.h
> > +++ b/net/net_rand.h
> > @@ -9,18 +9,36 @@
> >  #ifndef __NET_RAND_H__
> >  #define __NET_RAND_H__
> > 
> > -#define RAND_MAX 0xffffffff
> > +#include <common.h>
> > 
> >  /*
> > - * Seed the random number generator using the eth0 MAC address
> > + * Return a seed for the PRNG derived from the timer and the eth0 MAC
> > address. */
> > -void srand_mac(void);
> > +static inline unsigned int seed_mac(void)
> > +{
> > +       unsigned char enetaddr[6];
> > +       unsigned int seed;
> > +
> > +       /* get our mac */
> > +       eth_getenv_enetaddr("ethaddr", enetaddr);
> > +
> > +       seed = get_timer(0);
> 
> Do not include the timer here.  The seed should always start the same
> for a given MAC address.
i'll fix it. Just for the reference, i guess joe is referring to this section 
from rfc3927:

  If the host has access to persistent information that is different for
  each host, such as its IEEE 802 MAC address, then the pseudo-random
  number generator SHOULD be seeded using a value derived from this
  information.  This means that even without using any other persistent
  storage, a host will usually select the same IPv4 Link-Local address
  each time it is booted, which can be convenient for debugging and
  other operational reasons.



-- 
Michael