diff mbox

[U-Boot,RFC] sandbox: Add tap based networking

Message ID 1322575743-21760-1-git-send-email-weisserm@arcor.de
State Superseded
Delegated to: Mike Frysinger
Headers show

Commit Message

Matthias Weisser Nov. 29, 2011, 2:09 p.m. UTC
This patch adds a network driver for sandbox using tap.

Signed-off-by: Matthias Weisser <weisserm@arcor.de>
---
This patch adds support for networking to sandbox architecture using tap. A tap
device "tap0" has to be created e.g. using openvpn

$ openvpn --mktun --dev tap0

u-boot should then be able to detect the network device on startup. To test the
network related commands I used the following commands to create an ethernet
bridge to my local network connection

$ brctl addbr br0
$ ifconfig eth0 0.0.0.0 promisc
$ ifconfig tap0 0.0.0.0
$ brctl addif br0 eth0
$ brctl addif br0 tap0
$ ifconfig br0 up
$ pump -i br0

After this setup I was able to successfully execute the ping and tftp command.

To get timeouts working this patch also needs my timer patch for sandbox
http://patchwork.ozlabs.org/patch/128279/

Note:
As sandbox is build using the native compiler, which is in my case x86_64, ulong
is 64 bit in size. This caused non-working IP stuff as IPaddr_t is typedefed to
ulong. I changed this typedef to u32 which helped but is quite invasive to all
network related stuff. This is the main reason why this patched is marked as
RFC.

 arch/sandbox/cpu/cpu.c    |    9 +++
 arch/sandbox/cpu/os.c     |   39 ++++++++++++++
 arch/sandbox/lib/board.c  |    5 ++
 drivers/net/Makefile      |    1 +
 drivers/net/tap.c         |  125 +++++++++++++++++++++++++++++++++++++++++++++
 include/configs/sandbox.h |    9 ++--
 include/net.h             |    2 +-
 include/os.h              |    8 +++
 8 files changed, 193 insertions(+), 5 deletions(-)
 create mode 100644 drivers/net/tap.c

Comments

Mike Frysinger Nov. 29, 2011, 3:24 p.m. UTC | #1
On Tuesday 29 November 2011 09:09:03 Matthias Weisser wrote:
> This patch adds support for networking to sandbox architecture using tap. A
> tap device "tap0" has to be created e.g. using openvpn
> ....

this info should be in the changelog as it's useful

> As sandbox is build using the native compiler, which is in my case x86_64,
> ulong is 64 bit in size. This caused non-working IP stuff as IPaddr_t is
> typedefed to ulong. I changed this typedef to u32 which helped but is
> quite invasive to all network related stuff. This is the main reason why
> this patched is marked as RFC.

could you elaborate on "non-working IP stuff" ?

> --- a/arch/sandbox/cpu/cpu.c
> +++ b/arch/sandbox/cpu/cpu.c
>
> +#if defined(CONFIG_DRIVER_TAP)

i think Wolfgang NAK-ed the idea of using "DRIVER" in the config name.  so 
let's use something like CONFIG_NET_TAP.

> --- a/arch/sandbox/cpu/os.c
> +++ b/arch/sandbox/cpu/os.c
> 
> +#include <sys/ioctl.h>
> +#include <sys/socket.h>
> +#include <linux/if.h>
> +#include <linux/if_tun.h>

these should be behind the new CONFIG name, as should the func itself

> +int os_tap_open(char *dev)

const char *dev

> +	if (*dev)
> +		strncpy(ifr.ifr_name, dev, IFNAMSIZ);

let's just make it required

> --- /dev/null
> +++ b/drivers/net/tap.c
>
> +static int tap_send(struct eth_device *dev, void *packet, int length)
> ...
> +	os_write(priv->fd, (void *)packet, length);

packet is already void*, so no need to cast

> +static int tap_recv(struct eth_device *dev)
> +{
> +	struct tap_priv *priv = dev->priv;
> +	uchar buf[PKTSIZE];

there are already common network buffers for you to use: NetRxPackets[0]

> +	int length;
> +
> +	length = os_read(priv->fd, buf, PKTSIZE);

os_read returns ssize_t, not int

> +static int tap_set_hwaddr(struct eth_device *dev)
> +{
> +	/* Nothing to be done here */
> +	return 0;
> +}

isn't there an ioctl that lets you control this ?

> +int tap_initialize(bd_t *bd)
> ...
> +	struct eth_device *edev;

this should be named "dev" to stay consistent

> +	edev = (struct eth_device *)malloc(sizeof(struct eth_device));
> +	tap = (struct tap_priv *)malloc(sizeof(struct tap_priv));

no need for the casts

> +	if (!edev) {
> +		puts("tap: not enough malloc memory for eth_device\n");
> +		res = -ENOMEM;
> +		goto err1;
> +	}
> +
> +	if (!tap) {
> +		puts("tap: not enough malloc memory for tap_priv\n");
> +		res = -ENOMEM;
> +		goto err2;
> +	}

drop the puts(), and return 0, not -ENOMEM

> +	strncpy(tap->dev, "tap0", sizeof(tap->dev));

no reason we couldn't support more than one tap, is there ?  make the device 
name an argument to tap_initialize(), and then the board caller can do:
	tap_initialize("tap0");
	tap_initialize("tap1");
	tap_initialize("fooiefoofoo");

> +	tap->fd = os_tap_open(tap->dev);
> +	if (tap->fd < 0) {
> +		res = -EIO;
> +		goto err3;
> +	}

this should return -1

> +	sprintf(edev->name, "TAP");

use the same name as tap->dev.  in fact, i'd just chuck tap->dev and only use 
dev->name.
-mike
Matthias Weisser Nov. 29, 2011, 6:21 p.m. UTC | #2
Am 29.11.2011 16:24, schrieb Mike Frysinger:
> On Tuesday 29 November 2011 09:09:03 Matthias Weisser wrote:
>> This patch adds support for networking to sandbox architecture using tap. A
>> tap device "tap0" has to be created e.g. using openvpn
>> ....
> 
> this info should be in the changelog as it's useful

Will do

>> As sandbox is build using the native compiler, which is in my case x86_64,
>> ulong is 64 bit in size. This caused non-working IP stuff as IPaddr_t is
>> typedefed to ulong. I changed this typedef to u32 which helped but is
>> quite invasive to all network related stuff. This is the main reason why
>> this patched is marked as RFC.
> 
> could you elaborate on "non-working IP stuff" ?

Nothing worked. e.g. ARP wasn't working due to the memcpy in function
NetReadIP in net.h which uses sizeof(IPaddr_t) to copy the IP address
out of the network packet. sizeof(IPaddr_t) yields 8 on x86_64. After
changing the typedef to a type having 32 bits everything worked. So this
is one of the zillion problems fixed which we will see if u-boot is
ported to real 64 bit hardware :-)

>> --- a/arch/sandbox/cpu/cpu.c
>> +++ b/arch/sandbox/cpu/cpu.c
>>
>> +#if defined(CONFIG_DRIVER_TAP)
> 
> i think Wolfgang NAK-ed the idea of using "DRIVER" in the config name.  so 
> let's use something like CONFIG_NET_TAP.

Done.

>> --- a/arch/sandbox/cpu/os.c
>> +++ b/arch/sandbox/cpu/os.c
>>
>> +#include <sys/ioctl.h>
>> +#include <sys/socket.h>
>> +#include <linux/if.h>
>> +#include <linux/if_tun.h>
> 
> these should be behind the new CONFIG name, as should the func itself

Done

>> +int os_tap_open(char *dev)
> 
> const char *dev
> 
>> +	if (*dev)
>> +		strncpy(ifr.ifr_name, dev, IFNAMSIZ);
> 
> let's just make it required

Done

>> --- /dev/null
>> +++ b/drivers/net/tap.c
>>
>> +static int tap_send(struct eth_device *dev, void *packet, int length)
>> ...
>> +	os_write(priv->fd, (void *)packet, length);
> 
> packet is already void*, so no need to cast

packet of tap_send need to be volatile. So added volatile there and let
the cast there.

>> +static int tap_recv(struct eth_device *dev)
>> +{
>> +	struct tap_priv *priv = dev->priv;
>> +	uchar buf[PKTSIZE];
> 
> there are already common network buffers for you to use: NetRxPackets[0]

Will use them

>> +	int length;
>> +
>> +	length = os_read(priv->fd, buf, PKTSIZE);
> 
> os_read returns ssize_t, not int

NetReceive needs an int later on. I added an explicit cast.

>> +static int tap_set_hwaddr(struct eth_device *dev)
>> +{
>> +	/* Nothing to be done here */
>> +	return 0;
>> +}
> 
> isn't there an ioctl that lets you control this ?

Sure. But if I read the the docs correct it is an privileged operation
and I don't think we wan't to run u-boot as super user all the time. How
is the situation handled on real hardware when the MAC is programmed to
an EEPROM on the NIC. Can the MAC be read from the NIC and set to
u-boot? This would be the best solution as linux should take care about
MAC address assignment.

>> +int tap_initialize(bd_t *bd)
>> ...
>> +	struct eth_device *edev;
> 
> this should be named "dev" to stay consistent

Done.

>> +	edev = (struct eth_device *)malloc(sizeof(struct eth_device));
>> +	tap = (struct tap_priv *)malloc(sizeof(struct tap_priv));
> 
> no need for the casts

Right

>> +	if (!edev) {
>> +		puts("tap: not enough malloc memory for eth_device\n");
>> +		res = -ENOMEM;
>> +		goto err1;
>> +	}
>> +
>> +	if (!tap) {
>> +		puts("tap: not enough malloc memory for tap_priv\n");
>> +		res = -ENOMEM;
>> +		goto err2;
>> +	}
> 
> drop the puts(), and return 0, not -ENOMEM

I don't understand why, but done. We are in a PC environment where the
puts shouldn't hurt in code size and may help in debugging.

>> +	strncpy(tap->dev, "tap0", sizeof(tap->dev));
> 
> no reason we couldn't support more than one tap, is there ?  make the device 
> name an argument to tap_initialize(), and then the board caller can do:
> 	tap_initialize("tap0");
> 	tap_initialize("tap1");
> 	tap_initialize("fooiefoofoo");

Done.

>> +	tap->fd = os_tap_open(tap->dev);
>> +	if (tap->fd < 0) {
>> +		res = -EIO;
>> +		goto err3;
>> +	}
> 
> this should return -1

Done.

>> +	sprintf(edev->name, "TAP");
> 
> use the same name as tap->dev.  in fact, i'd just chuck tap->dev and only use 
> dev->name.

Done

Thanks for the review. Will wait for some additional comments and send a
new version then.
Mike Frysinger Nov. 29, 2011, 6:39 p.m. UTC | #3
On Tuesday 29 November 2011 13:21:38 Matthias Weisser wrote:
> Am 29.11.2011 16:24, schrieb Mike Frysinger:
> > On Tuesday 29 November 2011 09:09:03 Matthias Weisser wrote:
> >> As sandbox is build using the native compiler, which is in my case
> >> x86_64, ulong is 64 bit in size. This caused non-working IP stuff as
> >> IPaddr_t is typedefed to ulong. I changed this typedef to u32 which
> >> helped but is quite invasive to all network related stuff. This is the
> >> main reason why this patched is marked as RFC.
> > 
> > could you elaborate on "non-working IP stuff" ?
> 
> Nothing worked. e.g. ARP wasn't working due to the memcpy in function
> NetReadIP in net.h which uses sizeof(IPaddr_t) to copy the IP address
> out of the network packet. sizeof(IPaddr_t) yields 8 on x86_64. After
> changing the typedef to a type having 32 bits everything worked. So this
> is one of the zillion problems fixed which we will see if u-boot is
> ported to real 64 bit hardware :-)

ok, so IPaddr_t is holding the exact contents of the IP address and gets 
read/written directly to the network packets.  and being an IPv4 address, it 
needs to be 32bits exactly.

please split that one line change out into a dedicated patch.

> >> +static int tap_set_hwaddr(struct eth_device *dev)
> >> +{
> >> +	/* Nothing to be done here */
> >> +	return 0;
> >> +}
> > 
> > isn't there an ioctl that lets you control this ?
> 
> Sure. But if I read the the docs correct it is an privileged operation
> and I don't think we wan't to run u-boot as super user all the time. How
> is the situation handled on real hardware when the MAC is programmed to
> an EEPROM on the NIC. Can the MAC be read from the NIC and set to
> u-boot? This would be the best solution as linux should take care about
> MAC address assignment.

the tap_initialize() func should read the current MAC address assigned to the 
tap device and write that to dev->enetaddr

then when tap_set_hwaddr() gets called, if the MAC is different, it will 
attempt to set the MAC to what the user requested.  if they don't have 
permission, then the code can yell at them.  but if they do, this should work 
imo.  this gets us the best of all worlds i think.

> >> +	if (!edev) {
> >> +		puts("tap: not enough malloc memory for eth_device\n");
> >> +		res = -ENOMEM;
> >> +		goto err1;
> >> +	}
> >> +
> >> +	if (!tap) {
> >> +		puts("tap: not enough malloc memory for tap_priv\n");
> >> +		res = -ENOMEM;
> >> +		goto err2;
> >> +	}
> > 
> > drop the puts(), and return 0, not -ENOMEM
> 
> I don't understand why, but done. We are in a PC environment where the
> puts shouldn't hurt in code size and may help in debugging.

