From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.tuxdriver.com (charlotte.tuxdriver.com [70.61.120.58]) by dpdk.org (Postfix) with ESMTP id A53D6AFCE for ; Tue, 17 Jun 2014 22:40:11 +0200 (CEST) Received: from hmsreliant.think-freely.org ([2001:470:8:a08:7aac:c0ff:fec2:933b] helo=localhost) by smtp.tuxdriver.com with esmtpsa (TLSv1:AES128-SHA:128) (Exim 4.63) (envelope-from ) id 1Wx0Au-0006BV-K8; Tue, 17 Jun 2014 16:40:26 -0400 Date: Tue, 17 Jun 2014 16:40:11 -0400 From: Neil Horman To: "Richardson, Bruce" Message-ID: <20140617204011.GH8539@hmsreliant.think-freely.org> References: <1403031832-28540-1-git-send-email-nhorman@tuxdriver.com> <59AF69C657FD0841A61C55336867B5B01AA37370@IRSMSX103.ger.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <59AF69C657FD0841A61C55336867B5B01AA37370@IRSMSX103.ger.corp.intel.com> User-Agent: Mutt/1.5.23 (2014-03-12) X-Spam-Score: -2.9 (--) X-Spam-Status: No Cc: "dev@dpdk.org" Subject: Re: [dpdk-dev] [PATCH] vfio: correct system call error checking 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: Tue, 17 Jun 2014 20:40:11 -0000 On Tue, Jun 17, 2014 at 08:21:29PM +0000, Richardson, Bruce wrote: > > -----Original Message----- > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Neil Horman > > Sent: Tuesday, June 17, 2014 12:04 PM > > To: dev@dpdk.org > > Subject: [dpdk-dev] [PATCH] vfio: correct system call error checking > > > > Noticed today that ioctl error code return checking was incorrect in some of the > > vfio code. ioctl can return a negative value if the system detects an error > > before the target device/driver can produce a return code. The dpdk vfio code > > only checks specfically for the values that it expects, which leaves it open to > > accepting unexpected error codes as success. For instance, if the vfio layer > > noted that the iommu driver hadn't finished registering yet, it would return an > > -EINVAL error code, but the dpdk would accept that as success, becuase it > > wasn't > > 0. > > > > Fix this to specifically check for < 0 error codes > > > > Signed-off-by: Neil Horman > > CC: Thomas Monjalon > > --- > > lib/librte_eal/linuxapp/eal/eal_pci_vfio.c | 8 ++++---- > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c > > b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c > > index 4de6061..65aa8ad 100644 > > --- a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c > > +++ b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c > > @@ -319,16 +319,16 @@ pci_vfio_get_container_fd(void) > > > > /* check VFIO API version */ > > ret = ioctl(vfio_container_fd, VFIO_GET_API_VERSION); > > - if (ret != VFIO_API_VERSION) { > > - RTE_LOG(ERR, EAL, " unknown VFIO API version!\n"); > > + if ((ret < 0) || (ret != VFIO_API_VERSION)) { > > + RTE_LOG(ERR, EAL, " unknown VFIO API version! errno > > = %d\n", errno); > > close(vfio_container_fd); > > return -1; > > } > > Not sure how this change improves things, since the existing check will already trigger an error on all values <0. Can you please clarify why you think this needs to be changed? Ah, my bad, the ret < 0 is superfulous, as the != already catches it, but the log message change is valuable in that it differentiates bad API version detection from other system errors. I can respin that if you like. Neil > > > > > /* check if we support IOMMU type 1 */ > > ret = ioctl(vfio_container_fd, VFIO_CHECK_EXTENSION, > > VFIO_TYPE1_IOMMU); > > - if (!ret) { > > - RTE_LOG(ERR, EAL, " unknown IOMMU driver!\n"); > > + if (ret <= 0) { > > + RTE_LOG(ERR, EAL, " unknown IOMMU driver! errno = > > %d\n", errno); > > close(vfio_container_fd); > > return -1; > > } > > Ack on this change part. The previously code was incorrect according to what I read in the docs for VFIO. > > /Bruce >