mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH 0/8 v3] Network update DHCP/BOOTP
@ 2012-04-02 14:17 Jean-Christophe PLAGNIOL-VILLARD
  2012-04-02 14:19 ` [PATCH 1/8] net: dhcp: factorise option recption handling Jean-Christophe PLAGNIOL-VILLARD
                   ` (7 more replies)
  0 siblings, 8 replies; 13+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2012-04-02 14:17 UTC (permalink / raw)
  To: barebox

HI,

	the following patch series update the network dhcp and bootp support
	to support options 66, 61, 77, 97

	this will also help to add pxe boot support later

The following changes since commit c21492c293bcb97ac99f837489898582205cd168:

  AT91: at91sam9x5: add chip and board file (2012-04-02 10:31:23 +0200)

are available in the git repository at:

  git://git.jcrosoft.org/barebox.git net

for you to fetch changes up to f09107f16b01328138aed324b3e11c96b318369f:

  net: env: getenv_ip use resolv (2012-04-02 22:26:04 +0800)

----------------------------------------------------------------
Jean-Christophe PLAGNIOL-VILLARD (8):
      net: dhcp: factorise option recption handling
      net: dhcp: reset env variable before do a dhcp request
      net: dhcp: add support of tftp name server
      net: dhcp: factorise setting option code
      net: dhcp: allow to set transmitted client id
      net: dhcp: allow to set transmitted client uuid
      net: dhcp: allow to set transmitted user class
      net: env: getenv_ip use resolv

 include/net.h |    6 +-
 net/dhcp.c    |  451 +++++++++++++++++++++++++++++++++++++++++----------------
 net/dns.c     |    2 +-
 net/net.c     |    9 +-
 4 files changed, 340 insertions(+), 128 deletions(-)

Best Regards,
J.

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/8] net: dhcp: factorise option recption handling
  2012-04-02 14:17 [PATCH 0/8 v3] Network update DHCP/BOOTP Jean-Christophe PLAGNIOL-VILLARD
@ 2012-04-02 14:19 ` Jean-Christophe PLAGNIOL-VILLARD
  2012-04-02 19:02   ` Sascha Hauer
  2012-04-02 14:19 ` [PATCH 2/8] net: dhcp: reset env variable before do a dhcp request Jean-Christophe PLAGNIOL-VILLARD
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 13+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2012-04-02 14:19 UTC (permalink / raw)
  To: barebox

Instead of using a static switch case the handle the received option
use an array of supported option.

This will allow to unset the env var without duplicating the code.

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
 net/dhcp.c |  264 ++++++++++++++++++++++++++++++++++++++++--------------------
 1 files changed, 176 insertions(+), 88 deletions(-)

diff --git a/net/dhcp.c b/net/dhcp.c
index 53eed6c..3b035d7 100644
--- a/net/dhcp.c
+++ b/net/dhcp.c
@@ -76,6 +76,147 @@ static uint32_t dhcp_leasetime;
 static IPaddr_t net_dhcp_server_ip;
 static uint64_t dhcp_start;
 
+struct dhcp_opt {
+	unsigned char option;
+	const char *barebox_var_name;
+	void (*handle)(struct dhcp_opt *opt, unsigned char *data, int tlen);
+	void *data;
+
+	/* request automatically the option when creating the DHCP request */
+	bool optinal;
+
+	struct bootp *bp;
+};
+
+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);
+}
+
+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;
+
+	if (!opt->barebox_var_name) {
+		pr_err("dhcp: %s: option %d no barebox_var_name set\n",
+		       __func__, opt->option);
+		return;
+	}
+
+	ip = net_read_ip(popt);
+	setenv_ip(opt->barebox_var_name, ip);
+}
+
+static void env_str_handle(struct dhcp_opt *opt, unsigned char *popt, int optlen)
+{
+	char str[256];
+
+	if (!opt->barebox_var_name) {
+		pr_err("dhcp: %s: option %d no barebox_var_name set\n",
+		       __func__, opt->option);
+		return;
+	}
+
+	memcpy(str, popt, optlen);
+	str[optlen] = 0;
+	setenv(opt->barebox_var_name, str);
+}
+
+static void copy_uint32_handle(struct dhcp_opt *opt, unsigned char *popt, int optlen)
+{
+	if (!opt->data) {
+		pr_err("dhcp: %s: option %d no data set\n", __func__, opt->option);
+		return;
+	}
+
+	net_copy_uint32(opt->data, (uint32_t *)popt);
+};
+
+static void copy_ip_handle(struct dhcp_opt *opt, unsigned char *popt, int optlen)
+{
+	if (!opt->data) {
+		pr_err("dhcp: %s: option %d no data set\n", __func__, opt->option);
+		return;
+	}
+
+	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);
+}
+
+struct dhcp_opt dhcp_options[] = {
+	{
+		.option = 1,
+		.handle = netmask_handle,
+	}, {
+		.option = 3,
+		.handle = gateway_handle,
+	}, {
+		.option = 6,
+		.handle = env_ip_handle,
+		.barebox_var_name = "nameserver",
+	}, {
+		.option = 12,
+		.handle = env_str_handle,
+		.barebox_var_name = "hostname",
+	}, {
+		.option = 15,
+		.handle = env_str_handle,
+		.barebox_var_name = "domainname",
+	}, {
+		.option = 17,
+		.handle = env_str_handle,
+		.barebox_var_name = "rootpath",
+	}, {
+		.option = 51,
+		.handle = copy_uint32_handle,
+		.data = &dhcp_leasetime,
+	}, {
+		.option = 54,
+		.handle = copy_ip_handle,
+		.data = &net_dhcp_server_ip,
+		.optinal = true,
+	}, {
+		.option = 67,
+		.handle = bootfile_vendorex_handle,
+		.barebox_var_name = "bootfile",
+	},
+};
+
 static int bootp_check_packet(unsigned char *pkt, unsigned src, unsigned len)
 {
 	struct bootp *bp = (struct bootp *) pkt;
@@ -131,6 +272,7 @@ static void bootp_copy_net_params(struct bootp *bp)
 static int dhcp_extended (u8 *e, int message_type, IPaddr_t ServerID,
 			  IPaddr_t RequestedIP, char *vendor_id)
 {
+	int i;
 	u8 *start = e;
 	u8 *cnt;
 	int vendor_id_len = vendor_id ? strlen(vendor_id) : 0;
@@ -181,18 +323,13 @@ static int dhcp_extended (u8 *e, int message_type, IPaddr_t ServerID,
 	*e++ = 55;		/* Parameter Request List */
 	 cnt = e++;		/* Pointer to count of requested items */
 	*cnt = 0;
-	*e++  = 1;		/* Subnet Mask */
-	*cnt += 1;
-	*e++  = 3;		/* Router Option */
-	*cnt += 1;
-	*e++  = 6;		/* DNS Server(s) */
-	*cnt += 1;
-	*e++  = 12;		/* Hostname */
-	*cnt += 1;
-	*e++  = 15;		/* domain name */
-	*cnt += 1;
-	*e++  = 17;		/* Boot path */
-	*cnt += 1;
+
+	for (i = 0; i < ARRAY_SIZE(dhcp_options); i++) {
+		if (dhcp_options[i].optinal)
+			continue;
+		*e++  = dhcp_options[i].option;
+		*cnt += 1;
+	}
 	*e++  = 255;		/* End of the list */
 
 	/* Pad to minimal length */
@@ -246,89 +383,40 @@ static int bootp_request(void)
 	return ret;
 }
 
+static int dhcp_options_handle(unsigned char option, unsigned char *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;
+			opt->handle(opt, popt, optlen);
+			goto end;
+		}
+	}
+
+end:
+	return i;
+}
+
 static void dhcp_options_process(unsigned char *popt, struct bootp *bp)
 {
 	unsigned char *end = popt + sizeof(*bp) + OPT_SIZE;
 	int oplen;
-	IPaddr_t ip;
-	char str[256];
+	unsigned char option;
+	int i;
 
 	while (popt < end && *popt != 0xff) {
 		oplen = *(popt + 1);
-		switch (*popt) {
-		case 1:
-			ip = net_read_ip(popt + 2);
-			net_set_netmask(ip);
-			break;
-		case 3:
-			ip = net_read_ip(popt + 2);
-			net_set_gateway(ip);
-			break;
-		case 6:
-			ip = net_read_ip(popt + 2);
-			setenv_ip("nameserver", ip);
-			break;
-		case 12:
-			memcpy(str, popt + 2, oplen);
-			str[oplen] = 0;
-			setenv("hostname", str);
-			break;
-		case 15:
-			memcpy(str, popt + 2, oplen);
-			str[oplen] = 0;
-			setenv("domainname", str);
-			break;
-		case 17:
-			memcpy(str, popt + 2, oplen);
-			str[oplen] = 0;
-			setenv("rootpath", str);
-			break;
-		case 51:
-			net_copy_uint32 (&dhcp_leasetime, (uint32_t *)(popt + 2));
-			break;
-		case 53:	/* Ignore Message Type Option */
-			break;
-		case 54:
-			net_copy_ip(&net_dhcp_server_ip, (popt + 2));
-			break;
-		case 58:	/* Ignore Renewal Time Option */
-			break;
-		case 59:	/* Ignore Rebinding Time Option */
-			break;
-		case 66:	/* Ignore TFTP server name */
-			break;
-		case 67:	/* vendor opt bootfile */
-			/*
-			 * 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
-			 */
-			memcpy(str, popt + 2, oplen);
-			str[oplen] = 0;
-			if (bp->bp_file[0] == '\0') {
-				/*
-				 * 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
-				 */
-				printf("*** WARNING: using vendor "
-					"optional boot file\n");
-					setenv("bootfile", str);
-			}
-			break;
-		default:
-#ifdef CONFIG_BOOTP_VENDOREX
-			if (dhcp_vendorex_proc (popt))
-				break;
-#endif
-			debug("*** Unhandled DHCP Option in OFFER/ACK: %d\n", *popt);
-			break;
-		}
+		option = *popt;
+
+		i = dhcp_options_handle(option, popt + 2, oplen, bp);
+		if (i == ARRAY_SIZE(dhcp_options))
+			debug("*** Unhandled DHCP Option in OFFER/ACK: %d\n", option);
+
 		popt += oplen + 2;	/* Process next option */
 	}
 }
-- 
1.7.9.1


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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 2/8] net: dhcp: reset env variable before do a dhcp request
  2012-04-02 14:17 [PATCH 0/8 v3] Network update DHCP/BOOTP Jean-Christophe PLAGNIOL-VILLARD
  2012-04-02 14:19 ` [PATCH 1/8] net: dhcp: factorise option recption handling Jean-Christophe PLAGNIOL-VILLARD
