[dpdk-dev] [PATCH] net/bnxt: fix memory barriers

[dpdk-dev] [PATCH] net/bnxt: fix memory barriers

[Gavin Hu]

As there is an inclusive rte_io_wmb within the following rte_write32()
API who rings the doorbell, this makes the above rte_wmb unecessary and
remove it.

The doorbell ringing operation requires a rte_io_wmb immediately to make
it complete and visible to the device.

To read the doorbell response, which is held in the host CIO memory,
rte_cio_rmb is sufficient.

Fixes: 804e746c7b73 ("net/bnxt: add hardware resource manager init code")
Fixes: ca241d9a0952 ("net/bnxt: use I/O device memory read/write API")
Cc: stable@dpdk.org

Signed-off-by: Gavin Hu <gavin.hu@arm.com>
Reviewed-by: Steve Capper <steve.capper@arm.com>
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
Reviewed-by: Phil Yang <phil.yang@arm.com>
---
 drivers/net/bnxt/bnxt_hwrm.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index 9883fb5..ee07db9 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -115,9 +115,6 @@ static int bnxt_hwrm_send_message(struct bnxt *bp, void *msg,
 		data = (uint32_t *)&short_input;
 		msg_len = sizeof(short_input);
 
-		/* Sync memory write before updating doorbell */
-		rte_wmb();
-
 		max_req_len = BNXT_HWRM_SHORT_REQ_LEN;
 	}
 
@@ -137,11 +134,12 @@ static int bnxt_hwrm_send_message(struct bnxt *bp, void *msg,
 	/* Ring channel doorbell */
 	bar = (uint8_t *)bp->bar0 + mb_trigger_offset;
 	rte_write32(1, bar);
+	rte_io_wmb();
 
 	/* Poll for the valid bit */
 	for (i = 0; i < HWRM_CMD_TIMEOUT; i++) {
 		/* Sanity check on the resp->resp_len */
-		rte_rmb();
+		rte_cio_rmb();
 		if (resp->resp_len && resp->resp_len <= bp->max_resp_len) {
 			/* Last byte of resp contains the valid key */
 			valid = (uint8_t *)resp + resp->resp_len - 1;
-- 
2.7.4

[dpdk-dev] [PATCH v2 0/5] fix and optimize barriers usage with some PMDs

[Gavin Hu]

DPDK has well-defined barriers, such as CIO barriers and IO barriers.

X86, as a strong ordering model, implements the barriers as compiler
barriers, but on aarch64, as a weak memory ordering model, has fine
grained barriers. Using correct while as relaxed as possible barriers
makes a perf difference.

Upon investigation on a batch of PMDs and it was found that the barriers
are not always used correctly or relaxedly enough. 

This series of patches is to optimize the barrier usage with some selected
PMDs and aim at best performance on all arches/platforms. 

More PMDs may come next to this series but it takes time.

Gavin Hu (5):
  net/i40e: use relaxed and remove duplicate barrier
  net/ice: use relaxed and remove duplicate barrier
  net/bnxt: remove duplicate barrier
  net/bnxt: replace with cio barrier for doorbell resp
  net/bnxt: enforce io barrier for doorbell command

 drivers/net/bnxt/bnxt_hwrm.c | 11 +++++++----
 drivers/net/i40e/i40e_rxtx.c | 12 +++---------
 drivers/net/ice/ice_rxtx.c   |  6 ------
 3 files changed, 10 insertions(+), 19 deletions(-)

-- 
2.7.4

[dpdk-dev] [PATCH v2 1/5] net/i40e: use relaxed and remove duplicate barrier

[Gavin Hu]

To guarantee the orderings of successive stores to CIO and MMIO memory,
a lighter weight rte_io_wmb [1] can be used instead of rte_wmb, and since
the I40E_PCI_REG_WRITE API already has an inclusive rte_io_wmb, this
explicit call can be even saved.

[1] http://git.dpdk.org/dpdk/tree/lib/librte_eal/common/include/generic/
rte_atomic.h#n98

Signed-off-by: Gavin Hu <gavin.hu@arm.com>
---
 drivers/net/i40e/i40e_rxtx.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
index 692c3ba..bfe161f 100644
--- a/drivers/net/i40e/i40e_rxtx.c
+++ b/drivers/net/i40e/i40e_rxtx.c
@@ -564,8 +564,7 @@ i40e_rx_alloc_bufs(struct i40e_rx_queue *rxq)
 	}
 
 	/* Update rx tail regsiter */
-	rte_wmb();
-	I40E_PCI_REG_WRITE_RELAXED(rxq->qrx_tail, rxq->rx_free_trigger);
+	I40E_PCI_REG_WRITE(rxq->qrx_tail, rxq->rx_free_trigger);
 
 	rxq->rx_free_trigger =
 		(uint16_t)(rxq->rx_free_trigger + rxq->rx_free_thresh);
@@ -1208,13 +1207,11 @@ i40e_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
 	}
 
 end_of_tx:
-	rte_wmb();
-
 	PMD_TX_LOG(DEBUG, "port_id=%u queue_id=%u tx_tail=%u nb_tx=%u",
 		   (unsigned) txq->port_id, (unsigned) txq->queue_id,
 		   (unsigned) tx_id, (unsigned) nb_tx);
 
-	I40E_PCI_REG_WRITE_RELAXED(txq->qtx_tail, tx_id);
+	I40E_PCI_REG_WRITE(txq->qtx_tail, tx_id);
 	txq->tx_tail = tx_id;
 
 	return nb_tx;
@@ -1365,8 +1362,7 @@ tx_xmit_pkts(struct i40e_tx_queue *txq,
 		txq->tx_tail = 0;
 
 	/* Update the tx tail register */
-	rte_wmb();
-	I40E_PCI_REG_WRITE_RELAXED(txq->qtx_tail, txq->tx_tail);
+	I40E_PCI_REG_WRITE(txq->qtx_tail, txq->tx_tail);
 
 	return nb_pkts;
 }
@@ -1544,8 +1540,6 @@ i40e_dev_rx_queue_start(struct rte_eth_dev *dev, uint16_t rx_queue_id)
 		return err;
 	}
 
