Networking updates

Networking updates

[Sascha Hauer]

s series solves some issues that have been nagging me for some
time now.

First of all the manual selection of the current ethernet device is
unnecessary since the interface can be picked by the networking code
automatically based on the IP settings of the interface. With this
series the "ethact" command is gone and users of boards with multiple
network interfaces no longer have to worry about picking the right
one.

Then the network device configuration can now be done with nvvars.
This makes it unnecessary to edit scripts on the device (which may
be cumbersome on some devices which lose characters on serial ports)
and it makes it also easier for scripts or setup code to modify
the network config.

Finally the DHCP code is cleaned up, primarily to be able to pass
in the network interface that shall be used, but also to make more
clear which variables are input and which are output.

For DHCP users the configuration is simple: It is the default. A static
IP setup can now be done with the variables:

nv.net.server
nv.net.nameserver
nv.net.gateway

and the network device specific variables (assuming "eth0" as network device):

nv.dev.eth0.ipaddr
nv.dev.eth0.netmask

(For those who are not already familiar with it: It's the normal way to store
device parameters in nvvars, like already possible for example with NAND
partitioning, i.e. nv.dev.nand0.partitions=4M(barebox),-(root), see
http://www.barebox.org/doc/latest/user/variables.html#non-volatile-device-variables)

As usual, all comments are welcome.

Sascha

----------------------------------------------------------------
Sascha Hauer (21):
      driver: Add device_detect_all() function
      nvvar: when setting a nvvar to NULL just free the content
      net: Make domainname and nameserver globalvars
      net: Add functions to get/set nameserver and domainname
      net: introduce global.net.server
      net: dhcp: Do not overwrite serverip if it is valid
      net: Use a single gateway
      net: allow udp connections on specified network device
      net: dhcp: Allow to specify network device
      net: dhcp: avoid unnecessary casts
      net: dhcp: Coding style fixes
      net: dhcp: rework
      net: Pick network device based on IP settings
      net: remove "current" network device
      net: ifup: Factor out a eth_discover function
      ifup: Use dhcp C API rather than running command
      net: Provide new way to configure network devices
      net: update network docs
      net: environment: remove ethx setup files
      net: environment: update automounts
      defaultenv: Add README for new network config

 Documentation/user/automount.rst                   |   2 +-
 Documentation/user/networking.rst                  |  99 +++-
 .../arm/boards/afi-gf/defaultenv-gf/init/automount |  10 -
 arch/arm/boards/afi-gf/defaultenv-gf/network/eth1  |  18 -
 .../defaultenv-pico-hobbit/init/automount          |  11 -
 .../defaultenv-pico-hobbit/network/eth1            |  18 -
 .../zii-imx6q-rdu2/defaultenv-rdu2/init/automount  |  17 -
 .../zii-imx6q-rdu2/defaultenv-rdu2/network/eth1    |  18 -
 commands/Makefile                                  |   1 -
 commands/detect.c                                  |   6 +-
 commands/dhcp.c                                    |  29 +-
 commands/net.c                                     |  66 ---
 common/globalvar.c                                 |   6 +-
 defaultenv/defaultenv-2-base/init/automount        |   4 +-
 defaultenv/defaultenv-2-base/network/README        |   3 +
 defaultenv/defaultenv-2-base/network/eth0          |  18 -
 drivers/base/driver.c                              |   8 +
 include/dhcp.h                                     |  24 +-
 include/driver.h                                   |   1 +
 include/net.h                                      |  27 +-
 net/dhcp.c                                         | 568 +++++++++------------
 net/dns.c                                          |  10 +-
 net/eth.c                                          |  39 +-
 net/ifup.c                                         | 252 ++++++---
 net/net.c                                          | 143 ++++--
 25 files changed, 692 insertions(+), 706 deletions(-)
 delete mode 100644 arch/arm/boards/afi-gf/defaultenv-gf/init/automount
 delete mode 100644 arch/arm/boards/afi-gf/defaultenv-gf/network/eth1
 delete mode 100644 arch/arm/boards/technexion-pico-hobbit/defaultenv-pico-hobbit/init/automount
 delete mode 100644 arch/arm/boards/technexion-pico-hobbit/defaultenv-pico-hobbit/network/eth1
 delete mode 100644 arch/arm/boards/zii-imx6q-rdu2/defaultenv-rdu2/init/automount
 delete mode 100644 arch/arm/boards/zii-imx6q-rdu2/defaultenv-rdu2/network/eth1
 delete mode 100644 commands/net.c
 create mode 100644 defaultenv/defaultenv-2-base/network/README
 delete mode 100644 defaultenv/defaultenv-2-base/network/eth0

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 01/21] driver: Add device_detect_all() function

[Sascha Hauer]

Add a device_detect_all function to detect all devices and use it
in the detect command. This makes the functionality reusable in other
code.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 commands/detect.c     | 6 +-----
 drivers/base/driver.c | 8 ++++++++
 include/driver.h      | 1 +
 3 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/commands/detect.c b/commands/detect.c
index 1586a6fb54..42e111419f 100644
--- a/commands/detect.c
+++ b/commands/detect.c
@@ -56,11 +56,7 @@ static int do_detect(int argc, char *argv[])
 	}
 
 	if (option_all) {
-		for_each_device(dev) {
-			ret = device_detect(dev);
-			if (ret && ret != -ENOSYS && option_error)
-				return ret;
-		}
+		device_detect_all();
 		return 0;
 	}
 
diff --git a/drivers/base/driver.c b/drivers/base/driver.c
index 83260990af..c43a4bde2a 100644
--- a/drivers/base/driver.c
+++ b/drivers/base/driver.c
@@ -140,6 +140,14 @@ int device_detect_by_name(const char *__devname)
 	return ret;
 }
 
+void device_detect_all(void)
+{
+	struct device_d *dev;
+
+	for_each_device(dev)
+		device_detect(dev);
+}
+
 static int match(struct driver_d *drv, struct device_d *dev)
 {
 	int ret;
diff --git a/include/driver.h b/include/driver.h
index 8617872053..e571fbbec5 100644
--- a/include/driver.h
+++ b/include/driver.h
@@ -134,6 +134,7 @@ int device_probe(struct device_d *dev);
 /* detect devices attached to this device (cards, disks,...) */
 int device_detect(struct device_d *dev);
 int device_detect_by_name(const char *devname);
+void device_detect_all(void);
 
 /* Unregister a device. This function can fail, e.g. when the device
  * has children.
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 02/21] nvvar: when setting a nvvar to NULL just free the content

[Sascha Hauer]

When a nvvar is about to be removed then it is set to NULL
by barebox. This means for the variable to free it's memory.
In this case also do not pass the value to the corresponding
globalvar, since that would set the globalvar to NULL aswell,
but we want to keep its current value.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 common/globalvar.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/common/globalvar.c b/common/globalvar.c
index ee756e5140..59d299f019 100644
--- a/common/globalvar.c
+++ b/common/globalvar.c
@@ -167,8 +167,10 @@ static int nv_set(struct device_d *dev, struct param_d *p, const char *val)
 {
 	int ret;
 
-	if (!val)
-		val = "";
+	if (!val) {
+		free(p->value);
+		return 0;
+	}
 
 	ret = dev_set_param(&global_device, p->name, val);
 	if (ret)
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 03/21] net: Make domainname and nameserver globalvars

[Sascha Hauer]

Register domainname and nameserver as globalvars rather than attaching
them to a dedicated net device. the global device already exists and
already contains much of the barebox configuration, so no need to add
an extra device for network config.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 net/dhcp.c |  4 ++--
 net/dns.c  |  4 ++--
 net/net.c  | 25 ++++++++++---------------
 3 files changed, 14 insertions(+), 19 deletions(-)

diff --git a/net/dhcp.c b/net/dhcp.c
index c5386fe942..dfa593a7d1 100644
--- a/net/dhcp.c
+++ b/net/dhcp.c
@@ -242,7 +242,7 @@ struct dhcp_opt dhcp_options[] = {
 	}, {
 		.option = 6,
 		.handle = env_ip_handle,
-		.barebox_var_name = "net.nameserver",
+		.barebox_var_name = "global.net.nameserver",
 	}, {
 		.option = DHCP_HOSTNAME,
 		.copy_only_if_valid = 1,
@@ -252,7 +252,7 @@ struct dhcp_opt dhcp_options[] = {
 	}, {
 		.option = 15,
 		.handle = env_str_handle,
-		.barebox_var_name = "net.domainname",
+		.barebox_var_name = "global.net.domainname",
 	}, {
 		.option = 17,
 		.handle = env_str_handle,
diff --git a/net/dns.c b/net/dns.c
index 700c6b0259..69b8a24861 100644
--- a/net/dns.c
+++ b/net/dns.c
@@ -77,7 +77,7 @@ static int dns_send(const char *name)
 	header->nauth    = 0;
 	header->nother   = 0;
 
-	domain = getenv("net.domainname");
+	domain = getenv("global.net.domainname");
 
 	if (!strchr(name, '.') && domain && *domain)
 		fullname = basprintf(".%s.%s.", name, domain);
@@ -211,7 +211,7 @@ IPaddr_t resolv(const char *host)
 
 	dns_state = STATE_INIT;
 
-	ns = getenv("net.nameserver");
+	ns = getenv("global.net.nameserver");
 	if (!ns || !*ns) {
 		printk("%s: no nameserver specified in $net.nameserver\n",
 				__func__);
diff --git a/net/net.c b/net/net.c
index 19b081f6cf..ea302ce0b0 100644
--- a/net/net.c
+++ b/net/net.c
@@ -33,12 +33,17 @@
 #include <errno.h>
 #include <malloc.h>
 #include <init.h>
+#include <globalvar.h>
+#include <magicvar.h>
 #include <linux/ctype.h>
 #include <linux/err.h>
 
 unsigned char *NetRxPackets[PKTBUFSRX]; /* Receive packets		*/
 static unsigned int net_ip_id;
 
+static IPaddr_t net_nameserver;
+static char *net_domainname;
+
 int net_checksum_ok(unsigned char *ptr, int len)
 {
 	return net_checksum(ptr, len) == 0xffff;
@@ -568,14 +573,6 @@ out:
 	return ret;
 }
 
-static struct device_d net_device = {
-	.name = "net",
-	.id = DEVICE_ID_SINGLE,
-};
-
-static char *net_nameserver;
-static char *net_domainname;
-
 static int net_init(void)
 {
 	int i;
@@ -583,15 +580,13 @@ static int net_init(void)
 	for (i = 0; i < PKTBUFSRX; i++)
 		NetRxPackets[i] = net_alloc_packet();
 
-	register_device(&net_device);
-	net_nameserver = xstrdup("");
-	dev_add_param_string(&net_device, "nameserver", NULL, NULL,
-			     &net_nameserver, NULL);
-	net_domainname = xstrdup("");
-	dev_add_param_string(&net_device, "domainname", NULL, NULL,
-			     &net_domainname, NULL);
+	globalvar_add_simple_ip("net.nameserver", &net_nameserver);
+	globalvar_add_simple_string("net.domainname", &net_domainname);
 
 	return 0;
 }
 
 postcore_initcall(net_init);
+
+BAREBOX_MAGICVAR_NAMED(global_net_nameserver, global.net.nameserver, "The DNS server used for resolving host names");
+BAREBOX_MAGICVAR_NAMED(global_net_domainname, global.net.domainname, "Domain name used for DNS requests");
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 04/21] net: Add functions to get/set nameserver and domainname

[Sascha Hauer]

It's more convenient to have getter/setter functions for
variables rather than using the detour around global vars
which use string matching and all kinds of overhead in the
background.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 include/net.h |  4 ++++
 net/dns.c     |  8 ++------
 net/net.c     | 24 ++++++++++++++++++++++++
 3 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/include/net.h b/include/net.h
index 632b6d5410..0fcde2f0b3 100644
--- a/include/net.h
+++ b/include/net.h
@@ -218,8 +218,12 @@ void net_set_ip(IPaddr_t ip);
 void net_set_serverip(IPaddr_t ip);
 void net_set_netmask(IPaddr_t ip);
 void net_set_gateway(IPaddr_t ip);
+void net_set_nameserver(IPaddr_t ip);
+void net_set_domainname(const char *name);
 IPaddr_t net_get_ip(void);
 IPaddr_t net_get_serverip(void);
+IPaddr_t net_get_nameserver(void);
+const char *net_get_domainname(void);
 
 /* Do the work */
 void net_poll(void);
