patch 'net/mlx5: fix Rx/Tx stats concurrency' has been queued to stable release 20.11.6

Xueming Li xuemingl at nvidia.com
Tue Jun 21 10:01:17 CEST 2022


Hi,

FYI, your patch has been queued to stable release 20.11.6

Note it hasn't been pushed to http://dpdk.org/browse/dpdk-stable yet.
It will be pushed if I get no objections before 06/23/22. So please
shout if anyone has objections.

Also note that after the patch there's a diff of the upstream commit vs the
patch applied to the branch. This will indicate if there was any rebasing
needed to apply to the stable branch. If there were code changes for rebasing
(ie: not only metadata diffs), please double check that the rebase was
correctly done.

Queued patches are on a temporary branch at:
https://github.com/steevenlee/dpdk

This queued commit can be viewed at:
https://github.com/steevenlee/dpdk/commit/c4d51b3f05b56efe4e435187e67d698a07e30b94

Thanks.

Xueming Li <xuemingl at nvidia.com>

---
>From c4d51b3f05b56efe4e435187e67d698a07e30b94 Mon Sep 17 00:00:00 2001
From: Raja Zidane <rzidane at nvidia.com>
Date: Wed, 20 Apr 2022 18:32:17 +0300
Subject: [PATCH] net/mlx5: fix Rx/Tx stats concurrency
Cc: Xueming Li <xuemingl at nvidia.com>

[ upstream commit 773a7de21a984bb7bdb6396a659ccc585a44a806 ]

Queue statistics are being continuously updated in Rx/Tx burst
routines while handling traffic. In addition to that, statistics
can be reset (written with zeroes) on statistics reset in other
threads, causing a race condition, which in turn could result in
wrong stats.

The patch provides an approach with reference values, allowing
the actual counters to be writable within Rx/Tx burst threads
only, and updating reference values on stats reset.

Fixes: 87011737b715 ("mlx5: add software counters")

Signed-off-by: Raja Zidane <rzidane at nvidia.com>
Acked-by: Viacheslav Ovsiienko <viacheslavo at nvidia.com>
---
 drivers/net/mlx5/mlx5_rxtx.c  |  1 +
 drivers/net/mlx5/mlx5_rxtx.h  |  2 ++
 drivers/net/mlx5/mlx5_stats.c | 41 ++++++++++++++++++++---------------
 3 files changed, 27 insertions(+), 17 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_rxtx.c b/drivers/net/mlx5/mlx5_rxtx.c
index 734c462a09..edabe229d4 100644
--- a/drivers/net/mlx5/mlx5_rxtx.c
+++ b/drivers/net/mlx5/mlx5_rxtx.c
@@ -78,6 +78,7 @@ static uint16_t mlx5_tx_burst_##func(void *txq, \
 }
 
 #define MLX5_TXOFF_INFO(func, olx) {mlx5_tx_burst_##func, olx},
