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

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

[Gavin Hu]

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

[dpdk-dev] [PATCH RFC v1 1/7] net/ena: remove duplicate barrier

[Gavin Hu]

As there is necessary barriers before calling the function everywhere,
then using the relaxed write function ENA_REG_WRITE32_RELAXED is ok.

Fixes: 99ecfbf845b3 ("ena: import communication layer")
Cc: stable@dpdk.org

Signed-off-by: Gavin Hu <gavin.hu@arm.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
---
 drivers/net/ena/base/ena_eth_com.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ena/base/ena_eth_com.h b/drivers/net/ena/base/ena_eth_com.h
index e37b642d4..e56c33a64 100644
--- a/drivers/net/ena/base/ena_eth_com.h
+++ b/drivers/net/ena/base/ena_eth_com.h
@@ -156,7 +156,7 @@ static inline int ena_com_write_sq_doorbell(struct ena_com_io_sq *io_sq)
 	ena_trc_dbg("write submission queue doorbell for queue: %d tail: %d\n",
 		    io_sq->qid, tail);
 
-	ENA_REG_WRITE32(io_sq->bus, tail, io_sq->db_addr);
+	ENA_REG_WRITE32_RELAXED(io_sq->bus, tail, io_sq->db_addr);
 
 	if (is_llq_max_tx_burst_exists(io_sq)) {
 		ena_trc_dbg("reset available entries in tx burst for queue %d to %d\n",
-- 
2.17.1

[dpdk-dev] [PATCH RFC v1 2/7] net/ena: relax the barrier for doorbell ring

[Gavin Hu]

rte_cio_wmb is a light weight version of IO memory barrier that
guarantees that the stores to the coherent memory prior to the
the rte_cio_rmb() call are visible to the NIC HW before the doorbell
ring(or any other stores to the MMIO registers) that follows it.[1]

Fixes: 5e02e19eb14e ("net/ena: fix unneeded doorbell submission")
Cc: stable@dpdk.org

[1] http://code.dpdk.org/dpdk/v20.02/source/lib/librte_eal/common/
include/generic/rte_atomic.h#L137

Signed-off-by: Gavin Hu <gavin.hu@arm.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
---
 drivers/net/ena/ena_ethdev.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index 665afee4f..c268788fd 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -1425,7 +1425,7 @@ static int ena_populate_rx_queue(struct ena_ring *rxq, unsigned int count)
 		 * Add memory barrier to make sure the desc were written before
 		 * issue a doorbell
 		 */
-		rte_wmb();
+		rte_cio_wmb();
 		ena_com_write_sq_doorbell(rxq->ena_com_io_sq);
 
 		rxq->next_to_use = next_to_use;
@@ -2344,7 +2344,7 @@ static uint16_t eth_ena_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 			PMD_DRV_LOG(DEBUG, "llq tx max burst size of queue %d"
 				" achieved, writing doorbell to send burst\n",
 				tx_ring->id);
-			rte_wmb();
+			rte_cio_wmb();
 			ena_com_write_sq_doorbell(tx_ring->ena_com_io_sq);
 		}
 
@@ -2367,7 +2367,7 @@ static uint16_t eth_ena_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 	/* If there are ready packets to be xmitted... */
 	if (sent_idx > 0) {
 		/* ...let HW do its best :-) */
-		rte_wmb();
+		rte_cio_wmb();
 		ena_com_write_sq_doorbell(tx_ring->ena_com_io_sq);
 		tx_ring->tx_stats.doorbells++;
 		tx_ring->next_to_use = next_to_use;
-- 
2.17.1

[dpdk-dev] [PATCH RFC v1 3/7] net/ena: relax the rmb for DMA

[Gavin Hu]

The user space DPDK rte_cio_rmb barrier in definition corresponds to the
kernel dma_rmb barrier on all supported architectures[1][2][3].

As it is called in the data path[4], redefine it to relax the barrier and
uplift the performance.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/
linux.git/tree/arch/x86/include/asm/barrier.h?h=v5.5#n54
[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/
linux.git/tree/arch/powerpc/include/asm/barrier.h?h=v5.5#n46
[3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/
linux.git/tree/arch/arm64/include/asm/barrier.h?h=v5.5#n48
[4] http://code.dpdk.org/dpdk/v20.02/source/drivers/net/ena/
ena_ethdev.c#L2021

Fixes: b68309be44c0 ("net/ena/base: update communication layer for the ENAv2")
Cc: stable@dpdk.org

Signed-off-by: Gavin Hu <gavin.hu@arm.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
---
 drivers/net/ena/base/ena_plat_dpdk.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ena/base/ena_plat_dpdk.h b/drivers/net/ena/base/ena_plat_dpdk.h
index b611fb204..60327a726 100644
--- a/drivers/net/ena/base/ena_plat_dpdk.h
+++ b/drivers/net/ena/base/ena_plat_dpdk.h
@@ -254,7 +254,7 @@ extern uint32_t ena_alloc_cnt;
 #define msleep(x) rte_delay_us(x * 1000)
 #define udelay(x) rte_delay_us(x)
 
-#define dma_rmb() rmb()
+#define dma_rmb() rte_cio_rmb()
 
 #define MAX_ERRNO       4095
 #define IS_ERR(x) (((unsigned long)x) >= (unsigned long)-MAX_ERRNO)
-- 
2.17.1

[dpdk-dev] [PATCH RFC v1 4/7] net/ena: relax barrier for completion queue update

[Gavin Hu]

To gaurantee the update observed by NIC HW, a cio barrier is sufficient,
an io barrier, which translates to dsb on aarch64, is overkill.

Suggested-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
Signed-off-by: Gavin Hu <gavin.hu@arm.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
---
 drivers/net/ena/base/ena_eth_com.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ena/base/ena_eth_com.h b/drivers/net/ena/base/ena_eth_com.h
index e56c33a64..edfa98b72 100644
--- a/drivers/net/ena/base/ena_eth_com.h
+++ b/drivers/net/ena/base/ena_eth_com.h
@@ -180,7 +180,9 @@ static inline int ena_com_update_dev_comp_head(struct ena_com_io_cq *io_cq)
 		if (unlikely(need_update)) {
 			ena_trc_dbg("Write completion queue doorbell for queue %d: head: %d\n",
 				    io_cq->qid, head);
-			ENA_REG_WRITE32(io_cq->bus, head, io_cq->cq_head_db_reg);
+			rte_cio_wmb();
+			ENA_REG_WRITE32_RELAXED(io_cq->bus, head,
+				       io_cq->cq_head_db_reg);
 			io_cq->last_head_update = head;
 		}
 	}
-- 
2.17.1

[dpdk-dev] [PATCH RFC v1 5/7] net/ena: relax the barrier for bounce buffer

[Gavin Hu]

To gaurantee the update observed by NIC HW, a cio barrier is sufficient,
a io barrier, which translates to dsb on aarch64, is overkill.

Suggested-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
Signed-off-by: Gavin Hu <gavin.hu@arm.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
---
 drivers/net/ena/base/ena_eth_com.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ena/base/ena_eth_com.c b/drivers/net/ena/base/ena_eth_com.c
index d4d44226d..fce3f69eb 100644
--- a/drivers/net/ena/base/ena_eth_com.c
+++ b/drivers/net/ena/base/ena_eth_com.c
@@ -69,7 +69,7 @@ static int ena_com_write_bounce_buffer_to_dev(struct ena_com_io_sq *io_sq,
 	/* Make sure everything was written into the bounce buffer before
 	 * writing the bounce buffer to the device
 	 */
-	wmb();
+	rte_cio_wmb();
 
 	/* The line is completed. Copy it to dev */
 	ENA_MEMCPY_TO_DEVICE_64(io_sq->desc_addr.pbuf_dev_addr + dst_offset,
-- 
2.17.1

[dpdk-dev] [PATCH RFC v1 6/7] net/ena: use c11 atomic for statistics

[Gavin Hu]

The statistics logging are in the data path, as rte_atomic APIs enforce
unnecessary barriers on aarch64, this patch uses c11 atomic APIs with
the __ATOMIC_RELAXED memory ordering to give CPU more freedom to optimize
by making use of out-of-order execution.

Signed-off-by: Gavin Hu <gavin.hu@arm.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
---
 drivers/net/ena/ena_ethdev.c | 38 ++++++++++++++++++++++++------------
 drivers/net/ena/ena_ethdev.h |  8 ++++----
 2 files changed, 29 insertions(+), 17 deletions(-)

diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index c268788fd..8b566aecd 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -8,7 +8,6 @@
 #include <rte_ethdev_driver.h>
 #include <rte_ethdev_pci.h>
 #include <rte_tcp.h>
-#include <rte_atomic.h>
 #include <rte_dev.h>
 #include <rte_errno.h>
 #include <rte_version.h>
@@ -902,10 +901,14 @@ static void ena_stats_restart(struct rte_eth_dev *dev)
 {
 	struct ena_adapter *adapter = dev->data->dev_private;
 
-	rte_atomic64_init(&adapter->drv_stats->ierrors);
-	rte_atomic64_init(&adapter->drv_stats->oerrors);
-	rte_atomic64_init(&adapter->drv_stats->rx_nombuf);
-	rte_atomic64_init(&adapter->drv_stats->rx_drops);
+	__atomic_store_n(&adapter->drv_stats->ierrors, 1,
+		       __ATOMIC_RELAXED);
+	__atomic_store_n(&adapter->drv_stats->oerrors, 1,
+		       __ATOMIC_RELAXED);
+	__atomic_store_n(&adapter->drv_stats->rx_nombuf, 1,
+		       __ATOMIC_RELAXED);
+	__atomic_store_n(&adapter->drv_stats->rx_drops, 1,
+		       __ATOMIC_RELAXED);
 }
 
 static int ena_stats_get(struct rte_eth_dev *dev,
@@ -939,10 +942,14 @@ static int ena_stats_get(struct rte_eth_dev *dev,
 					ena_stats.tx_bytes_low);
 
 	/* Driver related stats */
-	stats->imissed = rte_atomic64_read(&adapter->drv_stats->rx_drops);
-	stats->ierrors = rte_atomic64_read(&adapter->drv_stats->ierrors);
-	stats->oerrors = rte_atomic64_read(&adapter->drv_stats->oerrors);
-	stats->rx_nombuf = rte_atomic64_read(&adapter->drv_stats->rx_nombuf);
+	stats->imissed = __atomic_load_n(&adapter->drv_stats->rx_drops,
+		       __ATOMIC_RELAXED);
+	stats->ierrors = __atomic_load_n(&adapter->drv_stats->ierrors,
+		       __ATOMIC_RELAXED);
+	stats->oerrors = __atomic_load_n(&adapter->drv_stats->oerrors,
+		       __ATOMIC_RELAXED);
+	stats->rx_nombuf = __atomic_load_n(&adapter->drv_stats->rx_nombuf,
+		       __ATOMIC_RELAXED);
 
 	max_rings_stats = RTE_MIN(dev->data->nb_rx_queues,
 		RTE_ETHDEV_QUEUE_STAT_CNTRS);
@@ -1376,7 +1383,8 @@ static int ena_populate_rx_queue(struct ena_ring *rxq, unsigned int count)
 	/* get resources for incoming packets */
 	rc = rte_mempool_get_bulk(rxq->mb_pool, (void **)mbufs, count);
 	if (unlikely(rc < 0)) {
-		rte_atomic64_inc(&rxq->adapter->drv_stats->rx_nombuf);
+		__atomic_add_fetch(&rxq->adapter->drv_stats->rx_nombuf, 1,
+				__ATOMIC_RELAXED);
 		++rxq->rx_stats.mbuf_alloc_fail;
 		PMD_RX_LOG(DEBUG, "there are no enough free buffers");
 		return 0;
@@ -2074,7 +2082,9 @@ static uint16_t eth_ena_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 
 		if (unlikely(mbuf_head->ol_flags &
 				(PKT_RX_IP_CKSUM_BAD | PKT_RX_L4_CKSUM_BAD))) {
-			rte_atomic64_inc(&rx_ring->adapter->drv_stats->ierrors);
+			__atomic_add_fetch
+				(&rx_ring->adapter->drv_stats->ierrors,
+				1, __ATOMIC_RELAXED);
 			++rx_ring->rx_stats.bad_csum;
 		}
 
@@ -2214,7 +2224,8 @@ static int ena_check_and_linearize_mbuf(struct ena_ring *tx_ring,
 	rc = rte_pktmbuf_linearize(mbuf);
 	if (unlikely(rc)) {
 		PMD_DRV_LOG(WARNING, "Mbuf linearize failed\n");
-		rte_atomic64_inc(&tx_ring->adapter->drv_stats->ierrors);
+		__atomic_add_fetch(&tx_ring->adapter->drv_stats->ierrors,
+			       1, __ATOMIC_RELAXED);
 		++tx_ring->tx_stats.linearize_failed;
 		return rc;
 	}
@@ -2682,7 +2693,8 @@ static void ena_keep_alive(void *adapter_data,
 
 	desc = (struct ena_admin_aenq_keep_alive_desc *)aenq_e;
 	rx_drops = ((uint64_t)desc->rx_drops_high << 32) | desc->rx_drops_low;
-	rte_atomic64_set(&adapter->drv_stats->rx_drops, rx_drops);
+	__atomic_store_n(&adapter->drv_stats->rx_drops, rx_drops,
+		       __ATOMIC_RELAXED);
 }
 
 /**
diff --git a/drivers/net/ena/ena_ethdev.h b/drivers/net/ena/ena_ethdev.h
index af5eeea28..e7d39c09f 100644
--- a/drivers/net/ena/ena_ethdev.h
+++ b/drivers/net/ena/ena_ethdev.h
@@ -129,10 +129,10 @@ enum ena_adapter_state {
 };
 
 struct ena_driver_stats {
-	rte_atomic64_t ierrors;
-	rte_atomic64_t oerrors;
-	rte_atomic64_t rx_nombuf;
-	rte_atomic64_t rx_drops;
+	uint64_t ierrors;
+	uint64_t oerrors;
+	uint64_t rx_nombuf;
+	uint64_t rx_drops;
 };
 
 struct ena_stats_dev {
-- 
2.17.1

[dpdk-dev] [PATCH RFC v1 7/7] net/ena: remove duplicate memset

[Gavin Hu]

There is a memset operation for the whole structure so no need
to memset the sub fields in it.

Fixes: 1173fca25af9 ("ena: add polling-mode driver")
Cc: stable@dpdk.org

Suggested-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
Signed-off-by: Gavin Hu <gavin.hu@arm.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
---
 drivers/net/ena/ena_ethdev.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index 8b566aecd..ab28cfc42 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -2280,8 +2280,6 @@ static uint16_t eth_ena_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 
 		/* Prepare TX context */
 		memset(&ena_tx_ctx, 0x0, sizeof(struct ena_com_tx_ctx));
-		memset(&ena_tx_ctx.ena_meta, 0x0,
-		       sizeof(struct ena_com_tx_meta));
 		ena_tx_ctx.ena_bufs = ebuf;
 		ena_tx_ctx.req_id = req_id;
 
-- 
2.17.1

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

[Chauskin, Igor]

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?

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

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

[Gavin Hu]

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

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

[Chauskin, Igor]

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

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

[Gavin Hu]

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

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

[Honnappa Nagarahalli]

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

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

[Ferruh Yigit]

On 3/16/2020 9:34 AM, Chauskin, Igor wrote:
> 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.

Hi Igor,

Is there any progress on reviewing the set?

Thanks,
ferruh

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

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

[Chauskin, Igor]

Hi,

These changes are ARM-oriented and the behavior will be different on x86-based systems. As such, we need to generalize them and it's not straightforward - we're still reviewing the implications. I will update when we're ready.

Thanks,
Igor 

-----Original Message-----
From: Ferruh Yigit <ferruh.yigit@intel.com> 
Sent: Wednesday, April 15, 2020 6:28 PM
To: Chauskin, Igor <igorch@amazon.com>; Gavin Hu <gavin.hu@arm.com>; 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>; mw@semihalf.com; Honnappa.Nagarahalli@arm.com; ruifeng.wang@arm.com; phil.yang@arm.com; joyce.kong@arm.com; Bshara, Saeed <saeedb@amazon.com>; Matushevsky, Alexander <matua@amazon.com>
Subject: RE: [EXTERNAL] [dpdk-dev] [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.



On 3/16/2020 9:34 AM, Chauskin, Igor wrote:
> 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.

Hi Igor,

Is there any progress on reviewing the set?

Thanks,
ferruh

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