while true, i'm thinking in terms of standard u-boot network driver style here 
rather than "let's be as helpful as possible to the user".  i'd still prefer 
we drop the puts(), but i understand what you mean ...

also, please keep replies on list :)
-mike
Matthias Weisser Dec. 3, 2011, 3:06 p.m. UTC | #4
Am 29.11.2011 19:39, schrieb Mike Frysinger:
>>>> +static int tap_set_hwaddr(struct eth_device *dev)
>>>> > >> +{
>>>> > >> +	/* Nothing to be done here */
>>>> > >> +	return 0;
>>>> > >> +}
>>> > > 
>>> > > isn't there an ioctl that lets you control this ?
>> > 
>> > Sure. But if I read the the docs correct it is an privileged operation
>> > and I don't think we wan't to run u-boot as super user all the time. How
>> > is the situation handled on real hardware when the MAC is programmed to
>> > an EEPROM on the NIC. Can the MAC be read from the NIC and set to
>> > u-boot? This would be the best solution as linux should take care about
>> > MAC address assignment.
> the tap_initialize() func should read the current MAC address assigned to the 
> tap device and write that to dev->enetaddr

Done.

> then when tap_set_hwaddr() gets called, if the MAC is different, it will 
> attempt to set the MAC to what the user requested.  if they don't have 
> permission, then the code can yell at them.  but if they do, this should work 
> imo.  this gets us the best of all worlds i think.

I looked into that. It seems that you have to shut down the interface,
change the MAC and up the interface again. I tried now to do this using
some ioctls but didn't got it working as user or as root. So I give up
at this point. Should I submit the V2 patch without setting MAC function
anyway or will it not be applied then?

If someone has an idea: Here is the (simplified) code to set the MAC of
a tap interface.

void os_tap_set_hw_addr(int fd, unsigned char *hwaddr)
{
	struct ifreq ifr;
	
	memset(&ifr, 0, sizeof(ifr));
	strncpy(ifr.ifr_name, "tap0", IFNAMSIZ);
	
	/* Get the interface flags */
	if (ioctl(fd, SIOCGIFFLAGS, &ifr) < 0) {
		perror("Could not get interface flags");
	}
	/* Shut down the interface */
	ifr.ifr_flags &= ~(IFF_UP);
	if (ioctl(fd, SIOCSIFFLAGS, &ifr) < 0) {
		perror("Could not down the interface");
	}

	/* Set the new hw address */
	ifr.ifr_hwaddr.sa_family = ARPHRD_ETHER;
        memcpy(&ifr.ifr_hwaddr.sa_data, hwaddr, ETH_ALEN);

        if (ioctl(fd, SIOCSIFHWADDR, &ifr) < 0) {
                perror("ioctl(SIOCSIFHWADDR)");
        }

        /* Get the interface flags */
	if (ioctl(fd, SIOCGIFFLAGS, &ifr) < 0) {
		perror("Could not get interface flags");
	}
	/* Shut down the interface */
	ifr.ifr_flags |= IFF_UP;
	if (ioctl(fd, SIOCSIFFLAGS, &ifr) < 0) {
		perror("Could not up the interface");
	}
}

The SIOC{G,S}IFFLAGS ioctl calls fail with "Invalid argument" and the
SIOCSIFHWADDR fails with "Device or resource busy".
Mike Frysinger Dec. 3, 2011, 5:02 p.m. UTC | #5
On Saturday 03 December 2011 10:06:58 Matthias Weisser wrote:
> Am 29.11.2011 19:39, schrieb Mike Frysinger:
> > then when tap_set_hwaddr() gets called, if the MAC is different, it will
> > attempt to set the MAC to what the user requested.  if they don't have
> > permission, then the code can yell at them.  but if they do, this should
> > work imo.  this gets us the best of all worlds i think.
> 
> I looked into that. It seems that you have to shut down the interface,
> change the MAC and up the interface again. I tried now to do this using
> some ioctls but didn't got it working as user or as root. So I give up
> at this point. Should I submit the V2 patch without setting MAC function
> anyway or will it not be applied then?

add a comment to the set_hwaddr that you tried XXX but couldn't get it to 
work, so for now it's disabled until someone can post a working solution
-mike
diff mbox

