[PATCH] ethdev: introduce generic dummy packet burst function

[Slava Ovsiienko]

Hi,

For mlx4/mlx5 tx_burst we build hardware descriptors, push ones into hardware and write
doorbell register. The doorbell register can be mapped by mlx4/mlx5 into user space either
with non-cache or with write-combining (just regular attribute) memory attributes.
The write-combining mapping requires the explicit memory barrier to push the written
data to the destination hardware, it takes noticeable time and PMDs try to optimize out.

mlx4 just does not perform wmb after the doorbelling, it is always supposed it will happen
on the next call. 

For mlx5 we have the very special Tx doorbelling mode (explicitly controlled via "tx_db_nc" devarg)
to skip the last wmb in tx_burst routine. The user requesting this should understand all risks
and take countermeasures in application if he/she cares about packet drops on queue/device stop.

In the worst case, if wmb is postponed (even never happens anymore) it just causes 
the increased send latency for the packets in the last burst. If queue stop happens during
the "non-promoted doorbell time period" the last burst packets might be dropped (and we suppose
this is not crucial for service being terminated)

To summarize:
- mlx4 is in moderate risk of final send burst packet drop (we've never seen this in practice, just did not check though)
- mlx5 is in minor risk of final send burst packet drop (in very special mode only, and we observed latency issues in practice), can be disregarded
- barrier in dummy routine does not fully resolve this mlx specific issue (not invoked in guaranteed way on right core)

My conclusion - I would prefer to keep barrier in dummy, "just for the case", but have no strong objections about removing,
we can accept the patch being discussed.

Acked-by: Viacheslav Ovsiienko <viacheslavo at nvidia.com>

With best regards,
Slava

> -----Original Message-----
> From: Andrew Rybchenko <andrew.rybchenko at oktetlabs.ru>
> Sent: Thursday, February 10, 2022 13:52
> To: Morten Brørup <mb at smartsharesystems.com>; Ferruh Yigit
> <ferruh.yigit at intel.com>; Shepard Siegel <shepard.siegel at atomicrules.com>;
> Ed Czeck <ed.czeck at atomicrules.com>; John Miller
> <john.miller at atomicrules.com>; Rasesh Mody <rmody at marvell.com>;
> Shahed Shaikh <shshaikh at marvell.com>; Ajit Khaparde
> <ajit.khaparde at broadcom.com>; Somnath Kotur
> <somnath.kotur at broadcom.com>; Nithin Dabilpuram
> <ndabilpuram at marvell.com>; Kiran Kumar K <kirankumark at marvell.com>;
> Sunil Kumar Kori <skori at marvell.com>; Satha Rao
> <skoteshwar at marvell.com>; Hemant Agrawal <hemant.agrawal at nxp.com>;
> Sachin Saxena <sachin.saxena at oss.nxp.com>; John Daley
> <johndale at cisco.com>; Hyong Youb Kim <hyonkim at cisco.com>; Min Hu
> (Connor) <humin29 at huawei.com>; Yisen Zhuang
> <yisen.zhuang at huawei.com>; Lijun Ou <oulijun at huawei.com>; Matan Azrad
> <matan at nvidia.com>; Slava Ovsiienko <viacheslavo at nvidia.com>;
> Gagandeep Singh <g.singh at nxp.com>; Devendra Singh Rawat
> <dsinghrawat at marvell.com>; NBU-Contact-Thomas Monjalon (EXTERNAL)
> <thomas at monjalon.net>
> Cc: dev at dpdk.org; Ciara Loftus <ciara.loftus at intel.com>
> Subject: Re: [PATCH] ethdev: introduce generic dummy packet burst function
> 
> On 2/10/22 14:47, Morten Brørup wrote:
> >> From: Andrew Rybchenko [mailto:andrew.rybchenko at oktetlabs.ru]
> >> Sent: Thursday, 10 February 2022 12.39
> >>
> >> On 2/10/22 14:04, Morten Brørup wrote:
> >>>> From: Ferruh Yigit [mailto:ferruh.yigit at intel.com]
> >>>> Sent: Tuesday, 8 February 2022 20.45
> >>>>
> >>>> Multiple PMDs have dummy/noop Rx/Tx packet burst functions.
> >>>>
> >>>> These dummy functions are very simple, introduce a common function
> >> in
> >>>> the ethdev and update drivers to use it instead of each driver
> >> having
> >>>> its own functions.
> >>>>
> >>>> Signed-off-by: Ferruh Yigit <ferruh.yigit at intel.com>
> >>>
> >>> After briefly considering if the dummy TX should free the burst, I
> >> concluded that the current behavior is correct.
> >>
> >> Could you share your thoughts, please. I'm wondering as well.
> >
> > Returning 0 means that the packets were not transmitted.
> >
> > This leaves it up to the application to decide what to do: drop or retransmit.
> >
> > If the dummy TX function frees the burst, it would effectively mean that the
> driver dropped the packets. (In that case, some drop counters should
> probably also be updated in the driver; but that is irrelevant now.)
> 
> Makes sense, thank you.
> 
> >
> > Not dropping the packets could be significant during startup.
> >
> >>
> >>>
> >>> Good clean-up. :-)
> >>>
> >>> Acked-by: Morten Brørup <mb at smartsharesystems.com>
> >>>
> >>
> >