[dpdk-dev] [PATCH 0/3] Reorganize resources of flow tables

[dpdk-dev] [PATCH 0/3] Reorganize resources of flow tables

[Bing Zhao]

Number of flow tables is limited by the memory resource, and the index could be
to as large as 2^^32 - 1. In the past, the flow tables are organized by arrays,
and this organization has some advantages and disadvantages. The lookup for the
table resource from a linear array is quite fast, the ID could be used as the
index in the array. But it will cost some extra memory resource after system
bring up and if only a small number of tables are created. In the meanwhile,
since we could not create the array with a huge number, so the maximal index of
the table is limited and it is  unreasonable.
If we change the array into some other tables, like some open addressing hash
table, the static memory cost is still to huge. But the index of the table
limitation could be get rid of. But in the meanwhile, it will introduce some
new issue that two tables with different ID may generate the same address index
in the table. Then it will degrade the performance of the lookup, creating and
deleting. Moreover, sometimes it will cause a failure if the collisions rate
are too heavy.
Then the simple hash list is used as the first step to get rid of this
limitations. The only static memory over head is array of the LIST HEADs. In
the next step, we could use some extendable hash tables for this. This will of
course introduce some performance degradation when lookup, creating and
removing tables in the lists if there are a lot of tables created in the
system. We need to trade off among the functionality, memory and performance.
Some other resources are associated with each flow tables and not global, like
flow matchers and jump table object used by driver. They could also be
reorganized and put into the flow table resources structure. Then the lookup
process of these resources will be speeded up significantly.

Bing Zhao (3):
  net/mlx5: reorganize flow tables with hash list
  net/mlx5: reorganize jump table resources
  net/mlx5: reorganize flow matcher resources

 drivers/net/mlx5/mlx5.c         |  16 +++
 drivers/net/mlx5/mlx5.h         |  25 ++--
 drivers/net/mlx5/mlx5_flow.h    |  24 ++--
 drivers/net/mlx5/mlx5_flow_dv.c | 307 +++++++++++++++++++++++-----------------
 4 files changed, 224 insertions(+), 148 deletions(-)

-- 
1.8.3.1

[dpdk-dev] [PATCH 1/3] net/mlx5: reorganize flow tables with hash list

[Bing Zhao]

In the current flow tables organization, arrays are used. This is
fast for searching, creating related object that will be used in
flow creation. But it introduces some limitation to the table index.
Then we can reorganize the flow tables information with hash list.
When using hash list, there is no need to maintain three arrays for
NIC TX, RX and FDB tables object information.
This attribute could be used together with the table ID to generate
a 64-bits key that is unique for the hash list insertion, lookup and
deletion.

Signed-off-by: Bing Zhao <bingz@mellanox.com>
---
 drivers/net/mlx5/mlx5.c         |  16 ++++
 drivers/net/mlx5/mlx5.h         |  23 ++++--
 drivers/net/mlx5/mlx5_flow.h    |   8 ++
 drivers/net/mlx5/mlx5_flow_dv.c | 176 +++++++++++++++++++++++++++-------------
 4 files changed, 159 insertions(+), 64 deletions(-)

diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index 91aaa9b..6474c53 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -184,6 +184,8 @@ struct mlx5_dev_spawn_data {
 #define MLX5_FLOW_MIN_ID_POOL_SIZE 512
 #define MLX5_ID_GENERATION_ARRAY_FACTOR 16
 
+#define MLX5_FLOW_TABLE_HLIST_ARRAY_SIZE 4096
+
 /**
  * Allocate ID pool structure.
  *
@@ -677,6 +679,7 @@ struct mlx5_flow_id_pool *
 	struct mlx5_ibv_shared *sh = priv->sh;
 	int err = 0;
 	void *domain;
+	char s[MLX5_HLIST_NAMESIZE];
 
 	assert(sh);
 	if (sh->dv_refcnt) {
@@ -716,6 +719,14 @@ struct mlx5_flow_id_pool *
 		sh->esw_drop_action = mlx5_glue->dr_create_flow_action_drop();
 	}
 #endif
+	snprintf(s, sizeof(s) - 1, "%s_flow_table", priv->sh->ibdev_name);
+	sh->flow_tbls = mlx5_hlist_create(s,
+					  MLX5_FLOW_TABLE_HLIST_ARRAY_SIZE);
+	if (!sh->flow_tbls) {
+		DRV_LOG(ERR, "flow tables with hash creation failed.\n");
+		err = -ENOMEM;
+		goto error;
+	}
 	sh->pop_vlan_action = mlx5_glue->dr_create_flow_action_pop_vlan();
 	sh->dv_refcnt++;
 	priv->dr_shared = 1;
@@ -770,6 +781,11 @@ struct mlx5_flow_id_pool *
 	assert(sh->dv_refcnt);
 	if (sh->dv_refcnt && --sh->dv_refcnt)
 		return;
+	if (sh->flow_tbls) {
+		/* flow table entries should be handled properly before. */
+		mlx5_hlist_destroy(sh->flow_tbls, NULL, NULL);
+		sh->flow_tbls = NULL;
+	}
 	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 fab58c9..73324da 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -559,10 +559,24 @@ struct mlx5_ibv_shared_port {
 	 */
 };
 
+/* Table key of the hash organization. */
+union mlx5_flow_tbl_key {
+	struct {
+		/* Table ID should be at the lowest address. */
+		uint32_t table_id;	/**< ID of the table. */
+		uint16_t reserved;	/**< must be zero for comparison. */
+		uint8_t domain;		/**< 1 - FDB, 0 - NIC TX/RX. */
+		uint8_t direction;	/**< 1 - egress, 0 - ingress. */
+	};
+	uint64_t v64;			/**< full 64bits value of key */
+};
+
 /* Table structure. */
 struct mlx5_flow_tbl_resource {
 	void *obj; /**< Pointer to DR table object. */
+#ifdef HAVE_MLX5DV_DR
 	rte_atomic32_t refcnt; /**< Reference counter. */
+#endif
 };
 
 #define MLX5_MAX_TABLES UINT16_MAX
@@ -630,17 +644,12 @@ struct mlx5_ibv_shared {
 	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. */
-	struct mlx5_flow_tbl_resource fdb_tbl[MLX5_MAX_TABLES_FDB];
-	/* FDB Direct Rules tables. */
 	void *rx_domain; /* RX Direct Rules name space handle. */
-	struct mlx5_flow_tbl_resource rx_tbl[MLX5_MAX_TABLES];
-	/* RX Direct Rules tables. */
 	void *tx_domain; /* TX Direct Rules name space handle. */
-	struct mlx5_flow_tbl_resource tx_tbl[MLX5_MAX_TABLES];
-	/* TX Direct Rules tables. */
+	struct mlx5_hlist *flow_tbls;
+	/* Direct Rules tables for FDB, NIC TX+RX */
 	void *esw_drop_action; /* Pointer to DR E-Switch drop action. */
 	void *pop_vlan_action; /* Pointer to DR pop VLAN action. */
-	/* TX Direct Rules tables/ */
 	LIST_HEAD(matchers, mlx5_flow_dv_matcher) matchers;
 	LIST_HEAD(encap_decap, mlx5_flow_dv_encap_decap_resource) encaps_decaps;
 	LIST_HEAD(modify_cmd, mlx5_flow_dv_modify_hdr_resource) modify_cmds;
diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index f84ea9d..8911f19 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -433,6 +433,14 @@ struct mlx5_flow_mreg_copy_resource {
 	struct rte_flow *flow; /* Built flow for copy. */
 };
 
+/* Table data structure of the hash organization. */
+struct mlx5_flow_tbl_data_entry {
+	struct mlx5_hlist_entry entry;
+	/**< flow table resource, better to locate at the beginning. */
+	struct mlx5_flow_tbl_resource tbl;
+	/**< flow table resource, better to locate at the beginning. */
+};
+
 /*
  * Max number of actions per DV flow.
  * See CREATE_FLOW_MAX_FLOW_ACTIONS_SUPPORTED
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 3f3dad0..2253e98 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -2184,7 +2184,7 @@ struct field_modify_info modify_tcp[] = {
 /**
  * Find existing encap/decap resource or create and register a new one.
  *
- * @param dev[in, out]
+ * @param[in, out] dev
  *   Pointer to rte_eth_dev structure.
  * @param[in, out] resource
  *   Pointer to encap/decap resource.
@@ -2265,7 +2265,7 @@ struct field_modify_info modify_tcp[] = {
 /**
  * Find existing table jump resource or create and register a new one.
  *
- * @param dev[in, out]
+ * @param[in, out] dev
  *   Pointer to rte_eth_dev structure.
  * @param[in, out] resource
  *   Pointer to jump table resource.
@@ -2328,7 +2328,7 @@ struct field_modify_info modify_tcp[] = {
 /**
  * Find existing table port ID resource or create and register a new one.
  *
- * @param dev[in, out]
+ * @param[in, out] dev
  *   Pointer to rte_eth_dev structure.
  * @param[in, out] resource
  *   Pointer to port ID action resource.
@@ -2392,7 +2392,7 @@ struct field_modify_info modify_tcp[] = {
 /**
  * Find existing push vlan resource or create and register a new one.
  *
- * @param dev[in, out]
+ * @param [in, out] dev
  *   Pointer to rte_eth_dev structure.
  * @param[in, out] resource
  *   Pointer to port ID action resource.
@@ -3230,8 +3230,6 @@ struct field_modify_info modify_tcp[] = {
 			     const struct rte_flow_attr *attributes,
 			     bool external, struct rte_flow_error *error)
 {
-	uint32_t max_group = attributes->transfer ? MLX5_MAX_TABLES_FDB :
-						    MLX5_MAX_TABLES;
 	uint32_t target_group, table;
 	int ret = 0;
 
@@ -3251,10 +3249,6 @@ struct field_modify_info modify_tcp[] = {
 				       &table, error);
 	if (ret)
 		return ret;
-	if (table >= max_group)
-		return rte_flow_error_set(error, EINVAL,
-					  RTE_FLOW_ERROR_TYPE_ATTR_GROUP, NULL,
-					  "target group index out of range");
 	if (attributes->group == target_group)
 		return rte_flow_error_set(error, EINVAL,
 					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
@@ -4086,11 +4080,6 @@ struct field_modify_info modify_tcp[] = {
 					  NULL,
 					  "groups are not supported");
 #else
-	uint32_t max_group = attributes->transfer ?
-			     MLX5_MAX_TABLES_FDB :
-				external ?
-				MLX5_MAX_TABLES_EXTERNAL :
-				MLX5_MAX_TABLES;
 	uint32_t table;
 	int ret;
 
@@ -4099,10 +4088,6 @@ struct field_modify_info modify_tcp[] = {
 				       &table, error);
 	if (ret)
 		return ret;
-	if (table >= max_group)
-		return rte_flow_error_set(error, EINVAL,
-					  RTE_FLOW_ERROR_TYPE_ATTR_GROUP, NULL,
-					  "group index out of range");
 #endif
 	if (attributes->priority != MLX5_FLOW_PRIO_RSVD &&
 	    attributes->priority >= priority_max)
@@ -6076,7 +6061,7 @@ struct field_modify_info modify_tcp[] = {
 /**
  * Get a flow table.
  *
- * @param dev[in, out]
+ * @param[in, out] dev
  *   Pointer to rte_eth_dev structure.
  * @param[in] table_id
  *   Table id to use.
@@ -6099,47 +6084,100 @@ struct field_modify_info modify_tcp[] = {
 	struct mlx5_priv *priv = dev->data->dev_private;
 	struct mlx5_ibv_shared *sh = priv->sh;
 	struct mlx5_flow_tbl_resource *tbl;
+	union mlx5_flow_tbl_key table_key = {
+		{
+			.table_id = table_id,
+			.reserved = 0,
+			.domain = !!transfer,
+			.direction = !!egress,
+		}
+	};
+	struct mlx5_hlist_entry *pos;
+	struct mlx5_flow_tbl_data_entry *tbl_data;
 
 #ifdef HAVE_MLX5DV_DR
-	if (transfer) {
-		tbl = &sh->fdb_tbl[table_id];
-		if (!tbl->obj)
-			tbl->obj = mlx5_glue->dr_create_flow_tbl
-				(sh->fdb_domain, table_id);
-	} else if (egress) {
-		tbl = &sh->tx_tbl[table_id];
-		if (!tbl->obj)
-			tbl->obj = mlx5_glue->dr_create_flow_tbl
-				(sh->tx_domain, table_id);
-	} else {
-		tbl = &sh->rx_tbl[table_id];
-		if (!tbl->obj)
-			tbl->obj = mlx5_glue->dr_create_flow_tbl
-				(sh->rx_domain, table_id);
+	int ret;
+	void *domain;
+
+	pos = mlx5_hlist_lookup(sh->flow_tbls, table_key.v64);
+	if (pos) {
+		tbl_data = container_of(pos, struct mlx5_flow_tbl_data_entry,
+					entry);
+		tbl = &tbl_data->tbl;
+		if (!tbl->obj) {
+			rte_flow_error_set(error, ENOKEY,
+					   RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
+					   NULL, "cannot find created table");
+			return NULL;
+		}
+		rte_atomic32_inc(&tbl->refcnt);
+		return tbl;
 	}
+	tbl_data = rte_zmalloc(NULL, sizeof(*tbl_data), 0);
+	if (!tbl_data) {
+		rte_flow_error_set(error, ENOMEM,
+				   RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
+				   NULL,
+				   "cannot allocate flow table data entry");
+		return NULL;
+	}
+	tbl = &tbl_data->tbl;
+	pos = &tbl_data->entry;
+	if (transfer)
+		domain = sh->fdb_domain;
+	else if (egress)
+		domain = sh->tx_domain;
+	else
+		domain = sh->rx_domain;
+	tbl->obj = mlx5_glue->dr_create_flow_tbl(domain, table_id);
 	if (!tbl->obj) {
 		rte_flow_error_set(error, ENOMEM,
 				   RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
-				   NULL, "cannot create table");
+				   NULL, "cannot create flow table object");
+		rte_free(tbl_data);
 		return NULL;
 	}
+	/*
+	 * No multi-threads now, but still better to initialize the reference
+	 * count before insert it into the hash list.
+	 */
+	rte_atomic32_init(&tbl->refcnt);
+	pos->key = table_key.v64;
+	ret = mlx5_hlist_insert(sh->flow_tbls, pos);
+	if (ret < 0) {
+		rte_flow_error_set(error, -ret,
+				   RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
+				   "cannot insert flow table data entry");
+		mlx5_glue->dr_destroy_flow_tbl(tbl->obj);
+		rte_free(tbl_data);
+	}
 	rte_atomic32_inc(&tbl->refcnt);
 	return tbl;
 #else
-	(void)error;
-	(void)tbl;
-	if (transfer)
-		return &sh->fdb_tbl[table_id];
-	else if (egress)
-		return &sh->tx_tbl[table_id];
-	else
-		return &sh->rx_tbl[table_id];
+	/* Just to make the compiling pass when no HAVE_MLX5DV_DR defined. */
+	pos = mlx5_hlist_lookup(sh->flow_tbls, table_key.v64);
+	if (pos) {
+		tbl_data = container_of(pos, struct mlx5_flow_tbl_data_entry,
+					entry);
+		tbl = &tbl_data->tbl;
+		if (!tbl->obj) {
+			rte_flow_error_set(error, ENOKEY,
+					   RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
+					   NULL, "cannot find created table");
+			return NULL;
+		}
+		rte_atomic32_inc(&tbl->refcnt);
+		return tbl;
+	}
+	return NULL;
 #endif
 }
 
 /**
  * Release a flow table.
  *
+ * @param[in] dev
+ *   Pointer to rte_eth_dev structure.
  * @param[in] tbl
  *   Table resource to be released.
  *
@@ -6147,13 +6185,24 @@ struct field_modify_info modify_tcp[] = {
  *   Returns 0 if table was released, else return 1;
  */
 static int
