DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] net/mlx5: fix port shared data reference count
@ 2020-10-21  9:17 Xueming Li
  2020-10-21 11:15 ` [dpdk-dev] [PATCH v1] " Xueming Li
  0 siblings, 1 reply; 3+ messages in thread
From: Xueming Li @ 2020-10-21  9:17 UTC (permalink / raw)
  To: Matan Azrad, Viacheslav Ovsiienko
  Cc: dev, xuemingl, Asaf Penso, jackmin, stable

When probe a representor, tag cache hash table and modification cache
hash table allocated memory upon each port, overwrote previous existing
cache in shared context data.

This patch moves reference check of shared data prior to hash table
allocation to avoid such issue.

Fixes: 6801116688fe ("net/mlx5: fix multiple flow table hash list")
Cc: jackmin@nvidia.com
Fixes: 1ef4cdef2682 ("net/mlx5: fix flow tag hash list conversion")
Cc: matan@nvidia.com

Cc: stable@dpdk.org
Acked-by: Matan Azrad matan@nvidia.com
Signed-off-by: Xueming Li <xuemingl@nvidia.com>
---
 drivers/net/mlx5/linux/mlx5_os.c | 30 ++++++++----------------------
 drivers/net/mlx5/mlx5.h          |  2 --
 drivers/net/mlx5/mlx5_flow_dv.c  |  4 ++--
 3 files changed, 10 insertions(+), 26 deletions(-)

diff --git a/drivers/net/mlx5/linux/mlx5_os.c b/drivers/net/mlx5/linux/mlx5_os.c
index 457008ef9e..40f9446d43 100644
--- a/drivers/net/mlx5/linux/mlx5_os.c
+++ b/drivers/net/mlx5/linux/mlx5_os.c
@@ -226,13 +226,12 @@ mlx5_alloc_shared_dr(struct mlx5_priv *priv)
 {
 	struct mlx5_dev_ctx_shared *sh = priv->sh;
 	char s[MLX5_HLIST_NAMESIZE];
-	int err = 0;
+	int err;
 
-	if (!sh->flow_tbls)
-		err = mlx5_alloc_table_hash_list(priv);
-	else
-		DRV_LOG(DEBUG, "sh->flow_tbls[%p] already created, reuse\n",
-			(void *)sh->flow_tbls);
+	MLX5_ASSERT(sh && sh->refcnt);
+	if (sh->refcnt > 1)
+		return 0;
+	err = mlx5_alloc_table_hash_list(priv);
 	if (err)
 		return err;
 	/* Create tags hash list table. */
@@ -261,12 +260,6 @@ mlx5_alloc_shared_dr(struct mlx5_priv *priv)
 #ifdef HAVE_MLX5DV_DR
 	void *domain;
 
-	if (sh->dv_refcnt) {
-		/* Shared DV/DR structures is already initialized. */
-		sh->dv_refcnt++;
-		priv->dr_shared = 1;
-		return 0;
-	}
 	/* Reference counter is zero, we should initialize structures. */
 	domain = mlx5_glue->dr_create_domain(sh->ctx,
 					     MLX5DV_DR_DOMAIN_TYPE_NIC_RX);
@@ -306,8 +299,6 @@ mlx5_alloc_shared_dr(struct mlx5_priv *priv)
 	}
 	sh->pop_vlan_action = mlx5_glue->dr_create_flow_action_pop_vlan();
 #endif /* HAVE_MLX5DV_DR */
-	sh->dv_refcnt++;
-	priv->dr_shared = 1;
 	return 0;
 error:
 	/* Rollback the created objects. */
@@ -357,17 +348,12 @@ mlx5_alloc_shared_dr(struct mlx5_priv *priv)
 void
 mlx5_os_free_shared_dr(struct mlx5_priv *priv)
 {
-	struct mlx5_dev_ctx_shared *sh;
+	struct mlx5_dev_ctx_shared *sh = priv->sh;
 
-	if (!priv->dr_shared)
+	MLX5_ASSERT(sh && sh->refcnt);
+	if (sh->refcnt > 1)
 		return;
-	priv->dr_shared = 0;
-	sh = priv->sh;
-	MLX5_ASSERT(sh);
 #ifdef HAVE_MLX5DV_DR
-	MLX5_ASSERT(sh->dv_refcnt);
-	if (sh->dv_refcnt && --sh->dv_refcnt)
-		return;
 	if (sh->rx_domain) {
 		mlx5_glue->dr_destroy_domain(sh->rx_domain);
 		sh->rx_domain = NULL;
diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index fa69c66137..c9d5d71630 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -635,7 +635,6 @@ struct mlx5_dev_ctx_shared {
 	uint32_t dv_meta_mask; /* flow META metadata supported mask. */
 	uint32_t dv_mark_mask; /* flow MARK metadata supported mask. */
 	uint32_t dv_regc0_mask; /* available bits of metatada reg_c[0]. */
-	uint32_t dv_refcnt; /* DV/DR data reference counter. */
 	void *fdb_domain; /* FDB Direct Rules name space handle. */
 	void *rx_domain; /* RX Direct Rules name space handle. */
 	void *tx_domain; /* TX Direct Rules name space handle. */
@@ -823,7 +822,6 @@ struct mlx5_priv {
 	unsigned int isolated:1; /* Whether isolated mode is enabled. */
 	unsigned int representor:1; /* Device is a port representor. */
 	unsigned int master:1; /* Device is a E-Switch master. */
-	unsigned int dr_shared:1; /* DV/DR data is shared. */
 	unsigned int txpp_en:1; /* Tx packet pacing enabled. */
 	unsigned int mtr_en:1; /* Whether support meter. */
 	unsigned int mtr_reg_share:1; /* Whether support meter REG_C share. */
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 49d96362f8..15cd34e331 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -293,7 +293,7 @@ flow_dv_shared_lock(struct rte_eth_dev *dev)
 	struct mlx5_priv *priv = dev->data->dev_private;
 	struct mlx5_dev_ctx_shared *sh = priv->sh;
 
-	if (sh->dv_refcnt > 1) {
+	if (sh->refcnt > 1) {
 		int ret;
 
 		ret = pthread_mutex_lock(&sh->dv_mutex);
@@ -308,7 +308,7 @@ flow_dv_shared_unlock(struct rte_eth_dev *dev)
 	struct mlx5_priv *priv = dev->data->dev_private;
 	struct mlx5_dev_ctx_shared *sh = priv->sh;
 
-	if (sh->dv_refcnt > 1) {
+	if (sh->refcnt > 1) {
 		int ret;
 
 		ret = pthread_mutex_unlock(&sh->dv_mutex);
-- 
2.25.1


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

* [dpdk-dev] [PATCH v1] net/mlx5: fix port shared data reference count
  2020-10-21  9:17 [dpdk-dev] [PATCH] net/mlx5: fix port shared data reference count Xueming Li
@ 2020-10-21 11:15 ` Xueming Li
  2020-10-21 20:33   ` Raslan Darawsheh
  0 siblings, 1 reply; 3+ messages in thread
From: Xueming Li @ 2020-10-21 11:15 UTC (permalink / raw)
  To: Matan Azrad, Viacheslav Ovsiienko
  Cc: dev, xuemingl, Asaf Penso, jackmin, stable

When probe a representor, tag cache hash table and modification cache
hash table allocated memory upon each port, overwrote previous existing
cache in shared context data.

This patch moves reference check of shared data prior to hash table
allocation to avoid such issue.

Fixes: 6801116688fe ("net/mlx5: fix multiple flow table hash list")
Cc: jackmin@nvidia.com
Fixes: 1ef4cdef2682 ("net/mlx5: fix flow tag hash list conversion")
Cc: matan@nvidia.com

Cc: stable@dpdk.org
Acked-by: Matan Azrad <matan@nvidia.com>
Signed-off-by: Xueming Li <xuemingl@nvidia.com>
---
 drivers/net/mlx5/linux/mlx5_os.c | 30 ++++++++----------------------
 drivers/net/mlx5/mlx5.h          |  2 --
 drivers/net/mlx5/mlx5_flow_dv.c  |  4 ++--
 3 files changed, 10 insertions(+), 26 deletions(-)

diff --git a/drivers/net/mlx5/linux/mlx5_os.c b/drivers/net/mlx5/linux/mlx5_os.c
index 457008ef9e..40f9446d43 100644
--- a/drivers/net/mlx5/linux/mlx5_os.c
+++ b/drivers/net/mlx5/linux/mlx5_os.c
@@ -226,13 +226,12 @@ mlx5_alloc_shared_dr(struct mlx5_priv *priv)
 {
 	struct mlx5_dev_ctx_shared *sh = priv->sh;
 	char s[MLX5_HLIST_NAMESIZE];
-	int err = 0;
+	int err;
 
-	if (!sh->flow_tbls)
-		err = mlx5_alloc_table_hash_list(priv);
-	else
-		DRV_LOG(DEBUG, "sh->flow_tbls[%p] already created, reuse\n",
-			(void *)sh->flow_tbls);
+	MLX5_ASSERT(sh && sh->refcnt);
+	if (sh->refcnt > 1)
+		return 0;
+	err = mlx5_alloc_table_hash_list(priv);
 	if (err)
 		return err;
 	/* Create tags hash list table. */
@@ -261,12 +260,6 @@ mlx5_alloc_shared_dr(struct mlx5_priv *priv)
 #ifdef HAVE_MLX5DV_DR
 	void *domain;
 
-	if (sh->dv_refcnt) {
-		/* Shared DV/DR structures is already initialized. */
-		sh->dv_refcnt++;
-		priv->dr_shared = 1;
-		return 0;
-	}
 	/* Reference counter is zero, we should initialize structures. */
 	domain = mlx5_glue->dr_create_domain(sh->ctx,
 					     MLX5DV_DR_DOMAIN_TYPE_NIC_RX);
@@ -306,8 +299,6 @@ mlx5_alloc_shared_dr(struct mlx5_priv *priv)
 	}
 	sh->pop_vlan_action = mlx5_glue->dr_create_flow_action_pop_vlan();
 #endif /* HAVE_MLX5DV_DR */
-	sh->dv_refcnt++;
-	priv->dr_shared = 1;
 	return 0;
 error:
 	/* Rollback the created objects. */
@@ -357,17 +348,12 @@ mlx5_alloc_shared_dr(struct mlx5_priv *priv)
 void
 mlx5_os_free_shared_dr(struct mlx5_priv *priv)
 {
-	struct mlx5_dev_ctx_shared *sh;
+	struct mlx5_dev_ctx_shared *sh = priv->sh;
 
-	if (!priv->dr_shared)
+	MLX5_ASSERT(sh && sh->refcnt);
+	if (sh->refcnt > 1)
 		return;
-	priv->dr_shared = 0;
-	sh = priv->sh;
-	MLX5_ASSERT(sh);
 #ifdef HAVE_MLX5DV_DR
-	MLX5_ASSERT(sh->dv_refcnt);
-	if (sh->dv_refcnt && --sh->dv_refcnt)
-		return;
 	if (sh->rx_domain) {
 		mlx5_glue->dr_destroy_domain(sh->rx_domain);
 		sh->rx_domain = NULL;
diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index fa69c66137..c9d5d71630 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -635,7 +635,6 @@ struct mlx5_dev_ctx_shared {
 	uint32_t dv_meta_mask; /* flow META metadata supported mask. */
 	uint32_t dv_mark_mask; /* flow MARK metadata supported mask. */
 	uint32_t dv_regc0_mask; /* available bits of metatada reg_c[0]. */
-	uint32_t dv_refcnt; /* DV/DR data reference counter. */
 	void *fdb_domain; /* FDB Direct Rules name space handle. */
 	void *rx_domain; /* RX Direct Rules name space handle. */
 	void *tx_domain; /* TX Direct Rules name space handle. */
@@ -823,7 +822,6 @@ struct mlx5_priv {
 	unsigned int isolated:1; /* Whether isolated mode is enabled. */
 	unsigned int representor:1; /* Device is a port representor. */
 	unsigned int master:1; /* Device is a E-Switch master. */
-	unsigned int dr_shared:1; /* DV/DR data is shared. */
 	unsigned int txpp_en:1; /* Tx packet pacing enabled. */
 	unsigned int mtr_en:1; /* Whether support meter. */
 	unsigned int mtr_reg_share:1; /* Whether support meter REG_C share. */
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 49d96362f8..15cd34e331 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -293,7 +293,7 @@ flow_dv_shared_lock(struct rte_eth_dev *dev)
 	struct mlx5_priv *priv = dev->data->dev_private;
 	struct mlx5_dev_ctx_shared *sh = priv->sh;
 
-	if (sh->dv_refcnt > 1) {
+	if (sh->refcnt > 1) {
 		int ret;
 
 		ret = pthread_mutex_lock(&sh->dv_mutex);
@@ -308,7 +308,7 @@ flow_dv_shared_unlock(struct rte_eth_dev *dev)
 	struct mlx5_priv *priv = dev->data->dev_private;
 	struct mlx5_dev_ctx_shared *sh = priv->sh;
 
-	if (sh->dv_refcnt > 1) {
+	if (sh->refcnt > 1) {
 		int ret;
 
 		ret = pthread_mutex_unlock(&sh->dv_mutex);
-- 
2.25.1


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

* Re: [dpdk-dev] [PATCH v1] net/mlx5: fix port shared data reference count
  2020-10-21 11:15 ` [dpdk-dev] [PATCH v1] " Xueming Li