Patch

diff --git a/arch/sandbox/cpu/cpu.c b/arch/sandbox/cpu/cpu.c
index d7684d3..53e58bd 100644
--- a/arch/sandbox/cpu/cpu.c
+++ b/arch/sandbox/cpu/cpu.c
@@ -60,3 +60,12 @@  void *map_physmem(phys_addr_t paddr, unsigned long len, unsigned long flags)
 void flush_dcache_range(unsigned long start, unsigned long stop)
 {
 }
+
+int cpu_eth_init(bd_t *bis)
+{
+#if defined(CONFIG_DRIVER_TAP)
+	return tap_initialize(bis);
+#else
+	return 0;
+#endif
+}
diff --git a/arch/sandbox/cpu/os.c b/arch/sandbox/cpu/os.c
index 6d55b5c..9985b33 100644
--- a/arch/sandbox/cpu/os.c
+++ b/arch/sandbox/cpu/os.c
@@ -21,14 +21,20 @@ 
 
 #include <fcntl.h>
 #include <stdlib.h>
+#include <stdio.h>
 #include <termios.h>
 #include <unistd.h>
+#include <string.h>
 #include <time.h>
 #include <errno.h>
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <sys/mman.h>
+#include <sys/ioctl.h>
+#include <sys/socket.h>
 #include <linux/types.h>
+#include <linux/if.h>
+#include <linux/if_tun.h>
 
 #include <os.h>
 
@@ -121,3 +127,36 @@  u64 os_get_nsec(void)
 	return tv.tv_sec * 1000000000ULL + tv.tv_usec * 1000;
 #endif
 }
+
+int os_tap_open(char *dev)
+{
+	struct ifreq ifr;
+	int fd, err;
+
+	fd = open("/dev/net/tun", O_RDWR | O_NONBLOCK);
+	if (fd < 0) {
+		perror("could not open /dev/net/tun");
+		return -1;
+	}
+
+	memset(&ifr, 0, sizeof(ifr));
+
+	/* Flags: IFF_TUN   - TUN device (no Ethernet headers)
+	 *        IFF_TAP   - TAP device
+	 *
+	 *        IFF_NO_PI - Do not provide packet information
+	 */
+	ifr.ifr_flags = IFF_TAP | IFF_NO_PI;
+	if (*dev)
+		strncpy(ifr.ifr_name, dev, IFNAMSIZ);
+
+	err = ioctl(fd, TUNSETIFF, (void *)&ifr);
+	if (err < 0) {
+		perror("could not get tap device");
+		close(fd);
+		return err;
+	}
+
+	strcpy(dev, ifr.ifr_name);
+	return fd;
+}
diff --git a/arch/sandbox/lib/board.c b/arch/sandbox/lib/board.c
index b7997e9..d02f6ca 100644
--- a/arch/sandbox/lib/board.c
+++ b/arch/sandbox/lib/board.c
@@ -257,6 +257,11 @@  void board_init_r(gd_t *id, ulong dest_addr)
 	board_late_init();
 #endif
 
+#if defined(CONFIG_CMD_NET)
+	puts("Net:   ");
+	eth_initialize(gd->bd);
+#endif
+
 #ifdef CONFIG_POST
 	post_run(NULL, POST_RAM | post_bootmode_get(0));
 #endif
diff --git a/drivers/net/Makefile b/drivers/net/Makefile
index d3df82e..1686405 100644
--- a/drivers/net/Makefile
+++ b/drivers/net/Makefile
@@ -69,6 +69,7 @@  COBJS-$(CONFIG_SH_ETHER) += sh_eth.o
 COBJS-$(CONFIG_SMC91111) += smc91111.o
 COBJS-$(CONFIG_SMC911X) += smc911x.o
 COBJS-$(CONFIG_DRIVER_TI_EMAC) += davinci_emac.o
+COBJS-$(CONFIG_DRIVER_TAP) += tap.o
 COBJS-$(CONFIG_TSEC_ENET) += tsec.o fsl_mdio.o
 COBJS-$(CONFIG_FMAN_ENET) += fsl_mdio.o
 COBJS-$(CONFIG_TSI108_ETH) += tsi108_eth.o
