patches for DPDK stable branches
 help / color / mirror / Atom feed
* [dpdk-stable] [PATCH] [18.11] net/mlx5: fix xstats reset reinitialization
@ 2020-12-08  7:54 Shiri Kuzin
  2020-12-08 10:37 ` Kevin Traynor
  0 siblings, 1 reply; 3+ messages in thread
From: Shiri Kuzin @ 2020-12-08  7:54 UTC (permalink / raw)
  To: stable; +Cc: ktraynor, viacheslavo, Ralf Hoffmann, Matan Azrad

[ 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>
---
 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;
 }
 
 /**
-- 
2.21.0


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [dpdk-stable] [PATCH] [18.11] net/mlx5: fix xstats reset reinitialization
  2020-12-08  7:54 [dpdk-stable] [PATCH] [18.11] net/mlx5: fix xstats reset reinitialization Shiri Kuzin
@ 2020-12-08 10:37 ` Kevin Traynor
  0 siblings, 0 replies; 3+ messages in thread
From: Kevin Traynor @ 2020-12-08 10:37 UTC (permalink / raw)
  To: Shiri Kuzin, stable; +Cc: viacheslavo, Ralf Hoffmann, Matan Azrad

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;
>  }
>  
>  /**
> 


^ permalink raw reply	[flat|nested] 3+ messages in thread

* [dpdk-stable] [PATCH] [18.11] net/mlx5: fix xstats reset reinitialization
@ 2020-12-08 12:28 Viacheslav Ovsiienko
  0 siblings, 0 replies; 3+ messages in thread
From: Viacheslav Ovsiienko @ 2020-12-08 12:28 UTC (permalink / raw)
  To: stable; +Cc: ktraynor, shirik

From: Shiri Kuzin <shirik@nvidia.com>

From: Shiri Kuzin <shirik@nvidia.com>

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

Reported-by: Ralf Hoffmann <ralf.hoffmann@allegro-packets.com>
Signed-off-by: Shiri Kuzin <shirik@nvidia.com>
Acked-by: Matan Azrad <matan@nvidia.com>
---
 drivers/net/mlx5/mlx5_stats.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_stats.c b/drivers/net/mlx5/mlx5_stats.c
index 0b1fe3d..5e3a0d2 100644
--- a/drivers/net/mlx5/mlx5_stats.c
+++ b/drivers/net/mlx5/mlx5_stats.c
@@ -480,8 +480,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_ethtool_get_stats_n(dev);
@@ -492,14 +491,26 @@
 	}
 	if (xstats_ctrl->stats_n != stats_n)
 		mlx5_stats_init(dev);
+	counters =  rte_malloc("xstats_counters",
+			       sizeof(*counters) * xstats_ctrl->mlx5_stats_n,
+			       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;
+	}
 	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));
+		rte_free(counters);
 		return;
 	}
-	for (i = 0; i != n; ++i)
+	for (i = 0; i != xstats_ctrl->mlx5_stats_n; ++i)
 		xstats_ctrl->base[i] = counters[i];
+	rte_free(counters);
 }
 
 /**
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2020-12-08 12:28 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-08  7:54 [dpdk-stable] [PATCH] [18.11] net/mlx5: fix xstats reset reinitialization Shiri Kuzin
2020-12-08 10:37 ` Kevin Traynor
2020-12-08 12:28 Viacheslav Ovsiienko

patches for DPDK stable branches

This inbox may be cloned and mirrored by anyone:

	git clone --mirror http://inbox.dpdk.org/stable/0 stable/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 stable stable/ http://inbox.dpdk.org/stable \
		stable@dpdk.org
	public-inbox-index stable

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.stable


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git