From patchwork Thu Jan 29 22:18:01 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Thomas Monjalon X-Patchwork-Id: 2758 Return-Path: X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [IPv6:::1]) by dpdk.org (Postfix) with ESMTP id 687F15A96; Thu, 29 Jan 2015 23:18:30 +0100 (CET) Received: from mail-wi0-f174.google.com (mail-wi0-f174.google.com [209.85.212.174]) by dpdk.org (Postfix) with ESMTP id 929A25A93 for ; Thu, 29 Jan 2015 23:18:28 +0100 (CET) Received: by mail-wi0-f174.google.com with SMTP id n3so33143110wiv.1 for ; Thu, 29 Jan 2015 14:18:28 -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=c8+raSoiBRbDVZz99i0waWyMqdZ1naymJAxTJLuHJmg=; b=K9dWm0Vr4peDuHTH0N1sh174HZ38rHXOtOWc3523JWbkjFyloG9fn1kmNEluURBFsP 2BRVwtIxZAtl1G+7ZwPF7H2FNBbOAy9fP4YbATRupYRWMZirVEpIZ67fz8EoD+RzJJTg fm2d4L1CQGlgqgCkdHJ95TLYJbWbowTC9xk+aQsifTYEW8vJLmfzMDr6ocDK7XA2GBL0 AC/062lm5RBUulujnPY4EiewbklWIAS5xVVXp6sCJcIhtWlm9kE/Ol5jO+qYTM2WkBnM UqCClSWRXNl0scwSTuJ6aGclzSGGIofAYqzE/TA7YicHQldNeRLGf+gQIct/QHWHwjkQ Nktg== X-Gm-Message-State: ALoCoQlby/J5YR2OlfFicg/MAwTJEfNvZTRwO/y53/YdS/TMmGqjoNmmGD/ywnn8DXikUrwT/G0U X-Received: by 10.194.5.37 with SMTP id p5mr6075704wjp.20.1422569908347; Thu, 29 Jan 2015 14:18:28 -0800 (PST) Received: from xps13.localnet (136-92-190-109.dsl.ovh.fr. [109.190.92.136]) by mx.google.com with ESMTPSA id ku8sm12196844wjb.23.2015.01.29.14.18.26 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 29 Jan 2015 14:18:27 -0800 (PST) From: Thomas Monjalon To: Bruce Richardson Date: Thu, 29 Jan 2015 23:18:01 +0100 Message-ID: <9179246.fLSLRuqZit@xps13> Organization: 6WIND User-Agent: KMail/4.14.4 (Linux/3.18.4-1-ARCH; KDE/4.14.4; x86_64; ; ) In-Reply-To: <1953153.vMxMoL6Atb@xps13> References: <5418900.qsIIdEyxOF@xps13> <20141201171854.GH4856@bricha3-MOBL3> <1953153.vMxMoL6Atb@xps13> MIME-Version: 1.0 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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): Thanks --- 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