[U-Boot] [PATCH v4 0/3] net: helper functions

[U-Boot] [PATCH v4 0/3] net: helper functions

[Michael Walle]

This patchset is a split-off from my patch series "Kirkwood: add lschlv2
and lsxhl board support".

Changes:
v4:
 - typo fixes (thanks Mike)
 - seed all 46bits of the generated ethernet address (suggested by Mike)
v3, v2:
 [contained only lsxhl board changes]

Michael Walle (3):
  lib: add rand() function
  net: add helper to generate random mac address
  net: add eth_setenv_enetaddr_by_index()

 include/common.h |    4 ++++
 include/net.h    |   27 +++++++++++++++++++++++++++
 lib/Makefile     |    1 +
 lib/rand.c       |   43 +++++++++++++++++++++++++++++++++++++++++++
 net/eth.c        |   28 ++++++++++++++++++++++++++++
 5 files changed, 103 insertions(+), 0 deletions(-)
 create mode 100644 lib/rand.c

-- 
1.7.2.5

[U-Boot] [PATCH v4 1/3] lib: add rand() function

[Michael Walle]

It's a PRNG using the simple and fast xorshift method.

Signed-off-by: Michael Walle <michael@walle.cc>
---
 include/common.h |    4 ++++
 lib/Makefile     |    1 +
 lib/rand.c       |   43 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 48 insertions(+), 0 deletions(-)
 create mode 100644 lib/rand.c

diff --git a/include/common.h b/include/common.h
index 4b5841e..fbea264 100644
--- a/include/common.h
+++ b/include/common.h
@@ -733,6 +733,10 @@ char *	strmhz(char *buf, unsigned long hz);
 /* lib/crc32.c */
 #include <u-boot/crc.h>
 
+/* lib/rand.c */
+void srand(unsigned int seed);
+unsigned int rand(void);
+
 /* common/console.c */
 int	console_init_f(void);	/* Before relocation; uses the serial  stuff	*/
 int	console_init_r(void);	/* After  relocation; uses the console stuff	*/
diff --git a/lib/Makefile b/lib/Makefile
index a0fec60..290bf6a 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -65,6 +65,7 @@ COBJS-y += string.o
 COBJS-y += time.o
 COBJS-$(CONFIG_BOOTP_PXE) += uuid.o
 COBJS-y += vsprintf.o
+COBJS-y += rand.o
 
 COBJS	:= $(COBJS-y)
 SRCS	:= $(COBJS:.o=.c)