diff --git a/net/dns.c b/net/dns.c
index 69b8a24861..a8ce7a4484 100644
--- a/net/dns.c
+++ b/net/dns.c
@@ -202,7 +202,6 @@ static void dns_handler(void *ctx, char *packet, unsigned len)
 IPaddr_t resolv(const char *host)
 {
 	IPaddr_t ip;
-	const char *ns;
 
 	if (!string_to_ip(host, &ip))
 		return ip;
@@ -211,16 +210,13 @@ IPaddr_t resolv(const char *host)
 
 	dns_state = STATE_INIT;
 
-	ns = getenv("global.net.nameserver");
-	if (!ns || !*ns) {
+	ip = net_get_nameserver();
+	if (!ip) {
 		printk("%s: no nameserver specified in $net.nameserver\n",
 				__func__);
 		return 0;
 	}
 
-	if (string_to_ip(ns, &ip))
-		return 0;
-
 	debug("resolving host %s via nameserver %pI4\n", host, &ip);
 
 	dns_con = net_udp_new(ip, DNS_PORT, dns_handler, NULL);
diff --git a/net/net.c b/net/net.c
index ea302ce0b0..8cb905c66b 100644
--- a/net/net.c
+++ b/net/net.c
@@ -44,6 +44,30 @@ static unsigned int net_ip_id;
 static IPaddr_t net_nameserver;
 static char *net_domainname;
 
+void net_set_nameserver(IPaddr_t nameserver)
+{
+	net_nameserver = nameserver;
+}
+
+IPaddr_t net_get_nameserver(void)
+{
+	return net_nameserver;
+}
+
+void net_set_domainname(const char *name)
+{
+	free(net_domainname);
+	if (name)
+		net_domainname = xstrdup(name);
+	else
+		net_domainname = xstrdup(name);
+};
+
+const char *net_get_domainname(void)
+{
+	return net_domainname;
+}
+
 int net_checksum_ok(unsigned char *ptr, int len)
 {
 	return net_checksum(ptr, len) == 0xffff;
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 05/21] net: introduce global.net.server

[Sascha Hauer]

The server to use is independent of the network device, there is
not much point to make the server specific to a network device.

This introduces global.net.server as the serverip which is
used as standard NFS/tftp server. The previously used eth
device specific parameters still exist, but are only aliases
for the global single variable.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 include/net.h |  1 -
 net/eth.c     |  4 +++-
 net/ifup.c    |  7 ++++++-
 net/net.c     | 11 +++++------
 4 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/include/net.h b/include/net.h
index 0fcde2f0b3..b3e44c7baa 100644
--- a/include/net.h
+++ b/include/net.h
@@ -58,7 +58,6 @@ struct eth_device {
 	struct list_head list;
 
 	IPaddr_t ipaddr;
-	IPaddr_t serverip;
 	IPaddr_t netmask;
 	IPaddr_t gateway;
 	char ethaddr[6];
diff --git a/net/eth.c b/net/eth.c
index dac2400b81..74666bbf22 100644
--- a/net/eth.c
+++ b/net/eth.c
@@ -348,6 +348,8 @@ static int eth_register_of_fixup(void)
 late_initcall(eth_register_of_fixup);
 #endif
 
+extern IPaddr_t net_serverip;
+
 int eth_register(struct eth_device *edev)
 {
 	struct device_d *dev = &edev->dev;
@@ -379,7 +381,7 @@ int eth_register(struct eth_device *edev)
 	edev->devname = xstrdup(dev_name(&edev->dev));
 
 	dev_add_param_ip(dev, "ipaddr", NULL, NULL, &edev->ipaddr, edev);
-	dev_add_param_ip(dev, "serverip", NULL, NULL, &edev->serverip, edev);
+	dev_add_param_ip(dev, "serverip", NULL, NULL, &net_serverip, edev);
 	dev_add_param_ip(dev, "gateway", NULL, NULL, &edev->gateway, edev);
 	dev_add_param_ip(dev, "netmask", NULL, NULL, &edev->netmask, edev);
 	dev_add_param_mac(dev, "ethaddr", eth_param_set_ethaddr, NULL,
diff --git a/net/ifup.c b/net/ifup.c
index 5113d13832..2b13a9f140 100644
--- a/net/ifup.c
+++ b/net/ifup.c
@@ -109,14 +109,19 @@ int ifup(const char *name, unsigned flags)
 		dev_set_param(dev, "linux.bootargs", "ip=dhcp");
 	} else if (!strcmp(ip, "static")) {
 		char *bootarg;
+		IPaddr_t serverip;
+
 		for (i = 0; i < ARRAY_SIZE(vars); i++) {
 			ret = eth_set_param(dev, vars[i]);
 			if (ret)
 				goto out;
 		}
+
+		serverip = net_get_serverip();
+
 		bootarg = basprintf("ip=%pI4:%pI4:%pI4:%pI4:::",
 				&edev->ipaddr,
-				&edev->serverip,
+				&serverip,
 				&edev->gateway,
 				&edev->netmask);
 		dev_set_param(dev, "linux.bootargs", bootarg);
diff --git a/net/net.c b/net/net.c
index 8cb905c66b..552897e6b8 100644
--- a/net/net.c
+++ b/net/net.c
@@ -41,6 +41,7 @@
 unsigned char *NetRxPackets[PKTBUFSRX]; /* Receive packets		*/
 static unsigned int net_ip_id;
 
+IPaddr_t net_serverip;
 static IPaddr_t net_nameserver;
 static char *net_domainname;
 
@@ -242,16 +243,12 @@ static uint16_t net_udp_new_localport(void)
 
 IPaddr_t net_get_serverip(void)
 {
-	struct eth_device *edev = eth_get_current();
-
-	return edev->serverip;
+	return net_serverip;
 }
 
 void net_set_serverip(IPaddr_t ip)
 {
-	struct eth_device *edev = eth_get_current();
-
-	edev->serverip = ip;
+	net_serverip = ip;
 }
 
 void net_set_ip(IPaddr_t ip)
@@ -606,6 +603,7 @@ static int net_init(void)
 
 	globalvar_add_simple_ip("net.nameserver", &net_nameserver);
 	globalvar_add_simple_string("net.domainname", &net_domainname);
+	globalvar_add_simple_ip("net.server", &net_serverip);
 
 	return 0;
 }
@@ -614,3 +612,4 @@ postcore_initcall(net_init);
 
 BAREBOX_MAGICVAR_NAMED(global_net_nameserver, global.net.nameserver, "The DNS server used for resolving host names");
 BAREBOX_MAGICVAR_NAMED(global_net_domainname, global.net.domainname, "Domain name used for DNS requests");
+BAREBOX_MAGICVAR_NAMED(global_net_server, global.net.server, "Standard server used for NFS/TFTP");
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 06/21] net: dhcp: Do not overwrite serverip if it is valid

[Sascha Hauer]

Some DHCP servers provide the wrong serverip in which case
it is desired to specify it manually and won't let the dhcp
command overwrite it.
This has previously been done by setting the serverip again
to the desired value after dhcp has been executed. With this
patch we do not overwrite it in the first place if it is valid
already. This is necessary when the serverip is not set via
/env/network/eth* but via nv.net.server.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 include/net.h | 2 +-
 net/dhcp.c    | 4 ++--
 net/ifup.c    | 9 ++++++---
 net/net.c     | 5 ++++-
 4 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/include/net.h b/include/net.h
index b3e44c7baa..7818bd7145 100644
--- a/include/net.h
+++ b/include/net.h
@@ -214,7 +214,7 @@ struct icmphdr {
 extern unsigned char *NetRxPackets[PKTBUFSRX];/* Receive packets		*/
 
 void net_set_ip(IPaddr_t ip);
-void net_set_serverip(IPaddr_t ip);
+void net_set_serverip(IPaddr_t ip, bool overwrite);
 void net_set_netmask(IPaddr_t ip);
 void net_set_gateway(IPaddr_t ip);
 void net_set_nameserver(IPaddr_t ip);
diff --git a/net/dhcp.c b/net/dhcp.c
index dfa593a7d1..61abf49272 100644
--- a/net/dhcp.c
+++ b/net/dhcp.c
@@ -373,7 +373,7 @@ static void bootp_copy_net_params(struct bootp *bp)
 
 	tmp_ip = net_read_ip(&bp->bp_siaddr);
 	if (tmp_ip != 0)
-		net_set_serverip(tmp_ip);
+		net_set_serverip(tmp_ip, false);
 
 	if (strlen(bp->bp_file) > 0) {
 		if (IS_ENABLED(CONFIG_ENVIRONMENT_VARIABLES))
@@ -702,7 +702,7 @@ int dhcp(int retries, struct dhcp_req_param *param)
 	if (dhcp_tftpname[0] != 0) {
 		IPaddr_t tftpserver = resolv(dhcp_tftpname);
 		if (tftpserver)
-			net_set_serverip(tftpserver);
+			net_set_serverip(tftpserver, false);
 	}
 
 out1:
diff --git a/net/ifup.c b/net/ifup.c
index 2b13a9f140..1e14b0f61c 100644
--- a/net/ifup.c
+++ b/net/ifup.c
@@ -100,12 +100,15 @@ int ifup(const char *name, unsigned flags)
 		ip = "";
 
 	if (!strcmp(ip, "dhcp")) {
+		IPaddr_t serverip;
+
+		serverip = getenv_ip("serverip");
+		if (serverip)
+			net_set_serverip(serverip, false);
+
 		ret = run_command("dhcp");
 		if (ret)
 			goto out;
-		ret = eth_set_param(dev, "serverip");
-		if (ret)
-			goto out;
 		dev_set_param(dev, "linux.bootargs", "ip=dhcp");
 	} else if (!strcmp(ip, "static")) {
 		char *bootarg;
diff --git a/net/net.c b/net/net.c
index 552897e6b8..28aeb4417a 100644
--- a/net/net.c
+++ b/net/net.c
@@ -246,8 +246,11 @@ IPaddr_t net_get_serverip(void)
 	return net_serverip;
 }
 
-void net_set_serverip(IPaddr_t ip)
+void net_set_serverip(IPaddr_t ip, bool overwrite)
 {
+	if (net_serverip && !overwrite)
+		return;
+
 	net_serverip = ip;
 }
 
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 07/21] net: Use a single gateway

[Sascha Hauer]

There is not much point in having a network device specific
gateway. If barebox really is part of such a complicated network
in which it needs multiple gateways, then we probably need a
real routing table. Until this happens, a single gateway should
be enough.

This introduces global.net.gateway which holds the gateway ip. The
previously used device specific <ethx>.gateway variables still exist,
but are only aliases for the single gateway.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 include/net.h |  2 +-
 net/eth.c     |  3 ++-
 net/ifup.c    |  4 +++-
 net/net.c     | 13 +++++++++----
 4 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/include/net.h b/include/net.h
index 7818bd7145..e75f64fe75 100644
--- a/include/net.h
+++ b/include/net.h
@@ -59,7 +59,6 @@ struct eth_device {
 
 	IPaddr_t ipaddr;
 	IPaddr_t netmask;
-	IPaddr_t gateway;
 	char ethaddr[6];
 	char *bootarg;
 };
@@ -221,6 +220,7 @@ void net_set_nameserver(IPaddr_t ip);
 void net_set_domainname(const char *name);
 IPaddr_t net_get_ip(void);
 IPaddr_t net_get_serverip(void);
+IPaddr_t net_get_gateway(void);
 IPaddr_t net_get_nameserver(void);
 const char *net_get_domainname(void);
 
diff --git a/net/eth.c b/net/eth.c
index 74666bbf22..6dae6b9feb 100644
--- a/net/eth.c
+++ b/net/eth.c
@@ -349,6 +349,7 @@ late_initcall(eth_register_of_fixup);
 #endif
 
 extern IPaddr_t net_serverip;
+extern IPaddr_t net_gateway;
 
 int eth_register(struct eth_device *edev)
 {
@@ -382,7 +383,7 @@ int eth_register(struct eth_device *edev)
 
 	dev_add_param_ip(dev, "ipaddr", NULL, NULL, &edev->ipaddr, edev);
 	dev_add_param_ip(dev, "serverip", NULL, NULL, &net_serverip, edev);
-	dev_add_param_ip(dev, "gateway", NULL, NULL, &edev->gateway, edev);
+	dev_add_param_ip(dev, "gateway", NULL, NULL, &net_gateway, edev);
 	dev_add_param_ip(dev, "netmask", NULL, NULL, &edev->netmask, edev);
 	dev_add_param_mac(dev, "ethaddr", eth_param_set_ethaddr, NULL,
 			edev->ethaddr, edev);
diff --git a/net/ifup.c b/net/ifup.c
index 1e14b0f61c..70ecf9c717 100644
--- a/net/ifup.c
+++ b/net/ifup.c
@@ -113,6 +113,7 @@ int ifup(const char *name, unsigned flags)
 	} else if (!strcmp(ip, "static")) {
 		char *bootarg;
 		IPaddr_t serverip;
+		IPaddr_t gateway;
 
 		for (i = 0; i < ARRAY_SIZE(vars); i++) {
 			ret = eth_set_param(dev, vars[i]);
@@ -121,11 +122,12 @@ int ifup(const char *name, unsigned flags)
 		}
 
 		serverip = net_get_serverip();
+		gateway = net_get_gateway();
 
 		bootarg = basprintf("ip=%pI4:%pI4:%pI4:%pI4:::",
 				&edev->ipaddr,
 				&serverip,
-				&edev->gateway,
+				&gateway,
 				&edev->netmask);
 		dev_set_param(dev, "linux.bootargs", bootarg);
 		free(bootarg);
diff --git a/net/net.c b/net/net.c
index 28aeb4417a..5f9535fc40 100644
--- a/net/net.c
+++ b/net/net.c
@@ -42,6 +42,7 @@ unsigned char *NetRxPackets[PKTBUFSRX]; /* Receive packets		*/
 static unsigned int net_ip_id;
 
 IPaddr_t net_serverip;
+IPaddr_t net_gateway;
 static IPaddr_t net_nameserver;
 static char *net_domainname;
 
@@ -181,10 +182,10 @@ static int arp_request(IPaddr_t dest, unsigned char *ether)
 	memset(arp->ar_data + 10, 0, 6);	/* dest ET addr = 0     */
 
 	if ((dest & edev->netmask) != (edev->ipaddr & edev->netmask)) {
-		if (!edev->gateway)
+		if (!net_gateway)
 			arp_wait_ip = dest;
 		else
-			arp_wait_ip = edev->gateway;
+			arp_wait_ip = net_gateway;
 	} else {
 		arp_wait_ip = dest;
 	}
@@ -277,9 +278,12 @@ void net_set_netmask(IPaddr_t nm)
 
 void net_set_gateway(IPaddr_t gw)
 {
-	struct eth_device *edev = eth_get_current();
+	net_gateway = gw;
+}
 
-	edev->gateway = gw;
+IPaddr_t net_get_gateway(void)
+{
+	return net_gateway;
 }
 
 static LIST_HEAD(connection_list);
@@ -607,6 +611,7 @@ static int net_init(void)
 	globalvar_add_simple_ip("net.nameserver", &net_nameserver);
 	globalvar_add_simple_string("net.domainname", &net_domainname);
 	globalvar_add_simple_ip("net.server", &net_serverip);
+	globalvar_add_simple_ip("net.gateway", &net_gateway);
 
 	return 0;
 }
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 08/21] net: allow udp connections on specified network device

[Sascha Hauer]

This allows the DHCP code to configure specific network
devices so that DHCP no longer depends on any "current"
network device.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 include/net.h |  4 ++++
 net/net.c     | 32 ++++++++++++++++++++------------
 2 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/include/net.h b/include/net.h
index e75f64fe75..6788f14cb0 100644
--- a/include/net.h
+++ b/include/net.h
@@ -443,6 +443,10 @@ static inline char *net_alloc_packet(void)
 struct net_connection *net_udp_new(IPaddr_t dest, uint16_t dport,
 		rx_handler_f *handler, void *ctx);
 
+struct net_connection *net_udp_eth_new(struct eth_device *edev, IPaddr_t dest,
+                                       uint16_t dport, rx_handler_f *handler,
+                                       void *ctx);
+
 struct net_connection *net_icmp_new(IPaddr_t dest, rx_handler_f *handler,
 		void *ctx);
 
diff --git a/net/net.c b/net/net.c
index 5f9535fc40..1d47bb449a 100644
--- a/net/net.c
+++ b/net/net.c
@@ -141,9 +141,8 @@ static void arp_handler(struct arprequest *arp)
 	}
 }
 
-static int arp_request(IPaddr_t dest, unsigned char *ether)
+static int arp_request(struct eth_device *edev, IPaddr_t dest, unsigned char *ether)
 {
-	struct eth_device *edev = eth_get_current();
 	char *pkt;
 	struct arprequest *arp;
 	uint64_t arp_start;
@@ -288,15 +287,17 @@ IPaddr_t net_get_gateway(void)
 
 static LIST_HEAD(connection_list);
 
-static struct net_connection *net_new(IPaddr_t dest, rx_handler_f *handler,
-		void *ctx)
+static struct net_connection *net_new(struct eth_device *edev, IPaddr_t dest,
+				      rx_handler_f *handler, void *ctx)
 {
-	struct eth_device *edev = eth_get_current();
 	struct net_connection *con;
 	int ret;
 
-	if (!edev)
-		return ERR_PTR(-ENETDOWN);
+	if (!edev) {
+		edev = eth_get_current();
+		if (!edev)
+			return ERR_PTR(-ENETDOWN);
+	}
 
 	if (!is_valid_ether_addr(edev->ethaddr)) {
 		char str[sizeof("xx:xx:xx:xx:xx:xx")];
@@ -325,7 +326,7 @@ static struct net_connection *net_new(IPaddr_t dest, rx_handler_f *handler,
 	if (dest == 0xffffffff) {
 		memset(con->et->et_dest, 0xff, 6);
 	} else {
-		ret = arp_request(dest, con->et->et_dest);
+		ret = arp_request(edev, dest, con->et->et_dest);
 		if (ret)
 			goto out;
 	}
@@ -349,10 +350,11 @@ out:
 	return ERR_PTR(ret);
 }
 
-struct net_connection *net_udp_new(IPaddr_t dest, uint16_t dport,
-		rx_handler_f *handler, void *ctx)
+struct net_connection *net_udp_eth_new(struct eth_device *edev, IPaddr_t dest,
+				       uint16_t dport, rx_handler_f *handler,
+				       void *ctx)
 {
-	struct net_connection *con = net_new(dest, handler, ctx);
+	struct net_connection *con = net_new(edev, 0xffffffff, handler, ctx);
 
 	if (IS_ERR(con))
 		return con;
@@ -365,10 +367,16 @@ struct net_connection *net_udp_new(IPaddr_t dest, uint16_t dport,
 	return con;
 }
 
+struct net_connection *net_udp_new(IPaddr_t dest, uint16_t dport,
+		rx_handler_f *handler, void *ctx)
+{
+	return net_udp_eth_new(NULL, dest, dport, handler, ctx);
+}
+
 struct net_connection *net_icmp_new(IPaddr_t dest, rx_handler_f *handler,
 		void *ctx)
 {
-	struct net_connection *con = net_new(dest, handler, ctx);
+	struct net_connection *con = net_new(NULL, dest, handler, ctx);
 
 	if (IS_ERR(con))
 		return con;
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 09/21] net: dhcp: Allow to specify network device

[Sascha Hauer]

Instead of allowing to DHCP only on the "current" network
device, allow to specify the desired network device. This
is a first step to get rid of the concept of a "current"
network device.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 commands/dhcp.c | 20 +++++++++++++++++---
 include/dhcp.h  |  4 +++-
 include/net.h   |  6 +++---
 net/dhcp.c      | 15 +++++++++------
 net/ifup.c      |  5 ++++-
 net/net.c       | 12 +++---------
 6 files changed, 39 insertions(+), 23 deletions(-)

diff --git a/commands/dhcp.c b/commands/dhcp.c
index 4f4f5490c5..ce4a78830d 100644
--- a/commands/dhcp.c
+++ b/commands/dhcp.c
@@ -15,12 +15,15 @@
 #include <environment.h>
 #include <getopt.h>
 #include <dhcp.h>
+#include <net.h>
 
 static int do_dhcp(int argc, char *argv[])
 {
 	int ret, opt;
 	int retries = DHCP_DEFAULT_RETRY;
 	struct dhcp_req_param dhcp_param;
+	struct eth_device *edev;
+	const char *edevname;
 
 	memset(&dhcp_param, 0, sizeof(struct dhcp_req_param));
 	getenv_uint("global.dhcp.retries", &retries);
@@ -50,12 +53,23 @@ static int do_dhcp(int argc, char *argv[])
 		}
 	}
 
+	if (optind == argc)
+		edevname = "eth0";
+	else
+		edevname = argv[optind];
+
+	edev = eth_get_byname(edevname);
+	if (!edev) {
+		printf("No such network device: %s\n", edevname);
+		return 1;
+	}
+
 	if (!retries) {
 		printf("retries is set to zero, set it to %d\n", DHCP_DEFAULT_RETRY);
 		retries = DHCP_DEFAULT_RETRY;
 	}
 
-	ret = dhcp(retries, &dhcp_param);
+	ret = dhcp(edev, retries, &dhcp_param);
 
 	return ret;
 }
@@ -73,8 +87,8 @@ BAREBOX_CMD_HELP_END
 BAREBOX_CMD_START(dhcp)
 	.cmd		= do_dhcp,
 	BAREBOX_CMD_DESC("DHCP client to obtain IP or boot params")
-	BAREBOX_CMD_OPTS("[-HvcuUr]")
+	BAREBOX_CMD_OPTS("[-HvcuUr] [device]")
 	BAREBOX_CMD_GROUP(CMD_GRP_NET)
 	BAREBOX_CMD_HELP(cmd_dhcp_help)
-	BAREBOX_CMD_COMPLETE(empty_complete)
+	BAREBOX_CMD_COMPLETE(eth_complete)
 BAREBOX_CMD_END
diff --git a/include/dhcp.h b/include/dhcp.h
index 0796b30cf1..20a523250f 100644
--- a/include/dhcp.h
+++ b/include/dhcp.h
@@ -20,6 +20,8 @@ struct dhcp_req_param {
 	char *client_uuid;
 };
 
-int dhcp(int retries, struct dhcp_req_param *param);
+struct eth_device;
+
+int dhcp(struct eth_device *edev, int retries, struct dhcp_req_param *param);
 
 #endif
diff --git a/include/net.h b/include/net.h
index 6788f14cb0..416fcde761 100644
--- a/include/net.h
+++ b/include/net.h
@@ -212,13 +212,13 @@ struct icmphdr {
 
 extern unsigned char *NetRxPackets[PKTBUFSRX];/* Receive packets		*/
 
-void net_set_ip(IPaddr_t ip);
+void net_set_ip(struct eth_device *edev, IPaddr_t ip);
 void net_set_serverip(IPaddr_t ip, bool overwrite);
-void net_set_netmask(IPaddr_t ip);
+void net_set_netmask(struct eth_device *edev, IPaddr_t ip);
 void net_set_gateway(IPaddr_t ip);
 void net_set_nameserver(IPaddr_t ip);
 void net_set_domainname(const char *name);
-IPaddr_t net_get_ip(void);
+IPaddr_t net_get_ip(struct eth_device *edev);
 IPaddr_t net_get_serverip(void);
 IPaddr_t net_get_gateway(void);
 IPaddr_t net_get_nameserver(void);
diff --git a/net/dhcp.c b/net/dhcp.c
index 61abf49272..270fe91ac1 100644
--- a/net/dhcp.c
+++ b/net/dhcp.c
@@ -79,6 +79,7 @@ static dhcp_state_t dhcp_state;
 static uint32_t dhcp_leasetime;
 static IPaddr_t net_dhcp_server_ip;
 static uint64_t dhcp_start;
+static struct eth_device *dhcp_edev;
 static char dhcp_tftpname[256];
 
 static const char* dhcp_get_barebox_global(const char * var)
@@ -126,7 +127,7 @@ static void netmask_handle(struct dhcp_opt *opt, unsigned char *popt, int optlen
 	IPaddr_t ip;
 
 	ip = net_read_ip(popt);
-	net_set_netmask(ip);
+	net_set_netmask(dhcp_edev, ip);
 }
 
 static void gateway_handle(struct dhcp_opt *opt, unsigned char *popt, int optlen)
@@ -369,7 +370,7 @@ static void bootp_copy_net_params(struct bootp *bp)
 	IPaddr_t tmp_ip;
 
 	tmp_ip = net_read_ip(&bp->bp_yiaddr);
-	net_set_ip(tmp_ip);
+	net_set_ip(dhcp_edev, tmp_ip);
 
 	tmp_ip = net_read_ip(&bp->bp_siaddr);
 	if (tmp_ip != 0)
@@ -618,7 +619,7 @@ static void dhcp_handler(void *ctx, char *packet, unsigned int len)
 				dhcp_options_process((u8 *)&bp->bp_vend[4], bp);
 			bootp_copy_net_params(bp); /* Store net params from reply */
 			dhcp_state = BOUND;
-			ip = net_get_ip();
+			ip = net_get_ip(dhcp_edev);
 			printf("DHCP client bound to address %pI4\n", &ip);
 			return;
 		}
@@ -646,12 +647,14 @@ static void dhcp_reset_env(void)
 	}
 }
 
-int dhcp(int retries, struct dhcp_req_param *param)
+int dhcp(struct eth_device *edev, int retries, struct dhcp_req_param *param)
 {
 	int ret = 0;
 
 	dhcp_reset_env();
 
+	dhcp_edev = edev;
+
 	dhcp_set_param_data(DHCP_HOSTNAME, param->hostname);
 	dhcp_set_param_data(DHCP_VENDOR_ID, param->vendor_id);
 	dhcp_set_param_data(DHCP_CLIENT_ID, param->client_id);
@@ -661,7 +664,7 @@ int dhcp(int retries, struct dhcp_req_param *param)
 	if (!retries)
 		retries = DHCP_DEFAULT_RETRY;
 
-	dhcp_con = net_udp_new(0xffffffff, PORT_BOOTPS, dhcp_handler, NULL);
+	dhcp_con = net_udp_eth_new(edev, 0xffffffff, PORT_BOOTPS, dhcp_handler, NULL);
 	if (IS_ERR(dhcp_con)) {
 		ret = PTR_ERR(dhcp_con);
 		goto out;
@@ -671,7 +674,7 @@ int dhcp(int retries, struct dhcp_req_param *param)
 	if (ret)
 		goto out1;
 
-	net_set_ip(0);
+	net_set_ip(dhcp_edev, 0);
 
 	dhcp_start = get_time_ns();
 	ret = bootp_request(); /* Basically same as BOOTP */
diff --git a/net/ifup.c b/net/ifup.c
index 70ecf9c717..6c298d8501 100644
--- a/net/ifup.c
+++ b/net/ifup.c
@@ -101,12 +101,15 @@ int ifup(const char *name, unsigned flags)
 
 	if (!strcmp(ip, "dhcp")) {
 		IPaddr_t serverip;
+		char *dhcp_cmd;
 
 		serverip = getenv_ip("serverip");
 		if (serverip)
 			net_set_serverip(serverip, false);
 
-		ret = run_command("dhcp");
+		dhcp_cmd = basprintf("dhcp %s", name);
+		ret = run_command(dhcp_cmd);
+		free(dhcp_cmd);
 		if (ret)
 			goto out;
 		dev_set_param(dev, "linux.bootargs", "ip=dhcp");
diff --git a/net/net.c b/net/net.c
index 1d47bb449a..cf1d0b32fa 100644
--- a/net/net.c
+++ b/net/net.c
@@ -254,24 +254,18 @@ void net_set_serverip(IPaddr_t ip, bool overwrite)
 	net_serverip = ip;
 }
 
-void net_set_ip(IPaddr_t ip)
+void net_set_ip(struct eth_device *edev, IPaddr_t ip)
 {
-	struct eth_device *edev = eth_get_current();
-
 	edev->ipaddr = ip;
 }
 
-IPaddr_t net_get_ip(void)
+IPaddr_t net_get_ip(struct eth_device *edev)
 {
-	struct eth_device *edev = eth_get_current();
-
 	return edev->ipaddr;
 }
 
-void net_set_netmask(IPaddr_t nm)
+void net_set_netmask(struct eth_device *edev, IPaddr_t nm)
 {
-	struct eth_device *edev = eth_get_current();
-
 	edev->netmask = nm;
 }
 
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 10/21] net: dhcp: avoid unnecessary casts

[Sascha Hauer]

Drop explicit casts to/from void pointers.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 net/dhcp.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/net/dhcp.c b/net/dhcp.c
index 270fe91ac1..fe3f7f28b6 100644
--- a/net/dhcp.c
+++ b/net/dhcp.c
@@ -447,14 +447,13 @@ static int bootp_request(void)
 	struct bootp *bp;
 	int ext_len;
 	int ret;
-	unsigned char *payload = net_udp_get_payload(dhcp_con);
 	const char *bfile;
 
 	dhcp_state = INIT;
 
 	debug("BOOTP broadcast\n");
 
-	bp = (struct bootp *)payload;
+	bp = net_udp_get_payload(dhcp_con);;
 	bp->bp_op = OP_BOOTREQUEST;
 	bp->bp_htype = HWT_ETHER;
 	bp->bp_hlen = HWL_ETHER;
@@ -471,7 +470,7 @@ static int bootp_request(void)
 		safe_strncpy (bp->bp_file, bfile, sizeof(bp->bp_file));
 
 	/* Request additional information from the BOOTP/DHCP server */
-	ext_len = dhcp_extended((u8 *)bp->bp_vend, DHCP_DISCOVER, 0, 0);
+	ext_len = dhcp_extended(bp->bp_vend, DHCP_DISCOVER, 0, 0);
 
 	Bootp_id = (uint32_t)get_time_ns();
 	net_copy_uint32(&bp->bp_id, &Bootp_id);
@@ -483,7 +482,7 @@ static int bootp_request(void)
 	return ret;
 }
 
-static void dhcp_options_handle(unsigned char option, unsigned char *popt,
+static void dhcp_options_handle(unsigned char option, void *popt,
 			       int optlen, struct bootp *bp)
 {
 	int i;
@@ -537,11 +536,10 @@ static void dhcp_send_request_packet(struct bootp *bp_offer)
 	struct bootp *bp;
 	int extlen;
 	IPaddr_t OfferedIP;
-	unsigned char *payload = net_udp_get_payload(dhcp_con);
 
 	debug("%s: Sending DHCPREQUEST\n", __func__);
 
-	bp = (struct bootp *)payload;
+	bp = net_udp_get_payload(dhcp_con);
 	bp->bp_op = OP_BOOTREQUEST;
 	bp->bp_htype = HWT_ETHER;
 	bp->bp_hlen = HWL_ETHER;
@@ -566,7 +564,7 @@ static void dhcp_send_request_packet(struct bootp *bp_offer)
 	 * Copy options from OFFER packet if present
 	 */
 	net_copy_ip(&OfferedIP, &bp_offer->bp_yiaddr);
-	extlen = dhcp_extended((u8 *)bp->bp_vend, DHCP_REQUEST, net_dhcp_server_ip,
+	extlen = dhcp_extended(bp->bp_vend, DHCP_REQUEST, net_dhcp_server_ip,
 				OfferedIP);
 
 	debug("Transmitting DHCPREQUEST packet\n");
@@ -601,7 +599,7 @@ static void dhcp_handler(void *ctx, char *packet, unsigned int len)
 		debug ("%s: state SELECTING, bp_file: \"%s\"\n", __func__, bp->bp_file);
 		dhcp_state = REQUESTING;
 
-		if (net_read_uint32((uint32_t *)&bp->bp_vend[0]) == htonl(BOOTP_VENDOR_MAGIC))
+		if (net_read_uint32(&bp->bp_vend[0]) == htonl(BOOTP_VENDOR_MAGIC))
 			dhcp_options_process((u8 *)&bp->bp_vend[4], bp);
 
 		bootp_copy_net_params(bp); /* Store net params from reply */
@@ -615,8 +613,8 @@ static void dhcp_handler(void *ctx, char *packet, unsigned int len)
 
 		if (dhcp_message_type((u8 *)bp->bp_vend) == DHCP_ACK ) {
 			IPaddr_t ip;
-			if (net_read_uint32((uint32_t *)&bp->bp_vend[0]) == htonl(BOOTP_VENDOR_MAGIC))
-				dhcp_options_process((u8 *)&bp->bp_vend[4], bp);
+			if (net_read_uint32(&bp->bp_vend[0]) == htonl(BOOTP_VENDOR_MAGIC))
+				dhcp_options_process(&bp->bp_vend[4], bp);
 			bootp_copy_net_params(bp); /* Store net params from reply */
 			dhcp_state = BOUND;
 			ip = net_get_ip(dhcp_edev);
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 11/21] net: dhcp: Coding style fixes

[Sascha Hauer]

Fix some spaces-before-function-opening-brackets and duplicate empty
lines.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 net/dhcp.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/net/dhcp.c b/net/dhcp.c
index fe3f7f28b6..4403166d2a 100644
--- a/net/dhcp.c
+++ b/net/dhcp.c
@@ -389,7 +389,7 @@ static void bootp_copy_net_params(struct bootp *bp)
 /*
  * Initialize BOOTP extension fields in the request.
  */
-static int dhcp_extended (u8 *e, int message_type, IPaddr_t ServerID,
+static int dhcp_extended(u8 *e, int message_type, IPaddr_t ServerID,
 			  IPaddr_t RequestedIP)
 {
 	struct dhcp_opt *opt;
@@ -411,7 +411,6 @@ static int dhcp_extended (u8 *e, int message_type, IPaddr_t ServerID,
 	*e++ = (576 - 312 + OPT_SIZE) >> 8;
 	*e++ = (576 - 312 + OPT_SIZE) & 0xff;
 
-
 	e += dhcp_set_ip_options(50, e, RequestedIP);
 	e += dhcp_set_ip_options(54, e, ServerID);
 
@@ -467,7 +466,7 @@ static int bootp_request(void)
 
 	bfile = getenv("bootfile");
 	if (bfile)
-		safe_strncpy (bp->bp_file, bfile, sizeof(bp->bp_file));
+		safe_strncpy(bp->bp_file, bfile, sizeof(bp->bp_file));
 
 	/* Request additional information from the BOOTP/DHCP server */
 	ext_len = dhcp_extended(bp->bp_vend, DHCP_DISCOVER, 0, 0);
@@ -523,8 +522,8 @@ static int dhcp_message_type(unsigned char *popt)
 		return -1;
 
 	popt += 4;
-	while ( *popt != 0xff ) {
-		if ( *popt == 53 )	/* DHCP Message Type */
+	while (*popt != 0xff) {
+		if (*popt == 53)	/* DHCP Message Type */
 			return *(popt + 2);
 		popt += *(popt + 1) + 2;	/* Scan through all options */
 	}
@@ -596,7 +595,7 @@ static void dhcp_handler(void *ctx, char *packet, unsigned int len)
 		 * If filename is in format we recognize, assume it is a valid
 		 * OFFER from a server we want.
 		 */
-		debug ("%s: state SELECTING, bp_file: \"%s\"\n", __func__, bp->bp_file);
+		debug("%s: state SELECTING, bp_file: \"%s\"\n", __func__, bp->bp_file);
 		dhcp_state = REQUESTING;
 
 		if (net_read_uint32(&bp->bp_vend[0]) == htonl(BOOTP_VENDOR_MAGIC))
@@ -609,7 +608,7 @@ static void dhcp_handler(void *ctx, char *packet, unsigned int len)
 
 		break;
 	case REQUESTING:
-		debug ("%s: State REQUESTING\n", __func__);
+		debug("%s: State REQUESTING\n", __func__);
 
 		if (dhcp_message_type((u8 *)bp->bp_vend) == DHCP_ACK ) {
 			IPaddr_t ip;
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 12/21] net: dhcp: rework

[Sascha Hauer]

The DHCP code is a mess. It is not clear which options are sent to the
server and which options are returned from the server. Also environment
variables are read from and written to all over the place.

This patch cleans this up. There now is struct dhcp_req_param which is
used for options sent to the server and struct dhcp_result which contains
the values sent from the server. The values from the server are written
to the barebox variables in a single place. Also it's now possible to
call the dhcp code without modifying barebox variables at all, storing
the result only in the dhcp result struct.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 commands/dhcp.c |  15 +-
 include/dhcp.h  |  22 ++-
 net/dhcp.c      | 532 ++++++++++++++++++++++----------------------------------
 3 files changed, 234 insertions(+), 335 deletions(-)

diff --git a/commands/dhcp.c b/commands/dhcp.c
index ce4a78830d..1f07b2f2ce 100644
--- a/commands/dhcp.c
+++ b/commands/dhcp.c
@@ -20,14 +20,10 @@
 static int do_dhcp(int argc, char *argv[])
 {
 	int ret, opt;
-	int retries = DHCP_DEFAULT_RETRY;
-	struct dhcp_req_param dhcp_param;
+	struct dhcp_req_param dhcp_param = {};
 	struct eth_device *edev;
 	const char *edevname;
 
-	memset(&dhcp_param, 0, sizeof(struct dhcp_req_param));
-	getenv_uint("global.dhcp.retries", &retries);
-
 	while ((opt = getopt(argc, argv, "H:v:c:u:U:r:")) > 0) {
 		switch (opt) {
 		case 'H':
@@ -46,7 +42,7 @@ static int do_dhcp(int argc, char *argv[])
 			dhcp_param.user_class = optarg;
 			break;
 		case 'r':
-			retries = simple_strtoul(optarg, NULL, 10);
+			dhcp_param.retries = simple_strtoul(optarg, NULL, 10);
 			break;
 		default:
 			return COMMAND_ERROR_USAGE;
@@ -64,12 +60,7 @@ static int do_dhcp(int argc, char *argv[])
 		return 1;
 	}
 
-	if (!retries) {
-		printf("retries is set to zero, set it to %d\n", DHCP_DEFAULT_RETRY);
-		retries = DHCP_DEFAULT_RETRY;
-	}
-
-	ret = dhcp(edev, retries, &dhcp_param);
+	ret = dhcp(edev, &dhcp_param);
 
 	return ret;
 }
diff --git a/include/dhcp.h b/include/dhcp.h
index 20a523250f..0977ff42da 100644
--- a/include/dhcp.h
+++ b/include/dhcp.h
@@ -18,10 +18,30 @@ struct dhcp_req_param {
 	char *client_id;
 	char *user_class;
 	char *client_uuid;
+	int retries;
+};
+
+struct dhcp_result {
+	IPaddr_t ip;
+	IPaddr_t netmask;
+	IPaddr_t gateway;
+	IPaddr_t nameserver;
+	IPaddr_t serverip;
+	char *hostname;
+	char *domainname;
+	char *rootpath;
+	char *devicetree;
+	char *bootfile;
+	char *tftp_server_name;
+	uint32_t leasetime;
 };
 
 struct eth_device;
 
-int dhcp(struct eth_device *edev, int retries, struct dhcp_req_param *param);
+int dhcp_request(struct eth_device *edev, const struct dhcp_req_param *param,
+		 struct dhcp_result **res);
+int dhcp_set_result(struct eth_device *edev, struct dhcp_result *res);
+void dhcp_result_free(struct dhcp_result *res);
+int dhcp(struct eth_device *edev, const struct dhcp_req_param *param);
 
 #endif
diff --git a/net/dhcp.c b/net/dhcp.c
index 4403166d2a..b3dcdfdd3e 100644
--- a/net/dhcp.c
+++ b/net/dhcp.c
@@ -76,244 +76,30 @@ typedef enum {
 
 static uint32_t Bootp_id;
 static dhcp_state_t dhcp_state;
-static uint32_t dhcp_leasetime;
 static IPaddr_t net_dhcp_server_ip;
 static uint64_t dhcp_start;
 static struct eth_device *dhcp_edev;
-static char dhcp_tftpname[256];
-
-static const char* dhcp_get_barebox_global(const char * var)
-{
-	char * var_global = basprintf("global.dhcp.%s", var);
-	const char *val;
-
-	if (!var_global)
-		return NULL;
-
-	val = getenv(var_global);
-	free(var_global);
-	return val;
-}
-
-static int dhcp_set_barebox_global(const char * var, char *val)
-{
-	char * var_global = basprintf("global.dhcp.%s", var);
-	int ret;
-
-	if (!var_global)
-		return -ENOMEM;
-
-	ret = setenv(var_global, val);
-	free(var_global);
-	return ret;
-}
-
-struct dhcp_opt {
-	unsigned char option;
-	/* request automatically the option when creating the DHCP request */
-	bool optional;
-	bool copy_only_if_valid;
-	const char *barebox_var_name;
-	const char *barebox_dhcp_global;
-	void (*handle)(struct dhcp_opt *opt, unsigned char *data, int tlen);
-	int (*handle_param)(struct dhcp_opt *dhcp_opt, u8 *e);
-	void *data;
-
-	struct bootp *bp;
+struct dhcp_req_param dhcp_param;
+struct dhcp_result *dhcp_result;
+
+struct dhcp_receivce_opts {
+	IPaddr_t netmask;
+	IPaddr_t gateway;
+	IPaddr_t nameserver;
+	IPaddr_t serverip;
+	char *hostname;
+	char *domainname;
+	char *rootpath;
+	char *devicetree;
+	char *bootfile;
 };
 
-static void netmask_handle(struct dhcp_opt *opt, unsigned char *popt, int optlen)
-{
-	IPaddr_t ip;
-
-	ip = net_read_ip(popt);
-	net_set_netmask(dhcp_edev, ip);
-}
-
-static void gateway_handle(struct dhcp_opt *opt, unsigned char *popt, int optlen)
-{
-	IPaddr_t ip;
-
-	ip = net_read_ip(popt);
-	net_set_gateway(ip);
-}
-
-static void env_ip_handle(struct dhcp_opt *opt, unsigned char *popt, int optlen)
-{
-	IPaddr_t ip;
-
-	ip = net_read_ip(popt);
-	if (IS_ENABLED(CONFIG_ENVIRONMENT_VARIABLES))
-		setenv_ip(opt->barebox_var_name, ip);
-}
-
-static void env_str_handle(struct dhcp_opt *opt, unsigned char *popt, int optlen)
-{
-	char str[256];
-	char *tmp = str;
-
-	if (opt->data)
-		tmp = opt->data;
-
-	memcpy(tmp, popt, optlen);
-	tmp[optlen] = 0;
-
-	if (opt->copy_only_if_valid && !strlen(tmp))
-		return;
-	if (opt->barebox_var_name && IS_ENABLED(CONFIG_ENVIRONMENT_VARIABLES))
-		setenv(opt->barebox_var_name, tmp);
-	if (opt->barebox_dhcp_global && IS_ENABLED(CONFIG_GLOBALVAR))
-		dhcp_set_barebox_global(opt->barebox_dhcp_global, tmp);
-
-}
-
-static void copy_uint32_handle(struct dhcp_opt *opt, unsigned char *popt, int optlen)
-{
-	net_copy_uint32(opt->data, (uint32_t *)popt);
-};
-
-static void copy_ip_handle(struct dhcp_opt *opt, unsigned char *popt, int optlen)
-{
-	net_copy_ip(opt->data, popt);
-};
-
-static void bootfile_vendorex_handle(struct dhcp_opt *opt, unsigned char *popt, int optlen)
-{
-	if (opt->bp->bp_file[0] != '\0')
-		return;
-
-	/*
-	 * only use vendor boot file if we didn't
-	 * receive a boot file in the main non-vendor
-	 * part of the packet - god only knows why
-	 * some vendors chose not to use this perfectly
-	 * good spot to store the boot file (join on
-	 * Tru64 Unix) it seems mind bogglingly crazy
-	 * to me
-	 */
-	pr_warn("*** WARNING: using vendor optional boot file\n");
-
-	/*
-	 * I can't use dhcp_vendorex_proc here because I need
-	 * to write into the bootp packet - even then I had to
-	 * pass the bootp packet pointer into here as the
-	 * second arg
-	 */
-	env_str_handle(opt, popt, optlen);
-}
-
-static int dhcp_set_string_options(struct dhcp_opt *param, u8 *e)
-{
-	int str_len;
-	const char *str = param->data;
-
-	if (!str && param->barebox_var_name && IS_ENABLED(CONFIG_ENVIRONMENT_VARIABLES))
-		str = getenv(param->barebox_var_name);
-
-	if (!str && param->barebox_dhcp_global && IS_ENABLED(CONFIG_GLOBALVAR))
-		str = dhcp_get_barebox_global(param->barebox_dhcp_global);
-
-	if (!str)
-		return 0;
-
-	str_len = strlen(str);
-	if (!str_len)
-		return 0;
-
-	*e++ = param->option;
-	*e++ = str_len;
-	memcpy(e, str, str_len);
-
-	return str_len + 2;
-}
-
 #define DHCP_HOSTNAME		12
 #define DHCP_VENDOR_ID		60
 #define DHCP_CLIENT_ID		61
 #define DHCP_USER_CLASS		77
 #define DHCP_CLIENT_UUID	97
 
-struct dhcp_opt dhcp_options[] = {
-	{
-		.option = 1,
-		.handle = netmask_handle,
-	}, {
-		.option = 3,
-		.handle = gateway_handle,
-	}, {
-		.option = 6,
-		.handle = env_ip_handle,
-		.barebox_var_name = "global.net.nameserver",
-	}, {
-		.option = DHCP_HOSTNAME,
-		.copy_only_if_valid = 1,
-		.handle = env_str_handle,
-		.handle_param = dhcp_set_string_options,
-		.barebox_var_name = "global.hostname",
-	}, {
-		.option = 15,
-		.handle = env_str_handle,
-		.barebox_var_name = "global.net.domainname",
-	}, {
-		.option = 17,
-		.handle = env_str_handle,
-		.barebox_dhcp_global = "rootpath",
-	}, {
-		.option = 51,
-		.handle = copy_uint32_handle,
-		.data = &dhcp_leasetime,
-	}, {
-		.option = 54,
-		.handle = copy_ip_handle,
-		.data = &net_dhcp_server_ip,
-		.optional = true,
-	}, {
-		.option = DHCP_VENDOR_ID,
-		.handle_param = dhcp_set_string_options,
-		.barebox_dhcp_global = "vendor_id",
-	},{
-		.option = 66,
-		.handle = env_str_handle,
-		.barebox_dhcp_global = "tftp_server_name",
-		.data = dhcp_tftpname,
-	}, {
-		.option = 67,
-		.handle = bootfile_vendorex_handle,
-		.barebox_dhcp_global = "bootfile",
-	}, {
-		.option = DHCP_CLIENT_ID,
-		.handle_param = dhcp_set_string_options,
-		.barebox_dhcp_global = "client_id",
-	}, {
-		.option = DHCP_USER_CLASS,
-		.handle_param = dhcp_set_string_options,
-		.barebox_dhcp_global = "user_class",
-	}, {
-		.option = DHCP_CLIENT_UUID,
-		.handle_param = dhcp_set_string_options,
-		.barebox_dhcp_global = "client_uuid",
-	}, {
-		.option = 224,
-		.handle = env_str_handle,
-		.barebox_dhcp_global = "oftree_file",
-	},
-};
-
-static void dhcp_set_param_data(int option, void* data)
-{
-	struct dhcp_opt *opt;
-	int i;
-
-	for (i = 0; i < ARRAY_SIZE(dhcp_options); i++) {
-		opt = &dhcp_options[i];
-
-		if (opt->option == option) {
-			opt->data = data;
-			return;
-		}
-	}
-}
-
 static int dhcp_set_ip_options(int option, u8 *e, IPaddr_t ip)
 {
 	int tmp;
@@ -367,23 +153,30 @@ static int bootp_check_packet(unsigned char *pkt, unsigned src, unsigned len)
  */
 static void bootp_copy_net_params(struct bootp *bp)
 {
-	IPaddr_t tmp_ip;
 
-	tmp_ip = net_read_ip(&bp->bp_yiaddr);
-	net_set_ip(dhcp_edev, tmp_ip);
+	dhcp_result->ip = net_read_ip(&bp->bp_yiaddr);
+	dhcp_result->serverip = net_read_ip(&bp->bp_siaddr);
+
+	if (strlen(bp->bp_file) > 0)
+		dhcp_result->bootfile = xstrdup(bp->bp_file);
+}
 
-	tmp_ip = net_read_ip(&bp->bp_siaddr);
-	if (tmp_ip != 0)
-		net_set_serverip(tmp_ip, false);
+static int dhcp_set_string_options(int option, const char *str, u8 *e)
+{
+	int str_len;
 
-	if (strlen(bp->bp_file) > 0) {
-		if (IS_ENABLED(CONFIG_ENVIRONMENT_VARIABLES))
-			setenv("bootfile", bp->bp_file);
-		if (IS_ENABLED(CONFIG_GLOBALVAR))
-			dhcp_set_barebox_global("bootfile", bp->bp_file);
-	}
+	if (!str)
+		return 0;
 
-	debug("bootfile: %s\n", bp->bp_file);
+	str_len = strlen(str);
+	if (!str_len)
+		return 0;
+
+	*e++ = option;
+	*e++ = str_len;
+	memcpy(e, str, str_len);
+
+	return str_len + 2;
 }
 
 /*
@@ -392,10 +185,11 @@ static void bootp_copy_net_params(struct bootp *bp)
 static int dhcp_extended(u8 *e, int message_type, IPaddr_t ServerID,
 			  IPaddr_t RequestedIP)
 {
-	struct dhcp_opt *opt;
 	int i;
 	u8 *start = e;
 	u8 *cnt;
+	u8 dhcp_options[] = {1, 3, 6, DHCP_HOSTNAME, 15, 17, 51, DHCP_VENDOR_ID, 66, 67, DHCP_CLIENT_ID,
+			     DHCP_USER_CLASS, DHCP_CLIENT_UUID, 224};
 
 	*e++ = 99;		/* RFC1048 Magic Cookie */
 	*e++ = 130;
@@ -414,22 +208,20 @@ static int dhcp_extended(u8 *e, int message_type, IPaddr_t ServerID,
 	e += dhcp_set_ip_options(50, e, RequestedIP);
 	e += dhcp_set_ip_options(54, e, ServerID);
 
-	for (i = 0; i < ARRAY_SIZE(dhcp_options); i++) {
-		opt = &dhcp_options[i];
-		if (opt->handle_param)
-			e += opt->handle_param(opt, e);
-	}
+	e += dhcp_set_string_options(DHCP_HOSTNAME, dhcp_param.hostname, e);
+	e += dhcp_set_string_options(DHCP_VENDOR_ID, dhcp_param.vendor_id, e);
+	e += dhcp_set_string_options(DHCP_CLIENT_ID, dhcp_param.client_id, e);
+	e += dhcp_set_string_options(DHCP_USER_CLASS, dhcp_param.user_class, e);
+	e += dhcp_set_string_options(DHCP_CLIENT_UUID, dhcp_param.client_uuid, e);
 
 	*e++ = 55;		/* Parameter Request List */
-	 cnt = e++;		/* Pointer to count of requested items */
-	*cnt = 0;
-
-	for (i = 0; i < ARRAY_SIZE(dhcp_options); i++) {
-		if (dhcp_options[i].optional)
-			continue;
-		*e++  = dhcp_options[i].option;
-		*cnt += 1;
-	}
+	cnt = e++;		/* Pointer to count of requested items */
+
+	for (i = 0; i < ARRAY_SIZE(dhcp_options); i++)
+		*e++ = dhcp_options[i];
+
+	*cnt = ARRAY_SIZE(dhcp_options);
+
 	*e++  = 255;		/* End of the list */
 
 	/* Pad to minimal length */
@@ -484,20 +276,44 @@ static int bootp_request(void)
 static void dhcp_options_handle(unsigned char option, void *popt,
 			       int optlen, struct bootp *bp)
 {
-	int i;
-	struct dhcp_opt *opt;
-
-	for (i = 0; i < ARRAY_SIZE(dhcp_options); i++) {
-		opt = &dhcp_options[i];
-		if (opt->option == option) {
-			opt->bp = bp;
-			if (opt->handle)
-				opt->handle(opt, popt, optlen);
-			return;
-		}
+	switch (option) {
+		case 1:
+			dhcp_result->netmask = net_read_ip(popt);
+			break;
+		case 3:
+			dhcp_result->gateway = net_read_ip(popt);
+			break;
+		case 6:
+			dhcp_result->nameserver = net_read_ip(popt);
+			break;
+		case DHCP_HOSTNAME:
+			dhcp_result->hostname = xstrndup(popt, optlen);
+			break;
+		case 15:
+			dhcp_result->domainname = xstrndup(popt, optlen);
+			break;
+		case 17:
+			dhcp_result->rootpath = xstrndup(popt, optlen);
+			break;
+		case 51:
+			net_copy_uint32(&dhcp_result->leasetime, popt);
+			break;
+		case 54:
+			dhcp_result->serverip = net_read_ip(popt);
+			break;
+		case 66:
+			dhcp_result->tftp_server_name = xstrndup(popt, optlen);
+			break;
+		case 67:
+			if (!dhcp_result->bootfile)
+				dhcp_result->bootfile = xstrndup(popt, optlen);
+			break;
+		case 224:
+			dhcp_result->devicetree = xstrndup(popt, optlen);
+			break;
+		default:
+			debug("*** Unhandled DHCP Option in OFFER/ACK: %d\n", option);
 	}
-
-	debug("*** Unhandled DHCP Option in OFFER/ACK: %d\n", option);
 }
 
 static void dhcp_options_process(unsigned char *popt, struct bootp *bp)
@@ -611,13 +427,11 @@ static void dhcp_handler(void *ctx, char *packet, unsigned int len)
 		debug("%s: State REQUESTING\n", __func__);
 
 		if (dhcp_message_type((u8 *)bp->bp_vend) == DHCP_ACK ) {
-			IPaddr_t ip;
 			if (net_read_uint32(&bp->bp_vend[0]) == htonl(BOOTP_VENDOR_MAGIC))
 				dhcp_options_process(&bp->bp_vend[4], bp);
 			bootp_copy_net_params(bp); /* Store net params from reply */
 			dhcp_state = BOUND;
-			ip = net_get_ip(dhcp_edev);
-			printf("DHCP client bound to address %pI4\n", &ip);
+			dev_info(&dhcp_edev->dev, "DHCP client bound to address %pI4\n", &dhcp_result->ip);
 			return;
 		}
 		break;
@@ -627,39 +441,48 @@ static void dhcp_handler(void *ctx, char *packet, unsigned int len)
 	}
 }
 
-static void dhcp_reset_env(void)
-{
-	struct dhcp_opt *opt;
-	int i;
+static char *global_dhcp_user_class;
+static char *global_dhcp_vendor_id;
+static char *global_dhcp_client_uuid;
+static char *global_dhcp_client_id;
+static char *global_dhcp_bootfile;
+static char *global_dhcp_oftree_file;
+static char *global_dhcp_rootpath;
+static char *global_dhcp_tftp_server_name;
 
-	for (i = 0; i < ARRAY_SIZE(dhcp_options); i++) {
-		opt = &dhcp_options[i];
-		if (!opt->barebox_var_name || opt->copy_only_if_valid)
-			continue;
+static void set_res(char **var, const char *res)
+{
+	free(*var);
 
-		if (IS_ENABLED(CONFIG_ENVIRONMENT_VARIABLES))
-			setenv(opt->barebox_var_name, "");
-		if (opt->barebox_dhcp_global && IS_ENABLED(CONFIG_GLOBALVAR))
-			dhcp_set_barebox_global(opt->barebox_dhcp_global, "");
-	}
+	if (res)
+		*var = xstrdup(res);
+	else
+		*var = xstrdup("");
 }
 
-int dhcp(struct eth_device *edev, int retries, struct dhcp_req_param *param)
+int dhcp_request(struct eth_device *edev, const struct dhcp_req_param *param,
+		 struct dhcp_result **res)
 {
 	int ret = 0;
 
-	dhcp_reset_env();
-
 	dhcp_edev = edev;
-
-	dhcp_set_param_data(DHCP_HOSTNAME, param->hostname);
-	dhcp_set_param_data(DHCP_VENDOR_ID, param->vendor_id);
-	dhcp_set_param_data(DHCP_CLIENT_ID, param->client_id);
-	dhcp_set_param_data(DHCP_USER_CLASS, param->user_class);
-	dhcp_set_param_data(DHCP_CLIENT_UUID, param->client_uuid);
-
-	if (!retries)
-		retries = DHCP_DEFAULT_RETRY;
+	if (param)
+		dhcp_param = *param;
+	else
+		memset(&dhcp_param, 0, sizeof(dhcp_param));
+
+	dhcp_result = xzalloc(sizeof(*dhcp_result));
+
+	if (!dhcp_param.user_class)
+		dhcp_param.user_class = global_dhcp_user_class;
+	if (!dhcp_param.vendor_id)
+		dhcp_param.vendor_id = global_dhcp_vendor_id;
+	if (!dhcp_param.client_uuid)
+		dhcp_param.client_uuid = global_dhcp_client_uuid;
+	if (!dhcp_param.client_id)
+		dhcp_param.client_id = global_dhcp_client_id;
+	if (!dhcp_param.retries)
+		dhcp_param.retries = DHCP_DEFAULT_RETRY;
 
 	dhcp_con = net_udp_eth_new(edev, 0xffffffff, PORT_BOOTPS, dhcp_handler, NULL);
 	if (IS_ERR(dhcp_con)) {
@@ -671,7 +494,7 @@ int dhcp(struct eth_device *edev, int retries, struct dhcp_req_param *param)
 	if (ret)
 		goto out1;
 
-	net_set_ip(dhcp_edev, 0);
+	net_set_ip(edev, 0);
 
 	dhcp_start = get_time_ns();
 	ret = bootp_request(); /* Basically same as BOOTP */
@@ -683,7 +506,7 @@ int dhcp(struct eth_device *edev, int retries, struct dhcp_req_param *param)
 			ret = -EINTR;
 			goto out1;
 		}
-		if (!retries) {
+		if (!dhcp_param.retries) {
 			ret = -ETIMEDOUT;
 			goto out1;
 		}
@@ -693,56 +516,122 @@ int dhcp(struct eth_device *edev, int retries, struct dhcp_req_param *param)
 			printf("T ");
 			ret = bootp_request();
 			/* no need to check if retries > 0 as we check if != 0 */
-			retries--;
+			dhcp_param.retries--;
 			if (ret)
 				goto out1;
 		}
 	}
 
-	if (dhcp_tftpname[0] != 0) {
-		IPaddr_t tftpserver = resolv(dhcp_tftpname);
-		if (tftpserver)
-			net_set_serverip(tftpserver, false);
-	}
+	pr_debug("DHCP result:\n"
+		"  ip: %pI4\n"
+		"  netmask: %pI4\n"
+		"  gateway: %pI4\n"
+		"  serverip: %pI4\n"
+		"  nameserver: %pI4\n"
+		"  hostname: %s\n"
+		"  domainname: %s\n"
+		"  rootpath: %s\n"
+		"  devicetree: %s\n"
+		"  tftp_server_name: %s\n",
+		&dhcp_result->ip,
+		&dhcp_result->netmask,
+		&dhcp_result->gateway,
+		&dhcp_result->serverip,
+		&dhcp_result->nameserver,
+		dhcp_result->hostname ? dhcp_result->hostname : "",
+		dhcp_result->domainname ? dhcp_result->domainname : "",
+		dhcp_result->rootpath ? dhcp_result->rootpath : "",
+		dhcp_result->devicetree ? dhcp_result->devicetree : "",
+		dhcp_result->tftp_server_name ? dhcp_result->tftp_server_name : "");
 
 out1:
 	net_unregister(dhcp_con);
 out:
-	if (ret)
+	if (ret) {
 		debug("dhcp failed: %s\n", strerror(-ret));
+		free(dhcp_result);
+	} else {
+		*res = dhcp_result;
+	}
 
 	return ret;
 }
 
-#ifdef CONFIG_GLOBALVAR
-static void dhcp_global_add(const char *var)
+int dhcp_set_result(struct eth_device *edev, struct dhcp_result *res)
 {
-	char *var_global = basprintf("dhcp.%s", var);
+	net_set_ip(edev, res->ip);
+	net_set_netmask(edev, res->netmask);
+	net_set_gateway(res->gateway);
+	net_set_nameserver(res->nameserver);
+
+	set_res(&global_dhcp_bootfile, res->bootfile);
+	set_res(&global_dhcp_oftree_file, res->devicetree);
+	set_res(&global_dhcp_rootpath, res->rootpath);
+	set_res(&global_dhcp_tftp_server_name, res->tftp_server_name);
+
+	if (res->hostname)
+		barebox_set_hostname(res->hostname);
+	if (res->domainname)
+		net_set_domainname(res->domainname);
+
+	if (res->serverip) {
+		net_set_serverip(res->serverip, false);
+	} else if (res->tftp_server_name) {
+		IPaddr_t ip;
+
+		ip = resolv(res->tftp_server_name);
+		if (ip)
+			net_set_serverip(ip, false);
+	}
 
-	if (!var_global)
-		return;
+	return 0;
+}
 
-	globalvar_add_simple(var_global, NULL);
-	free(var_global);
+void dhcp_result_free(struct dhcp_result *res)
+{
+	free(res->hostname);
+	free(res->domainname);
+	free(res->rootpath);
+	free(res->devicetree);
+	free(res->bootfile);
+	free(res->tftp_server_name);
+
+	free(res);
 }
 
-static int dhcp_global_init(void)
+int dhcp(struct eth_device *edev, const struct dhcp_req_param *param)
 {
-	struct dhcp_opt *opt;
-	int i;
+	struct dhcp_result *res;
+	int ret;
 
-	for (i = 0; i < ARRAY_SIZE(dhcp_options); i++) {
-		opt = &dhcp_options[i];
+	ret = dhcp_request(edev, param, &res);
+	if (ret)
+		return ret;
 
-		if (!opt->barebox_dhcp_global)
-			continue;
+	ret = dhcp_set_result(edev, res);
 
-		dhcp_global_add(opt->barebox_dhcp_global);
-	}
+	dhcp_result_free(res);
+
+	return ret;
+}
+
+#ifdef CONFIG_GLOBALVAR
+
+static int dhcp_global_init(void)
+{
+	globalvar_add_simple_string("dhcp.bootfile", &global_dhcp_bootfile);
+	globalvar_add_simple_string("dhcp.rootpath", &global_dhcp_rootpath);
+	globalvar_add_simple_string("dhcp.vendor_id", &global_dhcp_vendor_id);
+	globalvar_add_simple_string("dhcp.client_uuid", &global_dhcp_client_uuid);
+	globalvar_add_simple_string("dhcp.client_id", &global_dhcp_client_id);
+	globalvar_add_simple_string("dhcp.user_class", &global_dhcp_user_class);
+	globalvar_add_simple_string("dhcp.oftree_file", &global_dhcp_oftree_file);
+	globalvar_add_simple_string("dhcp.tftp_server_name", &global_dhcp_tftp_server_name);
 
 	return 0;
 }
 late_initcall(dhcp_global_init);
+#endif
 
 BAREBOX_MAGICVAR_NAMED(global_dhcp_bootfile, global.dhcp.bootfile, "bootfile returned from DHCP request");
 BAREBOX_MAGICVAR_NAMED(global_dhcp_rootpath, global.dhcp.rootpath, "rootpath returned from DHCP request");
@@ -753,4 +642,3 @@ BAREBOX_MAGICVAR_NAMED(global_dhcp_user_class, global.dhcp.user_class, "user cla
 BAREBOX_MAGICVAR_NAMED(global_dhcp_tftp_server_name, global.dhcp.tftp_server_name, "TFTP server Name returned from DHCP request");
 BAREBOX_MAGICVAR_NAMED(global_dhcp_oftree_file, global.dhcp.oftree_file, "OF tree returned from DHCP request (option 224)");
 BAREBOX_MAGICVAR_NAMED(global_dhcp_retries, global.dhcp.retries, "retry limit");
-#endif
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 13/21] net: Pick network device based on IP settings

[Sascha Hauer]

The IP/netmask/gateway settings contain all informations
needed to pick the correct network device. This patch
adds support for that and makes specifying the "current"
network interface using the ethact command unnecessary.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 net/net.c | 29 ++++++++++++++++++++++++++---
 1 file changed, 26 insertions(+), 3 deletions(-)

diff --git a/net/net.c b/net/net.c
index cf1d0b32fa..efe6a6369f 100644
--- a/net/net.c
+++ b/net/net.c
@@ -141,6 +141,24 @@ static void arp_handler(struct arprequest *arp)
 	}
 }
 
+static struct eth_device *net_route(IPaddr_t dest)
+{
+	struct eth_device *edev;
+
+	for_each_netdev(edev) {
+		if ((dest & edev->netmask) == (edev->ipaddr & edev->netmask)) {
+			debug("Route: Using %s (ip=%pI4, nm=%pI4) to reach %pI4\n",
+			      dev_name(&edev->dev), &edev->ipaddr, &edev->netmask,
+				       &dest);
+			return edev;
+		}
+	}
+
+	debug("Route: No device found for %pI4\n", &dest);
+
+	return NULL;
+}
+
 static int arp_request(struct eth_device *edev, IPaddr_t dest, unsigned char *ether)
 {
 	char *pkt;
@@ -151,6 +169,9 @@ static int arp_request(struct eth_device *edev, IPaddr_t dest, unsigned char *et
 	unsigned retries = 0;
 	int ret;
 
+	if (!edev)
+		return -EHOSTUNREACH;
+
 	if (!arp_packet) {
 		arp_packet = net_alloc_packet();
 		if (!arp_packet)
@@ -288,9 +309,11 @@ static struct net_connection *net_new(struct eth_device *edev, IPaddr_t dest,
 	int ret;
 
 	if (!edev) {
-		edev = eth_get_current();
+		edev = net_route(dest);
+		if (!edev && net_gateway)
+			edev = net_route(net_gateway);
 		if (!edev)
-			return ERR_PTR(-ENETDOWN);
+			return ERR_PTR(-EHOSTUNREACH);
 	}
 
 	if (!is_valid_ether_addr(edev->ethaddr)) {
@@ -348,7 +371,7 @@ struct net_connection *net_udp_eth_new(struct eth_device *edev, IPaddr_t dest,
 				       uint16_t dport, rx_handler_f *handler,
 				       void *ctx)
 {
-	struct net_connection *con = net_new(edev, 0xffffffff, handler, ctx);
+	struct net_connection *con = net_new(edev, dest, handler, ctx);
 
 	if (IS_ERR(con))
 		return con;
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 14/21] net: remove "current" network device

[Sascha Hauer]

Now that we can do routing we no longer need a "current"
network device. Remove it.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 commands/Makefile |  1 -
 commands/net.c    | 66 -------------------------------------------------------
 include/net.h     |  2 --
 net/eth.c         | 17 --------------
 net/ifup.c        |  2 --
 5 files changed, 88 deletions(-)
 delete mode 100644 commands/net.c

diff --git a/commands/Makefile b/commands/Makefile
index 37486dceb1..582175f631 100644
--- a/commands/Makefile
+++ b/commands/Makefile
@@ -22,7 +22,6 @@ obj-$(CONFIG_CMD_MSLEEP)	+= msleep.o
 obj-$(CONFIG_CMD_RESET)		+= reset.o
 obj-$(CONFIG_CMD_POWEROFF)	+= poweroff.o
 obj-$(CONFIG_CMD_GO)		+= go.o
-obj-$(CONFIG_NET)		+= net.o
 obj-$(CONFIG_CMD_PARTITION)	+= partition.o
 obj-$(CONFIG_CMD_LS)		+= ls.o
 obj-$(CONFIG_CMD_CD)		+= cd.o
diff --git a/commands/net.c b/commands/net.c
deleted file mode 100644
index 219c7efcda..0000000000
--- a/commands/net.c
+++ /dev/null
@@ -1,66 +0,0 @@
-/*
- * (C) Copyright 2000
- * Wolfgang Denk, DENX Software Engineering, wd@denx.de.
- *
- * 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.
- *
- */
-
-/**
- * @file
- * @brief tftp, rarpboot, dhcp, nfs, cdp - Boot support
- */
-
-#include <common.h>
-#include <command.h>
-#include <complete.h>
-#include <environment.h>
-#include <driver.h>
-#include <net.h>
-#include <fs.h>
-#include <errno.h>
-#include <libbb.h>
-
-static int do_ethact(int argc, char *argv[])
-{
-	struct eth_device *edev;
-
-	if (argc == 1) {
-		edev = eth_get_current();
-		if (edev)
-			printf("%s%d\n", edev->dev.name, edev->dev.id);
-		return 0;
-	}
-
-	if (argc != 2)
-		return COMMAND_ERROR_USAGE;
-
-	edev = eth_get_byname(argv[1]);
-	if (edev)
-		eth_set_current(edev);
-	else {
-		printf("no such net device: %s\n", argv[1]);
-		return 1;
-	}
-
-	return 0;
-}
-
-BAREBOX_CMD_START(ethact)
-	.cmd		= do_ethact,
-	BAREBOX_CMD_DESC("get or set current ethernet device")
-	BAREBOX_CMD_OPTS("[ETHX]")
-	BAREBOX_CMD_GROUP(CMD_GRP_NET)
-	BAREBOX_CMD_COMPLETE(eth_complete)
-BAREBOX_CMD_END
diff --git a/include/net.h b/include/net.h
index 416fcde761..c72197b2f0 100644
--- a/include/net.h
+++ b/include/net.h
@@ -409,8 +409,6 @@ static inline int is_valid_ether_addr(const u8 *addr)
 
 typedef void rx_handler_f(void *ctx, char *packet, unsigned int len);
 
-void eth_set_current(struct eth_device *eth);
-struct eth_device *eth_get_current(void);
 struct eth_device *eth_get_byname(const char *name);
 
 /**
diff --git a/net/eth.c b/net/eth.c
index 6dae6b9feb..a8f21b2277 100644
--- a/net/eth.c
+++ b/net/eth.c
@@ -28,7 +28,6 @@
 #include <errno.h>
 #include <malloc.h>
 
-static struct eth_device *eth_current;
 static uint64_t last_link_check;
 
 LIST_HEAD(netdev_list);
@@ -149,16 +148,6 @@ void of_eth_register_ethaddr(struct device_node *node, const char *ethaddr)
 	list_add_tail(&addr->list, &ethaddr_list);
 }
 
-void eth_set_current(struct eth_device *eth)
-{
-	eth_current = eth;
-}
-
-struct eth_device * eth_get_current(void)
-{
-	return eth_current;
-}
-
 struct eth_device *eth_get_byname(const char *ethname)
 {
 	struct eth_device *edev;
@@ -412,17 +401,11 @@ int eth_register(struct eth_device *edev)
 			edev->parent->device_node)
 		edev->nodepath = xstrdup(edev->parent->device_node->full_name);
 
-	if (!eth_current)
-		eth_current = edev;
-
 	return 0;
 }
 
 void eth_unregister(struct eth_device *edev)
 {
-	if (edev == eth_current)
-		eth_current = NULL;
-
 	if (edev->active)
 		edev->halt(edev);
 
diff --git a/net/ifup.c b/net/ifup.c
index 6c298d8501..bec254b64d 100644
--- a/net/ifup.c
+++ b/net/ifup.c
@@ -58,8 +58,6 @@ int ifup(const char *name, unsigned flags)
 	if (edev && edev->ipaddr && !(flags & IFUP_FLAG_FORCE))
 		return 0;
 
-	eth_set_current(edev);
-
 	env_push_context();
 
 	setenv("ip", "");
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 15/21] net: ifup: Factor out a eth_discover function

[Sascha Hauer]

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 net/ifup.c | 38 ++++++++++++++++++++++++++------------
 1 file changed, 26 insertions(+), 12 deletions(-)

diff --git a/net/ifup.c b/net/ifup.c
index bec254b64d..859cee717a 100644
--- a/net/ifup.c
+++ b/net/ifup.c
@@ -26,6 +26,30 @@
 #include <fs.h>
 #include <linux/stat.h>
 
+static int eth_discover(const char *name)
+{
+	char *cmd_discover;
+	struct stat s;
+	int ret;
+
+	cmd_discover = basprintf("/env/network/%s-discover", name);
+
+	ret = stat(cmd_discover, &s);
+	if (ret)
+		goto out;
+
+	ret = run_command(cmd_discover);
+	if (ret) {
+		pr_err("Running '%s' failed with %d\n", cmd_discover, ret);
+		goto out;
+	}
+
+out:
+	free(cmd_discover);
+
+	return ret;
+}
+
 static char *vars[] = {
 	"ipaddr",
 	"netmask",
@@ -48,9 +72,8 @@ static int eth_set_param(struct device_d *dev, const char *param)
 int ifup(const char *name, unsigned flags)
 {
 	int ret;
-	char *cmd, *cmd_discover;
+	char *cmd;
 	const char *ip;
-	struct stat s;
 	int i;
 	struct device_d *dev;
 	struct eth_device *edev = eth_get_byname(name);
@@ -66,7 +89,6 @@ int ifup(const char *name, unsigned flags)
 		setenv(vars[i], "");
 
 	cmd = basprintf("source /env/network/%s", name);
-	cmd_discover = basprintf("/env/network/%s-discover", name);
 
 	ret = run_command(cmd);
 	if (ret) {
@@ -74,14 +96,7 @@ int ifup(const char *name, unsigned flags)
 		goto out;
 	}
 
-	ret = stat(cmd_discover, &s);
-	if (!ret) {
-		ret = run_command(cmd_discover);
-		if (ret) {
-			pr_err("Running '%s' failed with %d\n", cmd, ret);
-			goto out;
-		}
-	}
+	eth_discover(name);
 
 	dev = get_device_by_name(name);
 	if (!dev) {
@@ -142,7 +157,6 @@ int ifup(const char *name, unsigned flags)
 out:
 	env_pop_context();
 	free(cmd);
-	free(cmd_discover);
 
 	return ret;
 }
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 16/21] ifup: Use dhcp C API rather than running command

[Sascha Hauer]

DHCP has a C API, so use it instead of running as command.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 net/ifup.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/net/ifup.c b/net/ifup.c
index 859cee717a..a65956bba1 100644
--- a/net/ifup.c
+++ b/net/ifup.c
@@ -22,6 +22,7 @@
 #include <command.h>
 #include <common.h>
 #include <getopt.h>
+#include <dhcp.h>
 #include <net.h>
 #include <fs.h>
 #include <linux/stat.h>
@@ -114,15 +115,12 @@ int ifup(const char *name, unsigned flags)
 
 	if (!strcmp(ip, "dhcp")) {
 		IPaddr_t serverip;
-		char *dhcp_cmd;
 
 		serverip = getenv_ip("serverip");
 		if (serverip)
 			net_set_serverip(serverip, false);
 
-		dhcp_cmd = basprintf("dhcp %s", name);
-		ret = run_command(dhcp_cmd);
-		free(dhcp_cmd);
+		ret = dhcp(edev, NULL);
 		if (ret)
 			goto out;
 		dev_set_param(dev, "linux.bootargs", "ip=dhcp");
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 17/21] net: Provide new way to configure network devices

[Sascha Hauer]

This provides a new way to configure network interfaces based
on nvvars. A network interface can now be configured with variables
in the nv.dev.<ethname>.* namespace. There is a new network device
parameter "mode" which specifies the mode used to obtain IP settings.
The mode can be "dhcp", "static" or "disabled":

nv.dev.eth0.mode=dhcp
(ipaddr, netmask are ignored in this setting)

nv.dev.eth0.mode=static
nv.dev.eth0.ipaddr=192.168.0.17
nv.dev.eth0.netmask=255.255.0.0

nv.dev.eth0.mode=disabled

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 include/net.h |   6 ++
 net/eth.c     |  15 ++++
 net/ifup.c    | 235 +++++++++++++++++++++++++++++++++++++---------------------
 3 files changed, 172 insertions(+), 84 deletions(-)

diff --git a/include/net.h b/include/net.h
index c72197b2f0..9b57dad909 100644
--- a/include/net.h
+++ b/include/net.h
@@ -61,6 +61,11 @@ struct eth_device {
 	IPaddr_t netmask;
 	char ethaddr[6];
 	char *bootarg;
+
+#define ETH_MODE_DHCP 0
+#define ETH_MODE_STATIC 1
+#define ETH_MODE_DISABLED 2
+	unsigned int global_mode;
 };
 
 #define dev_to_edev(d) container_of(d, struct eth_device, dev)
@@ -469,6 +474,7 @@ void led_trigger_network(enum led_trigger trigger);
 
 #define IFUP_FLAG_FORCE		(1 << 0)
 
+int ifup_edev(struct eth_device *edev);
 int ifup(const char *name, unsigned flags);
 int ifup_all(unsigned flags);
 
diff --git a/net/eth.c b/net/eth.c
index a8f21b2277..a9869f7d9d 100644
--- a/net/eth.c
+++ b/net/eth.c
@@ -21,12 +21,18 @@
 #include <command.h>
 #include <complete.h>
 #include <driver.h>
+#include <unistd.h>
 #include <init.h>
+#include <dhcp.h>
 #include <net.h>
 #include <of.h>
 #include <linux/phy.h>
 #include <errno.h>
 #include <malloc.h>
+#include <globalvar.h>
+#include <environment.h>
+#include <linux/ctype.h>
+#include <linux/stat.h>
 
 static uint64_t last_link_check;
 
@@ -340,6 +346,12 @@ late_initcall(eth_register_of_fixup);
 extern IPaddr_t net_serverip;
 extern IPaddr_t net_gateway;
 
+static const char * const eth_mode_names[] = {
+	[ETH_MODE_DHCP] = "dhcp",
+	[ETH_MODE_STATIC] = "static",
+	[ETH_MODE_DISABLED] = "disabled",
+};
+
 int eth_register(struct eth_device *edev)
 {
 	struct device_d *dev = &edev->dev;
@@ -378,6 +390,9 @@ int eth_register(struct eth_device *edev)
 			edev->ethaddr, edev);
 	edev->bootarg = xstrdup("");
 	dev_add_param_string(dev, "linux.bootargs", NULL, NULL, &edev->bootarg, NULL);
+	dev_add_param_enum(dev, "mode", NULL, NULL, &edev->global_mode,
+				  eth_mode_names, ARRAY_SIZE(eth_mode_names),
+				  NULL);
 
 	if (edev->init)
 		edev->init(edev);
diff --git a/net/ifup.c b/net/ifup.c
index a65956bba1..7df9f587cd 100644
--- a/net/ifup.c
+++ b/net/ifup.c
@@ -25,116 +25,147 @@
 #include <dhcp.h>
 #include <net.h>
 #include <fs.h>
+#include <globalvar.h>
+#include <string.h>
+#include <driver.h>
 #include <linux/stat.h>
 
-static int eth_discover(const char *name)
+static int eth_discover(char *file)
 {
-	char *cmd_discover;
 	struct stat s;
 	int ret;
 
-	cmd_discover = basprintf("/env/network/%s-discover", name);
-
-	ret = stat(cmd_discover, &s);
-	if (ret)
+	ret = stat(file, &s);
+	if (ret) {
+		ret = 0;
 		goto out;
+	}
 
-	ret = run_command(cmd_discover);
+	ret = run_command(file);
 	if (ret) {
-		pr_err("Running '%s' failed with %d\n", cmd_discover, ret);
+		pr_err("Running '%s' failed with %d\n", file, ret);
 		goto out;
 	}
 
 out:
-	free(cmd_discover);
+	free(file);
 
 	return ret;
 }
 
-static char *vars[] = {
-	"ipaddr",
-	"netmask",
-	"gateway",
-	"serverip",
-};
-
-static int eth_set_param(struct device_d *dev, const char *param)
+static int eth_discover_ethname(const char *ethname)
 {
-	const char *value = getenv(param);
-
-	if (!value)
-		return 0;
-	if (!*value)
-		return 0;
+	return eth_discover(basprintf("/env/network/%s-discover", ethname));
+}
 
-	return dev_set_param(dev, param, value);
+static int eth_discover_file(const char *filename)
+{
+	return eth_discover(basprintf("/env/network/%s", filename));
 }
 
-int ifup(const char *name, unsigned flags)
+static int source_env_network(struct eth_device *edev)
 {
-	int ret;
-	char *cmd;
-	const char *ip;
-	int i;
-	struct device_d *dev;
-	struct eth_device *edev = eth_get_byname(name);
+	char *vars[] = {
+		"ipaddr",
+		"netmask",
+		"gateway",
+		"serverip",
+		"ethaddr",
+		"ip",
+	};
+	IPaddr_t ipaddr, netmask, gateway, serverip;
+	unsigned char ethaddr[6];
+	char *file, *cmd;
+	const char *ethaddrstr, *modestr;
+	int ret, mode, ethaddr_valid = 0, i;
+	struct stat s;
 
-	if (edev && edev->ipaddr && !(flags & IFUP_FLAG_FORCE))
+	file = basprintf("/env/network/%s", edev->devname);
+	ret = stat(file, &s);
+	if (ret) {
+		free(file);
 		return 0;
+	}
 
-	env_push_context();
+	dev_info(&edev->dev, "/env/network/%s is deprecated.\n"
+		 "Use nv.dev.%s.* nvvars to configure your network device instead\n",
+		 edev->devname, edev->devname);
 
-	setenv("ip", "");
+	env_push_context();
 
 	for (i = 0; i < ARRAY_SIZE(vars); i++)
 		setenv(vars[i], "");
 
-	cmd = basprintf("source /env/network/%s", name);
-
+	cmd = basprintf("source /env/network/%s", edev->devname);
 	ret = run_command(cmd);
 	if (ret) {
 		pr_err("Running '%s' failed with %d\n", cmd, ret);
 		goto out;
 	}
 
-	eth_discover(name);
+	ipaddr = getenv_ip("ipaddr");
+	netmask = getenv_ip("netmask");
+	gateway = getenv_ip("gateway");
+	serverip = getenv_ip("serverip");
+	ethaddrstr = getenv("ethaddr");
+	if (ethaddrstr && *ethaddrstr) {
+		ret = string_to_ethaddr(ethaddrstr, ethaddr);
+		if (ret) {
+			dev_err(&edev->dev, "Cannot parse ethaddr \"%s\"\n", ethaddrstr);
+			ret = -EINVAL;
+			goto out;
+		}
+		ethaddr_valid = 1;
+	}
 
-	dev = get_device_by_name(name);
-	if (!dev) {
-		pr_err("Cannot find device %s\n", name);
+	modestr = getenv("ip");
+	if (!modestr) {
+		dev_err(&edev->dev, "No mode specified in \"ip\" variable\n");
+		ret = -EINVAL;
 		goto out;
 	}
 
-	ret = eth_set_param(dev, "ethaddr");
-	if (ret)
+	if (!strcmp(modestr, "static")) {
+		mode = ETH_MODE_STATIC;
+	} else if (!strcmp(modestr, "dhcp")) {
+		mode = ETH_MODE_DHCP;
+	} else {
+		dev_err(&edev->dev, "Invalid ip mode \"%s\" found\n", modestr);
+		ret = -EINVAL;
 		goto out;
+	}
 
-	ip = getenv("ip");
-	if (!ip)
-		ip = "";
+	edev->global_mode = mode;
 
-	if (!strcmp(ip, "dhcp")) {
-		IPaddr_t serverip;
+	if (ethaddr_valid)
+		memcpy(edev->ethaddr, ethaddr, 6);
 
-		serverip = getenv_ip("serverip");
+	if (mode == ETH_MODE_STATIC) {
+		edev->ipaddr = ipaddr;
+		edev->netmask = netmask;
+		if (gateway)
+			net_set_gateway(gateway);
 		if (serverip)
-			net_set_serverip(serverip, false);
+			net_set_serverip(serverip, true);
+	}
 
-		ret = dhcp(edev, NULL);
-		if (ret)
-			goto out;
-		dev_set_param(dev, "linux.bootargs", "ip=dhcp");
-	} else if (!strcmp(ip, "static")) {
+	ret = 0;
+
+out:
+	env_pop_context();
+	free(cmd);
+	free(file);
+
+	return ret;
+}
+
+static void set_linux_bootarg(struct eth_device *edev)
+{
+	if (edev->global_mode == ETH_MODE_STATIC) {
 		char *bootarg;
 		IPaddr_t serverip;
 		IPaddr_t gateway;
 
-		for (i = 0; i < ARRAY_SIZE(vars); i++) {
-			ret = eth_set_param(dev, vars[i]);
-			if (ret)
-				goto out;
-		}
-
 		serverip = net_get_serverip();
 		gateway = net_get_gateway();
 
@@ -143,46 +174,82 @@ int ifup(const char *name, unsigned flags)
 				&serverip,
 				&gateway,
 				&edev->netmask);
-		dev_set_param(dev, "linux.bootargs", bootarg);
+		dev_set_param(&edev->dev, "linux.bootargs", bootarg);
 		free(bootarg);
-	} else {
-		pr_err("unknown ip type: %s\n", ip);
-		ret = -EINVAL;
-		goto out;
+	} else if (edev->global_mode == ETH_MODE_DHCP) {
+		dev_set_param(&edev->dev, "linux.bootargs", "ip=dhcp");
 	}
+}
 
-	ret = 0;
-out:
-	env_pop_context();
-	free(cmd);
+int ifup_edev(struct eth_device *edev)
+{
+	int ret;
 
-	return ret;
+	if (edev->global_mode == ETH_MODE_DISABLED)
+		return 0;
+
+	ret = source_env_network(edev);
+	if (ret)
+		return ret;
+
+	if (edev->global_mode == ETH_MODE_DHCP) {
+		if (IS_ENABLED(CONFIG_NET_DHCP)) {
+			ret = dhcp(edev, NULL);
+		} else {
+			dev_err(&edev->dev, "DHCP support not available\n");
+			ret = -ENOSYS;
+		}
+		if (ret)
+			return ret;
+	}
+
+	set_linux_bootarg(edev);
+
+	return 0;
+}
+
+int ifup(const char *ethname, unsigned flags)
+{
+	struct eth_device *edev;
+	int ret;
+
+	ret = eth_discover_ethname(ethname);
+	if (ret)
+		return ret;
+
+	edev = eth_get_byname(ethname);
+	if (!edev)
+		return -ENODEV;
+
+	return ifup_edev(edev);
 }
 
 int ifup_all(unsigned flags)
 {
+	struct eth_device *edev;
 	DIR *dir;
 	struct dirent *d;
 
 	dir = opendir("/env/network");
-	if (!dir)
-		return -ENOENT;
-
-	while ((d = readdir(dir))) {
-		if (*d->d_name == '.')
-			continue;
-		/*
-		 * Skip xxx-discover files since these are no
-		 * network configuration files, but scripts to bring
-		 * up network interface xxx.
-		 */
-		if (strstr(d->d_name, "-discover"))
-			continue;
-		ifup(d->d_name, flags);
+	if (dir) {
+
+		while ((d = readdir(dir))) {
+			if (*d->d_name == '.')
+				continue;
+			if (!strstr(d->d_name, "-discover"))
+				continue;
+
+			eth_discover_file(d->d_name);
+		}
 	}
 
 	closedir(dir);
 
+	device_detect_all();
+
+	for_each_netdev(edev)
+		ifup_edev(edev);
+
 	return 0;
 }
 
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 18/21] net: update network docs

[Sascha Hauer]

The network configuration has changed in the previous patches. Update
the documentation accordingly.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 Documentation/user/networking.rst | 99 +++++++++++++++++++++++++++++++++++++--
 1 file changed, 94 insertions(+), 5 deletions(-)

diff --git a/Documentation/user/networking.rst b/Documentation/user/networking.rst
index 8afb433837..53fa5b3c40 100644
--- a/Documentation/user/networking.rst
+++ b/Documentation/user/networking.rst
@@ -7,6 +7,49 @@ barebox has IPv4 networking support. Several protocols such as :ref:`DHCP
 Network configuration
 ---------------------
 
+Lowlevel network device configuration
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+Network devices are configured with a set of device specific variables:
+
++-------------------+--------------+----------------------------------------------------+
+| name              | type         |                                                    |
++===================+==============+====================================================+
+| <devname>.mode    | enum         | "dhcp": DHCP is used to get IP address and netmask |
+|                   |              | "static": Static IP setup described by variables   |
+|                   |              | below                                              |
+|                   |              | "disabled": Interface unused                       |
++===================+==============+====================================================+
+| <devname>.ipaddr  | ipv4 address | The IP address when using static configuration     |
++-------------------+--------------+----------------------------------------------------+
+| <devname>.netmask | ipv4 address | The netmask when using static configuration        |
++-------------------+--------------+----------------------------------------------------+
+| <devname>.gateway | ipv4 address | Alias for global.net.gateway. For                  |
+|                   |              | compatibility, do not use.                         |
++-------------------+--------------+----------------------------------------------------+
+| <devname>.serverip| ipv4 address | Alias for global.net.server. For                   |
+|                   |              | compatibility, do not use.                         |
++-------------------+--------------+----------------------------------------------------+
+| <devname>.ethaddr | MAC address  | The MAC address of this device                     |
++-------------------+--------------+----------------------------------------------------+
+
+Additionally there are some more variables that are not specific to a
+device:
+
++------------------------------+--------------+------------------------------------------------+
+| name                         | type         |                                                |
++==============================+==============+================================================+
+| global.net.gateway           | ipv4 host    | The network gateway used when a host is not in |
+|                              |              | any directly visible subnet. May be set        |
+|                              |              | automatically by DHCP.                         |
++------------------------------+--------------+------------------------------------------------+
+| global.net.server            | ipv4 host    | The default server address. If unspecified, may|
+|                              |              | be set by DHCP                                 |
++------------------------------+--------------+------------------------------------------------+
+| global.net.nameserver        | ipv4 address | The DNS server used for resolving host names.  |
+|                              |              | May be set by DHCP                             |
++------------------------------+--------------+------------------------------------------------+
+
 The first step for networking is configuring the network device. The network
 device is usually ``eth0``. The current configuration can be viewed with the
 :ref:`devinfo <command_devinfo>` command:
@@ -16,10 +59,9 @@ device is usually ``eth0``. The current configuration can be viewed with the
   barebox:/ devinfo eth0
   Parameters:
     ethaddr: 00:1c:49:01:03:4b
-    gateway: 192.168.23.1
     ipaddr: 192.168.23.197
     netmask: 255.255.0.0
-    serverip: 192.168.23.1
+    [...]
 
 The configuration can be changed on the command line with:
 
@@ -30,9 +72,56 @@ The configuration can be changed on the command line with:
 The :ref:`dhcp command <command_dhcp>` will change the settings based on the answer
 from the DHCP server.
 
-This low-level configuration of the network interface is often not necessary. Normally
-the network settings should be edited in ``/env/network/eth0``, then the network interface
-can be brought up using the :ref:`ifup command <command_ifup>`.
+To make the network device settings persistent across reboots there is a nonvolatile
+variable (nvvar) for each of the varariables above. The network device specific variables
+are:
+
+.. code-block:: sh
+
+  nv.dev.<devname>.mode
+  nv.dev.<devname>.ipaddr
+  nv.dev.<devname>.netmask
+  nv.dev.<devname>.ethaddr
+
+The others are:
+
+.. code-block:: sh
+
+  nv.net.gateway
+  nv.net.server
+  nv.net.nameserver
+
+A typical simple network setting is to use DHCP. Provided the network interface is eth0
+then this would configure the network device for DHCP:
+
+.. code-block:: sh
+
+  nv dev.eth0.mode=dhcp
+
+(In fact DHCP is the default, so the above is not necessary)
+
+A static setup would look like:
+
+.. code-block:: sh
+
+  nv dev.eth0.mode=static
+  nv dev.eth0.ipaddr=192.168.0.17
+  nv dev.eth0.netmask=255.255.0.0
+  nv net.server=192.168.0.1
+
+The settings can be activated with the :ref:`ifup command <command_ifup>`:
+
+.. code-block:: sh
+
+  ifup eth0
+
+or:
+
+.. code-block:: sh
+
+  ifup -a
+
+'ifup -a' will activate all ethernet interfaces, also the ones on USB.
 
 Network filesystems
 -------------------
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 19/21] net: environment: remove ethx setup files

[Sascha Hauer]

Network configuration is now done with nvvars, so /env/network/eth*
files are no longer needed. Remove them.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 arch/arm/boards/afi-gf/defaultenv-gf/network/eth1      | 18 ------------------
 .../defaultenv-pico-hobbit/network/eth1                | 18 ------------------
 .../boards/zii-imx6q-rdu2/defaultenv-rdu2/network/eth1 | 18 ------------------
 defaultenv/defaultenv-2-base/network/eth0              | 18 ------------------
 4 files changed, 72 deletions(-)
 delete mode 100644 arch/arm/boards/afi-gf/defaultenv-gf/network/eth1
 delete mode 100644 arch/arm/boards/technexion-pico-hobbit/defaultenv-pico-hobbit/network/eth1
 delete mode 100644 arch/arm/boards/zii-imx6q-rdu2/defaultenv-rdu2/network/eth1
 delete mode 100644 defaultenv/defaultenv-2-base/network/eth0

diff --git a/arch/arm/boards/afi-gf/defaultenv-gf/network/eth1 b/arch/arm/boards/afi-gf/defaultenv-gf/network/eth1
deleted file mode 100644
index 1ed3017e96..0000000000
--- a/arch/arm/boards/afi-gf/defaultenv-gf/network/eth1
+++ /dev/null
@@ -1,18 +0,0 @@
-#!/bin/sh
-
-# ip setting (static/dhcp)
-ip=dhcp
-global.dhcp.vendor_id=barebox-${global.hostname}
-
-# static setup used if ip=static
-ipaddr=
-netmask=
-gateway=
-serverip=
-
-# MAC address if needed
-#ethaddr=xx:xx:xx:xx:xx:xx
-
-# put code to discover eth1 (i.e. 'usb') to /env/network/eth0-discover
-
-exit 0
diff --git a/arch/arm/boards/technexion-pico-hobbit/defaultenv-pico-hobbit/network/eth1 b/arch/arm/boards/technexion-pico-hobbit/defaultenv-pico-hobbit/network/eth1
deleted file mode 100644
index dfe63971df..0000000000
--- a/arch/arm/boards/technexion-pico-hobbit/defaultenv-pico-hobbit/network/eth1
+++ /dev/null
@@ -1,18 +0,0 @@
-#!/bin/sh
-
-# ip setting (static/dhcp)
-ip=dhcp
-global.dhcp.vendor_id=barebox-${global.hostname}
-
-# static setup used if ip=static
-ipaddr=
-netmask=
-gateway=
-serverip=
-
-# MAC address if needed
-#ethaddr=xx:xx:xx:xx:xx:xx
-
-# put code to discover eth1 (i.e. 'usb') to /env/network/eth1-discover
-
-exit 0
diff --git a/arch/arm/boards/zii-imx6q-rdu2/defaultenv-rdu2/network/eth1 b/arch/arm/boards/zii-imx6q-rdu2/defaultenv-rdu2/network/eth1
deleted file mode 100644
index 33fe7c1b2b..0000000000
--- a/arch/arm/boards/zii-imx6q-rdu2/defaultenv-rdu2/network/eth1
+++ /dev/null
@@ -1,18 +0,0 @@
-#!/bin/sh
-
-# ip setting (static/dhcp)
-ip=dhcp
-global.dhcp.vendor_id=barebox-${global.hostname}
-
-# static setup used if ip=static
-ipaddr=
-netmask=
-gateway=
-serverip=
-
-# MAC address if needed
-#ethaddr=xx:xx:xx:xx:xx:xx
-
-# put code to discover eth0 (i.e. 'usb') to /env/network/eth0-discover
-
-exit 0
diff --git a/defaultenv/defaultenv-2-base/network/eth0 b/defaultenv/defaultenv-2-base/network/eth0
deleted file mode 100644
index 33fe7c1b2b..0000000000
--- a/defaultenv/defaultenv-2-base/network/eth0
+++ /dev/null
@@ -1,18 +0,0 @@
-#!/bin/sh
-
-# ip setting (static/dhcp)
-ip=dhcp
-global.dhcp.vendor_id=barebox-${global.hostname}
-
-# static setup used if ip=static
-ipaddr=
-netmask=
-gateway=
-serverip=
-
-# MAC address if needed
-#ethaddr=xx:xx:xx:xx:xx:xx
-
-# put code to discover eth0 (i.e. 'usb') to /env/network/eth0-discover
-
-exit 0
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 20/21] net: environment: update automounts

[Sascha Hauer]

Instead of using "ifup ethx" use "ifup -a" which works with all
network interfaces. Also replace "$ethx.serverip" with
"$global.net.server". This makes the automount independent of
the actual network interface. Remove all board specific
/env/init/automount files which were only there to use eth1 instead
of the previously hardcoded eth0.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 Documentation/user/automount.rst                        |  2 +-
 arch/arm/boards/afi-gf/defaultenv-gf/init/automount     | 10 ----------
 .../defaultenv-pico-hobbit/init/automount               | 11 -----------
 .../zii-imx6q-rdu2/defaultenv-rdu2/init/automount       | 17 -----------------
 defaultenv/defaultenv-2-base/init/automount             |  4 ++--
 5 files changed, 3 insertions(+), 41 deletions(-)
 delete mode 100644 arch/arm/boards/afi-gf/defaultenv-gf/init/automount
 delete mode 100644 arch/arm/boards/technexion-pico-hobbit/defaultenv-pico-hobbit/init/automount
 delete mode 100644 arch/arm/boards/zii-imx6q-rdu2/defaultenv-rdu2/init/automount

diff --git a/Documentation/user/automount.rst b/Documentation/user/automount.rst
index a5e4313576..7de8261354 100644
--- a/Documentation/user/automount.rst
+++ b/Documentation/user/automount.rst
@@ -13,7 +13,7 @@ Typical usage is for accessing the TFTP server. To set up an automount for a
 TFTP server, the following is required::
 
   mkdir -p /mnt/tftp
-  automount /mnt/tftp 'ifup eth0 && mount -t tftp $eth0.serverip /mnt/tftp'
+  automount /mnt/tftp 'ifup -a && mount -t tftp $global.net.server /mnt/tftp'
 
 This creates an automountpoint on ``/mnt/tftp``. Whenever this directory is accessed,
 the command ``ifup eth0 && mount -t tftp $eth0.serverip /mnt/tftp`` is executed.
diff --git a/arch/arm/boards/afi-gf/defaultenv-gf/init/automount b/arch/arm/boards/afi-gf/defaultenv-gf/init/automount
deleted file mode 100644
index 560bdb7975..0000000000
--- a/arch/arm/boards/afi-gf/defaultenv-gf/init/automount
+++ /dev/null
@@ -1,10 +0,0 @@
-#!/bin/sh
-
-# automount tftp server based on $eth1.serverip
-
-mkdir -p /mnt/tftp
-automount /mnt/tftp 'ifup eth1 && mount -t tftp $eth1.serverip /mnt/tftp'
-
-# eth0 is on the mezzanine board
-mkdir -p /mnt/tftp-eth0
-automount /mnt/tftp-eth0 'ifup eth0 && mount -t tftp $eth0.serverip /mnt/tftp'
diff --git a/arch/arm/boards/technexion-pico-hobbit/defaultenv-pico-hobbit/init/automount b/arch/arm/boards/technexion-pico-hobbit/defaultenv-pico-hobbit/init/automount
deleted file mode 100644
index fdcfa36045..0000000000
--- a/arch/arm/boards/technexion-pico-hobbit/defaultenv-pico-hobbit/init/automount
+++ /dev/null
@@ -1,11 +0,0 @@
-#!/bin/sh
-
-# automount tftp server based on $eth1.serverip
-
-mkdir -p /mnt/tftp
-automount /mnt/tftp 'ifup eth1 && mount -t tftp $eth1.serverip /mnt/tftp'
-
-# automount nfs server's nfsroot
-
-mkdir -p /mnt/nfs
-automount /mnt/nfs 'ifup eth1 && mount -t nfs ${eth1.serverip}:/home/${global.user}/nfsroot/${global.hostname} /mnt/nfs'
diff --git a/arch/arm/boards/zii-imx6q-rdu2/defaultenv-rdu2/init/automount b/arch/arm/boards/zii-imx6q-rdu2/defaultenv-rdu2/init/automount
deleted file mode 100644
index 6c04eb48a1..0000000000
--- a/arch/arm/boards/zii-imx6q-rdu2/defaultenv-rdu2/init/automount
+++ /dev/null
@@ -1,17 +0,0 @@
-#!/bin/sh
-
-# automount tftp server based on $eth0.serverip
-
-mkdir -p /mnt/tftp
-automount /mnt/tftp 'ifup eth1 && mount -t tftp $eth1.serverip /mnt/tftp'
-
-# automount nfs server's nfsroot
-
-mkdir -p /mnt/nfs
-automount /mnt/nfs 'ifup eth1 && mount -t nfs ${eth1.serverip}:/home/${global.user}/nfsroot/${global.hostname} /mnt/nfs'
-
-
-# FAT on usb disk example
-
-#mkdir -p /mnt/fat
-#automount -d /mnt/fat 'usb && [ -e /dev/disk0.0 ] && mount /dev/disk0.0 /mnt/fat'
diff --git a/defaultenv/defaultenv-2-base/init/automount b/defaultenv/defaultenv-2-base/init/automount
index 959b2c148e..5e0cb4d938 100644
--- a/defaultenv/defaultenv-2-base/init/automount
+++ b/defaultenv/defaultenv-2-base/init/automount
@@ -3,12 +3,12 @@
 # automount tftp server based on $eth0.serverip
 
 mkdir -p /mnt/tftp
-automount /mnt/tftp 'ifup eth0 && mount -t tftp $eth0.serverip /mnt/tftp'
+automount /mnt/tftp 'ifup -a && mount -t tftp $global.net.server /mnt/tftp'
 
 # automount nfs server's nfsroot
 
 mkdir -p /mnt/nfs
-automount /mnt/nfs 'ifup eth0 && mount -t nfs ${eth0.serverip}:/home/${global.user}/nfsroot/${global.hostname} /mnt/nfs'
+automount /mnt/nfs 'ifup -a && mount -t nfs ${global.net.server}:/home/${global.user}/nfsroot/${global.hostname} /mnt/nfs'
 
 
 # FAT on usb disk example
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 21/21] defaultenv: Add README for new network config

[Sascha Hauer]

Network configuration has changed. In case somebody looks
to the old /env/network/ directoy place a README there to add
a pointer how the new configuration is done.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 defaultenv/defaultenv-2-base/network/README | 3 +++
 1 file changed, 3 insertions(+)
 create mode 100644 defaultenv/defaultenv-2-base/network/README

diff --git a/defaultenv/defaultenv-2-base/network/README b/defaultenv/defaultenv-2-base/network/README
new file mode 100644
index 0000000000..4b47807a1a
--- /dev/null
+++ b/defaultenv/defaultenv-2-base/network/README
@@ -0,0 +1,3 @@
+Network configuration is now done with nvvars, see
+Documentation/user/networking.rst. Files in this directory are not needed
+anymore.
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 01/21] driver: Add device_detect_all() function

[Sam Ravnborg]

Hi Sasha.

On Fri, Nov 24, 2017 at 09:12:17AM +0100, Sascha Hauer wrote:
> Add a device_detect_all function to detect all devices and use it
> in the detect command. This makes the functionality reusable in other
> code.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  commands/detect.c     | 6 +-----
>  drivers/base/driver.c | 8 ++++++++
>  include/driver.h      | 1 +
>  3 files changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/commands/detect.c b/commands/detect.c
> index 1586a6fb54..42e111419f 100644
> --- a/commands/detect.c
> +++ b/commands/detect.c
> @@ -56,11 +56,7 @@ static int do_detect(int argc, char *argv[])
>  	}
>  
>  	if (option_all) {
> -		for_each_device(dev) {
> -			ret = device_detect(dev);
> -			if (ret && ret != -ENOSYS && option_error)
> -				return ret;
> -		}
> +		device_detect_all();

With this change there is no longer any checks
if device_detect() fails.
so the option "-e" is no longer useful in combination
with option "-a" (which uses the patched code).

Should the code check for any errors or should
the help text be updated?


>  		return 0;
>  	}
>  
> diff --git a/drivers/base/driver.c b/drivers/base/driver.c
> index 83260990af..c43a4bde2a 100644
> --- a/drivers/base/driver.c
> +++ b/drivers/base/driver.c
> @@ -140,6 +140,14 @@ int device_detect_by_name(const char *__devname)
>  	return ret;
>  }
>  
> +void device_detect_all(void)
> +{
> +	struct device_d *dev;
> +
> +	for_each_device(dev)
> +		device_detect(dev);
> +}
> +
>  static int match(struct driver_d *drv, struct device_d *dev)
>  {
>  	int ret;
> diff --git a/include/driver.h b/include/driver.h
> index 8617872053..e571fbbec5 100644
> --- a/include/driver.h
> +++ b/include/driver.h
> @@ -134,6 +134,7 @@ int device_probe(struct device_d *dev);
>  /* detect devices attached to this device (cards, disks,...) */
>  int device_detect(struct device_d *dev);
>  int device_detect_by_name(const char *devname);
> +void device_detect_all(void);
>  
>  /* Unregister a device. This function can fail, e.g. when the device
>   * has children.

	Sam

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 04/21] net: Add functions to get/set nameserver and domainname

[Sam Ravnborg]

On Fri, Nov 24, 2017 at 09:12:20AM +0100, Sascha Hauer wrote:
> It's more convenient to have getter/setter functions for
> variables rather than using the detour around global vars
> which use string matching and all kinds of overhead in the
> background.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  include/net.h |  4 ++++
>  net/dns.c     |  8 ++------
>  net/net.c     | 24 ++++++++++++++++++++++++
>  3 files changed, 30 insertions(+), 6 deletions(-)
> 
> diff --git a/include/net.h b/include/net.h
> index 632b6d5410..0fcde2f0b3 100644
> --- a/include/net.h
> +++ b/include/net.h
> @@ -218,8 +218,12 @@ void net_set_ip(IPaddr_t ip);
>  void net_set_serverip(IPaddr_t ip);
>  void net_set_netmask(IPaddr_t ip);
>  void net_set_gateway(IPaddr_t ip);
> +void net_set_nameserver(IPaddr_t ip);
> +void net_set_domainname(const char *name);
>  IPaddr_t net_get_ip(void);
>  IPaddr_t net_get_serverip(void);
> +IPaddr_t net_get_nameserver(void);
> +const char *net_get_domainname(void);
>  
>  /* Do the work */
>  void net_poll(void);
> diff --git a/net/dns.c b/net/dns.c
> index 69b8a24861..a8ce7a4484 100644
> --- a/net/dns.c
> +++ b/net/dns.c
> @@ -202,7 +202,6 @@ static void dns_handler(void *ctx, char *packet, unsigned len)
>  IPaddr_t resolv(const char *host)
>  {
>  	IPaddr_t ip;
> -	const char *ns;
>  
>  	if (!string_to_ip(host, &ip))
>  		return ip;
> @@ -211,16 +210,13 @@ IPaddr_t resolv(const char *host)
>  
>  	dns_state = STATE_INIT;
>  
> -	ns = getenv("global.net.nameserver");
> -	if (!ns || !*ns) {
> +	ip = net_get_nameserver();
> +	if (!ip) {
>  		printk("%s: no nameserver specified in $net.nameserver\n",
>  				__func__);
Should this use pr_info - and not printk direct?



>  		return 0;
>  	}
>  
> -	if (string_to_ip(ns, &ip))
> -		return 0;
> -
>  	debug("resolving host %s via nameserver %pI4\n", host, &ip);

And this should be pr_debug (IMO).
The result is the same - but it is more obvious that the pr_ macros are used.

>  static char *net_domainname;
>  
> +void net_set_nameserver(IPaddr_t nameserver)
> +{
> +	net_nameserver = nameserver;
> +}
> +
> +IPaddr_t net_get_nameserver(void)
> +{
> +	return net_nameserver;
> +}
> +
> +void net_set_domainname(const char *name)
> +{
> +	free(net_domainname);
> +	if (name)
> +		net_domainname = xstrdup(name);

> +	else
> +		net_domainname = xstrdup(name);
Looks strange that we check name - and then the if and else clause is the same.

> +};


	Sam

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 06/21] net: dhcp: Do not overwrite serverip if it is valid

[Sam Ravnborg]

Hi Sasha.

>  }
>  
> -void net_set_serverip(IPaddr_t ip)
> +void net_set_serverip(IPaddr_t ip, bool overwrite)
>  {
> +	if (net_serverip && !overwrite)
> +		return;
> +
>  	net_serverip = ip;
>  }

An alternative solution had been to implement two functions:
net_set_serverip() - that would always set the ip.
net_set_empty_serverip() - update ip only if empty

This is more readable than a boolean flag which
you always need to look up to check the interpretation of.

	Sam

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 08/21] net: allow udp connections on specified network device

[Sam Ravnborg]

Hi Sasha.

On Fri, Nov 24, 2017 at 09:12:24AM +0100, Sascha Hauer wrote:
> This allows the DHCP code to configure specific network
> devices so that DHCP no longer depends on any "current"
> network device.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  include/net.h |  4 ++++
>  net/net.c     | 32 ++++++++++++++++++++------------
>  2 files changed, 24 insertions(+), 12 deletions(-)
> 
> diff --git a/include/net.h b/include/net.h
> index e75f64fe75..6788f14cb0 100644
> --- a/include/net.h
> +++ b/include/net.h
> @@ -443,6 +443,10 @@ static inline char *net_alloc_packet(void)
>  struct net_connection *net_udp_new(IPaddr_t dest, uint16_t dport,
>  		rx_handler_f *handler, void *ctx);
>  
> +struct net_connection *net_udp_eth_new(struct eth_device *edev, IPaddr_t dest,
> +                                       uint16_t dport, rx_handler_f *handler,
> +                                       void *ctx);
> +
>  struct net_connection *net_icmp_new(IPaddr_t dest, rx_handler_f *handler,
>  		void *ctx);
>  
> diff --git a/net/net.c b/net/net.c
> index 5f9535fc40..1d47bb449a 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -141,9 +141,8 @@ static void arp_handler(struct arprequest *arp)
>  	}
>  }
>  
> -static int arp_request(IPaddr_t dest, unsigned char *ether)
> +static int arp_request(struct eth_device *edev, IPaddr_t dest, unsigned char *ether)
>  {
> -	struct eth_device *edev = eth_get_current();
>  	char *pkt;
>  	struct arprequest *arp;
>  	uint64_t arp_start;
> @@ -288,15 +287,17 @@ IPaddr_t net_get_gateway(void)
>  
>  static LIST_HEAD(connection_list);
>  
> -static struct net_connection *net_new(IPaddr_t dest, rx_handler_f *handler,
> -		void *ctx)
> +static struct net_connection *net_new(struct eth_device *edev, IPaddr_t dest,
> +				      rx_handler_f *handler, void *ctx)
>  {
> -	struct eth_device *edev = eth_get_current();
>  	struct net_connection *con;
>  	int ret;
>  
> -	if (!edev)
> -		return ERR_PTR(-ENETDOWN);
> +	if (!edev) {
> +		edev = eth_get_current();
> +		if (!edev)
> +			return ERR_PTR(-ENETDOWN);
> +	}
>  
>  	if (!is_valid_ether_addr(edev->ethaddr)) {
>  		char str[sizeof("xx:xx:xx:xx:xx:xx")];
> @@ -325,7 +326,7 @@ static struct net_connection *net_new(IPaddr_t dest, rx_handler_f *handler,
>  	if (dest == 0xffffffff) {
>  		memset(con->et->et_dest, 0xff, 6);
>  	} else {
> -		ret = arp_request(dest, con->et->et_dest);
> +		ret = arp_request(edev, dest, con->et->et_dest);
>  		if (ret)
>  			goto out;
>  	}
> @@ -349,10 +350,11 @@ out:
>  	return ERR_PTR(ret);
>  }
>  
> -struct net_connection *net_udp_new(IPaddr_t dest, uint16_t dport,
> -		rx_handler_f *handler, void *ctx)
> +struct net_connection *net_udp_eth_new(struct eth_device *edev, IPaddr_t dest,
> +				       uint16_t dport, rx_handler_f *handler,
> +				       void *ctx)
>  {
> -	struct net_connection *con = net_new(dest, handler, ctx);
> +	struct net_connection *con = net_new(edev, 0xffffffff, handler, ctx);

I could not follow the code here.
But it looks strange that in this code snippet dest
was replaced by the magic 0xffffffff.

	Sam

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 09/21] net: dhcp: Allow to specify network device

[Sam Ravnborg]

Hi Sasha.

>  
> -	dhcp_con = net_udp_new(0xffffffff, PORT_BOOTPS, dhcp_handler, NULL);
> +	dhcp_con = net_udp_eth_new(edev, 0xffffffff, PORT_BOOTPS, dhcp_handler, NULL);

The constant 0xffffffff appears in a few places.
A proper define would properly be better.

#define IP_BROADCAST  0xffffffff

	Sam

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 12/21] net: dhcp: rework

[Sam Ravnborg]

Hi Sasha.

On Fri, Nov 24, 2017 at 09:12:28AM +0100, Sascha Hauer wrote:
> The DHCP code is a mess. It is not clear which options are sent to the
> server and which options are returned from the server. Also environment
> variables are read from and written to all over the place.
> 
> This patch cleans this up. There now is struct dhcp_req_param which is
> used for options sent to the server and struct dhcp_result which contains
> the values sent from the server. The values from the server are written
> to the barebox variables in a single place. Also it's now possible to
> call the dhcp code without modifying barebox variables at all, storing
> the result only in the dhcp result struct.

Browsing the code gave a nice impression that things
got much simpler.
Noticed one thing.

This struct is not the smallest..
> +
> +struct dhcp_result {
> +	IPaddr_t ip;
> +	IPaddr_t netmask;
> +	IPaddr_t gateway;
> +	IPaddr_t nameserver;
> +	IPaddr_t serverip;
> +	char *hostname;
> +	char *domainname;
> +	char *rootpath;
> +	char *devicetree;
> +	char *bootfile;
> +	char *tftp_server_name;
> +	uint32_t leasetime;
>  };

> +struct dhcp_req_param dhcp_param;
> +struct dhcp_result *dhcp_result;
Is this why the struct is xalloc()'er and not a static variable like dhcp_param?
Maybe the lifetime of the two differs - which may also explain it.

	Sam

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 08/21] net: allow udp connections on specified network device

[Sascha Hauer]

Hi Sam,

On Sat, Nov 25, 2017 at 05:41:57PM +0100, Sam Ravnborg wrote:
> Hi Sasha.
> 
> On Fri, Nov 24, 2017 at 09:12:24AM +0100, Sascha Hauer wrote:
> > This allows the DHCP code to configure specific network
> > devices so that DHCP no longer depends on any "current"
> > network device.
> > 
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > ---
> >  include/net.h |  4 ++++
> >  net/net.c     | 32 ++++++++++++++++++++------------
> >  2 files changed, 24 insertions(+), 12 deletions(-)
> > 
> > diff --git a/include/net.h b/include/net.h
> > index e75f64fe75..6788f14cb0 100644
> > --- a/include/net.h
> > +++ b/include/net.h
> > @@ -443,6 +443,10 @@ static inline char *net_alloc_packet(void)
> >  struct net_connection *net_udp_new(IPaddr_t dest, uint16_t dport,
> >  		rx_handler_f *handler, void *ctx);
> >  
> > +struct net_connection *net_udp_eth_new(struct eth_device *edev, IPaddr_t dest,
> > +                                       uint16_t dport, rx_handler_f *handler,
> > +                                       void *ctx);
> > +
> >  struct net_connection *net_icmp_new(IPaddr_t dest, rx_handler_f *handler,
> >  		void *ctx);
> >  
> > diff --git a/net/net.c b/net/net.c
> > index 5f9535fc40..1d47bb449a 100644
> > --- a/net/net.c
> > +++ b/net/net.c
> > @@ -141,9 +141,8 @@ static void arp_handler(struct arprequest *arp)
> >  	}
> >  }
> >  
> > -static int arp_request(IPaddr_t dest, unsigned char *ether)
> > +static int arp_request(struct eth_device *edev, IPaddr_t dest, unsigned char *ether)
> >  {
> > -	struct eth_device *edev = eth_get_current();
> >  	char *pkt;
> >  	struct arprequest *arp;
> >  	uint64_t arp_start;
> > @@ -288,15 +287,17 @@ IPaddr_t net_get_gateway(void)
> >  
> >  static LIST_HEAD(connection_list);
> >  
> > -static struct net_connection *net_new(IPaddr_t dest, rx_handler_f *handler,
> > -		void *ctx)
> > +static struct net_connection *net_new(struct eth_device *edev, IPaddr_t dest,
> > +				      rx_handler_f *handler, void *ctx)
> >  {
> > -	struct eth_device *edev = eth_get_current();
> >  	struct net_connection *con;
> >  	int ret;
> >  
> > -	if (!edev)
> > -		return ERR_PTR(-ENETDOWN);
> > +	if (!edev) {
> > +		edev = eth_get_current();
> > +		if (!edev)
> > +			return ERR_PTR(-ENETDOWN);
> > +	}
> >  
> >  	if (!is_valid_ether_addr(edev->ethaddr)) {
> >  		char str[sizeof("xx:xx:xx:xx:xx:xx")];
> > @@ -325,7 +326,7 @@ static struct net_connection *net_new(IPaddr_t dest, rx_handler_f *handler,
> >  	if (dest == 0xffffffff) {
> >  		memset(con->et->et_dest, 0xff, 6);
> >  	} else {
> > -		ret = arp_request(dest, con->et->et_dest);
> > +		ret = arp_request(edev, dest, con->et->et_dest);
> >  		if (ret)
> >  			goto out;
> >  	}
> > @@ -349,10 +350,11 @@ out:
> >  	return ERR_PTR(ret);
> >  }
> >  
> > -struct net_connection *net_udp_new(IPaddr_t dest, uint16_t dport,
> > -		rx_handler_f *handler, void *ctx)
> > +struct net_connection *net_udp_eth_new(struct eth_device *edev, IPaddr_t dest,
> > +				       uint16_t dport, rx_handler_f *handler,
> > +				       void *ctx)
> >  {
> > -	struct net_connection *con = net_new(dest, handler, ctx);
> > +	struct net_connection *con = net_new(edev, 0xffffffff, handler, ctx);
> 
> I could not follow the code here.
> But it looks strange that in this code snippet dest
> was replaced by the magic 0xffffffff.

This is wrong here. It gets replaced again with 'dest' in

[PATCH 13/21] net: Pick network device based on IP settings

I just removed this wrong hunk here.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 04/21] net: Add functions to get/set nameserver and domainname

[Sascha Hauer]

On Sat, Nov 25, 2017 at 12:47:09AM +0100, Sam Ravnborg wrote:
> On Fri, Nov 24, 2017 at 09:12:20AM +0100, Sascha Hauer wrote:
> > It's more convenient to have getter/setter functions for
> > variables rather than using the detour around global vars
> > which use string matching and all kinds of overhead in the
> > background.
> > 
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > ---
> >  include/net.h |  4 ++++
> >  net/dns.c     |  8 ++------
> >  net/net.c     | 24 ++++++++++++++++++++++++
> >  3 files changed, 30 insertions(+), 6 deletions(-)
> > 
> > diff --git a/include/net.h b/include/net.h
> > index 632b6d5410..0fcde2f0b3 100644
> > --- a/include/net.h
> > +++ b/include/net.h
> > @@ -218,8 +218,12 @@ void net_set_ip(IPaddr_t ip);
> >  void net_set_serverip(IPaddr_t ip);
> >  void net_set_netmask(IPaddr_t ip);
> >  void net_set_gateway(IPaddr_t ip);
> > +void net_set_nameserver(IPaddr_t ip);
> > +void net_set_domainname(const char *name);
> >  IPaddr_t net_get_ip(void);
> >  IPaddr_t net_get_serverip(void);
> > +IPaddr_t net_get_nameserver(void);
> > +const char *net_get_domainname(void);
> >  
> >  /* Do the work */
> >  void net_poll(void);
> > diff --git a/net/dns.c b/net/dns.c
> > index 69b8a24861..a8ce7a4484 100644
> > --- a/net/dns.c
> > +++ b/net/dns.c
> > @@ -202,7 +202,6 @@ static void dns_handler(void *ctx, char *packet, unsigned len)
> >  IPaddr_t resolv(const char *host)
> >  {
> >  	IPaddr_t ip;
> > -	const char *ns;
> >  
> >  	if (!string_to_ip(host, &ip))
> >  		return ip;
> > @@ -211,16 +210,13 @@ IPaddr_t resolv(const char *host)
> >  
> >  	dns_state = STATE_INIT;
> >  
> > -	ns = getenv("global.net.nameserver");
> > -	if (!ns || !*ns) {
> > +	ip = net_get_nameserver();
> > +	if (!ip) {
> >  		printk("%s: no nameserver specified in $net.nameserver\n",
> >  				__func__);
> Should this use pr_info - and not printk direct?

Yes, although this hasn't been touched in this patch. I added a patch to
this series changing this consitently for this file.

> > +void net_set_domainname(const char *name)
> > +{
> > +	free(net_domainname);
> > +	if (name)
> > +		net_domainname = xstrdup(name);
> 
> > +	else
> > +		net_domainname = xstrdup(name);
> Looks strange that we check name - and then the if and else clause is the same.

Yeah, indeed. Changed to xstrdup("") for the name == NULL case.

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 06/21] net: dhcp: Do not overwrite serverip if it is valid

[Sascha Hauer]

Hi Sam,

On Sat, Nov 25, 2017 at 05:36:23PM +0100, Sam Ravnborg wrote:
> Hi Sasha.
> 
> >  }
> >  
> > -void net_set_serverip(IPaddr_t ip)
> > +void net_set_serverip(IPaddr_t ip, bool overwrite)
> >  {
> > +	if (net_serverip && !overwrite)
> > +		return;
> > +
> >  	net_serverip = ip;
> >  }
> 
> An alternative solution had been to implement two functions:
> net_set_serverip() - that would always set the ip.
> net_set_empty_serverip() - update ip only if empty
> 
> This is more readable than a boolean flag which
> you always need to look up to check the interpretation of.

Good idea. Just did that.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 09/21] net: dhcp: Allow to specify network device

[Sascha Hauer]

Hi Sam,

On Sat, Nov 25, 2017 at 05:46:47PM +0100, Sam Ravnborg wrote:
> Hi Sasha.
> 
> >  
> > -	dhcp_con = net_udp_new(0xffffffff, PORT_BOOTPS, dhcp_handler, NULL);
> > +	dhcp_con = net_udp_eth_new(edev, 0xffffffff, PORT_BOOTPS, dhcp_handler, NULL);
> 
> The constant 0xffffffff appears in a few places.
> A proper define would properly be better.

Ok, added a patch for it.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 12/21] net: dhcp: rework

[Sascha Hauer]

On Sat, Nov 25, 2017 at 06:02:51PM +0100, Sam Ravnborg wrote:
> Hi Sasha.
> 
> On Fri, Nov 24, 2017 at 09:12:28AM +0100, Sascha Hauer wrote:
> > The DHCP code is a mess. It is not clear which options are sent to the
> > server and which options are returned from the server. Also environment
> > variables are read from and written to all over the place.
> > 
> > This patch cleans this up. There now is struct dhcp_req_param which is
> > used for options sent to the server and struct dhcp_result which contains
> > the values sent from the server. The values from the server are written
> > to the barebox variables in a single place. Also it's now possible to
> > call the dhcp code without modifying barebox variables at all, storing
> > the result only in the dhcp result struct.
> 
> Browsing the code gave a nice impression that things
> got much simpler.
> Noticed one thing.
> 
> This struct is not the smallest..
> > +
> > +struct dhcp_result {
> > +	IPaddr_t ip;
> > +	IPaddr_t netmask;
> > +	IPaddr_t gateway;
> > +	IPaddr_t nameserver;
> > +	IPaddr_t serverip;
> > +	char *hostname;
> > +	char *domainname;
> > +	char *rootpath;
> > +	char *devicetree;
> > +	char *bootfile;
> > +	char *tftp_server_name;
> > +	uint32_t leasetime;
> >  };
> 
> > +struct dhcp_req_param dhcp_param;
> > +struct dhcp_result *dhcp_result;
> Is this why the struct is xalloc()'er and not a static variable like dhcp_param?
> Maybe the lifetime of the two differs - which may also explain it.

Yes, indeed, it's the lifetime. dhcp_param is static because it's used
only internally during lifetime of the dhcp request. dhcp_result is
returned from the dhcp code, and I don't like to return static data
which potentially gets overwritten in the nect dhcp call.
(IMO It would be even nicer if the dhcp code didn't use static data at
all, maybe next time).

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 01/21] driver: Add device_detect_all() function

[Sascha Hauer]

On Sat, Nov 25, 2017 at 12:34:27AM +0100, Sam Ravnborg wrote:
> Hi Sasha.
> 
> On Fri, Nov 24, 2017 at 09:12:17AM +0100, Sascha Hauer wrote:
> > Add a device_detect_all function to detect all devices and use it
> > in the detect command. This makes the functionality reusable in other
> > code.
> > 
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > ---
> >  commands/detect.c     | 6 +-----
> >  drivers/base/driver.c | 8 ++++++++
> >  include/driver.h      | 1 +
> >  3 files changed, 10 insertions(+), 5 deletions(-)
> > 
> > diff --git a/commands/detect.c b/commands/detect.c
> > index 1586a6fb54..42e111419f 100644
> > --- a/commands/detect.c
> > +++ b/commands/detect.c
> > @@ -56,11 +56,7 @@ static int do_detect(int argc, char *argv[])
> >  	}
> >  
> >  	if (option_all) {
> > -		for_each_device(dev) {
> > -			ret = device_detect(dev);
> > -			if (ret && ret != -ENOSYS && option_error)
> > -				return ret;
> > -		}
> > +		device_detect_all();
> 
> With this change there is no longer any checks
> if device_detect() fails.
> so the option "-e" is no longer useful in combination
> with option "-a" (which uses the patched code).

Of course we could bail out of device_detect_all() when an error
occurs, I'm not sure though how useful this is. I mean when your
SATA drivers detect function returns an error because there is no
drive connected, why would you want to bail out of the detection of
other devices?

In an earlier version of this series I completely removed the -e
option. Maybe that would be better?

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 01/21] driver: Add device_detect_all() function

[Sam Ravnborg]

> > >  
> > >  	if (option_all) {
> > > -		for_each_device(dev) {
> > > -			ret = device_detect(dev);
> > > -			if (ret && ret != -ENOSYS && option_error)
> > > -				return ret;
> > > -		}
> > > +		device_detect_all();
> > 
> > With this change there is no longer any checks
> > if device_detect() fails.
> > so the option "-e" is no longer useful in combination
> > with option "-a" (which uses the patched code).
> 
> Of course we could bail out of device_detect_all() when an error
> occurs, I'm not sure though how useful this is. I mean when your
> SATA drivers detect function returns an error because there is no
> drive connected, why would you want to bail out of the detection of
> other devices?
> 
> In an earlier version of this series I completely removed the -e
> option. Maybe that would be better?

My comment was solely on the inconsistency.
When I want to stop if an error occur I specify -e, otherwise
I expect it to run to the end.
So we should either respect -e always or drop it.
Waht is the best I do not know, but the current middel ground
where -e only have effect in some cases is confusing and should be avoided.

	Sam

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox