<snip>
Hi Igor,
Few comments inline.
> > 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.
I think https://patches.dpdk.org/patch/70091/ should help in this. I have added
to that patch, please take a look.
> >
> > 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.
Do you have any particular compiler version in mind? There is a proposal [1] to
stop using rte_atomicNN_xxx APIs and use wrappers built around c11 __atomic
built-ins. C11 is supported in GCC from 4.7 and in clang from 3.1.
[1] https://patches.dpdk.org/cover/70097/
> >
> > 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...@arm.com>
> > Sent: Tuesday, March 17, 2020 9:59 AM
> > To: Chauskin, Igor <igo...@amazon.com>; dev@dpdk.org
> > Cc: nd <n...@arm.com>; david.march...@redhat.com;
> tho...@monjalon.net;
> > m...@semihalf.com; Tzalik, Guy <gtza...@amazon.com>; Schmeilin, Evgeny
> > <evge...@amazon.com>; m...@semihalf.com; Honnappa Nagarahalli
> > <honnappa.nagaraha...@arm.com>; Ruifeng Wang
> <ruifeng.w...@arm.com>;
> > Phil Yang <phil.y...@arm.com>; Joyce Kong <joyce.k...@arm.com>;
> > Bshara, Saeed <sae...@amazon.com>; Matushevsky, Alexander
> > <ma...@amazon.com>; Bruce Richardson <bruce.richard...@intel.com>;
> nd
> > <n...@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 <igo...@amazon.com>
> > > Sent: Monday, March 16, 2020 5:35 PM
> > > To: Gavin Hu <gavin...@arm.com>; dev@dpdk.org
> > > Cc: nd <n...@arm.com>; david.march...@redhat.com;
> > tho...@monjalon.net;
> > > m...@semihalf.com; Tzalik, Guy <gtza...@amazon.com>; Schmeilin, Evgeny
> > > <evge...@amazon.com>; m...@semihalf.com; Honnappa Nagarahalli
> > > <honnappa.nagaraha...@arm.com>; Ruifeng Wang
> > <ruifeng.w...@arm.com>;
> > > Phil Yang <phil.y...@arm.com>; Joyce Kong <joyce.k...@arm.com>;
> > > Bshara, Saeed <sae...@amazon.com>; Matushevsky, Alexander
> > > <ma...@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...@arm.com>
> > > Sent: Friday, March 13, 2020 11:18 AM
> > > To: dev@dpdk.org
> > > Cc: n...@arm.com; david.march...@redhat.com; tho...@monjalon.net;
> > > m...@semihalf.com; Tzalik, Guy <gtza...@amazon.com>; Schmeilin, Evgeny
> > > <evge...@amazon.com>; Chauskin, Igor <igo...@amazon.com>;
> > > m...@semihalf.com; honnappa.nagaraha...@arm.com;
> > ruifeng.w...@arm.com;
> > > phil.y...@arm.com; joyce.k...@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
>
