Re: [dpdk-dev] [PATCH v11 1/6] ethdev: add Tx preparation

[Thomas Monjalon <thomas.monjalon@6wind.com>]

2016-10-28 11:34, Ananyev, Konstantin:
> > > 2016-10-27 16:24, Ananyev, Konstantin:
> > > > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > > > 2016-10-27 15:52, Ananyev, Konstantin:
> > > > > > > 2016-10-26 14:56, Tomasz Kulasek:
> > > > > > > > --- a/config/common_base
> > > > > > > > +++ b/config/common_base
> > > > > > > > +CONFIG_RTE_ETHDEV_TX_PREP=y
> > > > > > >
> > > > > > > We cannot enable it until it is implemented in every drivers.
> > > > > >
> > > > > > Not sure why?
> > > > > > If tx_pkt_prep == NULL, then rte_eth_tx_prep() would just act as noop.
> > > > > > Right now it is not mandatory for the PMD to implement it.
> > > > >
> > > > > If it is not implemented, the application must do the preparation by itself.
> > > > > From patch 6:
> > > > > "
> > > > > Removed pseudo header calculation for udp/tcp/tso packets from
> > > > > application and used Tx preparation API for packet preparation and
> > > > > verification.
> > > > > "
> > > > > So how does it behave with other drivers?
> > > >
> > > > Hmm so it seems that we broke testpmd csumonly mode for non-intel drivers..
> > > > My bad, missed that part completely.
> > > > Yes, then I suppose for now we'll need to support both (with and without) code paths for testpmd.
> > > > Probably a new fwd mode or just extra parameter for the existing one?
> > > > Any other suggestions?
> > >
> > > Please think how we can use it in every applications.
> > > It is not ready.
> > > Either we introduce the API without enabling it, or we implement it
> > > in every drivers.
> > 
> > I understand your position here, but just like to point that:
> > 1) It is a new functionality optional to use.
> >      The app is free not to use that functionality and still do the preparation itself
> >      (as it has to do it now).
> >     All existing apps would keep working as expected without using that function.
> >     Though if the app developer knows that for all HW models he plans to run on
> >     tx_prep is implemented - he is free to use it.
> >     2) It would be difficult for Tomasz (and other Intel guys) to implement tx_prep()
> >      for all non-Intel HW that DPDK supports right now.
> >      We just don't have all the actual HW in stock and probably adequate knowledge of it.
> >     So we depend here on the good will of other PMD mainaners/developers to implement
> >     tx_prep() for these devices.
> >     From other side, if it will be disabled by default, then, I think,
> >     PMD developers just wouldn't be motivated to implement it.
> >     So it will be left untested and unused forever.
> 
> Actually as another thought:
> Can we have it enabled by default, but mark it as experimental or so?
> If memory serves me right, we've done that for cryptodev in the past, no?

Cryptodev was a whole new library.
We won't play the game "find which function is experimental or not".

We should not enable a function until it is fully implemented.

If the user really understands that it will work only with few drivers
then he can change the build configuration himself.
Enabling in the default configuration is a message to say that it works
everywhere without any risk.
It's so simple that I don't even understand why I must argue for.

And by the way, it is late for 16.11.
I suggest to integrate it in the beginning of 17.02 cycle, with the hope
that you can convince other developers to implement it in other drivers,
so we could finally enable it in the default config.

Oh, and I don't trust that nobody were thinking that it would break testpmd
for non-Intel drivers.