[PATCH v1 09/21] net/ixgbe: use common Tx queue structure

[Medvedkin, Vladimir]

Hi Bruce,

On 02/12/2024 11:24, Bruce Richardson wrote:
> Merge in additional fields used by the ixgbe driver and then convert it
> over to using the common Tx queue structure.
>
> Signed-off-by: Bruce Richardson<bruce.richardson at intel.com>
> ---
>   drivers/net/_common_intel/tx.h                | 14 +++-
>   drivers/net/ixgbe/ixgbe_ethdev.c              |  4 +-
>   .../ixgbe/ixgbe_recycle_mbufs_vec_common.c    |  2 +-
>   drivers/net/ixgbe/ixgbe_rxtx.c                | 64 +++++++++----------
>   drivers/net/ixgbe/ixgbe_rxtx.h                | 56 ++--------------
>   drivers/net/ixgbe/ixgbe_rxtx_vec_common.h     | 26 ++++----
>   drivers/net/ixgbe/ixgbe_rxtx_vec_neon.c       | 14 ++--
>   drivers/net/ixgbe/ixgbe_rxtx_vec_sse.c        | 14 ++--
>   8 files changed, 80 insertions(+), 114 deletions(-)
>
> diff --git a/drivers/net/_common_intel/tx.h b/drivers/net/_common_intel/tx.h
> index c4a1a0c816..51ae3b051d 100644
> --- a/drivers/net/_common_intel/tx.h
> +++ b/drivers/net/_common_intel/tx.h
> @@ -34,9 +34,13 @@ struct ci_tx_queue {
>   		volatile struct i40e_tx_desc *i40e_tx_ring;
>   		volatile struct iavf_tx_desc *iavf_tx_ring;
>   		volatile struct ice_tx_desc *ice_tx_ring;
> +		volatile union ixgbe_adv_tx_desc *ixgbe_tx_ring;
>   	};
>   	volatile uint8_t *qtx_tail;               /* register address of tail */
> -	struct ci_tx_entry *sw_ring; /* virtual address of SW ring */
> +	union {
> +		struct ci_tx_entry *sw_ring; /* virtual address of SW ring */
> +		struct ci_tx_entry_vec *sw_ring_vec;
> +	};
>   	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 */
> @@ -87,6 +91,14 @@ struct ci_tx_queue {
>   			uint8_t tc;
>   			bool use_ctx;  /* with ctx info, each pkt needs two descriptors */
>   		};
> +		struct { /* ixgbe specific values */
> +			const struct ixgbe_txq_ops *ops;
> +			struct ixgbe_advctx_info *ctx_cache;
'struct ixgbe_advctx_info ctx_cache[IXGBE_CTX_NUM];' takes only 80 bytes 
of memory, so using a pointer saves 72 bytes. Since the final version of 
the 'struct ci_tx_queue' without driver specific fields takes 96 bytes, 
embedding 'ixgbe_advctx_info ctx_cache[2]' array will take one more 
cache line, which is hot a huge deal in my opinion.

Or consider another (possibly better) approach, where for non IXGBE 
'struct ci_tx_queue' will remain the same size, but only for IXGBE an 
extra 80 bytes will be alllocated:

struct {/* ixgbe specific values */

                         const struct ixgbe_txq_ops *ops;

                         uint32_t ctx_curr;

                         uint8_t pthresh;  /**< Prefetch threshold 
register. */

                         uint8_t hthresh;  /**< Host threshold register. */

                         uint8_t wthresh;  /**< Write-back threshold reg. */

                         uint8_t using_ipsec; /**< indicates that IPsec 
TX feature is in use */
struct ixgbe_advctx_info ctx_cache[0];

                 };

> +			uint32_t ctx_curr;
> +#ifdef RTE_LIB_SECURITY
> +			uint8_t using_ipsec;  /**< indicates that IPsec TX feature is in use */
> +#endif
> +		};
>   	};
>   };
>   
<snip>

-- 
Regards,
Vladimir
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mails.dpdk.org/archives/dev/attachments/20241202/615466a9/attachment.htm>