net/mlx5e: Add mlx5e HV VHCA stats agent
Hello Eran Ben Elisha,
The patch cef35af34d6d: "net/mlx5e: Add mlx5e HV VHCA stats agent" from Aug 22, 2019, leads to the following static checker warning:
drivers/net/ethernet/mellanox/mlx5/core/en/hv_vhca_stats.c:41 mlx5e_hv_vhca_fill_stats()
warn: potential pointer math issue ('buf' is a u64 pointer)
drivers/net/ethernet/mellanox/mlx5/core/en/hv_vhca_stats.c
33 static void mlx5e_hv_vhca_fill_stats(struct mlx5e_priv *priv, u64 *data,
^^^^^^^^^
data is a u64 pointer.
34 int buf_len)
35 {
36 int ch, i = 0;
37
38 for (ch = 0; ch < priv->max_nch; ch++) {
39 u64 *buf = data + i;
^^^^^^^^
40
41 if (WARN_ON_ONCE(buf +
42 sizeof(struct mlx5e_hv_vhca_per_ring_stats) >
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^This pointer math doesn't work. I'm surprised the warning doesn't trigger.
43 data + buf_len))
44 return;
45
46 mlx5e_hv_vhca_fill_ring_stats(priv, ch,
47 (struct mlx5e_hv_vhca_per_ring_stats *)buf);
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^If we made both "data" and "buf" void pointers then this would be much easier and the casting could be removed.
48 i += sizeof(struct mlx5e_hv_vhca_per_ring_stats) / sizeof(u64);
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^This math works, but it's so complicated and you have to verify that mlx5e_hv_vhca_per_ring_stats % sizeof(u64) is zero.
49 }
50 }regards,
dan carpenter
Quoted 25 lines · Dan Carpenter
Hello Eran Ben Elisha,
The patch
cef35af34d6d: "net/mlx5e: Add mlx5e HV VHCA stats agent" from Aug 22, 2019, leads to the following static checker warning:
drivers/net/ethernet/mellanox/mlx5/core/en/hv_vhca_stats.c:41mlx5e_hv_vhca_fill_stats()
warn: potential pointer math issue ('buf' is a u64 pointer)
drivers/net/ethernet/mellanox/mlx5/core/en/hv_vhca_stats.c
33 static void mlx5e_hv_vhca_fill_stats(structmlx5e_priv*priv, u64 *data,
^^^^^^^^^
data is a u64 pointer.34 int buf_len) 35 { 36 int ch, i = 0; 37 38 for (ch = 0; ch < priv->max_nch; ch++) { 39 u64 *buf = data + i; ^^^^^^^^ 40 41 if (WARN_ON_ONCE(buf + 42 sizeof(struct mlx5e_hv_vhca_per_ring_stats) > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Quoted 2 lines
This pointer math doesn't work. I'm surprised the warning doesn't trigger.
It it not triggered as both 'data' and 'buf' are u64*,
and sizeof(struct mlx5e_hv_vhca_per_ring_stats) < buf_len as expected. This checker does the work, but over wrong range.
Quoted 9 lines
43 data + buf_len)) 44 return; 45 46 mlx5e_hv_vhca_fill_ring_stats(priv, ch, 47 (struct mlx5e_hv_vhca_per_ring_stats *)buf); ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^If we made both "data" and "buf" void pointers then this would be much easier and the casting could be removed.
I agree I can make it simpler, I will post a fix.
Quoted 6 lines
48 i += sizeof(struct mlx5e_hv_vhca_per_ring_stats) / sizeof(u64); ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^This math works, but it's so complicated and you have to verify that
mlx5e_hv_vhca_per_ring_stats% sizeof(u64) is zero.
It shouldn't be zero. Basically, it equals to a static value of 4... the code is written in that way to support extension of mlx5e_hv_vhca_per_ring_stats without touching here.
Quoted 6 lines
49 } 50 }regards,
dan carpenter
Thanks you for the report,
Eran
Quoted 1 line
Quoted 31 lines · Eran Ben Elisha
Quoted 25 lines · Dan Carpenter
Hello Eran Ben Elisha,
The patch
cef35af34d6d: "net/mlx5e: Add mlx5e HV VHCA stats agent" from Aug 22, 2019, leads to the following static checker warning:
drivers/net/ethernet/mellanox/mlx5/core/en/hv_vhca_stats.c:41mlx5e_hv_vhca_fill_stats()
warn: potential pointer math issue ('buf' is a u64 pointer)
drivers/net/ethernet/mellanox/mlx5/core/en/hv_vhca_stats.c
33 static void mlx5e_hv_vhca_fill_stats(structmlx5e_priv*priv, u64 *data,
^^^^^^^^^
data is a u64 pointer.34 int buf_len) 35 { 36 int ch, i = 0; 37 38 for (ch = 0; ch < priv->max_nch; ch++) { 39 u64 *buf = data + i; ^^^^^^^^ 40 41 if (WARN_ON_ONCE(buf + 42 sizeof(struct mlx5e_hv_vhca_per_ring_stats) > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^Quoted 2 lines
This pointer math doesn't work. I'm surprised the warning doesn't trigger.
Quoted 3 lines
It it not triggered as both 'data' and 'buf' are u64*,
andsizeof(struct mlx5e_hv_vhca_per_ring_stats)<buf_lenas expected. This checker does the work, but over wrong range.
Ah. Of course. Thanks!
regards,
dan carpenter