ixgbe vector cannot compile without bulk alloc

BUG In review Thomas Monjalon 3 participants 10 messages Dec 1, 2014 – Feb 20, 2015 2 messages not in the archive

Share & tools

Thomas Monjalon

author

2014-12-01 17:10 UTC #0 lore raw

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

Signature · 1 line

Thomas

Bruce Richardson

2014-12-01 17:18 UTC #1 lore raw

Quoted 11 lines · Thomas Monjalon

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
Signature · 1 line
Thomas

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.

/Bruce

Thomas Monjalon

author

2014-12-01 17:22 UTC #2 lore raw

2014-12-01 17:18, Bruce Richardson:

Quoted 8 lines · Thomas Monjalon

Quoted 7 lines
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’

Quoted 8 lines

Quoted 2 lines
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.

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.

Signature · 1 line

Thomas

Thomas Monjalon

author

2015-01-29 22:18 UTC #3 lore raw

2014-12-01 18:22, Thomas Monjalon:

Quoted 17 lines

2014-12-01 17:18, Bruce Richardson:
Quoted 16 lines · Thomas Monjalon
Quoted 9 lines
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):

lib/librte_pmd_ixgbe/ixgbe_rxtx.c +1 -1

@@ -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;
        }

lib/librte_pmd_ixgbe/ixgbe_rxtx.h +1 -1

@@ -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

Quoted 5 lines

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?

lib/librte_pmd_ixgbe/Makefile +4 -0

@@ -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

Signature · 1 line

Thomas

Bruce Richardson

2015-01-29 23:27 UTC #4 lore raw

Quoted 64 lines · Thomas Monjalon

2014-12-01 18:22, Thomas Monjalon:
Quoted 17 lines
2014-12-01 17:18, Bruce Richardson:
Quoted 16 lines · Thomas Monjalon
Quoted 9 lines
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):
lib/librte_pmd_ixgbe/ixgbe_rxtx.c +1 -1
@@ -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;
        }
lib/librte_pmd_ixgbe/ixgbe_rxtx.h +1 -1
@@ -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
Quoted 6 lines
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?

lib/librte_pmd_ixgbe/Makefile DEPDIRS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD) += lib/librte_eal lib/librte_ether · line 121

Quoted 8 lines

lib/librte_pmd_ixgbe/Makefile +4 -0

@@ -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

Quoted 3 lines

Thanks
Signature · 1 line
Thomas

Thomas Monjalon

author

2015-01-29 23:31 UTC #5 lore raw

PATCH 1/1 ixgbe: forbid building vpmd without Rx bulk alloc

CONFIG_RTE_LIBRTE_IXGBE_RX_ALLOW_BULK_ALLOC is a prerequisite of CONFIG_RTE_IXGBE_INC_VECTOR.

Reported-by Alexander Belyakov <[email protected]>

Signed-off-by Thomas Monjalon <[email protected]>

---
 lib/librte_pmd_ixgbe/Makefile | 4 ++++
 1 file changed, 4 insertions(+)

lib/librte_pmd_ixgbe/Makefile +4 -0

index 3588047..33c17db 100644
@@ -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

Signature · 1 line

2.2.2

A message that is not in the archive (sent off-list or not yet received): 1422574319-9428-1-git-send-email-thomas.monjalon-pdR9zngts4EAvxtiuMwx3w@public.gmane.org

Thomas Monjalon

author Applied

2015-02-20 11:03 UTC #7 lore raw

Re: [PATCH] ixgbe: forbid building vpmd without Rx bulk alloc

2015-01-30 00:31, Thomas Monjalon:

Quoted 5 lines

CONFIG_RTE_LIBRTE_IXGBE_RX_ALLOW_BULK_ALLOC is a prerequisite of CONFIG_RTE_IXGBE_INC_VECTOR.
Reported-by Alexander Belyakov <[email protected]>
Signed-off-by Thomas Monjalon <[email protected]>

Applied

Liang, Cunming

2015-01-29 23:39 UTC #8 lore raw

In reply to Bruce Richardson

Quoted 98 lines

