Re: [PATCH v3 03/36] net/intel: create common post-Tx cleanup function

Bruce Richardson
2026-02-09T10:42:09Z

On Fri, Feb 06, 2026 at 10:07:32AM +0000, Loftus, Ciara wrote:
>
>
> > -----Original Message-----
> > From: Bruce Richardson <bruce.richardson@intel.com>
> > Sent: Friday 30 January 2026 11:42
> > To: dev@dpdk.org
> > Cc: Richardson, Bruce <bruce.richardson@intel.com>; Medvedkin, Vladimir
> > <vladimir.medvedkin@intel.com>; Burakov, Anatoly
> > <anatoly.burakov@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Shetty,
> > Praveen <praveen.shetty@intel.com>
> > Subject: [PATCH v3 03/36] net/intel: create common post-Tx cleanup function
> >
> > The code used in ice, iavf, idpf and i40e for doing cleanup of mbufs
> > after they had been transmitted was identical. Therefore deduplicate it
> > by moving to common and remove the driver-specific versions.
> >
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > ---
> > drivers/net/intel/common/tx.h | 53 ++++++++++++++++++++
> > drivers/net/intel/i40e/i40e_rxtx.c | 49 ++----------------
> > drivers/net/intel/iavf/iavf_rxtx.c | 50 ++-----------------
> > drivers/net/intel/ice/ice_rxtx.c | 60 ++---------------------
> > drivers/net/intel/idpf/idpf_common_rxtx.c | 46 ++---------------
> > 5 files changed, 71 insertions(+), 187 deletions(-)
> >
> > diff --git a/drivers/net/intel/common/tx.h b/drivers/net/intel/common/tx.h
> > index 8cf63e59ab..a89412c195 100644
> > --- a/drivers/net/intel/common/tx.h
> > +++ b/drivers/net/intel/common/tx.h
> > @@ -259,6 +259,59 @@ ci_tx_free_bufs_vec(struct ci_tx_queue *txq,
> > ci_desc_done_fn desc_done, bool ctx
> > return txq->tx_rs_thresh;
> > }
> >
> > +/*
> > + * Common transmit descriptor cleanup function for Intel drivers.
> > + * Used by ice, i40e, iavf, and idpf drivers.
> > + *
> > + * Returns:
> > + * 0 on success
> > + * -1 if cleanup cannot proceed (descriptors not yet processed by HW)
> > + */
> > +static __rte_always_inline int
> > +ci_tx_xmit_cleanup(struct ci_tx_queue *txq)
> > +{
> > + struct ci_tx_entry *sw_ring = txq->sw_ring;
> > + volatile struct ci_tx_desc *txd = txq->ci_tx_ring;
> > + uint16_t last_desc_cleaned = txq->last_desc_cleaned;
> > + uint16_t nb_tx_desc = txq->nb_tx_desc;
> > + uint16_t desc_to_clean_to;
> > + uint16_t nb_tx_to_clean;
> > +
> > + /* Determine the last descriptor needing to be cleaned */
> > + desc_to_clean_to = (uint16_t)(last_desc_cleaned + txq->tx_rs_thresh);
> > + if (desc_to_clean_to >= nb_tx_desc)
> > + desc_to_clean_to = (uint16_t)(desc_to_clean_to -
> > nb_tx_desc);
> > +
> > + /* Check to make sure the last descriptor to clean is done */
>
> This comment is similar to the next one. Maybe merge them?
>
Ack

> > + desc_to_clean_to = sw_ring[desc_to_clean_to].last_id;
> > +
> > + /* Check if descriptor is done - all drivers use 0xF as done value in bits
> > 3:0 */
> > + if ((txd[desc_to_clean_to].cmd_type_offset_bsz &
> > rte_cpu_to_le_64(0xFUL)) !=
> > + rte_cpu_to_le_64(0xFUL)) {
> > + /* Descriptor not yet processed by hardware */
> > + return -1;
> > + }
> > +
> > + /* Figure out how many descriptors will be cleaned */
> > + if (last_desc_cleaned > desc_to_clean_to)
> > + nb_tx_to_clean = (uint16_t)((nb_tx_desc - last_desc_cleaned)
> > + desc_to_clean_to);
> > + else
> > + nb_tx_to_clean = (uint16_t)(desc_to_clean_to -
> > last_desc_cleaned);
> > +
> > + /* The last descriptor to clean is done, so that means all the
> > + * descriptors from the last descriptor that was cleaned
> > + * up to the last descriptor with the RS bit set
> > + * are done. Only reset the threshold descriptor.
> > + */
> > + txd[desc_to_clean_to].cmd_type_offset_bsz = 0;
> > +
> > + /* Update the txq to reflect the last descriptor that was cleaned */
> > + txq->last_desc_cleaned = desc_to_clean_to;
> > + txq->nb_tx_free = (uint16_t)(txq->nb_tx_free + nb_tx_to_clean);
> > +
> > + return 0;
> > +}
> > +
> > static inline void
> > ci_txq_release_all_mbufs(struct ci_tx_queue *txq, bool use_ctx)
> > {
> > diff --git a/drivers/net/intel/i40e/i40e_rxtx.c
> > b/drivers/net/intel/i40e/i40e_rxtx.c
> > index 210fc0201e..2760e76e99 100644
> > --- a/drivers/net/intel/i40e/i40e_rxtx.c
> > +++ b/drivers/net/intel/i40e/i40e_rxtx.c
> > @@ -384,45 +384,6 @@ i40e_build_ctob(uint32_t td_cmd,
> > ((uint64_t)td_tag <<
> > I40E_TXD_QW1_L2TAG1_SHIFT));
> > }
> >
> > -static inline int
> > -i40e_xmit_cleanup(struct ci_tx_queue *txq)
> > -{
> > - struct ci_tx_entry *sw_ring = txq->sw_ring;
> > - volatile struct ci_tx_desc *txd = txq->ci_tx_ring;
> > - uint16_t last_desc_cleaned = txq->last_desc_cleaned;
> > - uint16_t nb_tx_desc = txq->nb_tx_desc;
> > - uint16_t desc_to_clean_to;
> > - uint16_t nb_tx_to_clean;
> > -
> > - desc_to_clean_to = (uint16_t)(last_desc_cleaned + txq->tx_rs_thresh);
> > - if (desc_to_clean_to >= nb_tx_desc)
> > - desc_to_clean_to = (uint16_t)(desc_to_clean_to -
> > nb_tx_desc);
> > -
> > - desc_to_clean_to = sw_ring[desc_to_clean_to].last_id;
> > - if ((txd[desc_to_clean_to].cmd_type_offset_bsz &
> > - rte_cpu_to_le_64(I40E_TXD_QW1_DTYPE_MASK)) !=
> > -
> > rte_cpu_to_le_64(I40E_TX_DESC_DTYPE_DESC_DONE)) {
> > - PMD_TX_LOG(DEBUG, "TX descriptor %4u is not done "
> > - "(port=%d queue=%d)", desc_to_clean_to,
> > - txq->port_id, txq->queue_id);
>
> These logs are lost in each of the drivers. I'm not sure if they're terribly
> helpful though so I think it's fine that they're dropped.
>
Yes, I don't think they are that helpful either. However, I was in
discussion with Anatoly about how we might look to do logging from the
common Intel code, so we may look to add them back in future if we see the
need.

/Bruce