DPDK patches and discussions
 help / color / Atom feed
* [dpdk-dev] [PATCH 0/2] net/mlx5: manage modify actions with hashed list
@ 2020-07-31  3:34 Suanming Mou
  2020-07-31  3:34 ` [dpdk-dev] [PATCH 1/2] net/mlx5: add hash list extended lookup and insert Suanming Mou
  2020-07-31  3:34 ` [dpdk-dev] [PATCH 2/2] net/mlx5: manage modify actions with hashed list Suanming Mou
  0 siblings, 2 replies; 3+ messages in thread
From: Suanming Mou @ 2020-07-31  3:34 UTC (permalink / raw)
  To: viacheslavo, matan; +Cc: rasland, dev

To manage header modify actions mlx5 PMD used the single linked list and
lookup and insertion operations took too long times if there were millions
of objects and this impacted the flow insertion/deletion rate.

In order to optimize the performance the hashed list is engaged. The list
implementation is updated to support non-unique keys with few collisions.

Suanming Mou (2):
  net/mlx5: add hash list extended lookup and insert
  net/mlx5: manage modify actions with hashed list

 drivers/net/mlx5/linux/mlx5_os.c | 15 +++++++
 drivers/net/mlx5/mlx5.h          |  2 +-
 drivers/net/mlx5/mlx5_defs.h     |  3 ++
 drivers/net/mlx5/mlx5_flow.h     | 13 +++++-
 drivers/net/mlx5/mlx5_flow_dv.c  | 95 ++++++++++++++++++++++++++++++++--------
 drivers/net/mlx5/mlx5_utils.c    | 38 ++++++++++++++++
 drivers/net/mlx5/mlx5_utils.h    | 57 ++++++++++++++++++++++++
 7 files changed, 203 insertions(+), 20 deletions(-)

-- 
1.8.3.1


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

* [dpdk-dev] [PATCH 1/2] net/mlx5: add hash list extended lookup and insert
  2020-07-31  3:34 [dpdk-dev] [PATCH 0/2] net/mlx5: manage modify actions with hashed list Suanming Mou
@ 2020-07-31  3:34 ` Suanming Mou
  2020-07-31  3:34 ` [dpdk-dev] [PATCH 2/2] net/mlx5: manage modify actions with hashed list Suanming Mou
  1 sibling, 0 replies; 3+ messages in thread
From: Suanming Mou @ 2020-07-31  3:34 UTC (permalink / raw)
  To: viacheslavo, matan; +Cc: rasland, dev

The mlx5 PMD hashed list was designed in approach to contain the items
with unique keys only. Now there is the need to store the objects with
possible key collisions. It is not expected to have many collisions
(very likely to have a few ones), but keys become not unique.

This commit adds the hash list extended functions in order to support
insertion and lookup for the lists with non-unique keys.

Signed-off-by: Suanming Mou <suanmingm@mellanox.com>
Acked-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
---
 drivers/net/mlx5/mlx5_utils.c | 38 +++++++++++++++++++++++++++++
 drivers/net/mlx5/mlx5_utils.h | 57 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 95 insertions(+)

diff --git a/drivers/net/mlx5/mlx5_utils.c b/drivers/net/mlx5/mlx5_utils.c
index 25e8b27..fefe833 100644
--- a/drivers/net/mlx5/mlx5_utils.c
+++ b/drivers/net/mlx5/mlx5_utils.c
@@ -81,6 +81,44 @@ struct mlx5_hlist_entry *
 	return 0;
 }
 
+struct mlx5_hlist_entry *
+mlx5_hlist_lookup_ex(struct mlx5_hlist *h, uint64_t key,
+		     mlx5_hlist_match_callback_fn cb, void *ctx)
+{
+	uint32_t idx;
+	struct mlx5_hlist_head *first;
+	struct mlx5_hlist_entry *node;
+
+	MLX5_ASSERT(h && cb && ctx);
+	idx = rte_hash_crc_8byte(key, 0) & h->mask;
+	first = &h->heads[idx];
+	LIST_FOREACH(node, first, next) {
+		if (!cb(node, ctx))
+			return node;
+	}
+	return NULL;
+}
+
+int
+mlx5_hlist_insert_ex(struct mlx5_hlist *h, struct mlx5_hlist_entry *entry,
+		     mlx5_hlist_match_callback_fn cb, void *ctx)
+{
+	uint32_t idx;
+	struct mlx5_hlist_head *first;
+	struct mlx5_hlist_entry *node;
+
+	MLX5_ASSERT(h && entry && cb && ctx);
+	idx = rte_hash_crc_8byte(entry->key, 0) & h->mask;
+	first = &h->heads[idx];
+	/* No need to reuse the lookup function. */
+	LIST_FOREACH(node, first, next) {
+		if (!cb(node, ctx))
+			return -EEXIST;
+	}
+	LIST_INSERT_HEAD(first, entry, next);
+	return 0;
+}
+
 void
 mlx5_hlist_remove(struct mlx5_hlist *h __rte_unused,
 		  struct mlx5_hlist_entry *entry)
