From: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>
To: "Loftus, Ciara" <ciara.loftus@intel.com>, "dev@dpdk.org" <dev@dpdk.org>
Cc: Anthony Fee <anthonyx.fee@intel.com>
Subject: Re: [dpdk-dev] [PATCH] vhost: add interface name to virtio-net struct
Date: Thu, 18 Dec 2014 17:03:04 +0000 [thread overview]
Message-ID: <2601191342CEEE43887BDE71AB977258213C1FDC@IRSMSX105.ger.corp.intel.com> (raw)
In-Reply-To: <1418914523-24530-1-git-send-email-ciara.loftus@intel.com>
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of ciara.loftus@intel.com
> Sent: Thursday, December 18, 2014 2:55 PM
> To: dev@dpdk.org
> Cc: Anthony Fee
> Subject: [dpdk-dev] [PATCH] vhost: add interface name to virtio-net struct
>
> From: Ciara Loftus <ciara.loftus@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@intel.com>
> Signed-off-by: Anthony Fee <anthonyx.fee@intel.com>
> Acked-by: Huawei Xie <huawei.xie@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
next prev parent reply other threads:[~2014-12-18 17:05 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-12-18 14:55 ciara.loftus
2014-12-18 15:33 ` Thomas Monjalon
2014-12-18 17:01 ` Loftus, Ciara
2014-12-18 17:16 ` Xie, Huawei
2014-12-18 17:20 ` Czesnowicz, Przemyslaw
2014-12-18 16:20 ` Stephen Hemminger
2014-12-18 16:26 ` Vincent JARDIN
2014-12-18 17:03 ` Ananyev, Konstantin [this message]
2014-12-18 18:11 ` Loftus, Ciara
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2601191342CEEE43887BDE71AB977258213C1FDC@IRSMSX105.ger.corp.intel.com \
--to=konstantin.ananyev@intel.com \
--cc=anthonyx.fee@intel.com \
--cc=ciara.loftus@intel.com \
--cc=dev@dpdk.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).