Re: [dpdk-stable] [PATCH] [18.11] net/mlx5: fix xstats reset reinitialization

[Kevin Traynor <ktraynor@redhat.com>]

On 08/12/2020 07:54, Shiri Kuzin wrote:
> [ upstream commit 42dcd453d9b63841a5460a6ca3872eb7648d73bd ]
> 
> The mlx5_xstats_reset clears the device extended statistics.
> In this function the driver may reinitialize the structures
> that are used to read device counters.
> 
> In case of reinitialization, the number of counters may
> change, which wouldn't be taken into account by the
> reset API callback and can cause a segmentation fault.
> 
> This issue is fixed by allocating the counters size after
> the reinitialization.
> 
> Fixes: a4193ae3bc4f ("net/mlx5: support extended statistics")
> 
> Reported-by: Ralf Hoffmann <ralf.hoffmann@allegro-packets.com>
> Signed-off-by: Shiri Kuzin <shirik@nvidia.com>
> Acked-by: Matan Azrad <matan@nvidia.com>
> ---

Hi, this does not compile

../drivers/net/mlx5/mlx5_stats.c: In function ‘mlx5_xstats_reset’:
../drivers/net/mlx5/mlx5_stats.c:493:10: warning: ‘return’ with a value,
in function returning void [-Wreturn-type]
  493 |   return -rte_errno;
../drivers/net/mlx5/mlx5_stats.c:470:1: note: declared here
  470 | mlx5_xstats_reset(struct rte_eth_dev *dev)
      | ^~~~~~~~~~~~~~~~~
../drivers/net/mlx5/mlx5_stats.c:500:10: warning: ‘return’ with a value,
in function returning void [-Wreturn-type]
  500 |   return ret;
      |          ^~~
../drivers/net/mlx5/mlx5_stats.c:470:1: note: declared here
  470 | mlx5_xstats_reset(struct rte_eth_dev *dev)
      | ^~~~~~~~~~~~~~~~~
../drivers/net/mlx5/mlx5_stats.c:504:14: error: ‘struct
mlx5_xstats_ctrl’ has no member named ‘hw_stats’
  504 |   xstats_ctrl->hw_stats[i] = 0;
      |              ^~
../drivers/net/mlx5/mlx5_stats.c:507:9: warning: ‘return’ with a value,
in function returning void [-Wreturn-type]
  507 |  return 0;
      |         ^
../drivers/net/mlx5/mlx5_stats.c:470:1: note: declared here
  470 | mlx5_xstats_reset(struct rte_eth_dev *dev)
      | ^~~~~~~~~~~~~~~~~
[9/623] Compiling C object
drivers/libtmp_rte_pmd_mlx5.a.p/net_mlx5_mlx5_rxtx_vec.c.o



>  drivers/net/mlx5/mlx5_stats.c | 20 ++++++++++++++++----
>  1 file changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/mlx5/mlx5_stats.c b/drivers/net/mlx5/mlx5_stats.c
> index 6906dc81cc..38a2ffba4a 100644
> --- a/drivers/net/mlx5/mlx5_stats.c
> +++ b/drivers/net/mlx5/mlx5_stats.c
> @@ -473,8 +473,7 @@ mlx5_xstats_reset(struct rte_eth_dev *dev)
>  	struct mlx5_xstats_ctrl *xstats_ctrl = &priv->xstats_ctrl;
>  	int stats_n;
>  	unsigned int i;
> -	unsigned int n = xstats_ctrl->mlx5_stats_n;
> -	uint64_t counters[n];
> +	uint64_t *counters;
>  	int ret;
>  
>  	stats_n = mlx5_ethtool_get_stats_n(dev);
> @@ -485,14 +484,27 @@ mlx5_xstats_reset(struct rte_eth_dev *dev)
>  	}
>  	if (xstats_ctrl->stats_n != stats_n)
>  		mlx5_stats_init(dev);
> +	counters =  malloc(sizeof(*counters) * xstats_ctrl->mlx5_stats_n);
> +	if (!counters) {
> +		DRV_LOG(WARNING, "port %u unable to allocate memory for xstats "
> +				"counters",
> +		     dev->data->port_id);
> +		rte_errno = ENOMEM;
> +		return -rte_errno;
> +	}
>  	ret = mlx5_read_dev_counters(dev, counters);
>  	if (ret) {
>  		DRV_LOG(ERR, "port %u cannot read device counters: %s",
>  			dev->data->port_id, strerror(rte_errno));
> -		return;
> +		free(counters);
> +		return ret;
>  	}
> -	for (i = 0; i != n; ++i)
> +	for (i = 0; i != xstats_ctrl->mlx5_stats_n; ++i) {
>  		xstats_ctrl->base[i] = counters[i];
> +		xstats_ctrl->hw_stats[i] = 0;
> +	}
> +	free(counters);
> +	return 0;
>  }
>  
>  /**
>