[v2,2/4] net/iavf: fix buffer leak on Tx queue stop
Checks
Commit Message
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
> -----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>
> -----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
@@ -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;
}
}
@@ -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;
}
}