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 BF7637F44 for ; Mon, 24 Nov 2014 01:07:07 +0100 (CET) Received: from [2001:470:8:a08:215:ff:fecc:4872] (helo=localhost) by smtp.tuxdriver.com with esmtpsa (TLSv1:AES128-SHA:128) (Exim 4.63) (envelope-from ) id 1XshLg-0003V5-Hr; Sun, 23 Nov 2014 19:17:51 -0500 Date: Sun, 23 Nov 2014 19:17:44 -0500 From: Neil Horman To: Sujith Sankar Message-ID: <20141124001744.GA7751@localhost.localdomain> References: <1416758899-1351-1-git-send-email-ssujith@cisco.com> <1416758899-1351-7-git-send-email-ssujith@cisco.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1416758899-1351-7-git-send-email-ssujith@cisco.com> User-Agent: Mutt/1.5.23 (2014-03-12) X-Spam-Score: -2.9 (--) X-Spam-Status: No Cc: dev@dpdk.org, prrao@cisco.com Subject: Re: [dpdk-dev] [PATCH v3 6/6] DPDK changes for accommodating ENIC PMD 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: Mon, 24 Nov 2014 00:07:08 -0000 On Sun, Nov 23, 2014 at 09:38:19PM +0530, Sujith Sankar wrote: > Signed-off-by: Sujith Sankar > --- > config/common_linuxapp | 5 +++++ > lib/Makefile | 1 + > lib/librte_eal/linuxapp/eal/eal_pci_vfio.c | 7 +++++++ > lib/librte_eal/linuxapp/eal/include/eal_pci_init.h | 1 + > mk/rte.app.mk | 4 ++++ > 5 files changed, 18 insertions(+) > > diff --git a/config/common_linuxapp b/config/common_linuxapp > index 57b61c9..3c091e7 100644 > --- a/config/common_linuxapp > +++ b/config/common_linuxapp > @@ -210,6 +210,11 @@ CONFIG_RTE_LIBRTE_I40E_QUEUE_NUM_PER_VM=4 > CONFIG_RTE_LIBRTE_I40E_ITR_INTERVAL=-1 > > # > +# Compile burst-oriented Cisco ENIC PMD driver > +# > +CONFIG_RTE_LIBRTE_ENIC_PMD=y > + > +# > # Compile burst-oriented VIRTIO PMD driver > # > CONFIG_RTE_LIBRTE_VIRTIO_PMD=y > diff --git a/lib/Makefile b/lib/Makefile > index e3237ff..1911790 100644 > --- a/lib/Makefile > +++ b/lib/Makefile > @@ -43,6 +43,7 @@ DIRS-$(CONFIG_RTE_LIBRTE_CMDLINE) += librte_cmdline > DIRS-$(CONFIG_RTE_LIBRTE_ETHER) += librte_ether > DIRS-$(CONFIG_RTE_LIBRTE_E1000_PMD) += librte_pmd_e1000 > DIRS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD) += librte_pmd_ixgbe > +DIRS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD) += librte_pmd_enic > DIRS-$(CONFIG_RTE_LIBRTE_I40E_PMD) += librte_pmd_i40e > DIRS-$(CONFIG_RTE_LIBRTE_PMD_BOND) += librte_pmd_bond > DIRS-$(CONFIG_RTE_LIBRTE_PMD_RING) += librte_pmd_ring > diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c > index c776ddc..6bf8f2e 100644 > --- a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c > +++ b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c > @@ -736,6 +736,7 @@ pci_vfio_map_resource(struct rte_pci_device *dev) > maps[i].offset = reg.offset; > maps[i].size = reg.size; > dev->mem_resource[i].addr = bar_addr; > + dev->mem_resource[i].len = reg.size; > } > > /* if secondary process, do not set up interrupts */ > @@ -791,4 +792,10 @@ pci_vfio_is_enabled(void) > { > return vfio_cfg.vfio_enabled; > } > + > +int > +pci_vfio_container_fd(void) > +{ > + return vfio_cfg.vfio_container_fd; > +} You should move this function definition to a separate patch and put it earlier in the series, as you call this function two patches back. Also, this gives me pause, as it seems like you're working around the VFIO api. >>From what I see, you just use this to get an fd that you can use in an ioctl to set some DMA settings. First off, theres already a function called pci_vfio_get_container_fd, which does exactly what you are doing here, with additional safety checking. However, even though there is an existing function to do what you want, I would recommend that you not use it for your purposes. Whenever you expose something like a file descriptor, you run the risk of multiple accessors racing trying to set/unset features and preform operations. It would be better if you could add apropriate api calls to vfio interface to set what you want. That way the library can add appropriate locking if/when needed Neil > #endif > diff --git a/lib/librte_eal/linuxapp/eal/include/eal_pci_init.h b/lib/librte_eal/linuxapp/eal/include/eal_pci_init.h > index d758bee..c9e9e40 100644 > --- a/lib/librte_eal/linuxapp/eal/include/eal_pci_init.h > +++ b/lib/librte_eal/linuxapp/eal/include/eal_pci_init.h > @@ -71,6 +71,7 @@ int pci_uio_map_resource(struct rte_pci_device *dev); > > int pci_vfio_enable(void); > int pci_vfio_is_enabled(void); > +int pci_vfio_container_fd(void); > int pci_vfio_mp_sync_setup(void); > > /* map VFIO resource prototype */ > diff --git a/mk/rte.app.mk b/mk/rte.app.mk > index 285b65c..95c652f 100644 > --- a/mk/rte.app.mk > +++ b/mk/rte.app.mk > @@ -186,6 +186,10 @@ ifeq ($(CONFIG_RTE_LIBRTE_VMXNET3_PMD),y) > LDLIBS += -lrte_pmd_vmxnet3_uio > endif > > +ifeq ($(CONFIG_RTE_LIBRTE_ENIC_PMD),y) > +LDLIBS += -lrte_pmd_enic > +endif > + > ifeq ($(CONFIG_RTE_LIBRTE_VIRTIO_PMD),y) > LDLIBS += -lrte_pmd_virtio_uio > endif > -- > 1.9.1 > >