DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] net/mlx5: fix header reformat action hash key
@ 2020-11-12 13:55 Suanming Mou
  2020-11-18  1:38 ` [dpdk-dev] [PATCH v2] " Suanming Mou
  2020-11-18  2:20 ` [dpdk-dev] [PATCH v3] " Suanming Mou
  0 siblings, 2 replies; 4+ messages in thread
From: Suanming Mou @ 2020-11-12 13:55 UTC (permalink / raw)
  To: viacheslavo, matan; +Cc: rasland, dev

Currently, header reformat action uses the hash list 32-bit key generated
in header reformat register function directly. The key will not be
recalculated in the hash list function.

As the 64-bit key is composed of the 32-bit attributes and 32-bit reformat
buffer csum, the hash list function only gets 32-bit key directly will take
the attribute part only, csum part will be ignored. For different header
reformat actions, the attributes can be the same, while the buffer will
be different. Only take the attribute part causes lots of the conflicts.

This commits removes the key attribute part and uses the significant
different csum part for the key.

Fixes: f961fd490fd4 ("net/mlx5: make header reformat action thread safe")

Signed-off-by: Suanming Mou <suanmingm@nvidia.com>
---
 drivers/net/mlx5/mlx5_flow.h    | 12 ------------
 drivers/net/mlx5/mlx5_flow_dv.c | 13 +------------
 2 files changed, 1 insertion(+), 24 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index e3a5030..c577c89 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -413,18 +413,6 @@ struct mlx5_flow_dv_matcher {
 
 #define MLX5_ENCAP_MAX_LEN 132
 
-/* Encap/decap resource key of the hash organization. */
-union mlx5_flow_encap_decap_key {
-	struct {
-		uint32_t ft_type:8;	/**< Flow table type, Rx or Tx. */
-		uint32_t refmt_type:8;	/**< Header reformat type. */
-		uint32_t buf_size:8;	/**< Encap buf size. */
-		uint32_t table_level:8;	/**< Root table or not. */
-		uint32_t cksum;		/**< Encap buf check sum. */
-	};
-	uint64_t v64;			/**< full 64bits value of key */
-};
-
 /* Encap/decap resource structure. */
 struct mlx5_flow_dv_encap_decap_resource {
 	struct mlx5_hlist_entry entry;
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 78c710f..bca6289 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -2865,24 +2865,13 @@ struct mlx5_hlist_entry *
 	struct mlx5_priv *priv = dev->data->dev_private;
 	struct mlx5_dev_ctx_shared *sh = priv->sh;
 	struct mlx5_hlist_entry *entry;
-	union mlx5_flow_encap_decap_key encap_decap_key = {
-		{
-			.ft_type = resource->ft_type,
-			.refmt_type = resource->reformat_type,
-			.buf_size = resource->size,
-			.table_level = !!dev_flow->dv.group,
-			.cksum = 0,
-		}
-	};
 	struct mlx5_flow_cb_ctx ctx = {
 		.error = error,
 		.data = resource,
 	};
 
 	resource->flags = dev_flow->dv.group ? 0 : 1;
-	encap_decap_key.cksum = __rte_raw_cksum(resource->buf,
-						resource->size, 0);
-	resource->entry.key = encap_decap_key.v64;
+	resource->entry.key = __rte_raw_cksum(resource->buf, resource->size, 0);
 	entry = mlx5_hlist_register(sh->encaps_decaps, resource->entry.key,
 				    &ctx);
 	if (!entry)
-- 
1.8.3.1


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

* [dpdk-dev] [PATCH v2] net/mlx5: fix header reformat action hash key
  2020-11-12 13:55 [dpdk-dev] [PATCH] net/mlx5: fix header reformat action hash key Suanming Mou
@ 2020-11-18  1:38 ` Suanming Mou
  2020-11-18  2:20 ` [dpdk-dev] [PATCH v3] " Suanming Mou
  1 sibling, 0 replies; 4+ messages in thread
From: Suanming Mou @ 2020-11-18  1:38 UTC (permalink / raw)
  To: viacheslavo, matan; +Cc: dev, rasland

Currently, header reformat action uses the hash list 32-bit key generated
in header reformat register function directly. The key will not be
recalculated in the hash list function.

As the 64-bit key is composed of the 32-bit attributes and 32-bit reformat
buffer csum, the hash list function only gets 32-bit key directly will take
the attribute part only, csum part will be ignored. For different header
reformat actions, the attributes can be the same, while the buffer will
be different. Only take the attribute part causes lots of the conflicts.

This commits adds the attribute part and the significant different csum
part for the key.

Fixes: f961fd490fd4 ("net/mlx5: make header reformat action thread safe")

Signed-off-by: Suanming Mou <suanmingm@nvidia.com>
---

v2:
 - update key generate.

---
 drivers/net/mlx5/mlx5_flow.h    | 12 ------------
 drivers/net/mlx5/mlx5_flow_dv.c | 31 ++++++++++++++++++++++++-------
 2 files changed, 24 insertions(+), 19 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index f643842..eac9b30 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -413,18 +413,6 @@ struct mlx5_flow_dv_matcher {
 
 #define MLX5_ENCAP_MAX_LEN 132
 
-/* Encap/decap resource key of the hash organization. */
-union mlx5_flow_encap_decap_key {
-	struct {
-		uint32_t ft_type:8;	/**< Flow table type, Rx or Tx. */
-		uint32_t refmt_type:8;	/**< Header reformat type. */
-		uint32_t buf_size:8;	/**< Encap buf size. */
-		uint32_t table_level:8;	/**< Root table or not. */
-		uint32_t cksum;		/**< Encap buf check sum. */
-	};
-	uint64_t v64;			/**< full 64bits value of key */
-};
-
 /* Encap/decap resource structure. */
 struct mlx5_flow_dv_encap_decap_resource {
 	struct mlx5_hlist_entry entry;
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 5e230a3..2b398d5 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -2865,13 +2865,25 @@ struct mlx5_hlist_entry *
 	struct mlx5_priv *priv = dev->data->dev_private;
 	struct mlx5_dev_ctx_shared *sh = priv->sh;
 	struct mlx5_hlist_entry *entry;
-	union mlx5_flow_encap_decap_key encap_decap_key = {
+	union {
+		struct {
+			uint32_t ft_type:8;
+			uint32_t refmt_type:8;
+			/*
+			 * Header reformat actions can be shared between
+			 * non-root tables. One bit to indicate non-root
+			 * table or not.
+			 */
+			uint32_t is_root:1;
+			uint32_t reserve:15;
+		};
+		uint32_t v32;
+	} encap_decap_key = {
 		{
 			.ft_type = resource->ft_type,
 			.refmt_type = resource->reformat_type,
-			.buf_size = resource->size,
-			.table_level = !!dev_flow->dv.group,
-			.cksum = 0,
+			.is_root = !!dev_flow->dv.group,
+			.reserve = 0,
 		}
 	};
 	struct mlx5_flow_cb_ctx ctx = {
@@ -2880,9 +2892,14 @@ struct mlx5_hlist_entry *
 	};
 
 	resource->flags = dev_flow->dv.group ? 0 : 1;
-	encap_decap_key.cksum = __rte_raw_cksum(resource->buf,
-						resource->size, 0);
-	resource->entry.key = encap_decap_key.v64;
+	resource->entry.key =  __rte_raw_cksum(&encap_decap_key.v32,
+					       sizeof(encap_decap_key.v32), 0);
+	if (resource->reformat_type !=
+	    MLX5DV_FLOW_ACTION_PACKET_REFORMAT_TYPE_L2_TUNNEL_TO_L2 &&
+	    resource->size)
+		resource->entry.key = __rte_raw_cksum(resource->buf,
+						      resource->size,
+						      resource->entry.key);
 	entry = mlx5_hlist_register(sh->encaps_decaps, resource->entry.key,
 				    &ctx);
 	if (!entry)
-- 
1.8.3.1


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

* [dpdk-dev] [PATCH v3] net/mlx5: fix header reformat action hash key
  2020-11-12 13:55 [dpdk-dev] [PATCH] net/mlx5: fix header reformat action hash key Suanming Mou
  2020-11-18  1:38 ` [dpdk-dev] [PATCH v2] " Suanming Mou
@ 2020-11-18  2:20 ` Suanming Mou
  2020-11-18 13:46   ` Raslan Darawsheh
  1 sibling, 1 reply; 4+ messages in thread
From: Suanming Mou @ 2020-11-18  2:20 UTC (permalink / raw)
  To: viacheslavo, matan; +Cc: dev, rasland

Currently, header reformat action uses the hash list 32-bit key generated
in header reformat register function directly. The key will not be
recalculated in the hash list function.

As the 64-bit key is composed of the 32-bit attributes and 32-bit reformat
buffer csum, the hash list function only gets 32-bit key directly will take
the attribute part only, csum part will be ignored. For different header
reformat actions, the attributes can be the same, while the buffer will
be different. Only take the attribute part causes lots of the conflicts.

This commits adds the attribute part and the significant different csum
part for the key.

Fixes: f961fd490fd4 ("net/mlx5: make header reformat action thread safe")

Signed-off-by: Suanming Mou <suanmingm@nvidia.com>
Acked-by: Matan Azrad <matan@nvidia.com>
---

v3:
 - add the missing ack.

v2:
 - update key generate.

---
 drivers/net/mlx5/mlx5_flow.h    | 12 ------------
 drivers/net/mlx5/mlx5_flow_dv.c | 31 ++++++++++++++++++++++++-------
 2 files changed, 24 insertions(+), 19 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index f643842..eac9b30 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -413,18 +413,6 @@ struct mlx5_flow_dv_matcher {
 
 #define MLX5_ENCAP_MAX_LEN 132
 
-/* Encap/decap resource key of the hash organization. */
-union mlx5_flow_encap_decap_key {
-	struct {
-		uint32_t ft_type:8;	/**< Flow table type, Rx or Tx. */
-		uint32_t refmt_type:8;	/**< Header reformat type. */
-		uint32_t buf_size:8;	/**< Encap buf size. */
-		uint32_t table_level:8;	/**< Root table or not. */
-		uint32_t cksum;		/**< Encap buf check sum. */
-	};
-	uint64_t v64;			/**< full 64bits value of key */
-};
-
 /* Encap/decap resource structure. */
 struct mlx5_flow_dv_encap_decap_resource {
 	struct mlx5_hlist_entry entry;
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 5e230a3..2b398d5 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -2865,13 +2865,25 @@ struct mlx5_hlist_entry *
 	struct mlx5_priv *priv = dev->data->dev_private;
 	struct mlx5_dev_ctx_shared *sh = priv->sh;
 	struct mlx5_hlist_entry *entry;
-	union mlx5_flow_encap_decap_key encap_decap_key = {
+	union {
+		struct {
+			uint32_t ft_type:8;
+			uint32_t refmt_type:8;
+			/*
+			 * Header reformat actions can be shared between
+			 * non-root tables. One bit to indicate non-root
+			 * table or not.
+			 */
+			uint32_t is_root:1;
+			uint32_t reserve:15;
+		};
+		uint32_t v32;
+	} encap_decap_key = {
 		{
 			.ft_type = resource->ft_type,
 			.refmt_type = resource->reformat_type,
-			.buf_size = resource->size,
-			.table_level = !!dev_flow->dv.group,
-			.cksum = 0,
+			.is_root = !!dev_flow->dv.group,
+			.reserve = 0,
 		}
 	};
 	struct mlx5_flow_cb_ctx ctx = {
@@ -2880,9 +2892,14 @@ struct mlx5_hlist_entry *
 	};
 
 	resource->flags = dev_flow->dv.group ? 0 : 1;
-	encap_decap_key.cksum = __rte_raw_cksum(resource->buf,
-						resource->size, 0);
-	resource->entry.key = encap_decap_key.v64;
+	resource->entry.key =  __rte_raw_cksum(&encap_decap_key.v32,
+					       sizeof(encap_decap_key.v32), 0);
+	if (resource->reformat_type !=
+	    MLX5DV_FLOW_ACTION_PACKET_REFORMAT_TYPE_L2_TUNNEL_TO_L2 &&
+	    resource->size)
+		resource->entry.key = __rte_raw_cksum(resource->buf,
+						      resource->size,
+						      resource->entry.key);
 	entry = mlx5_hlist_register(sh->encaps_decaps, resource->entry.key,
 				    &ctx);
 	if (!entry)
-- 
1.8.3.1


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

* Re: [dpdk-dev] [PATCH v3] net/mlx5: fix header reformat action hash key
  2020-11-18  2:20 ` [dpdk-dev] [PATCH v3] " Suanming Mou
@ 2020-11-18 13:46   ` Raslan Darawsheh
  0 siblings, 0 replies; 4+ messages in thread
From: Raslan Darawsheh @ 2020-11-18 13:46 UTC (permalink / raw)
  To: Suanming Mou, Slava Ovsiienko, Matan Azrad; +Cc: dev

Hi,

> -----Original Message-----
> From: Suanming Mou <suanmingm@nvidia.com>
> Sent: Wednesday, November 18, 2020 4:20 AM
> To: Slava Ovsiienko <viacheslavo@nvidia.com>; Matan Azrad
> <matan@nvidia.com>
> Cc: dev@dpdk.org; Raslan Darawsheh <rasland@nvidia.com>
> Subject: [PATCH v3] net/mlx5: fix header reformat action hash key
> 
> Currently, header reformat action uses the hash list 32-bit key generated
> in header reformat register function directly. The key will not be
> recalculated in the hash list function.
> 
> As the 64-bit key is composed of the 32-bit attributes and 32-bit reformat
> buffer csum, the hash list function only gets 32-bit key directly will take
> the attribute part only, csum part will be ignored. For different header
> reformat actions, the attributes can be the same, while the buffer will
> be different. Only take the attribute part causes lots of the conflicts.
> 
> This commits adds the attribute part and the significant different csum
> part for the key.
> 
> Fixes: f961fd490fd4 ("net/mlx5: make header reformat action thread safe")
> 
> Signed-off-by: Suanming Mou <suanmingm@nvidia.com>
> Acked-by: Matan Azrad <matan@nvidia.com>
> ---
> 
> v3:
>  - add the missing ack.
> 
> v2:
>  - update key generate.
> 
> ---
>  drivers/net/mlx5/mlx5_flow.h    | 12 ------------
>  drivers/net/mlx5/mlx5_flow_dv.c | 31 ++++++++++++++++++++++++-------
>  2 files changed, 24 insertions(+), 19 deletions(-)
> 

Patch applied to next-net-mlx,

Kindest regards,
Raslan Darawsheh

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

end of thread, other threads:[~2020-11-18 13:46 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-12 13:55 [dpdk-dev] [PATCH] net/mlx5: fix header reformat action hash key Suanming Mou
2020-11-18  1:38 ` [dpdk-dev] [PATCH v2] " Suanming Mou
2020-11-18  2:20 ` [dpdk-dev] [PATCH v3] " Suanming Mou
2020-11-18 13:46   ` Raslan Darawsheh

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/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 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

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


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