Re: [dpdk-dev] [PATCH RFC v1 0/7] relax barriers for ENA PMD and small fixes

[Gavin Hu <Gavin.Hu@arm.com>]

> -----Original Message-----
> From: Chauskin, Igor <igorch@amazon.com>
> Sent: Thursday, April 16, 2020 9:38 PM
> To: Gavin Hu <Gavin.Hu@arm.com>; dev@dpdk.org
> Cc: nd <nd@arm.com>; david.marchand@redhat.com;
> thomas@monjalon.net; mk@semihalf.com; Tzalik, Guy
> <gtzalik@amazon.com>; Schmeilin, Evgeny <evgenys@amazon.com>;
> mw@semihalf.com; Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com>; Ruifeng Wang
> <Ruifeng.Wang@arm.com>; Phil Yang <Phil.Yang@arm.com>; Joyce Kong
> <Joyce.Kong@arm.com>; Bshara, Saeed <saeedb@amazon.com>;
> Matushevsky, Alexander <matua@amazon.com>; Bruce Richardson
> <bruce.richardson@intel.com>; nd <nd@arm.com>
> Subject: RE: [PATCH RFC v1 0/7] relax barriers for ENA PMD and small fixes
> 
> Hi all,
> 
> Please see the first batch of comments related to these patches:
> 
> 1. Relaxing the register read/write isn't always good enough. Specifically,
> when barriers are required between different memory types, the reordering
> can occur even on x86. Yet in DPDK the io/cio/smp barrier flavors for x86
> are defined as compiler-only barriers, which is not enough in cases involving
> different memory types. In ENA driver, when LLQ is on, there is a regular
> register memory access before the barrier and write-combined memory
> access after the barrier.
That's makes sense, we realized that also, we don't mean to change x86 behaviors.
> 
> We're working on a more extensive change that will include the
> optimizations proposed for barriers relaxing while making them applicable
> to all platforms.
We are working also on an extensive change, helpful to arm64 while not impacting x86.
More testing is ongoing. 
> 
> 2.  Regarding the changes for statistics logging - the patch relies on c11
> features. I'm not sure it's acceptable for all situations since we've already
> encountered a reports when even c99-compliant changes caused
> compilation issues.
C11 is already widely used in other components, even in other projects like vpp and ovs. 
Maybe it comes to time to drop C99 as a stringent requirement. 
> 
> 3. Removing redundant zeroing of sub-struct - we're currently working on
> some extensive changes to the Tx flow, which will include this change
> among other.
Ok, thanks. 
> 
> Thanks,
> Igor
> 
> -----Original Message-----
> From: Gavin Hu <Gavin.Hu@arm.com>
> Sent: Tuesday, March 17, 2020 9:59 AM
> To: Chauskin, Igor <igorch@amazon.com>; dev@dpdk.org
> Cc: nd <nd@arm.com>; david.marchand@redhat.com;
> thomas@monjalon.net; mk@semihalf.com; Tzalik, Guy
> <gtzalik@amazon.com>; Schmeilin, Evgeny <evgenys@amazon.com>;
> mw@semihalf.com; Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com>; Ruifeng Wang
> <Ruifeng.Wang@arm.com>; Phil Yang <Phil.Yang@arm.com>; Joyce Kong
> <Joyce.Kong@arm.com>; Bshara, Saeed <saeedb@amazon.com>;
> Matushevsky, Alexander <matua@amazon.com>; Bruce Richardson
> <bruce.richardson@intel.com>; nd <nd@arm.com>
> Subject: RE: [EXTERNAL] [PATCH RFC v1 0/7] relax barriers for ENA PMD and
> small fixes
> 
> CAUTION: This email originated from outside of the organization. Do not
> click links or open attachments unless you can confirm the sender and know
> the content is safe.
> 
> 
> 
> Hi Igor,
> 
> > -----Original Message-----
> > From: Chauskin, Igor <igorch@amazon.com>
> > Sent: Monday, March 16, 2020 5:35 PM
> > To: Gavin Hu <Gavin.Hu@arm.com>; dev@dpdk.org
> > Cc: nd <nd@arm.com>; david.marchand@redhat.com;
> thomas@monjalon.net;
> > mk@semihalf.com; Tzalik, Guy <gtzalik@amazon.com>; Schmeilin, Evgeny
> > <evgenys@amazon.com>; mw@semihalf.com; Honnappa Nagarahalli
> > <Honnappa.Nagarahalli@arm.com>; Ruifeng Wang
> <Ruifeng.Wang@arm.com>;
> > Phil Yang <Phil.Yang@arm.com>; Joyce Kong <Joyce.Kong@arm.com>;
> > Bshara, Saeed <saeedb@amazon.com>; Matushevsky, Alexander
> > <matua@amazon.com>
> > Subject: RE: [PATCH RFC v1 0/7] relax barriers for ENA PMD and small
> > fixes
> >
> > Hi Gavin,
> >
> > Thank you for the contribution.
> > Please do not merge these changes (patches 0..7) till we (the ENA
> > team) properly review and ack/nack.
> > These changes can potentially provide performance improvement, yet we
> > need to ensure they are applicable for all possible scenarios.
> > Specifically, the behavior on x86 platforms is likely to be different.
> > What testing have you done for these patches? Was x86 tested?
> As noted in the cover letter, these patches were not tested as we don't have
> ENA NICs.
> We rely on you to do that, any concerns and comments welcome.
> Yes, the behavior on x86 platforms is also different, Intel people are
> welcome to comment.
> /Gavin
> >
> > Thanks,
> > Igor
> >
> > -----Original Message-----
> > From: Gavin Hu <gavin.hu@arm.com>
> > Sent: Friday, March 13, 2020 11:18 AM
> > To: dev@dpdk.org
> > Cc: nd@arm.com; david.marchand@redhat.com; thomas@monjalon.net;
> > mk@semihalf.com; Tzalik, Guy <gtzalik@amazon.com>; Schmeilin, Evgeny
> > <evgenys@amazon.com>; Chauskin, Igor <igorch@amazon.com>;
> > mw@semihalf.com; Honnappa.Nagarahalli@arm.com;
> ruifeng.wang@arm.com;
> > phil.yang@arm.com; joyce.kong@arm.com
> > Subject: [EXTERNAL][PATCH RFC v1 0/7] relax barriers for ENA PMD and
> > small fixes
> >
> > CAUTION: This email originated from outside of the organization. Do
> > not click links or open attachments unless you can confirm the sender
> > and know the content is safe.
> >
> >
> >
> > To ensure the stores to the host memory are observed by NIC HW before
> > a door bell ring to the NIC HW and the HW starts actions, for example,
> > doing DMA, a barrier is required on weak memory ordering platforms, like
> aarch64.
> >
> > However, unnecessarily too strong barriers like 'dsb' on aarch64 will
> > dampen performance.
> >
> > In a typical doorbell use case, as NIC and CPU are in the outer
> > sharable domain, a lighter weight 'dmb osh' barrier is sufficient.
> >
> > The patch set relaxes the barriers in similar places and include one
> > more patch for statistics logging with relaxed ordering and the other
> > patch removing duplicate memset.
> >
> > Note this set is submitted for RFC as we don't have physical ENA NICs
> > in the lab and the patch set was not verified nor benchmarked.
> >
> > Gavin Hu (7):
> >   net/ena: remove duplicate barrier
> >   net/ena: relax the barrier for doorbell ring
> >   net/ena: relax the rmb for DMA
> >   net/ena: relax barrier for completion queue update
> >   net/ena: relax the barrier for bounce buffer
> >   net/ena: use c11 atomic for statistics
> >   net/ena: remove duplicate memset
> >
> >  drivers/net/ena/base/ena_eth_com.c   |  2 +-
> >  drivers/net/ena/base/ena_eth_com.h   |  6 ++--
> >  drivers/net/ena/base/ena_plat_dpdk.h |  2 +-
> >  drivers/net/ena/ena_ethdev.c         | 46 +++++++++++++++++-----------
> >  drivers/net/ena/ena_ethdev.h         |  8 ++---
> >  5 files changed, 38 insertions(+), 26 deletions(-)
> >
> > --
> > 2.17.1