RE: [PATCH] ethdev: introduce generic dummy packet burst function

[Slava Ovsiienko <viacheslavo@nvidia.com>]

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@nvidia.com>

With best regards,
Slava

> -----Original Message-----
> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Sent: Thursday, February 10, 2022 13:52
> To: Morten Brørup <mb@smartsharesystems.com>; Ferruh Yigit
> <ferruh.yigit@intel.com>; Shepard Siegel <shepard.siegel@atomicrules.com>;
> Ed Czeck <ed.czeck@atomicrules.com>; John Miller
> <john.miller@atomicrules.com>; Rasesh Mody <rmody@marvell.com>;
> Shahed Shaikh <shshaikh@marvell.com>; Ajit Khaparde
> <ajit.khaparde@broadcom.com>; Somnath Kotur
> <somnath.kotur@broadcom.com>; Nithin Dabilpuram
> <ndabilpuram@marvell.com>; Kiran Kumar K <kirankumark@marvell.com>;
> Sunil Kumar Kori <skori@marvell.com>; Satha Rao
> <skoteshwar@marvell.com>; Hemant Agrawal <hemant.agrawal@nxp.com>;
> Sachin Saxena <sachin.saxena@oss.nxp.com>; John Daley
> <johndale@cisco.com>; Hyong Youb Kim <hyonkim@cisco.com>; Min Hu
> (Connor) <humin29@huawei.com>; Yisen Zhuang
> <yisen.zhuang@huawei.com>; Lijun Ou <oulijun@huawei.com>; Matan Azrad
> <matan@nvidia.com>; Slava Ovsiienko <viacheslavo@nvidia.com>;
> Gagandeep Singh <g.singh@nxp.com>; Devendra Singh Rawat
> <dsinghrawat@marvell.com>; NBU-Contact-Thomas Monjalon (EXTERNAL)
> <thomas@monjalon.net>
> Cc: dev@dpdk.org; Ciara Loftus <ciara.loftus@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@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@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@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@smartsharesystems.com>
> >>>
> >>
> >