-----Original Message-----
From: dev [mailto:[email protected]] On Behalf Of Bruce Richardson
Sent: Thursday, January 29, 2015 4:28 PM
To: Thomas Monjalon
Cc: [email protected]
Subject: Re: [dpdk-dev] [BUG] ixgbe vector cannot compile without bulk alloc
Quoted 16 lines · Thomas Monjalon
2014-12-01 18:22, Thomas Monjalon:
Quoted 15 lines
2014-12-01 17:18, Bruce Richardson:
Quoted 14 lines · Thomas Monjalon
Quoted 9 lines
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
Quoted 1 line
Quoted 1 line
Quoted 1 line
out you do need to turn on bulk alloc support in order to use the vector
PMD.
Quoted 1 line
Quoted 1 line
Quoted 1 line
Why do you ask? There are no immediate plans to remove the dependency
on our end.
Quoted 9 lines
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):
lib/librte_pmd_ixgbe/ixgbe_rxtx.c +0 -0
@@ -2119,12 +2119,12 @@ ixgbe_reset_rx_queue(struct igb_rx_queue *rxq)
                rxq->rx_ring[i] = zeroed_desc;
        }
lib/librte_pmd_ixgbe/ixgbe_rxtx.c ixgbe_reset_rx_queue(struct igb_rx_queue *rxq) · line 2129
Quoted 10 lines
lib/librte_pmd_ixgbe/ixgbe_rxtx.c +1 -1
 
-#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;
lib/librte_pmd_ixgbe/ixgbe_rxtx.h struct igb_rx_queue { · line 127
Quoted 5 lines
lib/librte_pmd_ixgbe/ixgbe_rxtx.c +0 -0
        }
lib/librte_pmd_ixgbe/ixgbe_rxtx.h +0 -0
@@ -127,9 +127,9 @@ struct igb_rx_queue {
        uint8_t             crc_len;  /**< 0 if CRC stripped, 4 otherwise.
*/
lib/librte_pmd_ixgbe/ixgbe_rxtx.h struct igb_rx_queue { · line 128
Quoted 1 line
lib/librte_pmd_ixgbe/ixgbe_rxtx.h +0 -0
        uint8_t             drop_en;  /**< If not 0, set SRRCTL.Drop_En.
*/
lib/librte_pmd_ixgbe/ixgbe_rxtx.h struct igb_rx_queue { · line 129
Quoted 1 line
lib/librte_pmd_ixgbe/ixgbe_rxtx.h +0 -0
        uint8_t             rx_deferred_start; /**< not in global dev start.
*/
lib/librte_pmd_ixgbe/ixgbe_rxtx.h struct igb_rx_queue { · line 130
Quoted 2 lines
lib/librte_pmd_ixgbe/ixgbe_rxtx.h +0 -1
-#ifdef RTE_LIBRTE_IXGBE_RX_ALLOW_BULK_ALLOC
        /** need to alloc dummy mbuf, for wraparound when scanning hw
ring */
lib/librte_pmd_ixgbe/Makefile DEPDIRS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD) +=
Quoted 17 lines
lib/librte_pmd_ixgbe/ixgbe_rxtx.h +1 -0
        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
Quoted 6 lines
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?
lib/librte_pmd_ixgbe/Makefile +0 -0
@@ -114,4 +114,8 @@ DEPDIRS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD) +=
lib/librte_eal lib/librte_ether
lib/librte_pmd_ixgbe/Makefile DEPDIRS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD) += · line 114
Quoted 1 line
lib/librte_pmd_ixgbe/Makefile +0 -0
 DEPDIRS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD) += lib/librte_mempool
lib/librte_mbuf
lib/librte_pmd_ixgbe/Makefile DEPDIRS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD) += · line 115
Quoted 1 line
lib/librte_pmd_ixgbe/Makefile +0 -0
 DEPDIRS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD) += lib/librte_net
