lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20170512115650.11635-2-saeedm@mellanox.com>
Date:   Fri, 12 May 2017 14:56:45 +0300
From:   Saeed Mahameed <saeedm@...lanox.com>
To:     "David S. Miller" <davem@...emloft.net>
Cc:     netdev@...r.kernel.org, Gal Pressman <galp@...lanox.com>,
        kernel-team@...com, Saeed Mahameed <saeedm@...lanox.com>
Subject: [net 1/6] net/mlx5e: Use a spinlock to synchronize statistics

From: Gal Pressman <galp@...lanox.com>

Add a spinlock to prevent races when querying statistics, for example
querying counters in the middle of a non atomic memcpy() operation in
mlx5e_update_stats().

This RW lock should be held when accessing priv->stats, to prevent other
reads/writes.

Fixes: 9218b44dcc05 ("net/mlx5e: Statistics handling refactoring")
Signed-off-by: Gal Pressman <galp@...lanox.com>
Suggested-by: Eric Dumazet <eric.dumazet@...il.com>
Cc: kernel-team@...com
Signed-off-by: Saeed Mahameed <saeedm@...lanox.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en.h       |  1 +
 .../net/ethernet/mellanox/mlx5/core/en_ethtool.c   | 12 ++++--
 drivers/net/ethernet/mellanox/mlx5/core/en_main.c  | 45 ++++++++++++++++------
 3 files changed, 44 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en.h b/drivers/net/ethernet/mellanox/mlx5/core/en.h