-flow_dv_tbl_resource_release(struct mlx5_flow_tbl_resource *tbl)
+flow_dv_tbl_resource_release(struct rte_eth_dev *dev,
+			     struct mlx5_flow_tbl_resource *tbl)
 {
+	struct mlx5_priv *priv = dev->data->dev_private;
+	struct mlx5_ibv_shared *sh = priv->sh;
+	struct mlx5_flow_tbl_data_entry *tbl_data =
+		container_of(tbl, struct mlx5_flow_tbl_data_entry, tbl);
+
 	if (!tbl)
 		return 0;
 	if (rte_atomic32_dec_and_test(&tbl->refcnt)) {
+		struct mlx5_hlist_entry *pos = &tbl_data->entry;
+
 		mlx5_glue->dr_destroy_flow_tbl(tbl->obj);
 		tbl->obj = NULL;
+		/* remove the entry from the hash list and free memory. */
+		mlx5_hlist_remove(sh->flow_tbls, pos);
+		rte_free(tbl_data);
 		return 0;
 	}
 	return 1;
@@ -6162,7 +6211,7 @@ struct field_modify_info modify_tcp[] = {
 /**
  * Register the flow matcher.
  *
- * @param dev[in, out]
+ * @param[in, out] dev
  *   Pointer to rte_eth_dev structure.
  * @param[in, out] matcher
  *   Pointer to flow matcher.
@@ -6236,7 +6285,7 @@ struct field_modify_info modify_tcp[] = {
 	if (!cache_matcher->matcher_object) {
 		rte_free(cache_matcher);
 #ifdef HAVE_MLX5DV_DR
-		flow_dv_tbl_resource_release(tbl);
+		flow_dv_tbl_resource_release(dev, tbl);
 #endif
 		return rte_flow_error_set(error, ENOMEM,
 					  RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
@@ -6770,7 +6819,7 @@ struct field_modify_info modify_tcp[] = {
 			jump_tbl_resource.tbl = tbl;
 			if (flow_dv_jump_tbl_resource_register
 			    (dev, &jump_tbl_resource, dev_flow, error)) {
-				flow_dv_tbl_resource_release(tbl);
+				flow_dv_tbl_resource_release(dev, tbl);
 				return rte_flow_error_set
 						(error, errno,
 						 RTE_FLOW_ERROR_TYPE_ACTION,
@@ -7233,21 +7282,31 @@ struct field_modify_info modify_tcp[] = {
 	struct mlx5_flow_dv_matcher *matcher = flow->dv.matcher;
 	struct mlx5_priv *priv = dev->data->dev_private;
 	struct mlx5_ibv_shared *sh = priv->sh;
-	struct mlx5_flow_tbl_resource *tbl;
+	struct mlx5_flow_tbl_data_entry *tbl_data;
 
 	assert(matcher->matcher_object);
 	DRV_LOG(DEBUG, "port %u matcher %p: refcnt %d--",
 		dev->data->port_id, (void *)matcher,
 		rte_atomic32_read(&matcher->refcnt));
 	if (rte_atomic32_dec_and_test(&matcher->refcnt)) {
+		struct mlx5_hlist_entry *pos;
+		union mlx5_flow_tbl_key table_key = {
+			{
+				.table_id = matcher->group,
+				.reserved = 0,
+				.domain = !!matcher->transfer,
+				.direction = !!matcher->egress,
+			}
+		};
 		claim_zero(mlx5_glue->dv_destroy_flow_matcher
 			   (matcher->matcher_object));
 		LIST_REMOVE(matcher, next);
-		if (matcher->egress)
-			tbl = &sh->tx_tbl[matcher->group];
-		else
-			tbl = &sh->rx_tbl[matcher->group];
-		flow_dv_tbl_resource_release(tbl);
+		pos = mlx5_hlist_lookup(sh->flow_tbls, table_key.v64);
+		if (pos) {
+			tbl_data = container_of(pos,
+				struct mlx5_flow_tbl_data_entry, entry);
+			flow_dv_tbl_resource_release(dev, &tbl_data->tbl);
+		}
 		rte_free(matcher);
 		DRV_LOG(DEBUG, "port %u matcher %p: removed",
 			dev->data->port_id, (void *)matcher);
@@ -7290,6 +7349,8 @@ struct field_modify_info modify_tcp[] = {
 /**
  * Release an jump to table action resource.
  *
+ * @param dev
+ *   Pointer to Ethernet device.
  * @param flow
  *   Pointer to mlx5_flow.
  *
@@ -7297,7 +7358,8 @@ struct field_modify_info modify_tcp[] = {
  *   1 while a reference on it exists, 0 when freed.
  */
 static int
-flow_dv_jump_tbl_resource_release(struct mlx5_flow *flow)
+flow_dv_jump_tbl_resource_release(struct rte_eth_dev *dev,
+				  struct mlx5_flow *flow)
 {
 	struct mlx5_flow_dv_jump_tbl_resource *cache_resource =
 						flow->dv.jump;
@@ -7310,7 +7372,7 @@ struct field_modify_info modify_tcp[] = {
 		claim_zero(mlx5_glue->destroy_flow_action
 				(cache_resource->action));
 		LIST_REMOVE(cache_resource, next);
-		flow_dv_tbl_resource_release(cache_resource->tbl);
+		flow_dv_tbl_resource_release(dev, cache_resource->tbl);
 		rte_free(cache_resource);
 		DRV_LOG(DEBUG, "jump table resource %p: removed",
 			(void *)cache_resource);
@@ -7479,7 +7541,7 @@ struct field_modify_info modify_tcp[] = {
 		if (dev_flow->dv.modify_hdr)
 			flow_dv_modify_hdr_resource_release(dev_flow);
 		if (dev_flow->dv.jump)
-			flow_dv_jump_tbl_resource_release(dev_flow);
+			flow_dv_jump_tbl_resource_release(dev, dev_flow);
 		if (dev_flow->dv.port_id_action)
 			flow_dv_port_id_action_resource_release(dev_flow);
 		if (dev_flow->dv.push_vlan_res)
-- 
1.8.3.1

[dpdk-dev] [PATCH 2/3] net/mlx5: reorganize jump table resources

[Bing Zhao]

Jump object is associated with table object, so there is no need to
use a single linked list to store it. All the jump objects could be
put together with related flow tables.

Signed-off-by: Bing Zhao <bingz@mellanox.com>
---
 drivers/net/mlx5/mlx5.h         |  1 -
 drivers/net/mlx5/mlx5_flow.h    |  7 ++--
 drivers/net/mlx5/mlx5_flow_dv.c | 82 +++++++++++++++++------------------------
 3 files changed, 37 insertions(+), 53 deletions(-)

diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index 73324da..5a77870 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -654,7 +654,6 @@ struct mlx5_ibv_shared {
 	LIST_HEAD(encap_decap, mlx5_flow_dv_encap_decap_resource) encaps_decaps;
 	LIST_HEAD(modify_cmd, mlx5_flow_dv_modify_hdr_resource) modify_cmds;
 	LIST_HEAD(tag, mlx5_flow_dv_tag_resource) tags;
-	LIST_HEAD(jump, mlx5_flow_dv_jump_tbl_resource) jump_tbl;
 	LIST_HEAD(port_id_action_list, mlx5_flow_dv_port_id_action_resource)
 		port_id_action_list; /* List of port ID actions. */
 	LIST_HEAD(push_vlan_action_list, mlx5_flow_dv_push_vlan_action_resource)
diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index 8911f19..c21afd8 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -390,12 +390,9 @@ struct mlx5_flow_dv_modify_hdr_resource {
 
 /* Jump action resource structure. */
 struct mlx5_flow_dv_jump_tbl_resource {
-	LIST_ENTRY(mlx5_flow_dv_jump_tbl_resource) next;
-	/* Pointer to next element. */
 	rte_atomic32_t refcnt; /**< Reference counter. */
-	void *action; /**< Pointer to the rdma core action. */
 	uint8_t ft_type; /**< Flow table type, Rx or Tx. */
-	struct mlx5_flow_tbl_resource *tbl; /**< The target table. */
+	void *action; /**< Pointer to the rdma core action. */
 };
 
 /* Port ID resource structure. */
@@ -439,6 +436,8 @@ struct mlx5_flow_tbl_data_entry {
 	/**< flow table resource, better to locate at the beginning. */
 	struct mlx5_flow_tbl_resource tbl;
 	/**< flow table resource, better to locate at the beginning. */
+	struct mlx5_flow_dv_jump_tbl_resource jump;
+	/**< jump resource, at most one for each table created. */
 };
 
 /*
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 2253e98..2942850 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -2267,8 +2267,8 @@ struct field_modify_info modify_tcp[] = {
  *
  * @param[in, out] dev
  *   Pointer to rte_eth_dev structure.
- * @param[in, out] resource
- *   Pointer to jump table resource.
+ * @param[in, out] tbl
+ *   Pointer to flow table resource.
  * @parm[in, out] dev_flow
  *   Pointer to the dev_flow.
  * @param[out] error
@@ -2279,49 +2279,34 @@ struct field_modify_info modify_tcp[] = {
  */
 static int
 flow_dv_jump_tbl_resource_register
-			(struct rte_eth_dev *dev,
-			 struct mlx5_flow_dv_jump_tbl_resource *resource,
+			(struct rte_eth_dev *dev __rte_unused,
+			 struct mlx5_flow_tbl_resource *tbl,
 			 struct mlx5_flow *dev_flow,
 			 struct rte_flow_error *error)
 {
-	struct mlx5_priv *priv = dev->data->dev_private;
-	struct mlx5_ibv_shared *sh = priv->sh;
-	struct mlx5_flow_dv_jump_tbl_resource *cache_resource;
+	struct mlx5_flow_tbl_data_entry *tbl_data =
+		container_of(tbl, struct mlx5_flow_tbl_data_entry, tbl);
+	int cnt;
 
-	/* Lookup a matching resource from cache. */
-	LIST_FOREACH(cache_resource, &sh->jump_tbl, next) {
-		if (resource->tbl == cache_resource->tbl) {
-			DRV_LOG(DEBUG, "jump table resource resource %p: refcnt %d++",
-				(void *)cache_resource,
-				rte_atomic32_read(&cache_resource->refcnt));
-			rte_atomic32_inc(&cache_resource->refcnt);
-			dev_flow->dv.jump = cache_resource;
-			return 0;
-		}
-	}
-	/* Register new jump table resource. */
-	cache_resource = rte_calloc(__func__, 1, sizeof(*cache_resource), 0);
-	if (!cache_resource)
-		return rte_flow_error_set(error, ENOMEM,
-					  RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
-					  "cannot allocate resource memory");
-	*cache_resource = *resource;
-	cache_resource->action =
-		mlx5_glue->dr_create_flow_action_dest_flow_tbl
-		(resource->tbl->obj);
-	if (!cache_resource->action) {
-		rte_free(cache_resource);
-		return rte_flow_error_set(error, ENOMEM,
-					  RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
-					  NULL, "cannot create action");
+	assert(tbl);
+	cnt = rte_atomic32_read(&tbl_data->jump.refcnt);
+	if (!cnt) {
+		tbl_data->jump.action =
+			mlx5_glue->dr_create_flow_action_dest_flow_tbl
+			(tbl->obj);
+		if (!tbl_data->jump.action)
+			return rte_flow_error_set(error, ENOMEM,
+					RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
+					NULL, "cannot create jump action");
+		DRV_LOG(DEBUG, "new jump table resource %p: refcnt %d++",
+			(void *)&tbl_data->jump, cnt);
+	} else {
+		assert(tbl_data->jump.action);
+		DRV_LOG(DEBUG, "existed jump table resource %p: refcnt %d++",
+			(void *)&tbl_data->jump, cnt);
 	}
-	rte_atomic32_init(&cache_resource->refcnt);
-	rte_atomic32_inc(&cache_resource->refcnt);
-	LIST_INSERT_HEAD(&sh->jump_tbl, cache_resource, next);
-	dev_flow->dv.jump = cache_resource;
-	DRV_LOG(DEBUG, "new jump table  resource %p: refcnt %d++",
-		(void *)cache_resource,
-		rte_atomic32_read(&cache_resource->refcnt));
+	rte_atomic32_inc(&tbl_data->jump.refcnt);
+	dev_flow->dv.jump = &tbl_data->jump;
 	return 0;
 }
 
@@ -6142,6 +6127,8 @@ struct field_modify_info modify_tcp[] = {
 	 * count before insert it into the hash list.
 	 */
 	rte_atomic32_init(&tbl->refcnt);
+	/* Jump action reference count is initialized here. */
+	rte_atomic32_init(&tbl_data->jump.refcnt);
 	pos->key = table_key.v64;
 	ret = mlx5_hlist_insert(sh->flow_tbls, pos);
 	if (ret < 0) {
@@ -6551,7 +6538,6 @@ struct field_modify_info modify_tcp[] = {
 		const struct rte_flow_action_count *count = action->conf;
 		const uint8_t *rss_key;
 		const struct rte_flow_action_jump *jump_data;
-		struct mlx5_flow_dv_jump_tbl_resource jump_tbl_resource;
 		struct mlx5_flow_tbl_resource *tbl;
 		uint32_t port_id = 0;
 		struct mlx5_flow_dv_port_id_action_resource port_id_resource;
@@ -6816,9 +6802,8 @@ struct field_modify_info modify_tcp[] = {
 						 RTE_FLOW_ERROR_TYPE_ACTION,
 						 NULL,
 						 "cannot create jump action.");
-			jump_tbl_resource.tbl = tbl;
 			if (flow_dv_jump_tbl_resource_register
-			    (dev, &jump_tbl_resource, dev_flow, error)) {
+			    (dev, tbl, dev_flow, error)) {
 				flow_dv_tbl_resource_release(dev, tbl);
 				return rte_flow_error_set
 						(error, errno,
@@ -7361,8 +7346,10 @@ struct field_modify_info modify_tcp[] = {
 flow_dv_jump_tbl_resource_release(struct rte_eth_dev *dev,
 				  struct mlx5_flow *flow)
 {
-	struct mlx5_flow_dv_jump_tbl_resource *cache_resource =
-						flow->dv.jump;
+	struct mlx5_flow_dv_jump_tbl_resource *cache_resource = flow->dv.jump;
+	struct mlx5_flow_tbl_data_entry *tbl_data =
+			container_of(cache_resource,
+				     struct mlx5_flow_tbl_data_entry, jump);
 
 	assert(cache_resource->action);
 	DRV_LOG(DEBUG, "jump table resource %p: refcnt %d--",
@@ -7371,9 +7358,8 @@ struct field_modify_info modify_tcp[] = {
 	if (rte_atomic32_dec_and_test(&cache_resource->refcnt)) {
 		claim_zero(mlx5_glue->destroy_flow_action
 				(cache_resource->action));
-		LIST_REMOVE(cache_resource, next);
-		flow_dv_tbl_resource_release(dev, cache_resource->tbl);
-		rte_free(cache_resource);
+		/* jump action memory free is inside the table release. */
+		flow_dv_tbl_resource_release(dev, &tbl_data->tbl);
 		DRV_LOG(DEBUG, "jump table resource %p: removed",
 			(void *)cache_resource);
 		return 0;
-- 
1.8.3.1

[dpdk-dev] [PATCH 3/3] net/mlx5: reorganize flow matcher resources

[Bing Zhao]

Matchers are created on the specific table. If a single linked list
is used to store these, then the finding process might be the
bottleneck when there are a lot of different flow matchers on a
huge amount of tables. The matchers could be move into the table
data resource structure in order to reduce the comparasion times
when finding.

Signed-off-by: Bing Zhao <bingz@mellanox.com>
---
 drivers/net/mlx5/mlx5.h         |  1 -
 drivers/net/mlx5/mlx5_flow.h    | 13 ++++---
 drivers/net/mlx5/mlx5_flow_dv.c | 83 ++++++++++++++++++++---------------------
 3 files changed, 47 insertions(+), 50 deletions(-)

diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index 5a77870..91442fe 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -650,7 +650,6 @@ struct mlx5_ibv_shared {
 	/* Direct Rules tables for FDB, NIC TX+RX */
 	void *esw_drop_action; /* Pointer to DR E-Switch drop action. */
 	void *pop_vlan_action; /* Pointer to DR pop VLAN action. */
-	LIST_HEAD(matchers, mlx5_flow_dv_matcher) matchers;
 	LIST_HEAD(encap_decap, mlx5_flow_dv_encap_decap_resource) encaps_decaps;
 	LIST_HEAD(modify_cmd, mlx5_flow_dv_modify_hdr_resource) modify_cmds;
 	LIST_HEAD(tag, mlx5_flow_dv_tag_resource) tags;
diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index c21afd8..df5f604 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -329,14 +329,13 @@ struct mlx5_flow_dv_match_params {
 /* Matcher structure. */
 struct mlx5_flow_dv_matcher {
 	LIST_ENTRY(mlx5_flow_dv_matcher) next;
-	/* Pointer to the next element. */
+	/**< Pointer to the next element. */
+	struct mlx5_flow_tbl_resource *tbl;
+	/**< Pointer to the table(group) the matcher associated with. */
 	rte_atomic32_t refcnt; /**< Reference counter. */
 	void *matcher_object; /**< Pointer to DV matcher */
 	uint16_t crc; /**< CRC of key. */
 	uint16_t priority; /**< Priority of matcher. */
-	uint8_t egress; /**< Egress matcher. */
-	uint8_t transfer; /**< 1 if the flow is E-Switch flow. */
-	uint32_t group; /**< The matcher group. */
 	struct mlx5_flow_dv_match_params mask; /**< Matcher mask. */
 };
 
@@ -433,9 +432,11 @@ struct mlx5_flow_mreg_copy_resource {
 /* Table data structure of the hash organization. */
 struct mlx5_flow_tbl_data_entry {
 	struct mlx5_hlist_entry entry;
-	/**< flow table resource, better to locate at the beginning. */
+	/**< hash list entry, 64-bits key inside. */
 	struct mlx5_flow_tbl_resource tbl;
-	/**< flow table resource, better to locate at the beginning. */
+	/**< flow table resource. */
+	LIST_HEAD(matchers, mlx5_flow_dv_matcher) matchers;
+	/**< matchers' header associated with the flow table. */
 	struct mlx5_flow_dv_jump_tbl_resource jump;
 	/**< jump resource, at most one for each table created. */
 };
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 2942850..d33d4fd 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -6202,6 +6202,8 @@ struct field_modify_info modify_tcp[] = {
  *   Pointer to rte_eth_dev structure.
  * @param[in, out] matcher
  *   Pointer to flow matcher.
+ * @param[in, out] key
+ *   Pointer to flow table key.
  * @parm[in, out] dev_flow
  *   Pointer to the dev_flow.
  * @param[out] error
@@ -6213,6 +6215,7 @@ struct field_modify_info modify_tcp[] = {
 static int
 flow_dv_matcher_register(struct rte_eth_dev *dev,
 			 struct mlx5_flow_dv_matcher *matcher,
+			 union mlx5_flow_tbl_key *key,
 			 struct mlx5_flow *dev_flow,
 			 struct rte_flow_error *error)
 {
@@ -6223,49 +6226,53 @@ struct field_modify_info modify_tcp[] = {
 		.type = IBV_FLOW_ATTR_NORMAL,
 		.match_mask = (void *)&matcher->mask,
 	};
-	struct mlx5_flow_tbl_resource *tbl = NULL;
+	struct mlx5_flow_tbl_resource *tbl;
+	struct mlx5_flow_tbl_data_entry *tbl_data;
 
+	tbl = flow_dv_tbl_resource_get(dev, key->table_id, key->direction,
+				       key->domain, error);
+	if (!tbl)
+		return -rte_errno;	/* No need to refill the error info */
+	tbl_data = container_of(tbl, struct mlx5_flow_tbl_data_entry, tbl);
 	/* Lookup from cache. */
-	LIST_FOREACH(cache_matcher, &sh->matchers, next) {
+	LIST_FOREACH(cache_matcher, &tbl_data->matchers, next) {
 		if (matcher->crc == cache_matcher->crc &&
 		    matcher->priority == cache_matcher->priority &&
-		    matcher->egress == cache_matcher->egress &&
-		    matcher->group == cache_matcher->group &&
-		    matcher->transfer == cache_matcher->transfer &&
 		    !memcmp((const void *)matcher->mask.buf,
 			    (const void *)cache_matcher->mask.buf,
 			    cache_matcher->mask.size)) {
 			DRV_LOG(DEBUG,
-				"priority %hd use %s matcher %p: refcnt %d++",
+				"%s group %u priority %hd use %s "
+				"matcher %p: refcnt %d++",
+				key->domain ? "FDB" : "NIC", key->table_id,
 				cache_matcher->priority,
-				cache_matcher->egress ? "tx" : "rx",
+				key->direction ? "tx" : "rx",
 				(void *)cache_matcher,
 				rte_atomic32_read(&cache_matcher->refcnt));
 			rte_atomic32_inc(&cache_matcher->refcnt);
 			dev_flow->dv.matcher = cache_matcher;
+			/* old matcher should not make the table ref++. */
+#ifdef HAVE_MLX5DV_DR
+			flow_dv_tbl_resource_release(dev, tbl);
+#endif
 			return 0;
 		}
 	}
 	/* Register new matcher. */
 	cache_matcher = rte_calloc(__func__, 1, sizeof(*cache_matcher), 0);
-	if (!cache_matcher)
+	if (!cache_matcher) {
+#ifdef HAVE_MLX5DV_DR
+		flow_dv_tbl_resource_release(dev, tbl);
+#endif
 		return rte_flow_error_set(error, ENOMEM,
 					  RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
 					  "cannot allocate matcher memory");
-	tbl = flow_dv_tbl_resource_get(dev, matcher->group,
-				       matcher->egress, matcher->transfer,
-				       error);
-	if (!tbl) {
-		rte_free(cache_matcher);
-		return rte_flow_error_set(error, ENOMEM,
-					  RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
-					  NULL, "cannot create table");
 	}
 	*cache_matcher = *matcher;
 	dv_attr.match_criteria_enable =
 		flow_dv_matcher_enable(cache_matcher->mask.buf);
 	dv_attr.priority = matcher->priority;
-	if (matcher->egress)
+	if (key->direction)
 		dv_attr.flags |= IBV_FLOW_ATTR_FLAGS_EGRESS;
 	cache_matcher->matcher_object =
 		mlx5_glue->dv_create_flow_matcher(sh->ctx, &dv_attr, tbl->obj);
@@ -6278,14 +6285,18 @@ struct field_modify_info modify_tcp[] = {
 					  RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
 					  NULL, "cannot create matcher");
 	}
+	/* Save the table information */
+	cache_matcher->tbl = tbl;
+	rte_atomic32_init(&cache_matcher->refcnt);
+	/* only matcher ref++, table ref++ already done above in get API. */
 	rte_atomic32_inc(&cache_matcher->refcnt);
-	LIST_INSERT_HEAD(&sh->matchers, cache_matcher, next);
+	LIST_INSERT_HEAD(&tbl_data->matchers, cache_matcher, next);
 	dev_flow->dv.matcher = cache_matcher;
-	DRV_LOG(DEBUG, "priority %hd new %s matcher %p: refcnt %d",
+	DRV_LOG(DEBUG, "%s group %u priority %hd new %s matcher %p: refcnt %d",
+		key->domain ? "FDB" : "NIC", key->table_id,
 		cache_matcher->priority,
-		cache_matcher->egress ? "tx" : "rx", (void *)cache_matcher,
+		key->direction ? "tx" : "rx", (void *)cache_matcher,
 		rte_atomic32_read(&cache_matcher->refcnt));
-	rte_atomic32_inc(&tbl->refcnt);
 	return 0;
 }
 
@@ -6514,6 +6525,7 @@ struct field_modify_info modify_tcp[] = {
 	};
 	union flow_dv_attr flow_attr = { .attr = 0 };
 	struct mlx5_flow_dv_tag_resource tag_resource;
+	union mlx5_flow_tbl_key tbl_key;
 	uint32_t modify_action_position = UINT32_MAX;
 	void *match_mask = matcher.mask.buf;
 	void *match_value = dev_flow->dv.value.buf;
@@ -7126,10 +7138,11 @@ struct field_modify_info modify_tcp[] = {
 				    matcher.mask.size);
 	matcher.priority = mlx5_flow_adjust_priority(dev, priority,
 						     matcher.priority);
-	matcher.egress = attr->egress;
-	matcher.group = dev_flow->group;
-	matcher.transfer = attr->transfer;
-	if (flow_dv_matcher_register(dev, &matcher, dev_flow, error))
+	/* reserved field no needs to be set to 0 here. */
+	tbl_key.domain = attr->transfer;
+	tbl_key.direction = attr->egress;
+	tbl_key.table_id = dev_flow->group;
+	if (flow_dv_matcher_register(dev, &matcher, &tbl_key, dev_flow, error))
 		return -rte_errno;
 	return 0;
 }
@@ -7265,33 +7278,17 @@ struct field_modify_info modify_tcp[] = {
 			struct mlx5_flow *flow)
 {
 	struct mlx5_flow_dv_matcher *matcher = flow->dv.matcher;
-	struct mlx5_priv *priv = dev->data->dev_private;
-	struct mlx5_ibv_shared *sh = priv->sh;
-	struct mlx5_flow_tbl_data_entry *tbl_data;
 
 	assert(matcher->matcher_object);
 	DRV_LOG(DEBUG, "port %u matcher %p: refcnt %d--",
 		dev->data->port_id, (void *)matcher,
 		rte_atomic32_read(&matcher->refcnt));
 	if (rte_atomic32_dec_and_test(&matcher->refcnt)) {
-		struct mlx5_hlist_entry *pos;
-		union mlx5_flow_tbl_key table_key = {
-			{
-				.table_id = matcher->group,
-				.reserved = 0,
-				.domain = !!matcher->transfer,
-				.direction = !!matcher->egress,
-			}
-		};
 		claim_zero(mlx5_glue->dv_destroy_flow_matcher
 			   (matcher->matcher_object));
 		LIST_REMOVE(matcher, next);
-		pos = mlx5_hlist_lookup(sh->flow_tbls, table_key.v64);
-		if (pos) {
-			tbl_data = container_of(pos,
-				struct mlx5_flow_tbl_data_entry, entry);
-			flow_dv_tbl_resource_release(dev, &tbl_data->tbl);
-		}
+		/* table ref-- in release interface. */
+		flow_dv_tbl_resource_release(dev, matcher->tbl);
 		rte_free(matcher);
 		DRV_LOG(DEBUG, "port %u matcher %p: removed",
 			dev->data->port_id, (void *)matcher);
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH 0/3] Reorganize resources of flow tables

[Slava Ovsiienko]

> -----Original Message-----
> From: Bing Zhao <bingz@mellanox.com>
> Sent: Friday, November 8, 2019 6:45
> To: Slava Ovsiienko <viacheslavo@mellanox.com>
> Cc: Ori Kam <orika@mellanox.com>; Raslan Darawsheh
> <rasland@mellanox.com>; dev@dpdk.org
> Subject: [PATCH 0/3] Reorganize resources of flow tables
> 
> Number of flow tables is limited by the memory resource, and the index
> could be to as large as 2^^32 - 1. In the past, the flow tables are organized
> by arrays, and this organization has some advantages and disadvantages.
> The lookup for the table resource from a linear array is quite fast, the ID
> could be used as the index in the array. But it will cost some extra memory
> resource after system bring up and if only a small number of tables are
> created. In the meanwhile, since we could not create the array with a huge
> number, so the maximal index of the table is limited and it is  unreasonable.
> If we change the array into some other tables, like some open addressing
> hash table, the static memory cost is still to huge. But the index of the table
> limitation could be get rid of. But in the meanwhile, it will introduce some
> new issue that two tables with different ID may generate the same address
> index in the table. Then it will degrade the performance of the lookup,
> creating and deleting. Moreover, sometimes it will cause a failure if the
> collisions rate are too heavy.
> Then the simple hash list is used as the first step to get rid of this limitations.
> The only static memory over head is array of the LIST HEADs. In the next
> step, we could use some extendable hash tables for this. This will of course
> introduce some performance degradation when lookup, creating and
> removing tables in the lists if there are a lot of tables created in the system.
> We need to trade off among the functionality, memory and performance.
> Some other resources are associated with each flow tables and not global,
> like flow matchers and jump table object used by driver. They could also be
> reorganized and put into the flow table resources structure. Then the lookup
> process of these resources will be speeded up significantly.
> 
> Bing Zhao (3):
>   net/mlx5: reorganize flow tables with hash list
>   net/mlx5: reorganize jump table resources
>   net/mlx5: reorganize flow matcher resources
> 
>  drivers/net/mlx5/mlx5.c         |  16 +++
>  drivers/net/mlx5/mlx5.h         |  25 ++--
>  drivers/net/mlx5/mlx5_flow.h    |  24 ++--
>  drivers/net/mlx5/mlx5_flow_dv.c | 307 +++++++++++++++++++++++----------
> -------
>  4 files changed, 224 insertions(+), 148 deletions(-)
> 
> --
> 1.8.3.1
Acked-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>

Re: [dpdk-dev] [PATCH 2/3] net/mlx5: reorganize jump table resources

[Slava Ovsiienko]

> -----Original Message-----
> From: Bing Zhao <bingz@mellanox.com>
> Sent: Friday, November 8, 2019 6:45
> To: Slava Ovsiienko <viacheslavo@mellanox.com>
> Cc: Ori Kam <orika@mellanox.com>; Raslan Darawsheh
> <rasland@mellanox.com>; dev@dpdk.org
> Subject: [PATCH 2/3] net/mlx5: reorganize jump table resources
> 
> Jump object is associated with table object, so there is no need to use a
> single linked list to store it. All the jump objects could be put together with
> related flow tables.
> 
> Signed-off-by: Bing Zhao <bingz@mellanox.com>
Acked-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>

Re: [dpdk-dev] [PATCH 1/3] net/mlx5: reorganize flow tables with hash list

[Slava Ovsiienko]

> -----Original Message-----
> From: Bing Zhao <bingz@mellanox.com>
> Sent: Friday, November 8, 2019 6:45
> To: Slava Ovsiienko <viacheslavo@mellanox.com>
> Cc: Ori Kam <orika@mellanox.com>; Raslan Darawsheh
> <rasland@mellanox.com>; dev@dpdk.org
> Subject: [PATCH 1/3] net/mlx5: reorganize flow tables with hash list
> 
> In the current flow tables organization, arrays are used. This is fast for
> searching, creating related object that will be used in flow creation. But it
> introduces some limitation to the table index.
> Then we can reorganize the flow tables information with hash list.
> When using hash list, there is no need to maintain three arrays for NIC TX, RX
> and FDB tables object information.
> This attribute could be used together with the table ID to generate a 64-bits
> key that is unique for the hash list insertion, lookup and deletion.
> 
> Signed-off-by: Bing Zhao <bingz@mellanox.com>
Acked-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>

Re: [dpdk-dev] [PATCH 3/3] net/mlx5: reorganize flow matcher resources

[Slava Ovsiienko]

> -----Original Message-----
> From: Bing Zhao <bingz@mellanox.com>
> Sent: Friday, November 8, 2019 6:45
> To: Slava Ovsiienko <viacheslavo@mellanox.com>
> Cc: Ori Kam <orika@mellanox.com>; Raslan Darawsheh
> <rasland@mellanox.com>; dev@dpdk.org
> Subject: [PATCH 3/3] net/mlx5: reorganize flow matcher resources
> 
> Matchers are created on the specific table. If a single linked list is used to
> store these, then the finding process might be the bottleneck when there are
> a lot of different flow matchers on a huge amount of tables. The matchers
> could be move into the table data resource structure in order to reduce the
> comparasion times when finding.
> 
> Signed-off-by: Bing Zhao <bingz@mellanox.com>
Acked-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>

[dpdk-dev] [PATCH v2 0/3] Reorganize resources of flow tables

[Bing Zhao]

From: Bing Zhao <bingz@mtbc-r640-01.mtbc.labs.mlnx>

Number of flow tables is limited by the memory resource, and the index could be
to as large as 2^^32 - 1. In the past, the flow tables are organized by arrays,
and this organization has some advantages and disadvantages. The lookup for the
table resource from a linear array is quite fast, the ID could be used as the
index in the array. But it will cost some extra memory resource after system
bring up and if only a small number of tables are created. In the meanwhile,
since we could not create the array with a huge number, so the maximal index of
the table is limited and it is  unreasonable.
If we change the array into some other tables, like some open addressing hash
table, the static memory cost is still to huge. But the index of the table
limitation could be get rid of. But in the meanwhile, it will introduce some
new issue that two tables with different ID may generate the same address index
in the table. Then it will degrade the performance of the lookup, creating and
deleting. Moreover, sometimes it will cause a failure if the collisions rate
are too heavy.
Then the simple hash list is used as the first step to get rid of this
limitations. The only static memory over head is array of the LIST HEADs. In
the next step, we could use some extendable hash tables for this. This will of
course introduce some performance degradation when lookup, creating and
removing tables in the lists if there are a lot of tables created in the
system. We need to trade off among the functionality, memory and performance.
Some other resources are associated with each flow tables and not global, like
flow matchers and jump table object used by driver. They could also be
reorganized and put into the flow table resources structure. Then the lookup
process of these resources will be speeded up significantly.

Bing Zhao (3):
  net/mlx5: reorganize flow tables with hash list
  net/mlx5: reorganize jump table resources
  net/mlx5: reorganize flow matcher resources

 drivers/net/mlx5/mlx5.c         |  16 ++
 drivers/net/mlx5/mlx5.h         |  25 ++--
 drivers/net/mlx5/mlx5_flow.h    |  24 ++-
 drivers/net/mlx5/mlx5_flow_dv.c | 316 +++++++++++++++++++++++-----------------
 4 files changed, 230 insertions(+), 151 deletions(-)

-- 
1.8.3.1

[dpdk-dev] [PATCH v2 1/3] net/mlx5: reorganize flow tables with hash list

[Bing Zhao]

From: Bing Zhao <bingz@mellanox.com>

In the current flow tables organization, arrays are used. This is
fast for searching, creating related object that will be used in
flow creation. But it introduces some limitation to the table index.
Then we can reorganize the flow tables information with hash list.
When using hash list, there is no need to maintain three arrays for
NIC TX, RX and FDB tables object information.
This attribute could be used together with the table ID to generate
a 64-bits key that is unique for the hash list insertion, lookup and
deletion.

Signed-off-by: Bing Zhao <bingz@mellanox.com>
Signed-off-by: Bing Zhao <bingz@mtbc-r640-01.mtbc.labs.mlnx>
---
 drivers/net/mlx5/mlx5.c         |  16 ++++
 drivers/net/mlx5/mlx5.h         |  23 +++--
 drivers/net/mlx5/mlx5_flow.h    |   8 ++
 drivers/net/mlx5/mlx5_flow_dv.c | 185 +++++++++++++++++++++++++++-------------
 4 files changed, 165 insertions(+), 67 deletions(-)

diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index 9a2c711..ebee6c8 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -184,6 +184,8 @@ struct mlx5_dev_spawn_data {
 #define MLX5_FLOW_MIN_ID_POOL_SIZE 512
 #define MLX5_ID_GENERATION_ARRAY_FACTOR 16
 
+#define MLX5_FLOW_TABLE_HLIST_ARRAY_SIZE 4096
+
 /**
  * Allocate ID pool structure.
  *
@@ -677,6 +679,7 @@ struct mlx5_flow_id_pool *
 	struct mlx5_ibv_shared *sh = priv->sh;
 	int err = 0;
 	void *domain;
+	char s[MLX5_HLIST_NAMESIZE];
 
 	assert(sh);
 	if (sh->dv_refcnt) {
@@ -716,6 +719,14 @@ struct mlx5_flow_id_pool *
 		sh->esw_drop_action = mlx5_glue->dr_create_flow_action_drop();
 	}
 #endif
+	snprintf(s, sizeof(s) - 1, "%s_flow_table", priv->sh->ibdev_name);
+	sh->flow_tbls = mlx5_hlist_create(s,
+					  MLX5_FLOW_TABLE_HLIST_ARRAY_SIZE);
+	if (!sh->flow_tbls) {
+		DRV_LOG(ERR, "flow tables with hash creation failed.\n");
+		err = -ENOMEM;
+		goto error;
+	}
 	sh->pop_vlan_action = mlx5_glue->dr_create_flow_action_pop_vlan();
 	sh->dv_refcnt++;
 	priv->dr_shared = 1;
@@ -770,6 +781,11 @@ struct mlx5_flow_id_pool *
 	assert(sh->dv_refcnt);
 	if (sh->dv_refcnt && --sh->dv_refcnt)
 		return;
+	if (sh->flow_tbls) {
+		/* flow table entries should be handled properly before. */
+		mlx5_hlist_destroy(sh->flow_tbls, NULL, NULL);
+		sh->flow_tbls = NULL;
+	}
 	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 e8148ce..2f4ccb7 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -571,10 +571,24 @@ struct mlx5_ibv_shared_port {
 	 */
 };
 
+/* Table key of the hash organization. */
+union mlx5_flow_tbl_key {
+	struct {
+		/* Table ID should be at the lowest address. */
+		uint32_t table_id;	/**< ID of the table. */
+		uint16_t reserved;	/**< must be zero for comparison. */
+		uint8_t domain;		/**< 1 - FDB, 0 - NIC TX/RX. */
+		uint8_t direction;	/**< 1 - egress, 0 - ingress. */
+	};
+	uint64_t v64;			/**< full 64bits value of key */
+};
+
 /* Table structure. */
 struct mlx5_flow_tbl_resource {
 	void *obj; /**< Pointer to DR table object. */
+#ifdef HAVE_MLX5DV_DR
 	rte_atomic32_t refcnt; /**< Reference counter. */
+#endif
 };
 
 #define MLX5_MAX_TABLES UINT16_MAX
@@ -644,23 +658,18 @@ struct mlx5_ibv_shared {
 	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. */
-	struct mlx5_flow_tbl_resource fdb_tbl[MLX5_MAX_TABLES_FDB];
-	/* FDB Direct Rules tables. */
 	struct mlx5_flow_tbl_resource *fdb_mtr_sfx_tbl;
 	/* FDB meter suffix rules table. */
 	void *rx_domain; /* RX Direct Rules name space handle. */
-	struct mlx5_flow_tbl_resource rx_tbl[MLX5_MAX_TABLES];
-	/* RX Direct Rules tables. */
 	struct mlx5_flow_tbl_resource *rx_mtr_sfx_tbl;
 	/* RX meter suffix rules table. */
 	void *tx_domain; /* TX Direct Rules name space handle. */
-	struct mlx5_flow_tbl_resource tx_tbl[MLX5_MAX_TABLES];
-	/* TX Direct Rules tables. */
 	struct mlx5_flow_tbl_resource *tx_mtr_sfx_tbl;
 	/* TX meter suffix rules table. */
+	struct mlx5_hlist *flow_tbls;
+	/* Direct Rules tables for FDB, NIC TX+RX */
 	void *esw_drop_action; /* Pointer to DR E-Switch drop action. */
 	void *pop_vlan_action; /* Pointer to DR pop VLAN action. */
-	/* TX Direct Rules tables/ */
 	LIST_HEAD(matchers, mlx5_flow_dv_matcher) matchers;
 	LIST_HEAD(encap_decap, mlx5_flow_dv_encap_decap_resource) encaps_decaps;
 	LIST_HEAD(modify_cmd, mlx5_flow_dv_modify_hdr_resource) modify_cmds;
diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index 875947c..c0fc357 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -437,6 +437,14 @@ struct mlx5_flow_mreg_copy_resource {
 	struct rte_flow *flow; /* Built flow for copy. */
 };
 
+/* Table data structure of the hash organization. */
+struct mlx5_flow_tbl_data_entry {
+	struct mlx5_hlist_entry entry;
+	/**< flow table resource, better to locate at the beginning. */
+	struct mlx5_flow_tbl_resource tbl;
+	/**< flow table resource, better to locate at the beginning. */
+};
+
 /*
  * Max number of actions per DV flow.
  * See CREATE_FLOW_MAX_FLOW_ACTIONS_SUPPORTED
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 3e5717e..a2963de 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -2209,7 +2209,7 @@ struct field_modify_info modify_tcp[] = {
 /**
  * Find existing encap/decap resource or create and register a new one.
  *
- * @param dev[in, out]
+ * @param[in, out] dev
  *   Pointer to rte_eth_dev structure.
  * @param[in, out] resource
  *   Pointer to encap/decap resource.
@@ -2290,7 +2290,7 @@ struct field_modify_info modify_tcp[] = {
 /**
  * Find existing table jump resource or create and register a new one.
  *
- * @param dev[in, out]
+ * @param[in, out] dev
  *   Pointer to rte_eth_dev structure.
  * @param[in, out] resource
  *   Pointer to jump table resource.
@@ -2353,7 +2353,7 @@ struct field_modify_info modify_tcp[] = {
 /**
  * Find existing table port ID resource or create and register a new one.
  *
- * @param dev[in, out]
+ * @param[in, out] dev
  *   Pointer to rte_eth_dev structure.
  * @param[in, out] resource
  *   Pointer to port ID action resource.
@@ -2417,7 +2417,7 @@ struct field_modify_info modify_tcp[] = {
 /**
  * Find existing push vlan resource or create and register a new one.
  *
- * @param dev[in, out]
+ * @param [in, out] dev
  *   Pointer to rte_eth_dev structure.
  * @param[in, out] resource
  *   Pointer to port ID action resource.
@@ -3255,8 +3255,6 @@ struct field_modify_info modify_tcp[] = {
 			     const struct rte_flow_attr *attributes,
 			     bool external, struct rte_flow_error *error)
 {
-	uint32_t max_group = attributes->transfer ? MLX5_MAX_TABLES_FDB :
-						    MLX5_MAX_TABLES;
 	uint32_t target_group, table;
 	int ret = 0;
 
@@ -3280,10 +3278,6 @@ struct field_modify_info modify_tcp[] = {
 				       &table, error);
 	if (ret)
 		return ret;
-	if (table >= max_group)
-		return rte_flow_error_set(error, EINVAL,
-					  RTE_FLOW_ERROR_TYPE_ATTR_GROUP, NULL,
-					  "target group index out of range");
 	if (attributes->group == target_group)
 		return rte_flow_error_set(error, EINVAL,
 					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
@@ -4172,11 +4166,6 @@ struct field_modify_info modify_tcp[] = {
 					  NULL,
 					  "groups are not supported");
 #else
-	uint32_t max_group = attributes->transfer ?
-			     MLX5_MAX_TABLES_FDB :
-				external ?
-				MLX5_MAX_TABLES_EXTERNAL :
-				MLX5_MAX_TABLES;
 	uint32_t table;
 	int ret;
 
@@ -4185,10 +4174,6 @@ struct field_modify_info modify_tcp[] = {
 				       &table, error);
 	if (ret)
 		return ret;
-	if (table >= max_group)
-		return rte_flow_error_set(error, EINVAL,
-					  RTE_FLOW_ERROR_TYPE_ATTR_GROUP, NULL,
-					  "group index out of range");
 #endif
 	if (attributes->priority != MLX5_FLOW_PRIO_RSVD &&
 	    attributes->priority >= priority_max)
@@ -6172,7 +6157,7 @@ struct field_modify_info modify_tcp[] = {
 /**
  * Get a flow table.
  *
- * @param dev[in, out]
+ * @param[in, out] dev
  *   Pointer to rte_eth_dev structure.
  * @param[in] table_id
  *   Table id to use.
@@ -6195,47 +6180,100 @@ struct field_modify_info modify_tcp[] = {
 	struct mlx5_priv *priv = dev->data->dev_private;
 	struct mlx5_ibv_shared *sh = priv->sh;
 	struct mlx5_flow_tbl_resource *tbl;
+	union mlx5_flow_tbl_key table_key = {
+		{
+			.table_id = table_id,
+			.reserved = 0,
+			.domain = !!transfer,
+			.direction = !!egress,
+		}
+	};
+	struct mlx5_hlist_entry *pos;
+	struct mlx5_flow_tbl_data_entry *tbl_data;
 
 #ifdef HAVE_MLX5DV_DR
-	if (transfer) {
-		tbl = &sh->fdb_tbl[table_id];
-		if (!tbl->obj)
-			tbl->obj = mlx5_glue->dr_create_flow_tbl
-				(sh->fdb_domain, table_id);
-	} else if (egress) {
-		tbl = &sh->tx_tbl[table_id];
-		if (!tbl->obj)
-			tbl->obj = mlx5_glue->dr_create_flow_tbl
-				(sh->tx_domain, table_id);
-	} else {
-		tbl = &sh->rx_tbl[table_id];
-		if (!tbl->obj)
-			tbl->obj = mlx5_glue->dr_create_flow_tbl
-				(sh->rx_domain, table_id);
+	int ret;
+	void *domain;
+
+	pos = mlx5_hlist_lookup(sh->flow_tbls, table_key.v64);
+	if (pos) {
+		tbl_data = container_of(pos, struct mlx5_flow_tbl_data_entry,
+					entry);
+		tbl = &tbl_data->tbl;
+		if (!tbl->obj) {
+			rte_flow_error_set(error, ENOKEY,
+					   RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
+					   NULL, "cannot find created table");
+			return NULL;
+		}
+		rte_atomic32_inc(&tbl->refcnt);
+		return tbl;
 	}
+	tbl_data = rte_zmalloc(NULL, sizeof(*tbl_data), 0);
+	if (!tbl_data) {
+		rte_flow_error_set(error, ENOMEM,
+				   RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
+				   NULL,
+				   "cannot allocate flow table data entry");
+		return NULL;
+	}
+	tbl = &tbl_data->tbl;
+	pos = &tbl_data->entry;
+	if (transfer)
+		domain = sh->fdb_domain;
+	else if (egress)
+		domain = sh->tx_domain;
+	else
+		domain = sh->rx_domain;
+	tbl->obj = mlx5_glue->dr_create_flow_tbl(domain, table_id);
 	if (!tbl->obj) {
 		rte_flow_error_set(error, ENOMEM,
 				   RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
-				   NULL, "cannot create table");
+				   NULL, "cannot create flow table object");
+		rte_free(tbl_data);
 		return NULL;
 	}
+	/*
+	 * No multi-threads now, but still better to initialize the reference
+	 * count before insert it into the hash list.
+	 */
+	rte_atomic32_init(&tbl->refcnt);
+	pos->key = table_key.v64;
+	ret = mlx5_hlist_insert(sh->flow_tbls, pos);
+	if (ret < 0) {
+		rte_flow_error_set(error, -ret,
+				   RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
+				   "cannot insert flow table data entry");
+		mlx5_glue->dr_destroy_flow_tbl(tbl->obj);
+		rte_free(tbl_data);
+	}
 	rte_atomic32_inc(&tbl->refcnt);
 	return tbl;
 #else
-	(void)error;
-	(void)tbl;
-	if (transfer)
-		return &sh->fdb_tbl[table_id];
-	else if (egress)
-		return &sh->tx_tbl[table_id];
-	else
-		return &sh->rx_tbl[table_id];
+	/* Just to make the compiling pass when no HAVE_MLX5DV_DR defined. */
+	pos = mlx5_hlist_lookup(sh->flow_tbls, table_key.v64);
+	if (pos) {
+		tbl_data = container_of(pos, struct mlx5_flow_tbl_data_entry,
+					entry);
+		tbl = &tbl_data->tbl;
+		if (!tbl->obj) {
+			rte_flow_error_set(error, ENOKEY,
+					   RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
+					   NULL, "cannot find created table");
+			return NULL;
+		}
+		rte_atomic32_inc(&tbl->refcnt);
+		return tbl;
+	}
+	return NULL;
 #endif
 }
 
 /**
  * Release a flow table.
  *
+ * @param[in] dev
+ *   Pointer to rte_eth_dev structure.
  * @param[in] tbl
  *   Table resource to be released.
  *
@@ -6243,13 +6281,24 @@ struct field_modify_info modify_tcp[] = {
  *   Returns 0 if table was released, else return 1;
  */
 static int
-flow_dv_tbl_resource_release(struct mlx5_flow_tbl_resource *tbl)
+flow_dv_tbl_resource_release(struct rte_eth_dev *dev,
+			     struct mlx5_flow_tbl_resource *tbl)
 {
+	struct mlx5_priv *priv = dev->data->dev_private;
+	struct mlx5_ibv_shared *sh = priv->sh;
+	struct mlx5_flow_tbl_data_entry *tbl_data =
+		container_of(tbl, struct mlx5_flow_tbl_data_entry, tbl);
+
 	if (!tbl)
 		return 0;
 	if (rte_atomic32_dec_and_test(&tbl->refcnt)) {
+		struct mlx5_hlist_entry *pos = &tbl_data->entry;
+
 		mlx5_glue->dr_destroy_flow_tbl(tbl->obj);
 		tbl->obj = NULL;
+		/* remove the entry from the hash list and free memory. */
+		mlx5_hlist_remove(sh->flow_tbls, pos);
+		rte_free(tbl_data);
 		return 0;
 	}
 	return 1;
@@ -6258,7 +6307,7 @@ struct field_modify_info modify_tcp[] = {
 /**
  * Register the flow matcher.
  *
- * @param dev[in, out]
+ * @param[in, out] dev
  *   Pointer to rte_eth_dev structure.
  * @param[in, out] matcher
  *   Pointer to flow matcher.
@@ -6332,7 +6381,7 @@ struct field_modify_info modify_tcp[] = {
 	if (!cache_matcher->matcher_object) {
 		rte_free(cache_matcher);
 #ifdef HAVE_MLX5DV_DR
-		flow_dv_tbl_resource_release(tbl);
+		flow_dv_tbl_resource_release(dev, tbl);
 #endif
 		return rte_flow_error_set(error, ENOMEM,
 					  RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
@@ -6867,7 +6916,7 @@ struct field_modify_info modify_tcp[] = {
 			jump_tbl_resource.tbl = tbl;
 			if (flow_dv_jump_tbl_resource_register
 			    (dev, &jump_tbl_resource, dev_flow, error)) {
-				flow_dv_tbl_resource_release(tbl);
+				flow_dv_tbl_resource_release(dev, tbl);
 				return rte_flow_error_set
 						(error, errno,
 						 RTE_FLOW_ERROR_TYPE_ACTION,
@@ -7349,21 +7398,31 @@ struct field_modify_info modify_tcp[] = {
 	struct mlx5_flow_dv_matcher *matcher = flow->dv.matcher;
 	struct mlx5_priv *priv = dev->data->dev_private;
 	struct mlx5_ibv_shared *sh = priv->sh;
-	struct mlx5_flow_tbl_resource *tbl;
+	struct mlx5_flow_tbl_data_entry *tbl_data;
 
 	assert(matcher->matcher_object);
 	DRV_LOG(DEBUG, "port %u matcher %p: refcnt %d--",
 		dev->data->port_id, (void *)matcher,
 		rte_atomic32_read(&matcher->refcnt));
 	if (rte_atomic32_dec_and_test(&matcher->refcnt)) {
+		struct mlx5_hlist_entry *pos;
+		union mlx5_flow_tbl_key table_key = {
+			{
+				.table_id = matcher->group,
+				.reserved = 0,
+				.domain = !!matcher->transfer,
+				.direction = !!matcher->egress,
+			}
+		};
 		claim_zero(mlx5_glue->dv_destroy_flow_matcher
 			   (matcher->matcher_object));
 		LIST_REMOVE(matcher, next);
-		if (matcher->egress)
-			tbl = &sh->tx_tbl[matcher->group];
-		else
-			tbl = &sh->rx_tbl[matcher->group];
-		flow_dv_tbl_resource_release(tbl);
+		pos = mlx5_hlist_lookup(sh->flow_tbls, table_key.v64);
+		if (pos) {
+			tbl_data = container_of(pos,
+				struct mlx5_flow_tbl_data_entry, entry);
+			flow_dv_tbl_resource_release(dev, &tbl_data->tbl);
+		}
 		rte_free(matcher);
 		DRV_LOG(DEBUG, "port %u matcher %p: removed",
 			dev->data->port_id, (void *)matcher);
@@ -7406,6 +7465,8 @@ struct field_modify_info modify_tcp[] = {
 /**
  * Release an jump to table action resource.
  *
+ * @param dev
+ *   Pointer to Ethernet device.
  * @param flow
  *   Pointer to mlx5_flow.
  *
@@ -7413,7 +7474,8 @@ struct field_modify_info modify_tcp[] = {
  *   1 while a reference on it exists, 0 when freed.
  */
 static int
-flow_dv_jump_tbl_resource_release(struct mlx5_flow *flow)
+flow_dv_jump_tbl_resource_release(struct rte_eth_dev *dev,
+				  struct mlx5_flow *flow)
 {
 	struct mlx5_flow_dv_jump_tbl_resource *cache_resource =
 						flow->dv.jump;
@@ -7426,7 +7488,7 @@ struct field_modify_info modify_tcp[] = {
 		claim_zero(mlx5_glue->destroy_flow_action
 				(cache_resource->action));
 		LIST_REMOVE(cache_resource, next);
-		flow_dv_tbl_resource_release(cache_resource->tbl);
+		flow_dv_tbl_resource_release(dev, cache_resource->tbl);
 		rte_free(cache_resource);
 		DRV_LOG(DEBUG, "jump table resource %p: removed",
 			(void *)cache_resource);
@@ -7599,7 +7661,7 @@ struct field_modify_info modify_tcp[] = {
 		if (dev_flow->dv.modify_hdr)
 			flow_dv_modify_hdr_resource_release(dev_flow);
 		if (dev_flow->dv.jump)
-			flow_dv_jump_tbl_resource_release(dev_flow);
+			flow_dv_jump_tbl_resource_release(dev, dev_flow);
 		if (dev_flow->dv.port_id_action)
 			flow_dv_port_id_action_resource_release(dev_flow);
 		if (dev_flow->dv.push_vlan_res)
@@ -7732,7 +7794,8 @@ struct field_modify_info modify_tcp[] = {
 		claim_zero(mlx5_glue->dv_destroy_flow_matcher
 			  (mtd->egress.any_matcher));
 	if (mtd->egress.tbl)
-		claim_zero(flow_dv_tbl_resource_release(mtd->egress.tbl));
+		claim_zero(flow_dv_tbl_resource_release(dev,
+							mtd->egress.tbl));
 	if (mtd->ingress.color_matcher)
 		claim_zero(mlx5_glue->dv_destroy_flow_matcher
 			  (mtd->ingress.color_matcher));
@@ -7740,7 +7803,8 @@ struct field_modify_info modify_tcp[] = {
 		claim_zero(mlx5_glue->dv_destroy_flow_matcher
 			  (mtd->ingress.any_matcher));
 	if (mtd->ingress.tbl)
-		claim_zero(flow_dv_tbl_resource_release(mtd->ingress.tbl));
+		claim_zero(flow_dv_tbl_resource_release(dev,
+							mtd->ingress.tbl));
 	if (mtd->transfer.color_matcher)
 		claim_zero(mlx5_glue->dv_destroy_flow_matcher
 			  (mtd->transfer.color_matcher));
@@ -7748,7 +7812,8 @@ struct field_modify_info modify_tcp[] = {
 		claim_zero(mlx5_glue->dv_destroy_flow_matcher
 			  (mtd->transfer.any_matcher));
 	if (mtd->transfer.tbl)
-		claim_zero(flow_dv_tbl_resource_release(mtd->transfer.tbl));
+		claim_zero(flow_dv_tbl_resource_release(dev,
+							mtd->transfer.tbl));
 	if (mtd->drop_actn)
 		claim_zero(mlx5_glue->destroy_flow_action(mtd->drop_actn));
 	rte_free(mtd);
-- 
1.8.3.1

[dpdk-dev] [PATCH v2 2/3] net/mlx5: reorganize jump table resources

[Bing Zhao]

From: Bing Zhao <bingz@mellanox.com>

Jump object is associated with table object, so there is no need to
use a single linked list to store it. All the jump objects could be
put together with related flow tables.

Signed-off-by: Bing Zhao <bingz@mellanox.com>
Signed-off-by: Bing Zhao <bingz@mtbc-r640-01.mtbc.labs.mlnx>
---
 drivers/net/mlx5/mlx5.h         |  1 -
 drivers/net/mlx5/mlx5_flow.h    |  7 ++--
 drivers/net/mlx5/mlx5_flow_dv.c | 82 +++++++++++++++++------------------------
 3 files changed, 37 insertions(+), 53 deletions(-)

diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index 2f4ccb7..021c9db 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -674,7 +674,6 @@ struct mlx5_ibv_shared {
 	LIST_HEAD(encap_decap, mlx5_flow_dv_encap_decap_resource) encaps_decaps;
 	LIST_HEAD(modify_cmd, mlx5_flow_dv_modify_hdr_resource) modify_cmds;
 	LIST_HEAD(tag, mlx5_flow_dv_tag_resource) tags;
-	LIST_HEAD(jump, mlx5_flow_dv_jump_tbl_resource) jump_tbl;
 	LIST_HEAD(port_id_action_list, mlx5_flow_dv_port_id_action_resource)
 		port_id_action_list; /* List of port ID actions. */
 	LIST_HEAD(push_vlan_action_list, mlx5_flow_dv_push_vlan_action_resource)
diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index c0fc357..7bcdd5f 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -394,12 +394,9 @@ struct mlx5_flow_dv_modify_hdr_resource {
 
 /* Jump action resource structure. */
 struct mlx5_flow_dv_jump_tbl_resource {
-	LIST_ENTRY(mlx5_flow_dv_jump_tbl_resource) next;
-	/* Pointer to next element. */
 	rte_atomic32_t refcnt; /**< Reference counter. */
-	void *action; /**< Pointer to the rdma core action. */
 	uint8_t ft_type; /**< Flow table type, Rx or Tx. */
-	struct mlx5_flow_tbl_resource *tbl; /**< The target table. */
+	void *action; /**< Pointer to the rdma core action. */
 };
 
 /* Port ID resource structure. */
@@ -443,6 +440,8 @@ struct mlx5_flow_tbl_data_entry {
 	/**< flow table resource, better to locate at the beginning. */
 	struct mlx5_flow_tbl_resource tbl;
 	/**< flow table resource, better to locate at the beginning. */
+	struct mlx5_flow_dv_jump_tbl_resource jump;
+	/**< jump resource, at most one for each table created. */
 };
 
 /*
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index a2963de..d3faac3 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -2292,8 +2292,8 @@ struct field_modify_info modify_tcp[] = {
  *
  * @param[in, out] dev
  *   Pointer to rte_eth_dev structure.
- * @param[in, out] resource
- *   Pointer to jump table resource.
+ * @param[in, out] tbl
+ *   Pointer to flow table resource.
  * @parm[in, out] dev_flow
  *   Pointer to the dev_flow.
  * @param[out] error
@@ -2304,49 +2304,34 @@ struct field_modify_info modify_tcp[] = {
  */
 static int
 flow_dv_jump_tbl_resource_register
-			(struct rte_eth_dev *dev,
-			 struct mlx5_flow_dv_jump_tbl_resource *resource,
+			(struct rte_eth_dev *dev __rte_unused,
+			 struct mlx5_flow_tbl_resource *tbl,
 			 struct mlx5_flow *dev_flow,
 			 struct rte_flow_error *error)
 {
-	struct mlx5_priv *priv = dev->data->dev_private;
-	struct mlx5_ibv_shared *sh = priv->sh;
-	struct mlx5_flow_dv_jump_tbl_resource *cache_resource;
+	struct mlx5_flow_tbl_data_entry *tbl_data =
+		container_of(tbl, struct mlx5_flow_tbl_data_entry, tbl);
+	int cnt;
 
-	/* Lookup a matching resource from cache. */
-	LIST_FOREACH(cache_resource, &sh->jump_tbl, next) {
-		if (resource->tbl == cache_resource->tbl) {
-			DRV_LOG(DEBUG, "jump table resource resource %p: refcnt %d++",
-				(void *)cache_resource,
-				rte_atomic32_read(&cache_resource->refcnt));
-			rte_atomic32_inc(&cache_resource->refcnt);
-			dev_flow->dv.jump = cache_resource;
-			return 0;
-		}
-	}
-	/* Register new jump table resource. */
-	cache_resource = rte_calloc(__func__, 1, sizeof(*cache_resource), 0);
-	if (!cache_resource)
-		return rte_flow_error_set(error, ENOMEM,
-					  RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
-					  "cannot allocate resource memory");
-	*cache_resource = *resource;
-	cache_resource->action =
-		mlx5_glue->dr_create_flow_action_dest_flow_tbl
-		(resource->tbl->obj);
-	if (!cache_resource->action) {
-		rte_free(cache_resource);
-		return rte_flow_error_set(error, ENOMEM,
-					  RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
-					  NULL, "cannot create action");
+	assert(tbl);
+	cnt = rte_atomic32_read(&tbl_data->jump.refcnt);
+	if (!cnt) {
+		tbl_data->jump.action =
+			mlx5_glue->dr_create_flow_action_dest_flow_tbl
+			(tbl->obj);
+		if (!tbl_data->jump.action)
+			return rte_flow_error_set(error, ENOMEM,
+					RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
+					NULL, "cannot create jump action");
+		DRV_LOG(DEBUG, "new jump table resource %p: refcnt %d++",
+			(void *)&tbl_data->jump, cnt);
+	} else {
+		assert(tbl_data->jump.action);
+		DRV_LOG(DEBUG, "existed jump table resource %p: refcnt %d++",
+			(void *)&tbl_data->jump, cnt);
 	}
-	rte_atomic32_init(&cache_resource->refcnt);
-	rte_atomic32_inc(&cache_resource->refcnt);
-	LIST_INSERT_HEAD(&sh->jump_tbl, cache_resource, next);
-	dev_flow->dv.jump = cache_resource;
-	DRV_LOG(DEBUG, "new jump table  resource %p: refcnt %d++",
-		(void *)cache_resource,
-		rte_atomic32_read(&cache_resource->refcnt));
+	rte_atomic32_inc(&tbl_data->jump.refcnt);
+	dev_flow->dv.jump = &tbl_data->jump;
 	return 0;
 }
 
@@ -6238,6 +6223,8 @@ struct field_modify_info modify_tcp[] = {
 	 * count before insert it into the hash list.
 	 */
 	rte_atomic32_init(&tbl->refcnt);
+	/* Jump action reference count is initialized here. */
+	rte_atomic32_init(&tbl_data->jump.refcnt);
 	pos->key = table_key.v64;
 	ret = mlx5_hlist_insert(sh->flow_tbls, pos);
 	if (ret < 0) {
@@ -6648,7 +6635,6 @@ struct field_modify_info modify_tcp[] = {
 		const uint8_t *rss_key;
 		const struct rte_flow_action_jump *jump_data;
 		const struct rte_flow_action_meter *mtr;
-		struct mlx5_flow_dv_jump_tbl_resource jump_tbl_resource;
 		struct mlx5_flow_tbl_resource *tbl;
 		uint32_t port_id = 0;
 		struct mlx5_flow_dv_port_id_action_resource port_id_resource;
@@ -6913,9 +6899,8 @@ struct field_modify_info modify_tcp[] = {
 						 RTE_FLOW_ERROR_TYPE_ACTION,
 						 NULL,
 						 "cannot create jump action.");
-			jump_tbl_resource.tbl = tbl;
 			if (flow_dv_jump_tbl_resource_register
-			    (dev, &jump_tbl_resource, dev_flow, error)) {
+			    (dev, tbl, dev_flow, error)) {
 				flow_dv_tbl_resource_release(dev, tbl);
 				return rte_flow_error_set
 						(error, errno,
@@ -7477,8 +7462,10 @@ struct field_modify_info modify_tcp[] = {
 flow_dv_jump_tbl_resource_release(struct rte_eth_dev *dev,
 				  struct mlx5_flow *flow)
 {
-	struct mlx5_flow_dv_jump_tbl_resource *cache_resource =
-						flow->dv.jump;
+	struct mlx5_flow_dv_jump_tbl_resource *cache_resource = flow->dv.jump;
+	struct mlx5_flow_tbl_data_entry *tbl_data =
+			container_of(cache_resource,
+				     struct mlx5_flow_tbl_data_entry, jump);
 
 	assert(cache_resource->action);
 	DRV_LOG(DEBUG, "jump table resource %p: refcnt %d--",
@@ -7487,9 +7474,8 @@ struct field_modify_info modify_tcp[] = {
 	if (rte_atomic32_dec_and_test(&cache_resource->refcnt)) {
 		claim_zero(mlx5_glue->destroy_flow_action
 				(cache_resource->action));
-		LIST_REMOVE(cache_resource, next);
-		flow_dv_tbl_resource_release(dev, cache_resource->tbl);
-		rte_free(cache_resource);
+		/* jump action memory free is inside the table release. */
+		flow_dv_tbl_resource_release(dev, &tbl_data->tbl);
 		DRV_LOG(DEBUG, "jump table resource %p: removed",
 			(void *)cache_resource);
 		return 0;
-- 
1.8.3.1

[dpdk-dev] [PATCH v2 3/3] net/mlx5: reorganize flow matcher resources

[Bing Zhao]

From: Bing Zhao <bingz@mellanox.com>

Matchers are created on the specific table. If a single linked list
is used to store these, then the finding process might be the
bottleneck when there are a lot of different flow matchers on a
huge amount of tables. The matchers could be move into the table
data resource structure in order to reduce the comparasion times
when finding.

Signed-off-by: Bing Zhao <bingz@mellanox.com>
Signed-off-by: Bing Zhao <bingz@mtbc-r640-01.mtbc.labs.mlnx>
---
 drivers/net/mlx5/mlx5.h         |  1 -
 drivers/net/mlx5/mlx5_flow.h    | 13 ++++---
 drivers/net/mlx5/mlx5_flow_dv.c | 83 ++++++++++++++++++++---------------------
 3 files changed, 47 insertions(+), 50 deletions(-)

diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index 021c9db..f47b8cf 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -670,7 +670,6 @@ struct mlx5_ibv_shared {
 	/* Direct Rules tables for FDB, NIC TX+RX */
 	void *esw_drop_action; /* Pointer to DR E-Switch drop action. */
 	void *pop_vlan_action; /* Pointer to DR pop VLAN action. */
-	LIST_HEAD(matchers, mlx5_flow_dv_matcher) matchers;
 	LIST_HEAD(encap_decap, mlx5_flow_dv_encap_decap_resource) encaps_decaps;
 	LIST_HEAD(modify_cmd, mlx5_flow_dv_modify_hdr_resource) modify_cmds;
 	LIST_HEAD(tag, mlx5_flow_dv_tag_resource) tags;
diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index 7bcdd5f..26c6848 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -333,14 +333,13 @@ struct mlx5_flow_dv_match_params {
 /* Matcher structure. */
 struct mlx5_flow_dv_matcher {
 	LIST_ENTRY(mlx5_flow_dv_matcher) next;
-	/* Pointer to the next element. */
+	/**< Pointer to the next element. */
+	struct mlx5_flow_tbl_resource *tbl;
+	/**< Pointer to the table(group) the matcher associated with. */
 	rte_atomic32_t refcnt; /**< Reference counter. */
 	void *matcher_object; /**< Pointer to DV matcher */
 	uint16_t crc; /**< CRC of key. */
 	uint16_t priority; /**< Priority of matcher. */
-	uint8_t egress; /**< Egress matcher. */
-	uint8_t transfer; /**< 1 if the flow is E-Switch flow. */
-	uint32_t group; /**< The matcher group. */
 	struct mlx5_flow_dv_match_params mask; /**< Matcher mask. */
 };
 
@@ -437,9 +436,11 @@ struct mlx5_flow_mreg_copy_resource {
 /* Table data structure of the hash organization. */
 struct mlx5_flow_tbl_data_entry {
 	struct mlx5_hlist_entry entry;
-	/**< flow table resource, better to locate at the beginning. */
+	/**< hash list entry, 64-bits key inside. */
 	struct mlx5_flow_tbl_resource tbl;
-	/**< flow table resource, better to locate at the beginning. */
+	/**< flow table resource. */
+	LIST_HEAD(matchers, mlx5_flow_dv_matcher) matchers;
+	/**< matchers' header associated with the flow table. */
 	struct mlx5_flow_dv_jump_tbl_resource jump;
 	/**< jump resource, at most one for each table created. */
 };
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index d3faac3..3ba1e52 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -6298,6 +6298,8 @@ struct field_modify_info modify_tcp[] = {
  *   Pointer to rte_eth_dev structure.
  * @param[in, out] matcher
  *   Pointer to flow matcher.
+ * @param[in, out] key
+ *   Pointer to flow table key.
  * @parm[in, out] dev_flow
  *   Pointer to the dev_flow.
  * @param[out] error
@@ -6309,6 +6311,7 @@ struct field_modify_info modify_tcp[] = {
 static int
 flow_dv_matcher_register(struct rte_eth_dev *dev,
 			 struct mlx5_flow_dv_matcher *matcher,
+			 union mlx5_flow_tbl_key *key,
 			 struct mlx5_flow *dev_flow,
 			 struct rte_flow_error *error)
 {
@@ -6319,49 +6322,53 @@ struct field_modify_info modify_tcp[] = {
 		.type = IBV_FLOW_ATTR_NORMAL,
 		.match_mask = (void *)&matcher->mask,
 	};
-	struct mlx5_flow_tbl_resource *tbl = NULL;
+	struct mlx5_flow_tbl_resource *tbl;
+	struct mlx5_flow_tbl_data_entry *tbl_data;
 
+	tbl = flow_dv_tbl_resource_get(dev, key->table_id, key->direction,
+				       key->domain, error);
+	if (!tbl)
+		return -rte_errno;	/* No need to refill the error info */
+	tbl_data = container_of(tbl, struct mlx5_flow_tbl_data_entry, tbl);
 	/* Lookup from cache. */
-	LIST_FOREACH(cache_matcher, &sh->matchers, next) {
+	LIST_FOREACH(cache_matcher, &tbl_data->matchers, next) {
 		if (matcher->crc == cache_matcher->crc &&
 		    matcher->priority == cache_matcher->priority &&
-		    matcher->egress == cache_matcher->egress &&
-		    matcher->group == cache_matcher->group &&
-		    matcher->transfer == cache_matcher->transfer &&
 		    !memcmp((const void *)matcher->mask.buf,
 			    (const void *)cache_matcher->mask.buf,
 			    cache_matcher->mask.size)) {
 			DRV_LOG(DEBUG,
-				"priority %hd use %s matcher %p: refcnt %d++",
+				"%s group %u priority %hd use %s "
+				"matcher %p: refcnt %d++",
+				key->domain ? "FDB" : "NIC", key->table_id,
 				cache_matcher->priority,
-				cache_matcher->egress ? "tx" : "rx",
+				key->direction ? "tx" : "rx",
 				(void *)cache_matcher,
 				rte_atomic32_read(&cache_matcher->refcnt));
 			rte_atomic32_inc(&cache_matcher->refcnt);
 			dev_flow->dv.matcher = cache_matcher;
+			/* old matcher should not make the table ref++. */
+#ifdef HAVE_MLX5DV_DR
+			flow_dv_tbl_resource_release(dev, tbl);
+#endif
 			return 0;
 		}
 	}
 	/* Register new matcher. */
 	cache_matcher = rte_calloc(__func__, 1, sizeof(*cache_matcher), 0);
-	if (!cache_matcher)
+	if (!cache_matcher) {
+#ifdef HAVE_MLX5DV_DR
+		flow_dv_tbl_resource_release(dev, tbl);
+#endif
 		return rte_flow_error_set(error, ENOMEM,
 					  RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
 					  "cannot allocate matcher memory");
-	tbl = flow_dv_tbl_resource_get(dev, matcher->group,
-				       matcher->egress, matcher->transfer,
-				       error);
-	if (!tbl) {
-		rte_free(cache_matcher);
-		return rte_flow_error_set(error, ENOMEM,
-					  RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
-					  NULL, "cannot create table");
 	}
 	*cache_matcher = *matcher;
 	dv_attr.match_criteria_enable =
 		flow_dv_matcher_enable(cache_matcher->mask.buf);
 	dv_attr.priority = matcher->priority;
-	if (matcher->egress)
+	if (key->direction)
 		dv_attr.flags |= IBV_FLOW_ATTR_FLAGS_EGRESS;
 	cache_matcher->matcher_object =
 		mlx5_glue->dv_create_flow_matcher(sh->ctx, &dv_attr, tbl->obj);
@@ -6374,14 +6381,18 @@ struct field_modify_info modify_tcp[] = {
 					  RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
 					  NULL, "cannot create matcher");
 	}
+	/* Save the table information */
+	cache_matcher->tbl = tbl;
+	rte_atomic32_init(&cache_matcher->refcnt);
+	/* only matcher ref++, table ref++ already done above in get API. */
 	rte_atomic32_inc(&cache_matcher->refcnt);
-	LIST_INSERT_HEAD(&sh->matchers, cache_matcher, next);
+	LIST_INSERT_HEAD(&tbl_data->matchers, cache_matcher, next);
 	dev_flow->dv.matcher = cache_matcher;
-	DRV_LOG(DEBUG, "priority %hd new %s matcher %p: refcnt %d",
+	DRV_LOG(DEBUG, "%s group %u priority %hd new %s matcher %p: refcnt %d",
+		key->domain ? "FDB" : "NIC", key->table_id,
 		cache_matcher->priority,
-		cache_matcher->egress ? "tx" : "rx", (void *)cache_matcher,
+		key->direction ? "tx" : "rx", (void *)cache_matcher,
 		rte_atomic32_read(&cache_matcher->refcnt));
-	rte_atomic32_inc(&tbl->refcnt);
 	return 0;
 }
 
@@ -6610,6 +6621,7 @@ struct field_modify_info modify_tcp[] = {
 	};
 	union flow_dv_attr flow_attr = { .attr = 0 };
 	struct mlx5_flow_dv_tag_resource tag_resource;
+	union mlx5_flow_tbl_key tbl_key;
 	uint32_t modify_action_position = UINT32_MAX;
 	void *match_mask = matcher.mask.buf;
 	void *match_value = dev_flow->dv.value.buf;
@@ -7242,10 +7254,11 @@ struct field_modify_info modify_tcp[] = {
 				    matcher.mask.size);
 	matcher.priority = mlx5_flow_adjust_priority(dev, priority,
 						     matcher.priority);
-	matcher.egress = attr->egress;
-	matcher.group = dev_flow->group;
-	matcher.transfer = attr->transfer;
-	if (flow_dv_matcher_register(dev, &matcher, dev_flow, error))
+	/* reserved field no needs to be set to 0 here. */
+	tbl_key.domain = attr->transfer;
+	tbl_key.direction = attr->egress;
+	tbl_key.table_id = dev_flow->group;
+	if (flow_dv_matcher_register(dev, &matcher, &tbl_key, dev_flow, error))
 		return -rte_errno;
 	return 0;
 }
@@ -7381,33 +7394,17 @@ struct field_modify_info modify_tcp[] = {
 			struct mlx5_flow *flow)
 {
 	struct mlx5_flow_dv_matcher *matcher = flow->dv.matcher;
-	struct mlx5_priv *priv = dev->data->dev_private;
-	struct mlx5_ibv_shared *sh = priv->sh;
-	struct mlx5_flow_tbl_data_entry *tbl_data;
 
 	assert(matcher->matcher_object);
 	DRV_LOG(DEBUG, "port %u matcher %p: refcnt %d--",
 		dev->data->port_id, (void *)matcher,
 		rte_atomic32_read(&matcher->refcnt));
 	if (rte_atomic32_dec_and_test(&matcher->refcnt)) {
-		struct mlx5_hlist_entry *pos;
-		union mlx5_flow_tbl_key table_key = {
-			{
-				.table_id = matcher->group,
-				.reserved = 0,
-				.domain = !!matcher->transfer,
-				.direction = !!matcher->egress,
-			}
-		};
 		claim_zero(mlx5_glue->dv_destroy_flow_matcher
 			   (matcher->matcher_object));
 		LIST_REMOVE(matcher, next);
-		pos = mlx5_hlist_lookup(sh->flow_tbls, table_key.v64);
-		if (pos) {
-			tbl_data = container_of(pos,
-				struct mlx5_flow_tbl_data_entry, entry);
-			flow_dv_tbl_resource_release(dev, &tbl_data->tbl);
-		}
+		/* table ref-- in release interface. */
+		flow_dv_tbl_resource_release(dev, matcher->tbl);
 		rte_free(matcher);
 		DRV_LOG(DEBUG, "port %u matcher %p: removed",
 			dev->data->port_id, (void *)matcher);
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH v2 0/3] Reorganize resources of flow tables

[Raslan Darawsheh]

Hi,
> -----Original Message-----
> From: Bing Zhao <bingz@mellanox.com>
> Sent: Friday, November 8, 2019 5:23 PM
> To: Slava Ovsiienko <viacheslavo@mellanox.com>
> Cc: Bing Zhao <bingz@mtbc-r640-01.mtbc.labs.mlnx>; Ori Kam
> <orika@mellanox.com>; Raslan Darawsheh <rasland@mellanox.com>;
> dev@dpdk.org
> Subject: [PATCH v2 0/3] Reorganize resources of flow tables
> 
> From: Bing Zhao <bingz@mtbc-r640-01.mtbc.labs.mlnx>
> 
> Number of flow tables is limited by the memory resource, and the index
> could be to as large as 2^^32 - 1. In the past, the flow tables are organized by
> arrays, and this organization has some advantages and disadvantages. The
> lookup for the table resource from a linear array is quite fast, the ID could be
> used as the index in the array. But it will cost some extra memory resource
> after system bring up and if only a small number of tables are created. In the
> meanwhile, since we could not create the array with a huge number, so the
> maximal index of the table is limited and it is  unreasonable.
> If we change the array into some other tables, like some open addressing
> hash table, the static memory cost is still to huge. But the index of the table
> limitation could be get rid of. But in the meanwhile, it will introduce some
> new issue that two tables with different ID may generate the same address
> index in the table. Then it will degrade the performance of the lookup,
> creating and deleting. Moreover, sometimes it will cause a failure if the
> collisions rate are too heavy.
> Then the simple hash list is used as the first step to get rid of this limitations.
> The only static memory over head is array of the LIST HEADs. In the next
> step, we could use some extendable hash tables for this. This will of course
> introduce some performance degradation when lookup, creating and
> removing tables in the lists if there are a lot of tables created in the system.
> We need to trade off among the functionality, memory and performance.
> Some other resources are associated with each flow tables and not global,
> like flow matchers and jump table object used by driver. They could also be
> reorganized and put into the flow table resources structure. Then the lookup
> process of these resources will be speeded up significantly.
> 
> Bing Zhao (3):
>   net/mlx5: reorganize flow tables with hash list
>   net/mlx5: reorganize jump table resources
>   net/mlx5: reorganize flow matcher resources
> 
>  drivers/net/mlx5/mlx5.c         |  16 ++
>  drivers/net/mlx5/mlx5.h         |  25 ++--
>  drivers/net/mlx5/mlx5_flow.h    |  24 ++-
>  drivers/net/mlx5/mlx5_flow_dv.c | 316 +++++++++++++++++++++++-------
> ----------
>  4 files changed, 230 insertions(+), 151 deletions(-)
> 
> --
> 1.8.3.1
Added missing:
Acked-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
Removed wrong Signed-of-by signature.

Series applied to next-net-mlx,

Kindest regards,
Raslan Darawsheh