diff --git a/drivers/net/tap.c b/drivers/net/tap.c
new file mode 100644
index 0000000..e6689bd
--- /dev/null
+++ b/drivers/net/tap.c
@@ -0,0 +1,125 @@ 
+/*
+ * tap.c - A tap ethernet driver for u-boot
+ *
+ * Copyright (c) 2011 Matthias Weißer <weisserm@arcor.de>
+ *
+ * Based on fec_mxc.c and tap.c from barebox:
+ * Copyright (c) 2007 Sascha Hauer <s.hauer@pengutronix.de>, Pengutronix
+ *
+ * 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 version 2
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#include <common.h>
+#include <malloc.h>
+#include <errno.h>
+#include <net.h>
+#include <os.h>
+
+struct tap_priv {
+	int fd; /* file descriptor */
+	char dev[128];
+};
+
+static int tap_init(struct eth_device *dev, bd_t* bd)
+{
+	/* Nothing to be done here */
+	return 0;
+}
+
+static void tap_halt(struct eth_device *dev)
+{
+	/* Nothing to be done here */
+}
+
+static int tap_send(struct eth_device *dev, void *packet, int length)
+{
+	struct tap_priv *priv = dev->priv;
+
+	os_write(priv->fd, (void *)packet, length);
+	return 0;
+}
+
+static int tap_recv(struct eth_device *dev)
+{
+	struct tap_priv *priv = dev->priv;
+	uchar buf[PKTSIZE];
+	int length;
+
+	length = os_read(priv->fd, buf, PKTSIZE);
+
+	if (length > 0)
+		NetReceive(buf, length);
+
+	return 0;
+}
+
+static int tap_set_hwaddr(struct eth_device *dev)
+{
+	/* Nothing to be done here */
+	return 0;
+}
+
+int tap_initialize(bd_t *bd)
+{
+	struct eth_device *edev;
+	struct tap_priv *tap;
+	int res = 0;
+
+	/* create and fill edev struct */
+	edev = (struct eth_device *)malloc(sizeof(struct eth_device));
+	if (!edev) {
+		puts("tap: not enough malloc memory for eth_device\n");
+		res = -ENOMEM;
+		goto err1;
+	}
+
+	tap = (struct tap_priv *)malloc(sizeof(struct tap_priv));
+	if (!tap) {
+		puts("tap: not enough malloc memory for tap_priv\n");
+		res = -ENOMEM;
+		goto err2;
+	}
+
+	memset(edev, 0, sizeof(*edev));
+	memset(tap, 0, sizeof(*tap));
+
+	strncpy(tap->dev, "tap0", sizeof(tap->dev));
+	tap->fd = os_tap_open(tap->dev);
+	if (tap->fd < 0) {
+		res = -EIO;
+		goto err3;
+	}
+
+	edev->priv = tap;
+	edev->init = tap_init;
+	edev->send = tap_send;
+	edev->recv = tap_recv;
+	edev->halt = tap_halt;
+	edev->write_hwaddr = tap_set_hwaddr;
+
+	sprintf(edev->name, "TAP");
+	eth_register(edev);
+
+	return res;
+
+err3:
+	free(tap);
+err2:
+	free(edev);
+err1:
+	return res;
+}
diff --git a/include/configs/sandbox.h b/include/configs/sandbox.h
index 10565e6..1ca27e5 100644
--- a/include/configs/sandbox.h
+++ b/include/configs/sandbox.h
@@ -69,12 +69,13 @@ 
 
 #define CONFIG_SYS_NO_FLASH
 
+/* Network support */
+#define CONFIG_DRIVER_TAP
+
 /* include default commands */
 #include <config_cmd_default.h>
-
-/* We don't have networking support yet */
-#undef CONFIG_CMD_NET
-#undef CONFIG_CMD_NFS
+#define CONFIG_CMD_DHCP
+#define CONFIG_CMD_PING
 
 #define CONFIG_BOOTARGS ""
 
diff --git a/include/net.h b/include/net.h
index ad9afbf..9d878e5 100644
--- a/include/net.h
+++ b/include/net.h
@@ -33,7 +33,7 @@ 
 
 #define PKTALIGN	32
 
-typedef ulong		IPaddr_t;
+typedef u32		IPaddr_t;
 
 
 /**
diff --git a/include/os.h b/include/os.h
index f3af4f0..ba76419 100644
--- a/include/os.h
+++ b/include/os.h
@@ -98,3 +98,11 @@  void os_usleep(unsigned long usec);
  * \return A monotonic increasing time scaled in nano seconds
  */
 u64 os_get_nsec(void);
+
+/**
+ * Opens a TAP device for network emulation
+ *
+ * \param dev Device name to be opened
+ * \return File descriptor of the device
+ */
+int os_tap_open(char *dev);