On Thu, Oct 22, 2026 at 10:52:07AM +0530, Shaiq Wani wrote:
> Observed on ACC while running NVMe traffic tests: under sustained
> high-throughput split-queue Tx, payload bytes on the wire did not
> match what the application submitted, while headers and checksums
> looked valid.
>
> The RS-completion path freed mbufs by walking sw_ring[] slots between
> first_id and the EOP sw_id. RE reclaims and reuses those slots before
> RS lands, so the walker freed mbufs still in flight for later packets,
> whose buffers were then recycled and overwritten mid-DMA.
>
> Track completion ownership in a shadow ring indexed by a software-
> defined compl_tag written to all data descriptors of a packet (per the
> IDPF spec) and echoed back by HW on RS. The shadow ring is the sole
> owner of in-flight mbufs, so sw_ring[].mbuf is no longer touched and
> queue stop/release drain only the shadow ring. Size it to nb_tx_desc
> and stop Tx when the next compl_tag slot is still occupied, since RE
> runs ahead of RS and nb_tx_free does not bound pending completions.
> Allocate it at queue setup (failing on -ENOMEM) and free it on all
> setup-error and release paths.
>
> Remove the now unused ci_tx_entry.first_id field.
>
> Fixes: 96cc9b6ea60c ("net/idpf: fix multi-segment mbuf leak in split Tx path")
> Cc: [email protected]
>
> Signed-off-by: Shaiq Wani <[email protected]>
> ---
> v3: shrink ci_tx_queue and trim the fast path per review:
> * Drop the redundant tx_pending_size field (always nb_tx_desc).
> * Reuse the now-dead sw_tail slot as tx_next_compl_tag, so the shadow
> ring adds only one pointer to ci_tx_queue.
> * Remove the dead sw_ring[]/next_id walk from idpf_dp_splitq_xmit_pkts.
> Kept a dedicated shadow ring rather than sw_ring[tag].mbuf: sw_ring is
> indexed by descriptor and drained per-segment, so it cannot track
> per-packet compl_tag ownership of multi-seg chains.
>
> v2: no mbufs in sw_ring[] (shadow ring is sole owner); stop Tx on a
> pending compl_tag slot; -ENOMEM propagation and cleanup on all paths;
> same compl_tag on all data descs; shadow ring sized to nb_tx_desc.
> ---
> drivers/net/intel/common/tx.h | 8 +-
> drivers/net/intel/cpfl/cpfl_rxtx.c | 6 ++
> drivers/net/intel/idpf/idpf_common_rxtx.c | 119 ++++++++++++++--------
> drivers/net/intel/idpf/idpf_common_rxtx.h | 4 +
> drivers/net/intel/idpf/idpf_rxtx.c | 5 +
> 5 files changed, 99 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/net/intel/common/tx.h b/drivers/net/intel/common/tx.h
> index 2a63cd4330..65e8f3fc59 100644
> --- a/drivers/net/intel/common/tx.h
> +++ b/drivers/net/intel/common/tx.h
> @@ -166,7 +166,6 @@ 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.
> */
> };
>
> /**
> @@ -260,8 +259,13 @@ struct ci_tx_queue {
> struct idpf_complq *complq;
> uint32_t latch_idx; /* Tx timestamp latch index
> */
> uint16_t sw_nb_desc;
> - uint16_t sw_tail;
> uint16_t rs_compl_count;
> + /* Split-Tx completion tracking: shadow ring
> indexed by a
> + * rolling compl_tag decouples RS-completion
> mbuf lifetime
> + * from sw_ring[] slot reuse. Sized nb_tx_desc.
> + */
> + struct rte_mbuf **tx_pending_pkts;
>From my analysis, as I said in v2 comment, I think this can be put in the
union with sw_ring and sw_ring_vec, since for scalar splitq it replaces
those rings.
AI suggests the following diff on top of your patch to improve the memory
footprint situation, and reports "At 1,024 descriptors, ring storage drops
from 40 KiB to 8 KiB per split queue", which is a reasonable cache
footprint saving. This is untested, but including here to show the scope
of suggested changes.
/Bruce
---
diff --git a/drivers/net/intel/common/tx.h b/drivers/net/intel/common/tx.h
index 65e8f3fc59..00bf90181c 100644
--- a/drivers/net/intel/common/tx.h
+++ b/drivers/net/intel/common/tx.h
@@ -186,6 +186,8 @@ struct ci_tx_queue {
union {
struct ci_tx_entry *sw_ring; /* virtual address of SW ring */
struct ci_tx_entry_vec *sw_ring_vec;
+ /* IDPF split scalar: packet ownership indexed by completion
tag. */
+ struct rte_mbuf **tx_pending_pkts;
};
/* Scalar TX path: Array tracking last_id at each RS threshold boundary
*/
uint16_t *rs_last_id;
@@ -260,11 +262,6 @@ struct ci_tx_queue {
uint32_t latch_idx; /* Tx timestamp latch index
*/
uint16_t sw_nb_desc;
uint16_t rs_compl_count;
- /* Split-Tx completion tracking: shadow ring
indexed by a
- * rolling compl_tag decouples RS-completion
mbuf lifetime
- * from sw_ring[] slot reuse. Sized nb_tx_desc.
- */
- struct rte_mbuf **tx_pending_pkts;
uint16_t tx_next_compl_tag;
};
};
diff --git a/drivers/net/intel/cpfl/cpfl_rxtx.c
b/drivers/net/intel/cpfl/cpfl_rxtx.c
index 0dbe47d336..a358beca23 100644
--- a/drivers/net/intel/cpfl/cpfl_rxtx.c
+++ b/drivers/net/intel/cpfl/cpfl_rxtx.c
@@ -325,12 +325,12 @@ cpfl_tx_queue_release(void *txq)
q = &cpfl_txq->base;
if (q->complq) {
+ idpf_qc_split_tx_release_mbufs(q);
rte_memzone_free(q->complq->mz);
rte_free(q->complq);
- }
+ } else
+ ci_txq_release_all_mbufs(q, q->use_vec_entry);
- idpf_qc_split_tx_pending_free(q);
- ci_txq_release_all_mbufs(q, q->use_vec_entry);
rte_free(q->sw_ring);
rte_free(q->rs_last_id);
rte_memzone_free(q->mz);
@@ -539,7 +539,6 @@ cpfl_tx_queue_setup(struct rte_eth_dev *dev, uint16_t
queue_idx,
const struct rte_memzone *mz;
struct ci_tx_queue *txq;
uint64_t offloads;
- uint16_t len;
bool is_splitq;
int ret;
@@ -584,11 +583,7 @@ cpfl_tx_queue_setup(struct rte_eth_dev *dev, uint16_t
queue_idx,
txq->offloads = cpfl_tx_offload_convert(offloads);
txq->tx_deferred_start = tx_conf->tx_deferred_start;
- if (is_splitq)
- len = 2 * nb_desc;
- else
- len = nb_desc;
- txq->sw_nb_desc = len;
+ txq->sw_nb_desc = nb_desc;
/* Allocate TX hardware ring descriptors. */
mz = cpfl_dma_zone_reserve(dev, queue_idx, nb_desc,
VIRTCHNL2_QUEUE_TYPE_TX,
@@ -600,34 +595,34 @@ cpfl_tx_queue_setup(struct rte_eth_dev *dev, uint16_t
queue_idx,
txq->tx_ring_dma = mz->iova;
txq->mz = mz;
- txq->sw_ring = rte_zmalloc_socket("cpfl tx sw ring",
- sizeof(struct ci_tx_entry) * len,
- RTE_CACHE_LINE_SIZE, socket_id);
- if (txq->sw_ring == NULL) {
- PMD_INIT_LOG(ERR, "Failed to allocate memory for SW TX ring");
- ret = -ENOMEM;
- goto err_sw_ring_alloc;
- }
+ if (!is_splitq) {
+ txq->sw_ring = rte_zmalloc_socket("cpfl tx sw ring",
+ sizeof(struct ci_tx_entry) * nb_desc,
+ RTE_CACHE_LINE_SIZE, socket_id);
+ if (txq->sw_ring == NULL) {
+ PMD_INIT_LOG(ERR, "Failed to allocate memory for SW TX
ring");
+ ret = -ENOMEM;
+ goto err_sw_ring_alloc;
+ }
- /* Allocate RS last_id tracking array */
- uint16_t num_rs_buckets = nb_desc / tx_rs_thresh;
- txq->rs_last_id = rte_zmalloc_socket("cpfl tx rs_last_id",
- sizeof(txq->rs_last_id[0]) * num_rs_buckets,
- RTE_CACHE_LINE_SIZE, socket_id);
- if (txq->rs_last_id == NULL) {
- PMD_INIT_LOG(ERR, "Failed to allocate memory for RS last_id
array");
- ret = -ENOMEM;
- goto err_rs_last_id_alloc;
- }
+ /* Allocate RS last_id tracking array */
+ const uint16_t num_rs_buckets = nb_desc / tx_rs_thresh;
+ txq->rs_last_id = rte_zmalloc_socket("cpfl tx rs_last_id",
+ sizeof(txq->rs_last_id[0]) * num_rs_buckets,
+ RTE_CACHE_LINE_SIZE, socket_id);
+ if (txq->rs_last_id == NULL) {
+ PMD_INIT_LOG(ERR, "Failed to allocate memory for RS
last_id array");
+ ret = -ENOMEM;
+ goto err_rs_last_id_alloc;
+ }
- if (!is_splitq) {
txq->ci_tx_ring = mz->addr;
idpf_qc_single_tx_queue_reset(txq);
} else {
txq->desc_ring = mz->addr;
ret = idpf_qc_split_tx_pending_alloc(txq, socket_id);
if (ret != 0)
- goto err_pending_alloc;
+ goto err_sw_ring_alloc;
idpf_qc_split_tx_descq_reset(txq);
/* Setup tx completion queue if split model */
@@ -646,8 +641,6 @@ cpfl_tx_queue_setup(struct rte_eth_dev *dev, uint16_t
queue_idx,
return 0;
err_complq_setup:
- idpf_qc_split_tx_pending_free(txq);
-err_pending_alloc:
rte_free(txq->rs_last_id);
err_rs_last_id_alloc:
rte_free(txq->sw_ring);
@@ -1371,8 +1364,8 @@ cpfl_tx_queue_stop(struct rte_eth_dev *dev, uint16_t
tx_queue_id)
}
txq = &cpfl_txq->base;
- ci_txq_release_all_mbufs(txq, txq->use_vec_entry);
if (vport->txq_model == VIRTCHNL2_QUEUE_MODEL_SINGLE) {
+ ci_txq_release_all_mbufs(txq, txq->use_vec_entry);
idpf_qc_single_tx_queue_reset(txq);
} else {
if (cpfl_txq->hairpin_info.hairpin_q) {
@@ -1551,14 +1544,12 @@ cpfl_set_tx_function(struct rte_eth_dev *dev)
IDPF_TX_DEFAULT,
false);
- /* Set use_vec_entry for single queue mode - only IDPF_TX_SINGLEQ uses
regular entries */
- if (vport->txq_model == VIRTCHNL2_QUEUE_MODEL_SINGLE) {
- for (i = 0; i < dev->data->nb_tx_queues; i++) {
- txq = dev->data->tx_queues[i];
- if (txq == NULL)
- continue;
- txq->use_vec_entry = (ad->tx_func_type !=
IDPF_TX_SINGLEQ);
- }
+ for (i = 0; i < dev->data->nb_tx_queues; i++) {
+ txq = dev->data->tx_queues[i];
+ if (txq == NULL)
+ continue;
+ txq->use_vec_entry = (ad->tx_func_type != IDPF_TX_SINGLEQ &&
+ ad->tx_func_type != IDPF_TX_DEFAULT);
}
out:
diff --git a/drivers/net/intel/idpf/idpf_common_rxtx.c
b/drivers/net/intel/idpf/idpf_common_rxtx.c
index 496900b697..eb29024547 100644
--- a/drivers/net/intel/idpf/idpf_common_rxtx.c
+++ b/drivers/net/intel/idpf/idpf_common_rxtx.c
@@ -202,9 +202,7 @@ RTE_EXPORT_INTERNAL_SYMBOL(idpf_qc_split_tx_descq_reset)
void
idpf_qc_split_tx_descq_reset(struct ci_tx_queue *txq)
{
- struct ci_tx_entry *txe;
uint32_t i, size;
- uint16_t prev;
if (txq == NULL) {
DRV_LOG(DEBUG, "Pointer to txq is NULL");
@@ -215,13 +213,7 @@ idpf_qc_split_tx_descq_reset(struct ci_tx_queue *txq)
for (i = 0; i < size; i++)
((volatile char *)txq->desc_ring)[i] = 0;
- txe = txq->sw_ring;
- prev = (uint16_t)(txq->sw_nb_desc - 1);
- for (i = 0; i < txq->sw_nb_desc; i++) {
- txe[i].mbuf = NULL;
- txe[prev].next_id = i;
- prev = i;
- }
+ idpf_qc_split_tx_release_mbufs(txq);
txq->tx_tail = 0;
@@ -233,14 +225,6 @@ idpf_qc_split_tx_descq_reset(struct ci_tx_queue *txq)
txq->tx_next_dd = txq->tx_rs_thresh - 1;
txq->tx_next_rs = txq->tx_rs_thresh - 1;
- if (txq->tx_pending_pkts != NULL) {
- for (i = 0; i < txq->nb_tx_desc; i++) {
- if (txq->tx_pending_pkts[i] != NULL) {
- rte_pktmbuf_free(txq->tx_pending_pkts[i]);
- txq->tx_pending_pkts[i] = NULL;
- }
- }
- }
txq->tx_next_compl_tag = 0;
}
@@ -248,7 +232,7 @@ RTE_EXPORT_INTERNAL_SYMBOL(idpf_qc_split_tx_pending_alloc)
int
idpf_qc_split_tx_pending_alloc(struct ci_tx_queue *txq, unsigned int socket_id)
{
- /* Bounding pending RS to nb_tx_desc keeps the 2 * nb_tx_desc complq
from overflowing. */
+ /* Scalar completion tags and vector descriptor indices both use
nb_tx_desc pointers. */
txq->tx_pending_pkts = rte_zmalloc_socket("idpf_tx_pending",
sizeof(struct rte_mbuf *) * txq->nb_tx_desc,
RTE_CACHE_LINE_SIZE, socket_id);
@@ -261,21 +245,24 @@ idpf_qc_split_tx_pending_alloc(struct ci_tx_queue *txq,
unsigned int socket_id)
return 0;
}
-RTE_EXPORT_INTERNAL_SYMBOL(idpf_qc_split_tx_pending_free)
+RTE_EXPORT_INTERNAL_SYMBOL(idpf_qc_split_tx_release_mbufs)
void
-idpf_qc_split_tx_pending_free(struct ci_tx_queue *txq)
+idpf_qc_split_tx_release_mbufs(struct ci_tx_queue *txq)
{
- uint32_t i;
-
if (txq->tx_pending_pkts == NULL)
return;
- for (i = 0; i < txq->nb_tx_desc; i++) {
+ if (txq->use_vec_entry) {
+ ci_txq_release_all_mbufs(txq, false);
+ return;
+ }
+
+ /* Scalar completion tags can outlive descriptor reuse, so drain every
slot. */
+ for (uint16_t i = 0; i < txq->nb_tx_desc; i++) {
if (txq->tx_pending_pkts[i] != NULL)
rte_pktmbuf_free(txq->tx_pending_pkts[i]);
+ txq->tx_pending_pkts[i] = NULL;
}
- rte_free(txq->tx_pending_pkts);
- txq->tx_pending_pkts = NULL;
}
RTE_EXPORT_INTERNAL_SYMBOL(idpf_qc_split_tx_complq_reset)
@@ -426,12 +413,12 @@ idpf_qc_tx_queue_release(void *txq)
return;
if (q->complq) {
+ idpf_qc_split_tx_release_mbufs(q);
rte_memzone_free(q->complq->mz);
rte_free(q->complq);
- }
+ } else
+ ci_txq_release_all_mbufs(q, false);
- idpf_qc_split_tx_pending_free(q);
- ci_txq_release_all_mbufs(q, false);
rte_free(q->rs_last_id);
rte_free(q->sw_ring);
rte_memzone_free(q->mz);
diff --git a/drivers/net/intel/idpf/idpf_common_rxtx.h
b/drivers/net/intel/idpf/idpf_common_rxtx.h
index 3365631d80..a4843c9f69 100644
--- a/drivers/net/intel/idpf/idpf_common_rxtx.h
+++ b/drivers/net/intel/idpf/idpf_common_rxtx.h
@@ -218,7 +218,7 @@ void idpf_qc_split_tx_descq_reset(struct ci_tx_queue *txq);
__rte_internal
int idpf_qc_split_tx_pending_alloc(struct ci_tx_queue *txq, unsigned int
socket_id);
__rte_internal
-void idpf_qc_split_tx_pending_free(struct ci_tx_queue *txq);
+void idpf_qc_split_tx_release_mbufs(struct ci_tx_queue *txq);
__rte_internal
void idpf_qc_split_tx_complq_reset(struct idpf_complq *cq);
__rte_internal
diff --git a/drivers/net/intel/idpf/idpf_rxtx.c
b/drivers/net/intel/idpf/idpf_rxtx.c
index 0d41cd87eb..3bee388c17 100644
--- a/drivers/net/intel/idpf/idpf_rxtx.c
+++ b/drivers/net/intel/idpf/idpf_rxtx.c
@@ -413,7 +413,6 @@ idpf_tx_queue_setup(struct rte_eth_dev *dev, uint16_t
queue_idx,
const struct rte_memzone *mz;
struct ci_tx_queue *txq;
uint64_t offloads;
- uint16_t len;
bool is_splitq;
int ret;
@@ -456,11 +455,7 @@ idpf_tx_queue_setup(struct rte_eth_dev *dev, uint16_t
queue_idx,
txq->offloads = idpf_tx_offload_convert(offloads);
txq->tx_deferred_start = tx_conf->tx_deferred_start;
- if (is_splitq)
- len = 2 * nb_desc;
- else
- len = nb_desc;
- txq->sw_nb_desc = len;
+ txq->sw_nb_desc = nb_desc;
/* Allocate TX hardware ring descriptors. */
mz = idpf_dma_zone_reserve(dev, queue_idx, nb_desc,
VIRTCHNL2_QUEUE_TYPE_TX,
@@ -472,32 +467,32 @@ idpf_tx_queue_setup(struct rte_eth_dev *dev, uint16_t
queue_idx,
txq->tx_ring_dma = mz->iova;
txq->mz = mz;
- txq->sw_ring = rte_zmalloc_socket("idpf tx sw ring",
- sizeof(struct ci_tx_entry) * len,
- RTE_CACHE_LINE_SIZE, socket_id);
- if (txq->sw_ring == NULL) {
- PMD_INIT_LOG(ERR, "Failed to allocate memory for SW TX ring");
- ret = -ENOMEM;
- goto err_sw_ring_alloc;
- }
+ if (!is_splitq) {
+ txq->sw_ring = rte_zmalloc_socket("idpf tx sw ring",
+ sizeof(struct ci_tx_entry) * nb_desc,
+ RTE_CACHE_LINE_SIZE, socket_id);
+ if (txq->sw_ring == NULL) {
+ PMD_INIT_LOG(ERR, "Failed to allocate memory for SW TX
ring");
+ ret = -ENOMEM;
+ goto err_sw_ring_alloc;
+ }
- txq->rs_last_id = rte_zmalloc_socket("idpf tx rs_last_id",
- sizeof(txq->rs_last_id[0]) * (nb_desc >>
txq->log2_rs_thresh),
- RTE_CACHE_LINE_SIZE, socket_id);
- if (txq->rs_last_id == NULL) {
- PMD_INIT_LOG(ERR, "Failed to allocate memory for TX RS
tracking");
- ret = -ENOMEM;
- goto err_rs_last_id_alloc;
- }
+ txq->rs_last_id = rte_zmalloc_socket("idpf tx rs_last_id",
+ sizeof(txq->rs_last_id[0]) * (nb_desc >>
txq->log2_rs_thresh),
+ RTE_CACHE_LINE_SIZE, socket_id);
+ if (txq->rs_last_id == NULL) {
+ PMD_INIT_LOG(ERR, "Failed to allocate memory for TX RS
tracking");
+ ret = -ENOMEM;
+ goto err_rs_last_id_alloc;
+ }
- if (!is_splitq) {
txq->ci_tx_ring = mz->addr;
idpf_qc_single_tx_queue_reset(txq);
} else {
txq->desc_ring = mz->addr;
ret = idpf_qc_split_tx_pending_alloc(txq, socket_id);
if (ret != 0)
- goto err_pending_alloc;
+ goto err_sw_ring_alloc;
idpf_qc_split_tx_descq_reset(txq);
/* Setup tx completion queue if split model */
@@ -515,8 +510,6 @@ idpf_tx_queue_setup(struct rte_eth_dev *dev, uint16_t
queue_idx,
return 0;
err_complq_setup:
- idpf_qc_split_tx_pending_free(txq);
-err_pending_alloc:
rte_free(txq->rs_last_id);
err_rs_last_id_alloc:
rte_free(txq->sw_ring);
@@ -738,8 +731,8 @@ idpf_tx_queue_stop(struct rte_eth_dev *dev, uint16_t
tx_queue_id)
}
txq = dev->data->tx_queues[tx_queue_id];
- ci_txq_release_all_mbufs(txq, false);
if (vport->txq_model == VIRTCHNL2_QUEUE_MODEL_SINGLE) {
+ ci_txq_release_all_mbufs(txq, false);
idpf_qc_single_tx_queue_reset(txq);
} else {
idpf_qc_split_tx_descq_reset(txq);
@@ -895,14 +888,12 @@ idpf_set_tx_function(struct rte_eth_dev *dev)
IDPF_TX_DEFAULT,
false);
- /* Set use_vec_entry for single queue mode - only IDPF_TX_SINGLEQ uses
regular entries */
- if (vport->txq_model == VIRTCHNL2_QUEUE_MODEL_SINGLE) {
- for (i = 0; i < dev->data->nb_tx_queues; i++) {
- txq = dev->data->tx_queues[i];
- if (txq == NULL)
- continue;
- txq->use_vec_entry = (ad->tx_func_type !=
IDPF_TX_SINGLEQ);
- }
+ for (i = 0; i < dev->data->nb_tx_queues; i++) {
+ txq = dev->data->tx_queues[i];
+ if (txq == NULL)
+ continue;
+ txq->use_vec_entry = (ad->tx_func_type != IDPF_TX_SINGLEQ &&
+ ad->tx_func_type != IDPF_TX_DEFAULT);
}
out:
