From: Thomas Monjalon <thomas.monjalon@6wind.com>
To: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH v2] vhost: add interface name to virtio-net struct
Date: Fri, 19 Dec 2014 17:02:19 +0100 [thread overview]
Message-ID: <2876256.xg5KmeNcVB@xps13> (raw)
In-Reply-To: <2601191342CEEE43887BDE71AB977258213C2436@IRSMSX105.ger.corp.intel.com>
Hi Konstantin,
2014-12-19 15:39, Ananyev, Konstantin:
> Hi Ciara,
>
> > -----Original Message-----
> > From: Loftus, Ciara
> > Sent: Thursday, December 18, 2014 6:07 PM
> > To: dev@dpdk.org
> > Cc: thomas.monjalon@6wind.com; stephen@networkplumber.org; vincent.jardin@6wind.com; Ananyev, Konstantin; Czesnowicz,
> > Przemyslaw; Loftus, Ciara; Anthony Fee
> > Subject: [PATCH v2] 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>
[...]
> > /*
> > + * 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, ifr_size;
> > + int ret;
> > +
> > + 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;
> > +
> > + ret = ioctl(fd_tap.source_fd, TUNGETIFF, &ifr);
> > +
> > + if (close(fd_tap.source_fd) < 0)
> > + RTE_LOG(ERR, VHOST_CONFIG,
> > + "(%"PRIu64") fd close failed\n",
> > + dev->device_fh);
> > +
> > + if (ret >= 0) {
> > + ifr_size = strnlen(ifr.ifr_name, sizeof(ifr.ifr_name));
> > + size = ifr_size > sizeof(dev->ifname)?
> > + sizeof(dev->ifname): ifr_size;
>
> If you saying that sizeof(ifr.ifr_name) would always be equal to sizeof(dev->ifname),
> then why are you doing the comparison above?
> "ifr_size <= sizeof(dev->ifname)" would always be true, right?
> From other side, if if strlen(ifr.ifr_name) < sizeof(dev->ifname), you wouldn't copy over terminating 0.
> I think it needs to be something like:
>
> size = strnlen(ifr.ifr_name, sizeof(ifr.ifr_name)) + 1;
> size = RTE_MIN(size, sizeof(dev->ifname));
> strncpy(dev->ifname, ifr.ifr_name, size);
> dev->ifname[sizeof(dev->ifname) - 1] = 0;
This patch has been applied as hot fix.
If you think you can improve it, please submit a patch.
We are releasing in few hours (probably tomorrow morning).
> > +
> > + strncpy(dev->ifname, ifr.ifr_name, size);
> > + } else
> > + RTE_LOG(ERR, VHOST_CONFIG,
> > + "(%"PRIu64") TUNGETIFF ioctl failed\n",
> > + dev->device_fh);
> > +
> > + return 0;
>
> Shouldn't we return a failure if ret is negative (ioclt failed)?
> Konstantin
next prev parent reply other threads:[~2014-12-19 16:02 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-12-18 18:07 ciara.loftus
2014-12-18 22:02 ` Thomas Monjalon
2014-12-19 15:39 ` Ananyev, Konstantin
2014-12-19 16:02 ` Thomas Monjalon [this message]
2015-01-25 4:41 ` Linhaifeng
2015-01-25 8:34 ` Xie, Huawei
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=2876256.xg5KmeNcVB@xps13 \
--to=thomas.monjalon@6wind.com \
--cc=dev@dpdk.org \
--cc=konstantin.ananyev@intel.com \
/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).