Re: [PATCH v5 2/4] net/intel: use common Tx queue structure

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

On Thu, Mar 27, 2025 at 09:34:35PM +0530, Shaiq Wani wrote:
> Merge in additional fields used by the idpf driver and then convert it
> over to using the common Tx queue structure
> 
> Signed-off-by: Shaiq Wani <shaiq.wani@intel.com>

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

Some comments inline below.

> ---
>  drivers/net/intel/common/tx.h                 | 20 +++++++
>  drivers/net/intel/cpfl/cpfl_ethdev.c          |  3 +-
>  drivers/net/intel/cpfl/cpfl_ethdev.h          |  2 +-
>  drivers/net/intel/cpfl/cpfl_rxtx.c            | 26 ++++-----
>  drivers/net/intel/cpfl/cpfl_rxtx.h            |  3 +-
>  drivers/net/intel/cpfl/cpfl_rxtx_vec_common.h |  3 +-
>  drivers/net/intel/idpf/idpf_common_rxtx.c     | 36 ++++++------
>  drivers/net/intel/idpf/idpf_common_rxtx.h     | 58 +++----------------
>  .../net/intel/idpf/idpf_common_rxtx_avx2.c    | 12 ++--
>  .../net/intel/idpf/idpf_common_rxtx_avx512.c  | 21 +++----
>  drivers/net/intel/idpf/idpf_common_virtchnl.c |  2 +-
>  drivers/net/intel/idpf/idpf_common_virtchnl.h |  2 +-
>  drivers/net/intel/idpf/idpf_ethdev.c          |  3 +-
>  drivers/net/intel/idpf/idpf_rxtx.c            | 21 ++++---
>  drivers/net/intel/idpf/idpf_rxtx.h            |  1 +
>  drivers/net/intel/idpf/idpf_rxtx_vec_common.h |  5 +-
>  16 files changed, 101 insertions(+), 117 deletions(-)
> 

<snip>

>  static int
> -cpfl_tx_complq_setup(struct rte_eth_dev *dev, struct idpf_tx_queue *txq,
> +cpfl_tx_complq_setup(struct rte_eth_dev *dev, struct ci_tx_queue *txq,
>  		     uint16_t queue_idx, uint16_t nb_desc,
>  		     unsigned int socket_id)
>  {
>  	struct cpfl_vport *cpfl_vport = dev->data->dev_private;
>  	struct idpf_vport *vport = &cpfl_vport->base;
>  	const struct rte_memzone *mz;
> -	struct idpf_tx_queue *cq;
> +	struct ci_tx_queue *cq;
>  	int ret;
>  
>  	cq = rte_zmalloc_socket("cpfl splitq cq",
> -				sizeof(struct idpf_tx_queue),
> +				sizeof(struct ci_tx_queue),

Minor nit, but this would be more maintainable I think as "sizeof(*cq)" so
if you have to change the type of cq again in future, only on line needs to
change.

>  				RTE_CACHE_LINE_SIZE,
>  				socket_id);
>  	if (cq == NULL) {
> @@ -528,7 +528,7 @@ cpfl_tx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx,
>  	struct cpfl_tx_queue *cpfl_txq;

<snip>

>  void
> -idpf_qc_split_tx_descq_reset(struct idpf_tx_queue *txq)
> +idpf_qc_split_tx_descq_reset(struct ci_tx_queue *txq)
>  {
>  	struct idpf_tx_entry *txe;
>  	uint32_t i, size;
> @@ -223,7 +223,7 @@ idpf_qc_split_tx_descq_reset(struct idpf_tx_queue *txq)
>  	for (i = 0; i < size; i++)
>  		((volatile char *)txq->desc_ring)[i] = 0;
>  
> -	txe = txq->sw_ring;
> +	txe = (struct idpf_tx_entry *)txq->sw_ring;

As I comment on the next patch in the series, if you change the order of
these two patches, you won't need to introduce these new typecasts.


>  	prev = (uint16_t)(txq->sw_nb_desc - 1);
>  	for (i = 0; i < txq->sw_nb_desc; i++) {
>  		txe[i].mbuf = NULL;
> @@ -246,7 +246,7 @@ idpf_qc_split_tx_descq_reset(struct idpf_tx_queue *txq)
>  }

<snip>