|SUCCESS| [v3] net/idpf: fix multi-segment mbuf leak in split Tx

[dpdklab at iol.unh.edu]

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

_AI Code Review_

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

This patch addresses a resource leak in the split queue TX path for
multi-segment mbufs, where only the EOP mbuf was freed on RS completion,
leaking preceding segments.

commit 8c6098afa075 ("common/idpf: add Rx/Tx data path")
Author: Shaiq Wani <shaiq.wani at intel.com>

net/idpf: fix multi-segment mbuf leak in split Tx

RS completion only frees the EOP entry's mbuf, leaking preceding
segments of multi-segment packets. Add a first_id field to the Tx
entry struct, record the first sw_id at the EOP entry during submit,
and walk forward from first to EOP to free all segments on completion.

> diff --git a/drivers/net/intel/common/tx.h b/drivers/net/intel/common/tx.h
> index 283bd58d5d..53e61c0b62 100644
> --- a/drivers/net/intel/common/tx.h
> +++ b/drivers/net/intel/common/tx.h
> @@ -121,6 +121,7 @@ struct ci_tx_queue;
>  struct ci_tx_entry {
>  	struct rte_mbuf *mbuf; /* mbuf associated with TX desc, if any. */
>  	uint16_t next_id; /* Index of next descriptor in ring. */
> +	uint16_t first_id; /* Split-queue: first sw_id of packet at EOP entry. */
>  };

[ ... ]

> diff --git a/drivers/net/intel/idpf/idpf_common_rxtx.c b/drivers/net/intel/idpf/idpf_common_rxtx.c
> index f73716e57c..c56ba86f1e 100644
> --- a/drivers/net/intel/idpf/idpf_common_rxtx.c
> +++ b/drivers/net/intel/idpf/idpf_common_rxtx.c

[ ... ]

> @@ -979,6 +991,8 @@ idpf_dp_splitq_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
>  				tx_id = 0;
>  		}
>  
> +		uint16_t first_sw_id = sw_id;
> +

Per the DPDK coding style guide, local variable declarations must be
placed at the beginning of the block, before any statements. Can this
declaration of first_sw_id be moved to the top of idpf_dp_splitq_xmit_pkts()
or the start of the enclosing block?

>  		do {
>  			txd = &txr[tx_id];
>  			txn = &sw_ring[txe->next_id];
> @@ -1002,6 +1016,9 @@ idpf_dp_splitq_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
>  		/* fill the last descriptor with End of Packet (EOP) bit */
>  		txd->qw1.cmd_dtype |= IDPF_TXD_FLEX_FLOW_CMD_EOP;
>  
> +		/* Record first sw_id at EOP so completion can walk forward. */
> +		sw_ring[txd->qw1.compl_tag].first_id = first_sw_id;
> +

In idpf_dp_splitq_xmit_pkts(), is the value of txd->qw1.compl_tag
guaranteed to be valid before first_id is assigned? Does the do-while loop
above set compl_tag for all descriptors, or only for the final one?

>  		txq->nb_tx_free = (uint16_t)(txq->nb_tx_free - nb_used);
>  		txq->rs_compl_count += nb_used;