-	rte_wmb();
-
 	/* Init the RX tail regieter. */
 	I40E_PCI_REG_WRITE(rxq->qrx_tail, rxq->nb_rx_desc - 1);
 
-- 
2.7.4

[dpdk-dev] [PATCH v2 2/5] net/ice: use relaxed and remove duplicate barrier

[Gavin Hu]

To guarantee the orderings of successive stores to CIO and MMIO memory,
a lighter weight rte_io_wmb [1] can be used instead of rte_wmb, and since
the ICE_PCI_REG_WRITE API already has an inclusive rte_io_wmb, this
explicit call can even be saved.

[1] http://git.dpdk.org/dpdk/tree/lib/librte_eal/common/include/generic/
rte_atomic.h#n98

Signed-off-by: Gavin Hu <gavin.hu@arm.com>
---
 drivers/net/ice/ice_rxtx.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/net/ice/ice_rxtx.c b/drivers/net/ice/ice_rxtx.c
index 81af814..2366119 100644
--- a/drivers/net/ice/ice_rxtx.c
+++ b/drivers/net/ice/ice_rxtx.c
@@ -363,8 +363,6 @@ ice_rx_queue_start(struct rte_eth_dev *dev, uint16_t rx_queue_id)
 		return -ENOMEM;
 	}
 
-	rte_wmb();
-
 	/* Init the RX tail register. */
 	ICE_PCI_REG_WRITE(rxq->qrx_tail, rxq->nb_rx_desc - 1);
 
@@ -1212,7 +1210,6 @@ ice_rx_alloc_bufs(struct ice_rx_queue *rxq)
 	}
 
 	/* Update rx tail regsiter */
-	rte_wmb();
 	ICE_PCI_REG_WRITE(rxq->qrx_tail, rxq->rx_free_trigger);
 
 	rxq->rx_free_trigger =
@@ -2132,8 +2129,6 @@ ice_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
 					 ICE_TXD_QW1_CMD_S);
 	}
 end_of_tx:
