[dpdk-dev] net/mlx5: cleanup the allocation of the buffer used for the ethtool stats

Message ID ab5d3d0e8269f0f30759f4b536287b8e03f27147.1510849974.git.thierry.herbelot@6wind.com (mailing list archive)
State Accepted, archived
Delegated to: Ferruh Yigit
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Thierry Herbelot Nov. 17, 2017, 1:51 p.m. UTC
  Simplify the computation for the needed size:
- exact size for the structure header,
- exact size for a number of 64-bit counters.

Fixes: a4193ae3bc4f ('net/mlx5: support extended statistics')

Signed-off-by: Thierry Herbelot <thierry.herbelot@6wind.com>
---
 drivers/net/mlx5/mlx5_stats.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)
  

Comments

Nélio Laranjeiro Nov. 17, 2017, 3:08 p.m. UTC | #1
On Fri, Nov 17, 2017 at 02:51:34PM +0100, Thierry Herbelot wrote:
> Simplify the computation for the needed size:
> - exact size for the structure header,
> - exact size for a number of 64-bit counters.
> 
> Fixes: a4193ae3bc4f ('net/mlx5: support extended statistics')
> 
> Signed-off-by: Thierry Herbelot <thierry.herbelot@6wind.com>
> ---
>  drivers/net/mlx5/mlx5_stats.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/mlx5/mlx5_stats.c b/drivers/net/mlx5/mlx5_stats.c
> index 5e225d3740d2..2427585f71ca 100644
> --- a/drivers/net/mlx5/mlx5_stats.c
> +++ b/drivers/net/mlx5/mlx5_stats.c
> @@ -143,11 +143,9 @@ priv_read_dev_counters(struct priv *priv, uint64_t *stats)
>  	struct mlx5_xstats_ctrl *xstats_ctrl = &priv->xstats_ctrl;
>  	unsigned int i;
>  	struct ifreq ifr;
> -	unsigned int stats_sz = (xstats_ctrl->stats_n * sizeof(uint64_t)) +
> -				 sizeof(struct ethtool_stats);
> -	struct ethtool_stats et_stats[(stats_sz + (
> -				      sizeof(struct ethtool_stats) - 1)) /
> -				      sizeof(struct ethtool_stats)];
> +	unsigned int stats_sz = xstats_ctrl->stats_n * sizeof(uint64_t);
> +	unsigned char et_stat_buf[sizeof(struct ethtool_stats) + stats_sz];
> +	struct ethtool_stats *et_stats = (struct ethtool_stats *)et_stat_buf;
>  
>  	et_stats->cmd = ETHTOOL_GSTATS;
>  	et_stats->n_stats = xstats_ctrl->stats_n;
> -- 
> 2.11.0
 
Acked-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>

Thanks,
  
Shahaf Shuler Nov. 23, 2017, 1 p.m. UTC | #2
Hi Thierry

Friday, November 17, 2017 9:55 AM, Nelio Laranjeiro:
> > 2.11.0
> 
> Acked-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>

Am having the following check-git-log errors:

Headline too long:
        net/mlx5: cleanup the allocation of the buffer used for the ethtool stats
Wrong tag:
        Fixes: a4193ae3bc4f ('net/mlx5: support extended statistics')
Wrong 'Fixes' reference:
        Fixes: a4193ae3bc4f ('net/mlx5: support extended statistics')
Is it candidate for Cc: stable@dpdk.org backport?
        net/mlx5: cleanup the allocation of the buffer used for the ethtool stats

is the below suggestion[1] acceptable? If so I will merge into next-net-mlx

[1]
Author: Thierry Herbelot <thierry.herbelot@6wind.com>
Date:   Fri Nov 17 14:51:34 2017 +0100

    net/mlx5: cleanup allocation of ethtool stats
    
    Simplify the computation for the needed size:
    - exact size for the structure header,
    - exact size for a number of 64-bit counters.
    
    Fixes: a4193ae3bc4f ("net/mlx5: support extended statistics")
    Cc: stable@dpdk.org
    
    Signed-off-by: Thierry Herbelot <thierry.herbelot@6wind.com>
    Acked-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>




