| Message ID | 1338487965-31791-3-git-send-email-michael@walle.cc |
|---|---|
| State | Superseded |
| Delegated to: | Prafulla Wadaskar |
| Headers | show |
Hi Michael, On Thu, May 31, 2012 at 1:12 PM, Michael Walle <michael@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@walle.cc> > Cc: Joe Hershberger <joe.hershberger@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. > 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. > + seed ^= enetaddr[5]; > + seed ^= enetaddr[4] << 8; > + seed ^= enetaddr[3] << 16; > + seed ^= enetaddr[2] << 24; > + seed ^= enetaddr[1]; > + seed ^= enetaddr[0] << 8; > + > + return seed; > +} Thanks, -Joe
On Thu, May 31, 2012 at 2:04 PM, Joe Hershberger <joe.hershberger@gmail.com> wrote: > Hi Michael, > > On Thu, May 31, 2012 at 1:12 PM, Michael Walle <michael@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@walle.cc> >> Cc: Joe Hershberger <joe.hershberger@ni.com> >> --- >> #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. Actually you could also add CONFIG_RANDOM_MACADDR in the same way. >> COBJS-$(CONFIG_CMD_NFS) += nfs.o >> COBJS-$(CONFIG_CMD_PING) += ping.o >> COBJS-$(CONFIG_CMD_RARP) += rarp.o Thanks, -Joe
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@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@walle.cc> > > Cc: Joe Hershberger <joe.hershberger@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.
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 */ #define CONFIG_ETHADDR 08:00:06:00:00:00 diff --git a/include/configs/MERGERBOX.h b/include/configs/MERGERBOX.h index 8176916..c2b962f 100644 --- a/include/configs/MERGERBOX.h +++ b/include/configs/MERGERBOX.h @@ -315,6 +315,7 @@ /* * BOOTP options */ +#define CONFIG_RAND #define CONFIG_BOOTP_BOOTFILESIZE #define CONFIG_BOOTP_BOOTPATH #define CONFIG_BOOTP_GATEWAY diff --git a/include/configs/MVBC_P.h b/include/configs/MVBC_P.h index ade4893..d38da0b 100644 --- a/include/configs/MVBC_P.h +++ b/include/configs/MVBC_P.h @@ -110,6 +110,7 @@ #undef CONFIG_WATCHDOG +#define CONFIG_RAND #define CONFIG_BOOTP_VENDOREX #define CONFIG_BOOTP_SUBNETMASK #define CONFIG_BOOTP_GATEWAY diff --git a/include/configs/MVBLM7.h b/include/configs/MVBLM7.h index 8b20f72..7c8c808 100644 --- a/include/configs/MVBLM7.h +++ b/include/configs/MVBLM7.h @@ -232,6 +232,7 @@ #define CONFIG_ETHPRIME "TSEC0" +#define CONFIG_RAND #define CONFIG_BOOTP_VENDOREX #define CONFIG_BOOTP_SUBNETMASK #define CONFIG_BOOTP_GATEWAY diff --git a/include/configs/MVSMR.h b/include/configs/MVSMR.h index f94ad5c..6f10907 100644 --- a/include/configs/MVSMR.h +++ b/include/configs/MVSMR.h @@ -98,6 +98,7 @@ #define CONFIG_CMD_PING #define CONFIG_CMD_SDRAM +#define CONFIG_RAND #define CONFIG_BOOTP_BOOTFILESIZE #define CONFIG_BOOTP_BOOTPATH #define CONFIG_BOOTP_DNS diff --git a/include/configs/bfin_adi_common.h b/include/configs/bfin_adi_common.h index 3fbf5c6..0f4c2d8 100644 --- a/include/configs/bfin_adi_common.h +++ b/include/configs/bfin_adi_common.h @@ -11,6 +11,7 @@ #ifndef _CONFIG_CMD_DEFAULT_H # include <config_cmd_default.h> # if ADI_CMDS_NETWORK +# define CONFIG_RAND # define CONFIG_CMD_DHCP # define CONFIG_BOOTP_SUBNETMASK # define CONFIG_BOOTP_GATEWAY diff --git a/include/configs/sacsng.h b/include/configs/sacsng.h index 43036b2..d604de3 100644 --- a/include/configs/sacsng.h +++ b/include/configs/sacsng.h @@ -475,6 +475,7 @@ "bootm" #endif /* CONFIG_BOOT_ROOT_NFS */ +#define CONFIG_RAND #define CONFIG_BOOTP_RANDOM_DELAY /* Randomize the BOOTP retry delay */ /* 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 COBJS-$(CONFIG_CMD_NFS) += nfs.o COBJS-$(CONFIG_CMD_PING) += ping.o COBJS-$(CONFIG_CMD_RARP) += rarp.o diff --git a/net/link_local.c b/net/link_local.c index 3362863..582d011 100644 --- a/net/link_local.c +++ b/net/link_local.c @@ -56,6 +56,7 @@ static unsigned conflicts; static unsigned nprobes; static unsigned nclaims; static int ready; +static unsigned int seed; static void link_local_timeout(void); @@ -68,7 +69,7 @@ static IPaddr_t pick(void) unsigned tmp; do { - tmp = rand() & IN_CLASSB_HOST; + tmp = rand_r(&seed) & IN_CLASSB_HOST; } while (tmp > (IN_CLASSB_HOST - 0x0200)); return (IPaddr_t) htonl((LINKLOCAL_ADDR + 0x0100) + tmp); } @@ -78,7 +79,7 @@ static IPaddr_t pick(void) */ static inline unsigned random_delay_ms(unsigned secs) { - return rand() % (secs * 1000); + return rand_r(&seed) % (secs * 1000); } static void configure_wait(void) @@ -109,7 +110,7 @@ void link_local_start(void) } NetOurSubnetMask = IN_CLASSB_NET; - srand_mac(); + seed = seed_mac(); if (ip == 0) ip = pick(); diff --git a/net/net_rand.c b/net/net_rand.c deleted file mode 100644 index 5387aba..0000000 --- a/net/net_rand.c +++ /dev/null @@ -1,68 +0,0 @@ -/* - * 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 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); + seed ^= enetaddr[5]; + seed ^= enetaddr[4] << 8; + seed ^= enetaddr[3] << 16; + seed ^= enetaddr[2] << 24; + seed ^= enetaddr[1]; + seed ^= enetaddr[0] << 8; + + return seed; +} /* - * Get a random number (after seeding with MAC address) - * - * @return random number + * Seed the random number generator using the timer and the eth0 MAC address. */ -unsigned long rand(void); +static inline void srand_mac(void) +{ + srand(seed_mac()); +} #endif /* __NET_RAND_H__ */
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@walle.cc> Cc: Joe Hershberger <joe.hershberger@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