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

["Chauskin, Igor" <igorch@amazon.com>]

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.

We're working on a more extensive change that will include the optimizations proposed for barriers relaxing while making them applicable to all platforms.

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.

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.

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