On Thu, May 15, 2025 at 01:09:31PM +0200, Burakov, Anatoly wrote:
> On 5/14/2025 6:52 PM, Bruce Richardson wrote:
> > On Mon, May 12, 2025 at 01:54:30PM +0100, Anatoly Burakov wrote:
> > > Make the i40e driver use the new common Rx queue structure.
> > >
> > > Because the i40e driver supports both 16-byte and 32-byte descriptor
> > > formats (controlled by RTE_LIBRTE_I40E_16BYTE_RX_DESC define), the common
> > > queue structure has to take that into account, so the ring queue structure
> > > will have both, while the actual descriptor format is picked by i40e at
> > > compile time using the above macro. Direct usage of Rx queue structure is
> > > now meant to be replaced with a macro access that takes descriptor size
> > > into account.
> > >
> > > Signed-off-by: Anatoly Burakov <anatoly.bura...@intel.com>
> > > ---
> > > drivers/net/intel/common/rx.h | 14 ++
> > > drivers/net/intel/i40e/i40e_ethdev.c | 4 +-
> > > drivers/net/intel/i40e/i40e_ethdev.h | 4 +-
> > > drivers/net/intel/i40e/i40e_fdir.c | 16 +--
> > > .../i40e/i40e_recycle_mbufs_vec_common.c | 6 +-
> > > drivers/net/intel/i40e/i40e_rxtx.c | 126 +++++++++---------
> > > drivers/net/intel/i40e/i40e_rxtx.h | 74 +++-------
> > > drivers/net/intel/i40e/i40e_rxtx_common_avx.h | 6 +-
> > > .../net/intel/i40e/i40e_rxtx_vec_altivec.c | 22 +--
> > > drivers/net/intel/i40e/i40e_rxtx_vec_avx2.c | 12 +-
> > > drivers/net/intel/i40e/i40e_rxtx_vec_avx512.c | 12 +-
> > > drivers/net/intel/i40e/i40e_rxtx_vec_common.h | 4 +-
> > > drivers/net/intel/i40e/i40e_rxtx_vec_neon.c | 24 ++--
> > > drivers/net/intel/i40e/i40e_rxtx_vec_sse.c | 24 ++--
> > > 14 files changed, 160 insertions(+), 188 deletions(-)
> > >
> > > diff --git a/drivers/net/intel/common/rx.h b/drivers/net/intel/common/rx.h
> > > index 524de39f9c..db49db57d0 100644
> > > --- a/drivers/net/intel/common/rx.h
> > > +++ b/drivers/net/intel/common/rx.h
> > > @@ -30,6 +30,8 @@ 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 union i40e_16byte_rx_desc *i40e_rx_16b_ring;
> > > + volatile union i40e_32byte_rx_desc *i40e_rx_32b_ring;
> >
> > Rather than doing this, would it not be better to keep using the current
> > scheme of just having an #ifdef aliasing i40e_rx_desc to either the 16byte
> > or 32byte variants?
> >
> > /Bruce
>
> I had that in my first iteration, but I don't really like having
> driver-specific #ifdefs in a common header. Plus, it also introduces header
> ordering dependencies (need to include header with definitions before
> including this one), so I decided against it.
>
The header ordering should not be a problem. The definition placeholder for
the 16-byte descriptors is in the global rte_config.h.
/Bruce