index 0099a3e397bc..c41cf7e82795 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en.h
@@ -756,6 +756,7 @@ struct mlx5e_priv {
 	struct mlx5_core_dev      *mdev;
 	struct net_device         *netdev;
 	struct mlx5e_stats         stats;
+	rwlock_t                   stats_lock;
 	struct mlx5e_tstamp        tstamp;
 	u16 q_counter;
 #ifdef CONFIG_MLX5_CORE_EN_DCB
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
index ce7b09d72ff6..0a7734761ece 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
@@ -305,6 +305,7 @@ static void mlx5e_get_ethtool_stats(struct net_device *dev,
 	struct mlx5_priv *mlx5_priv;
 	int i, j, tc, prio, idx = 0;
 	unsigned long pfc_combined;
+	bool global_pause;
 
 	if (!data)
 		return;
@@ -313,8 +314,11 @@ static void mlx5e_get_ethtool_stats(struct net_device *dev,
 	if (test_bit(MLX5E_STATE_OPENED, &priv->state))
 		mlx5e_update_stats(priv);
 	channels = &priv->channels;
+	pfc_combined = mlx5e_query_pfc_combined(priv);
+	global_pause = mlx5e_query_global_pause_combined(priv);
 	mutex_unlock(&priv->state_lock);
 
+	read_lock(&priv->stats_lock);
 	for (i = 0; i < NUM_SW_COUNTERS; i++)
 		data[idx++] = MLX5E_READ_CTR64_CPU(&priv->stats.sw,
 						   sw_stats_desc, i);
@@ -353,7 +357,6 @@ static void mlx5e_get_ethtool_stats(struct net_device *dev,
 						 pport_per_prio_traffic_stats_desc, i);
 	}
 
-	pfc_combined = mlx5e_query_pfc_combined(priv);
 	for_each_set_bit(prio, &pfc_combined, NUM_PPORT_PRIO) {
 		for (i = 0; i < NUM_PPORT_PER_PRIO_PFC_COUNTERS; i++) {
 			data[idx++] = MLX5E_READ_CTR64_BE(&priv->stats.pport.per_prio_counters[prio],
@@ -361,7 +364,7 @@ static void mlx5e_get_ethtool_stats(struct net_device *dev,
 		}
 	}
 
-	if (mlx5e_query_global_pause_combined(priv)) {
+	if (global_pause) {
 		for (i = 0; i < NUM_PPORT_PER_PRIO_PFC_COUNTERS; i++) {
 			data[idx++] = MLX5E_READ_CTR64_BE(&priv->stats.pport.per_prio_counters[0],
 							  pport_per_prio_pfc_stats_desc, i);
@@ -379,7 +382,7 @@ static void mlx5e_get_ethtool_stats(struct net_device *dev,
 						   mlx5e_pme_error_desc, i);
 
 	if (!test_bit(MLX5E_STATE_OPENED, &priv->state))
-		return;
+		goto out;
 
 	/* per channel counters */
 	for (i = 0; i < channels->num; i++)
@@ -393,6 +396,9 @@ static void mlx5e_get_ethtool_stats(struct net_device *dev,
 			for (j = 0; j < NUM_SQ_STATS; j++)
 				data[idx++] = MLX5E_READ_CTR64_CPU(&channels->c[i]->sq[tc].stats,
 								   sq_stats_desc, j);
+
+out:
+	read_unlock(&priv->stats_lock);
 }
 
 static u32 mlx5e_rx_wqes_to_packets(struct mlx5e_priv *priv, int rq_wq_type,
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index a61b71b6fff3..7ac6bcc0e227 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -166,6 +166,13 @@ static void mlx5e_tx_timeout_work(struct work_struct *work)
 	rtnl_unlock();
 }
 
+#define MLX5E_STATS_WRITE(priv, name, s)                            \
+	do {                                                        \
+		write_lock(&(priv)->stats_lock);                    \
+		memcpy(&(priv)->stats.name, (s), sizeof(*(s)));     \
+		write_unlock(&(priv)->stats_lock);                  \
+	} while (0)
+
 static void mlx5e_update_sw_counters(struct mlx5e_priv *priv)
 {
 	struct mlx5e_sw_stats temp, *s = &temp;
@@ -222,17 +229,21 @@ static void mlx5e_update_sw_counters(struct mlx5e_priv *priv)
 	s->tx_csum_partial = s->tx_packets - tx_offload_none - s->tx_csum_partial_inner;
 	s->rx_csum_unnecessary = s->rx_packets - s->rx_csum_none - s->rx_csum_complete;
 
+	read_lock(&priv->stats_lock);
 	s->link_down_events_phy = MLX5_GET(ppcnt_reg,
 				priv->stats.pport.phy_counters,
 				counter_set.phys_layer_cntrs.link_down_events);
-	memcpy(&priv->stats.sw, s, sizeof(*s));
+	read_unlock(&priv->stats_lock);
+
+	MLX5E_STATS_WRITE(priv, sw, s);
 }
 
 static void mlx5e_update_vport_counters(struct mlx5e_priv *priv)
 {
 	int outlen = MLX5_ST_SZ_BYTES(query_vport_counter_out);
-	u32 *out = (u32 *)priv->stats.vport.query_vport_out;
 	u32 in[MLX5_ST_SZ_DW(query_vport_counter_in)] = {0};
+	struct mlx5e_vport_stats          vstats;
+	u32 *out = (u32 *)vstats.query_vport_out;
 	struct mlx5_core_dev *mdev = priv->mdev;
 
 	MLX5_SET(query_vport_counter_in, in, opcode,
@@ -241,19 +252,22 @@ static void mlx5e_update_vport_counters(struct mlx5e_priv *priv)
 	MLX5_SET(query_vport_counter_in, in, other_vport, 0);
 
 	mlx5_cmd_exec(mdev, in, sizeof(in), out, outlen);
+
+	MLX5E_STATS_WRITE(priv, vport, &vstats);
 }
 
 static void mlx5e_update_pport_counters(struct mlx5e_priv *priv)
 {
-	struct mlx5e_pport_stats *pstats = &priv->stats.pport;
 	struct mlx5_core_dev *mdev = priv->mdev;
 	int sz = MLX5_ST_SZ_BYTES(ppcnt_reg);
-	int prio;
+	struct mlx5e_pport_stats *pstats;
 	void *out;
+	int prio;
 	u32 *in;
 
-	in = mlx5_vzalloc(sz);
-	if (!in)
+	in     = mlx5_vzalloc(sz);
+	pstats = mlx5_vzalloc(sizeof(*pstats));
+	if (!in || !pstats)
 		goto free_out;
 
 	MLX5_SET(ppcnt_reg, in, local_port, 1);
@@ -288,26 +302,31 @@ static void mlx5e_update_pport_counters(struct mlx5e_priv *priv)
 				     MLX5_REG_PPCNT, 0, 0);
 	}
 
+	MLX5E_STATS_WRITE(priv, pport, pstats);
+
 free_out:
 	kvfree(in);
+	kvfree(pstats);
 }
 
 static void mlx5e_update_q_counter(struct mlx5e_priv *priv)
 {
-	struct mlx5e_qcounter_stats *qcnt = &priv->stats.qcnt;
+	struct mlx5e_qcounter_stats qcnt;
 
 	if (!priv->q_counter)
 		return;
 
 	mlx5_core_query_out_of_buffer(priv->mdev, priv->q_counter,
-				      &qcnt->rx_out_of_buffer);
+				      &qcnt.rx_out_of_buffer);
+
+	MLX5E_STATS_WRITE(priv, qcnt, &qcnt);
 }
 
 static void mlx5e_update_pcie_counters(struct mlx5e_priv *priv)
 {
-	struct mlx5e_pcie_stats *pcie_stats = &priv->stats.pcie;
 	struct mlx5_core_dev *mdev = priv->mdev;
 	int sz = MLX5_ST_SZ_BYTES(mpcnt_reg);
+	struct mlx5e_pcie_stats pcie_stats;
 	void *out;
 	u32 *in;
 
@@ -318,10 +337,12 @@ static void mlx5e_update_pcie_counters(struct mlx5e_priv *priv)
 	if (!in)
 		return;
 
-	out = pcie_stats->pcie_perf_counters;
+	out = pcie_stats.pcie_perf_counters;
 	MLX5_SET(mpcnt_reg, in, grp, MLX5_PCIE_PERFORMANCE_COUNTERS_GROUP);
 	mlx5_core_access_reg(mdev, in, sz, out, sz, MLX5_REG_MPCNT, 0, 0);
 
+	MLX5E_STATS_WRITE(priv, pcie, &pcie_stats);
+
 	kvfree(in);
 }
 
@@ -3030,6 +3051,7 @@ mlx5e_get_stats(struct net_device *dev, struct rtnl_link_stats64 *stats)
 	struct mlx5e_vport_stats *vstats = &priv->stats.vport;
 	struct mlx5e_pport_stats *pstats = &priv->stats.pport;
 
+	read_lock(&priv->stats_lock);
 	if (mlx5e_is_uplink_rep(priv)) {
 		stats->rx_packets = PPORT_802_3_GET(pstats, a_frames_received_ok);
 		stats->rx_bytes   = PPORT_802_3_GET(pstats, a_octets_received_ok);
@@ -3064,7 +3086,7 @@ mlx5e_get_stats(struct net_device *dev, struct rtnl_link_stats64 *stats)
 	 */
 	stats->multicast =
 		VPORT_COUNTER_GET(vstats, received_eth_multicast.packets);
-
+	read_unlock(&priv->stats_lock);
 }
 
 static void mlx5e_set_rx_mode(struct net_device *dev)
@@ -3901,6 +3923,7 @@ static void mlx5e_build_nic_netdev_priv(struct mlx5_core_dev *mdev,
 	mlx5e_build_nic_params(mdev, &priv->channels.params, profile->max_nch(mdev));
 
 	mutex_init(&priv->state_lock);
+	rwlock_init(&priv->stats_lock);
 
 	INIT_WORK(&priv->update_carrier_work, mlx5e_update_carrier_work);
 	INIT_WORK(&priv->set_rx_mode_work, mlx5e_set_rx_mode_work);
-- 
2.11.0

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