From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mike Frysinger Date: Tue, 29 Nov 2011 10:24:57 -0500 Subject: [U-Boot] [PATCH RFC] sandbox: Add tap based networking In-Reply-To: <1322575743-21760-1-git-send-email-weisserm@arcor.de> References: <1322575743-21760-1-git-send-email-weisserm@arcor.de> Message-ID: <201111291024.58943.vapier@gentoo.org> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 > +#include > +#include > +#include 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 -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 836 bytes Desc: This is a digitally signed message part. URL: