[U-Boot] [PATCH] sandbox: Add tap based networking

Simon Glass sjg at chromium.org
Wed Dec 14 00:54:59 CET 2011


Hi Matthias,

On Sun, Dec 4, 2011 at 9:57 AM, Matthias Weisser <weisserm at arcor.de> wrote:
> 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 the following commands can be used
> to create an ethernet bridge to 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
>
> Signed-off-by: Matthias Weisser <weisserm at arcor.de>

Unfortunately I was unable to test this easily and then ran out of
time. But it looks good, with a few comments.

> ---
> This patch also needs these patches to work
> http://patchwork.ozlabs.org/patch/128279/
> http://patchwork.ozlabs.org/patch/129084/
>
> Changes from RFC:
>  Multiple tap interfaces possible now
>  Minor cleanups due to comments from Mike Frysinger
>
>  arch/sandbox/cpu/cpu.c    |    8 +++
>  arch/sandbox/cpu/os.c     |  101 ++++++++++++++++++++++++++++++++++++++
>  arch/sandbox/lib/board.c  |    5 ++
>  drivers/net/Makefile      |    1 +
>  drivers/net/tap.c         |  118 +++++++++++++++++++++++++++++++++++++++++++++
>  include/configs/sandbox.h |    9 ++--
>  include/netdev.h          |    1 +
>  include/os.h              |   18 +++++++
>  8 files changed, 257 insertions(+), 4 deletions(-)
>  create mode 100644 drivers/net/tap.c
>
> diff --git a/arch/sandbox/cpu/cpu.c b/arch/sandbox/cpu/cpu.c
> index d7684d3..2fb3bae 100644
> --- a/arch/sandbox/cpu/cpu.c
> +++ b/arch/sandbox/cpu/cpu.c
> @@ -21,6 +21,7 @@
>
>  #include <common.h>
>  #include <os.h>
> +#include <netdev.h>
>
>  DECLARE_GLOBAL_DATA_PTR;
>
> @@ -60,3 +61,10 @@ void *map_physmem(phys_addr_t paddr, unsigned long len, unsigned long flags)
>  void flush_dcache_range(unsigned long start, unsigned long stop)
>  {
>  }
> +
> +#ifdef CONFIG_NET_TAP
> +int cpu_eth_init(bd_t *bis)
> +{
> +       return tap_initialize(bis, "tap0");
> +}
> +#endif
> diff --git a/arch/sandbox/cpu/os.c b/arch/sandbox/cpu/os.c
> index 6d55b5c..0c8cc82 100644
> --- a/arch/sandbox/cpu/os.c
> +++ b/arch/sandbox/cpu/os.c
> @@ -19,6 +19,7 @@
>  * MA 02111-1307 USA
>  */
>
> +#include <config.h>
>  #include <fcntl.h>
>  #include <stdlib.h>
>  #include <termios.h>
> @@ -30,6 +31,19 @@
>  #include <sys/mman.h>
>  #include <linux/types.h>
>
> +#ifdef CONFIG_NET_TAP

I don't think we need this #ifdef

> +#include <string.h>
> +#include <stdio.h>
> +#include <sys/ioctl.h>
> +#include <sys/socket.h>
> +#include <linux/socket.h>
> +#include <linux/if.h>
> +#include <linux/if_tun.h>
> +#include <linux/if_arp.h>
> +#include <linux/if_ether.h>
> +
> +#endif
> +
>  #include <os.h>
>
>  /* Operating System Interface */
> @@ -121,3 +135,90 @@ u64 os_get_nsec(void)
>        return tv.tv_sec * 1000000000ULL + tv.tv_usec * 1000;
>  #endif
>  }
> +
> +#ifdef CONFIG_NET_TAP

Is there a reason for not always enabling this function?

> +int os_tap_open(const char *dev, unsigned char *hwaddr)
> +{
> +       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)

Comment style:

/*
 * Flags:

> +        *        IFF_TAP   - TAP device
> +        *
> +        *        IFF_NO_PI - Do not provide packet information
> +        */
> +       ifr.ifr_flags = IFF_TAP | IFF_NO_PI;
> +       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;
> +       }
> +
> +       if (ioctl(fd, SIOCGIFHWADDR, (void *)&ifr) >= 0)
> +               memcpy(hwaddr, &ifr.ifr_hwaddr.sa_data, ETH_ALEN);
> +
> +       return fd;
> +}
> +
> +int os_tap_set_hw_addr(int fd, unsigned char *hwaddr)
> +{
> +       /*
> +        * The following code should be able to change the MAC of a TAP
> +        * interface but doesn't work on my box. So leave it disabled
> +        * here as a reference if someone is going to pick that up in
> +        * the future.
> +        */
> +#if 0
> +       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");
> +               return -1;
> +       }
> +       /* Shut down the interface */
> +       ifr.ifr_flags &= ~(IFF_UP);
> +       if (ioctl(fd, SIOCSIFFLAGS, &ifr) < 0) {
> +               perror("Could not down the interface");
> +               return -1;
> +       }
> +
> +       /* 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)");
> +               return -1;
> +       }
> +
> +       /* Get the interface flags */
> +       if (ioctl(fd, SIOCGIFFLAGS, &ifr) < 0) {
> +               perror("Could not get interface flags");
> +               return -1;
> +       }
> +       /* Shut down the interface */
> +       ifr.ifr_flags |= IFF_UP;
> +       if (ioctl(fd, SIOCSIFFLAGS, &ifr) < 0) {
> +               perror("Could not up the interface");
> +               return -1;
> +       }
> +       return 0;
> +#endif
> +       return -1;
> +}
> +#endif
> 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..914b1b7 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_NET_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..c02bb1b
> --- /dev/null
> +++ b/drivers/net/tap.c
> @@ -0,0 +1,118 @@
> +/*
> + * tap.c - A tap ethernet driver for u-boot
> + *
> + * Copyright (c) 2011 Matthias Weißer <weisserm at arcor.de>
> + *
> + * Based on fec_mxc.c and tap.c from barebox:
> + * Copyright (c) 2007 Sascha Hauer <s.hauer at 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 */
> +};
> +
> +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, volatile void *packet, int length)
> +{
> +       struct tap_priv *tap = dev->priv;
> +
> +       os_write(tap->fd, (void *)packet, length);

check for os_write() failure?

> +       return 0;
> +}
> +
> +static int tap_recv(struct eth_device *dev)
> +{
> +       struct tap_priv *tap = dev->priv;
> +       int length;

Use ssize_t here?

> +
> +       length = (int)os_read(tap->fd, (void *)NetRxPackets[0], PKTSIZE);
> +
> +       if (length > 0)
> +               NetReceive(NetRxPackets[0], length);
> +
> +       return 0;
> +}
> +
> +static int tap_set_hwaddr(struct eth_device *dev)
> +{
> +       struct tap_priv *tap = dev->priv;
> +
> +       return os_tap_set_hw_addr(tap->fd, dev->enetaddr);
> +}
> +
> +int tap_initialize(bd_t *bd, char * dname)
> +{
> +       struct eth_device *dev;
> +       struct tap_priv *tap;
> +       int res = 0;

Perhaps set this to -1 until you get to the success path? Otherwise
you see success with err1 and err2 below.

> +
> +       /* create and fill edev struct */
> +       dev = malloc(sizeof(struct eth_device));
> +       if (!dev)
> +               goto err1;
> +
> +       tap = malloc(sizeof(struct tap_priv));
> +       if (!tap)
> +               goto err2;
> +
> +       memset(dev, 0, sizeof(*dev));
> +       memset(tap, 0, sizeof(*tap));
> +
> +       strncpy(dev->name, dname, sizeof(dev->name));
> +       tap->fd = os_tap_open(dev->name, dev->enetaddr);
> +       if (tap->fd < 0) {
> +               res = -1;
> +               goto err3;
> +       }
> +
> +       dev->priv = tap;
> +       dev->init = tap_init;
> +       dev->send = tap_send;
> +       dev->recv = tap_recv;
> +       dev->halt = tap_halt;
> +       dev->write_hwaddr = tap_set_hwaddr;
> +
> +       strncpy(dev->name, dname, sizeof(dev->name));
> +       eth_register(dev);
> +
> +       return res;
> +
> +err3:
> +       free(tap);
> +err2:
> +       free(dev);
> +err1:
> +       return res;
> +}
> diff --git a/include/configs/sandbox.h b/include/configs/sandbox.h
> index 10565e6..33b1a19 100644
> --- a/include/configs/sandbox.h
> +++ b/include/configs/sandbox.h
> @@ -69,12 +69,13 @@
>
>  #define CONFIG_SYS_NO_FLASH
>
> +/* Network support */
> +#define CONFIG_NET_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/netdev.h b/include/netdev.h
> index 04d9f75..25173a8 100644
> --- a/include/netdev.h
> +++ b/include/netdev.h
> @@ -100,6 +100,7 @@ int xilinx_axiemac_initialize(bd_t *bis, unsigned long base_addr,
>                                                        unsigned long dma_addr);
>  int xilinx_emaclite_initialize(bd_t *bis, unsigned long base_addr,
>                                                        int txpp, int rxpp);
> +int tap_initialize(bd_t *bd, char * dname);

Can we please have arg/return comments on this new function?

>
>  /* Boards with PCI network controllers can call this from their board_eth_init()
>  * function to initialize whatever's on board.
> diff --git a/include/os.h b/include/os.h
> index f3af4f0..3b0c629 100644
> --- a/include/os.h
> +++ b/include/os.h
> @@ -98,3 +98,21 @@ 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
> + * \param hwaddr Ethernet address of interface is copied here (6 byte)
> + * \return File descriptor of the device

While I'm fine with \ I think U-Boot tends to use @.

> + */
> +int os_tap_open(const char *dev, unsigned char *hwaddr);
> +
> +/**
> + * Sets the hw address of a open tap interface
> + *
> + * \param fd The fd of the tap interface
> + * \param hwaddr The address to be set (6 byte)
> + * \return -1 if the hwaddr couldn't be set

or 0 on success?

> + */
> +int os_tap_set_hw_addr(int fd, unsigned char *hwaddr);
> --
> 1.7.5.4
>

Regards,
Simon


More information about the U-Boot mailing list