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

[Slava Ovsiienko <viacheslavo@nvidia.com>]

Hi, Andrew

Thank you for the comments, please see my replies below.

> -----Original Message-----
> From: Andrew Rybchenko <arybchenko@solarflare.com>
> Sent: Thursday, September 17, 2020 19:55
> To: Slava Ovsiienko <viacheslavo@nvidia.com>; dev@dpdk.org
> Cc: Thomas Monjalon <thomasm@mellanox.com>;
> stephen@networkplumber.org; ferruh.yigit@intel.com; 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
> 
[snip]
> >
> > For example, let's suppose we configured the Rx queue with the
> > following segments:
> >      seg0 - pool0, len0=14B, off0=RTE_PKTMBUF_HEADROOM
> >      seg1 - pool1, len1=20B, off1=0B
> >      seg2 - pool2, len2=20B, off2=0B
> >      seg3 - pool3, len3=512B, off3=0B
> >
> > The packet 46 bytes long will look like the following:
> >      seg0 - 14B long @ RTE_PKTMBUF_HEADROOM in mbuf from pool0
> >      seg1 - 20B long @ 0 in mbuf from pool1
> >      seg2 - 12B long @ 0 in mbuf from pool2
> >
> > The packet 1500 bytes long will look like the following:
> >      seg0 - 14B @ RTE_PKTMBUF_HEADROOM in mbuf from pool0
> >      seg1 - 20B @ 0 in mbuf from pool1
> >      seg2 - 20B @ 0 in mbuf from pool2
> >      seg3 - 512B @ 0 in mbuf from pool3
> >      seg4 - 512B @ 0 in mbuf from pool3
> >      seg5 - 422B @ 0 in mbuf from pool3
> 
> The behaviour is logical, but what to do if HW can't do it, i.e. use the last
> segment many times. Should it reject configuration if provided segments are
> insufficient to fit MTU packet? How to report the limitation?
> (I'm still trying to convince that SCATTER and BUFFER_SPLIT should be
> independent).

BUFFER_SPLIT is rather the way to tune SCATTER. Currently scattering
happens on unconditional mbuf data buffer boundaries (we have reserved
HEAD space in the first mbuf and fill this one to the buffer end,
the next mbuf buffers might be filled completely). BUFFER_SPLIT provides
the way to specify the desired points to split packet, not just blindly
follow buffer boundaries. There is the check inplemented in common part
if each split segment fits the mbuf allocated from appropriate pool.
PMD should do extra check internally whether it supports the requested
split settings, if not - call will be rejected.

[snip]
> 
> I dislike the idea to introduce new device operation.
> rte_eth_rxconf has reserved space and BUFFER_SPLIT offload will mean that
> PMD looks at the split configuration location there.
> 
We considered the approach of pushing split setting to the rxconf structure.
[http://patches.dpdk.org/patch/75205/]
But it seems there are some issues:

- the split configuration description requires the variable length array (due
  to variations in number of segments), so rte_eth_rxconf structure would
  have the variable length (not nice, IMO).

  We could push pointers to the array of rte_eth_rxseg, but we would lost
  the single structure (and contiguous memory) simplicity, this approach has
  no advantages over the specifying the split configuration as parameters
  of setup_ex().

- it would introduces the ambiguity, rte_eth_rx_queue_setup() specifies the single
  mbuf pool as parameter. What should we do with it? Set to NULL? 
  Treat as the first  pool? I would prefer to specify all split segments in
  uniform fashion, i.e. as array or rte_eth_rxseg structures (and it can be
  easily updated with some extra segment attributes if needed). So, in my
  opinion, we should remove/replace the pool parameter in rx_queue_setup
 (by introducing new func).

- specifying the new extended setup roiutine has an advantage that we should
  not update any PMDs code in part of existing implementations of
  rte_eth_rx_queue_setup().

  If PMD supports BUFFER_SPLIT (or other related feature) it just should provide
  rte_eth_rx_queue_setup_ex() and check the DEV_RX_OFFLOAD_BUFFER_SPLIT
  (or HEADER_SPLIT, or ever feature) it supports. The common code does
  not check the feature flags - it is on PMDs' own. In order to configure PMD
  to perfrom arbitrary desired Rx spliting the application should check
  DEV_RX_OFFLOAD_BUFFER_SPLIT in port capabilites, if found - set
  DEV_RX_OFFLOAD_BUFFER_SPLIT in configuration and call rte_eth_rx_queue_setup_ex().
  And this approach can be followed for any other split related feature.

With best regards, Slava