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 at dpdk.org > > Cc: thomas.monjalon at 6wind.com; stephen at networkplumber.org; > > vincent.jardin at 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 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> [...] > > /* > > + * 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