Re: [PATCH v3 05/36] net/intel: create separate header for Tx scalar fns

[Bruce Richardson <bruce.richardson@intel.com>]

On Fri, Feb 06, 2026 at 10:23:43AM +0000, Loftus, Ciara wrote:
> > Subject: [PATCH v3 05/36] net/intel: create separate header for Tx scalar fns
> > 
> > Rather than having all Tx code in the one file, which could start
> > getting rather long, move the scalar datapath functions to a new header
> > file.
> > 
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > ---
> >  drivers/net/intel/common/tx.h            | 58 ++------------------
> >  drivers/net/intel/common/tx_scalar_fns.h | 67
> > ++++++++++++++++++++++++
> >  2 files changed, 72 insertions(+), 53 deletions(-)
> >  create mode 100644 drivers/net/intel/common/tx_scalar_fns.h
> 
> Why not create the file when ci_tx_xmit_cleanup was first introduced?
> I prefer the naming tx_scalar.h but keep tx_scalar_fns.h if you feel it's better.
> 

Not pushed about the name, so happy with your suggestion. I'll try and
create this file back in patch 3 - some heavy rebasing will be necessary!

> > 
> > diff --git a/drivers/net/intel/common/tx.h b/drivers/net/intel/common/tx.h
> > index 03245d4fba..01e42303b4 100644
> > --- a/drivers/net/intel/common/tx.h
> > +++ b/drivers/net/intel/common/tx.h
> > @@ -319,59 +319,6 @@ 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 */
> > -	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(CI_TXD_QW1_DTYPE_M)) !=
> > -			rte_cpu_to_le_64(CI_TX_DESC_DTYPE_DESC_DONE))
> > {
> > -		/* 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)
> >  {
> > @@ -490,4 +437,9 @@ ci_tx_path_select(const struct ci_tx_path_features
> > *req_features,
> >  	return idx;
> >  }
> > 
> > +/* include the scalar functions at the end, so they can use the common
> > definitions.
> > + * This is done so drivers can use all functions just by including tx.h
> > + */
> > +#include "tx_scalar_fns.h"
> > +
> >  #endif /* _COMMON_INTEL_TX_H_ */
> > diff --git a/drivers/net/intel/common/tx_scalar_fns.h
> > b/drivers/net/intel/common/tx_scalar_fns.h
> > new file mode 100644
> > index 0000000000..c79210d084
> > --- /dev/null
> > +++ b/drivers/net/intel/common/tx_scalar_fns.h
> > @@ -0,0 +1,67 @@
> > +/* SPDX-License-Identifier: BSD-3-Clause
> > + * Copyright(c) 2025 Intel Corporation
> 
> Should this be 2026?
> 

Code was written in 2025 and actually submitted upstream for the first time
in December 2025, so I think this is correct as-is. [Open to correction]

> > + */
> > +
> > +#ifndef _COMMON_INTEL_TX_SCALAR_FNS_H_
> > +#define _COMMON_INTEL_TX_SCALAR_FNS_H_
> > +
> > +#include <stdint.h>
> > +#include <rte_byteorder.h>
> > +
> > +/* depends on common Tx definitions. */
> > +#include "tx.h"
> > +
> > +/*
> > + * Common transmit descriptor cleanup function for Intel drivers.
> > + * Used by ice, i40e, iavf, and idpf drivers.
> 
> Do we need to call out the driver names in the comment? If a new driver
> were to adopt this function they would need to patch this file too to
> update the comment.
> 

Good point, I'll try and generalize

/Bruce