@ 2012-04-02 14:19 ` Jean-Christophe PLAGNIOL-VILLARD
  2012-04-02 14:19 ` [PATCH 3/8] net: dhcp: add support of tftp name server Jean-Christophe PLAGNIOL-VILLARD
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2012-04-02 14:19 UTC (permalink / raw)
  To: barebox

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
 net/dhcp.c |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/net/dhcp.c b/net/dhcp.c
index 3b035d7..3c48aaa 100644
--- a/net/dhcp.c
+++ b/net/dhcp.c
@@ -536,11 +536,27 @@ static void dhcp_handler(void *ctx, char *packet, unsigned int len)
 	}
 }
 
+static void dhcp_reset_env(void)
+{
+	struct dhcp_opt *opt;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(dhcp_options); i++) {
+		opt = &dhcp_options[i];
+		if (!opt->barebox_var_name)
+			continue;
+
+		setenv(opt->barebox_var_name,"");
+	}
+}
+
 static int do_dhcp(int argc, char *argv[])
 {
 	int ret, opt;
 	char *vendor_id = (char*)getenv("dhcp_vendor_id");
 
+	dhcp_reset_env();
+
 	while((opt = getopt(argc, argv, "v:")) > 0) {
 		switch(opt) {
 		case 'v':
-- 
1.7.9.1


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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 3/8] net: dhcp: add support of tftp name server
  2012-04-02 14:17 [PATCH 0/8 v3] Network update DHCP/BOOTP Jean-Christophe PLAGNIOL-VILLARD
  2012-04-02 14:19 ` [PATCH 1/8] net: dhcp: factorise option recption handling Jean-Christophe PLAGNIOL-VILLARD
  2012-04-02 14:19 ` [PATCH 2/8] net: dhcp: reset env variable before do a dhcp request Jean-Christophe PLAGNIOL-VILLARD
@ 2012-04-02 14:19 ` Jean-Christophe PLAGNIOL-VILLARD
  2012-04-02 14:19 ` [PATCH 4/8] net: dhcp: factorise setting option code Jean-Christophe PLAGNIOL-VILLARD
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2012-04-02 14:19 UTC (permalink / raw)
  To: barebox

if the DNS is enable resolve it. The server ip will be set if no server ip it
is set in the bootp.
Export it via env dhcp_tftp_server_name.

E.g. the ISC dhcp server can be configured with

   | class "at91sam9x5ek" {
   |         match if substring (option vendor-class-identifier,0,20) = "barebox-at91sam9x
   |
   |         filename "/tftpboot/atmel/at91sam9x5/sam9x5ek/zImage";
   |         option tftp-server-name "192.168.200.98";
   |         option root-path "192.168.200.98:/opt/work/buildroot/build/sam9x5/target";
   |         }
   | }

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
 net/dhcp.c |   20 +++++++++++++++++---
 1 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/net/dhcp.c b/net/dhcp.c
index 3c48aaa..12df542 100644
--- a/net/dhcp.c
+++ b/net/dhcp.c
@@ -75,6 +75,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 char dhcp_tftpname[256];
 
 struct dhcp_opt {
 	unsigned char option;
@@ -121,6 +122,7 @@ static void env_ip_handle(struct dhcp_opt *opt, unsigned char *popt, int optlen)
 static void env_str_handle(struct dhcp_opt *opt, unsigned char *popt, int optlen)
 {
 	char str[256];
+	char *tmp = str;
 
 	if (!opt->barebox_var_name) {
 		pr_err("dhcp: %s: option %d no barebox_var_name set\n",
@@ -128,9 +130,12 @@ static void env_str_handle(struct dhcp_opt *opt, unsigned char *popt, int optlen
 		return;
 	}
 
-	memcpy(str, popt, optlen);
-	str[optlen] = 0;
-	setenv(opt->barebox_var_name, str);
+	if (opt->data)
+		tmp = opt->data;
+
+	memcpy(tmp, popt, optlen);
+	tmp[optlen] = 0;
+	setenv(opt->barebox_var_name, tmp);
 }
 
 static void copy_uint32_handle(struct dhcp_opt *opt, unsigned char *popt, int optlen)
@@ -211,6 +216,11 @@ struct dhcp_opt dhcp_options[] = {
 		.data = &net_dhcp_server_ip,
 		.optinal = true,
 	}, {
+		.option = 66,
+		.handle = env_str_handle,
+		.barebox_var_name = "dhcp_tftp_server_name",
+		.data = dhcp_tftpname,
+	}, {
 		.option = 67,
 		.handle = bootfile_vendorex_handle,
 		.barebox_var_name = "bootfile",
@@ -419,6 +429,9 @@ static void dhcp_options_process(unsigned char *popt, struct bootp *bp)
 
 		popt += oplen + 2;	/* Process next option */
 	}
+
+	if (dhcp_tftpname[0] != 0)
+		net_set_serverip(resolv(dhcp_tftpname));
 }
 
 static int dhcp_message_type(unsigned char *popt)
@@ -624,3 +637,4 @@ BAREBOX_MAGICVAR(hostname, "hostname returned from DHCP request");
 BAREBOX_MAGICVAR(domainname, "domainname returned from DHCP request");
 BAREBOX_MAGICVAR(rootpath, "rootpath returned from DHCP request");
 BAREBOX_MAGICVAR(dhcp_vendor_id, "vendor id to send to the DHCP server");
+BAREBOX_MAGICVAR(dhcp_tftp_server_name, "TFTP server Name returned from DHCP request");
-- 
1.7.9.1


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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 4/8] net: dhcp: factorise setting option code
  2012-04-02 14:17 [PATCH 0/8 v3] Network update DHCP/BOOTP Jean-Christophe PLAGNIOL-VILLARD
                   ` (2 preceding siblings ...)
  2012-04-02 14:19 ` [PATCH 3/8] net: dhcp: add support of tftp name server Jean-Christophe PLAGNIOL-VILLARD
@ 2012-04-02 14:19 ` Jean-Christophe PLAGNIOL-VILLARD
  2012-04-02 14:19 ` [PATCH 5/8] net: dhcp: allow to set transmitted client id Jean-Christophe PLAGNIOL-VILLARD
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2012-04-02 14:19 UTC (permalink / raw)
  To: barebox

This will allow to add more easly new option with less impact in the binary.

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
 net/dhcp.c |  121 +++++++++++++++++++++++++++++++++++++++++++-----------------
 1 files changed, 87 insertions(+), 34 deletions(-)

diff --git a/net/dhcp.c b/net/dhcp.c
index 12df542..7d01d4b 100644
--- a/net/dhcp.c
+++ b/net/dhcp.c
@@ -227,6 +227,79 @@ struct dhcp_opt dhcp_options[] = {
 	},
 };
 
+struct dhcp_param {
+	unsigned char option;
+	const char *barebox_var_name;
+	int (*handle)(struct dhcp_param *param, u8 *e);
+	void *data;
+};
+
+static int dhcp_set_string_options(struct dhcp_param *param, u8 *e)
+{
+	int str_len;
+	char* str = param->data;
+
+	if (!str && param->barebox_var_name)
+		str = (char*)getenv(param->barebox_var_name);
+
+	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_VENDOR_ID		60
+
+struct dhcp_param dhcp_params[] = {
+	{
+		.option = DHCP_VENDOR_ID,
+		.handle = dhcp_set_string_options,
+		.barebox_var_name = "dhcp_vendor_id",
+	}
+};
+
+static void dhcp_set_param_data(int option, void* data)
+{
+	struct dhcp_param *param;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(dhcp_params); i++) {
+		param = &dhcp_params[i];
+
+		if (param->option == option) {
+			param->data = data;
+			return;
+		}
+	}
+}
+
+static int dhcp_set_ip_options(int option, u8 *e, IPaddr_t ip)
+{
+	int tmp;
+
+	if (!ip)
+		return 0;
+
+	tmp = ntohl(ip);
+
+	*e++ = option;
+	*e++ = 4;
+	*e++ = tmp >> 24;
+	*e++ = tmp >> 16;
+	*e++ = tmp >> 8;
+	*e++ = tmp & 0xff;
+
+	return 6;
+}
+
 static int bootp_check_packet(unsigned char *pkt, unsigned src, unsigned len)
 {
 	struct bootp *bp = (struct bootp *) pkt;
@@ -280,12 +353,11 @@ 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,
-			  IPaddr_t RequestedIP, char *vendor_id)
+			  IPaddr_t RequestedIP)
 {
 	int i;
 	u8 *start = e;
 	u8 *cnt;
-	int vendor_id_len = vendor_id ? strlen(vendor_id) : 0;
 
 	*e++ = 99;		/* RFC1048 Magic Cookie */
 	*e++ = 130;
@@ -301,34 +373,12 @@ static int dhcp_extended (u8 *e, int message_type, IPaddr_t ServerID,
 	*e++ = (576 - 312 + OPT_SIZE) >> 8;
 	*e++ = (576 - 312 + OPT_SIZE) & 0xff;
 
-	if (ServerID) {
-		int tmp = ntohl (ServerID);
-
-		*e++ = 54;	/* ServerID */
-		*e++ = 4;
-		*e++ = tmp >> 24;
-		*e++ = tmp >> 16;
-		*e++ = tmp >> 8;
-		*e++ = tmp & 0xff;
-	}
 
-	if (RequestedIP) {
-		int tmp = ntohl (RequestedIP);
+	e += dhcp_set_ip_options(50, e, RequestedIP);
+	e += dhcp_set_ip_options(54, e, ServerID);
 
-		*e++ = 50;	/* Requested IP */
-		*e++ = 4;
-		*e++ = tmp >> 24;
-		*e++ = tmp >> 16;
-		*e++ = tmp >> 8;
-		*e++ = tmp & 0xff;
-	}
-
-	if (vendor_id_len > 0) {
-		*e++ = 60;
-		*e++ = vendor_id_len;
-		memcpy(e, vendor_id, vendor_id_len);
-		 e  += vendor_id_len;
-	}
+	for (i = 0; i < ARRAY_SIZE(dhcp_params); i++)
+		e += dhcp_params[i].handle(&dhcp_params[i], e);
 
 	*e++ = 55;		/* Parameter Request List */
 	 cnt = e++;		/* Pointer to count of requested items */
@@ -380,8 +430,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,
-				dhcp_con->priv);
+	ext_len = dhcp_extended((u8 *)bp->bp_vend, DHCP_DISCOVER, 0, 0);
 
 	Bootp_id = (uint32_t)get_time_ns();
 	net_copy_uint32(&bp->bp_id, &Bootp_id);
@@ -486,7 +535,7 @@ static void dhcp_send_request_packet(struct bootp *bp_offer)
 	 */
 	net_copy_ip(&OfferedIP, &bp->bp_yiaddr);
 	extlen = dhcp_extended((u8 *)bp->bp_vend, DHCP_REQUEST, net_dhcp_server_ip,
-				OfferedIP, dhcp_con->priv);
+				OfferedIP);
 
 	debug("Transmitting DHCPREQUEST packet\n");
 	net_udp_send(dhcp_con, sizeof(*bp) + extlen);
