Re: [PATCH 01/13] net/intel: introduce infrastructure for Tx path selection

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

On Tue, Dec 09, 2025 at 11:26:40AM +0000, Ciara Loftus wrote:
> The code for determining which Tx path to select during initialisation
> has become complicated in many intel drivers due to the amount of
> different paths and features available within each path. This commit
> aims to simplify and genericize the path selection logic.
> 
> The following information about each Tx burst function is stored and
> used by the new common function to select the appropriate Tx path:
> - Tx Offloads
> - SIMD bitwidth
> 
> The implementation is based off the Rx path selection infrastructure
> introduced in a previous commit.
> 
> Signed-off-by: Ciara Loftus <ciara.loftus@intel.com>

Generally looks good, a few minor suggestions inline below.

Acked-by: Bruce Richardson <bruce.richardson@intel.com>


> ---
>  drivers/net/intel/common/tx.h | 71 +++++++++++++++++++++++++++++++++++
>  1 file changed, 71 insertions(+)
> 
> diff --git a/drivers/net/intel/common/tx.h b/drivers/net/intel/common/tx.h
> index 5af64a4cfe..c6c1904ba3 100644
> --- a/drivers/net/intel/common/tx.h
> +++ b/drivers/net/intel/common/tx.h
> @@ -8,6 +8,7 @@
>  #include <stdint.h>
>  #include <rte_mbuf.h>
>  #include <rte_ethdev.h>
> +#include <rte_vect.h>
>  
>  /* forward declaration of the common intel (ci) queue structure */
>  struct ci_tx_queue;
> @@ -117,6 +118,17 @@ struct ci_tx_queue {
>  	};
>  };
>  
> +struct ci_tx_path_features {
> +	uint32_t tx_offloads;
> +	enum rte_vect_max_simd simd_width;
> +};
> +
> +struct ci_tx_path_info {
> +	eth_tx_burst_t pkt_burst;
> +	const char *info;
> +	struct ci_tx_path_features features;
> +};
> +
>  static __rte_always_inline void
>  ci_tx_backlog_entry(struct ci_tx_entry *txep, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
>  {
> @@ -262,4 +274,63 @@ ci_txq_release_all_mbufs(struct ci_tx_queue *txq, bool use_ctx)
>  	memset(txq->sw_ring_vec, 0, sizeof(txq->sw_ring_vec[0]) * nb_desc);
>  }
>  
> +/**
> + * Select the best matching Tx path based on features
> + *
> + * @param req_features
> + *   The requested features for the Tx path
> + * @param infos
> + *   Array of information about the available Tx paths
> + * @param num_paths
> + *   Number of available paths in the infos array
> + * @param default_path
> + *   Index of the default path to use if no suitable path is found
> + *
> + * @return
> + *   The packet burst function index that best matches the requested features,
> + *   or default_path if no suitable path is found
> + */
> +static inline int
> +ci_tx_path_select(struct ci_tx_path_features req_features,

Is there a reason we are passing this by value, rather than as a const
pointer?

> +			const struct ci_tx_path_info *infos,
> +			int num_paths,

Minor nit: general the size of arrays should be of type "size_t", which
matches the value got from sizeof, and hence RTE_DIM. Since we don't need
to support negative sized arrays, an unsigned type is better generally.

> +			int default_path)
> +{
> +	int i, idx = default_path;
> +	const struct ci_tx_path_features *chosen_path_features = NULL;
> +
> +	for (i = 0; i < num_paths; i++) {

If num_paths is size_t, "i" should be an unsigned value to match. I'd also
to prefer that "i" also be defined inside the "for" statement, since its
unneeded outside of it.

> +		const struct ci_tx_path_features *path_features = &infos[i].features;
> +
> +		/* Ensure the path supports the requested TX offloads. */
> +		if ((path_features->tx_offloads & req_features.tx_offloads) !=
> +				req_features.tx_offloads)
> +			continue;
> +
> +		/* Ensure the path's SIMD width is compatible with the requested width. */
> +		if (path_features->simd_width > req_features.simd_width)
> +			continue;
> +
> +		/* Do not select the path if it is less suitable than the chosen path. */
> +		if (chosen_path_features != NULL) {
> +			/* Do not select paths with lower SIMD width than the chosen path. */
> +			if (path_features->simd_width < chosen_path_features->simd_width)
> +				continue;
> +			/* Do not select paths with more offloads enabled than the chosen path if
> +			 * the SIMD widths are the same.
> +			 */
> +			if (path_features->simd_width == chosen_path_features->simd_width &&
> +					rte_popcount32(path_features->tx_offloads) >
> +					rte_popcount32(chosen_path_features->tx_offloads))
> +				continue;
> +		}
> +
> +		/* Finally, select the path since it has met all the requirements. */
> +		idx = i;
> +		chosen_path_features = &infos[idx].features;
> +	}
> +
> +	return idx;
> +}
> +
>  #endif /* _COMMON_INTEL_TX_H_ */
> -- 
> 2.43.0
>