On Thu, Oct 15, 2020 at 4:21 PM Slava Ovsiienko <viachesl...@nvidia.com> wrote:
>
> Hi, Jerin
>
> > -----Original Message-----
> > From: Jerin Jacob <jerinjac...@gmail.com>
> > Sent: Thursday, October 15, 2020 13:28
> > To: Slava Ovsiienko <viachesl...@nvidia.com>
> > Cc: dpdk-dev <dev@dpdk.org>; NBU-Contact-Thomas Monjalon
> > <tho...@monjalon.net>; Stephen Hemminger
> > <step...@networkplumber.org>; Ferruh Yigit <ferruh.yi...@intel.com>;
> > Olivier Matz <olivier.m...@6wind.com>; Maxime Coquelin
> > <maxime.coque...@redhat.com>; David Marchand
> > <david.march...@redhat.com>; Andrew Rybchenko
> > <arybche...@solarflare.com>
> > Subject: Re: [PATCH v6 1/6] ethdev: introduce Rx buffer split
> >
> [..snip..]
> >
> > struct rte_eth_rxseg {
> > enum rte_eth_rxseg_mode mode ;
> > union {
> > struct rte_eth_rxseg_mode xxx {
> > struct rte_mempool *mp; /**< Memory pool to
> > allocate
> > segment from. */
> > uint16_t length; /**< Segment data length,
> > configures split
> > point. */
> > uint16_t offset; /**< Data offset from
> > beginning of mbuf data
> > buffer. */
> > uint32_t reserved; /**< Reserved field. */
> > }
> > }
>
> There is an array of rte_eth_rxseg. It would introduce multiple "enum
> rte_eth_rxseg_mode mode"
> and would cause some ambiguity. About mode selection - please, see below.
> Union seems to be good idea, let's adopt.
Ack. Let's take the only union concept.
>
> >
> > Another mode, Marvell PMD has it(I believe Intel also) i.e When we say:
> >
> > seg0 - pool0, len0=2000B, off0=0
> > seg1 - pool1, len1=2001B, off1=0
> >
> > packet size up to, 2000B goes to pool 0 and if is >=2001 goes to pool1.
> > I think, it is better to have mode param in rte_eth_rxseg for avoiding ABI
> > changes.(Just like clean rte_flow APIs)
>
> It is supposed to choose with RTE_ETH_RX_OFFLOAD_xxx flags.
> For packet sorting it should be something like this RTE_ETH_RX_OFFLOAD_SORT.
> PMD reports it supports the feature, the flag is set in rx_conf->offloads
> and rxseg structure is interpreted according to these flags.
Works for me.
>
> Please, note, there is intentionally no check for RTE_ETH_RX_OFFLOAD_xxx
> in rte_eth_dev_rx_queue_setup() - it should be done on PMD side.
>
> >
> > > > If we see some of the features of such kind or other PMDs adopts the
> > > > split feature - we'll try to find the common root and consider the way
> > > > how
> > to report it.
> > >
> > > My only concern with that approach will be ABI break again if
> > > something needs to exposed over rte_eth_dev_info().
>
> Let's reserve the pointer to struct rte_eth_rxseg_limitations
> in the rte_eth_dev_info to avoid ABI break?
Works for me. If we add an additional reserved field.
Due to RC1 time constraint, I am OK to leave it as a reserved filed and fill
meat when it is required if other ethdev maintainers are OK.
I will be required for feature complete.
>
> With best regards, Slava