@@ -566,19 +615,18 @@ static void dhcp_reset_env(void)
 static int do_dhcp(int argc, char *argv[])
 {
 	int ret, opt;
-	char *vendor_id = (char*)getenv("dhcp_vendor_id");
 
 	dhcp_reset_env();
 
 	while((opt = getopt(argc, argv, "v:")) > 0) {
 		switch(opt) {
 		case 'v':
-			vendor_id = optarg;
+			dhcp_set_param_data(DHCP_VENDOR_ID, optarg);
 			break;
 		}
 	}
 
-	dhcp_con = net_udp_new(0xffffffff, PORT_BOOTPS, dhcp_handler, vendor_id);
+	dhcp_con = net_udp_new(0xffffffff, PORT_BOOTPS, dhcp_handler, NULL);
 	if (IS_ERR(dhcp_con)) {
 		ret = PTR_ERR(dhcp_con);
 		goto out;
@@ -622,6 +670,10 @@ BAREBOX_CMD_HELP_SHORT("Invoke dhcp client to obtain ip/boot params.\n")
 BAREBOX_CMD_HELP_OPT  ("-v <vendor_id>",
 "DHCP Vendor ID (code 60) submitted in DHCP requests. It can\n"
 "be used in the DHCP server's configuration to select options\n"
+"(e.g. bootfile or server) which are valid for barebox clients only.\n")
+BAREBOX_CMD_HELP_OPT  ("-c <client_id>",
+"DHCP Client ID (code 61) submitted in DHCP requests. It can\n"
+"be used in the DHCP server's configuration to select options\n"
 "(e.g. bootfile or server) which are valid for barebox clients only.\n");
 BAREBOX_CMD_HELP_END
 
@@ -637,4 +689,5 @@ BAREBOX_MAGICVAR(hostname, "hostname returned from DHCP request");
 BAREBOX_MAGICVAR(domainname, "domainname returned from DHCP request");
 BAREBOX_MAGICVAR(rootpath, "rootpath returned from DHCP request");
 BAREBOX_MAGICVAR(dhcp_vendor_id, "vendor id to send to the DHCP server");
+BAREBOX_MAGICVAR(dhcp_client_id, "cliend id to send to the DHCP server");
 BAREBOX_MAGICVAR(dhcp_tftp_server_name, "TFTP server Name returned from DHCP request");
-- 
1.7.9.1


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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 5/8] net: dhcp: allow to set transmitted client id
  2012-04-02 14:17 [PATCH 0/8 v3] Network update DHCP/BOOTP Jean-Christophe PLAGNIOL-VILLARD
                   ` (3 preceding siblings ...)
  2012-04-02 14:19 ` [PATCH 4/8] net: dhcp: factorise setting option code Jean-Christophe PLAGNIOL-VILLARD
@ 2012-04-02 14:19 ` Jean-Christophe PLAGNIOL-VILLARD
  2012-04-02 14:19 ` [PATCH 6/8] net: dhcp: allow to set transmitted client uuid Jean-Christophe PLAGNIOL-VILLARD
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2012-04-02 14:19 UTC (permalink / raw)
  To: barebox

For net boot setups it is useful to submit boot params like server or
bootfile over dhcp. To distinguish diffrent type of OS running on the same hardware,
a custom client id can be sent in dhcp discover/request messages.

E.g. the ISC dhcp server can be configured with

 | class "at91sam9x5ek" {
 |         match if substring (option vendor-class-identifier,0,20) = "barebox-at91sam9x5ek";
 |
 |         filename "/tftpboot/atmel/at91sam9x5/sam9x5ek/zImage";
 |         if substring (option dhcp-client-identifier,0,7) = "ser2net" {
 |                 filename "/tftpboot/atmel/at91sam9x5/sam9x5ek/zImage-ser2net";
 |         }
 |         option tftp-server-name "192.168.200.98";
 |         option option-150 192.168.200.98;
 |         next-server 192.168.200.98;
 |         option root-path "192.168.200.98:/opt/work/buildroot/build/sam9x5/target";
 | }

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
 net/dhcp.c |   10 +++++++++-
 1 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/net/dhcp.c b/net/dhcp.c
index 7d01d4b..5b1ab5a 100644
--- a/net/dhcp.c
+++ b/net/dhcp.c
@@ -257,12 +257,17 @@ static int dhcp_set_string_options(struct dhcp_param *param, u8 *e)
 }
 
 #define DHCP_VENDOR_ID		60