lib/librte_malloc
lib/librte_pmd_ixgbe/Makefile DEPDIRS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD) += · line 116
Quoted 2 lines
lib/librte_pmd_ixgbe/Makefile +1 -0
+ifeq
($(CONFIG_RTE_IXGBE_INC_VECTOR)$(CONFIG_RTE_LIBRTE_IXGBE_RX_ALLOW_B ULK_ALLOC),yn)
lib/librte_pmd_ixgbe/Makefile DEPDIRS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD) += lib/librte_eal lib/librte_ether
Quoted 5 lines
lib/librte_pmd_ixgbe/Makefile +3 -0
+$(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.

Quoted 1 line

/Bruce

[Liang, Cunming] To avoid compile complain, this one is ok. It's doable to remove the dependence between two. We can submit it in a separate patch.

Quoted 4 lines

Quoted 4 lines
Thanks
--
Thomas

A message that is not in the archive (sent off-list or not yet received): D0158A423229094DA7ABF71CF2FA0DA3118C1680-E2R4CRU6q/[email protected]

Bruce Richardson

2015-01-30 03:38 UTC #10 lore raw

Quoted 98 lines · Cunming

Quoted 98 lines
-----Original Message-----
From: dev [mailto:[email protected]] On Behalf Of Bruce Richardson
Sent: Thursday, January 29, 2015 4:28 PM
To: Thomas Monjalon
Cc: [email protected]
Subject: Re: [dpdk-dev] [BUG] ixgbe vector cannot compile without bulk alloc
Quoted 16 lines · Thomas Monjalon
2014-12-01 18:22, Thomas Monjalon:
Quoted 15 lines
2014-12-01 17:18, Bruce Richardson:
Quoted 14 lines · Thomas Monjalon
Quoted 9 lines
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
Quoted 1 line
Quoted 1 line
Quoted 1 line
out you do need to turn on bulk alloc support in order to use the vector
PMD.
Quoted 1 line
Quoted 1 line
Quoted 1 line
Why do you ask? There are no immediate plans to remove the dependency
on our end.
Quoted 9 lines
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):
lib/librte_pmd_ixgbe/ixgbe_rxtx.c +0 -0
@@ -2119,12 +2119,12 @@ ixgbe_reset_rx_queue(struct igb_rx_queue *rxq)
                rxq->rx_ring[i] = zeroed_desc;
        }
lib/librte_pmd_ixgbe/ixgbe_rxtx.c ixgbe_reset_rx_queue(struct igb_rx_queue *rxq) · line 2129
Quoted 10 lines
lib/librte_pmd_ixgbe/ixgbe_rxtx.c +1 -1
 
-#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;
lib/librte_pmd_ixgbe/ixgbe_rxtx.h struct igb_rx_queue { · line 127
Quoted 5 lines
lib/librte_pmd_ixgbe/ixgbe_rxtx.c +0 -0
        }
lib/librte_pmd_ixgbe/ixgbe_rxtx.h +0 -0
@@ -127,9 +127,9 @@ struct igb_rx_queue {
        uint8_t             crc_len;  /**< 0 if CRC stripped, 4 otherwise.
*/
lib/librte_pmd_ixgbe/ixgbe_rxtx.h struct igb_rx_queue { · line 128
Quoted 1 line
lib/librte_pmd_ixgbe/ixgbe_rxtx.h +0 -0
        uint8_t             drop_en;  /**< If not 0, set SRRCTL.Drop_En.
*/
lib/librte_pmd_ixgbe/ixgbe_rxtx.h struct igb_rx_queue { · line 129
Quoted 1 line
lib/librte_pmd_ixgbe/ixgbe_rxtx.h +0 -0
        uint8_t             rx_deferred_start; /**< not in global dev start.
*/
lib/librte_pmd_ixgbe/ixgbe_rxtx.h struct igb_rx_queue { · line 130
Quoted 2 lines
lib/librte_pmd_ixgbe/ixgbe_rxtx.h +0 -1
-#ifdef RTE_LIBRTE_IXGBE_RX_ALLOW_BULK_ALLOC
        /** need to alloc dummy mbuf, for wraparound when scanning hw
ring */
lib/librte_pmd_ixgbe/Makefile DEPDIRS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD) +=
Quoted 17 lines
lib/librte_pmd_ixgbe/ixgbe_rxtx.h +1 -0
        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
Quoted 6 lines
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?
lib/librte_pmd_ixgbe/Makefile +0 -0
@@ -114,4 +114,8 @@ DEPDIRS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD) +=
lib/librte_eal lib/librte_ether
lib/librte_pmd_ixgbe/Makefile DEPDIRS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD) += · line 114
Quoted 1 line
lib/librte_pmd_ixgbe/Makefile +0 -0
 DEPDIRS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD) += lib/librte_mempool
