[v2] net/ixgbe: fix RxQ/TxQ release

Message ID 20210928081238.16782-1-julien.meunier@nokia.com (mailing list archive)
State Accepted, archived
Delegated to: Qi Zhang
Headers
Series [v2] net/ixgbe: fix RxQ/TxQ release |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/github-robot: build success github build: passed
ci/intel-Testing success Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-intel-Functional fail Functional Testing issues
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS

Commit Message

Julien Meunier Sept. 28, 2021, 8:12 a.m. UTC
  On the vector implementation, during the tear-down, the mbufs not
drained in the RxQ and TxQ are freed based on an algorithm which
supposed that the number of descriptors is a power of 2 (max_desc).
Based on this hypothesis, this algorithm uses a bitmask in order to
detect an index overflow during the iteration, and to restart the loop
from 0.

However, there is no such power of 2 requirement in the ixgbe for the
number of descriptors in the RxQ / TxQ. The only requirement is to have
a number correctly aligned.

If a user requested to configure a number of descriptors which is not a
power of 2, as a consequence, during the tear-down, it was possible to
be in an infinite loop, and to never reach the exit loop condition.

By removing the bitmask and changing the loop method, we can avoid this
issue, and allow the user to configure a RxQ / TxQ which is not a power
of 2.

Fixes: c95584dc2b18 ("ixgbe: new vectorized functions for Rx/Tx")
Cc: bruce.richardson@intel.com
Cc: stable@dpdk.org

Signed-off-by: Julien Meunier <julien.meunier@nokia.com>
---
 drivers/net/ixgbe/ixgbe_rxtx_vec_common.h | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

--
2.17.1
  

Comments

Wang, Haiyue Sept. 28, 2021, 12:18 p.m. UTC | #1
> -----Original Message-----
> From: Julien Meunier <julien.meunier@nokia.com>
> Sent: Tuesday, September 28, 2021 16:13
> To: dev@dpdk.org
> Cc: stable@dpdk.org; Richardson, Bruce <bruce.richardson@intel.com>; Wang, Haiyue
> <haiyue.wang@intel.com>
> Subject: [PATCH v2] net/ixgbe: fix RxQ/TxQ release
> 
> On the vector implementation, during the tear-down, the mbufs not
> drained in the RxQ and TxQ are freed based on an algorithm which
> supposed that the number of descriptors is a power of 2 (max_desc).
> Based on this hypothesis, this algorithm uses a bitmask in order to
> detect an index overflow during the iteration, and to restart the loop
> from 0.
> 
> However, there is no such power of 2 requirement in the ixgbe for the
> number of descriptors in the RxQ / TxQ. The only requirement is to have
> a number correctly aligned.
> 
> If a user requested to configure a number of descriptors which is not a
> power of 2, as a consequence, during the tear-down, it was possible to
> be in an infinite loop, and to never reach the exit loop condition.
> 
> By removing the bitmask and changing the loop method, we can avoid this
> issue, and allow the user to configure a RxQ / TxQ which is not a power
> of 2.
> 
> Fixes: c95584dc2b18 ("ixgbe: new vectorized functions for Rx/Tx")
> Cc: bruce.richardson@intel.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Julien Meunier <julien.meunier@nokia.com>
> ---
>  drivers/net/ixgbe/ixgbe_rxtx_vec_common.h | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 

Acked-by: Haiyue Wang <haiyue.wang@intel.com>

> --
> 2.17.1
  
Qi Zhang Oct. 9, 2021, 4:43 a.m. UTC | #2
> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Wang, Haiyue
> Sent: Tuesday, September 28, 2021 8:19 PM
> To: Meunier, Julien <julien.meunier@nokia.com>; dev@dpdk.org
> Cc: stable@dpdk.org; Richardson, Bruce <bruce.richardson@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v2] net/ixgbe: fix RxQ/TxQ release
> 
> > -----Original Message-----
> > From: Julien Meunier <julien.meunier@nokia.com>
> > Sent: Tuesday, September 28, 2021 16:13
> > To: dev@dpdk.org
> > Cc: stable@dpdk.org; Richardson, Bruce <bruce.richardson@intel.com>;
> > Wang, Haiyue <haiyue.wang@intel.com>
> > Subject: [PATCH v2] net/ixgbe: fix RxQ/TxQ release
> >
> > On the vector implementation, during the tear-down, the mbufs not
> > drained in the RxQ and TxQ are freed based on an algorithm which
> > supposed that the number of descriptors is a power of 2 (max_desc).
> > Based on this hypothesis, this algorithm uses a bitmask in order to
> > detect an index overflow during the iteration, and to restart the loop
> > from 0.
> >
> > However, there is no such power of 2 requirement in the ixgbe for the
> > number of descriptors in the RxQ / TxQ. The only requirement is to
> > have a number correctly aligned.
> >
> > If a user requested to configure a number of descriptors which is not
> > a power of 2, as a consequence, during the tear-down, it was possible
> > to be in an infinite loop, and to never reach the exit loop condition.
> >
> > By removing the bitmask and changing the loop method, we can avoid
> > this issue, and allow the user to configure a RxQ / TxQ which is not a
> > power of 2.
> >
> > Fixes: c95584dc2b18 ("ixgbe: new vectorized functions for Rx/Tx")
> > Cc: bruce.richardson@intel.com
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Julien Meunier <julien.meunier@nokia.com>
> > ---
> >  drivers/net/ixgbe/ixgbe_rxtx_vec_common.h | 5 ++---
> >  1 file changed, 2 insertions(+), 3 deletions(-)
> >
> 
> Acked-by: Haiyue Wang <haiyue.wang@intel.com>

Applied to dpdk-next-net-intel.

Thanks
Qi
> 
> > --
> > 2.17.1
  

Patch

diff --git a/drivers/net/ixgbe/ixgbe_rxtx_vec_common.h b/drivers/net/ixgbe/ixgbe_rxtx_vec_common.h
index adba855ca3..005e60668a 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx_vec_common.h
+++ b/drivers/net/ixgbe/ixgbe_rxtx_vec_common.h
@@ -152,7 +152,7 @@  _ixgbe_tx_queue_release_mbufs_vec(struct ixgbe_tx_queue *txq)
 	/* release the used mbufs in sw_ring */
 	for (i = txq->tx_next_dd - (txq->tx_rs_thresh - 1);
 	     i != txq->tx_tail;
-	     i = (i + 1) & max_desc) {
+	     i = (i + 1) % txq->nb_tx_desc) {
 		txe = &txq->sw_ring_v[i];
 		rte_pktmbuf_free_seg(txe->mbuf);
 	}
@@ -168,7 +168,6 @@  _ixgbe_tx_queue_release_mbufs_vec(struct ixgbe_tx_queue *txq)
 static inline void
 _ixgbe_rx_queue_release_mbufs_vec(struct ixgbe_rx_queue *rxq)
 {
-	const unsigned int mask = rxq->nb_rx_desc - 1;
 	unsigned int i;

 	if (rxq->sw_ring == NULL || rxq->rxrearm_nb >= rxq->nb_rx_desc)
@@ -183,7 +182,7 @@  _ixgbe_rx_queue_release_mbufs_vec(struct ixgbe_rx_queue *rxq)
 	} else {
 		for (i = rxq->rx_tail;
 		     i != rxq->rxrearm_start;
-		     i = (i + 1) & mask) {
+		     i = (i + 1) % rxq->nb_rx_desc) {
 			if (rxq->sw_ring[i].mbuf != NULL)
 				rte_pktmbuf_free_seg(rxq->sw_ring[i].mbuf);
 		}