net/mlx5: fix xstats reset reinitialization

Message ID 1603089410-11248-1-git-send-email-shirik@nvidia.com (mailing list archive)
State Accepted, archived
Delegated to: Raslan Darawsheh
Headers
Series net/mlx5: fix xstats reset reinitialization |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-testing success Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/travis-robot success Travis build: passed
ci/Intel-compilation success Compilation OK

Commit Message

Shiri Kuzin Oct. 19, 2020, 6:36 a.m. UTC
  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")
Cc: stable@dpdk.org

Signed-off-by: Shiri Kuzin <shirik@nvidia.com>
Acked-by: Matan Azrad <matan@nvidia.com>
---
 drivers/net/mlx5/mlx5_stats.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)
  

Comments

Shiri Kuzin Oct. 19, 2020, 1:39 p.m. UTC | #1
> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Shiri Kuzin
> Sent: Monday, October 19, 2020 9:37 AM
> To: dev@dpdk.org
> Cc: matan@mellanox.com; rasland@mellanox.com;
> viacheslavo@mellanox.com; stable@dpdk.org
> Subject: [dpdk-dev] [PATCH] net/mlx5: fix xstats reset reinitialization
> 
> 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")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Shiri Kuzin <shirik@nvidia.com>
> Acked-by: Matan Azrad <matan@nvidia.com>
Reported-by: Ralf Hoffmann <ralf.hoffmann@allegro-packets.com>
> ---
>  drivers/net/mlx5/mlx5_stats.c | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/mlx5/mlx5_stats.c b/drivers/net/mlx5/mlx5_stats.c
> index e30542e..82d4d4a 100644
> --- a/drivers/net/mlx5/mlx5_stats.c
> +++ b/drivers/net/mlx5/mlx5_stats.c
> @@ -17,6 +17,7 @@
>  #include "mlx5_defs.h"
>  #include "mlx5.h"
>  #include "mlx5_rxtx.h"
> +#include "mlx5_malloc.h"
> 
>  /**
>   * DPDK callback to get extended device statistics.
> @@ -216,8 +217,7 @@
>  	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_os_get_stats_n(dev);
> @@ -228,17 +228,29 @@
>  	}
>  	if (xstats_ctrl->stats_n != stats_n)
>  		mlx5_os_stats_init(dev);
> +	counters =  mlx5_malloc(MLX5_MEM_SYS, sizeof(*counters) *
> +			xstats_ctrl->mlx5_stats_n, 0,
> +			SOCKET_ID_ANY);
> +	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_os_read_dev_counters(dev, counters);
>  	if (ret) {
>  		DRV_LOG(ERR, "port %u cannot read device counters: %s",
>  			dev->data->port_id, strerror(rte_errno));
> +		mlx5_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;
>  	}
>  	mlx5_txpp_xstats_reset(dev);
> +	mlx5_free(counters);
>  	return 0;
>  }
> 
> --
> 1.8.3.1
  
Raslan Darawsheh Oct. 21, 2020, 11:03 a.m. UTC | #2
Hi,

> -----Original Message-----
> From: Shiri Kuzin <shirik@nvidia.com>
> Sent: Monday, October 19, 2020 9:37 AM
> To: dev@dpdk.org
> Cc: matan@mellanox.com; rasland@mellanox.com;
> viacheslavo@mellanox.com; stable@dpdk.org
> Subject: [PATCH] net/mlx5: fix xstats reset reinitialization
> 
> 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")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Shiri Kuzin <shirik@nvidia.com>
> Acked-by: Matan Azrad <matan@nvidia.com>
> ---
>  drivers/net/mlx5/mlx5_stats.c | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/mlx5/mlx5_stats.c b/drivers/net/mlx5/mlx5_stats.c
> index e30542e..82d4d4a 100644
> --- a/drivers/net/mlx5/mlx5_stats.c
> +++ b/drivers/net/mlx5/mlx5_stats.c
> @@ -17,6 +17,7 @@
>  #include "mlx5_defs.h"
>  #include "mlx5.h"
>  #include "mlx5_rxtx.h"
> +#include "mlx5_malloc.h"
> 
>  /**
>   * DPDK callback to get extended device statistics.
> @@ -216,8 +217,7 @@
>  	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_os_get_stats_n(dev);
> @@ -228,17 +228,29 @@
>  	}
>  	if (xstats_ctrl->stats_n != stats_n)
>  		mlx5_os_stats_init(dev);
> +	counters =  mlx5_malloc(MLX5_MEM_SYS, sizeof(*counters) *
> +			xstats_ctrl->mlx5_stats_n, 0,
> +			SOCKET_ID_ANY);
> +	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_os_read_dev_counters(dev, counters);
>  	if (ret) {
>  		DRV_LOG(ERR, "port %u cannot read device counters: %s",
>  			dev->data->port_id, strerror(rte_errno));
> +		mlx5_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;
>  	}
>  	mlx5_txpp_xstats_reset(dev);
> +	mlx5_free(counters);
>  	return 0;
>  }
> 
> --
> 1.8.3.1

Patch applied to next-net-mlx,

Kindest regards,
Raslan Darawsheh
  

Patch

diff --git a/drivers/net/mlx5/mlx5_stats.c b/drivers/net/mlx5/mlx5_stats.c
index e30542e..82d4d4a 100644
--- a/drivers/net/mlx5/mlx5_stats.c
+++ b/drivers/net/mlx5/mlx5_stats.c
@@ -17,6 +17,7 @@ 
 #include "mlx5_defs.h"
 #include "mlx5.h"
 #include "mlx5_rxtx.h"
+#include "mlx5_malloc.h"
 
 /**
  * DPDK callback to get extended device statistics.
@@ -216,8 +217,7 @@ 
 	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_os_get_stats_n(dev);
@@ -228,17 +228,29 @@ 
 	}
 	if (xstats_ctrl->stats_n != stats_n)
 		mlx5_os_stats_init(dev);
+	counters =  mlx5_malloc(MLX5_MEM_SYS, sizeof(*counters) *
+			xstats_ctrl->mlx5_stats_n, 0,
+			SOCKET_ID_ANY);
+	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_os_read_dev_counters(dev, counters);
 	if (ret) {
 		DRV_LOG(ERR, "port %u cannot read device counters: %s",
 			dev->data->port_id, strerror(rte_errno));
+		mlx5_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;
 	}
 	mlx5_txpp_xstats_reset(dev);
+	mlx5_free(counters);
 	return 0;
 }