[dpdk-dev] [PATCH v8 1/6] ethdev: introduce Rx buffer split

[Slava Ovsiienko]

> -----Original Message-----
> From: Andrew Rybchenko <andrew.rybchenko at oktetlabs.ru>
> Sent: Friday, October 16, 2020 12:28
> To: Slava Ovsiienko <viacheslavo at nvidia.com>; dev at dpdk.org
> Cc: NBU-Contact-Thomas Monjalon <thomas at monjalon.net>;
> stephen at networkplumber.org; ferruh.yigit at intel.com;
> olivier.matz at 6wind.com; jerinjacobk at gmail.com;
> maxime.coquelin at redhat.com; david.marchand at redhat.com
> Subject: Re: [dpdk-dev] [PATCH v8 1/6] ethdev: introduce Rx buffer split
> 
> On 10/16/20 12:15 PM, Slava Ovsiienko wrote:
> > Hi, Andrew
> >
> >> -----Original Message-----
> >> From: Andrew Rybchenko <arybchenko at solarflare.com>
> >> Sent: Friday, October 16, 2020 11:52
> >> To: Slava Ovsiienko <viacheslavo at nvidia.com>; dev at dpdk.org
> >> Cc: NBU-Contact-Thomas Monjalon <thomas at monjalon.net>;
> >> stephen at networkplumber.org; ferruh.yigit at intel.com;
> >> olivier.matz at 6wind.com; jerinjacobk at gmail.com;
> >> maxime.coquelin at redhat.com; david.marchand at redhat.com
> >> Subject: Re: [PATCH v8 1/6] ethdev: introduce Rx buffer split
> >>
> >> On 10/16/20 10:48 AM, Viacheslav Ovsiienko wrote:
> >>> The DPDK datapath in the transmit direction is very flexible.
> >>> An application can build the multi-segment packet and manages almost
> >>> all data aspects - the memory pools where segments are allocated
> >>> from, the segment lengths, the memory attributes like external
> >>> buffers, registered for DMA, etc.
> >>>
> > [snip]
> >>> +struct rte_eth_rxseg {
> >>> +	union {
> >>
> >> Why not just 'union rte_eth_rxseg' ?
> >>
> >>> +		/* The settings for buffer split offload. */
> >>> +		struct rte_eth_rxseg_split split;
> >>
> >> Pointer to a split table must be here. I.e.
> >> struct rte_eth_rxseg_split *split;
> > OK, will try to simplify with that, thanks.
> >
> >> Also it must be specified how the array is terminated.
> >> We need either a number of define last item condition (mp == NULL ?)
> >
> > We have one, please see: "rte_eth_rxconf->rx_nseg"
> 
> A bit confusing to have it outside of union far from split but may be acceptable
> for the experimental API...
> If there are better ideas - welcome.

It can't be inside union - it does not share the memory layout with descriptors itself.
It can't be inside the descriptor structure either - rx_nseg describes the size of array, not segment property.
It is uint16_t rx_nseg and is placed before offload to save some space due to rte_eth_rxconf members alignment.
Will mark rte_eth_rxseg_split as experimental, nice clue, thanks.

With best regards, Slava