diff --git a/lib/rand.c b/lib/rand.c
new file mode 100644
index 0000000..9923f67
--- /dev/null
+++ b/lib/rand.c
@@ -0,0 +1,43 @@
+/*
+ * Simple xorshift PRNG
+ *   see http://www.jstatsoft.org/v08/i14/paper
+ *
+ * Copyright (c) 2012 Michael Walle
+ * Michael Walle <michael@walle.cc>
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include <common.h>
+
+static unsigned int y = 2463534242U;
+
+void srand(unsigned int seed)
+{
+	y = seed;
+}
+
+unsigned int rand(void)
+{
+	y ^= (y << 13);
+	y ^= (y >> 17);
+	y ^= (y << 5);
+
+	return y;
+}
-- 
1.7.2.5

[U-Boot] [PATCH v4 1/3] lib: add rand() function

[Wolfgang Denk]

Dear Michael Walle,

In message <1336671134-16342-2-git-send-email-michael@walle.cc> you wrote:
> It's a PRNG using the simple and fast xorshift method.
...
> +static unsigned int y = 2463534242U;

Hm... can we introduce at least a little entropy somewhere?


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 at denx.de
A Puritan is someone who is deathly afraid that  someone,  somewhere,
is having fun.

[U-Boot] [PATCH v4 1/3] lib: add rand() function

[Michael Walle]

Am Freitag 11 Mai 2012, 21:20:02 schrieb Wolfgang Denk:
> Dear Michael Walle,
> 
> In message <1336671134-16342-2-git-send-email-michael@walle.cc> you wrote:
> > It's a PRNG using the simple and fast xorshift method.
> 
> ...
> 
> > +static unsigned int y = 2463534242U;
> 
> Hm... can we introduce at least a little entropy somewhere?
Mh? A user is supposed to seed via srand().

-- 
michael

[U-Boot] [PATCH v4 1/3] lib: add rand() function

[Wolfgang Denk]

Dear Michael Walle,

In message <201205112232.20664.michael@walle.cc> you wrote:
>
> > > +static unsigned int y = 2463534242U;
> > 
> > Hm... can we introduce at least a little entropy somewhere?
> Mh? A user is supposed to seed via srand().

Then why initialize y at all?

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 at denx.de
And now remains  That we find out the cause of this effect, Or rather
say, the cause of this defect...           -- Hamlet, Act II, Scene 2

[U-Boot] [PATCH v4 1/3] lib: add rand() function

[Wolfgang Denk]

Dear Michael Walle,

please keep the ML on Cc: - thanks.

In message <201205112249.27871.michael@walle.cc> you wrote:
>
> > > > > +static unsigned int y = 2463534242U;
> > > > 
> > > > Hm... can we introduce at least a little entropy somewhere?
> > > 
> > > Mh? A user is supposed to seed via srand().
> > 
> > Then why initialize y at all?
> To have a sane fallback. Maybe i should have written
>  "A user is supposed to seed via srand() if he wan't some 
> entropy."

This is inconsequent.  Either we assume that the user will seed
srand(), then y should not be initialized at all, so it goes to the
BSS segment and does not waste space in the binary image.  Or we want
to deal with cases where the user doesn call srand(), and then we
should provide a bit better than such a static initialization.

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 at denx.de
If all the Chinese simultaneously jumped into the Pacific  off  a  10
foot platform erected 10 feet off their coast, it would cause a tidal
wave that would destroy everything in this country west of Nebraska.

[U-Boot] [PATCH v4 1/3] lib: add rand() function

[Michael Walle]

> please keep the ML on Cc: - thanks.
sorry wrong key

> In message <201205112249.27871.michael@walle.cc> you wrote:
> > > > > > +static unsigned int y = 2463534242U;
> > > > > 
> > > > > Hm... can we introduce at least a little entropy somewhere?
> > > > 
> > > > Mh? A user is supposed to seed via srand().
> > > 
> > > Then why initialize y at all?
> > 
> > To have a sane fallback. Maybe i should have written
> > 
> >  "A user is supposed to seed via srand() if he wan't some
> > 
> > entropy."
> 
> This is inconsequent.  Either we assume that the user will seed
> srand(), then y should not be initialized at all, so it goes to the
> BSS segment and does not waste space in the binary image.  Or we want
> to deal with cases where the user doesn call srand(), and then we
> should provide a bit better than such a static initialization.

Again, i wanted to stick with the regular rand() srand() semantics.

http://pubs.opengroup.org/onlinepubs/009695399/functions/rand.html
"If rand() is called before any calls to srand() are made, the same sequence 
shall be generated as when srand() is first called with a seed value of 1."

But i'm fine with removing the initialization.

-- 
Michael

[U-Boot] [PATCH v4 1/3] lib: add rand() function

[Wolfgang Denk]

Dear Michael Walle,

In message <201205112320.15462.michael@walle.cc> you wrote:
> 
> > This is inconsequent.  Either we assume that the user will seed
> > srand(), then y should not be initialized at all, so it goes to the
> > BSS segment and does not waste space in the binary image.  Or we want
> > to deal with cases where the user doesn call srand(), and then we
> > should provide a bit better than such a static initialization.
> 
> Again, i wanted to stick with the regular rand() srand() semantics.
> 
> http://pubs.opengroup.org/onlinepubs/009695399/functions/rand.html
> "If rand() is called before any calls to srand() are made, the same sequence 
> shall be generated as when srand() is first called with a seed value of 1."

Good point.

But then y should be initialized as 1 ?

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 at denx.de
"Security is mostly a superstition. It does not  exist  in  nature...
Life is either a daring adventure or nothing."         - Helen Keller

[U-Boot] [PATCH v4 1/3] lib: add rand() function

[Michael Walle]

Am Freitag 11 Mai 2012, 23:29:47 schrieb Wolfgang Denk:
> Dear Michael Walle,
> 
> In message <201205112320.15462.michael@walle.cc> you wrote:
> > > This is inconsequent.  Either we assume that the user will seed
> > > srand(), then y should not be initialized at all, so it goes to the
> > > BSS segment and does not waste space in the binary image.  Or we want
> > > to deal with cases where the user doesn call srand(), and then we
> > > should provide a bit better than such a static initialization.
> > 
> > Again, i wanted to stick with the regular rand() srand() semantics.
> > 
> > http://pubs.opengroup.org/onlinepubs/009695399/functions/rand.html
> > "If rand() is called before any calls to srand() are made, the same
> > sequence shall be generated as when srand() is first called with a seed
> > value of 1."
> 
> Good point.
> 
> But then y should be initialized as 1 ?
I'm no crypto expert, so i sticked with the values/examples from the cited 
paper. But i guess it shouldn't make any difference, i'll change it to 1.

-- 
michael

[U-Boot] [PATCH v4 2/3] net: add helper to generate random mac address

[Michael Walle]

Add new function eth_random_enetaddr() to generate a locally administered
ethernet address.

Signed-off-by: Michael Walle <michael@walle.cc>
---
 include/net.h |   15 +++++++++++++++
 net/eth.c     |   20 ++++++++++++++++++++
 2 files changed, 35 insertions(+), 0 deletions(-)

diff --git a/include/net.h b/include/net.h
index ee11f82..0da5679 100644
--- a/include/net.h
+++ b/include/net.h
@@ -118,6 +118,21 @@ extern int eth_setenv_enetaddr(char *name, const uchar *enetaddr);
 extern int eth_getenv_enetaddr_by_index(const char *base_name, int index,
 					uchar *enetaddr);
 
+/*
+ * The u-boot policy does not allow hardcoded ethernet addresses. Under the
+ * following circumstances a random generated address is allowed:
+ *  - in emergency cases, where you need a working network connection to set
+ *    the ethernet address.
+ *    Eg. you want a rescue boot and don't have a serial port to access the
+ *    CLI to set environment variables.
+ *
+ * In these cases, we generate a random locally administered ethernet address.
+ *
+ * Args:
+ *  enetaddr - returns 6 byte hardware address
+ */
+extern void eth_random_enetaddr(uchar *enetaddr);
+
 extern int usb_eth_initialize(bd_t *bi);
 extern int eth_init(bd_t *bis);			/* Initialize the device */
 extern int eth_send(volatile void *packet, int length);	   /* Send a packet */
