From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wi0-f178.google.com (mail-wi0-f178.google.com [209.85.212.178]) by dpdk.org (Postfix) with ESMTP id 7CE248041 for ; Fri, 19 Dec 2014 17:02:46 +0100 (CET) Received: by mail-wi0-f178.google.com with SMTP id em10so2231135wid.17 for ; Fri, 19 Dec 2014 08:02:46 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:organization :user-agent:in-reply-to:references:mime-version :content-transfer-encoding:content-type; bh=YVRmHuaiJGxdxEtLsZ0GvQZ93lXrFhmEfDr9Q2CijqM=; b=DcXogY3QbZp2E5puonV/Jhz8w3Svr9AV7upxbAVVEQ62cyFAFwdsXF9CtCMjIRUplk 2nlEoOv4g0c0vZrPro5tAsy9UP1s2B1JgWkaMIyj5+JMqXDEDhPwTfLOLAEyMEQ35kvI Sm2nDN5KxFQLvj+cn19lQFxDetgjSAFQmZf8+mjtvzD3CyEafTxcYUD2qn4hRpIBbJLw kfEcvVMXkyQNaoJXMSYsraaxv96WQEXFQVH9c+tOJxXbpZIfuCRXJfunAU35MR7+CdnU cX9bD7bfxc16wmCqOp7KoP7q25kDv7Il4k8gl2H8n+lQi4JkKNJMPTVlQ76LQkBY1/xP X7ag== X-Gm-Message-State: ALoCoQn1lSH1rLQ/shpRa1OaoCUh/ZhzYg3nnf9irKfsAUG//YoOs9tSSPxZiC8lfHoHGvjnH+74 X-Received: by 10.194.24.103 with SMTP id t7mr16183193wjf.15.1419004966143; Fri, 19 Dec 2014 08:02:46 -0800 (PST) Received: from xps13.localnet (136-92-190-109.dsl.ovh.fr. [109.190.92.136]) by mx.google.com with ESMTPSA id js5sm2817149wid.11.2014.12.19.08.02.44 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 19 Dec 2014 08:02:45 -0800 (PST) From: Thomas Monjalon To: "Ananyev, Konstantin" Date: Fri, 19 Dec 2014 17:02:19 +0100 Message-ID: <2876256.xg5KmeNcVB@xps13> Organization: 6WIND User-Agent: KMail/4.14.3 (Linux/3.17.4-1-ARCH; KDE/4.14.3; x86_64; ; ) In-Reply-To: <2601191342CEEE43887BDE71AB977258213C2436@IRSMSX105.ger.corp.intel.com> References: <1418926027-19794-1-git-send-email-ciara.loftus@intel.com> <2601191342CEEE43887BDE71AB977258213C2436@IRSMSX105.ger.corp.intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH v2] vhost: add interface name to virtio-net struct X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 19 Dec 2014 16:02:46 -0000 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 > > > > 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 > > Signed-off-by: Anthony Fee > > Acked-by: Huawei Xie [...] > > /* > > + * 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