+#define DHCP_CLIENT_ID		61
 
 struct dhcp_param dhcp_params[] = {
 	{
 		.option = DHCP_VENDOR_ID,
 		.handle = dhcp_set_string_options,
 		.barebox_var_name = "dhcp_vendor_id",
+	}, {
+		.option = DHCP_CLIENT_ID,
+		.handle = dhcp_set_string_options,
+		.barebox_var_name = "dhcp_client_id",
 	}
 };
 
@@ -618,11 +623,14 @@ static int do_dhcp(int argc, char *argv[])
 
 	dhcp_reset_env();
 
-	while((opt = getopt(argc, argv, "v:")) > 0) {
+	while((opt = getopt(argc, argv, "v:c:")) > 0) {
 		switch(opt) {
 		case 'v':
 			dhcp_set_param_data(DHCP_VENDOR_ID, optarg);
 			break;
+		case 'c':
+			dhcp_set_param_data(DHCP_CLIENT_ID, optarg);
+			break;
 		}
 	}
 
-- 
1.7.9.1


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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 6/8] net: dhcp: allow to set transmitted client uuid
  2012-04-02 14:17 [PATCH 0/8 v3] Network update DHCP/BOOTP Jean-Christophe PLAGNIOL-VILLARD
                   ` (4 preceding siblings ...)
  2012-04-02 14:19 ` [PATCH 5/8] net: dhcp: allow to set transmitted client id Jean-Christophe PLAGNIOL-VILLARD
