Re: [dpdk-dev] [RFC] ethdev: introduce Rx buffer split

[Slava Ovsiienko <viacheslavo@nvidia.com>]

> -----Original Message-----
> From: Andrew Rybchenko <Andrew.Rybchenko@oktetlabs.ru>
> Sent: Monday, October 12, 2020 19:04
> To: NBU-Contact-Thomas Monjalon <thomas@monjalon.net>;
> ferruh.yigit@intel.com; Slava Ovsiienko <viacheslavo@nvidia.com>
> Cc: dev@dpdk.org; stephen@networkplumber.org; Shahaf Shuler
> <shahafs@nvidia.com>; olivier.matz@6wind.com; jerinjacobk@gmail.com;
> maxime.coquelin@redhat.com; david.marchand@redhat.com; Asaf Penso
> <asafp@nvidia.com>
> Subject: Re: [dpdk-dev] [RFC] ethdev: introduce Rx buffer split
> 
> On 10/12/20 6:14 PM, Thomas Monjalon wrote:
> > 12/10/2020 11:56, Slava Ovsiienko:
> >> Hi, Andrew
> >>
> >> Thank you for the comments.
> >>
> >> We have two approaches how to specify multiple segments to split Rx
> packets:
> >> 1. update queue configuration structure 2. introduce new
> >> rx_queue_setup_ex() routine with extra parameters.
> >>
> >> For [1] my only actual dislike is that we would have multiple places
> >> to specify the pool - in rx_queue_setup() and in the config
> >> structure. So, we should implement some checking (if we have offload
> >> flag set we should check whether mp parameter is NULL and segment
> >> descriptions array pointer/size is provided, if no offload flag set - we must
> check the description array is empty).
> >>
> >>> @Thomas, @Ferruh: I'd like to hear what other ethdev maintainers
> >>> think about it.
> >>
> >> Yes, it would be very nice to hear extra opinions. Do we think the
> >> providing of extra API function is worse than extending existing
> >> structure, introducing some conditional ambiguity and complicating
> >> the parameter compliance check?
> >
> > Let's try listing pros and cons of each approach, so we can conclude.
> >
> > 1/ update queue config struct
> >
> > 	1.1 pro: keep same queue setup function
> 
> pro: no code duplication
> 
> > 	1.2 con: two mempool pointers (struct or function)
> > 	1.3 con: variable size of segment description array
> >
> > 2/ new queue setup function
> >
> > 	2.1 con: two functions for queue setup
> 
> con: code duplication or refactoring of existing stable code

- no refactoring of existing rte_eth_rx_queue_setup() - it is kept intact
- yes, there is some duplication in rte_eth_rxseg_queue_setup, but
  the large part of code is new - there is very specific check for the
 split buffer parameters.
- no code duplication at PMD level - both ways go to the same
  internal routine
- PMD code must be refactored anyway, no con/pro for this point -
  updated PMD rx queue setup must handle the new split format,
 no way to drop it.

> 
> > 	2.2 pro: mempool pointer is not redundant
> > 	2.3 pro: segment description array size defined by the caller
> >
> > What else I'm missing?
> >

With best regards, Slava