diff mbox series

[v2,2/4] net/iavf: fix buffer leak on Tx queue stop

Message ID 20230831123337.871496-3-bruce.richardson@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Qi Zhang
Headers
Series Fix i40e/iavf queue reconfig and restarting |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Bruce Richardson Aug. 31, 2023, 12:33 p.m. UTC 
  When a queue is stopped all buffers are meant to released from it, and
then a new set of buffers reallocated on start. For the iavf code when
using vector Tx, some buffers were left in the ring, and so those
buffers were leaked. The buffers were missed as the release code only
handled one side of a wrap-around case in the ring.

Fix the issue by doing a single iteration of the ring freeing all
buffers in it, handling wraparound through a simple post-increment
check.

Fixes: 319c421f3890 ("net/avf: enable SSE Rx Tx")
Fixes: 9ab9514c150e ("net/iavf: enable AVX512 for Tx")
Cc: wenzhuo.lu@intel.com
Cc: jingjing.wu@intel.com
Cc: stable@dpdk.org

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 drivers/net/iavf/iavf_rxtx_vec_avx512.c | 17 ++++++++---------
 drivers/net/iavf/iavf_rxtx_vec_common.h | 11 +++++------
 2 files changed, 13 insertions(+), 15 deletions(-)

Comments

Wenzhuo Lu Sept. 4, 2023, 2:17 a.m. UTC | #1 
> -----Original Message-----
> From: Richardson, Bruce <bruce.richardson@intel.com>
> Sent: Thursday, August 31, 2023 8:34 PM
> To: dev@dpdk.org
> Cc: Richardson, Bruce <bruce.richardson@intel.com>; Lu, Wenzhuo
> <wenzhuo.lu@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>;
> stable@dpdk.org
> Subject: [PATCH v2 2/4] net/iavf: fix buffer leak on Tx queue stop
> 
> When a queue is stopped all buffers are meant to released from it, and then a
> new set of buffers reallocated on start. For the iavf code when using vector Tx,
> some buffers were left in the ring, and so those buffers were leaked. The
> buffers were missed as the release code only handled one side of a
> wrap-around case in the ring.
> 
> Fix the issue by doing a single iteration of the ring freeing all buffers in it,
> handling wraparound through a simple post-increment check.
> 
> Fixes: 319c421f3890 ("net/avf: enable SSE Rx Tx")
> Fixes: 9ab9514c150e ("net/iavf: enable AVX512 for Tx")
> Cc: wenzhuo.lu@intel.com
> Cc: jingjing.wu@intel.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
Acked-by: Wenzhuo Lu <wenzhuo.lu@intel.com>
Qi Zhang Sept. 4, 2023, 2:30 a.m. UTC | #2 
> -----Original Message-----
> From: Lu, Wenzhuo <wenzhuo.lu@intel.com>
> Sent: Monday, September 4, 2023 10:18 AM
> To: Richardson, Bruce <bruce.richardson@intel.com>; dev@dpdk.org
> Cc: Wu, Jingjing <jingjing.wu@intel.com>; stable@dpdk.org
> Subject: RE: [PATCH v2 2/4] net/iavf: fix buffer leak on Tx queue stop
> 
> 
> 
> > -----Original Message-----
> > From: Richardson, Bruce <bruce.richardson@intel.com>
> > Sent: Thursday, August 31, 2023 8:34 PM
> > To: dev@dpdk.org
> > Cc: Richardson, Bruce <bruce.richardson@intel.com>; Lu, Wenzhuo
> > <wenzhuo.lu@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>;
> > stable@dpdk.org
> > Subject: [PATCH v2 2/4] net/iavf: fix buffer leak on Tx queue stop
> >
> > When a queue is stopped all buffers are meant to released from it, and
> > then a new set of buffers reallocated on start. For the iavf code when
> > using vector Tx, some buffers were left in the ring, and so those
> > buffers were leaked. The buffers were missed as the release code only
> > handled one side of a wrap-around case in the ring.
> >
> > Fix the issue by doing a single iteration of the ring freeing all
> > buffers in it, handling wraparound through a simple post-increment check.
> >
> > Fixes: 319c421f3890 ("net/avf: enable SSE Rx Tx")
> > Fixes: 9ab9514c150e ("net/iavf: enable AVX512 for Tx")
> > Cc: wenzhuo.lu@intel.com
> > Cc: jingjing.wu@intel.com
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> Acked-by: Wenzhuo Lu <wenzhuo.lu@intel.com>

Applied to dpdk-next-net-intel.

Thanks
Qi
diff mbox series

Patch

diff --git a/drivers/net/iavf/iavf_rxtx_vec_avx512.c b/drivers/net/iavf/iavf_rxtx_vec_avx512.c
index 3e66df5341..48337d5e03 100644
--- a/drivers/net/iavf/iavf_rxtx_vec_avx512.c
+++ b/drivers/net/iavf/iavf_rxtx_vec_avx512.c
@@ -2460,20 +2460,19 @@  iavf_tx_queue_release_mbufs_avx512(struct iavf_tx_queue *txq)
 {
 	unsigned int i;
 	const uint16_t max_desc = (uint16_t)(txq->nb_tx_desc - 1);
+	const uint16_t end_desc = txq->tx_tail >> txq->use_ctx; /* next empty slot */
+	const uint16_t wrap_point = txq->nb_tx_desc >> txq->use_ctx;  /* end of SW ring */
 	struct iavf_tx_vec_entry *swr = (void *)txq->sw_ring;
 
 	if (!txq->sw_ring || txq->nb_free == max_desc)
 		return;
 
-	i = (txq->next_dd >> txq->use_ctx) + 1 -
-			(txq->rs_thresh >> txq->use_ctx);
-
-	if (txq->tx_tail < i) {
-		for (; i < (unsigned int)(txq->nb_tx_desc >> txq->use_ctx); i++) {
-			rte_pktmbuf_free_seg(swr[i].mbuf);
-			swr[i].mbuf = NULL;
-		}
-		i = 0;
+	i = (txq->next_dd - txq->rs_thresh + 1) >> txq->use_ctx;
+	while (i != end_desc) {
+		rte_pktmbuf_free_seg(swr[i].mbuf);
+		swr[i].mbuf = NULL;
+		if (++i == wrap_point)
+			i = 0;
 	}
 }
 
diff --git a/drivers/net/iavf/iavf_rxtx_vec_common.h b/drivers/net/iavf/iavf_rxtx_vec_common.h
index ddb13ce8c3..1fac957fe1 100644
--- a/drivers/net/iavf/iavf_rxtx_vec_common.h
+++ b/drivers/net/iavf/iavf_rxtx_vec_common.h
@@ -186,12 +186,11 @@  _iavf_tx_queue_release_mbufs_vec(struct iavf_tx_queue *txq)
 		return;
 
 	i = txq->next_dd - txq->rs_thresh + 1;
-	if (txq->tx_tail < i) {
-		for (; i < txq->nb_tx_desc; i++) {
-			rte_pktmbuf_free_seg(txq->sw_ring[i].mbuf);
-			txq->sw_ring[i].mbuf = NULL;
-		}
-		i = 0;
+	while (i != txq->tx_tail) {
+		rte_pktmbuf_free_seg(txq->sw_ring[i].mbuf);
+		txq->sw_ring[i].mbuf = NULL;
+		if (++i == txq->nb_tx_desc)
+			i = 0;
 	}
 }