> 
> Thanks,
> 
> --
> Nélio Laranjeiro
> 6WIND
  
Thierry Herbelot Nov. 23, 2017, 1:06 p.m. UTC | #3
On 11/23/2017 02:00 PM, Shahaf Shuler wrote:
> Hi Thierry
> 
> Friday, November 17, 2017 9:55 AM, Nelio Laranjeiro:
>>> 2.11.0
>>
>> Acked-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
> 
> Am having the following check-git-log errors:
> 
> Headline too long:
>          net/mlx5: cleanup the allocation of the buffer used for the ethtool stats
> Wrong tag:
>          Fixes: a4193ae3bc4f ('net/mlx5: support extended statistics')
> Wrong 'Fixes' reference:
>          Fixes: a4193ae3bc4f ('net/mlx5: support extended statistics')
> Is it candidate for Cc: stable@dpdk.org backport?
>          net/mlx5: cleanup the allocation of the buffer used for the ethtool stats
> 
> is the below suggestion[1] acceptable? If so I will merge into next-net-mlx
> 
> [1]
> Author: Thierry Herbelot <thierry.herbelot@6wind.com>
> Date:   Fri Nov 17 14:51:34 2017 +0100
> 
>      net/mlx5: cleanup allocation of ethtool stats
>      
>      Simplify the computation for the needed size:
>      - exact size for the structure header,
>      - exact size for a number of 64-bit counters.
>      
>      Fixes: a4193ae3bc4f ("net/mlx5: support extended statistics")
>      Cc: stable@dpdk.org
>      
>      Signed-off-by: Thierry Herbelot <thierry.herbelot@6wind.com>
>      Acked-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>


Hello Shahaf

Your suggestion is excellent: thanks for it.

	Best regards

	Thierry
> 
> 
> 
> 
>>
>> Thanks,
>>
>> --
>> Nélio Laranjeiro
>> 6WIND
  
Shahaf Shuler Nov. 23, 2017, 1:20 p.m. UTC | #4
Thursday, November 23, 2017 3:06 PM, Thierry Herbelot:
> On 11/23/2017 02:00 PM, Shahaf Shuler wrote:

> Hello Shahaf

> 

> Your suggestion is excellent: thanks for it.

> 

> 	Best regards

> 

> 	Thierry


Applied to next-net-mlx, thanks.

> >

> >

> >

> >

> >>

> >> Thanks,

> >>

> >> --

> >> Nélio Laranjeiro

> >> 6WIND

> 

> 

> --

> Thierry Herbelot

> 6WIND

> Software Engineer
  

Patch

diff --git a/drivers/net/mlx5/mlx5_stats.c b/drivers/net/mlx5/mlx5_stats.c
index 5e225d3740d2..2427585f71ca 100644
--- a/drivers/net/mlx5/mlx5_stats.c
+++ b/drivers/net/mlx5/mlx5_stats.c
@@ -143,11 +143,9 @@  priv_read_dev_counters(struct priv *priv, uint64_t *stats)
 	struct mlx5_xstats_ctrl *xstats_ctrl = &priv->xstats_ctrl;
 	unsigned int i;
 	struct ifreq ifr;
-	unsigned int stats_sz = (xstats_ctrl->stats_n * sizeof(uint64_t)) +
-				 sizeof(struct ethtool_stats);
-	struct ethtool_stats et_stats[(stats_sz + (
-				      sizeof(struct ethtool_stats) - 1)) /
-				      sizeof(struct ethtool_stats)];
+	unsigned int stats_sz = xstats_ctrl->stats_n * sizeof(uint64_t);
+	unsigned char et_stat_buf[sizeof(struct ethtool_stats) + stats_sz];
+	struct ethtool_stats *et_stats = (struct ethtool_stats *)et_stat_buf;
 
 	et_stats->cmd = ETHTOOL_GSTATS;
 	et_stats->n_stats = xstats_ctrl->stats_n;