| Message ID | 419e5c6e-b2ef-44c2-a4c1-bb25c50fcb57@mary.at.omicron.at |
|---|---|
| State | Rejected |
| Delegated to: | Wolfgang Denk |
| Headers | show |
Dear Christian Riesch, In message <419e5c6e-b2ef-44c2-a4c1-bb25c50fcb57@mary.at.omicron.at> you wrote: > Currently eth_random_enetaddr() seeds the random number generator with > get_timer(0). Some boards might want to use other sources for the seed, > therefore move the call of srand() to the board specific code. > > Signed-off-by: Christian Riesch <christian.riesch@omicron.at> > Cc: Michael Walle <michael@walle.cc> > Cc: Joe Hershberger <joe.hershberger@gmail.com> I don't like this change. What exactly is wrong with using the timer here? It is probably much more random that the (so-called) "un-initialized" memory you suggest to use instead. If there is really need to use another inital valu, only this should be fixed - but the srand() call itself should remain as is. You cannot do this. Best regards, Wolfgang Denk
Hello Wolfgang, Thank you for your comments. On Tuesday, January 8, 2013, Wolfgang Denk wrote: > Dear Christian Riesch, > > In message <419e5c6e-b2ef-44c2-a4c1-bb25c50fcb57@mary.at.omicron.at<javascript:;>> > you wrote: > > Currently eth_random_enetaddr() seeds the random number generator with > > get_timer(0). Some boards might want to use other sources for the seed, > > therefore move the call of srand() to the board specific code. > > > > Signed-off-by: Christian Riesch <christian.riesch@omicron.at<javascript:;> > > > > Cc: Michael Walle <michael@walle.cc> > > Cc: Joe Hershberger <joe.hershberger@gmail.com <javascript:;>> > > I don't like this change. What exactly is wrong with using the timer > here? It is probably much more random that the (so-called) > "un-initialized" memory you suggest to use instead. On the AM1808 SoC the counter is reset to zero on power up. So using it to generate random numbers in code that is called interactively by the user is fine and will yield a random MAC address, but in my case I will get the same MAC address on each board at each power up. > If there is really need to use another inital valu, only this should > be fixed - but the srand() call itself should remain as is. > > > For other boards it may be ok to use a counter, and for some there may be no SRAM or it may be already overwritten, e.g by the SPL... Therefore I am not changing this for all boards, but make it board specific. Regards, Christian > You cannot do this. > > Best regards, > > Wolfgang Denk > > -- > DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de<javascript:;> > You can observe a lot just by watchin'. - Yogi Berra > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de <javascript:;> > http://lists.denx.de/mailman/listinfo/u-boot >
Dear Christian Riesch, In message <CABkLObrL2BhKmjh9NCcv1UQ-cbfJisUidK8JoE8DSAOEkqiZ1g@mail.gmail.com> you wrote: > > > I don't like this change. What exactly is wrong with using the timer > > here? It is probably much more random that the (so-called) > > "un-initialized" memory you suggest to use instead. > > On the AM1808 SoC the counter is reset to zero on power up. So using it to > generate random numbers in code that is called interactively by the user is > fine and will yield a random MAC address, but in my case I will get the > same MAC address on each board at each power up. I think the whole concept of using random MAC adresses is broken. You should consider thinking about fixing the root cause of your problem. > > If there is really need to use another inital valu, only this should > > be fixed - but the srand() call itself should remain as is. > > For other boards it may be ok to use a counter, and for some there may be > no SRAM or it may be already overwritten, e.g by the SPL... Therefore I am > not changing this for all boards, but make it board specific. I don't want to see any board specific code here. If using the timer is not good enough, then pass a weak funtion as argument to srand() which defaults to using the timer as we do now, and which can be redefined in board specific code. Other than this, the common code should not be changed. Best regards, Wolfgang Denk
diff --git a/board/buffalo/lsxl/lsxl.c b/board/buffalo/lsxl/lsxl.c index 57776fb..b7eb0dc 100644 --- a/board/buffalo/lsxl/lsxl.c +++ b/board/buffalo/lsxl/lsxl.c @@ -248,6 +248,7 @@ static void rescue_mode(void) printf("Entering rescue mode..\n"); #ifdef CONFIG_RANDOM_MACADDR if (!eth_getenv_enetaddr("ethaddr", enetaddr)) { + srand(get_timer(0)); eth_random_enetaddr(enetaddr); if (eth_setenv_enetaddr("ethaddr", enetaddr)) { printf("Failed to set ethernet address\n"); diff --git a/include/net.h b/include/net.h index 970d4d1..5fc3693 100644 --- a/include/net.h +++ b/include/net.h @@ -141,6 +141,9 @@ extern int eth_getenv_enetaddr_by_index(const char *base_name, int index, * * In these cases, we generate a random locally administered ethernet address. * + * Remember to seed the random number generator with srand() before calling + * this functon. + * * Args: * enetaddr - returns 6 byte hardware address */ diff --git a/net/eth.c b/net/eth.c index 321d5b1..dc4cc20 100644 --- a/net/eth.c +++ b/net/eth.c @@ -84,8 +84,6 @@ void eth_random_enetaddr(uchar *enetaddr) { uint32_t rval; - srand(get_timer(0)); - rval = rand(); enetaddr[0] = rval & 0xff; enetaddr[1] = (rval >> 8) & 0xff;
Currently eth_random_enetaddr() seeds the random number generator with get_timer(0). Some boards might want to use other sources for the seed, therefore move the call of srand() to the board specific code. Signed-off-by: Christian Riesch <christian.riesch@omicron.at> Cc: Michael Walle <michael@walle.cc> Cc: Joe Hershberger <joe.hershberger@gmail.com> --- board/buffalo/lsxl/lsxl.c | 1 + include/net.h | 3 +++ net/eth.c | 2 -- 3 files changed, 4 insertions(+), 2 deletions(-)