DPDK patches and discussions
 help / color / mirror / Atom feed
From: Thomas Monjalon <thomas.monjalon@6wind.com>
To: Bruce Richardson <bruce.richardson@intel.com>
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [BUG] ixgbe vector cannot compile without bulk alloc
Date: Thu, 29 Jan 2015 23:18:01 +0100	[thread overview]
Message-ID: <9179246.fLSLRuqZit@xps13> (raw)
In-Reply-To: <1953153.vMxMoL6Atb@xps13>

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

Thanks
-- 
Thomas

  reply	other threads:[~2015-01-29 22:18 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-01 17:10 Thomas Monjalon
2014-12-01 17:18 ` Bruce Richardson
2014-12-01 17:22   ` Thomas Monjalon
2015-01-29 22:18     ` Thomas Monjalon [this message]
2015-01-29 23:27       ` Bruce Richardson
2015-01-29 23:31         ` [dpdk-dev] [PATCH] ixgbe: forbid building vpmd without Rx " Thomas Monjalon
2015-02-20 11:03           ` Thomas Monjalon
2015-01-29 23:39         ` [dpdk-dev] [BUG] ixgbe vector cannot compile without " Liang, Cunming
2015-01-30  3:38           ` Bruce Richardson
2015-01-30 19:21             ` Liang, Cunming

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=9179246.fLSLRuqZit@xps13 \
    --to=thomas.monjalon@6wind.com \
    --cc=bruce.richardson@intel.com \
    --cc=dev@dpdk.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).