@ 2012-04-02 14:19 ` Jean-Christophe PLAGNIOL-VILLARD
  2012-04-02 14:19 ` [PATCH 7/8] net: dhcp: allow to set transmitted user class Jean-Christophe PLAGNIOL-VILLARD
  2012-04-02 14:19 ` [PATCH 8/8] net: env: getenv_ip use resolv Jean-Christophe PLAGNIOL-VILLARD
  7 siblings, 0 replies; 13+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2012-04-02 14:19 UTC (permalink / raw)
  To: barebox

For net boot setups it is useful to submit boot params like server or
bootfile over dhcp. To distinguish diffrent type of OS running on the same hardware,
a custom client uuid can be sent in dhcp discover/request messages.

E.g. the ISC dhcp server can be configured with

 | option client-uuid code 97 = { unsigned integer 8, string };
 | class "at91sam9x5ek" {
 |         match if substring (option vendor-class-identifier,0,20) = "barebox-at91sam9x5ek";
 |
 |         filename "/tftpboot/atmel/at91sam9x5/sam9x5ek/zImage";
 |         if substring (option dhcp-client-identifier,0,7) = "ser2net" {
 |                 filename "/tftpboot/atmel/at91sam9x5/sam9x5ek/zImage-ser2net";
 |         }
 |         if substring (option client-uuid,0,7) = "test" {
 |                 filename "/tftpboot/atmel/at91sam9x5/sam9x5ek/zImage-ser2net";
 |         }
 |         option tftp-server-name "192.168.200.98";
 |         option option-150 192.168.200.98;
 |         next-server 192.168.200.98;
 |         option root-path "192.168.200.98:/opt/work/buildroot/build/sam9x5/target";
 | }

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
 net/dhcp.c |   15 ++++++++++++++-
 1 files changed, 14 insertions(+), 1 deletions(-)

diff --git a/net/dhcp.c b/net/dhcp.c
index 5b1ab5a..e452a92 100644
--- a/net/dhcp.c
+++ b/net/dhcp.c
@@ -258,6 +258,7 @@ static int dhcp_set_string_options(struct dhcp_param *param, u8 *e)
 
 #define DHCP_VENDOR_ID		60
 #define DHCP_CLIENT_ID		61
+#define DHCP_CLIENT_UUID	97
 
 struct dhcp_param dhcp_params[] = {
 	{
@@ -268,6 +269,10 @@ struct dhcp_param dhcp_params[] = {
 		.option = DHCP_CLIENT_ID,
 		.handle = dhcp_set_string_options,
 		.barebox_var_name = "dhcp_client_id",
+	}, {
+		.option = DHCP_CLIENT_UUID,
+		.handle = dhcp_set_string_options,
+		.barebox_var_name = "dhcp_client_uuid",
 	}
 };
 
@@ -623,7 +628,7 @@ static int do_dhcp(int argc, char *argv[])
 
 	dhcp_reset_env();
 
-	while((opt = getopt(argc, argv, "v:c:")) > 0) {
+	while((opt = getopt(argc, argv, "v:c:u:")) > 0) {
 		switch(opt) {
 		case 'v':
 			dhcp_set_param_data(DHCP_VENDOR_ID, optarg);
@@ -631,6 +636,9 @@ static int do_dhcp(int argc, char *argv[])
 		case 'c':
 			dhcp_set_param_data(DHCP_CLIENT_ID, optarg);
 			break;
+		case 'u':
+			dhcp_set_param_data(DHCP_CLIENT_UUID, optarg);
+			break;
 		}
 	}
 
@@ -682,6 +690,10 @@ BAREBOX_CMD_HELP_OPT  ("-v <vendor_id>",
 BAREBOX_CMD_HELP_OPT  ("-c <client_id>",
 "DHCP Client ID (code 61) submitted in DHCP requests. It can\n"
 "be used in the DHCP server's configuration to select options\n"
+"(e.g. bootfile or server) which are valid for barebox clients only.\n")
+BAREBOX_CMD_HELP_OPT  ("-u <client_uuid>",
+"DHCP Client UUID (code 97) submitted in DHCP requests. It can\n"
+"be used in the DHCP server's configuration to select options\n"
 "(e.g. bootfile or server) which are valid for barebox clients only.\n");
 BAREBOX_CMD_HELP_END
 
@@ -697,5 +709,6 @@ BAREBOX_MAGICVAR(hostname, "hostname returned from DHCP request");
 BAREBOX_MAGICVAR(domainname, "domainname returned from DHCP request");
 BAREBOX_MAGICVAR(rootpath, "rootpath returned from DHCP request");
 BAREBOX_MAGICVAR(dhcp_vendor_id, "vendor id to send to the DHCP server");
+BAREBOX_MAGICVAR(dhcp_client_uuid, "cliend uuid to send to the DHCP server");
 BAREBOX_MAGICVAR(dhcp_client_id, "cliend id to send to the DHCP server");
 BAREBOX_MAGICVAR(dhcp_tftp_server_name, "TFTP server Name returned from DHCP request");
-- 
1.7.9.1


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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 7/8] net: dhcp: allow to set transmitted user class
  2012-04-02 14:17 [PATCH 0/8 v3] Network update DHCP/BOOTP Jean-Christophe PLAGNIOL-VILLARD
                   ` (5 preceding siblings ...)
  2012-04-02 14:19 ` [PATCH 6/8] net: dhcp: allow to set transmitted client uuid Jean-Christophe PLAGNIOL-VILLARD
@ 2012-04-02 14:19 ` Jean-Christophe PLAGNIOL-VILLARD
  2012-04-02 14:19 ` [PATCH 8/8] net: env: getenv_ip use resolv Jean-Christophe PLAGNIOL-VILLARD
  7 siblings, 0 replies; 13+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2012-04-02 14:19 UTC (permalink / raw)
  To: barebox

For net boot setups it is useful to submit boot params like server or
bootfile over dhcp. To distinguish diffrent type of OS running on the same hardware,
a custom vendor id can be sent in dhcp discover/request messages.

E.g. the ISC dhcp server can be configured with

 | option client-uuid code 97 = { unsigned integer 8, string };
 | class "at91sam9x5ek" {
 |         match if substring (option vendor-class-identifier,0,20) = "barebox-at91sam9x5ek";
 |
 |         filename "/tftpboot/atmel/at91sam9x5/sam9x5ek/zImage";
 |         if substring (option dhcp-client-identifier,0,7) = "ser2net" {
 |                 filename "/tftpboot/atmel/at91sam9x5/sam9x5ek/zImage-ser2net";
 |         }
 |         if substring (option client-uuid,0,7) = "test" {
 |                 filename "/tftpboot/atmel/at91sam9x5/sam9x5ek/zImage-ser2net";
 |         }
 |         if substring (option user-class,0,4) = "toto" {
 |                 filename "/tftpboot/atmel/at91sam9x5/sam9x5ek/zImage-toto";
 |         }
 |         option tftp-server-name "192.168.200.98";
 |         option option-150 192.168.200.98;
 |         next-server 192.168.200.98;
 |         option root-path "192.168.200.98:/opt/work/buildroot/build/sam9x5/target";
 | }

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
 net/dhcp.c |   15 ++++++++++++++-
 1 files changed, 14 insertions(+), 1 deletions(-)

diff --git a/net/dhcp.c b/net/dhcp.c
index e452a92..27a55d0 100644
--- a/net/dhcp.c
+++ b/net/dhcp.c
@@ -258,6 +258,7 @@ static int dhcp_set_string_options(struct dhcp_param *param, u8 *e)
 
 #define DHCP_VENDOR_ID		60
 #define DHCP_CLIENT_ID		61
+#define DHCP_USER_CLASS		77
 #define DHCP_CLIENT_UUID	97
 
 struct dhcp_param dhcp_params[] = {
@@ -270,6 +271,10 @@ struct dhcp_param dhcp_params[] = {
 		.handle = dhcp_set_string_options,
 		.barebox_var_name = "dhcp_client_id",
 	}, {
+		.option = DHCP_USER_CLASS,
+		.handle = dhcp_set_string_options,
+		.barebox_var_name = "dhcp_user_class",
+	}, {
 		.option = DHCP_CLIENT_UUID,
 		.handle = dhcp_set_string_options,
 		.barebox_var_name = "dhcp_client_uuid",
@@ -628,7 +633,7 @@ static int do_dhcp(int argc, char *argv[])
 
 	dhcp_reset_env();
 
-	while((opt = getopt(argc, argv, "v:c:u:")) > 0) {
+	while((opt = getopt(argc, argv, "v:c:u:U:")) > 0) {
 		switch(opt) {
 		case 'v':
 			dhcp_set_param_data(DHCP_VENDOR_ID, optarg);
@@ -639,6 +644,9 @@ static int do_dhcp(int argc, char *argv[])
 		case 'u':
 			dhcp_set_param_data(DHCP_CLIENT_UUID, optarg);
 			break;
+		case 'U':
+			dhcp_set_param_data(DHCP_USER_CLASS, optarg);
+			break;
 		}
 	}
 
@@ -694,6 +702,10 @@ BAREBOX_CMD_HELP_OPT  ("-c <client_id>",
 BAREBOX_CMD_HELP_OPT  ("-u <client_uuid>",
 "DHCP Client UUID (code 97) submitted in DHCP requests. It can\n"
 "be used in the DHCP server's configuration to select options\n"
+"(e.g. bootfile or server) which are valid for barebox clients only.\n")
+BAREBOX_CMD_HELP_OPT  ("-U <user_class>",
+"DHCP User class (code 77) submitted in DHCP requests. It can\n"
+"be used in the DHCP server's configuration to select options\n"
 "(e.g. bootfile or server) which are valid for barebox clients only.\n");
 BAREBOX_CMD_HELP_END
 
@@ -711,4 +723,5 @@ BAREBOX_MAGICVAR(rootpath, "rootpath returned from DHCP request");
 BAREBOX_MAGICVAR(dhcp_vendor_id, "vendor id to send to the DHCP server");
 BAREBOX_MAGICVAR(dhcp_client_uuid, "cliend uuid to send to the DHCP server");
 BAREBOX_MAGICVAR(dhcp_client_id, "cliend id to send to the DHCP server");
+BAREBOX_MAGICVAR(dhcp_user_class, "user class to send to the DHCP server");
 BAREBOX_MAGICVAR(dhcp_tftp_server_name, "TFTP server Name returned from DHCP request");
-- 
1.7.9.1


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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 8/8] net: env: getenv_ip use resolv
  2012-04-02 14:17 [PATCH 0/8 v3] Network update DHCP/BOOTP Jean-Christophe PLAGNIOL-VILLARD
                   ` (6 preceding siblings ...)
  2012-04-02 14:19 ` [PATCH 7/8] net: dhcp: allow to set transmitted user class Jean-Christophe PLAGNIOL-VILLARD
@ 2012-04-02 14:19 ` Jean-Christophe PLAGNIOL-VILLARD
  7 siblings, 0 replies; 13+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2012-04-02 14:19 UTC (permalink / raw)
  To: barebox

