> -----Original Message-----
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of ciara.loftus at 
> intel.com
> Sent: Thursday, December 18, 2014 2:55 PM
> To: dev at dpdk.org
> Cc: Anthony Fee
> Subject: [dpdk-dev] [PATCH] vhost: add interface name to virtio-net struct
> 
> From: Ciara Loftus <ciara.loftus at intel.com>
> 
> This patch fixes the issue whereby when using userspace vhost ports
> in the context of vSwitching, the name provided to the hypervisor/QEMU
> of the vhost tap device needs to be exposed in the library, in order
> for the vSwitch to be able to direct packets to the correct device.
> This patch introduces an 'ifname' member to the virtio-net structure
> which is populated with the tap device name when QEMU is brought up
> with a vhost device.
> 
> Signed-off-by: Ciara Loftus <ciara.loftus at intel.com>
> Signed-off-by: Anthony Fee <anthonyx.fee at intel.com>
> Acked-by: Huawei Xie <huawei.xie at intel.com>
> ---
>  lib/librte_vhost/rte_virtio_net.h |    1 +
>  lib/librte_vhost/virtio-net.c     |   41 
> ++++++++++++++++++++++++++++++++++++-
>  2 files changed, 41 insertions(+), 1 deletions(-)
> 
> diff --git a/lib/librte_vhost/rte_virtio_net.h 
> b/lib/librte_vhost/rte_virtio_net.h
> index 00b1328..aebb4b5 100644
> --- a/lib/librte_vhost/rte_virtio_net.h
> +++ b/lib/librte_vhost/rte_virtio_net.h
> @@ -96,6 +96,7 @@ struct virtio_net {
>       uint64_t                features;       /**< Negotiated feature set. */
>       uint64_t                device_fh;      /**< device identifier. */
>       uint32_t                flags;          /**< Device flags. Only used to 
> check if device is running on data core. */
> +     char                    ifname[32];     /** Name of the tap device **/
>       void                    *priv;          /**< private context */
>  } __rte_cache_aligned;
> 
> diff --git a/lib/librte_vhost/virtio-net.c b/lib/librte_vhost/virtio-net.c
> index 852b6d1..7f90ecf 100644
> --- a/lib/librte_vhost/virtio-net.c
> +++ b/lib/librte_vhost/virtio-net.c
> @@ -43,6 +43,10 @@
>  #include <sys/mman.h>
>  #include <unistd.h>
> 
> +#include <sys/socket.h>
> +#include <linux/if_tun.h>
> +#include <linux/if.h>
> +
>  #include <rte_ethdev.h>
>  #include <rte_log.h>
>  #include <rte_string_fns.h>
> @@ -1000,6 +1004,39 @@ set_vring_kick(struct vhost_device_ctx ctx, struct 
> vhost_vring_file *file)
>  }
> 
>  /*
> + * Function to get the tap device name from the provided file descriptor and
> + * save it in the device structure.
> + */
> +static int
> +get_ifname(struct virtio_net *dev, int tap_fd, int pid)
> +{
> +     struct eventfd_copy fd_tap;
> +     struct ifreq ifr;
> +     uint32_t size;
> +
> +     fd_tap.source_fd = eventfd(0, EFD_NONBLOCK | EFD_CLOEXEC);
> +     fd_tap.target_fd = tap_fd;
> +     fd_tap.target_pid = pid;
> +
> +     if (eventfd_copy(dev, &fd_tap))
> +             return -1;
> +
> +     ioctl(fd_tap.source_fd, TUNGETIFF, &ifr);

Shouldn't we check that ioctl() returns with success here,
and if it fails, don't copy stuff over?

> +
> +     if (close(fd_tap.source_fd) < 0)
> +             RTE_LOG(ERR, VHOST_CONFIG,
> +                     "(%"PRIu64") fd close failed\n",
> +                     dev->device_fh);
> +
> +     size = strnlen(ifr.ifr_name, sizeof(ifr.ifr_name)) > 
> sizeof(dev->ifname)?
> +                     sizeof(dev->ifname): strnlen(ifr.ifr_name, 
> sizeof(ifr.ifr_name));
> +

So if sizeof(ifr.ifr_name) > sizeof(dev->ifname) then we can endup with 
dev->ifname not being null-termianted?
Another nit: there is no need to call strnlen() twice.

Konstantin

> +     strncpy(dev->ifname, ifr.ifr_name, size);
> +
> +     return 0;
> +}
> +
> +/*
>   * Called from CUSE IOCTL: VHOST_NET_SET_BACKEND
>   * To complete device initialisation when the virtio driver is loaded,
>   * we are provided with a valid fd for a tap device (not used by us).
> @@ -1026,8 +1063,10 @@ set_backend(struct vhost_device_ctx ctx, struct 
> vhost_vring_file *file)
>        */
>       if (!(dev->flags & VIRTIO_DEV_RUNNING)) {
>               if (((int)dev->virtqueue[VIRTIO_TXQ]->backend != 
> VIRTIO_DEV_STOPPED) &&
> -                     ((int)dev->virtqueue[VIRTIO_RXQ]->backend != 
> VIRTIO_DEV_STOPPED))
> +                     ((int)dev->virtqueue[VIRTIO_RXQ]->backend != 
> VIRTIO_DEV_STOPPED)) {
> +                     get_ifname(dev, file->fd, ctx.pid);
>                       return notify_ops->new_device(dev);
> +             }
>       /* Otherwise we remove it. */
>       } else
>               if (file->fd == VIRTIO_DEV_STOPPED)
> --
> 1.7.4.1

Reply via email to