Re: [PATCH 2/4] net/idpf: use the common Rx path selection infrastructure

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

On Thu, Sep 11, 2025 at 02:31:43PM +0000, Ciara Loftus wrote:
> Update the common rx path selection infrastructure to include the
> feature "single queue" which is relevant for the idpf and cpfl drivers.
> Replace the existing complicated logic in the idpf driver with the use
> of the common function.
> 
> Signed-off-by: Ciara Loftus <ciara.loftus@intel.com>
> ---
>  drivers/net/intel/common/rx.h               |   5 +
>  drivers/net/intel/idpf/idpf_common_device.h |  12 +++
>  drivers/net/intel/idpf/idpf_common_rxtx.c   |  24 +++++
>  drivers/net/intel/idpf/idpf_common_rxtx.h   |  12 +++
>  drivers/net/intel/idpf/idpf_ethdev.c        |   2 +
>  drivers/net/intel/idpf/idpf_rxtx.c          | 103 ++++++--------------
>  6 files changed, 83 insertions(+), 75 deletions(-)
> 
> diff --git a/drivers/net/intel/common/rx.h b/drivers/net/intel/common/rx.h
> index 770284f7ab..741808f573 100644
> --- a/drivers/net/intel/common/rx.h
> +++ b/drivers/net/intel/common/rx.h
> @@ -131,6 +131,7 @@ struct ci_rx_path_features_extra {
>  	bool flex_desc;
>  	bool bulk_alloc;
>  	bool disabled;
> +	bool single_queue;
>  };
>  
>  struct ci_rx_path_features {
> @@ -278,6 +279,10 @@ ci_rx_path_select(struct ci_rx_path_features req_features,
>  		if (path_features->extra.flex_desc != req_features.extra.flex_desc)
>  			continue;
>  
> +		/* If requested, ensure the path supports single queue RX. */
> +		if (path_features->extra.single_queue != req_features.extra.single_queue)
> +			continue;
> +
>  		/* If requested, ensure the path supports scattered RX. */
>  		if (path_features->extra.scattered != req_features.extra.scattered)
>  			continue;
> diff --git a/drivers/net/intel/idpf/idpf_common_device.h b/drivers/net/intel/idpf/idpf_common_device.h
> index 5f3e4a4fcf..62665ad286 100644
> --- a/drivers/net/intel/idpf/idpf_common_device.h
> +++ b/drivers/net/intel/idpf/idpf_common_device.h
> @@ -44,6 +44,16 @@
>  	(sizeof(struct virtchnl2_ptype) +				\
>  	 (((p)->proto_id_count ? ((p)->proto_id_count - 1) : 0) * sizeof((p)->proto_id[0])))
>  
> +enum idpf_rx_func_type {
> +	IDPF_RX_DEFAULT,
> +	IDPF_RX_SINGLEQ,
> +	IDPF_RX_SINGLEQ_SCATTERED,
> +	IDPF_RX_SINGLEQ_AVX2,
> +	IDPF_RX_AVX512,
> +	IDPF_RX_SINGLQ_AVX512,
> +	IDPF_RX_MAX
> +};
> +
>  struct idpf_adapter {
>  	struct idpf_hw hw;
>  	struct virtchnl2_version_info virtchnl_version;
> @@ -59,6 +69,8 @@ struct idpf_adapter {
>  
>  	/* For timestamp */
>  	uint64_t time_hw;
> +
> +	enum idpf_rx_func_type rx_func_type;
>  };
>  
>  struct idpf_chunks_info {
> diff --git a/drivers/net/intel/idpf/idpf_common_rxtx.c b/drivers/net/intel/idpf/idpf_common_rxtx.c
> index eb25b091d8..97a5ce9b87 100644
> --- a/drivers/net/intel/idpf/idpf_common_rxtx.c
> +++ b/drivers/net/intel/idpf/idpf_common_rxtx.c
> @@ -7,6 +7,8 @@
>  #include <rte_errno.h>
>  
>  #include "idpf_common_rxtx.h"
> +#include "idpf_common_device.h"
> +#include "../common/rx.h"
>  
>  int idpf_timestamp_dynfield_offset = -1;
>  uint64_t idpf_timestamp_dynflag;
> @@ -1622,3 +1624,25 @@ idpf_qc_splitq_rx_vec_setup(struct idpf_rx_queue *rxq)
>  	rxq->bufq2->idpf_ops = &def_rx_ops_vec;
>  	return idpf_rxq_vec_setup_default(rxq->bufq2);
>  }
> +
> +RTE_EXPORT_INTERNAL_SYMBOL(idpf_rx_path_infos)
> +const struct ci_rx_path_info idpf_rx_path_infos[] = {
> +	[IDPF_RX_DEFAULT] = {idpf_dp_splitq_recv_pkts, "Scalar Split",
> +		{IDPF_RX_SCALAR_OFFLOADS, RTE_VECT_SIMD_DISABLED}},
> +	[IDPF_RX_SINGLEQ] = {idpf_dp_singleq_recv_pkts, "Scalar Single Queue",
> +		{IDPF_RX_SCALAR_OFFLOADS, RTE_VECT_SIMD_DISABLED, {.single_queue = true}}},
> +	[IDPF_RX_SINGLEQ_SCATTERED] = {
> +		idpf_dp_singleq_recv_scatter_pkts, "Scalar Single Queue Scattered",
> +		{IDPF_RX_SCALAR_OFFLOADS, RTE_VECT_SIMD_DISABLED,
> +		{.scattered = true, .single_queue = true}}},
> +#ifdef RTE_ARCH_X86
> +	[IDPF_RX_SINGLEQ_AVX2] = {idpf_dp_singleq_recv_pkts_avx2, "AVX2 Single Queue",
> +		{IDPF_RX_VECTOR_OFFLOADS, RTE_VECT_SIMD_256, {.single_queue = true}}},
> +#ifdef CC_AVX512_SUPPORT
> +	[IDPF_RX_AVX512] = {idpf_dp_splitq_recv_pkts_avx512, "AVX-512 Split",
> +		{IDPF_RX_VECTOR_OFFLOADS, RTE_VECT_SIMD_512}},
> +	[IDPF_RX_SINGLQ_AVX512] = {idpf_dp_singleq_recv_pkts_avx512, "AVX-512 Single Queue",
> +		{IDPF_RX_VECTOR_OFFLOADS, RTE_VECT_SIMD_512, {.single_queue = true}}},
> +#endif /* CC_AVX512_SUPPORT */
> +#endif /* RTE_ARCH_X86 */

A minor point, but I don't particularly like this form of init for the
array. If it was all in table form, with each entry on a single line, then
it would be ok, since we could have neat columns. However, since we are
word-wrapping the lines, I'd tend towards having one element per line and
using named initializers.