From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id 837FE234 for ; Fri, 30 Jan 2015 00:27:54 +0100 (CET) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga102.fm.intel.com with ESMTP; 29 Jan 2015 15:27:52 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.09,488,1418112000"; d="scan'208";a="669977117" Received: from unknown ([10.232.198.31]) by fmsmga002.fm.intel.com with SMTP; 29 Jan 2015 15:27:50 -0800 Received: by (sSMTP sendmail emulation); Thu, 29 Jan 2015 16:27:49 -0600 Date: Thu, 29 Jan 2015 16:27:49 -0700 From: Bruce Richardson To: Thomas Monjalon Message-ID: <20150129232748.GA11276@bricha3-MOBL3> References: <5418900.qsIIdEyxOF@xps13> <20141201171854.GH4856@bricha3-MOBL3> <1953153.vMxMoL6Atb@xps13> <9179246.fLSLRuqZit@xps13> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <9179246.fLSLRuqZit@xps13> Organization: Intel Shannon Ltd. User-Agent: Mutt/1.5.23 (2014-03-12) Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [BUG] ixgbe vector cannot compile without bulk alloc 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: Thu, 29 Jan 2015 23:27:54 -0000 On Thu, Jan 29, 2015 at 11:18:01PM +0100, Thomas Monjalon wrote: > 2014-12-01 18:22, Thomas Monjalon: > > 2014-12-01 17:18, Bruce Richardson: > > > On Mon, Dec 01, 2014 at 06:10:18PM +0100, Thomas Monjalon wrote: > > > > These 2 configuration options are incompatible: > > > > CONFIG_RTE_LIBRTE_IXGBE_RX_ALLOW_BULK_ALLOC=n > > > > CONFIG_RTE_IXGBE_INC_VECTOR=y > > > > Building this config gives this error: > > > > lib/librte_pmd_ixgbe/ixgbe_rxtx_vec.c:69:24: > > > > error: ‘struct igb_rx_queue’ has no member named ‘fake_mbuf’ > > > > > > > > I'd like a confirmation that it will be always incompatible. > > > > Thanks > > > > > > Hi Thomas, > > > > > > I don't think these options should always be incompatible, though as you point > > > out you do need to turn on bulk alloc support in order to use the vector PMD. > > > Why do you ask? There are no immediate plans to remove the dependency on our end. > > So you confirm that the ixgbe vpmd really needs Rx bulk alloc and this kind of > patch cannot work at all (I don't know the design of vpmd): > > --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.c > +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.c > @@ -2119,12 +2119,12 @@ ixgbe_reset_rx_queue(struct igb_rx_queue *rxq) > rxq->rx_ring[i] = zeroed_desc; > } > > -#ifdef RTE_LIBRTE_IXGBE_RX_ALLOW_BULK_ALLOC > /* > * initialize extra software ring entries. Space for these extra > * entries is always allocated > */ > memset(&rxq->fake_mbuf, 0x0, sizeof(rxq->fake_mbuf)); > +#ifdef RTE_LIBRTE_IXGBE_RX_ALLOW_BULK_ALLOC > for (i = 0; i < RTE_PMD_IXGBE_RX_MAX_BURST; ++i) { > rxq->sw_ring[rxq->nb_rx_desc + i].mbuf = &rxq->fake_mbuf; > } > --- a/lib/librte_pmd_ixgbe/ixgbe_rxtx.h > +++ b/lib/librte_pmd_ixgbe/ixgbe_rxtx.h > @@ -127,9 +127,9 @@ struct igb_rx_queue { > uint8_t crc_len; /**< 0 if CRC stripped, 4 otherwise. */ > uint8_t drop_en; /**< If not 0, set SRRCTL.Drop_En. */ > uint8_t rx_deferred_start; /**< not in global dev start. */ > -#ifdef RTE_LIBRTE_IXGBE_RX_ALLOW_BULK_ALLOC > /** need to alloc dummy mbuf, for wraparound when scanning hw ring */ > struct rte_mbuf fake_mbuf; > +#ifdef RTE_LIBRTE_IXGBE_RX_ALLOW_BULK_ALLOC > /** hold packets to return to application */ > struct rte_mbuf *rx_stage[RTE_PMD_IXGBE_RX_MAX_BURST*2]; > #endif > > > I think the compilation shouldn't fail without a proper message. > > In order to distinguish a real compilation error from an incompatibility, > > we should add a warning in the makefile. > > Ideally, the build system should handle dependencies. But waiting this ideal > > time, a warning would be graceful. > > Do you agree that something like this would be OK? > > --- a/lib/librte_pmd_ixgbe/Makefile > +++ b/lib/librte_pmd_ixgbe/Makefile > @@ -114,4 +114,8 @@ DEPDIRS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD) += lib/librte_eal lib/librte_ether > DEPDIRS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD) += lib/librte_mempool lib/librte_mbuf > DEPDIRS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD) += lib/librte_net lib/librte_malloc > > +ifeq ($(CONFIG_RTE_IXGBE_INC_VECTOR)$(CONFIG_RTE_LIBRTE_IXGBE_RX_ALLOW_BULK_ALLOC),yn) > +$(error The ixgbe vpmd depends on Rx bulk alloc) > +endif > + > include $(RTE_SDK)/mk/rte.lib.mk > Something like the above looks like a good solution to me. /Bruce > Thanks > -- > Thomas