RE: [PATCH v2] app/testpmd: use Tx preparation in txonly engine

[Konstantin Ananyev <konstantin.ananyev@huawei.com>]

> > > > TSO breaks when MSS spans more than 8 data fragments. Those
> > > > packets will be dropped by Tx preparation API, but it will cause
> > > > MDD event if txonly forwarding engine does not call the Tx
> > preparation
> > > > API before transmitting packets.
> > > >
> > >
> > > txonly is used commonly, adding Tx prepare for a specific case may
> > > impact performance for users.
> > >
> > > What happens when driver throws MDD (Malicious Driver Detection)
> > event,
> > > can't it be ignored? As you are already OK to drop the packet, can
> > > device be configured to drop these packages?
> > >
> > >
> > > Or as Jerin suggested adding a new forwarding engine is a solution,
> > but
> > > that will create code duplication, I prefer to not have it if this
> > can
> > > be handled in device level.
> >
> > Actually I am agree with the author of the patch - when TX offloads
> > and/or multisegs are enabled,
> > user supposed to invoke eth_tx_prepare().
> > Not doing that seems like a bug to me.
> 
> I strongly disagree with that statement, Konstantin!
> It is not documented anywhere that using TX offloads and/or multisegs requires calling rte_eth_tx_prepare() before
> rte_eth_tx_burst(). And none of the examples do it.

In fact, we do use it for test-pmd/csumonly.c.
About other sample apps:
AFAIK, not many of other DPDK apps do use L4 offloads.
Right now special treatment (pseudo-header cksum calculation) is needed only for L4 offloads (CKSUM, SEG).
So, majority of our apps who rely on other TX offloads (multi-seg, ipv4 cksum, vlan insertion) happily run without
calling tx_prepare(), even though it is not the safest way.   

> 
> In my opinion:
> If some driver has limitations for a feature, e.g. max 8 fragments, it should be documented for that driver, so the application
> developer can make the appropriate decisions when designing the application.
> Furthermore, we have APIs for the drivers to expose to the applications what the driver supports, so the application can configure
> itself optimally at startup. Perhaps those APIs need to be expanded.
> And if a feature limitation is common across the majority of drivers, that limitation should be mentioned in the documentation of the
> feature itself.

Many of such limitations *are* documented and in fact we do have an API to check max segments that each driver support,
see struct rte_eth_desc_lim.
The problem is:
- none of our sample app does proper check on these values, so users don't have a good example how to do it.
- with current DPDK API not all of HW/PMD requirements could be extracted programmatically:
  let say majority of Intel PMDs for TCP offloads expect pseudo-header cksum to be pre-calculated by the SW.
  another example, some HW expects pkt_len to be bigger then some threshold value, otherwise HW hang may appear.
- As new HW and PMD keep appearing it is hard to predict what extra limitations/requirements will arise,
  that's why tx_prepare() was introduced as s driver op.    

> 
> We don't want to check in the fast path what can be checked at startup or build time!

If your app supposed to work with just a few, known in advance, NIC models, then sure, you can do that.
For apps that supposed to work 'in general'  with any possible PMDs that DPDK supports - that might be a problem.
That's why tx_prepare() was introduced and it is strongly recommended to use it by the apps that do use TX offloads.
Probably tx_prepare() is not the best possible approach, but right now there are not many alternatives within DPDK.

> 
> > If it still works for some cases, that's a lucky coincidence, but not
> > the expected behavior.
> > About performance - first we can check is that really a drop.
> > Also as I remember most drivers set it to non-NULL value, only when
> > some TX offloads were
> > enabled by the user on that port, so hopefully for simple case (one
> > segment, no tx offloads) it
> > should be negligible.
> > Again, we can add manual check in testpmd tx-only code to decide do we
> > need a TX prepare
> > to be called or not.
> > Konstantin