+	struct mlx5_txq_stats stats_reset; /* stats on last reset. */
 
 static __rte_always_inline uint32_t
 rxq_cq_to_pkt_type(struct mlx5_rxq_data *rxq, volatile struct mlx5_cqe *cqe,
diff --git a/drivers/net/mlx5/mlx5_rxtx.h b/drivers/net/mlx5/mlx5_rxtx.h
index 237a7faa5c..8760ab78f7 100644
--- a/drivers/net/mlx5/mlx5_rxtx.h
+++ b/drivers/net/mlx5/mlx5_rxtx.h
@@ -153,6 +153,7 @@ struct mlx5_rxq_data {
 	struct mlx5_dev_ctx_shared *sh; /* Shared context. */
 	uint16_t idx; /* Queue index. */
 	struct mlx5_rxq_stats stats;
+	struct mlx5_rxq_stats stats_reset; /* stats on last reset. */
 	rte_xmm_t mbuf_initializer; /* Default rearm/flags for vectorized Rx. */
 	struct rte_mbuf fake_mbuf; /* elts padding for vectorized Rx. */
 	void *cq_uar; /* Verbs CQ user access region. */
@@ -271,6 +272,7 @@ struct mlx5_txq_data {
 	int32_t ts_offset; /* Timestamp field dynamic offset. */
 	struct mlx5_dev_ctx_shared *sh; /* Shared context. */
 	struct mlx5_txq_stats stats; /* TX queue counters. */
+	struct mlx5_txq_stats stats_reset; /* stats on last reset. */
 #ifndef RTE_ARCH_64
 	rte_spinlock_t *uar_lock;
 	/* UAR access lock required for 32bit implementations */
diff --git a/drivers/net/mlx5/mlx5_stats.c b/drivers/net/mlx5/mlx5_stats.c
index 82d4d4a745..450037aea0 100644
--- a/drivers/net/mlx5/mlx5_stats.c
+++ b/drivers/net/mlx5/mlx5_stats.c
@@ -113,18 +113,23 @@ mlx5_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 		idx = rxq->idx;
 		if (idx < RTE_ETHDEV_QUEUE_STAT_CNTRS) {
 #ifdef MLX5_PMD_SOFT_COUNTERS
-			tmp.q_ipackets[idx] += rxq->stats.ipackets;
-			tmp.q_ibytes[idx] += rxq->stats.ibytes;
+			tmp.q_ipackets[idx] += rxq->stats.ipackets -
+				rxq->stats_reset.ipackets;
+			tmp.q_ibytes[idx] += rxq->stats.ibytes -
+				rxq->stats_reset.ibytes;
 #endif
 			tmp.q_errors[idx] += (rxq->stats.idropped +
-					      rxq->stats.rx_nombuf);
+					      rxq->stats.rx_nombuf) -
+					      (rxq->stats_reset.idropped +
+					      rxq->stats_reset.rx_nombuf);
 		}
 #ifdef MLX5_PMD_SOFT_COUNTERS
-		tmp.ipackets += rxq->stats.ipackets;
-		tmp.ibytes += rxq->stats.ibytes;
+		tmp.ipackets += rxq->stats.ipackets - rxq->stats_reset.ipackets;
+		tmp.ibytes += rxq->stats.ibytes - rxq->stats_reset.ibytes;
 #endif
-		tmp.ierrors += rxq->stats.idropped;
-		tmp.rx_nombuf += rxq->stats.rx_nombuf;
+		tmp.ierrors += rxq->stats.idropped - rxq->stats_reset.idropped;
+		tmp.rx_nombuf += rxq->stats.rx_nombuf -
+					rxq->stats_reset.rx_nombuf;
 	}
 	for (i = 0; (i != priv->txqs_n); ++i) {
 		struct mlx5_txq_data *txq = (*priv->txqs)[i];
@@ -134,15 +139,17 @@ mlx5_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 		idx = txq->idx;
 		if (idx < RTE_ETHDEV_QUEUE_STAT_CNTRS) {
 #ifdef MLX5_PMD_SOFT_COUNTERS
-			tmp.q_opackets[idx] += txq->stats.opackets;
-			tmp.q_obytes[idx] += txq->stats.obytes;
+			tmp.q_opackets[idx] += txq->stats.opackets -
+						txq->stats_reset.opackets;
+			tmp.q_obytes[idx] += txq->stats.obytes -
+						txq->stats_reset.obytes;
 #endif
 		}
 #ifdef MLX5_PMD_SOFT_COUNTERS
-		tmp.opackets += txq->stats.opackets;
-		tmp.obytes += txq->stats.obytes;
+		tmp.opackets += txq->stats.opackets - txq->stats_reset.opackets;
+		tmp.obytes += txq->stats.obytes - txq->stats_reset.obytes;
 #endif
-		tmp.oerrors += txq->stats.oerrors;
+		tmp.oerrors += txq->stats.oerrors - txq->stats_reset.oerrors;
 	}
 	ret = mlx5_os_read_dev_stat(priv, "out_of_buffer", &tmp.imissed);
 	if (ret == 0) {
@@ -182,14 +189,14 @@ mlx5_stats_reset(struct rte_eth_dev *dev)
 	for (i = 0; (i != priv->rxqs_n); ++i) {
 		if ((*priv->rxqs)[i] == NULL)
 			continue;
-		memset(&(*priv->rxqs)[i]->stats, 0,
-		       sizeof(struct mlx5_rxq_stats));
+		(*priv->rxqs)[i]->stats_reset = (*priv->rxqs)[i]->stats;
 	}
 	for (i = 0; (i != priv->txqs_n); ++i) {
-		if ((*priv->txqs)[i] == NULL)
+		struct mlx5_txq_data *txq_data = (*priv->txqs)[i];
+
+		if (txq_data == NULL)
 			continue;
-		memset(&(*priv->txqs)[i]->stats, 0,
-		       sizeof(struct mlx5_txq_stats));
+		txq_data->stats_reset = txq_data->stats;
 	}
 	mlx5_os_read_dev_stat(priv, "out_of_buffer", &stats_ctrl->imissed_base);
 	stats_ctrl->imissed = 0;
-- 
2.35.1

---
  Diff of the applied patch vs upstream commit (please double-check if non-empty:
---
--- -	2022-06-21 15:37:49.793124379 +0800
+++ 0011-net-mlx5-fix-Rx-Tx-stats-concurrency.patch	2022-06-21 15:37:48.977784372 +0800
@@ -1 +1 @@
-From 773a7de21a984bb7bdb6396a659ccc585a44a806 Mon Sep 17 00:00:00 2001
+From c4d51b3f05b56efe4e435187e67d698a07e30b94 Mon Sep 17 00:00:00 2001
@@ -4,0 +5,3 @@
+Cc: Xueming Li <xuemingl at nvidia.com>
+
+[ upstream commit 773a7de21a984bb7bdb6396a659ccc585a44a806 ]
@@ -17 +19,0 @@
-Cc: stable at dpdk.org
@@ -22,10 +24,22 @@
- drivers/net/mlx5/mlx5_rx.h    |  1 +
- drivers/net/mlx5/mlx5_stats.c | 40 +++++++++++++++++++++--------------
- drivers/net/mlx5/mlx5_tx.h    |  1 +
- 3 files changed, 26 insertions(+), 16 deletions(-)
-
-diff --git a/drivers/net/mlx5/mlx5_rx.h b/drivers/net/mlx5/mlx5_rx.h
-index 5bf88b6181..e715ed6b62 100644
---- a/drivers/net/mlx5/mlx5_rx.h
-+++ b/drivers/net/mlx5/mlx5_rx.h
-@@ -126,6 +126,7 @@ struct mlx5_rxq_data {
+ drivers/net/mlx5/mlx5_rxtx.c  |  1 +
+ drivers/net/mlx5/mlx5_rxtx.h  |  2 ++
+ drivers/net/mlx5/mlx5_stats.c | 41 ++++++++++++++++++++---------------
+ 3 files changed, 27 insertions(+), 17 deletions(-)
+
+diff --git a/drivers/net/mlx5/mlx5_rxtx.c b/drivers/net/mlx5/mlx5_rxtx.c
+index 734c462a09..edabe229d4 100644
+--- a/drivers/net/mlx5/mlx5_rxtx.c
++++ b/drivers/net/mlx5/mlx5_rxtx.c
+@@ -78,6 +78,7 @@ static uint16_t mlx5_tx_burst_##func(void *txq, \
+ }
+ 
+ #define MLX5_TXOFF_INFO(func, olx) {mlx5_tx_burst_##func, olx},
++	struct mlx5_txq_stats stats_reset; /* stats on last reset. */
+ 
+ static __rte_always_inline uint32_t
+ rxq_cq_to_pkt_type(struct mlx5_rxq_data *rxq, volatile struct mlx5_cqe *cqe,
+diff --git a/drivers/net/mlx5/mlx5_rxtx.h b/drivers/net/mlx5/mlx5_rxtx.h
+index 237a7faa5c..8760ab78f7 100644
+--- a/drivers/net/mlx5/mlx5_rxtx.h
++++ b/drivers/net/mlx5/mlx5_rxtx.h
+@@ -153,6 +153,7 @@ struct mlx5_rxq_data {
@@ -38 +52,9 @@
- 	struct mlx5_uar_data uar_data; /* CQ doorbell. */
+ 	void *cq_uar; /* Verbs CQ user access region. */
+@@ -271,6 +272,7 @@ struct mlx5_txq_data {
+ 	int32_t ts_offset; /* Timestamp field dynamic offset. */
+ 	struct mlx5_dev_ctx_shared *sh; /* Shared context. */
+ 	struct mlx5_txq_stats stats; /* TX queue counters. */
++	struct mlx5_txq_stats stats_reset; /* stats on last reset. */
+ #ifndef RTE_ARCH_64
+ 	rte_spinlock_t *uar_lock;
+ 	/* UAR access lock required for 32bit implementations */
@@ -40 +62 @@
-index 732775954a..f64fa3587b 100644
+index 82d4d4a745..450037aea0 100644
@@ -43 +65 @@
-@@ -114,18 +114,23 @@ mlx5_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
+@@ -113,18 +113,23 @@ mlx5_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
@@ -74 +96 @@
-@@ -135,15 +140,17 @@ mlx5_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
+@@ -134,15 +139,17 @@ mlx5_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
@@ -97,3 +119,3 @@
-@@ -185,13 +192,14 @@ mlx5_stats_reset(struct rte_eth_dev *dev)
- 
- 		if (rxq_data == NULL)
+@@ -182,14 +189,14 @@ mlx5_stats_reset(struct rte_eth_dev *dev)
+ 	for (i = 0; (i != priv->rxqs_n); ++i) {
+ 		if ((*priv->rxqs)[i] == NULL)
@@ -101,2 +123,3 @@
--		memset(&rxq_data->stats, 0, sizeof(struct mlx5_rxq_stats));
-+		rxq_data->stats_reset = rxq_data->stats;
+-		memset(&(*priv->rxqs)[i]->stats, 0,
+-		       sizeof(struct mlx5_rxq_stats));
++		(*priv->rxqs)[i]->stats_reset = (*priv->rxqs)[i]->stats;
@@ -116,12 +138,0 @@
-diff --git a/drivers/net/mlx5/mlx5_tx.h b/drivers/net/mlx5/mlx5_tx.h
-index dfa04612ff..20776919c2 100644
---- a/drivers/net/mlx5/mlx5_tx.h
-+++ b/drivers/net/mlx5/mlx5_tx.h
-@@ -164,6 +164,7 @@ struct mlx5_txq_data {
- 	int32_t ts_offset; /* Timestamp field dynamic offset. */
- 	struct mlx5_dev_ctx_shared *sh; /* Shared context. */
- 	struct mlx5_txq_stats stats; /* TX queue counters. */
-+	struct mlx5_txq_stats stats_reset; /* stats on last reset. */
- 	struct mlx5_uar_data uar_data;
- 	struct rte_mbuf *elts[0];
- 	/* Storage for queued packets, must be the last field. */


More information about the stable mailing list