* [PATCH net v3] net/mlx5e: Fix race in mlx5e_sw_stats and mlx5e_vport_stats
@ 2017-04-21 4:40 Martin KaFai Lau
2017-04-22 0:24 ` Saeed Mahameed
2017-04-24 16:17 ` David Miller
0 siblings, 2 replies; 3+ messages in thread
From: Martin KaFai Lau @ 2017-04-21 4:40 UTC (permalink / raw)
To: netdev; +Cc: Saeed Mahameed, Eric Dumazet, kernel-team
We have observed a sudden spike in rx/tx_packets and rx/tx_bytes
reported under /proc/net/dev. There is a race in mlx5e_update_stats()
and some of the get-stats functions (the one that we hit is the
mlx5e_get_stats() which is called by ndo_get_stats64()).
In particular, the very first thing mlx5e_update_sw_counters()
does is 'memset(s, 0, sizeof(*s))'. For example, if mlx5e_get_stats()
is unlucky at one point, rx_bytes and rx_packets could be 0. One second
later, a normal (and much bigger than 0) value will be reported.
This patch is to use a 'struct mlx5e_sw_stats temp' to avoid
a direct memset zero on priv->stats.sw.
mlx5e_update_vport_counters() has a similar race. Hence, addressed
together. However, memset zero is removed instead because
it is not needed.
I am lucky enough to catch this 0-reset in rx multicast:
eth0: 41457665 76804 70 0 0 70 0 47085 15586634 87502 3 0 0 0 3 0
eth0: 41459860 76815 70 0 0 70 0 47094 15588376 87516 3 0 0 0 3 0
eth0: 41460577 76822 70 0 0 70 0 0 15589083 87521 3 0 0 0 3 0
eth0: 41463293 76838 70 0 0 70 0 47108 15595872 87538 3 0 0 0 3 0
eth0: 41463379 76839 70 0 0 70 0 47116 15596138 87539 3 0 0 0 3 0
v2: Remove memset zero from mlx5e_update_vport_counters()
v1: Use temp and memcpy
Fixes: 9218b44dcc05 ("net/mlx5e: Statistics handling refactoring")
Suggested-by: Eric Dumazet <eric.dumazet@gmail.com>
Suggested-by: Saeed Mahameed <saeedm@mellanox.com>
Signed-off-by: Martin KaFai Lau <kafai@fb.com>
---
drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index 66c133757a5e..15cc7b469d2e 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -174,7 +174,7 @@ static void mlx5e_tx_timeout_work(struct work_struct *work)
static void mlx5e_update_sw_counters(struct mlx5e_priv *priv)
{
- struct mlx5e_sw_stats *s = &priv->stats.sw;
+ struct mlx5e_sw_stats temp, *s = &temp;
struct mlx5e_rq_stats *rq_stats;
struct mlx5e_sq_stats *sq_stats;
u64 tx_offload_none = 0;
@@ -229,6 +229,7 @@ static void mlx5e_update_sw_counters(struct mlx5e_priv *priv)
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));
}
static void mlx5e_update_vport_counters(struct mlx5e_priv *priv)
@@ -243,7 +244,6 @@ static void mlx5e_update_vport_counters(struct mlx5e_priv *priv)
MLX5_SET(query_vport_counter_in, in, op_mod, 0);
MLX5_SET(query_vport_counter_in, in, other_vport, 0);
- memset(out, 0, outlen);
mlx5_cmd_exec(mdev, in, sizeof(in), out, outlen);
}
--
2.9.3
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH net v3] net/mlx5e: Fix race in mlx5e_sw_stats and mlx5e_vport_stats
2017-04-21 4:40 [PATCH net v3] net/mlx5e: Fix race in mlx5e_sw_stats and mlx5e_vport_stats Martin KaFai Lau
@ 2017-04-22 0:24 ` Saeed Mahameed
2017-04-24 16:17 ` David Miller
1 sibling, 0 replies; 3+ messages in thread
From: Saeed Mahameed @ 2017-04-22 0:24 UTC (permalink / raw)
To: Martin KaFai Lau
Cc: Linux Netdev List, Saeed Mahameed, Eric Dumazet, Kernel Team
On Fri, Apr 21, 2017 at 7:40 AM, Martin KaFai Lau <kafai@fb.com> wrote:
> We have observed a sudden spike in rx/tx_packets and rx/tx_bytes
> reported under /proc/net/dev. There is a race in mlx5e_update_stats()
> and some of the get-stats functions (the one that we hit is the
> mlx5e_get_stats() which is called by ndo_get_stats64()).
>
> In particular, the very first thing mlx5e_update_sw_counters()
> does is 'memset(s, 0, sizeof(*s))'. For example, if mlx5e_get_stats()
> is unlucky at one point, rx_bytes and rx_packets could be 0. One second
> later, a normal (and much bigger than 0) value will be reported.
>
> This patch is to use a 'struct mlx5e_sw_stats temp' to avoid
> a direct memset zero on priv->stats.sw.
>
> mlx5e_update_vport_counters() has a similar race. Hence, addressed
> together. However, memset zero is removed instead because
> it is not needed.
>
> I am lucky enough to catch this 0-reset in rx multicast:
> eth0: 41457665 76804 70 0 0 70 0 47085 15586634 87502 3 0 0 0 3 0
> eth0: 41459860 76815 70 0 0 70 0 47094 15588376 87516 3 0 0 0 3 0
> eth0: 41460577 76822 70 0 0 70 0 0 15589083 87521 3 0 0 0 3 0
> eth0: 41463293 76838 70 0 0 70 0 47108 15595872 87538 3 0 0 0 3 0
> eth0: 41463379 76839 70 0 0 70 0 47116 15596138 87539 3 0 0 0 3 0
>
> v2: Remove memset zero from mlx5e_update_vport_counters()
> v1: Use temp and memcpy
>
> Fixes: 9218b44dcc05 ("net/mlx5e: Statistics handling refactoring")
> Suggested-by: Eric Dumazet <eric.dumazet@gmail.com>
> Suggested-by: Saeed Mahameed <saeedm@mellanox.com>
> Signed-off-by: Martin KaFai Lau <kafai@fb.com>
> ---
Thank you Martin and Eric for the fix, we will follow up with the
spinlock fix soon.
Acked-by: Saeed Mahameed <saeedm@mellanox.com>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH net v3] net/mlx5e: Fix race in mlx5e_sw_stats and mlx5e_vport_stats
2017-04-21 4:40 [PATCH net v3] net/mlx5e: Fix race in mlx5e_sw_stats and mlx5e_vport_stats Martin KaFai Lau
2017-04-22 0:24 ` Saeed Mahameed
@ 2017-04-24 16:17 ` David Miller
1 sibling, 0 replies; 3+ messages in thread
From: David Miller @ 2017-04-24 16:17 UTC (permalink / raw)
To: kafai; +Cc: netdev, saeedm, eric.dumazet, kernel-team
From: Martin KaFai Lau <kafai@fb.com>
Date: Thu, 20 Apr 2017 21:40:12 -0700
> We have observed a sudden spike in rx/tx_packets and rx/tx_bytes
> reported under /proc/net/dev. There is a race in mlx5e_update_stats()
> and some of the get-stats functions (the one that we hit is the
> mlx5e_get_stats() which is called by ndo_get_stats64()).
>
> In particular, the very first thing mlx5e_update_sw_counters()
> does is 'memset(s, 0, sizeof(*s))'. For example, if mlx5e_get_stats()
> is unlucky at one point, rx_bytes and rx_packets could be 0. One second
> later, a normal (and much bigger than 0) value will be reported.
>
> This patch is to use a 'struct mlx5e_sw_stats temp' to avoid
> a direct memset zero on priv->stats.sw.
>
> mlx5e_update_vport_counters() has a similar race. Hence, addressed
> together. However, memset zero is removed instead because
> it is not needed.
>
> I am lucky enough to catch this 0-reset in rx multicast:
> eth0: 41457665 76804 70 0 0 70 0 47085 15586634 87502 3 0 0 0 3 0
> eth0: 41459860 76815 70 0 0 70 0 47094 15588376 87516 3 0 0 0 3 0
> eth0: 41460577 76822 70 0 0 70 0 0 15589083 87521 3 0 0 0 3 0
> eth0: 41463293 76838 70 0 0 70 0 47108 15595872 87538 3 0 0 0 3 0
> eth0: 41463379 76839 70 0 0 70 0 47116 15596138 87539 3 0 0 0 3 0
>
> v2: Remove memset zero from mlx5e_update_vport_counters()
> v1: Use temp and memcpy
>
> Fixes: 9218b44dcc05 ("net/mlx5e: Statistics handling refactoring")
> Suggested-by: Eric Dumazet <eric.dumazet@gmail.com>
> Suggested-by: Saeed Mahameed <saeedm@mellanox.com>
> Signed-off-by: Martin KaFai Lau <kafai@fb.com>
Applied, thanks Martin.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-04-24 16:17 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-21 4:40 [PATCH net v3] net/mlx5e: Fix race in mlx5e_sw_stats and mlx5e_vport_stats Martin KaFai Lau
2017-04-22 0:24 ` Saeed Mahameed
2017-04-24 16:17 ` David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.