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

[Bing Zhao <bingz@mellanox.com>]

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