lib/librte_mbuf
lib/librte_pmd_ixgbe/Makefile DEPDIRS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD) += · line 115
Quoted 1 line
lib/librte_pmd_ixgbe/Makefile +0 -0
 DEPDIRS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD) += lib/librte_net
lib/librte_malloc
lib/librte_pmd_ixgbe/Makefile DEPDIRS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD) += · line 116
Quoted 2 lines
lib/librte_pmd_ixgbe/Makefile +1 -0
+ifeq
($(CONFIG_RTE_IXGBE_INC_VECTOR)$(CONFIG_RTE_LIBRTE_IXGBE_RX_ALLOW_B ULK_ALLOC),yn)
lib/librte_pmd_ixgbe/Makefile DEPDIRS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD) += lib/librte_eal lib/librte_ether
Quoted 5 lines
lib/librte_pmd_ixgbe/Makefile +3 -0
+$(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.

Quoted 5 lines

Quoted 1 line
/Bruce
[Liang, Cunming] To avoid compile complain, this one is ok. It's doable to remove the dependence between two. We can submit it in a separate patch.
Quoted 1 line

Sure, if that can be done, it sounds good. I don't see a huge problem with having a dependency between the two - I can't really see a use case for someone wanting the vector driver but to have the bulk-alloc scalar one disabled. So, I'm easy either way, with just flagging the warning or removing the dependency completely.

Follow-on question - can we look to remove the bulk alloc switch completely. The user can force the selection of the RX function and TX functions at run time via the nic setup parameters, so I don't see the need to limit the choices at compile time - other than the vpmd which obviously has an instruction set dependency.

/Bruce

Liang, Cunming

2015-01-30 19:21 UTC #11 lore raw

Quoted 130 lines

-----Original Message-----
From: Richardson, Bruce
Sent: Thursday, January 29, 2015 8:38 PM
To: Liang, Cunming
Cc: Thomas Monjalon; [email protected]
Subject: Re: [dpdk-dev] [BUG] ixgbe vector cannot compile without bulk alloc
Quoted 11 lines · Cunming
Quoted 11 lines
-----Original Message-----
From: dev [mailto:[email protected]] On Behalf Of Bruce Richardson
Sent: Thursday, January 29, 2015 4:28 PM
To: Thomas Monjalon
Cc: [email protected]
Subject: Re: [dpdk-dev] [BUG] ixgbe vector cannot compile without bulk alloc
Quoted 1 line · Thomas Monjalon
2014-12-01 18:22, Thomas Monjalon:
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
2014-12-01 17:18, Bruce Richardson:
Quoted 7 lines · Thomas Monjalon
Quoted 6 lines
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’
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 3 lines
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
Quoted 4 lines
Quoted 4 lines
point
Quoted 1 line
Quoted 1 line
Quoted 1 line
out you do need to turn on bulk alloc support in order to use the vector
PMD.
Quoted 1 line
Quoted 1 line
Quoted 1 line
Why do you ask? There are no immediate plans to remove the
dependency
Quoted 3 lines
Quoted 3 lines
on our end.
Quoted 2 lines
So you confirm that the ixgbe vpmd really needs Rx bulk alloc and this kind
of
Quoted 5 lines
Quoted 5 lines
lib/librte_pmd_ixgbe/ixgbe_rxtx.c ixgbe_reset_rx_queue(struct igb_rx_queue
Quoted 5 lines
patch cannot work at all (I don't know the design of vpmd):
lib/librte_pmd_ixgbe/ixgbe_rxtx.c +0 -0
@@ -2119,12 +2119,12 @@ ixgbe_reset_rx_queue(struct igb_rx_queue
*rxq)
Quoted 13 lines
Quoted 13 lines
lib/librte_pmd_ixgbe/ixgbe_rxtx.c ixgbe_reset_rx_queue(struct igb_rx_queue *rxq)
Quoted 12 lines
lib/librte_pmd_ixgbe/ixgbe_rxtx.c +1 -1
                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;
Quoted 5 lines
Quoted 5 lines
lib/librte_pmd_ixgbe/ixgbe_rxtx.h struct igb_rx_queue { · line 127
Quoted 5 lines
lib/librte_pmd_ixgbe/ixgbe_rxtx.c +0 -0
        }
lib/librte_pmd_ixgbe/ixgbe_rxtx.h +0 -0
@@ -127,9 +127,9 @@ struct igb_rx_queue {
        uint8_t             crc_len;  /**< 0 if CRC stripped, 4
otherwise.
Quoted 2 lines
Quoted 2 lines
*/
Quoted 1 line
lib/librte_pmd_ixgbe/ixgbe_rxtx.h +0 -0
        uint8_t             drop_en;  /**< If not 0, set
SRRCTL.Drop_En.
Quoted 2 lines
Quoted 2 lines
*/
Quoted 1 line
lib/librte_pmd_ixgbe/ixgbe_rxtx.h +0 -0
        uint8_t             rx_deferred_start; /**< not in global dev
start.
Quoted 3 lines
Quoted 3 lines
*/
Quoted 2 lines
lib/librte_pmd_ixgbe/ixgbe_rxtx.h +0 -1
-#ifdef RTE_LIBRTE_IXGBE_RX_ALLOW_BULK_ALLOC
        /** need to alloc dummy mbuf, for wraparound when scanning
hw
Quoted 11 lines
Quoted 11 lines
ring */
Quoted 10 lines
lib/librte_pmd_ixgbe/ixgbe_rxtx.h +1 -0
        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
Quoted 5 lines
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
Quoted 8 lines
Quoted 8 lines
lib/librte_pmd_ixgbe/Makefile DEPDIRS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD) +=
Quoted 7 lines
Quoted 1 line
time, a warning would be graceful.
Do you agree that something like this would be OK?
lib/librte_pmd_ixgbe/Makefile +0 -0
@@ -114,4 +114,8 @@ DEPDIRS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD) +=
lib/librte_eal lib/librte_ether
Quoted 7 lines
Quoted 7 lines
lib/librte_pmd_ixgbe/Makefile DEPDIRS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD) += · line 114
Quoted 1 line
lib/librte_pmd_ixgbe/Makefile +0 -0
 DEPDIRS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD) += lib/librte_mempool