Introduce getenv_ip_dns to be able to do not do the resolv when using it in
resolv for the nameserver (Do not loop).

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
 include/net.h |    6 +++++-
 net/dns.c     |    2 +-
 net/net.c     |    9 ++++++---
 3 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/include/net.h b/include/net.h
index 2eec47f..0ebe198 100644
--- a/include/net.h
+++ b/include/net.h
@@ -277,7 +277,11 @@ char *ip_to_string (IPaddr_t x, char *s);
 /* Convert a string to ip address */
 int string_to_ip(const char *s, IPaddr_t *ip);
 
-IPaddr_t getenv_ip(const char *name);
+IPaddr_t getenv_ip_dns(const char *name, int dns);
+static inline IPaddr_t getenv_ip(const char *name)
+{
+	return getenv_ip_dns(name, 0);
+}
 int setenv_ip(const char *name, IPaddr_t ip);
 
 int string_to_ethaddr(const char *str, char *enetaddr);
diff --git a/net/dns.c b/net/dns.c
index 3c7aa5f..fb1178a 100644
--- a/net/dns.c
+++ b/net/dns.c
@@ -205,7 +205,7 @@ IPaddr_t resolv(char *host)
 
 	dns_state = STATE_INIT;
 
-	ip = getenv_ip("nameserver");
+	ip = getenv_ip_dns("nameserver", 0);
 	if (!ip)
 		return 0;
 
diff --git a/net/net.c b/net/net.c
index 2752884..39db75e 100644
--- a/net/net.c
+++ b/net/net.c
@@ -127,7 +127,7 @@ int string_to_ip(const char *s, IPaddr_t *ip)
 	return 0;
 }
 
-IPaddr_t getenv_ip(const char *name)
+IPaddr_t getenv_ip_dns(const char *name, int dns)
 {
 	IPaddr_t ip;
 	const char *var = getenv(name);
@@ -135,10 +135,13 @@ IPaddr_t getenv_ip(const char *name)
 	if (!var)
 		return 0;
 
-	if (string_to_ip(var, &ip))
+	if (!string_to_ip(var, &ip))
+		return ip;
+
+	if (!dns)
 		return 0;
 
-	return ip;
+	return resolv((char*)var);
 }
 
 int setenv_ip(const char *name, IPaddr_t ip)
-- 
1.7.9.1


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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/8] net: dhcp: factorise option recption handling
  2012-04-02 14:19 ` [PATCH 1/8] net: dhcp: factorise option recption handling Jean-Christophe PLAGNIOL-VILLARD
@ 2012-04-02 19:02   ` Sascha Hauer
  2012-04-03  4:55     ` Jean-Christophe PLAGNIOL-VILLARD
  2012-04-03  4:57     ` [PATCH 1/8 v4] " Jean-Christophe PLAGNIOL-VILLARD
  0 siblings, 2 replies; 13+ messages in thread
From: Sascha Hauer @ 2012-04-02 19:02 UTC (permalink / raw)
  To: Jean-Christophe PLAGNIOL-VILLARD; +Cc: barebox

On Mon, Apr 02, 2012 at 04:19:03PM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> Instead of using a static switch case the handle the received option
> use an array of supported option.
> 
> This will allow to unset the env var without duplicating the code.
> 
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>

