DPDK patches and discussions
 help / color / mirror / Atom feed
From: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
To: Gavin Hu <Gavin.Hu@arm.com>, "Chauskin, Igor" <igorch@amazon.com>,
	"dev@dpdk.org" <dev@dpdk.org>
Cc: nd <nd@arm.com>,
	"david.marchand@redhat.com" <david.marchand@redhat.com>,
	 "thomas@monjalon.net" <thomas@monjalon.net>,
	"mk@semihalf.com" <mk@semihalf.com>,
	"Tzalik, Guy" <gtzalik@amazon.com>,
	"Schmeilin, Evgeny" <evgenys@amazon.com>,
	"mw@semihalf.com" <mw@semihalf.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>,
	Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>,
	nd <nd@arm.com>
Subject: Re: [dpdk-dev] [PATCH RFC v1 0/7] relax barriers for ENA PMD and small fixes
Date: Tue, 12 May 2020 21:22:45 +0000	[thread overview]
Message-ID: <DBBPR08MB4646D4E167AA99AD8710056D98BE0@DBBPR08MB4646.eurprd08.prod.outlook.com> (raw)
In-Reply-To: <VI1PR08MB537603DEF37EDB2772BF7C918FD50@VI1PR08MB5376.eurprd08.prod.outlook.com>

<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.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
> 


  reply	other threads:[~2020-05-12 21:22 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-13  9:18 Gavin Hu
2020-03-13  9:18 ` [dpdk-dev] [PATCH RFC v1 1/7] net/ena: remove duplicate barrier Gavin Hu
2020-03-13  9:18 ` [dpdk-dev] [PATCH RFC v1 2/7] net/ena: relax the barrier for doorbell ring Gavin Hu
2020-03-13  9:18 ` [dpdk-dev] [PATCH RFC v1 3/7] net/ena: relax the rmb for DMA Gavin Hu
2020-03-13  9:18 ` [dpdk-dev] [PATCH RFC v1 4/7] net/ena: relax barrier for completion queue update Gavin Hu
2020-03-13  9:18 ` [dpdk-dev] [PATCH RFC v1 5/7] net/ena: relax the barrier for bounce buffer Gavin Hu
2020-03-13  9:18 ` [dpdk-dev] [PATCH RFC v1 6/7] net/ena: use c11 atomic for statistics Gavin Hu
2020-03-13  9:18 ` [dpdk-dev] [PATCH RFC v1 7/7] net/ena: remove duplicate memset Gavin Hu
2020-03-16  9:34 ` [dpdk-dev] [PATCH RFC v1 0/7] relax barriers for ENA PMD and small fixes Chauskin, Igor
2020-03-17  7:58   ` Gavin Hu
2020-04-16 13:37     ` Chauskin, Igor
2020-04-21  7:45       ` Gavin Hu
2020-05-12 21:22         ` Honnappa Nagarahalli [this message]
2020-04-15 15:27   ` Ferruh Yigit
2020-04-15 15:59     ` Chauskin, Igor

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DBBPR08MB4646D4E167AA99AD8710056D98BE0@DBBPR08MB4646.eurprd08.prod.outlook.com \
    --to=honnappa.nagarahalli@arm.com \
    --cc=Gavin.Hu@arm.com \
    --cc=Joyce.Kong@arm.com \
    --cc=Phil.Yang@arm.com \
    --cc=Ruifeng.Wang@arm.com \
    --cc=bruce.richardson@intel.com \
    --cc=david.marchand@redhat.com \
    --cc=dev@dpdk.org \
    --cc=evgenys@amazon.com \
    --cc=gtzalik@amazon.com \
    --cc=igorch@amazon.com \
    --cc=matua@amazon.com \
    --cc=mk@semihalf.com \
    --cc=mw@semihalf.com \
    --cc=nd@arm.com \
    --cc=saeedb@amazon.com \
    --cc=thomas@monjalon.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).