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