Re: [dpdk-dev] [PATCH v2 05/39] examples/l3fwd: move to ethdev offloads API

[Jerin Jacob <jerin.jacob@caviumnetworks.com>]

-----Original Message-----
> Date: Wed, 13 Dec 2017 07:21:01 +0000
> From: Shahaf Shuler <shahafs@mellanox.com>
> To: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>, "dev@dpdk.org"
>  <dev@dpdk.org>, "Nicolau, Radu" <radu.nicolau@intel.com>,
>  "arybchenko@solarflare.com" <arybchenko@solarflare.com>
> Subject: Re: [dpdk-dev] [PATCH v2 05/39] examples/l3fwd: move to ethdev
>  offloads API
> 
> Tuesday, December 12, 2017 7:12 PM, Ananyev, Konstantin:
> > > -----Original Message-----
> > > From: Shahaf Shuler [mailto:shahafs@mellanox.com]
> > > Sent: Tuesday, December 12, 2017 12:26 PM
> > > To: dev@dpdk.org; Ananyev, Konstantin
> > <konstantin.ananyev@intel.com>;
> > > Nicolau, Radu <radu.nicolau@intel.com>; arybchenko@solarflare.com
> > > Subject: [PATCH v2 05/39] examples/l3fwd: move to ethdev offloads API
> > >
> > > Ethdev offloads API has changed since:
> > >
> > > commit ce17eddefc20 ("ethdev: introduce Rx queue offloads API") commit
> > > cba7f53b717d ("ethdev: introduce Tx queue offloads API")
> > >
> > > This commit support the new API.
> > >
> > > Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
> > > ---
> > >  examples/l3fwd/main.c | 40 ++++++++++++++++++++++++++++++--------
> > --
> > >  1 file changed, 30 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/examples/l3fwd/main.c b/examples/l3fwd/main.c index
> > > 6229568..3bdf4d5 100644
> > > --- a/examples/l3fwd/main.c
> > > +++ b/examples/l3fwd/main.c
> > > @@ -149,11 +149,9 @@ struct lcore_params {
> > >  		.mq_mode = ETH_MQ_RX_RSS,
> > >  		.max_rx_pkt_len = ETHER_MAX_LEN,
> > >  		.split_hdr_size = 0,
> > > -		.header_split   = 0, /**< Header Split disabled */
> > > -		.hw_ip_checksum = 1, /**< IP checksum offload enabled */
> > > -		.hw_vlan_filter = 0, /**< VLAN filtering disabled */
> > > -		.jumbo_frame    = 0, /**< Jumbo Frame Support disabled */
> > > -		.hw_strip_crc   = 1, /**< CRC stripped by hardware */
> > > +		.ignore_offload_bitfield = 1,
> > > +		.offloads = (DEV_RX_OFFLOAD_CRC_STRIP |
> > > +			     DEV_RX_OFFLOAD_CHECKSUM),
> > >  	},
> > >  	.rx_adv_conf = {
> > >  		.rss_conf = {
> > > @@ -163,6 +161,7 @@ struct lcore_params {
> > >  	},
> > >  	.txmode = {
> > >  		.mq_mode = ETH_MQ_TX_NONE,
> > > +		.offloads = DEV_TX_OFFLOAD_MBUF_FAST_FREE,
> > 
> > Hmm, does it mean a new warning for all PMDs (majority) which don't
> > support DEV_TX_OFFLOAD_MBUF_FAST_FREE?
> 
> Good point.
> Unlike other offloads which are must for the application proper run, this one it only for optimizing the performance and should be set only if PMD supports. 
> Am continuing to aggregate reasons why the DEV_TX_OFFLOAD_MBUF_FAST_FREE should not be defined as an offload. Anyway we passed that... 
> 
> I will fix on v3.

Removing is not an option as the PMDs rely on that flag to will have the
impact.
# I see DEV_TX_OFFLOAD_MBUF_FAST_FREE as hint driver to depict the application requirements
# All the drivers by default can support DEV_TX_OFFLOAD_MBUF_FAST_FREE(They are using the hint or
not is a different question)

So, How about setting DEV_TX_OFFLOAD_MBUF_FAST_FREE in all PMD driver as
dummy one? I think, currently, it can be moved to old API to new API
transition function till the drivers change to new offload flag scheme.

We are planning to change nicvf driver to new offload scheme for this
release so with this change, we have the performance impact on l3fwd
application.

I think, the other option could be to change usage/meaning of
DEV_TX_OFFLOAD_MBUF_FAST_FREE flag where when the application needs
multi-pool and reference count scheme then "it sets" the offload flags.
If so, we don't need to set by default on the these applications.