Re: [PATCH v5 07/25] net/intel/common: merge ice and i40e Tx queue struct

[David Marchand <david.marchand@redhat.com>]

On Mon, Jan 20, 2025 at 1:00 PM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> The queue structures of i40e and ice drivers are virtually identical, so
> merge them into a common struct. This should allow easier function
> merging in future using that common struct.
>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>  drivers/net/intel/common/tx.h                 | 55 +++++++++++++++++
>  drivers/net/intel/i40e/i40e_ethdev.c          |  4 +-
>  drivers/net/intel/i40e/i40e_ethdev.h          |  4 +-
>  drivers/net/intel/i40e/i40e_fdir.c            |  4 +-
>  .../i40e/i40e_recycle_mbufs_vec_common.c      |  2 +-
>  drivers/net/intel/i40e/i40e_rxtx.c            | 58 +++++++++---------
>  drivers/net/intel/i40e/i40e_rxtx.h            | 50 ++--------------
>  .../net/intel/i40e/i40e_rxtx_vec_altivec.c    |  4 +-
>  drivers/net/intel/i40e/i40e_rxtx_vec_avx2.c   |  4 +-
>  drivers/net/intel/i40e/i40e_rxtx_vec_avx512.c |  6 +-
>  drivers/net/intel/i40e/i40e_rxtx_vec_common.h |  2 +-
>  drivers/net/intel/i40e/i40e_rxtx_vec_neon.c   |  4 +-
>  drivers/net/intel/i40e/i40e_rxtx_vec_sse.c    |  4 +-
>  drivers/net/intel/ice/ice_dcf.c               |  4 +-
>  drivers/net/intel/ice/ice_dcf_ethdev.c        | 10 ++--
>  drivers/net/intel/ice/ice_diagnose.c          |  2 +-
>  drivers/net/intel/ice/ice_ethdev.c            |  2 +-
>  drivers/net/intel/ice/ice_ethdev.h            |  4 +-
>  drivers/net/intel/ice/ice_rxtx.c              | 60 +++++++++----------
>  drivers/net/intel/ice/ice_rxtx.h              | 41 +------------
>  drivers/net/intel/ice/ice_rxtx_vec_avx2.c     |  4 +-
>  drivers/net/intel/ice/ice_rxtx_vec_avx512.c   |  8 +--
>  drivers/net/intel/ice/ice_rxtx_vec_common.h   |  8 +--
>  drivers/net/intel/ice/ice_rxtx_vec_sse.c      |  6 +-
>  24 files changed, 165 insertions(+), 185 deletions(-)
>
> diff --git a/drivers/net/intel/common/tx.h b/drivers/net/intel/common/tx.h
> index 5397007411..c965f5ee6c 100644
> --- a/drivers/net/intel/common/tx.h
> +++ b/drivers/net/intel/common/tx.h
> @@ -8,6 +8,9 @@
>  #include <stdint.h>
>  #include <rte_mbuf.h>
>
> +/* forward declaration of the common intel (ci) queue structure */
> +struct ci_tx_queue;
> +
>  /**
>   * Structure associated with each descriptor of the TX ring of a TX queue.
>   */
> @@ -24,6 +27,58 @@ struct ci_tx_entry_vec {
>         struct rte_mbuf *mbuf; /* mbuf associated with TX desc, if any. */
>  };
>
> +typedef void (*ice_tx_release_mbufs_t)(struct ci_tx_queue *txq);
> +
> +struct ci_tx_queue {
> +       union { /* TX ring virtual address */
> +               volatile struct ice_tx_desc *ice_tx_ring;
> +               volatile struct i40e_tx_desc *i40e_tx_ring;
> +       };

Minor nit.. this gets alphabetically sorted in a later patch.

> +       volatile uint8_t *qtx_tail;               /* register address of tail */
> +       struct ci_tx_entry *sw_ring; /* virtual address of SW ring */
> +       rte_iova_t tx_ring_dma;        /* TX ring DMA address */
> +       uint16_t nb_tx_desc;           /* number of TX descriptors */
> +       uint16_t tx_tail; /* current value of tail register */
> +       uint16_t nb_tx_used; /* number of TX desc used since RS bit set */
> +       /* index to last TX descriptor to have been cleaned */
> +       uint16_t last_desc_cleaned;
> +       /* Total number of TX descriptors ready to be allocated. */
> +       uint16_t nb_tx_free;
> +       /* Start freeing TX buffers if there are less free descriptors than
> +        * this value.
> +        */
> +       uint16_t tx_free_thresh;
> +       /* Number of TX descriptors to use before RS bit is set. */
> +       uint16_t tx_rs_thresh;
> +       uint8_t pthresh;   /**< Prefetch threshold register. */
> +       uint8_t hthresh;   /**< Host threshold register. */
> +       uint8_t wthresh;   /**< Write-back threshold reg. */
> +       uint16_t port_id;  /* Device port identifier. */
> +       uint16_t queue_id; /* TX queue index. */
> +       uint16_t reg_idx;
> +       uint64_t offloads;
> +       uint16_t tx_next_dd;
> +       uint16_t tx_next_rs;
> +       uint64_t mbuf_errors;
> +       bool tx_deferred_start; /* don't start this queue in dev start */
> +       bool q_set;             /* indicate if tx queue has been configured */
> +       union {                  /* the VSI this queue belongs to */
> +               struct ice_vsi *ice_vsi;
> +               struct i40e_vsi *i40e_vsi;
> +       };

Idem.

> +       const struct rte_memzone *mz;
> +
> +       union {
> +               struct { /* ICE driver specific values */
> +                       ice_tx_release_mbufs_t tx_rel_mbufs;
> +                       uint32_t q_teid; /* TX schedule node id. */
> +               };
> +               struct { /* I40E driver specific values */
> +                       uint8_t dcb_tc;
> +               };
> +       };
> +};
> +
>  static __rte_always_inline void
>  ci_tx_backlog_entry(struct ci_tx_entry *txep, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
>  {


-- 
David Marchand