Much better, thanks.

> +struct dhcp_opt {
> +	unsigned char option;
> +	const char *barebox_var_name;
> +	void (*handle)(struct dhcp_opt *opt, unsigned char *data, int tlen);
> +	void *data;
> +
> +	/* request automatically the option when creating the DHCP request */
> +	bool optinal;

s/optinal/optional/?

You can optimize the size of this struct a bit by putting this bool
after the unsigned char above.

> +
> +	struct bootp *bp;
> +};
> +
> +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);
> +}
> +
> +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;
> +
> +	if (!opt->barebox_var_name) {
> +		pr_err("dhcp: %s: option %d no barebox_var_name set\n",
> +		       __func__, opt->option);
> +		return;
> +	}

I think we don't need this check. It is easy enough to review the code
to not have this bug.

> +
> +	ip = net_read_ip(popt);
> +	setenv_ip(opt->barebox_var_name, ip);
> +}
> +
> +static void env_str_handle(struct dhcp_opt *opt, unsigned char *popt, int optlen)
> +{
> +	char str[256];
> +
> +	if (!opt->barebox_var_name) {
> +		pr_err("dhcp: %s: option %d no barebox_var_name set\n",
> +		       __func__, opt->option);
> +		return;
> +	}

ditto

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/8] net: dhcp: factorise option recption handling
  2012-04-02 19:02   ` Sascha Hauer
@ 2012-04-03  4:55     ` Jean-Christophe PLAGNIOL-VILLARD
  2012-04-03  6:57       ` Sascha Hauer
  2012-04-03  4:57     ` [PATCH 1/8 v4] " Jean-Christophe PLAGNIOL-VILLARD
  1 sibling, 1 reply; 13+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2012-04-03  4:55 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox

On 21:02 Mon 02 Apr     , Sascha Hauer wrote:
> On Mon, Apr 02, 2012 at 04:19:03PM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > Instead of using a static switch case the handle the received option
> > use an array of supported option.
> > 
> > This will allow to unset the env var without duplicating the code.
> > 
> > Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> 
> Much better, thanks.
> 
> > +struct dhcp_opt {
> > +	unsigned char option;
> > +	const char *barebox_var_name;
> > +	void (*handle)(struct dhcp_opt *opt, unsigned char *data, int tlen);
> > +	void *data;
> > +
> > +	/* request automatically the option when creating the DHCP request */
> > +	bool optinal;
> 
> s/optinal/optional/?
> 
> You can optimize the size of this struct a bit by putting this bool
> after the unsigned char above.
branch updated with it

Best Regards,
J.
> 
> > +
> > +	struct bootp *bp;
> > +};
> > +
> > +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);
> > +}
> > +
> > +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;
> > +
> > +	if (!opt->barebox_var_name) {
> > +		pr_err("dhcp: %s: option %d no barebox_var_name set\n",
> > +		       __func__, opt->option);
> > +		return;
> > +	}
> 
> I think we don't need this check. It is easy enough to review the code
> to not have this bug.
> 
> > +
> > +	ip = net_read_ip(popt);
> > +	setenv_ip(opt->barebox_var_name, ip);
> > +}
> > +
> > +static void env_str_handle(struct dhcp_opt *opt, unsigned char *popt, int optlen)
> > +{
> > +	char str[256];
> > +
> > +	if (!opt->barebox_var_name) {
> > +		pr_err("dhcp: %s: option %d no barebox_var_name set\n",
> > +		       __func__, opt->option);
> > +		return;
> > +	}
> 
> ditto
> 
> 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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/8 v4] net: dhcp: factorise option recption handling
  2012-04-02 19:02   ` Sascha Hauer
  2012-04-03  4:55     ` Jean-Christophe PLAGNIOL-VILLARD
