[dpdk-dev] [RFC PATCH 2/4] ethdev: introduce Rx queue offloads API

[Shahaf Shuler]

Tuesday, August 29, 2017 3:50 PM, Ferruh Yigit:
> On 8/7/2017 11:54 AM, Shahaf Shuler wrote:
> > Introduce a new API to configure Rx offloads.
> >
> > The new API will re-use existing DEV_RX_OFFLOAD_* flags to enable the
> > different offloads. This will ease the process of adding a new Rx
> > offloads, as no ABI breakage is involved.
> > In addition, the offload configuration can be done per queue, instead
> > of per port.
> 
> If a device doesn't have capability to set the offload per queue how should it
> behave, I think it is good to define this.

Yes, will add documentation. 
How about If device cannot set offloads per queue, then the queue_setup function should return with ENOTSUP ?

> 
> >
> > The Rx queue offload API can be used only with devices which advertize
> > the RTE_ETH_DEV_RXQ_OFFLOAD capability.
> >
> > The old Rx offloads API is kept for the meanwhile, in order to enable
> > a smooth transition for PMDs and application to the new API.
> >
> > Signed-off-by: Shahaf Shuler <shahafs at mellanox.com>
> 
> <...>
> 
> > @@ -357,7 +357,14 @@ struct rte_eth_rxmode {
> >  		jumbo_frame      : 1, /**< Jumbo Frame Receipt enable. */
> >  		hw_strip_crc     : 1, /**< Enable CRC stripping by hardware. */
> >  		enable_scatter   : 1, /**< Enable scatter packets rx handler */
> > -		enable_lro       : 1; /**< Enable LRO */
> > +		enable_lro       : 1, /**< Enable LRO */
> > +		ignore		 : 1;
> 
> what do you think making this variable more verbose, like
> "ignore_rx_offloads"
> 
> "dev_conf.rxmode.ignore" doesn't say on its own what is ignored.

Maybe ignore_offloads ? Rx is quite explicit from rxomde.

> 
> > +		/**
> > +		 * When set the rxmode offloads should be ignored,
> > +		 * instead the Rx offloads will be set on rte_eth_rxq_conf.
> > +		 * This bit is temporary till rxmode Rx offloads API will
> > +		 * be deprecated.
> > +		 */
> >  };
> 
> <...>
> 
> > +/** Device supports the rte_eth_rxq_conf offloads API */ #define
> > +RTE_ETH_DEV_RXQ_OFFLOAD 0x0010
> Since this is temporary flag and with current implementation this is local to
> library, should we put this into public header?
> 
> Later when all PMDs implemented this new method and we want to remove
> the flag, can we remove them or do we have to keep them reserved for any
> conflict for further new values?
> 
> I guess this should be part of missing pmd-ethdev interface file
> (rte_ethdev_pmd.h ?).

Yes it is better fits to inner interface between ethdev and PMDs.
Wondering, do we have other motivation to have such header? 

> 
> >
> >  /**
> >   * @internal
> >