Re: [PATCH v3 06/13] net/iavf: use the common Rx queue structure

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

On Thu, May 15, 2025 at 01:11:03PM +0200, Burakov, Anatoly wrote:
> On 5/15/2025 12:59 PM, Bruce Richardson wrote:
> > On Mon, May 12, 2025 at 01:54:32PM +0100, Anatoly Burakov wrote:
> > > Make the iavf driver use the new common Rx queue structure.
> > > 
> > > Because the iavf driver supports both 16-byte and 32-byte descriptor
> > > formats (controlled by RTE_LIBRTE_IAVF_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 iavf 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.burakov@intel.com>
> > > ---
> > > 
> > > Notes:
> > >      v2:
> > >      - Fix compile issues for Arm
> > > 
> > >   drivers/net/intel/common/rx.h                 |  12 ++
> > >   drivers/net/intel/iavf/iavf.h                 |   4 +-
> > >   drivers/net/intel/iavf/iavf_ethdev.c          |  12 +-
> > >   drivers/net/intel/iavf/iavf_rxtx.c            | 192 +++++++++---------
> > >   drivers/net/intel/iavf/iavf_rxtx.h            |  76 ++-----
> > >   drivers/net/intel/iavf/iavf_rxtx_vec_avx2.c   |  24 +--
> > >   drivers/net/intel/iavf/iavf_rxtx_vec_avx512.c |  22 +-
> > >   drivers/net/intel/iavf/iavf_rxtx_vec_common.h |  27 ++-
> > >   drivers/net/intel/iavf/iavf_rxtx_vec_neon.c   |  30 +--
> > >   drivers/net/intel/iavf/iavf_rxtx_vec_sse.c    |  46 ++---
> > >   drivers/net/intel/iavf/iavf_vchnl.c           |   6 +-
> > >   11 files changed, 207 insertions(+), 244 deletions(-)
> > > 
> > > diff --git a/drivers/net/intel/common/rx.h b/drivers/net/intel/common/rx.h
> > > index 9a691971bc..2d9328ae89 100644
> > > --- a/drivers/net/intel/common/rx.h
> > > +++ b/drivers/net/intel/common/rx.h
> > > @@ -38,6 +38,8 @@ struct ci_rx_queue {
> > >   		volatile union i40e_32byte_rx_desc *i40e_rx_32b_ring;
> > >   		volatile union ice_16b_rx_flex_desc *ice_rx_16b_ring;
> > >   		volatile union ice_32b_rx_flex_desc *ice_rx_32b_ring;
> > > +		volatile union iavf_16byte_rx_desc *iavf_rx_16b_ring;
> > > +		volatile union iavf_32byte_rx_desc *iavf_rx_32b_ring;
> > >   	};
> > 
> > Aren't the descriptors used by iavf the same as those used by i40e, and the
> > flex descriptors used by it the same as those used by ice? If so, we should
> > look to merge their definitions.
> > 
> 
> They're coming from base code. We can do that, but I'm not sure how that
> would play with the rest of the driver and compatibility between ethdev
> parts and calling into base code.
> 
Hmmm, for the base code, we could just add a typedef for iavf_rx_desc to
i40e_rx_desc in the osdep.h header.

/Bruce