lib/librte_mbuf
lib/librte_pmd_ixgbe/Makefile DEPDIRS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD) += · line 115
Quoted 1 line
lib/librte_pmd_ixgbe/Makefile +0 -0
 DEPDIRS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD) += lib/librte_net
lib/librte_malloc
Quoted 2 lines
lib/librte_pmd_ixgbe/Makefile +1 -0
+ifeq
($(CONFIG_RTE_IXGBE_INC_VECTOR)$(CONFIG_RTE_LIBRTE_IXGBE_RX_ALLOW_B
Quoted 9 lines
Quoted 9 lines
ULK_ALLOC),yn)
lib/librte_pmd_ixgbe/Makefile DEPDIRS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD) += lib/librte_eal lib/librte_ether
Quoted 5 lines
lib/librte_pmd_ixgbe/Makefile +3 -0
+$(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.
Quoted 5 lines
Quoted 1 line
/Bruce
[Liang, Cunming] To avoid compile complain, this one is ok. It's doable to remove the dependence between two. We can submit it in a separate patch.
Quoted 1 line
Sure, if that can be done, it sounds good. I don't see a huge problem with having a dependency between the two - I can't really see a use case for someone wanting the vector driver but to have the bulk-alloc scalar one disabled. So, I'm easy either way, with just flagging the warning or removing the dependency completely.

Quoted 5 lines

Follow-on question - can we look to remove the bulk alloc switch completely. The user can force the selection of the RX function and TX functions at run time via the nic setup parameters, so I don't see the need to limit the choices at compile time - other than the vpmd which obviously has an instruction set dependency.

[Liang, Cunming] Agree.

Quoted 2 lines

/Bruce