[U-Boot] [PATCH v4 0/3] net: helper functions
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
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; +}
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
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().
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
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
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
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
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];
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
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.
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
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 ;)
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?
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
-----Original Message----- From: Joe Hershberger [mailto:joe.hershberger@gmail.com] Sent: 11 May 2012 09:47 To: Michael Walle Cc: u-boot@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 . . .
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/)?
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
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.
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
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?
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
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
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
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?
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
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
participants (5)
-
Joe Hershberger -
Michael Walle -
Mike Frysinger -
Prafulla Wadaskar -
Wolfgang Denk