@ 2020-10-21 20:33   ` Raslan Darawsheh
  0 siblings, 0 replies; 3+ messages in thread
From: Raslan Darawsheh @ 2020-10-21 20:33 UTC (permalink / raw)
  To: Xueming(Steven) Li, Matan Azrad, Slava Ovsiienko
  Cc: dev, Xueming(Steven) Li, Asaf Penso, Jack Min, stable

Hi,

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Xueming Li
> Sent: Wednesday, October 21, 2020 2:15 PM
> To: Matan Azrad <matan@nvidia.com>; Slava Ovsiienko
> <viacheslavo@nvidia.com>
> Cc: dev@dpdk.org; Xueming(Steven) Li <xuemingl@nvidia.com>; Asaf Penso
> <asafp@nvidia.com>; Jack Min <jackmin@nvidia.com>; stable@dpdk.org
> Subject: [dpdk-dev] [PATCH v1] net/mlx5: fix port shared data reference
> count
> 
> When probe a representor, tag cache hash table and modification cache
> hash table allocated memory upon each port, overwrote previous existing
> cache in shared context data.
> 
> This patch moves reference check of shared data prior to hash table
> allocation to avoid such issue.
> 
> Fixes: 6801116688fe ("net/mlx5: fix multiple flow table hash list")
> Cc: jackmin@nvidia.com
> Fixes: 1ef4cdef2682 ("net/mlx5: fix flow tag hash list conversion")
> Cc: matan@nvidia.com
> 
> Cc: stable@dpdk.org
> Acked-by: Matan Azrad <matan@nvidia.com>
> Signed-off-by: Xueming Li <xuemingl@nvidia.com>
> ---
>  drivers/net/mlx5/linux/mlx5_os.c | 30 ++++++++----------------------
>  drivers/net/mlx5/mlx5.h          |  2 --
>  drivers/net/mlx5/mlx5_flow_dv.c  |  4 ++--
>  3 files changed, 10 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/net/mlx5/linux/mlx5_os.c
> b/drivers/net/mlx5/linux/mlx5_os.c
> index 457008ef9e..40f9446d43 100644
> --- a/drivers/net/mlx5/linux/mlx5_os.c
> +++ b/drivers/net/mlx5/linux/mlx5_os.c
> @@ -226,13 +226,12 @@ mlx5_alloc_shared_dr(struct mlx5_priv *priv)
>  {
>  	struct mlx5_dev_ctx_shared *sh = priv->sh;
>  	char s[MLX5_HLIST_NAMESIZE];
> -	int err = 0;
> +	int err;
> 
> -	if (!sh->flow_tbls)
> -		err = mlx5_alloc_table_hash_list(priv);
> -	else
> -		DRV_LOG(DEBUG, "sh->flow_tbls[%p] already created,
> reuse\n",
> -			(void *)sh->flow_tbls);
> +	MLX5_ASSERT(sh && sh->refcnt);
> +	if (sh->refcnt > 1)
> +		return 0;
> +	err = mlx5_alloc_table_hash_list(priv);
>  	if (err)
>  		return err;
>  	/* Create tags hash list table. */
> @@ -261,12 +260,6 @@ mlx5_alloc_shared_dr(struct mlx5_priv *priv)
>  #ifdef HAVE_MLX5DV_DR
>  	void *domain;
> 
> -	if (sh->dv_refcnt) {
> -		/* Shared DV/DR structures is already initialized. */
> -		sh->dv_refcnt++;
> -		priv->dr_shared = 1;
> -		return 0;
> -	}
>  	/* Reference counter is zero, we should initialize structures. */
>  	domain = mlx5_glue->dr_create_domain(sh->ctx,
> 
> MLX5DV_DR_DOMAIN_TYPE_NIC_RX);
> @@ -306,8 +299,6 @@ mlx5_alloc_shared_dr(struct mlx5_priv *priv)
>  	}
>  	sh->pop_vlan_action = mlx5_glue-
> >dr_create_flow_action_pop_vlan();
>  #endif /* HAVE_MLX5DV_DR */
> -	sh->dv_refcnt++;
> -	priv->dr_shared = 1;
>  	return 0;
>  error:
>  	/* Rollback the created objects. */
> @@ -357,17 +348,12 @@ mlx5_alloc_shared_dr(struct mlx5_priv *priv)
>  void
>  mlx5_os_free_shared_dr(struct mlx5_priv *priv)
>  {
> -	struct mlx5_dev_ctx_shared *sh;
> +	struct mlx5_dev_ctx_shared *sh = priv->sh;
> 
> -	if (!priv->dr_shared)
> +	MLX5_ASSERT(sh && sh->refcnt);
> +	if (sh->refcnt > 1)
>  		return;
> -	priv->dr_shared = 0;
> -	sh = priv->sh;
> -	MLX5_ASSERT(sh);
>  #ifdef HAVE_MLX5DV_DR
> -	MLX5_ASSERT(sh->dv_refcnt);
> -	if (sh->dv_refcnt && --sh->dv_refcnt)
> -		return;
>  	if (sh->rx_domain) {
>  		mlx5_glue->dr_destroy_domain(sh->rx_domain);
>  		sh->rx_domain = NULL;
> diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
> index fa69c66137..c9d5d71630 100644
> --- a/drivers/net/mlx5/mlx5.h
> +++ b/drivers/net/mlx5/mlx5.h
> @@ -635,7 +635,6 @@ struct mlx5_dev_ctx_shared {
>  	uint32_t dv_meta_mask; /* flow META metadata supported mask.
> */
>  	uint32_t dv_mark_mask; /* flow MARK metadata supported mask.
> */
>  	uint32_t dv_regc0_mask; /* available bits of metatada reg_c[0]. */
> -	uint32_t dv_refcnt; /* DV/DR data reference counter. */
>  	void *fdb_domain; /* FDB Direct Rules name space handle. */
>  	void *rx_domain; /* RX Direct Rules name space handle. */
>  	void *tx_domain; /* TX Direct Rules name space handle. */
> @@ -823,7 +822,6 @@ struct mlx5_priv {
>  	unsigned int isolated:1; /* Whether isolated mode is enabled. */
>  	unsigned int representor:1; /* Device is a port representor. */
>  	unsigned int master:1; /* Device is a E-Switch master. */
> -	unsigned int dr_shared:1; /* DV/DR data is shared. */
>  	unsigned int txpp_en:1; /* Tx packet pacing enabled. */
>  	unsigned int mtr_en:1; /* Whether support meter. */
>  	unsigned int mtr_reg_share:1; /* Whether support meter REG_C
> share. */
> diff --git a/drivers/net/mlx5/mlx5_flow_dv.c
> b/drivers/net/mlx5/mlx5_flow_dv.c
> index 49d96362f8..15cd34e331 100644
> --- a/drivers/net/mlx5/mlx5_flow_dv.c
> +++ b/drivers/net/mlx5/mlx5_flow_dv.c
> @@ -293,7 +293,7 @@ flow_dv_shared_lock(struct rte_eth_dev *dev)
>  	struct mlx5_priv *priv = dev->data->dev_private;
>  	struct mlx5_dev_ctx_shared *sh = priv->sh;
> 
> -	if (sh->dv_refcnt > 1) {
> +	if (sh->refcnt > 1) {
>  		int ret;
> 
>  		ret = pthread_mutex_lock(&sh->dv_mutex);
> @@ -308,7 +308,7 @@ flow_dv_shared_unlock(struct rte_eth_dev *dev)
>  	struct mlx5_priv *priv = dev->data->dev_private;
>  	struct mlx5_dev_ctx_shared *sh = priv->sh;
> 
> -	if (sh->dv_refcnt > 1) {
> +	if (sh->refcnt > 1) {
>  		int ret;
> 
>  		ret = pthread_mutex_unlock(&sh->dv_mutex);
> --
> 2.25.1

Patch applied to next-net-mlx,

Kindest regards,
Raslan Darawsheh 

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

end of thread, other threads:[~2020-10-21 20:33 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-21  9:17 [dpdk-dev] [PATCH] net/mlx5: fix port shared data reference count Xueming Li
2020-10-21 11:15 ` [dpdk-dev] [PATCH v1] " Xueming Li
2020-10-21 20:33   ` Raslan Darawsheh

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).