|SUCCESS| net/mana: fix RX mempool leak on port stop

[dpdklab at iol.unh.edu]

Test-Label: ai-code-review
Test-Status: SUCCESS
https://dpdk.org/patch/162956

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-04-09

Reviewing the DPDK patch for net/mana: fix RX mempool leak on port stop.

Commit: 20260409015519.350034-1-longli at microsoft.com
Author: Long Li <longli at microsoft.com>
Subject: net/mana: fix RX mempool leak on port stop

This patch fixes a resource leak in the mana driver by adding a
desc_ring_len counter to properly drain RX and TX rings on port stop.

>  drivers/net/mana/mana.h | 2 +-
>  drivers/net/mana/rx.c   | 6 +++++-
>  drivers/net/mana/tx.c   | 2 +-
>  3 files changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/mana/mana.h b/drivers/net/mana/mana.h
> index 7d94840dc4..79cc47b6ab 100644
> --- a/drivers/net/mana/mana.h
> +++ b/drivers/net/mana/mana.h
> @@ -443,7 +443,7 @@ struct mana_rxq {
>  	/* desc_ring_head is where we put pending requests to ring,
>  	 * completion pull off desc_ring_tail
>  	 */
> -	uint32_t desc_ring_head, desc_ring_tail;
> +	uint32_t desc_ring_head, desc_ring_tail, desc_ring_len;

Can the desc_ring_len overflow during normal operation? With
desc_ring_head incrementing in mana_post_rx_wqe() and desc_ring_len
incrementing with each post, does this code handle the scenario where
desc_ring_len exceeds num_desc due to rapid posting without consumption?

> diff --git a/drivers/net/mana/rx.c b/drivers/net/mana/rx.c
> index f196d43aee..1b8ba1f3a9 100644
> --- a/drivers/net/mana/rx.c
> +++ b/drivers/net/mana/rx.c
> @@ -102,6 +102,7 @@ mana_post_rx_wqe(struct mana_rxq *rxq, struct rte_mbuf *mbuf)
>  		rxq->wqe_cnt_to_short_db += wqe_size_in_bu;
>  #endif
>  		rxq->desc_ring_head = (rxq->desc_ring_head + 1) % rxq->num_desc;
> +		rxq->desc_ring_len++;
>  	} else {
>  		DP_LOG(DEBUG, "failed to post recv ret %d", ret);
>  		return ret;

Does desc_ring_len need to be decremented when mana_post_rx_wqe() fails
and returns an error? The mbuf may already be attached to the descriptor
ring but desc_ring_len wasn't incremented, potentially causing a
mismatch between the actual ring state and the counter.

[ ... ]

> @@ -560,6 +563,7 @@ mana_rx_burst(void *dpdk_rxq, struct rte_mbuf **pkts, uint16_t pkts_n)
>  			rxq->desc_ring_tail = 0;
>  
>  		rxq->gdma_rq.tail += desc->wqe_size_in_bu;
> +		rxq->desc_ring_len--;

Is desc_ring_len properly protected from concurrent access? The RX burst
function decrements desc_ring_len while mana_post_rx_wqe() increments it.
Does this code need synchronization between the RX path and any other
threads that might call mana_post_rx_wqe()?

> diff --git a/drivers/net/mana/tx.c b/drivers/net/mana/tx.c
> index e5ab566e8a..57dbbc3651 100644
> --- a/drivers/net/mana/tx.c
> +++ b/drivers/net/mana/tx.c
> @@ -39,7 +39,7 @@ mana_stop_tx_queues(struct rte_eth_dev *dev)
>  		}
>  
>  		/* Drain and free posted WQEs */
> -		while (txq->desc_ring_tail != txq->desc_ring_head) {
> +		while (txq->desc_ring_len > 0) {
>  			struct mana_txq_desc *desc =
>  				&txq->desc_ring[txq->desc_ring_tail];

Does the TX path properly maintain desc_ring_len throughout the existing
code? The patch modifies the drain condition but doesn't show the
corresponding increment/decrement logic in the TX path. Is desc_ring_len
already being maintained correctly in the TX code paths?