diff --git a/net/eth.c b/net/eth.c
index 3eeb908..3623825 100644
--- a/net/eth.c
+++ b/net/eth.c
@@ -70,6 +70,26 @@ static int eth_mac_skip(int index)
 	return ((skip_state = getenv(enetvar)) != NULL);
 }
 
+void eth_random_enetaddr(uchar *enetaddr)
+{
+	uint32_t rval;
+
+	srand(get_timer(0));
+
+	rval = rand();
+	enetaddr[0] = rval & 0xff;
+	enetaddr[1] = (rval >> 8) & 0xff;
+	enetaddr[2] = (rval >> 16) & 0xff;
+
+	rval = rand();
+	enetaddr[3] = rval & 0xff;
+	enetaddr[4] = (rval >> 8) & 0xff;
+	enetaddr[5] = (rval >> 16) & 0xff;
+
+	/* make sure it's local and unicast */
+	enetaddr[0] = (enetaddr[0] | 0x02) & ~0x01;
+}
+
 /*
  * CPU and board-specific Ethernet initializations.  Aliased function
  * signals caller to move on
-- 
1.7.2.5

[U-Boot] [PATCH v4 2/3] net: add helper to generate random mac address

[Wolfgang Denk]

Dear Michael Walle,

In message <1336671134-16342-3-git-send-email-michael@walle.cc> you wrote:
> Add new function eth_random_enetaddr() to generate a locally administered
> ethernet address.
> 
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---
>  include/net.h |   15 +++++++++++++++
>  net/eth.c     |   20 ++++++++++++++++++++
>  2 files changed, 35 insertions(+), 0 deletions(-)

Please make this code configurable.  I don't want to bloat code size
for all boards when only one or two will ever use this feature.

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 at denx.de
I'd like to meet the man who invented sex and see what  he's  working
on now.

[U-Boot] [PATCH v4 2/3] net: add helper to generate random mac address

[Mike Frysinger]

On Friday 11 May 2012 15:21:19 Wolfgang Denk wrote:
> Michael Walle wrote:
> > Add new function eth_random_enetaddr() to generate a locally administered
> > ethernet address.
> > 
> > Signed-off-by: Michael Walle <michael@walle.cc>
> > ---
> > 
> >  include/net.h |   15 +++++++++++++++
> >  net/eth.c     |   20 ++++++++++++++++++++
> >  2 files changed, 35 insertions(+), 0 deletions(-)
> 
> Please make this code configurable.  I don't want to bloat code size
> for all boards when only one or two will ever use this feature.

it would be nice if we could standardize gc-sections across the board
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120514/fb715b46/attachment.pgp>

[U-Boot] [PATCH v4 3/3] net: add eth_setenv_enetaddr_by_index()

[Michael Walle]

Signed-off-by: Michael Walle <michael@walle.cc>
---
 include/net.h |   12 ++++++++++++
 net/eth.c     |    8 ++++++++
 2 files changed, 20 insertions(+), 0 deletions(-)

diff --git a/include/net.h b/include/net.h
index 0da5679..eec846b 100644
--- a/include/net.h
+++ b/include/net.h
@@ -119,6 +119,18 @@ extern int eth_getenv_enetaddr_by_index(const char *base_name, int index,
 					uchar *enetaddr);
 
 /*
+ * Set the hardware address for an ethernet interface .
+ * Args:
+ *	base_name - base name for device (normally "eth")
+ *	index - device index number (0 for first)
+ *	enetaddr - returns 6 byte hardware address
+ * Returns:
+ *	0 on success, else 1.
+ */
+extern int eth_setenv_enetaddr_by_index(const char *base_name, int index,
+					const uchar *enetaddr);
+
+/*
  * The u-boot policy does not allow hardcoded ethernet addresses. Under the
  * following circumstances a random generated address is allowed:
  *  - in emergency cases, where you need a working network connection to set
diff --git a/net/eth.c b/net/eth.c
index 3623825..79b81a2 100644
--- a/net/eth.c
+++ b/net/eth.c
@@ -62,6 +62,14 @@ int eth_getenv_enetaddr_by_index(const char *base_name, int index,
 	return eth_getenv_enetaddr(enetvar, enetaddr);
 }
 
+int eth_setenv_enetaddr_by_index(const char *base_name, int index,
+		const uchar *enetaddr)
+{
+	char enetvar[32];
+	sprintf(enetvar, index ? "%s%daddr" : "%saddr", base_name, index);
+	return eth_setenv_enetaddr(enetvar, enetaddr);
+}
+
 static int eth_mac_skip(int index)
 {
 	char enetvar[15];
-- 
1.7.2.5

[U-Boot] [PATCH v4 3/3] net: add eth_setenv_enetaddr_by_index()

[Wolfgang Denk]

Dear Michael Walle,

In message <1336671134-16342-4-git-send-email-michael@walle.cc> you wrote:
> Signed-off-by: Michael Walle <michael@walle.cc>

How many boards will be using this code?  

Eventually this should be made configurable?

> +	sprintf(enetvar, index ? "%s%daddr" : "%saddr", base_name, index);

This should generate a warning: too many arguments for format.

Please fix.

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 at denx.de
In the pitiful, multipage, connection-boxed form to which  the  flow-
chart  has  today  been  elaborated, it has proved to be useless as a
design tool -- programmers draw flowcharts after, not before, writing
the programs they describe.                        - Fred Brooks, Jr.

[U-Boot] [PATCH v4 3/3] net: add eth_setenv_enetaddr_by_index()

[Michael Walle]

Am Freitag 11 Mai 2012, 21:25:27 schrieb Wolfgang Denk:
> Dear Michael Walle,
> 
> In message <1336671134-16342-4-git-send-email-michael@walle.cc> you wrote:
> > Signed-off-by: Michael Walle <michael@walle.cc>
> 
> How many boards will be using this code?
Hopefully more boars will use this function in the future, instead of messing 
around with the variable names themselves.

> Eventually this should be made configurable?
Given the above, i would keep it non-configurable if youre fine with that.

> 
> > +	sprintf(enetvar, index ? "%s%daddr" : "%saddr", base_name, index);
> 
> This should generate a warning: too many arguments for format.
Mh, nice catch. That was just copied from the eth_getenv_enetaddr_by_index() 
function. I'll fix all of them.

-- 
michael

[U-Boot] [PATCH v4 3/3] net: add eth_setenv_enetaddr_by_index()

[Wolfgang Denk]

Dear Michael Walle,

In message <201205112301.50311.michael@walle.cc> you wrote:
>
> > Eventually this should be made configurable?
> Given the above, i would keep it non-configurable if youre fine with that.

Only if the code size for the existing boards that don't need this
does not grow.


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 at denx.de
If you fail to plan, plan to fail.

[U-Boot] [PATCH v4 3/3] net: add eth_setenv_enetaddr_by_index()

[Michael Walle]

Am Freitag 11 Mai 2012, 23:16:02 schrieb Wolfgang Denk:
> Dear Michael Walle,
> 
> In message <201205112301.50311.michael@walle.cc> you wrote:
> > > Eventually this should be made configurable?
> > 
> > Given the above, i would keep it non-configurable if youre fine with
> > that.
> 
> Only if the code size for the existing boards that don't need this
> does not grow.
Well i can't add a new function without increasing the filesize, can i? 

Using inline functions would be one option, but that would be inconsistend 
with the eth_getenv_enetaddr_by_index().

phew, i wonder how the eth_getenv_enetaddr_by_index() made it into uboot ;)

-- 
michael

[U-Boot] [PATCH v4 0/3] net: helper functions

[Michael Walle]

Am Donnerstag 10 Mai 2012, 19:32:11 schrieb Michael Walle:
> This patchset is a split-off from my patch series "Kirkwood: add lschlv2
> and lsxhl board support".

Hi Joe,

will you ack/nak these patch series and in case of an ack, Prafulla will fetch 
these into his tree? Or how is it supposed to work?

-- 
Michael

[U-Boot] [PATCH v4 0/3] net: helper functions

[Joe Hershberger]

Hi Michael,

On Fri, May 11, 2012 at 11:32 AM, Michael Walle <michael@walle.cc> wrote:
> Am Donnerstag 10 Mai 2012, 19:32:11 schrieb Michael Walle:
>> This patchset is a split-off from my patch series "Kirkwood: add lschlv2
>> and lsxhl board support".
>
> Hi Joe,
>
> will you ack/nak these patch series and in case of an ack, Prafulla will fetch
> these into his tree? Or how is it supposed to work?

It is up to Prafulla.  If it makes more sense due to being required
for your other patch, then it can go through his branch.  Let me know.

-Joe

[U-Boot] [PATCH v4 0/3] net: helper functions

[Prafulla Wadaskar]

> -----Original Message-----
> From: Joe Hershberger [mailto:joe.hershberger at gmail.com]
> Sent: 11 May 2012 09:47
> To: Michael Walle
> Cc: u-boot at lists.denx.de; Prafulla Wadaskar
> Subject: Re: [PATCH v4 0/3] net: helper functions
> 
> Hi Michael,
> 
> On Fri, May 11, 2012 at 11:32 AM, Michael Walle <michael@walle.cc>
> wrote:
> > Am Donnerstag 10 Mai 2012, 19:32:11 schrieb Michael Walle:
> >> This patchset is a split-off from my patch series "Kirkwood: add
> lschlv2
> >> and lsxhl board support".
> >
> > Hi Joe,
> >
> > will you ack/nak these patch series and in case of an ack, Prafulla
> will fetch
> > these into his tree? Or how is it supposed to work?
> 
> It is up to Prafulla.  If it makes more sense due to being required
> for your other patch, then it can go through his branch.  Let me know.

I will check on this patch in this week end since I am travelling.

Regards..
Prafulla . . .

[U-Boot] [PATCH v4 0/3] net: helper functions

[Michael Walle]

Am Freitag 11 Mai 2012, 18:51:09 schrieb Prafulla Wadaskar:
> I will check on this patch in this week end since I am travelling.

Hi Prafulla,

did you find the time to review the arm/board specific patches in this series 
(http://patchwork.ozlabs.org/patch/158627/)?

-- 
Michael

[U-Boot] [PATCH v4 0/3] net: helper functions

[Wolfgang Denk]

Dear Michael Walle,

In message <1336671134-16342-1-git-send-email-michael@walle.cc> you wrote:
> This patchset is a split-off from my patch series "Kirkwood: add lschlv2
> and lsxhl board support".
> 
> Changes:
> v4:
>  - typo fixes (thanks Mike)
>  - seed all 46bits of the generated ethernet address (suggested by Mike)
> v3, v2:
>  [contained only lsxhl board changes]
> 
> Michael Walle (3):
>   lib: add rand() function
>   net: add helper to generate random mac address
>   net: add eth_setenv_enetaddr_by_index()

This patchset alone just adds unused (= dead code), and will not be
added because of this.

Please submit with the patches that actually use this code.

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 at denx.de
The IQ of the group is the lowest IQ of a member of the group divided
by the number of people in the group.

[U-Boot] [PATCH v4 0/3] net: helper functions

[Michael Walle]

Am Freitag 11 Mai 2012, 21:18:50 schrieb Wolfgang Denk:
> Dear Michael Walle,
> 
> In message <1336671134-16342-1-git-send-email-michael@walle.cc> you wrote:
> > This patchset is a split-off from my patch series "Kirkwood: add lschlv2
> > and lsxhl board support".
> > 
> > Changes:
> > 
> > v4:
> >  - typo fixes (thanks Mike)
> >  - seed all 46bits of the generated ethernet address (suggested by Mike)
> > 
> > v3, v2:
> >  [contained only lsxhl board changes]
> > 
> > Michael Walle (3):
> >   lib: add rand() function
> >   net: add helper to generate random mac address
> >   net: add eth_setenv_enetaddr_by_index()
> 
> This patchset alone just adds unused (= dead code), and will not be
> added because of this.
> 
> Please submit with the patches that actually use this code.

Please have a look at
  "[PATCH v4] Kirkwood: add lschlv2 and lsxhl board support"

All introduced functions are used by this bsp. The original patch series was 
split because those three patches affect the net subsystem and should be 
reviewed/picked by Joe Hershberger.

-- 
Michael

[U-Boot] [PATCH v4 0/3] net: helper functions

[Wolfgang Denk]

Dear Michael Walle,

In message <201205112229.44084.michael@walle.cc> you wrote:
>
> > Please submit with the patches that actually use this code.
> 
> Please have a look at
>   "[PATCH v4] Kirkwood: add lschlv2 and lsxhl board support"
> 
> All introduced functions are used by this bsp. The original patch series was 
> split because those three patches affect the net subsystem and should be 
> reviewed/picked by Joe Hershberger.

Keep these patches together, please.  The responsible custodians can
still ACK individual patches of the series.

The fact that only a single board ever needed these functions makes me
wonder if these are really of any general use.

In any case, I want to see these configurable, so that boards that do
not need these will not have to suffer.

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 at denx.de
When in doubt, mumble;   when in trouble, delegate;  when in  charge,
ponder.                                             -- James H. Boren

[U-Boot] [PATCH v4 0/3] net: helper functions

[Michael Walle]

Am Freitag 11 Mai 2012, 22:43:16 schrieb Wolfgang Denk:
> Keep these patches together, please.  The responsible 
custodians can
> still ACK individual patches of the series.
> 
> The fact that only a single board ever needed these 
functions makes me
> wonder if these are really of any general use.
Sorry, Prafulla asked me to split the patches, so should 
i combine them with the v5 respin again?


> In any case, I want to see these configurable, so that 
boards that do
> not need these will not have to suffer.
Agreed.

What about the rand function? I guess that should be two 
compile time options

- CONFIG_RAND
- CONFIG_RANDOM_HWADDR

Any better names?

-- 
Michael

[U-Boot] [PATCH v4 0/3] net: helper functions

[Wolfgang Denk]

Dear Michael Walle,

In message <201205112254.13896.michael@walle.cc> you wrote:
>
> What about the rand function? I guess that should be two 
> compile time options
> 
> - CONFIG_RAND
> - CONFIG_RANDOM_HWADDR
> 
> Any better names?

Please make this a single option, CONFIG_RANDOM_MACADDR.  It appears
nobody else ever needed a random number before, and we can still
factor this out if another user pops up after another 12 years.

Eventually, also move the code together.

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 at denx.de
"The lesser of two evils -- is evil."             - Seymour (Sy) Leon

[U-Boot] [PATCH v4 0/3] net: helper functions

[Michael Walle]

Am Freitag 11 Mai 2012, 23:14:25 schrieb Wolfgang Denk:
> Dear Michael Walle,
> 
> In message <201205112254.13896.michael@walle.cc> you wrote:
> > What about the rand function? I guess that should be two
> > compile time options
> > 
> > - CONFIG_RAND
> > - CONFIG_RANDOM_HWADDR
> > 
> > Any better names?
> 
> Please make this a single option, CONFIG_RANDOM_MACADDR.  It appears
> nobody else ever needed a random number before, and we can still
> factor this out if another user pops up after another 12 years.

from net/net.c:
unsigned int random_port(void)
{
        return 1024 + (get_timer(0) % 0x4000);
}

CONFIG_BOOTP_RANDOM_DELAY, in net/bootp.c

-- 
michael

[U-Boot] [PATCH v4 0/3] net: helper functions

[Wolfgang Denk]

Dear Michael Walle,

In message <201205112322.23891.michael@walle.cc> you wrote:
>
> > > - CONFIG_RAND
> > > - CONFIG_RANDOM_HWADDR
> > > 
> > > Any better names?
> > 
> > Please make this a single option, CONFIG_RANDOM_MACADDR.  It appears
> > nobody else ever needed a random number before, and we can still
> > factor this out if another user pops up after another 12 years.
> 
> from net/net.c:
> unsigned int random_port(void)
> {
>         return 1024 + (get_timer(0) % 0x4000);
> }
> 
> CONFIG_BOOTP_RANDOM_DELAY, in net/bootp.c

Well, you patch did not change this code.

And I have to admit that I doubt the increased code size would be
worth such a change, at this place.

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 at denx.de
core error - bus dumped

[U-Boot] [PATCH v4 0/3] net: helper functions

[Michael Walle]

Am Freitag 11 Mai 2012, 23:30:59 schrieb Wolfgang Denk:
> Well, you patch did not change this code.
> 
> And I have to admit that I doubt the increased code size would be
> worth such a change, at this place.
Ok, can we agree on keeping the rand function in lib/ but using just one macro 
CONFIG_RANDOM_MACADDR?

-- 
Michael

[U-Boot] [PATCH v4 0/3] net: helper functions

[Wolfgang Denk]

Dear Michael Walle,

In message <201205112342.43133.michael@walle.cc> you wrote:
> Am Freitag 11 Mai 2012, 23:30:59 schrieb Wolfgang Denk:
> > Well, you patch did not change this code.
> > 
> > And I have to admit that I doubt the increased code size would be
> > worth such a change, at this place.
> Ok, can we agree on keeping the rand function in lib/ but using just one macro 
> CONFIG_RANDOM_MACADDR?

Yes, that should do for now. thanks.

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 at denx.de
The complexity of software is an essential property, not an  acciden-
tal  one. Hence, descriptions of a software entity that abstract away
its complexity often abstract away its essence.    - Fred Brooks, Jr.

[U-Boot] [PATCH v4 0/3] net: helper functions

[Mike Frysinger]

On Friday 11 May 2012 17:22:23 Michael Walle wrote:
> Am Freitag 11 Mai 2012, 23:14:25 schrieb Wolfgang Denk:
> > Michael Walle wrote:
> > > What about the rand function? I guess that should be two
> > > compile time options
> > > 
> > > - CONFIG_RAND
> > > - CONFIG_RANDOM_HWADDR
> > > 
> > > Any better names?
> > 
> > Please make this a single option, CONFIG_RANDOM_MACADDR.  It appears
> > nobody else ever needed a random number before, and we can still
> > factor this out if another user pops up after another 12 years.
> 
> from net/net.c:
> unsigned int random_port(void)
> {
>         return 1024 + (get_timer(0) % 0x4000);
> }
> 
> CONFIG_BOOTP_RANDOM_DELAY, in net/bootp.c

note that if you do change this code, the 0x4000 clamp is important.  read the 
git history of this file/func fore more details.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120514/ef9e58c9/attachment.pgp>