@ 2012-04-03  4:57     ` Jean-Christophe PLAGNIOL-VILLARD
  1 sibling, 0 replies; 13+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2012-04-03  4:57 UTC (permalink / raw)
  To: barebox

Instead of using a static switch case the handle the received option
use an array of supported option.

This will allow to unset the env var without duplicating the code.

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
 net/dhcp.c |  241 ++++++++++++++++++++++++++++++++++++++----------------------
 1 files changed, 153 insertions(+), 88 deletions(-)

diff --git a/net/dhcp.c b/net/dhcp.c
index 53eed6c..750d879 100644
--- a/net/dhcp.c
+++ b/net/dhcp.c
@@ -76,6 +76,124 @@ static uint32_t dhcp_leasetime;
 static IPaddr_t net_dhcp_server_ip;
 static uint64_t dhcp_start;
 
+struct dhcp_opt {
+	unsigned char option;
+	/* request automatically the option when creating the DHCP request */
+	bool optional;
+	const char *barebox_var_name;
+	void (*handle)(struct dhcp_opt *opt, unsigned char *data, int tlen);
+	void *data;
+
+	struct bootp *bp;
+};
+
+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);
+}
+
+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);
+	setenv_ip(opt->barebox_var_name, ip);
+}
+
+static void env_str_handle(struct dhcp_opt *opt, unsigned char *popt, int optlen)
+{
+	char str[256];
+
+	memcpy(str, popt, optlen);
+	str[optlen] = 0;
+	setenv(opt->barebox_var_name, str);
+}
+
+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);
+}
+
+struct dhcp_opt dhcp_options[] = {
+	{
+		.option = 1,
+		.handle = netmask_handle,
+	}, {
+		.option = 3,
+		.handle = gateway_handle,
+	}, {
+		.option = 6,
+		.handle = env_ip_handle,
+		.barebox_var_name = "nameserver",
+	}, {
+		.option = 12,
+		.handle = env_str_handle,
+		.barebox_var_name = "hostname",
+	}, {
+		.option = 15,
+		.handle = env_str_handle,
+		.barebox_var_name = "domainname",
+	}, {
+		.option = 17,
+		.handle = env_str_handle,
+		.barebox_var_name = "rootpath",
+	}, {
+		.option = 51,
+		.handle = copy_uint32_handle,
+		.data = &dhcp_leasetime,
+	}, {
+		.option = 54,
+		.handle = copy_ip_handle,
+		.data = &net_dhcp_server_ip,
+		.optional = true,
+	}, {
+		.option = 67,
+		.handle = bootfile_vendorex_handle,
+		.barebox_var_name = "bootfile",
+	},
+};
+
 static int bootp_check_packet(unsigned char *pkt, unsigned src, unsigned len)
 {
 	struct bootp *bp = (struct bootp *) pkt;
@@ -131,6 +249,7 @@ static void bootp_copy_net_params(struct bootp *bp)
 static int dhcp_extended (u8 *e, int message_type, IPaddr_t ServerID,
 			  IPaddr_t RequestedIP, char *vendor_id)
 {
+	int i;
 	u8 *start = e;
 	u8 *cnt;
 	int vendor_id_len = vendor_id ? strlen(vendor_id) : 0;
@@ -181,18 +300,13 @@ static int dhcp_extended (u8 *e, int message_type, IPaddr_t ServerID,
 	*e++ = 55;		/* Parameter Request List */
 	 cnt = e++;		/* Pointer to count of requested items */
 	*cnt = 0;
-	*e++  = 1;		/* Subnet Mask */
-	*cnt += 1;
-	*e++  = 3;		/* Router Option */
-	*cnt += 1;
-	*e++  = 6;		/* DNS Server(s) */
-	*cnt += 1;
-	*e++  = 12;		/* Hostname */
-	*cnt += 1;
-	*e++  = 15;		/* domain name */
-	*cnt += 1;
-	*e++  = 17;		/* Boot path */
-	*cnt += 1;
+
+	for (i = 0; i < ARRAY_SIZE(dhcp_options); i++) {
+		if (dhcp_options[i].optional)
+			continue;
+		*e++  = dhcp_options[i].option;
+		*cnt += 1;
+	}
 	*e++  = 255;		/* End of the list */
 
 	/* Pad to minimal length */
@@ -246,89 +360,40 @@ static int bootp_request(void)
 	return ret;
 }
 
+static int dhcp_options_handle(unsigned char option, unsigned char *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;
+			opt->handle(opt, popt, optlen);
+			goto end;
+		}
+	}
+
+end:
+	return i;
+}
+
 static void dhcp_options_process(unsigned char *popt, struct bootp *bp)
 {
 	unsigned char *end = popt + sizeof(*bp) + OPT_SIZE;
 	int oplen;
-	IPaddr_t ip;
-	char str[256];
+	unsigned char option;
+	int i;
 
 	while (popt < end && *popt != 0xff) {
 		oplen = *(popt + 1);
-		switch (*popt) {
-		case 1:
-			ip = net_read_ip(popt + 2);
-			net_set_netmask(ip);
-			break;
-		case 3:
-			ip = net_read_ip(popt + 2);
-			net_set_gateway(ip);
-			break;
-		case 6:
-			ip = net_read_ip(popt + 2);
-			setenv_ip("nameserver", ip);
-			break;
-		case 12:
-			memcpy(str, popt + 2, oplen);
-			str[oplen] = 0;
-			setenv("hostname", str);
-			break;
-		case 15:
-			memcpy(str, popt + 2, oplen);
-			str[oplen] = 0;
-			setenv("domainname", str);
-			break;
-		case 17:
-			memcpy(str, popt + 2, oplen);
-			str[oplen] = 0;
-			setenv("rootpath", str);
-			break;
-		case 51:
-			net_copy_uint32 (&dhcp_leasetime, (uint32_t *)(popt + 2));
-			break;
-		case 53:	/* Ignore Message Type Option */
-			break;
-		case 54:
-			net_copy_ip(&net_dhcp_server_ip, (popt + 2));
-			break;
-		case 58:	/* Ignore Renewal Time Option */
-			break;
-		case 59:	/* Ignore Rebinding Time Option */
-			break;
-		case 66:	/* Ignore TFTP server name */
-			break;
-		case 67:	/* vendor opt bootfile */
-			/*
-			 * 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
-			 */
-			memcpy(str, popt + 2, oplen);
-			str[oplen] = 0;
-			if (bp->bp_file[0] == '\0') {
-				/*
-				 * 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
-				 */
-				printf("*** WARNING: using vendor "
-					"optional boot file\n");
-					setenv("bootfile", str);
-			}
-			break;
-		default:
-#ifdef CONFIG_BOOTP_VENDOREX
-			if (dhcp_vendorex_proc (popt))
-				break;
-#endif
-			debug("*** Unhandled DHCP Option in OFFER/ACK: %d\n", *popt);
-			break;
-		}
+		option = *popt;
+
+		i = dhcp_options_handle(option, popt + 2, oplen, bp);
+		if (i == ARRAY_SIZE(dhcp_options))
+			debug("*** Unhandled DHCP Option in OFFER/ACK: %d\n", option);
+
 		popt += oplen + 2;	/* Process next option */
 	}
 }
-- 
1.7.9.1


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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/8] net: dhcp: factorise option recption handling
  2012-04-03  4:55     ` Jean-Christophe PLAGNIOL-VILLARD
@ 2012-04-03  6:57       ` Sascha Hauer
  0 siblings, 0 replies; 13+ messages in thread
From: Sascha Hauer @ 2012-04-03  6:57 UTC (permalink / raw)
  To: Jean-Christophe PLAGNIOL-VILLARD; +Cc: barebox

On Tue, Apr 03, 2012 at 06:55:54AM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 21:02 Mon 02 Apr     , Sascha Hauer wrote:
> > On Mon, Apr 02, 2012 at 04:19:03PM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > > Instead of using a static switch case the handle the received option
> > > use an array of supported option.
> > > 
> > > This will allow to unset the env var without duplicating the code.
> > > 
> > > Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> > 
> > Much better, thanks.
> > 
> > > +struct dhcp_opt {
> > > +	unsigned char option;
> > > +	const char *barebox_var_name;
> > > +	void (*handle)(struct dhcp_opt *opt, unsigned char *data, int tlen);
> > > +	void *data;
> > > +
> > > +	/* request automatically the option when creating the DHCP request */
> > > +	bool optinal;
> > 
> > s/optinal/optional/?
> > 
> > You can optimize the size of this struct a bit by putting this bool
> > after the unsigned char above.
> branch updated with it

Pulled, thanks

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2012-04-03  6:58 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-04-02 14:17 [PATCH 0/8 v3] Network update DHCP/BOOTP Jean-Christophe PLAGNIOL-VILLARD
2012-04-02 14:19 ` [PATCH 1/8] net: dhcp: factorise option recption handling Jean-Christophe PLAGNIOL-VILLARD
2012-04-02 19:02   ` Sascha Hauer
2012-04-03  4:55     ` Jean-Christophe PLAGNIOL-VILLARD
2012-04-03  6:57       ` Sascha Hauer
2012-04-03  4:57     ` [PATCH 1/8 v4] " Jean-Christophe PLAGNIOL-VILLARD
2012-04-02 14:19 ` [PATCH 2/8] net: dhcp: reset env variable before do a dhcp request Jean-Christophe PLAGNIOL-VILLARD
2012-04-02 14:19 ` [PATCH 3/8] net: dhcp: add support of tftp name server Jean-Christophe PLAGNIOL-VILLARD
2012-04-02 14:19 ` [PATCH 4/8] net: dhcp: factorise setting option code Jean-Christophe PLAGNIOL-VILLARD
2012-04-02 14:19 ` [PATCH 5/8] net: dhcp: allow to set transmitted client id Jean-Christophe PLAGNIOL-VILLARD
2012-04-02 14:19 ` [PATCH 6/8] net: dhcp: allow to set transmitted client uuid Jean-Christophe PLAGNIOL-VILLARD
2012-04-02 14:19 ` [PATCH 7/8] net: dhcp: allow to set transmitted user class Jean-Christophe PLAGNIOL-VILLARD
2012-04-02 14:19 ` [PATCH 8/8] net: env: getenv_ip use resolv Jean-Christophe PLAGNIOL-VILLARD

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox