Patchwork [U-Boot,1/3] Add support for Net Boot Controller (NBC) packet

login
register
mail settings
Submitter tristan.lelong@blunderer.org
Date Nov. 16, 2010, 6:04 p.m.
Message ID <732b4e4395e5c02bb01c1fd7b7f8ce085a208950.1289929762.git.blunderer@blunderer.org>
Download mbox | patch
Permalink /patch/71534/
State Rejected
Headers show

Comments

tristan.lelong@blunderer.org - Nov. 16, 2010, 6:04 p.m.
From: Tristan Lelong <tristan.lelong@blunderer.org>

This adds the core NBC feature:
 * Integration into u-boot
 * NBC packet processing
 * NetConsole reconfigure

Signed-off-by: Tristan Lelong <tristan.lelong@blunderer.org>
---
 common/main.c |    5 ++
 include/net.h |    6 ++-
 net/Makefile  |    2 +
 net/nbc.c     |  143 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 net/nbc.h     |   39 +++++++++++++++
 net/net.c     |   17 +++++++
 6 files changed, 211 insertions(+), 1 deletions(-)
 create mode 100644 net/nbc.c
 create mode 100644 net/nbc.h
Mike Frysinger - Nov. 17, 2010, 10:54 a.m.
On Tuesday, November 16, 2010 13:04:31 tristan.lelong@blunderer.org wrote:
> --- a/common/main.c
> +++ b/common/main.c
> 
> +#ifdef CONFIG_CMD_NBC

why did you pick "CONFIG_CMD_NBC" ?  there is no "nbc" command that is run 
from the u-boot console, so "CONFIG_CMD_xxx" makes no sense.

> +	if (!abort)
> +		abort = NBCWait ();

incorrect style here and in many places -- no space before the paren.  i wont 
bother pointing out this in other places ... find & fix yourself.

> --- a/net/Makefile
> +++ b/net/Makefile
> @@ -35,6 +35,8 @@ COBJS-$(CONFIG_CMD_NFS)  += nfs.o
>  COBJS-$(CONFIG_CMD_RARP) += rarp.o
>  COBJS-$(CONFIG_CMD_SNTP) += sntp.o
>  COBJS-$(CONFIG_CMD_NET)  += tftp.o
> +COBJS-$(CONFIG_CMD_NBC)	 += nbc.o
> +
> 

do not add useless blank lines

> --- /dev/null
> +++ b/net/nbc.c
> @@ -0,0 +1,143 @@
> +/*
> + * NBC support driver
> + * Network Boot Controller
> + *
> + * Copyright (c) 2010 Tristan Lelong <tristan.lelong@blunderer.org>
> + *
> + */

licensing info is missing

> +static void NBCHandler (uchar * pkt, unsigned dest, unsigned src, unsigned
> len) +{

style problems here -- no space after that asterisk

> +	int cnt = 0;
> +	char src_ip[16] = "";
> +	char *nbcsource = NULL;
> +	char ip[16] = "";
> +	char mac[18] = "";
> +	char hostname[255] = "";
> +
> +	struct _nbc_packet *nbc_pkt = (struct _nbc_packet *)pkt;
> +	nbcsource = getenv ("nbcsource");
> +
> +	/* check packet validity */
> +	if (nbc_pkt->id != 0xD3)

magic numbers should be defines in headers, not inlined in source files

> +		goto fail;
> +	if (strncmp (nbc_pkt->head, "NBC", 3))
> +		goto fail;
> +	if (nbc_pkt->size != len)
> +		goto fail;
> +
> +	sprintf (src_ip, "%lu.%lu.%lu.%lu", ((NetSrcIP >> 0) & 0xFF),
> +		 ((NetSrcIP >> 8) & 0xFF), ((NetSrcIP >> 16) & 0xFF),
> +		 ((NetSrcIP >> 24) & 0xFF));

we have printf modifiers to handle ip addresses already.  use that instead.

> +			unsigned char *pkt_mac = (unsigned char *)chk->data;
> +			sprintf (mac, "%02X:%02X:%02X:%02X:%02X:%02X",
> +				 pkt_mac[0], pkt_mac[1], pkt_mac[2], pkt_mac[3],
> +				 pkt_mac[4], pkt_mac[5]);

there are printf modifiers for MAC addresse too.  although i dont think you 
need to go screwing with this in the first place.  there are eth helpers for 
manipulating MAC addresses you should be using instead.  see 
doc/README.enetaddr.

> +	if (NBC_Mode_On) {
> +		return 1;
> +	}

useless braces

> --- /dev/null
> +++ b/net/nbc.h
> @@ -0,0 +1,39 @@
> +#define NBC_SERVICE_PORT 	4446
> +#define NBC_TIMEOUT      	2000
> +
> +#define NBC_CHK_HEADER_SIZE	5
> +#define NBC_HEADER_SIZE		5
> +#define NBC_CHK_IP_SIZE		4
> +#define NBC_CHK_MAC_SIZE	6
> +#define NBC_CHK_HOSTNAME_SIZE	255

indentation here is all screwd up.  it's inconsistent and mixes spaces & tabs.

> --- a/net/net.c
> +++ b/net/net.c
> @@ -1631,6 +1643,7 @@ NetReceive(volatile uchar * inpkt, int len)
>  			ushort *sumptr;
>  			ushort  sumlen;
> 
> +
>  			xsum  = ip->ip_p;
>  			xsum += (ntohs(ip->udp_len));
>  			xsum += (ntohl(ip->ip_src) >> 16) & 0x0000ffff;

please clean your patches of useless whitespace noise before submitting
-mike
Wolfgang Denk - Nov. 17, 2010, 1:28 p.m.
Dear tristan.lelong@blunderer.org,

In message <732b4e4395e5c02bb01c1fd7b7f8ce085a208950.1289929762.git.blunderer@blunderer.org> you wrote:
> From: Tristan Lelong <tristan.lelong@blunderer.org>
> 
> This adds the core NBC feature:
>  * Integration into u-boot
>  * NBC packet processing
>  * NetConsole reconfigure


Like Stefano I wonder if this is protocol is ion any way standardized?
For example, I cannot find a coresponding RFC for it.

And why is it needed?  DHCP should work fine, doesn't it?

Best regards,

Wolfgang Denk

Patch

diff --git a/common/main.c b/common/main.c
index d97ccd7..e23b204 100644
--- a/common/main.c
+++ b/common/main.c
@@ -259,6 +259,11 @@  static __inline__ int abortboot(int bootdelay)
 
 	putc('\n');
 
+#ifdef CONFIG_CMD_NBC
+	if (!abort)
+		abort = NBCWait ();
+#endif
+
 #ifdef CONFIG_SILENT_CONSOLE
 	if (abort)
 		gd->flags &= ~GD_FLG_SILENT;
diff --git a/include/net.h b/include/net.h
index a29dafc..25e177f 100644
--- a/include/net.h
+++ b/include/net.h
@@ -359,7 +359,7 @@  extern int		NetState;		/* Network loop state		*/
 extern int		NetRestartWrap;		/* Tried all network devices	*/
 #endif
 
-typedef enum { BOOTP, RARP, ARP, TFTP, DHCP, PING, DNS, NFS, CDP, NETCONS, SNTP } proto_t;
+typedef enum { BOOTP, RARP, ARP, TFTP, DHCP, PING, DNS, NFS, CDP, NETCONS, SNTP, NBC } proto_t;
 
 /* from net/net.c */
 extern char	BootFile[128];			/* Boot File name		*/
@@ -384,6 +384,10 @@  extern IPaddr_t	NetNtpServerIP;			/* the ip address to NTP	*/
 extern int NetTimeOffset;			/* offset time from UTC		*/
 #endif
 
+#if defined(CONFIG_CMD_NBC)
+extern IPaddr_t	NetSrcIP;			/* Packet Src IP addr (0 = unknown)	*/
+#endif
+
 /* Initialize the network adapter */
 extern int	NetLoop(proto_t);
 
diff --git a/net/Makefile b/net/Makefile
index 216d1ec..735e8c6 100644
--- a/net/Makefile
+++ b/net/Makefile
@@ -35,6 +35,8 @@  COBJS-$(CONFIG_CMD_NFS)  += nfs.o
 COBJS-$(CONFIG_CMD_RARP) += rarp.o
 COBJS-$(CONFIG_CMD_SNTP) += sntp.o
 COBJS-$(CONFIG_CMD_NET)  += tftp.o
+COBJS-$(CONFIG_CMD_NBC)	 += nbc.o
+
 
 COBJS	:= $(COBJS-y)
 SRCS	:= $(COBJS:.o=.c)
diff --git a/net/nbc.c b/net/nbc.c
new file mode 100644
index 0000000..e1788e4
--- /dev/null
+++ b/net/nbc.c
@@ -0,0 +1,143 @@ 
+/*
+ * NBC support driver
+ * Network Boot Controller
+ *
+ * Copyright (c) 2010 Tristan Lelong <tristan.lelong@blunderer.org>
+ *
+ */
+
+#include <common.h>
+#include <command.h>
+#include <net.h>
+
+#include "nbc.h"
+
+#ifndef CONFIG_NETCONSOLE
+#error CONFIG_NETCONSOLE should be defined so that nbc can redirect stdin/stdout/stderr
+#endif
+
+static char NBC_Mode_On = 0;
+
+static void NBCHandler (uchar * pkt, unsigned dest, unsigned src, unsigned len)
+{
+	int cnt = 0;
+	char src_ip[16] = "";
+	char *nbcsource = NULL;
+	char ip[16] = "";
+	char mac[18] = "";
+	char hostname[255] = "";
+
+	struct _nbc_packet *nbc_pkt = (struct _nbc_packet *)pkt;
+	nbcsource = getenv ("nbcsource");
+
+	/* check packet validity */
+	if (nbc_pkt->id != 0xD3)
+		goto fail;
+	if (strncmp (nbc_pkt->head, "NBC", 3))
+		goto fail;
+	if (nbc_pkt->size != len)
+		goto fail;
+
+	sprintf (src_ip, "%lu.%lu.%lu.%lu", ((NetSrcIP >> 0) & 0xFF),
+		 ((NetSrcIP >> 8) & 0xFF), ((NetSrcIP >> 16) & 0xFF),
+		 ((NetSrcIP >> 24) & 0xFF));
+
+	if (nbcsource && strcmp (nbcsource, src_ip))
+		goto fail;
+
+	for (cnt = 0; cnt < (nbc_pkt->size - NBC_HEADER_SIZE);) {
+		struct _nbc_chunk *chk =
+		    (struct _nbc_chunk *)(pkt + NBC_HEADER_SIZE + cnt);
+		if (strncmp (chk->name, "IP\0\0", 4) == 0) {
+			if (chk->size != NBC_CHK_IP_SIZE)
+				goto fail;
+			unsigned char *pkt_ip = (unsigned char *)chk->data;
+			sprintf (ip, "%d.%d.%d.%d", pkt_ip[0], pkt_ip[1],
+				 pkt_ip[2], pkt_ip[3]);
+		} else if (strncmp (chk->name, "MAC\0", 4) == 0) {
+			if (chk->size != NBC_CHK_MAC_SIZE)
+				goto fail;
+			unsigned char *pkt_mac = (unsigned char *)chk->data;
+			sprintf (mac, "%02X:%02X:%02X:%02X:%02X:%02X",
+				 pkt_mac[0], pkt_mac[1], pkt_mac[2], pkt_mac[3],
+				 pkt_mac[4], pkt_mac[5]);
+		} else if (strncmp (chk->name, "HOST", 4) == 0) {
+			unsigned char *pkt_hostname =
+			    (unsigned char *)chk->data;
+			if (chk->size >= NBC_CHK_HOSTNAME_SIZE)
+				goto fail;
+			strncpy (hostname, pkt_hostname, chk->size);
+			hostname[chk->size] = '\0';
+		}
+		cnt += NBC_CHK_HEADER_SIZE + chk->size;
+	}
+
+	/* test if we are targeted by the magic packet */
+	if (strlen (mac)) {
+		char *our_mac = getenv ("ethaddr");
+		debug ("mac filter: cmp(%s,%s)\n", our_mac, mac);
+		if (strcmp (mac, our_mac))
+			goto fail;
+	}
+
+	if (strlen (hostname)) {
+		char *our_hostname = getenv ("hostname");
+		debug ("hostname filter: cmp(%s,%s)\n", our_hostname, hostname);
+		if (strcmp (hostname, our_hostname))
+			goto fail;
+	}
+
+	/* set server ip */
+	setenv ("serverip", src_ip);
+
+	/* set gateway ip */
+	setenv ("gatewayip", src_ip);
+
+	/* set addrip */
+	setenv ("ipaddr", ip);
+
+	printf ("activated nc from [%s] to [%s]\n", ip, src_ip);
+
+	/* activate the netconsole */
+	setenv ("ncip", src_ip);
+	setenv ("stdin", "nc");
+	setenv ("stdout", "nc");
+	setenv ("stderr", "nc");
+	console_assign (stdin, "nc");
+	console_assign (stdout, "nc");
+	console_assign (stderr, "nc");
+
+	NBC_Mode_On = 1;
+
+	NetState = NETLOOP_SUCCESS;
+
+	return;
+fail:
+	debug ("not a valid nbc magic packet\n");
+}
+
+static void NBCTimeout (void)
+{
+	NetState = NETLOOP_FAIL;
+	debug ("NBC Timeout!\n");
+}
+
+int NBCWait (void)
+{
+	char *nbcinhibit = getenv ("nbcinhibit");
+	if (nbcinhibit)
+		return 0;
+
+	NetLoop (NBC);
+
+	if (NBC_Mode_On) {
+		return 1;
+	}
+	return 0;
+}
+
+void NBCStart (void)
+{
+	NetSetTimeout (1000, NBCTimeout);
+	NetSetHandler (NBCHandler);
+}
diff --git a/net/nbc.h b/net/nbc.h
new file mode 100644
index 0000000..40371a5
--- /dev/null
+++ b/net/nbc.h
@@ -0,0 +1,39 @@ 
+/*
+ * (C) Tristan Lelong <tristan.lelong@blunderer.org> 2010
+ *  Copyright 2010, Tristan Lelong <tristan.lelong@blunderer.org>
+ *
+ * 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, or (at
+ * your option) any later version.
+ */
+
+#ifndef __NBC_H__
+#define __NBC_H__
+
+#define NBC_SERVICE_PORT 	4446
+#define NBC_TIMEOUT      	2000
+
+#define NBC_CHK_HEADER_SIZE	5
+#define NBC_HEADER_SIZE		5
+#define NBC_CHK_IP_SIZE		4
+#define NBC_CHK_MAC_SIZE	6
+#define NBC_CHK_HOSTNAME_SIZE	255
+
+struct _nbc_packet {
+	unsigned char id;
+	unsigned char head[3];
+	unsigned char size;
+	unsigned char data[255];
+};
+
+struct _nbc_chunk {
+	unsigned char name[4];
+	unsigned char size;
+	unsigned char data[255];
+};
+
+extern void NBCStart (void);
+extern int NBCWait (void);
+
+#endif
diff --git a/net/net.c b/net/net.c
index d5a5429..b722048 100644
--- a/net/net.c
+++ b/net/net.c
@@ -97,6 +97,9 @@ 
 #if defined(CONFIG_CMD_DNS)
 #include "dns.h"
 #endif
+#if defined(CONFIG_CMD_NBC)
+#include "nbc.h"
+#endif
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -180,6 +183,10 @@  IPaddr_t	NetNtpServerIP;		/* NTP server IP address		*/
 int		NetTimeOffset=0;	/* offset time from UTC			*/
 #endif
 
+#if defined(CONFIG_CMD_NBC)
+IPaddr_t	NetSrcIP;		/* Src IP addr (0 = unknown)		*/
+#endif
+
 #ifdef CONFIG_NETCONSOLE
 void NcStart(void);
 int nc_input_packet(uchar *pkt, unsigned dest, unsigned src, unsigned len);
@@ -384,6 +391,11 @@  restart:
 		NetDevExists = 1;
 #endif
 		switch (protocol) {
+#if defined(CONFIG_CMD_NBC)
+		case NBC:
+			NBCStart ();
+			break;
+#endif
 		case TFTP:
 			/* always use ARP to get server ethernet address */
 			TftpStart();
@@ -1631,6 +1643,7 @@  NetReceive(volatile uchar * inpkt, int len)
 			ushort *sumptr;
 			ushort  sumlen;
 
+
 			xsum  = ip->ip_p;
 			xsum += (ntohs(ip->udp_len));
 			xsum += (ntohl(ip->ip_src) >> 16) & 0x0000ffff;
@@ -1673,6 +1686,10 @@  NetReceive(volatile uchar * inpkt, int len)
 						ntohs(ip->udp_src),
 						ntohs(ip->udp_len) - 8);
 #endif
+
+#ifdef CONFIG_CMD_NBC
+		NetSrcIP = NetReadIP ((void *)&ip->ip_src);
+#endif
 		/*
 		 *	IP header OK.  Pass the packet to the current handler.
 		 */