-	rte_wmb();
-
 	/* update Tail register */
 	ICE_PCI_REG_WRITE(txq->qtx_tail, tx_id);
 	txq->tx_tail = tx_id;
@@ -2289,7 +2284,6 @@ tx_xmit_pkts(struct ice_tx_queue *txq,
 		txq->tx_tail = 0;
 
 	/* Update the tx tail register */
-	rte_wmb();
 	ICE_PCI_REG_WRITE(txq->qtx_tail, txq->tx_tail);
 
 	return nb_pkts;
-- 
2.7.4

[dpdk-dev] [PATCH v2 3/5] net/bnxt: remove duplicate barrier

[Gavin Hu]

As there is an inclusive rte_io_wmb within the following rte_write32()
API who rings the doorbell, this makes the above rte_wmb unnecessary and
remove it.

Fixes: 1cd45aeb3270 ("net/bnxt: support Stratus VF device")
Cc: stable@dpdk.org

Signed-off-by: Gavin Hu <gavin.hu@arm.com>
Reviewed-by: Steve Capper <steve.capper@arm.com>
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
Reviewed-by: Phil Yang <phil.yang@arm.com>
---
 drivers/net/bnxt/bnxt_hwrm.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index 9883fb5..1d9fb17 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -115,9 +115,6 @@ static int bnxt_hwrm_send_message(struct bnxt *bp, void *msg,
 		data = (uint32_t *)&short_input;
 		msg_len = sizeof(short_input);
 
-		/* Sync memory write before updating doorbell */
-		rte_wmb();
-
 		max_req_len = BNXT_HWRM_SHORT_REQ_LEN;
 	}
 
-- 
2.7.4

[dpdk-dev] [PATCH v2 4/5] net/bnxt: replace with cio barrier for doorbell resp

[Gavin Hu]

To read the doorbell response, which is held in the host CIO memory,
rte_cio_rmb is sufficient.

Fixes: 804e746c7b73 ("net/bnxt: add hardware resource manager init code")
Cc: stable@dpdk.org

Signed-off-by: Gavin Hu <gavin.hu@arm.com>
---
 drivers/net/bnxt/bnxt_hwrm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index 1d9fb17..2d5dc00 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -138,7 +138,7 @@ static int bnxt_hwrm_send_message(struct bnxt *bp, void *msg,
 	/* Poll for the valid bit */
 	for (i = 0; i < HWRM_CMD_TIMEOUT; i++) {
 		/* Sanity check on the resp->resp_len */
-		rte_rmb();
+		rte_cio_rmb();
 		if (resp->resp_len && resp->resp_len <= bp->max_resp_len) {
 			/* Last byte of resp contains the valid key */
 			valid = (uint8_t *)resp + resp->resp_len - 1;
-- 
2.7.4

[dpdk-dev] [PATCH v2 5/5] net/bnxt: enforce io barrier for doorbell command

[Gavin Hu]

The doorbell ringing operation requires a rte_io_mb immediately to make
the command complete and visible to the device before reading the
response, otherwise it may read stale or invalid responses.

Fixes: ca241d9a0952 ("net/bnxt: use I/O device memory read/write API")
Cc: stable@dpdk.org

Signed-off-by: Gavin Hu <gavin.hu@arm.com>
---
 drivers/net/bnxt/bnxt_hwrm.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
index 2d5dc00..7d192e3 100644
--- a/drivers/net/bnxt/bnxt_hwrm.c
+++ b/drivers/net/bnxt/bnxt_hwrm.c
@@ -134,6 +134,12 @@ static int bnxt_hwrm_send_message(struct bnxt *bp, void *msg,
 	/* Ring channel doorbell */
 	bar = (uint8_t *)bp->bar0 + mb_trigger_offset;
 	rte_write32(1, bar);
+	/*
+	 * Make sure the channel doorbell ring command complete before
+	 * reading the response to avoid getting stale or invalid
+	 * responses.
+	 */
+	rte_io_mb();
 
 	/* Poll for the valid bit */
 	for (i = 0; i < HWRM_CMD_TIMEOUT; i++) {
-- 
2.7.4

Re: [dpdk-dev] [PATCH v2 2/5] net/ice: use relaxed and remove duplicate barrier

[Zhang, Qi Z]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Gavin Hu
> Sent: Monday, September 16, 2019 7:27 PM
> To: dev@dpdk.org
> Cc: nd@arm.com; thomas@monjalon.net; Richardson, Bruce
> <bruce.richardson@intel.com>; Liu, Yong <yong.liu@intel.com>; Wang, Yinan
> <yinan.wang@intel.com>; ajit.khaparde@broadcom.com;
> somnath.kotur@broadcom.com; Honnappa.Nagarahalli@arm.com;
> ruifeng.wang@arm.com; steve.capper@arm.com
> Subject: [dpdk-dev] [PATCH v2 2/5] net/ice: use relaxed and remove duplicate
> barrier
> 
> To guarantee the orderings of successive stores to CIO and MMIO memory, a
> lighter weight rte_io_wmb [1] can be used instead of rte_wmb, and since the
> ICE_PCI_REG_WRITE API already has an inclusive rte_io_wmb, this explicit call
> can even be saved.
> 
> [1] http://git.dpdk.org/dpdk/tree/lib/librte_eal/common/include/generic/
> rte_atomic.h#n98
> 
> Signed-off-by: Gavin Hu <gavin.hu@arm.com>

Acked-by: Qi Zhang <qi.z.zhang@intel.com>

Re: [dpdk-dev] [PATCH v2 1/5] net/i40e: use relaxed and remove duplicate barrier

[Zhang, Qi Z]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Gavin Hu
> Sent: Monday, September 16, 2019 7:27 PM
> To: dev@dpdk.org
> Cc: nd@arm.com; thomas@monjalon.net; Richardson, Bruce
> <bruce.richardson@intel.com>; Liu, Yong <yong.liu@intel.com>; Wang, Yinan
> <yinan.wang@intel.com>; ajit.khaparde@broadcom.com;
> somnath.kotur@broadcom.com; Honnappa.Nagarahalli@arm.com;
> ruifeng.wang@arm.com; steve.capper@arm.com
> Subject: [dpdk-dev] [PATCH v2 1/5] net/i40e: use relaxed and remove
> duplicate barrier
> 
> To guarantee the orderings of successive stores to CIO and MMIO memory, a
> lighter weight rte_io_wmb [1] can be used instead of rte_wmb, and since the
> I40E_PCI_REG_WRITE API already has an inclusive rte_io_wmb, this explicit call
> can be even saved.
> 
> [1] http://git.dpdk.org/dpdk/tree/lib/librte_eal/common/include/generic/
> rte_atomic.h#n98
> 
> Signed-off-by: Gavin Hu <gavin.hu@arm.com>

Can you also capture the one at the tail of i40e_xmit_pkts?

Otherwise
Acked-by: Qi Zhang <qi.z.zhang@intel.com>

Re: [dpdk-dev] [PATCH v2 5/5] net/bnxt: enforce io barrier for doorbell command

[Ajit Khaparde]

On Mon, Sep 16, 2019 at 4:27 AM Gavin Hu <gavin.hu@arm.com> wrote:

> The doorbell ringing operation requires a rte_io_mb immediately to make
> the command complete and visible to the device before reading the
> response, otherwise it may read stale or invalid responses.
>
> Fixes: ca241d9a0952 ("net/bnxt: use I/O device memory read/write API")
> Cc: stable@dpdk.org
>
> Signed-off-by: Gavin Hu <gavin.hu@arm.com>
>
Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>


> ---
>  drivers/net/bnxt/bnxt_hwrm.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
> index 2d5dc00..7d192e3 100644
> --- a/drivers/net/bnxt/bnxt_hwrm.c
> +++ b/drivers/net/bnxt/bnxt_hwrm.c
> @@ -134,6 +134,12 @@ static int bnxt_hwrm_send_message(struct bnxt *bp,
> void *msg,
>         /* Ring channel doorbell */
>         bar = (uint8_t *)bp->bar0 + mb_trigger_offset;
>         rte_write32(1, bar);
> +       /*
> +        * Make sure the channel doorbell ring command complete before
> +        * reading the response to avoid getting stale or invalid
> +        * responses.
> +        */
> +       rte_io_mb();
>
>         /* Poll for the valid bit */
>         for (i = 0; i < HWRM_CMD_TIMEOUT; i++) {
> --
> 2.7.4
>
>

Re: [dpdk-dev] [PATCH v2 3/5] net/bnxt: remove duplicate barrier

[Ajit Khaparde]

On Mon, Sep 16, 2019 at 4:27 AM Gavin Hu <gavin.hu@arm.com> wrote:

> As there is an inclusive rte_io_wmb within the following rte_write32()
> API who rings the doorbell, this makes the above rte_wmb unnecessary and
> remove it.
>
> Fixes: 1cd45aeb3270 ("net/bnxt: support Stratus VF device")
> Cc: stable@dpdk.org
>
> Signed-off-by: Gavin Hu <gavin.hu@arm.com>
> Reviewed-by: Steve Capper <steve.capper@arm.com>
> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> Reviewed-by: Phil Yang <phil.yang@arm.com>
>

Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>


> ---
>  drivers/net/bnxt/bnxt_hwrm.c | 3 ---
>  1 file changed, 3 deletions(-)
>
> diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
> index 9883fb5..1d9fb17 100644
> --- a/drivers/net/bnxt/bnxt_hwrm.c
> +++ b/drivers/net/bnxt/bnxt_hwrm.c
> @@ -115,9 +115,6 @@ static int bnxt_hwrm_send_message(struct bnxt *bp,
> void *msg,
>                 data = (uint32_t *)&short_input;
>                 msg_len = sizeof(short_input);
>
> -               /* Sync memory write before updating doorbell */
> -               rte_wmb();
> -
>                 max_req_len = BNXT_HWRM_SHORT_REQ_LEN;
>         }
>
> --
> 2.7.4
>
>

Re: [dpdk-dev] [PATCH v2 4/5] net/bnxt: replace with cio barrier for doorbell resp

[Ajit Khaparde]

On Mon, Sep 16, 2019 at 4:27 AM Gavin Hu <gavin.hu@arm.com> wrote:

> To read the doorbell response, which is held in the host CIO memory,
> rte_cio_rmb is sufficient.
>
> Fixes: 804e746c7b73 ("net/bnxt: add hardware resource manager init code")
> Cc: stable@dpdk.org
>
> Signed-off-by: Gavin Hu <gavin.hu@arm.com>
>
Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>


> ---
>  drivers/net/bnxt/bnxt_hwrm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
> index 1d9fb17..2d5dc00 100644
> --- a/drivers/net/bnxt/bnxt_hwrm.c
> +++ b/drivers/net/bnxt/bnxt_hwrm.c
> @@ -138,7 +138,7 @@ static int bnxt_hwrm_send_message(struct bnxt *bp,
> void *msg,
>         /* Poll for the valid bit */
>         for (i = 0; i < HWRM_CMD_TIMEOUT; i++) {
>                 /* Sanity check on the resp->resp_len */
> -               rte_rmb();
> +               rte_cio_rmb();
>                 if (resp->resp_len && resp->resp_len <= bp->max_resp_len) {
>                         /* Last byte of resp contains the valid key */
>                         valid = (uint8_t *)resp + resp->resp_len - 1;
> --
> 2.7.4
>
>

Re: [dpdk-dev] [PATCH v2 1/5] net/i40e: use relaxed and remove duplicate barrier

[Gavin Hu (Arm Technology China)]

Hi Qi,

> -----Original Message-----
> From: Zhang, Qi Z <qi.z.zhang@intel.com>
> Sent: Tuesday, September 17, 2019 9:53 AM
> To: Gavin Hu (Arm Technology China) <Gavin.Hu@arm.com>; dev@dpdk.org
> Cc: nd <nd@arm.com>; thomas@monjalon.net; Richardson, Bruce
> <bruce.richardson@intel.com>; Liu, Yong <yong.liu@intel.com>; Wang, Yinan
> <yinan.wang@intel.com>; ajit.khaparde@broadcom.com;
> somnath.kotur@broadcom.com; Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com>; Ruifeng Wang (Arm Technology China)
> <Ruifeng.Wang@arm.com>; Steve Capper <Steve.Capper@arm.com>
> Subject: RE: [dpdk-dev] [PATCH v2 1/5] net/i40e: use relaxed and remove
> duplicate barrier
> 
> 
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Gavin Hu
> > Sent: Monday, September 16, 2019 7:27 PM
> > To: dev@dpdk.org
> > Cc: nd@arm.com; thomas@monjalon.net; Richardson, Bruce
> > <bruce.richardson@intel.com>; Liu, Yong <yong.liu@intel.com>; Wang,
> Yinan
> > <yinan.wang@intel.com>; ajit.khaparde@broadcom.com;
> > somnath.kotur@broadcom.com; Honnappa.Nagarahalli@arm.com;
> > ruifeng.wang@arm.com; steve.capper@arm.com
> > Subject: [dpdk-dev] [PATCH v2 1/5] net/i40e: use relaxed and remove
> > duplicate barrier
> >
> > To guarantee the orderings of successive stores to CIO and MMIO memory,
> a
> > lighter weight rte_io_wmb [1] can be used instead of rte_wmb, and since
> the
> > I40E_PCI_REG_WRITE API already has an inclusive rte_io_wmb, this explicit
> call
> > can be even saved.
> >
> > [1] http://git.dpdk.org/dpdk/tree/lib/librte_eal/common/include/generic/
> > rte_atomic.h#n98
> >
> > Signed-off-by: Gavin Hu <gavin.hu@arm.com>
> 
> Can you also capture the one at the tail of i40e_xmit_pkts?
Thanks for your review, I will fix this in next version.
> 
> Otherwise
> Acked-by: Qi Zhang <qi.z.zhang@intel.com>
> 

Re: [dpdk-dev] [PATCH v2 1/5] net/i40e: use relaxed and remove duplicate barrier

[Gavin Hu (Arm Technology China)]

Hi Qi, 

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Gavin Hu (Arm
> Technology China)
> Sent: Tuesday, September 17, 2019 10:08 AM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; dev@dpdk.org
> Cc: nd <nd@arm.com>; thomas@monjalon.net; Richardson, Bruce
> <bruce.richardson@intel.com>; Liu, Yong <yong.liu@intel.com>; Wang,
> Yinan <yinan.wang@intel.com>; ajit.khaparde@broadcom.com;
> somnath.kotur@broadcom.com; Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com>; Ruifeng Wang (Arm Technology China)
> <Ruifeng.Wang@arm.com>; Steve Capper <Steve.Capper@arm.com>
> Subject: Re: [dpdk-dev] [PATCH v2 1/5] net/i40e: use relaxed and remove
> duplicate barrier
> 
> Hi Qi,
> 
> > -----Original Message-----
> > From: Zhang, Qi Z <qi.z.zhang@intel.com>
> > Sent: Tuesday, September 17, 2019 9:53 AM
> > To: Gavin Hu (Arm Technology China) <Gavin.Hu@arm.com>;
> dev@dpdk.org
> > Cc: nd <nd@arm.com>; thomas@monjalon.net; Richardson, Bruce
> > <bruce.richardson@intel.com>; Liu, Yong <yong.liu@intel.com>; Wang,
> Yinan
> > <yinan.wang@intel.com>; ajit.khaparde@broadcom.com;
> > somnath.kotur@broadcom.com; Honnappa Nagarahalli
> > <Honnappa.Nagarahalli@arm.com>; Ruifeng Wang (Arm Technology
> China)
> > <Ruifeng.Wang@arm.com>; Steve Capper <Steve.Capper@arm.com>
> > Subject: RE: [dpdk-dev] [PATCH v2 1/5] net/i40e: use relaxed and remove
> > duplicate barrier
> >
> >
> >
> > > -----Original Message-----
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Gavin Hu
> > > Sent: Monday, September 16, 2019 7:27 PM
> > > To: dev@dpdk.org
> > > Cc: nd@arm.com; thomas@monjalon.net; Richardson, Bruce
> > > <bruce.richardson@intel.com>; Liu, Yong <yong.liu@intel.com>; Wang,
> > Yinan
> > > <yinan.wang@intel.com>; ajit.khaparde@broadcom.com;
> > > somnath.kotur@broadcom.com; Honnappa.Nagarahalli@arm.com;
> > > ruifeng.wang@arm.com; steve.capper@arm.com
> > > Subject: [dpdk-dev] [PATCH v2 1/5] net/i40e: use relaxed and remove
> > > duplicate barrier
> > >
> > > To guarantee the orderings of successive stores to CIO and MMIO
> memory,
> > a
> > > lighter weight rte_io_wmb [1] can be used instead of rte_wmb, and
> since
> > the
> > > I40E_PCI_REG_WRITE API already has an inclusive rte_io_wmb, this
> explicit
> > call
> > > can be even saved.
> > >
> > > [1]
> http://git.dpdk.org/dpdk/tree/lib/librte_eal/common/include/generic/
> > > rte_atomic.h#n98
> > >
> > > Signed-off-by: Gavin Hu <gavin.hu@arm.com>
> >
> > Can you also capture the one at the tail of i40e_xmit_pkts?
> Thanks for your review, I will fix this in next version.
I checked again, in this version, the rte_wmb at the tail of i40e_xmit_pkts was already removed
and the following I40E_PCI_REG_WRITE_RELAXED was replaced by  I40E_PCI_REG_WRITE, which has an inclusive rte_io_wmb. 
> >
> > Otherwise
> > Acked-by: Qi Zhang <qi.z.zhang@intel.com>
> >

Re: [dpdk-dev] [PATCH v2 1/5] net/i40e: use relaxed and remove duplicate barrier

[Zhang, Qi Z]

> -----Original Message-----
> From: Gavin Hu (Arm Technology China) [mailto:Gavin.Hu@arm.com]
> Sent: Tuesday, September 17, 2019 11:24 AM
> To: Gavin Hu (Arm Technology China) <Gavin.Hu@arm.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>; dev@dpdk.org
> Cc: nd <nd@arm.com>; thomas@monjalon.net; Richardson, Bruce
> <bruce.richardson@intel.com>; Liu, Yong <yong.liu@intel.com>; Wang, Yinan
> <yinan.wang@intel.com>; ajit.khaparde@broadcom.com;
> somnath.kotur@broadcom.com; Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com>; Ruifeng Wang (Arm Technology China)
> <Ruifeng.Wang@arm.com>; Steve Capper <Steve.Capper@arm.com>
> Subject: RE: [dpdk-dev] [PATCH v2 1/5] net/i40e: use relaxed and remove
> duplicate barrier
> 
> Hi Qi,
> 
> > -----Original Message-----
> > From: dev <dev-bounces@dpdk.org> On Behalf Of Gavin Hu (Arm Technology
> > China)
> > Sent: Tuesday, September 17, 2019 10:08 AM
> > To: Zhang, Qi Z <qi.z.zhang@intel.com>; dev@dpdk.org
> > Cc: nd <nd@arm.com>; thomas@monjalon.net; Richardson, Bruce
> > <bruce.richardson@intel.com>; Liu, Yong <yong.liu@intel.com>; Wang,
> > Yinan <yinan.wang@intel.com>; ajit.khaparde@broadcom.com;
> > somnath.kotur@broadcom.com; Honnappa Nagarahalli
> > <Honnappa.Nagarahalli@arm.com>; Ruifeng Wang (Arm Technology China)
> > <Ruifeng.Wang@arm.com>; Steve Capper <Steve.Capper@arm.com>
> > Subject: Re: [dpdk-dev] [PATCH v2 1/5] net/i40e: use relaxed and
> > remove duplicate barrier
> >
> > Hi Qi,
> >
> > > -----Original Message-----
> > > From: Zhang, Qi Z <qi.z.zhang@intel.com>
> > > Sent: Tuesday, September 17, 2019 9:53 AM
> > > To: Gavin Hu (Arm Technology China) <Gavin.Hu@arm.com>;
> > dev@dpdk.org
> > > Cc: nd <nd@arm.com>; thomas@monjalon.net; Richardson, Bruce
> > > <bruce.richardson@intel.com>; Liu, Yong <yong.liu@intel.com>; Wang,
> > Yinan
> > > <yinan.wang@intel.com>; ajit.khaparde@broadcom.com;
> > > somnath.kotur@broadcom.com; Honnappa Nagarahalli
> > > <Honnappa.Nagarahalli@arm.com>; Ruifeng Wang (Arm Technology
> > China)
> > > <Ruifeng.Wang@arm.com>; Steve Capper <Steve.Capper@arm.com>
> > > Subject: RE: [dpdk-dev] [PATCH v2 1/5] net/i40e: use relaxed and
> > > remove duplicate barrier
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Gavin Hu
> > > > Sent: Monday, September 16, 2019 7:27 PM
> > > > To: dev@dpdk.org
> > > > Cc: nd@arm.com; thomas@monjalon.net; Richardson, Bruce
> > > > <bruce.richardson@intel.com>; Liu, Yong <yong.liu@intel.com>;
> > > > Wang,
> > > Yinan
> > > > <yinan.wang@intel.com>; ajit.khaparde@broadcom.com;
> > > > somnath.kotur@broadcom.com; Honnappa.Nagarahalli@arm.com;
> > > > ruifeng.wang@arm.com; steve.capper@arm.com
> > > > Subject: [dpdk-dev] [PATCH v2 1/5] net/i40e: use relaxed and
> > > > remove duplicate barrier
> > > >
> > > > To guarantee the orderings of successive stores to CIO and MMIO
> > memory,
> > > a
> > > > lighter weight rte_io_wmb [1] can be used instead of rte_wmb, and
> > since
> > > the
> > > > I40E_PCI_REG_WRITE API already has an inclusive rte_io_wmb, this
> > explicit
> > > call
> > > > can be even saved.
> > > >
> > > > [1]
> > http://git.dpdk.org/dpdk/tree/lib/librte_eal/common/include/generic/
> > > > rte_atomic.h#n98
> > > >
> > > > Signed-off-by: Gavin Hu <gavin.hu@arm.com>
> > >
> > > Can you also capture the one at the tail of i40e_xmit_pkts?
> > Thanks for your review, I will fix this in next version.
> I checked again, in this version, the rte_wmb at the tail of i40e_xmit_pkts was
> already removed and the following I40E_PCI_REG_WRITE_RELAXED was
> replaced by  I40E_PCI_REG_WRITE, which has an inclusive rte_io_wmb.

Sorry, I must mixed the with ice patch, yes you are right, its already there.

> > >
> > > Otherwise
> > > Acked-by: Qi Zhang <qi.z.zhang@intel.com>
> > >

Re: [dpdk-dev] [PATCH v2 0/5] fix and optimize barriers usage with some PMDs

[Ferruh Yigit]

On 9/16/2019 12:27 PM, Gavin Hu wrote:
> DPDK has well-defined barriers, such as CIO barriers and IO barriers.
> 
> X86, as a strong ordering model, implements the barriers as compiler
> barriers, but on aarch64, as a weak memory ordering model, has fine
> grained barriers. Using correct while as relaxed as possible barriers
> makes a perf difference.
> 
> Upon investigation on a batch of PMDs and it was found that the barriers
> are not always used correctly or relaxedly enough. 
> 
> This series of patches is to optimize the barrier usage with some selected
> PMDs and aim at best performance on all arches/platforms. 
> 
> More PMDs may come next to this series but it takes time.
> 
> Gavin Hu (5):
>   net/i40e: use relaxed and remove duplicate barrier
>   net/ice: use relaxed and remove duplicate barrier
>   net/bnxt: remove duplicate barrier
>   net/bnxt: replace with cio barrier for doorbell resp
>   net/bnxt: enforce io barrier for doorbell command

Series applied to dpdk-next-net/master, thanks.