diff --git a/drivers/net/mlx5/mlx5_utils.h b/drivers/net/mlx5/mlx5_utils.h
index 562b9b1..97d931f 100644
--- a/drivers/net/mlx5/mlx5_utils.h
+++ b/drivers/net/mlx5/mlx5_utils.h
@@ -265,6 +265,20 @@ struct mlx5_hlist_entry {
 /** Type of function that is used to handle the data before freeing. */
 typedef void (*mlx5_hlist_destroy_callback_fn)(void *p, void *ctx);
 
+/**
+ * Type of function for user defined matching.
+ *
+ * @param entry
+ *   The entry in the list.
+ * @param ctx
+ *   The pointer to new entry context.
+ *
+ * @return
+ *   0 if matching, -1 otherwise.
+ */
+typedef int (*mlx5_hlist_match_callback_fn)(struct mlx5_hlist_entry *entry,
+					     void *ctx);
+
 /** hash list table structure */
 struct mlx5_hlist {
 	char name[MLX5_HLIST_NAMESIZE]; /**< Name of the hash list. */
@@ -323,6 +337,49 @@ struct mlx5_hlist {
 int mlx5_hlist_insert(struct mlx5_hlist *h, struct mlx5_hlist_entry *entry);
 
 /**
+ * Extended routine to search an entry matching the context with
+ * user defined match function.
+ *
+ * @param h
+ *   Pointer to the hast list table.
+ * @param key
+ *   Key for the searching entry.
+ * @param cb
+ *   Callback function to match the node with context.
+ * @param ctx
+ *   Common context parameter used by callback function.
+ *
+ * @return
+ *   Pointer of the hlist entry if found, NULL otherwise.
+ */
+struct mlx5_hlist_entry *mlx5_hlist_lookup_ex(struct mlx5_hlist *h,
+					      uint64_t key,
+					      mlx5_hlist_match_callback_fn cb,
+					      void *ctx);
+
+/**
+ * Extended routine to insert an entry to the list with key collisions.
+ *
+ * For the list have key collision, the extra user defined match function
+ * allows node with same key will be inserted.
+ *
+ * @param h
+ *   Pointer to the hast list table.
+ * @param entry
+ *   Entry to be inserted into the hash list table.
+ * @param cb
+ *   Callback function to match the node with context.
+ * @param ctx
+ *   Common context parameter used by callback function.
+ *
+ * @return
+ *   - zero for success.
+ *   - -EEXIST if the entry is already inserted.
+ */
+int mlx5_hlist_insert_ex(struct mlx5_hlist *h, struct mlx5_hlist_entry *entry,
+			 mlx5_hlist_match_callback_fn cb, void *ctx);
+
+/**
  * Remove an entry from the hash list table. User should guarantee the validity
  * of the entry.
  *
-- 
1.8.3.1


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

* [dpdk-dev] [PATCH 2/2] net/mlx5: manage modify actions with hashed list
  2020-07-31  3:34 [dpdk-dev] [PATCH 0/2] net/mlx5: manage modify actions with hashed list Suanming Mou
  2020-07-31  3:34 ` [dpdk-dev] [PATCH 1/2] net/mlx5: add hash list extended lookup and insert Suanming Mou
@ 2020-07-31  3:34 ` Suanming Mou
  1 sibling, 0 replies; 3+ messages in thread
From: Suanming Mou @ 2020-07-31  3:34 UTC (permalink / raw)
  To: viacheslavo, matan; +Cc: rasland, dev

To manage header modify actions mlx5 PMD used the single linked list and
lookup and insertion operations took too long times if there were millions
of objects and this impacted the flow insertion/deletion rate.

In order to optimize the performance the hashed list is engaged. The list
implementation is updated to support non-unique keys with few collisions.

Signed-off-by: Suanming Mou <suanmingm@mellanox.com>
Acked-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
---
 drivers/net/mlx5/linux/mlx5_os.c | 15 +++++++
 drivers/net/mlx5/mlx5.h          |  2 +-
 drivers/net/mlx5/mlx5_defs.h     |  3 ++
 drivers/net/mlx5/mlx5_flow.h     | 13 +++++-
 drivers/net/mlx5/mlx5_flow_dv.c  | 95 ++++++++++++++++++++++++++++++++--------
 5 files changed, 108 insertions(+), 20 deletions(-)

diff --git a/drivers/net/mlx5/linux/mlx5_os.c b/drivers/net/mlx5/linux/mlx5_os.c
index 69123e1..db955ae 100644
--- a/drivers/net/mlx5/linux/mlx5_os.c
+++ b/drivers/net/mlx5/linux/mlx5_os.c
@@ -241,6 +241,13 @@
 		err = ENOMEM;
 		goto error;
 	}
+	snprintf(s, sizeof(s), "%s_hdr_modify", sh->ibdev_name);
+	sh->modify_cmds = mlx5_hlist_create(s, MLX5_FLOW_HDR_MODIFY_HTABLE_SZ);
+	if (!sh->modify_cmds) {
+		DRV_LOG(ERR, "hdr modify hash creation failed");
+		err = ENOMEM;
+		goto error;
+	}
 #ifdef HAVE_MLX5DV_DR
 	void *domain;
 
@@ -314,6 +321,10 @@
 		mlx5_glue->destroy_flow_action(sh->pop_vlan_action);
 		sh->pop_vlan_action = NULL;
 	}
+	if (sh->modify_cmds) {
+		mlx5_hlist_destroy(sh->modify_cmds, NULL, NULL);
+		sh->modify_cmds = NULL;
+	}
 	if (sh->tag_table) {
 		/* tags should be destroyed with flow before. */
 		mlx5_hlist_destroy(sh->tag_table, NULL, NULL);
@@ -367,6 +378,10 @@
 	}
 	pthread_mutex_destroy(&sh->dv_mutex);
 #endif /* HAVE_MLX5DV_DR */
+	if (sh->modify_cmds) {
+		mlx5_hlist_destroy(sh->modify_cmds, NULL, NULL);
+		sh->modify_cmds = NULL;
+	}
 	if (sh->tag_table) {
 		/* tags should be destroyed with flow before. */
 		mlx5_hlist_destroy(sh->tag_table, NULL, NULL);
diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index 78d6eb7..1880a82 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -638,7 +638,7 @@ struct mlx5_dev_ctx_shared {
 	void *esw_drop_action; /* Pointer to DR E-Switch drop action. */
 	void *pop_vlan_action; /* Pointer to DR pop VLAN action. */
 	uint32_t encaps_decaps; /* Encap/decap action indexed memory list. */
-	LIST_HEAD(modify_cmd, mlx5_flow_dv_modify_hdr_resource) modify_cmds;
+	struct mlx5_hlist *modify_cmds;
 	struct mlx5_hlist *tag_table;
 	uint32_t port_id_action_list; /* List of port ID actions. */
 	uint32_t push_vlan_action_list; /* List of push VLAN actions. */
diff --git a/drivers/net/mlx5/mlx5_defs.h b/drivers/net/mlx5/mlx5_defs.h
index e5f7acc..4be33d6 100644
--- a/drivers/net/mlx5/mlx5_defs.h
+++ b/drivers/net/mlx5/mlx5_defs.h
@@ -187,6 +187,9 @@
 #define MLX5_FLOW_MREG_HNAME "MARK_COPY_TABLE"
 #define MLX5_DEFAULT_COPY_ID UINT32_MAX
 
+/* Size of the simple hash table for header modify table. */
+#define MLX5_FLOW_HDR_MODIFY_HTABLE_SZ (1 << 16)
+
 /* Hairpin TX/RX queue configuration parameters. */
 #define MLX5_HAIRPIN_QUEUE_STRIDE 6
 #define MLX5_HAIRPIN_JUMBO_LOG_SIZE (14 + 2)
diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index 66caefc..92301e4 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -427,7 +427,7 @@ struct mlx5_flow_dv_tag_resource {
 
 /* Modify resource structure */
 struct mlx5_flow_dv_modify_hdr_resource {
-	LIST_ENTRY(mlx5_flow_dv_modify_hdr_resource) next;
+	struct mlx5_hlist_entry entry;
 	/* Pointer to next element. */
 	rte_atomic32_t refcnt; /**< Reference counter. */
 	void *action;
@@ -439,6 +439,17 @@ struct mlx5_flow_dv_modify_hdr_resource {
 	/**< Modification actions. */
 };
 
+/* Modify resource key of the hash organization. */
+union mlx5_flow_modify_hdr_key {
+	struct {
+		uint32_t ft_type:8;	/**< Flow table type, Rx or Tx. */
+		uint32_t actions_num:5;	/**< Number of modification actions. */
+		uint32_t group:19;	/**< Flow group id. */
+		uint32_t cksum;		/**< Actions check sum. */
+	};
+	uint64_t v64;			/**< full 64bits value of key */
+};
+
 /* Jump action resource structure. */
 struct mlx5_flow_dv_jump_tbl_resource {
 	rte_atomic32_t refcnt; /**< Reference counter. */
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 5339980..dd35959 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -3958,6 +3958,40 @@ struct field_modify_info modify_tcp[] = {
 }
 
 /**
+ * Match modify-header resource.
+ *
+ * @param entry
+ *   Pointer to exist resource entry object.
+ * @param ctx
+ *   Pointer to new modify-header resource.
+ *
+ * @return
+ *   0 on matching, -1 otherwise.
+ */
+static int
+flow_dv_modify_hdr_resource_match(struct mlx5_hlist_entry *entry, void *ctx)
+{
+	struct mlx5_flow_dv_modify_hdr_resource *resource;
+	struct mlx5_flow_dv_modify_hdr_resource *cache_resource;
+	uint32_t actions_len;
+
+	resource = (struct mlx5_flow_dv_modify_hdr_resource *)ctx;
+	cache_resource = container_of(entry,
+				      struct mlx5_flow_dv_modify_hdr_resource,
+				      entry);
+	actions_len = resource->actions_num * sizeof(resource->actions[0]);
+	if (resource->entry.key == cache_resource->entry.key &&
+	    resource->ft_type == cache_resource->ft_type &&
+	    resource->actions_num == cache_resource->actions_num &&
+	    resource->flags == cache_resource->flags &&
+	    !memcmp((const void *)resource->actions,
+		    (const void *)cache_resource->actions,
+		    actions_len))
+		return 0;
+	return -1;
+}
+
+/**
  * Find existing modify-header resource or create and register a new one.
  *
  * @param dev[in, out]
@@ -3984,6 +4018,15 @@ struct field_modify_info modify_tcp[] = {
 	struct mlx5_flow_dv_modify_hdr_resource *cache_resource;
 	struct mlx5dv_dr_domain *ns;
 	uint32_t actions_len;
+	struct mlx5_hlist_entry *entry;
+	union mlx5_flow_modify_hdr_key hdr_mod_key = {
+		{
+			.ft_type = resource->ft_type,
+			.actions_num = resource->actions_num,
+			.group = dev_flow->dv.group,
+			.cksum = 0,
+		}
+	};
 	int ret;
 
 	resource->flags = dev_flow->dv.group ? 0 :
@@ -4001,20 +4044,22 @@ struct field_modify_info modify_tcp[] = {
 		ns = sh->rx_domain;
 	/* Lookup a matching resource from cache. */
 	actions_len = resource->actions_num * sizeof(resource->actions[0]);
-	LIST_FOREACH(cache_resource, &sh->modify_cmds, next) {
-		if (resource->ft_type == cache_resource->ft_type &&
-		    resource->actions_num == cache_resource->actions_num &&
-		    resource->flags == cache_resource->flags &&
-		    !memcmp((const void *)resource->actions,
-			    (const void *)cache_resource->actions,
-			    actions_len)) {
-			DRV_LOG(DEBUG, "modify-header resource %p: refcnt %d++",
-				(void *)cache_resource,
-				rte_atomic32_read(&cache_resource->refcnt));
-			rte_atomic32_inc(&cache_resource->refcnt);
-			dev_flow->handle->dvh.modify_hdr = cache_resource;
-			return 0;
-		}
+	hdr_mod_key.cksum = __rte_raw_cksum(resource->actions, actions_len, 0);
+	resource->entry.key = hdr_mod_key.v64;
+	entry = mlx5_hlist_lookup_ex(sh->modify_cmds, resource->entry.key,
+				     flow_dv_modify_hdr_resource_match,
+				     (void *)resource);
+	if (entry) {
+		cache_resource = container_of(entry,
+					struct mlx5_flow_dv_modify_hdr_resource,
+					entry);
+		DRV_LOG(DEBUG, "modify-header resource %p: refcnt %d++",
+			(void *)cache_resource,
+			rte_atomic32_read(&cache_resource->refcnt));
+		rte_atomic32_inc(&cache_resource->refcnt);
+		dev_flow->handle->dvh.modify_hdr = cache_resource;
+		return 0;
+
 	}
 	/* Register new modify-header resource. */
 	cache_resource = mlx5_malloc(MLX5_MEM_ZERO,
@@ -4037,7 +4082,16 @@ struct field_modify_info modify_tcp[] = {
 	}
 	rte_atomic32_init(&cache_resource->refcnt);
 	rte_atomic32_inc(&cache_resource->refcnt);
-	LIST_INSERT_HEAD(&sh->modify_cmds, cache_resource, next);
+	if (mlx5_hlist_insert_ex(sh->modify_cmds, &cache_resource->entry,
+				 flow_dv_modify_hdr_resource_match,
+				 (void *)cache_resource)) {
+		claim_zero(mlx5_flow_os_destroy_flow_action
+						(cache_resource->action));
+		mlx5_free(cache_resource);
+		return rte_flow_error_set(error, EEXIST,
+					  RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
+					  NULL, "action exist");
+	}
 	dev_flow->handle->dvh.modify_hdr = cache_resource;
 	DRV_LOG(DEBUG, "new modify-header resource %p: refcnt %d++",
 		(void *)cache_resource,
@@ -9122,6 +9176,8 @@ struct field_modify_info modify_tcp[] = {
 /**
  * Release a modify-header resource.
  *
+ * @param dev
+ *   Pointer to Ethernet device.
  * @param handle
  *   Pointer to mlx5_flow_handle.
  *
@@ -9129,8 +9185,10 @@ struct field_modify_info modify_tcp[] = {
  *   1 while a reference on it exists, 0 when freed.
  */
 static int
-flow_dv_modify_hdr_resource_release(struct mlx5_flow_handle *handle)
+flow_dv_modify_hdr_resource_release(struct rte_eth_dev *dev,
+				    struct mlx5_flow_handle *handle)
 {
+	struct mlx5_priv *priv = dev->data->dev_private;
 	struct mlx5_flow_dv_modify_hdr_resource *cache_resource =
 							handle->dvh.modify_hdr;
 
@@ -9141,7 +9199,8 @@ struct field_modify_info modify_tcp[] = {
 	if (rte_atomic32_dec_and_test(&cache_resource->refcnt)) {
 		claim_zero(mlx5_flow_os_destroy_flow_action
 						(cache_resource->action));
-		LIST_REMOVE(cache_resource, next);
+		mlx5_hlist_remove(priv->sh->modify_cmds,
+				  &cache_resource->entry);
 		mlx5_free(cache_resource);
 		DRV_LOG(DEBUG, "modify-header resource %p: removed",
 			(void *)cache_resource);
@@ -9351,7 +9410,7 @@ struct field_modify_info modify_tcp[] = {
 		if (dev_handle->dvh.rix_encap_decap)
 			flow_dv_encap_decap_resource_release(dev, dev_handle);
 		if (dev_handle->dvh.modify_hdr)
-			flow_dv_modify_hdr_resource_release(dev_handle);
+			flow_dv_modify_hdr_resource_release(dev, dev_handle);
 		if (dev_handle->dvh.rix_push_vlan)
 			flow_dv_push_vlan_action_resource_release(dev,
 								  dev_handle);
-- 
1.8.3.1


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

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-31  3:34 [dpdk-dev] [PATCH 0/2] net/mlx5: manage modify actions with hashed list Suanming Mou
2020-07-31  3:34 ` [dpdk-dev] [PATCH 1/2] net/mlx5: add hash list extended lookup and insert Suanming Mou
2020-07-31  3:34 ` [dpdk-dev] [PATCH 2/2] net/mlx5: manage modify actions with hashed list Suanming Mou

DPDK patches and discussions

Archives are clonable:
	git clone --mirror http://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/ http://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev


Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


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