Re: [PATCH v4 15/25] net/ixgbe: create common Rx queue structure

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

On Fri, May 30, 2025 at 02:57:11PM +0100, Anatoly Burakov wrote:
> In preparation for deduplication effort, generalize the Rx queue structure.
> 
> The entire Rx queue structure is moved to common/rx.h, clarifying the
> comments where necessary, and separating common parts from ixgbe-specific
> parts.
> 
> Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> ---
Acked-by: Bruce Richardson <bruce.richardson@intel.com>

One minor nit inline below.
> 
> Notes:
>     v3 -> v4:
>     - Separate out some of the changes from this commit into previous commits
>     - Rename CI_RX_BURST to CI_RX_MAX_BURST to match the driver naming convention
> 
>  drivers/net/intel/common/rx.h                 |  67 ++++++++++-
>  drivers/net/intel/ixgbe/ixgbe_ethdev.c        |   8 +-
>  .../ixgbe/ixgbe_recycle_mbufs_vec_common.c    |   6 +-
>  drivers/net/intel/ixgbe/ixgbe_rxtx.c          | 110 +++++++++---------
>  drivers/net/intel/ixgbe/ixgbe_rxtx.h          |  65 +----------
>  .../net/intel/ixgbe/ixgbe_rxtx_vec_common.h   |   4 +-
>  drivers/net/intel/ixgbe/ixgbe_rxtx_vec_neon.c |  18 +--
>  drivers/net/intel/ixgbe/ixgbe_rxtx_vec_sse.c  |  18 +--
>  8 files changed, 150 insertions(+), 146 deletions(-)
> 
> diff --git a/drivers/net/intel/common/rx.h b/drivers/net/intel/common/rx.h
> index abb01ba5e7..80a9f21303 100644
> --- a/drivers/net/intel/common/rx.h
> +++ b/drivers/net/intel/common/rx.h
> @@ -10,14 +10,75 @@
>  #include <rte_mbuf.h>
>  #include <rte_ethdev.h>
>  
> -#define CI_RX_BURST 32
> +#define CI_RX_MAX_BURST 32
> +
> +struct ci_rx_queue;
> +
> +struct ci_rx_entry {
> +	struct rte_mbuf *mbuf; /* mbuf associated with RX descriptor. */
> +};
> +
> +struct ci_rx_entry_sc {
> +	struct rte_mbuf *fbuf; /* First segment of the fragmented packet.*/
> +};
> +
> +/**
> + * Structure associated with each RX queue.
> + */
> +struct ci_rx_queue {
> +	struct rte_mempool  *mp; /**< mbuf pool to populate RX ring. */
> +	union { /* RX ring virtual address */
> +		volatile union ixgbe_adv_rx_desc *ixgbe_rx_ring;
> +	};
> +	volatile uint8_t *qrx_tail;   /**< register address of tail */
> +	struct ci_rx_entry *sw_ring; /**< address of RX software ring. */
> +	struct ci_rx_entry_sc *sw_sc_ring; /**< address of scattered Rx software ring. */
> +	rte_iova_t rx_ring_phys_addr; /**< RX ring DMA address. */
> +	struct rte_mbuf *pkt_first_seg; /**< First segment of current packet. */
> +	struct rte_mbuf *pkt_last_seg; /**< Last segment of current packet. */
> +	/** hold packets to return to application */
> +	struct rte_mbuf *rx_stage[CI_RX_MAX_BURST * 2];
> +	uint16_t nb_rx_desc; /**< number of RX descriptors. */
> +	uint16_t rx_tail;  /**< current value of tail register. */
> +	uint16_t rx_nb_avail; /**< nr of staged pkts ready to ret to app */
> +	uint16_t nb_rx_hold; /**< number of held free RX desc. */
> +	uint16_t rx_next_avail; /**< idx of next staged pkt to ret to app */
> +	uint16_t rx_free_thresh; /**< max free RX desc to hold. */
> +	uint16_t rx_free_trigger; /**< triggers rx buffer allocation */
> +	uint16_t rxrearm_nb;     /**< number of remaining to be re-armed */
> +	uint16_t rxrearm_start;  /**< the idx we start the re-arming from */
> +	uint16_t queue_id; /**< RX queue index. */
> +	uint16_t port_id;  /**< Device port identifier. */
> +	uint16_t reg_idx;  /**< RX queue register index. */
> +	uint8_t crc_len;  /**< 0 if CRC stripped, 4 otherwise. */
> +	bool rx_deferred_start; /**< queue is not started on dev start. */
> +	bool vector_rx; /**< indicates that vector RX is in use */
> +	bool drop_en;  /**< if 1, drop packets if no descriptors are available. */
> +	uint64_t mbuf_initializer; /**< value to init mbufs */
> +	uint64_t offloads; /**< Rx offloads with RTE_ETH_RX_OFFLOAD_* */
> +	/** need to alloc dummy mbuf, for wraparound when scanning hw ring */
> +	struct rte_mbuf fake_mbuf;
> +	const struct rte_memzone *mz;
> +	union {
> +		struct { /* ixgbe specific values */
> +			/** indicates that IPsec RX feature is in use */
> +			uint8_t using_ipsec;
> +			/** Packet type mask for different NICs. */
> +			uint16_t pkt_type_mask;
> +			/** UDP frames with a 0 checksum can be marked as checksum errors. */
> +			uint8_t rx_udp_csum_zero_err;
> +			/** flags to set in mbuf when a vlan is detected. */
> +			uint64_t vlan_flags;

These ixgbe fields should probably be sorted by size.

> +		};
> +	};
> +};
<snip>