DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH 00/10] net/mlx5: optimize flow structure
@ 2020-04-15  6:39 Suanming Mou
  2020-04-15  6:39 ` [dpdk-dev] [PATCH 01/10] net/mlx5: reorganize fate actions as union Suanming Mou
                   ` (9 more replies)
  0 siblings, 10 replies; 11+ messages in thread
From: Suanming Mou @ 2020-04-15  6:39 UTC (permalink / raw)
  Cc: wentaoc, rasland, dev

This patch set is third part of the flow memory consumption optimization.
It mainly optimizes the rte flow and mlx5 flow handle structure.

For mlx5 flow handle structure:
1. As one flow has only one fate action, the fate actions are reorganized
as union.
2. The action flags are optimized to be the limited bits the flow needs.
3. The bits members are reorganized to align with bytes.

For rte flow structure:
1. Meter handle is converted to uint32_t type as unique meter id.
2. Fdir handle is converted to one bit to save the pointer to another list.
3. Metadata handle is converted to uint32_t type as indexed.
4. The bit members are reorganized to align with bytes.

Some extra optimizations:
1. Allocate meter resource from indexed memory pool.
2. Allocate none RSS rte flow from indexed memory pool.
3. Split RSS type, level, key to common memory as they are not required
for the flow destroy.

This patch set should be applied after the patch series as below:
https://patches.dpdk.org/cover/68226/

Suanming Mou (9):
  net/mlx5: reorganize fate actions as union
  net/mlx5: optimize action flags in flow handle
  net/mlx5: reorganize the mlx5 flow handle struct
  net/mlx5: optimize mlx5 flow RSS struct
  net/mlx5: allocate rte flow from indexed pool
  net/mlx5: optimize flow meter handle type
  net/mlx5: allocate meter from indexed pool
  net/mlx5: allocate metadata object from indexed pool
  net/mlx5: reorganize rte flow structure

Wentao Cui (1):
  net/mlx5: optimize flow director filter memory

 drivers/net/mlx5/mlx5.c            |  35 +++++++-
 drivers/net/mlx5/mlx5.h            |   5 ++
 drivers/net/mlx5/mlx5_flow.c       | 168 +++++++++++++++++++++++++++----------
 drivers/net/mlx5/mlx5_flow.h       |  83 +++++++++++-------
 drivers/net/mlx5/mlx5_flow_dv.c    | 135 ++++++++++++++++++-----------
 drivers/net/mlx5/mlx5_flow_meter.c |  11 +--
 drivers/net/mlx5/mlx5_flow_verbs.c |  91 +++++++++++---------
 7 files changed, 363 insertions(+), 165 deletions(-)

-- 
1.8.3.1


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

* [dpdk-dev] [PATCH 01/10] net/mlx5: reorganize fate actions as union
  2020-04-15  6:39 [dpdk-dev] [PATCH 00/10] net/mlx5: optimize flow structure Suanming Mou
@ 2020-04-15  6:39 ` Suanming Mou
  2020-04-15  6:39 ` [dpdk-dev] [PATCH 02/10] net/mlx5: optimize action flags in flow handle Suanming Mou
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Suanming Mou @ 2020-04-15  6:39 UTC (permalink / raw)
  To: Matan Azrad, Shahaf Shuler, Viacheslav Ovsiienko; +Cc: wentaoc, rasland, dev

Currently, one flow only has one fate action, the fate actions members
in the flow struct can be reorganized as union to save the memory for
flow struct.

This commit reorganizes the fate actions as union, the act_flags helps
to identify the fate action type when flow destroys.

Signed-off-by: Suanming Mou <suanmingm@mellanox.com>
---
 drivers/net/mlx5/mlx5_flow.h       | 11 +++++----
 drivers/net/mlx5/mlx5_flow_dv.c    | 48 +++++++++++++++++++++++++-------------
 drivers/net/mlx5/mlx5_flow_verbs.c | 22 ++++++++++++-----
 3 files changed, 54 insertions(+), 27 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index e828af3..e89a092 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -488,10 +488,6 @@ struct mlx5_flow_handle_dv {
 	/**< Index to encap/decap resource in cache. */
 	struct mlx5_flow_dv_modify_hdr_resource *modify_hdr;
 	/**< Pointer to modify header resource in cache. */
-	uint32_t jump;
-	/**< Index to the jump action resource. */
-	uint32_t port_id_action;
-	/**< Index to port ID action resource. */
 	struct mlx5_vf_vlan vf_vlan;
 	/**< Structure for VF VLAN workaround. */
 	uint32_t push_vlan_res;
@@ -509,12 +505,17 @@ struct mlx5_flow_handle {
 	uint64_t act_flags;
 	/**< Bit-fields of detected actions, see MLX5_FLOW_ACTION_*. */
 	void *ib_flow; /**< Verbs flow pointer. */
-	uint32_t hrxq; /**< Hash Rx queue object index. */
 	struct mlx5_vf_vlan vf_vlan; /**< Structure for VF VLAN workaround. */
 	union {
 		uint32_t qrss_id; /**< Uniqie Q/RSS suffix subflow tag. */
 		uint32_t mtr_flow_id; /**< Unique meter match flow id. */
 	};
+	union {
+		uint32_t hrxq; /**< Hash Rx queue object index. */
+		uint32_t jump; /**< Index to the jump action resource. */
+		uint32_t port_id_action;
+		/**< Index to port ID action resource. */
+	};
 #ifdef HAVE_IBV_FLOW_DV_SUPPORT
 	struct mlx5_flow_handle_dv dvh;
 #endif
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 0162c66..54bd3fd 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -2540,7 +2540,7 @@ struct field_modify_info modify_tcp[] = {
 			(void *)&tbl_data->jump, cnt);
 	}
 	rte_atomic32_inc(&tbl_data->jump.refcnt);
-	dev_flow->handle->dvh.jump = tbl_data->idx;
+	dev_flow->handle->jump = tbl_data->idx;
 	dev_flow->dv.jump = &tbl_data->jump;
 	return 0;
 }
@@ -2581,14 +2581,14 @@ struct field_modify_info modify_tcp[] = {
 				(void *)cache_resource,
 				rte_atomic32_read(&cache_resource->refcnt));
 			rte_atomic32_inc(&cache_resource->refcnt);
-			dev_flow->handle->dvh.port_id_action = idx;
+			dev_flow->handle->port_id_action = idx;
 			dev_flow->dv.port_id_action = cache_resource;
 			return 0;
 		}
 	}
 	/* Register new port id action resource. */
 	cache_resource = mlx5_ipool_zmalloc(sh->ipool[MLX5_IPOOL_PORT_ID],
-				       &dev_flow->handle->dvh.port_id_action);
+				       &dev_flow->handle->port_id_action);
 	if (!cache_resource)
 		return rte_flow_error_set(error, ENOMEM,
 					  RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
@@ -2611,8 +2611,7 @@ struct field_modify_info modify_tcp[] = {
 	rte_atomic32_init(&cache_resource->refcnt);
 	rte_atomic32_inc(&cache_resource->refcnt);
 	ILIST_INSERT(sh->ipool[MLX5_IPOOL_PORT_ID], &sh->port_id_action_list,
-		     dev_flow->handle->dvh.port_id_action, cache_resource,
-		     next);
+		     dev_flow->handle->port_id_action, cache_resource, next);
 	dev_flow->dv.port_id_action = cache_resource;
 	DRV_LOG(DEBUG, "new port id action resource %p: refcnt %d++",
 		(void *)cache_resource,
@@ -7406,7 +7405,7 @@ struct field_modify_info modify_tcp[] = {
 			if (flow_dv_port_id_action_resource_register
 			    (dev, &port_id_resource, dev_flow, error))
 				return -rte_errno;
-			MLX5_ASSERT(!handle->dvh.port_id_action);
+			MLX5_ASSERT(!handle->port_id_action);
 			dev_flow->dv.actions[actions_n++] =
 					dev_flow->dv.port_id_action->action;
 			action_flags |= MLX5_FLOW_ACTION_PORT_ID;
@@ -8077,6 +8076,13 @@ struct field_modify_info modify_tcp[] = {
 						 "cannot get drop hash queue");
 					goto error;
 				}
+				/*
+				 * Drop queues will be released by the specify
+				 * mlx5_hrxq_drop_release() function. Assign
+				 * the special index to hrxq to mark the queue
+				 * has been allocated.
+				 */
+				dh->hrxq = UINT32_MAX;
 				dv->actions[n++] = drop_hrxq->action;
 			}
 		} else if (dh->act_flags &
@@ -8139,12 +8145,17 @@ struct field_modify_info modify_tcp[] = {
 	err = rte_errno; /* Save rte_errno before cleanup. */
 	SILIST_FOREACH(priv->sh->ipool[MLX5_IPOOL_MLX5_FLOW], flow->dev_handles,
 		       handle_idx, dh, next) {
+		/* hrxq is union, don't clear it if the flag is not set. */
 		if (dh->hrxq) {
-			if (dh->act_flags & MLX5_FLOW_ACTION_DROP)
+			if (dh->act_flags & MLX5_FLOW_ACTION_DROP) {
 				mlx5_hrxq_drop_release(dev);
-			else
+				dh->hrxq = 0;
+			} else if (dh->act_flags &
+				  (MLX5_FLOW_ACTION_QUEUE |
+				  MLX5_FLOW_ACTION_RSS)) {
 				mlx5_hrxq_release(dev, dh->hrxq);
-			dh->hrxq = 0;
+				dh->hrxq = 0;
+			}
 		}
 		if (dh->vf_vlan.tag && dh->vf_vlan.created)
 			mlx5_vlan_vmwa_release(dev, &dh->vf_vlan);
@@ -8249,7 +8260,7 @@ struct field_modify_info modify_tcp[] = {
 	struct mlx5_flow_tbl_data_entry *tbl_data;
 
 	tbl_data = mlx5_ipool_get(priv->sh->ipool[MLX5_IPOOL_JUMP],
-			     handle->dvh.jump);
+			     handle->jump);
 	if (!tbl_data)
 		return 0;
 	cache_resource = &tbl_data->jump;
@@ -8317,7 +8328,7 @@ struct field_modify_info modify_tcp[] = {
 {
 	struct mlx5_priv *priv = dev->data->dev_private;
 	struct mlx5_flow_dv_port_id_action_resource *cache_resource;
-	uint32_t idx = handle->dvh.port_id_action;
+	uint32_t idx = handle->port_id_action;
 
 	cache_resource = mlx5_ipool_get(priv->sh->ipool[MLX5_IPOOL_PORT_ID],
 					idx);
@@ -8410,12 +8421,17 @@ struct field_modify_info modify_tcp[] = {
 			claim_zero(mlx5_glue->dv_destroy_flow(dh->ib_flow));
 			dh->ib_flow = NULL;
 		}
+		/* hrxq is union, don't touch it only the flag is set. */
 		if (dh->hrxq) {
-			if (dh->act_flags & MLX5_FLOW_ACTION_DROP)
+			if (dh->act_flags & MLX5_FLOW_ACTION_DROP) {
 				mlx5_hrxq_drop_release(dev);
-			else
+				dh->hrxq = 0;
+			} else if (dh->act_flags &
+				  (MLX5_FLOW_ACTION_QUEUE |
+				  MLX5_FLOW_ACTION_RSS)) {
 				mlx5_hrxq_release(dev, dh->hrxq);
-			dh->hrxq = 0;
+				dh->hrxq = 0;
+			}
 		}
 		if (dh->vf_vlan.tag && dh->vf_vlan.created)
 			mlx5_vlan_vmwa_release(dev, &dh->vf_vlan);
@@ -8463,9 +8479,9 @@ struct field_modify_info modify_tcp[] = {
 			flow_dv_encap_decap_resource_release(dev, dev_handle);
 		if (dev_handle->dvh.modify_hdr)
 			flow_dv_modify_hdr_resource_release(dev_handle);
-		if (dev_handle->dvh.jump)
+		if (dev_handle->act_flags & MLX5_FLOW_ACTION_JUMP)
 			flow_dv_jump_tbl_resource_release(dev, dev_handle);
-		if (dev_handle->dvh.port_id_action)
+		if (dev_handle->act_flags & MLX5_FLOW_ACTION_PORT_ID)
 			flow_dv_port_id_action_resource_release(dev,
 								dev_handle);
 		if (dev_handle->dvh.push_vlan_res)
diff --git a/drivers/net/mlx5/mlx5_flow_verbs.c b/drivers/net/mlx5/mlx5_flow_verbs.c
index 9525fd4..931d547 100644
--- a/drivers/net/mlx5/mlx5_flow_verbs.c
+++ b/drivers/net/mlx5/mlx5_flow_verbs.c
@@ -1754,12 +1754,17 @@
 			claim_zero(mlx5_glue->destroy_flow(handle->ib_flow));
 			handle->ib_flow = NULL;
 		}
+		/* hrxq is union, don't touch it only the flag is set. */
 		if (handle->hrxq) {
-			if (handle->act_flags & MLX5_FLOW_ACTION_DROP)
+			if (handle->act_flags & MLX5_FLOW_ACTION_DROP) {
 				mlx5_hrxq_drop_release(dev);
-			else
+				handle->hrxq = 0;
+			} else if (handle->act_flags &
+				  (MLX5_FLOW_ACTION_QUEUE |
+				  MLX5_FLOW_ACTION_RSS)) {
 				mlx5_hrxq_release(dev, handle->hrxq);
-			handle->hrxq = 0;
+				handle->hrxq = 0;
+			}
 		}
 		if (handle->vf_vlan.tag && handle->vf_vlan.created)
 			mlx5_vlan_vmwa_release(dev, &handle->vf_vlan);
@@ -1891,12 +1896,17 @@
 	err = rte_errno; /* Save rte_errno before cleanup. */
 	SILIST_FOREACH(priv->sh->ipool[MLX5_IPOOL_MLX5_FLOW], flow->dev_handles,
 		       dev_handles, handle, next) {
+		/* hrxq is union, don't touch it only the flag is set. */
 		if (handle->hrxq) {
-			if (handle->act_flags & MLX5_FLOW_ACTION_DROP)
+			if (handle->act_flags & MLX5_FLOW_ACTION_DROP) {
 				mlx5_hrxq_drop_release(dev);
-			else
+				handle->hrxq = 0;
+			} else if (handle->act_flags &
+				  (MLX5_FLOW_ACTION_QUEUE |
+				  MLX5_FLOW_ACTION_RSS)) {
 				mlx5_hrxq_release(dev, handle->hrxq);
-			handle->hrxq = 0;
+				handle->hrxq = 0;
+			}
 		}
 		if (handle->vf_vlan.tag && handle->vf_vlan.created)
 			mlx5_vlan_vmwa_release(dev, &handle->vf_vlan);
-- 
1.8.3.1


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

* [dpdk-dev] [PATCH 02/10] net/mlx5: optimize action flags in flow handle
  2020-04-15  6:39 [dpdk-dev] [PATCH 00/10] net/mlx5: optimize flow structure Suanming Mou
  2020-04-15  6:39 ` [dpdk-dev] [PATCH 01/10] net/mlx5: reorganize fate actions as union Suanming Mou
@ 2020-04-15  6:39 ` Suanming Mou
  2020-04-15  6:39 ` [dpdk-dev] [PATCH 03/10] net/mlx5: reorganize the mlx5 flow handle struct Suanming Mou
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Suanming Mou @ 2020-04-15  6:39 UTC (permalink / raw)
  To: Matan Azrad, Shahaf Shuler, Viacheslav Ovsiienko; +Cc: wentaoc, rasland, dev

As only limited bits is used in act_flags for flow destroy, it's a bit
expensive to save the whole 64 bits. Move the act_flags out of flow
handle and save the needed bits for flow destroy to save some bytes for
the flow handle data struct.

The fate action type and mark bits are reserved as they will be used in
flow destroy.

Signed-off-by: Suanming Mou <suanmingm@mellanox.com>
---
 drivers/net/mlx5/mlx5_flow.c       |  8 +++-----
 drivers/net/mlx5/mlx5_flow.h       | 16 ++++++++++++++--
 drivers/net/mlx5/mlx5_flow_dv.c    | 30 ++++++++++++++++--------------
 drivers/net/mlx5/mlx5_flow_verbs.c | 23 +++++++++++++----------
 4 files changed, 46 insertions(+), 31 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index bf95a40..ffc2910 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -722,8 +722,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 		       struct mlx5_flow_handle *dev_handle)
 {
 	struct mlx5_priv *priv = dev->data->dev_private;
-	const int mark = !!(dev_handle->act_flags &
-			    (MLX5_FLOW_ACTION_FLAG | MLX5_FLOW_ACTION_MARK));
+	const int mark = dev_handle->mark;
 	const int tunnel = !!(dev_handle->layers & MLX5_FLOW_LAYER_TUNNEL);
 	unsigned int i;
 
@@ -800,8 +799,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 			struct mlx5_flow_handle *dev_handle)
 {
 	struct mlx5_priv *priv = dev->data->dev_private;
-	const int mark = !!(dev_handle->act_flags &
-			    (MLX5_FLOW_ACTION_FLAG | MLX5_FLOW_ACTION_MARK));
+	const int mark = dev_handle->mark;
 	const int tunnel = !!(dev_handle->layers & MLX5_FLOW_LAYER_TUNNEL);
 	unsigned int i;
 
@@ -2718,7 +2716,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 	 * help to do the optimization work for source code.
 	 * If no decap actions, use the layers directly.
 	 */
-	if (!(dev_flow->handle->act_flags & MLX5_FLOW_ACTION_DECAP))
+	if (!(dev_flow->act_flags & MLX5_FLOW_ACTION_DECAP))
 		return dev_flow->handle->layers;
 	/* Convert L3 layers with decap action. */
 	if (dev_flow->handle->layers & MLX5_FLOW_LAYER_INNER_L3_IPV4)
diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index e89a092..0f5c5d4 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -339,6 +339,16 @@ enum mlx5_flow_drv_type {
 	MLX5_FLOW_TYPE_MAX,
 };
 
+/* Fate action type. */
+enum mlx5_flow_fate_type {
+	MLX5_FLOW_FATE_NONE, /* Egress flow. */
+	MLX5_FLOW_FATE_QUEUE,
+	MLX5_FLOW_FATE_JUMP,
+	MLX5_FLOW_FATE_PORT_ID,
+	MLX5_FLOW_FATE_DROP,
+	MLX5_FLOW_FATE_MAX,
+};
+
 /* Matcher PRM representation */
 struct mlx5_flow_dv_match_params {
 	size_t size;
@@ -502,14 +512,14 @@ struct mlx5_flow_handle {
 	/**< Index to next device flow handle. */
 	uint64_t layers;
 	/**< Bit-fields of present layers, see MLX5_FLOW_LAYER_*. */
-	uint64_t act_flags;
-	/**< Bit-fields of detected actions, see MLX5_FLOW_ACTION_*. */
 	void *ib_flow; /**< Verbs flow pointer. */
 	struct mlx5_vf_vlan vf_vlan; /**< Structure for VF VLAN workaround. */
 	union {
 		uint32_t qrss_id; /**< Uniqie Q/RSS suffix subflow tag. */
 		uint32_t mtr_flow_id; /**< Unique meter match flow id. */
 	};
+	uint32_t mark:1; /**< Metadate rxq mark flag. */
+	uint32_t fate_action:3; /**< Fate action type. */
 	union {
 		uint32_t hrxq; /**< Hash Rx queue object index. */
 		uint32_t jump; /**< Index to the jump action resource. */
@@ -624,6 +634,8 @@ struct mlx5_flow_verbs_workspace {
 struct mlx5_flow {
 	struct rte_flow *flow; /**< Pointer to the main flow. */
 	uint64_t hash_fields; /**< Verbs hash Rx queue hash fields. */
+	uint64_t act_flags;
+	/**< Bit-fields of detected actions, see MLX5_FLOW_ACTION_*. */
 	bool external; /**< true if the flow is created external to PMD. */
 	uint8_t ingress; /**< 1 if the flow is ingress. */
 	union {
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 54bd3fd..de27b74 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -7409,9 +7409,11 @@ struct field_modify_info modify_tcp[] = {
 			dev_flow->dv.actions[actions_n++] =
 					dev_flow->dv.port_id_action->action;
 			action_flags |= MLX5_FLOW_ACTION_PORT_ID;
+			dev_flow->handle->fate_action = MLX5_FLOW_FATE_PORT_ID;
 			break;
 		case RTE_FLOW_ACTION_TYPE_FLAG:
 			action_flags |= MLX5_FLOW_ACTION_FLAG;
+			dev_flow->handle->mark = 1;
 			if (dev_conf->dv_xmeta_en != MLX5_XMETA_MODE_LEGACY) {
 				struct rte_flow_action_mark mark = {
 					.id = MLX5_FLOW_MARK_DEFAULT,
@@ -7440,6 +7442,7 @@ struct field_modify_info modify_tcp[] = {
 			break;
 		case RTE_FLOW_ACTION_TYPE_MARK:
 			action_flags |= MLX5_FLOW_ACTION_MARK;
+			dev_flow->handle->mark = 1;
 			if (dev_conf->dv_xmeta_en != MLX5_XMETA_MODE_LEGACY) {
 				const struct rte_flow_action_mark *mark =
 					(const struct rte_flow_action_mark *)
@@ -7484,6 +7487,7 @@ struct field_modify_info modify_tcp[] = {
 			break;
 		case RTE_FLOW_ACTION_TYPE_DROP:
 			action_flags |= MLX5_FLOW_ACTION_DROP;
+			dev_flow->handle->fate_action = MLX5_FLOW_FATE_DROP;
 			break;
 		case RTE_FLOW_ACTION_TYPE_QUEUE:
 			MLX5_ASSERT(flow->rss.queue);
@@ -7491,6 +7495,7 @@ struct field_modify_info modify_tcp[] = {
 			flow->rss.queue_num = 1;
 			(*flow->rss.queue)[0] = queue->index;
 			action_flags |= MLX5_FLOW_ACTION_QUEUE;
+			dev_flow->handle->fate_action = MLX5_FLOW_FATE_QUEUE;
 			break;
 		case RTE_FLOW_ACTION_TYPE_RSS:
 			MLX5_ASSERT(flow->rss.queue);
@@ -7507,6 +7512,7 @@ struct field_modify_info modify_tcp[] = {
 			 * when expanding items for RSS.
 			 */
 			action_flags |= MLX5_FLOW_ACTION_RSS;
+			dev_flow->handle->fate_action = MLX5_FLOW_FATE_QUEUE;
 			break;
 		case RTE_FLOW_ACTION_TYPE_COUNT:
 			if (!dev_conf->devx) {
@@ -7664,6 +7670,7 @@ struct field_modify_info modify_tcp[] = {
 			dev_flow->dv.actions[actions_n++] =
 					dev_flow->dv.jump->action;
 			action_flags |= MLX5_FLOW_ACTION_JUMP;
+			dev_flow->handle->fate_action = MLX5_FLOW_FATE_JUMP;
 			break;
 		case RTE_FLOW_ACTION_TYPE_SET_MAC_SRC:
 		case RTE_FLOW_ACTION_TYPE_SET_MAC_DST:
@@ -7806,7 +7813,7 @@ struct field_modify_info modify_tcp[] = {
 			modify_action_position = actions_n++;
 	}
 	dev_flow->dv.actions_n = actions_n;
-	handle->act_flags = action_flags;
+	dev_flow->act_flags = action_flags;
 	for (; items->type != RTE_FLOW_ITEM_TYPE_END; items++) {
 		int tunnel = !!(item_flags & MLX5_FLOW_LAYER_TUNNEL);
 		int item_type = items->type;
@@ -8062,7 +8069,7 @@ struct field_modify_info modify_tcp[] = {
 		dh = dev_flow->handle;
 		dv_h = &dh->dvh;
 		n = dv->actions_n;
-		if (dh->act_flags & MLX5_FLOW_ACTION_DROP) {
+		if (dh->fate_action == MLX5_FLOW_FATE_DROP) {
 			if (dv->transfer) {
 				dv->actions[n++] = priv->sh->esw_drop_action;
 			} else {
@@ -8085,8 +8092,7 @@ struct field_modify_info modify_tcp[] = {
 				dh->hrxq = UINT32_MAX;
 				dv->actions[n++] = drop_hrxq->action;
 			}
-		} else if (dh->act_flags &
-			   (MLX5_FLOW_ACTION_QUEUE | MLX5_FLOW_ACTION_RSS)) {
+		} else if (dh->fate_action == MLX5_FLOW_FATE_QUEUE) {
 			struct mlx5_hrxq *hrxq;
 			uint32_t hrxq_idx;
 
@@ -8147,12 +8153,10 @@ struct field_modify_info modify_tcp[] = {
 		       handle_idx, dh, next) {
 		/* hrxq is union, don't clear it if the flag is not set. */
 		if (dh->hrxq) {
-			if (dh->act_flags & MLX5_FLOW_ACTION_DROP) {
+			if (dh->fate_action == MLX5_FLOW_FATE_DROP) {
 				mlx5_hrxq_drop_release(dev);
 				dh->hrxq = 0;
-			} else if (dh->act_flags &
-				  (MLX5_FLOW_ACTION_QUEUE |
-				  MLX5_FLOW_ACTION_RSS)) {
+			} else if (dh->fate_action == MLX5_FLOW_FATE_QUEUE) {
 				mlx5_hrxq_release(dev, dh->hrxq);
 				dh->hrxq = 0;
 			}
@@ -8423,12 +8427,10 @@ struct field_modify_info modify_tcp[] = {
 		}
 		/* hrxq is union, don't touch it only the flag is set. */
 		if (dh->hrxq) {
-			if (dh->act_flags & MLX5_FLOW_ACTION_DROP) {
+			if (dh->fate_action == MLX5_FLOW_FATE_DROP) {
 				mlx5_hrxq_drop_release(dev);
 				dh->hrxq = 0;
-			} else if (dh->act_flags &
-				  (MLX5_FLOW_ACTION_QUEUE |
-				  MLX5_FLOW_ACTION_RSS)) {
+			} else if (dh->fate_action == MLX5_FLOW_FATE_QUEUE) {
 				mlx5_hrxq_release(dev, dh->hrxq);
 				dh->hrxq = 0;
 			}
@@ -8479,9 +8481,9 @@ struct field_modify_info modify_tcp[] = {
 			flow_dv_encap_decap_resource_release(dev, dev_handle);
 		if (dev_handle->dvh.modify_hdr)
 			flow_dv_modify_hdr_resource_release(dev_handle);
-		if (dev_handle->act_flags & MLX5_FLOW_ACTION_JUMP)
+		if (dev_handle->fate_action == MLX5_FLOW_FATE_JUMP)
 			flow_dv_jump_tbl_resource_release(dev, dev_handle);
-		if (dev_handle->act_flags & MLX5_FLOW_ACTION_PORT_ID)
+		if (dev_handle->fate_action == MLX5_FLOW_FATE_PORT_ID)
 			flow_dv_port_id_action_resource_release(dev,
 								dev_handle);
 		if (dev_handle->dvh.push_vlan_res)
diff --git a/drivers/net/mlx5/mlx5_flow_verbs.c b/drivers/net/mlx5/mlx5_flow_verbs.c
index 931d547..1d56b03 100644
--- a/drivers/net/mlx5/mlx5_flow_verbs.c
+++ b/drivers/net/mlx5/mlx5_flow_verbs.c
@@ -1589,22 +1589,27 @@
 		case RTE_FLOW_ACTION_TYPE_FLAG:
 			flow_verbs_translate_action_flag(dev_flow, actions);
 			action_flags |= MLX5_FLOW_ACTION_FLAG;
+			dev_flow->handle->mark = 1;
 			break;
 		case RTE_FLOW_ACTION_TYPE_MARK:
 			flow_verbs_translate_action_mark(dev_flow, actions);
 			action_flags |= MLX5_FLOW_ACTION_MARK;
+			dev_flow->handle->mark = 1;
 			break;
 		case RTE_FLOW_ACTION_TYPE_DROP:
 			flow_verbs_translate_action_drop(dev_flow, actions);
 			action_flags |= MLX5_FLOW_ACTION_DROP;
+			dev_flow->handle->fate_action = MLX5_FLOW_FATE_DROP;
 			break;
 		case RTE_FLOW_ACTION_TYPE_QUEUE:
 			flow_verbs_translate_action_queue(dev_flow, actions);
 			action_flags |= MLX5_FLOW_ACTION_QUEUE;
+			dev_flow->handle->fate_action = MLX5_FLOW_FATE_QUEUE;
 			break;
 		case RTE_FLOW_ACTION_TYPE_RSS:
 			flow_verbs_translate_action_rss(dev_flow, actions);
 			action_flags |= MLX5_FLOW_ACTION_RSS;
+			dev_flow->handle->fate_action = MLX5_FLOW_FATE_QUEUE;
 			break;
 		case RTE_FLOW_ACTION_TYPE_COUNT:
 			ret = flow_verbs_translate_action_count(dev_flow,
@@ -1621,7 +1626,7 @@
 						  "action not supported");
 		}
 	}
-	dev_flow->handle->act_flags = action_flags;
+	dev_flow->act_flags = action_flags;
 	for (; items->type != RTE_FLOW_ITEM_TYPE_END; items++) {
 		int tunnel = !!(item_flags & MLX5_FLOW_LAYER_TUNNEL);
 
@@ -1756,12 +1761,11 @@
 		}
 		/* hrxq is union, don't touch it only the flag is set. */
 		if (handle->hrxq) {
-			if (handle->act_flags & MLX5_FLOW_ACTION_DROP) {
+			if (handle->fate_action == MLX5_FLOW_FATE_DROP) {
 				mlx5_hrxq_drop_release(dev);
 				handle->hrxq = 0;
-			} else if (handle->act_flags &
-				  (MLX5_FLOW_ACTION_QUEUE |
-				  MLX5_FLOW_ACTION_RSS)) {
+			} else if (handle->fate_action ==
+				   MLX5_FLOW_FATE_QUEUE) {
 				mlx5_hrxq_release(dev, handle->hrxq);
 				handle->hrxq = 0;
 			}
@@ -1833,7 +1837,7 @@
 	for (idx = priv->flow_idx - 1; idx >= priv->flow_nested_idx; idx--) {
 		dev_flow = &((struct mlx5_flow *)priv->inter_flows)[idx];
 		handle = dev_flow->handle;
-		if (handle->act_flags & MLX5_FLOW_ACTION_DROP) {
+		if (handle->fate_action == MLX5_FLOW_FATE_DROP) {
 			hrxq = mlx5_hrxq_drop_new(dev);
 			if (!hrxq) {
 				rte_flow_error_set
@@ -1898,12 +1902,11 @@
 		       dev_handles, handle, next) {
 		/* hrxq is union, don't touch it only the flag is set. */
 		if (handle->hrxq) {
-			if (handle->act_flags & MLX5_FLOW_ACTION_DROP) {
+			if (handle->fate_action == MLX5_FLOW_FATE_DROP) {
 				mlx5_hrxq_drop_release(dev);
 				handle->hrxq = 0;
-			} else if (handle->act_flags &
-				  (MLX5_FLOW_ACTION_QUEUE |
-				  MLX5_FLOW_ACTION_RSS)) {
+			} else if (handle->fate_action ==
+				   MLX5_FLOW_FATE_QUEUE) {
 				mlx5_hrxq_release(dev, handle->hrxq);
 				handle->hrxq = 0;
 			}
-- 
1.8.3.1


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

* [dpdk-dev] [PATCH 03/10] net/mlx5: reorganize the mlx5 flow handle struct
  2020-04-15  6:39 [dpdk-dev] [PATCH 00/10] net/mlx5: optimize flow structure Suanming Mou
  2020-04-15  6:39 ` [dpdk-dev] [PATCH 01/10] net/mlx5: reorganize fate actions as union Suanming Mou
  2020-04-15  6:39 ` [dpdk-dev] [PATCH 02/10] net/mlx5: optimize action flags in flow handle Suanming Mou
@ 2020-04-15  6:39 ` Suanming Mou
  2020-04-15  6:39 ` [dpdk-dev] [PATCH 04/10] net/mlx5: optimize mlx5 flow RSS struct Suanming Mou
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Suanming Mou @ 2020-04-15  6:39 UTC (permalink / raw)
  To: Matan Azrad, Shahaf Shuler, Viacheslav Ovsiienko; +Cc: wentaoc, rasland, dev

Currently, the mlx5_flow_handle struct is not fully aligned and has some
bits wasted. The members can be optimized and reorganized to save memory.

1. As metadata and meter is sharing the same flow match id, now the flow
id is limited to 24 bits due to the 8 MSBs are used as for the meter color.
Align the flow id to other bit members to 32 bits to save the mlx5 flow
handle memory.

2. The vlan_vf in struct mlx5_flow_handle_dv was already moved to struct
mlx5_flow_handle. Remove the legacy vlan_vf in struct mlx5_flow_handle_dv.

3. Reorganize the vlan_vf in mlx5_flow_handle with member SILIST_ENTRY
next to make it align with 8 bytes.

4. Reorganize the header modify in mlx5_flow_handle_dv to ILIST_ENTRY next
to make it align to with bytes.

5. Introduce __rte_pack attribute to make the struct tightly organized.

It will totally save 20 bytes memory for mlx5_flow_handle struct.

Signed-off-by: Suanming Mou <suanmingm@mellanox.com>
---
 drivers/net/mlx5/mlx5_flow.c |  8 ++++----
 drivers/net/mlx5/mlx5_flow.h | 17 ++++++-----------
 2 files changed, 10 insertions(+), 15 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index ffc2910..71c6bce 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -2323,8 +2323,8 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 
 	SILIST_FOREACH(priv->sh->ipool[MLX5_IPOOL_MLX5_FLOW], flow->dev_handles,
 		       handle_idx, dev_handle, next)
-		if (dev_handle->qrss_id)
-			flow_qrss_free_id(dev, dev_handle->qrss_id);
+		if (dev_handle->flow_id)
+			flow_qrss_free_id(dev, dev_handle->flow_id);
 }
 
 static int
@@ -3988,7 +3988,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 			 * reallocation becomes possible (for example, for
 			 * other flows in other threads).
 			 */
-			dev_flow->handle->qrss_id = qrss_id;
+			dev_flow->handle->flow_id = qrss_id;
 			ret = mlx5_flow_get_reg_id(dev, MLX5_COPY_MARK, 0,
 						   error);
 			if (ret < 0)
@@ -4101,7 +4101,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 			ret = -rte_errno;
 			goto exit;
 		}
-		dev_flow->handle->mtr_flow_id = mtr_tag_id;
+		dev_flow->handle->flow_id = mtr_tag_id;
 		/* Setting the sfx group atrr. */
 		sfx_attr.group = sfx_attr.transfer ?
 				(MLX5_FLOW_TABLE_LEVEL_SUFFIX - 1) :
diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index 0f5c5d4..db42189 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -494,30 +494,25 @@ struct mlx5_flow_rss {
 struct mlx5_flow_handle_dv {
 	/* Flow DV api: */
 	struct mlx5_flow_dv_matcher *matcher; /**< Cache to matcher. */
-	uint32_t encap_decap;
-	/**< Index to encap/decap resource in cache. */
 	struct mlx5_flow_dv_modify_hdr_resource *modify_hdr;
 	/**< Pointer to modify header resource in cache. */
-	struct mlx5_vf_vlan vf_vlan;
-	/**< Structure for VF VLAN workaround. */
+	uint32_t encap_decap;
+	/**< Index to encap/decap resource in cache. */
 	uint32_t push_vlan_res;
 	/**< Index to push VLAN action resource in cache. */
 	uint32_t tag_resource;
 	/**< Index to the tag action. */
-};
+} __rte_packed;
 
 /** Device flow handle structure: used both for creating & destroying. */
 struct mlx5_flow_handle {
 	SILIST_ENTRY(uint32_t)next;
+	struct mlx5_vf_vlan vf_vlan; /**< Structure for VF VLAN workaround. */
 	/**< Index to next device flow handle. */
 	uint64_t layers;
 	/**< Bit-fields of present layers, see MLX5_FLOW_LAYER_*. */
 	void *ib_flow; /**< Verbs flow pointer. */
-	struct mlx5_vf_vlan vf_vlan; /**< Structure for VF VLAN workaround. */
-	union {
-		uint32_t qrss_id; /**< Uniqie Q/RSS suffix subflow tag. */
-		uint32_t mtr_flow_id; /**< Unique meter match flow id. */
-	};
+	uint32_t flow_id:28; /**< Unique match flow id. */
 	uint32_t mark:1; /**< Metadate rxq mark flag. */
 	uint32_t fate_action:3; /**< Fate action type. */
 	union {
@@ -529,7 +524,7 @@ struct mlx5_flow_handle {
 #ifdef HAVE_IBV_FLOW_DV_SUPPORT
 	struct mlx5_flow_handle_dv dvh;
 #endif
-};
+} __rte_packed;
 
 /*
  * Size for Verbs device flow handle structure only. Do not use the DV only
-- 
1.8.3.1


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

* [dpdk-dev] [PATCH 04/10] net/mlx5: optimize mlx5 flow RSS struct
  2020-04-15  6:39 [dpdk-dev] [PATCH 00/10] net/mlx5: optimize flow structure Suanming Mou
                   ` (2 preceding siblings ...)
  2020-04-15  6:39 ` [dpdk-dev] [PATCH 03/10] net/mlx5: reorganize the mlx5 flow handle struct Suanming Mou
@ 2020-04-15  6:39 ` Suanming Mou
  2020-04-15  6:39 ` [dpdk-dev] [PATCH 05/10] net/mlx5: allocate rte flow from indexed pool Suanming Mou
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Suanming Mou @ 2020-04-15  6:39 UTC (permalink / raw)
  To: Matan Azrad, Shahaf Shuler, Viacheslav Ovsiienko; +Cc: wentaoc, rasland, dev

When destroy the flow with RSS, only RSS queue is needed. Currently,
even the RSS key, types, level, are all saved to the rte flow.

Saves only the mlx5 flow RSS queue information to save memory for
rte flow.

As the maximum queue number is UINT16_MAX, the queue number type is
changed to uint16_t.

Signed-off-by: Suanming Mou <suanmingm@mellanox.com>
---
 drivers/net/mlx5/mlx5.h            |  1 +
 drivers/net/mlx5/mlx5_flow.c       | 26 +++++++++++-----
 drivers/net/mlx5/mlx5_flow.h       | 14 ++++++---
 drivers/net/mlx5/mlx5_flow_dv.c    | 48 +++++++++++++++++------------
 drivers/net/mlx5/mlx5_flow_verbs.c | 62 ++++++++++++++++++++------------------
 5 files changed, 89 insertions(+), 62 deletions(-)

diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index 31e0da2..41c87ec 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -556,6 +556,7 @@ struct mlx5_priv {
 	struct mlx5_flows flows; /* RTE Flow rules. */
 	struct mlx5_flows ctrl_flows; /* Control flow rules. */
 	void *inter_flows; /* Intermediate resources for flow creation. */
+	void *rss_meta; /* Intermediate rss metadata resources for rss. */
 	int flow_idx; /* Intermediate device flow index. */
 	int flow_nested_idx; /* Intermediate device flow index, nested. */
 	LIST_HEAD(rxq, mlx5_rxq_ctrl) rxqsctrl; /* DPDK Rx queues. */
diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 71c6bce..9f79031 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -658,13 +658,12 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
  *   The hash fields that should be used.
  */
 uint64_t
-mlx5_flow_hashfields_adjust(struct mlx5_flow *dev_flow,
+mlx5_flow_hashfields_adjust(struct mlx5_flow_rss_meta *rss_meta,
 			    int tunnel __rte_unused, uint64_t layer_types,
 			    uint64_t hash_fields)
 {
-	struct rte_flow *flow = dev_flow->flow;
 #ifdef HAVE_IBV_DEVICE_TUNNEL_SUPPORT
-	int rss_request_inner = flow->rss.level >= 2;
+	int rss_request_inner = rss_meta->level >= 2;
 
 	/* Check RSS hash level for tunnel. */
 	if (tunnel && rss_request_inner)
@@ -673,7 +672,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 		return 0;
 #endif
 	/* Check if requested layer matches RSS hash fields. */
-	if (!(flow->rss.types & layer_types))
+	if (!(rss_meta->types & layer_types))
 		return 0;
 	return hash_fields;
 }
@@ -4223,6 +4222,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 		uint8_t buffer[2048];
 	} items_tx;
 	struct rte_flow_expand_rss *buf = &expand_buffer.buf;
+	struct mlx5_flow_rss_meta *rss_meta = priv->rss_meta;
 	const struct rte_flow_action *p_actions_rx = actions;
 	uint32_t i;
 	uint32_t flow_size;
@@ -4263,14 +4263,16 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 	MLX5_ASSERT(flow->drv_type > MLX5_FLOW_TYPE_MIN &&
 		    flow->drv_type < MLX5_FLOW_TYPE_MAX);
 	flow->rss.queue = (void *)(flow + 1);
+	memset(rss_meta, 0, sizeof(*rss_meta));
+	rss_meta->queue = flow->rss.queue;
 	if (rss) {
 		/*
 		 * The following information is required by
 		 * mlx5_flow_hashfields_adjust() in advance.
 		 */
-		flow->rss.level = rss->level;
+		rss_meta->level = rss->level;
 		/* RSS type 0 indicates default RSS type (ETH_RSS_IP). */
-		flow->rss.types = !rss->types ? ETH_RSS_IP : rss->types;
+		rss_meta->types = !rss->types ? ETH_RSS_IP : rss->types;
 	}
 	flow->dev_handles = 0;
 	if (rss && rss->types) {
@@ -4623,9 +4625,17 @@ struct rte_flow *
 {
 	struct mlx5_priv *priv = dev->data->dev_private;
 
-	if (!priv->inter_flows)
+	if (!priv->inter_flows) {
 		priv->inter_flows = rte_calloc(__func__, MLX5_NUM_MAX_DEV_FLOWS,
-					       sizeof(struct mlx5_flow), 0);
+				    sizeof(struct mlx5_flow) +
+				    sizeof(struct mlx5_flow_rss_meta), 0);
+		if (!priv->inter_flows) {
+			DRV_LOG(ERR, "can't allocate intermediate memory.");
+			return;
+		}
+	}
+	priv->rss_meta = &((struct mlx5_flow *)priv->inter_flows)
+			 [MLX5_NUM_MAX_DEV_FLOWS];
 	/* Reset the index. */
 	priv->flow_idx = 0;
 	priv->flow_nested_idx = 0;
diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index db42189..4516b51 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -482,7 +482,12 @@ struct ibv_spec_header {
 	uint16_t size;
 };
 
-struct mlx5_flow_rss {
+struct mlx5_flow_rss_queue {
+	uint16_t queue_num; /**< Number of entries in @p queue. */
+	uint16_t (*queue)[]; /**< Destination queues to redirect traffic to. */
+} __rte_packed;
+
+struct mlx5_flow_rss_meta {
 	uint32_t level;
 	uint32_t queue_num; /**< Number of entries in @p queue. */
 	uint64_t types; /**< Specific RSS hash types (see ETH_RSS_*). */
@@ -490,6 +495,7 @@ struct mlx5_flow_rss {
 	uint8_t key[MLX5_RSS_HASH_KEY_LEN]; /**< RSS hash key. */
 };
 
+
 /** Device flow handle structure for DV mode only. */
 struct mlx5_flow_handle_dv {
 	/* Flow DV api: */
@@ -749,7 +755,7 @@ struct mlx5_flow_meter_profile {
 struct rte_flow {
 	TAILQ_ENTRY(rte_flow) next; /**< Pointer to the next flow structure. */
 	enum mlx5_flow_drv_type drv_type; /**< Driver type. */
-	struct mlx5_flow_rss rss; /**< RSS context. */
+	struct mlx5_flow_rss_queue rss; /**< RSS queue context. */
 	uint32_t counter; /**< Holds flow counter. */
 	struct mlx5_flow_mreg_copy_resource *mreg_copy;
 	/**< pointer to metadata register copy table resource. */
@@ -842,8 +848,8 @@ uint32_t mlx5_flow_id_release(struct mlx5_flow_id_pool *pool,
 int mlx5_flow_group_to_table(const struct rte_flow_attr *attributes,
 			     bool external, uint32_t group, bool fdb_def_rule,
 			     uint32_t *table, struct rte_flow_error *error);
-uint64_t mlx5_flow_hashfields_adjust(struct mlx5_flow *dev_flow, int tunnel,
-				     uint64_t layer_types,
+uint64_t mlx5_flow_hashfields_adjust(struct mlx5_flow_rss_meta *rss_meta,
+				     int tunnel, uint64_t layer_types,
 				     uint64_t hash_fields);
 uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 				   uint32_t subpriority);
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index de27b74..26733e0 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -7241,18 +7241,20 @@ struct field_modify_info modify_tcp[] = {
  *
  * @param[in] dev_flow
  *   Pointer to the mlx5_flow.
+ * @param[in] rss_meta
+ *   Pointer to the mlx5_flow_rss_meta.
  */
 static void
-flow_dv_hashfields_set(struct mlx5_flow *dev_flow)
+flow_dv_hashfields_set(struct mlx5_flow *dev_flow,
+		       struct mlx5_flow_rss_meta *rss_meta)
 {
-	struct rte_flow *flow = dev_flow->flow;
 	uint64_t items = dev_flow->handle->layers;
 	int rss_inner = 0;
-	uint64_t rss_types = rte_eth_rss_hf_refine(flow->rss.types);
+	uint64_t rss_types = rte_eth_rss_hf_refine(rss_meta->types);
 
 	dev_flow->hash_fields = 0;
 #ifdef HAVE_IBV_DEVICE_TUNNEL_SUPPORT
-	if (flow->rss.level >= 2) {
+	if (rss_meta->level >= 2) {
 		dev_flow->hash_fields |= IBV_RX_HASH_INNER;
 		rss_inner = 1;
 	}
@@ -7337,6 +7339,8 @@ struct field_modify_info modify_tcp[] = {
 	struct mlx5_dev_config *dev_conf = &priv->config;
 	struct rte_flow *flow = dev_flow->flow;
 	struct mlx5_flow_handle *handle = dev_flow->handle;
+	struct mlx5_flow_rss_meta *rss_meta = (struct mlx5_flow_rss_meta *)
+					      priv->rss_meta;
 	uint64_t item_flags = 0;
 	uint64_t last_item = 0;
 	uint64_t action_flags = 0;
@@ -7493,6 +7497,7 @@ struct field_modify_info modify_tcp[] = {
 			MLX5_ASSERT(flow->rss.queue);
 			queue = actions->conf;
 			flow->rss.queue_num = 1;
+			rss_meta->queue_num = flow->rss.queue_num;
 			(*flow->rss.queue)[0] = queue->index;
 			action_flags |= MLX5_FLOW_ACTION_QUEUE;
 			dev_flow->handle->fate_action = MLX5_FLOW_FATE_QUEUE;
@@ -7504,9 +7509,10 @@ struct field_modify_info modify_tcp[] = {
 				memcpy((*flow->rss.queue), rss->queue,
 				       rss->queue_num * sizeof(uint16_t));
 			flow->rss.queue_num = rss->queue_num;
+			rss_meta->queue_num = flow->rss.queue_num;
 			/* NULL RSS key indicates default RSS key. */
 			rss_key = !rss->key ? rss_hash_default_key : rss->key;
-			memcpy(flow->rss.key, rss_key, MLX5_RSS_HASH_KEY_LEN);
+			memcpy(rss_meta->key, rss_key, MLX5_RSS_HASH_KEY_LEN);
 			/*
 			 * rss->level and rss.types should be set in advance
 			 * when expanding items for RSS.
@@ -7904,7 +7910,7 @@ struct field_modify_info modify_tcp[] = {
 		case RTE_FLOW_ITEM_TYPE_GRE:
 			flow_dv_translate_item_gre(match_mask, match_value,
 						   items, tunnel);
-			matcher.priority = flow->rss.level >= 2 ?
+			matcher.priority = rss_meta->level >= 2 ?
 				    MLX5_PRIORITY_MAP_L2 : MLX5_PRIORITY_MAP_L4;
 			last_item = MLX5_FLOW_LAYER_GRE;
 			break;
@@ -7916,14 +7922,14 @@ struct field_modify_info modify_tcp[] = {
 		case RTE_FLOW_ITEM_TYPE_NVGRE:
 			flow_dv_translate_item_nvgre(match_mask, match_value,
 						     items, tunnel);
-			matcher.priority = flow->rss.level >= 2 ?
+			matcher.priority = rss_meta->level >= 2 ?
 				    MLX5_PRIORITY_MAP_L2 : MLX5_PRIORITY_MAP_L4;
 			last_item = MLX5_FLOW_LAYER_GRE;
 			break;
 		case RTE_FLOW_ITEM_TYPE_VXLAN:
 			flow_dv_translate_item_vxlan(match_mask, match_value,
 						     items, tunnel);
-			matcher.priority = flow->rss.level >= 2 ?
+			matcher.priority = rss_meta->level >= 2 ?
 				    MLX5_PRIORITY_MAP_L2 : MLX5_PRIORITY_MAP_L4;
 			last_item = MLX5_FLOW_LAYER_VXLAN;
 			break;
@@ -7931,21 +7937,21 @@ struct field_modify_info modify_tcp[] = {
 			flow_dv_translate_item_vxlan_gpe(match_mask,
 							 match_value, items,
 							 tunnel);
-			matcher.priority = flow->rss.level >= 2 ?
+			matcher.priority = rss_meta->level >= 2 ?
 				    MLX5_PRIORITY_MAP_L2 : MLX5_PRIORITY_MAP_L4;
 			last_item = MLX5_FLOW_LAYER_VXLAN_GPE;
 			break;
 		case RTE_FLOW_ITEM_TYPE_GENEVE:
 			flow_dv_translate_item_geneve(match_mask, match_value,
 						      items, tunnel);
-			matcher.priority = flow->rss.level >= 2 ?
+			matcher.priority = rss_meta->level >= 2 ?
 				    MLX5_PRIORITY_MAP_L2 : MLX5_PRIORITY_MAP_L4;
 			last_item = MLX5_FLOW_LAYER_GENEVE;
 			break;
 		case RTE_FLOW_ITEM_TYPE_MPLS:
 			flow_dv_translate_item_mpls(match_mask, match_value,
 						    items, last_item, tunnel);
-			matcher.priority = flow->rss.level >= 2 ?
+			matcher.priority = rss_meta->level >= 2 ?
 				    MLX5_PRIORITY_MAP_L2 : MLX5_PRIORITY_MAP_L4;
 			last_item = MLX5_FLOW_LAYER_MPLS;
 			break;
@@ -7988,7 +7994,7 @@ struct field_modify_info modify_tcp[] = {
 		case RTE_FLOW_ITEM_TYPE_GTP:
 			flow_dv_translate_item_gtp(match_mask, match_value,
 						   items, tunnel);
-			matcher.priority = flow->rss.level >= 2 ?
+			matcher.priority = rss_meta->level >= 2 ?
 				    MLX5_PRIORITY_MAP_L2 : MLX5_PRIORITY_MAP_L4;
 			last_item = MLX5_FLOW_LAYER_GTP;
 			break;
@@ -8020,7 +8026,7 @@ struct field_modify_info modify_tcp[] = {
 	 */
 	handle->layers |= item_flags;
 	if (action_flags & MLX5_FLOW_ACTION_RSS)
-		flow_dv_hashfields_set(dev_flow);
+		flow_dv_hashfields_set(dev_flow, rss_meta);
 	/* Register matcher. */
 	matcher.crc = rte_raw_cksum((const void *)matcher.mask.buf,
 				    matcher.mask.size);
@@ -8095,20 +8101,22 @@ struct field_modify_info modify_tcp[] = {
 		} else if (dh->fate_action == MLX5_FLOW_FATE_QUEUE) {
 			struct mlx5_hrxq *hrxq;
 			uint32_t hrxq_idx;
+			struct mlx5_flow_rss_meta *rss_meta =
+				(struct mlx5_flow_rss_meta *)priv->rss_meta;
 
-			MLX5_ASSERT(flow->rss.queue);
-			hrxq_idx = mlx5_hrxq_get(dev, flow->rss.key,
+			MLX5_ASSERT(rss_meta->queue);
+			hrxq_idx = mlx5_hrxq_get(dev, rss_meta->key,
 						 MLX5_RSS_HASH_KEY_LEN,
 						 dev_flow->hash_fields,
-						 (*flow->rss.queue),
-						 flow->rss.queue_num);
+						 (*rss_meta->queue),
+						 rss_meta->queue_num);
 			if (!hrxq_idx) {
 				hrxq_idx = mlx5_hrxq_new
-						(dev, flow->rss.key,
+						(dev, rss_meta->key,
 						MLX5_RSS_HASH_KEY_LEN,
 						dev_flow->hash_fields,
-						(*flow->rss.queue),
-						flow->rss.queue_num,
+						(*rss_meta->queue),
+						rss_meta->queue_num,
 						!!(dh->layers &
 						MLX5_FLOW_LAYER_TUNNEL));
 			}
diff --git a/drivers/net/mlx5/mlx5_flow_verbs.c b/drivers/net/mlx5/mlx5_flow_verbs.c
index 1d56b03..403a101 100644
--- a/drivers/net/mlx5/mlx5_flow_verbs.c
+++ b/drivers/net/mlx5/mlx5_flow_verbs.c
@@ -941,21 +941,20 @@
  * the input is valid and that there is space to insert the requested action
  * into the flow.
  *
- * @param[in] dev_flow
- *   Pointer to mlx5_flow.
+ * @param[in] rss_meta
+ *   Pointer to mlx5_flow_rss_meta.
  * @param[in] action
  *   Action configuration.
  */
 static void
-flow_verbs_translate_action_queue(struct mlx5_flow *dev_flow,
+flow_verbs_translate_action_queue(struct mlx5_flow_rss_meta *rss_meta,
 				  const struct rte_flow_action *action)
 {
 	const struct rte_flow_action_queue *queue = action->conf;
-	struct rte_flow *flow = dev_flow->flow;
 
-	if (flow->rss.queue)
-		(*flow->rss.queue)[0] = queue->index;
-	flow->rss.queue_num = 1;
+	if (rss_meta->queue)
+		(*rss_meta->queue)[0] = queue->index;
+	rss_meta->queue_num = 1;
 }
 
 /**
@@ -963,28 +962,25 @@
  * the input is valid and that there is space to insert the requested action
  * into the flow.
  *
+ * @param[in] rss_meta
+ *   Pointer to mlx5_flow_rss_meta.
  * @param[in] action
  *   Action configuration.
- * @param[in, out] action_flags
- *   Pointer to the detected actions.
- * @param[in] dev_flow
- *   Pointer to mlx5_flow.
  */
 static void
-flow_verbs_translate_action_rss(struct mlx5_flow *dev_flow,
+flow_verbs_translate_action_rss(struct mlx5_flow_rss_meta *rss_meta,
 				const struct rte_flow_action *action)
 {
 	const struct rte_flow_action_rss *rss = action->conf;
 	const uint8_t *rss_key;
-	struct rte_flow *flow = dev_flow->flow;
 
-	if (flow->rss.queue)
-		memcpy((*flow->rss.queue), rss->queue,
+	if (rss_meta->queue)
+		memcpy((*rss_meta->queue), rss->queue,
 		       rss->queue_num * sizeof(uint16_t));
-	flow->rss.queue_num = rss->queue_num;
+	rss_meta->queue_num = rss->queue_num;
 	/* NULL RSS key indicates default RSS key. */
 	rss_key = !rss->key ? rss_hash_default_key : rss->key;
-	memcpy(flow->rss.key, rss_key, MLX5_RSS_HASH_KEY_LEN);
+	memcpy(rss_meta->key, rss_key, MLX5_RSS_HASH_KEY_LEN);
 	/*
 	 * rss->level and rss.types should be set in advance when expanding
 	 * items for RSS.
@@ -1577,6 +1573,8 @@
 	uint64_t priority = attr->priority;
 	uint32_t subpriority = 0;
 	struct mlx5_priv *priv = dev->data->dev_private;
+	struct mlx5_flow_rss_meta *rss_meta = (struct mlx5_flow_rss_meta *)
+					      priv->rss_meta;
 
 	if (priority == MLX5_FLOW_PRIO_RSVD)
 		priority = priv->config.flow_prio - 1;
@@ -1602,12 +1600,14 @@
 			dev_flow->handle->fate_action = MLX5_FLOW_FATE_DROP;
 			break;
 		case RTE_FLOW_ACTION_TYPE_QUEUE:
-			flow_verbs_translate_action_queue(dev_flow, actions);
+			flow_verbs_translate_action_queue(rss_meta, actions);
 			action_flags |= MLX5_FLOW_ACTION_QUEUE;
+			dev_flow->flow->rss.queue_num = rss_meta->queue_num;
 			dev_flow->handle->fate_action = MLX5_FLOW_FATE_QUEUE;
 			break;
 		case RTE_FLOW_ACTION_TYPE_RSS:
-			flow_verbs_translate_action_rss(dev_flow, actions);
+			flow_verbs_translate_action_rss(rss_meta, actions);
+			dev_flow->flow->rss.queue_num = rss_meta->queue_num;
 			action_flags |= MLX5_FLOW_ACTION_RSS;
 			dev_flow->handle->fate_action = MLX5_FLOW_FATE_QUEUE;
 			break;
@@ -1655,7 +1655,7 @@
 			subpriority = MLX5_PRIORITY_MAP_L3;
 			dev_flow->hash_fields |=
 				mlx5_flow_hashfields_adjust
-					(dev_flow, tunnel,
+					(rss_meta, tunnel,
 					 MLX5_IPV4_LAYER_TYPES,
 					 MLX5_IPV4_IBV_RX_HASH);
 			item_flags |= tunnel ? MLX5_FLOW_LAYER_INNER_L3_IPV4 :
@@ -1667,7 +1667,7 @@
 			subpriority = MLX5_PRIORITY_MAP_L3;
 			dev_flow->hash_fields |=
 				mlx5_flow_hashfields_adjust
-					(dev_flow, tunnel,
+					(rss_meta, tunnel,
 					 MLX5_IPV6_LAYER_TYPES,
 					 MLX5_IPV6_IBV_RX_HASH);
 			item_flags |= tunnel ? MLX5_FLOW_LAYER_INNER_L3_IPV6 :
@@ -1679,7 +1679,7 @@
 			subpriority = MLX5_PRIORITY_MAP_L4;
 			dev_flow->hash_fields |=
 				mlx5_flow_hashfields_adjust
-					(dev_flow, tunnel, ETH_RSS_TCP,
+					(rss_meta, tunnel, ETH_RSS_TCP,
 					 (IBV_RX_HASH_SRC_PORT_TCP |
 					  IBV_RX_HASH_DST_PORT_TCP));
 			item_flags |= tunnel ? MLX5_FLOW_LAYER_INNER_L4_TCP :
@@ -1691,7 +1691,7 @@
 			subpriority = MLX5_PRIORITY_MAP_L4;
 			dev_flow->hash_fields |=
 				mlx5_flow_hashfields_adjust
-					(dev_flow, tunnel, ETH_RSS_UDP,
+					(rss_meta, tunnel, ETH_RSS_UDP,
 					 (IBV_RX_HASH_SRC_PORT_UDP |
 					  IBV_RX_HASH_DST_PORT_UDP));
 			item_flags |= tunnel ? MLX5_FLOW_LAYER_INNER_L4_UDP :
@@ -1848,19 +1848,21 @@
 			}
 		} else {
 			uint32_t hrxq_idx;
+			struct mlx5_flow_rss_meta *rss_meta =
+				(struct mlx5_flow_rss_meta *)priv->rss_meta;
 
-			MLX5_ASSERT(flow->rss.queue);
-			hrxq_idx = mlx5_hrxq_get(dev, flow->rss.key,
+			MLX5_ASSERT(rss_meta->queue);
+			hrxq_idx = mlx5_hrxq_get(dev, rss_meta->key,
 					     MLX5_RSS_HASH_KEY_LEN,
 					     dev_flow->hash_fields,
-					     (*flow->rss.queue),
-					     flow->rss.queue_num);
+					     (*rss_meta->queue),
+					     rss_meta->queue_num);
 			if (!hrxq_idx)
-				hrxq_idx = mlx5_hrxq_new(dev, flow->rss.key,
+				hrxq_idx = mlx5_hrxq_new(dev, rss_meta->key,
 						MLX5_RSS_HASH_KEY_LEN,
 						dev_flow->hash_fields,
-						(*flow->rss.queue),
-						flow->rss.queue_num,
+						(*rss_meta->queue),
+						rss_meta->queue_num,
 						!!(handle->layers &
 						MLX5_FLOW_LAYER_TUNNEL));
 			hrxq = mlx5_ipool_get(priv->sh->ipool[MLX5_IPOOL_HRXQ],
-- 
1.8.3.1


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

* [dpdk-dev] [PATCH 05/10] net/mlx5: allocate rte flow from indexed pool
  2020-04-15  6:39 [dpdk-dev] [PATCH 00/10] net/mlx5: optimize flow structure Suanming Mou
                   ` (3 preceding siblings ...)
  2020-04-15  6:39 ` [dpdk-dev] [PATCH 04/10] net/mlx5: optimize mlx5 flow RSS struct Suanming Mou
@ 2020-04-15  6:39 ` Suanming Mou
  2020-04-15  6:39 ` [dpdk-dev] [PATCH 06/10] net/mlx5: optimize flow meter handle type Suanming Mou
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Suanming Mou @ 2020-04-15  6:39 UTC (permalink / raw)
  To: Matan Azrad, Shahaf Shuler, Viacheslav Ovsiienko; +Cc: wentaoc, rasland, dev

Currently, rte flow with RSS action may have different queue number. The
indexed memory pool is not suitable for the flow with RSS action.

For flows without RSS action, the size are fixed. Allocate the none RSS
rte flow memory from indexed memory pool helps save MALLOC_ELEM_OVERHEAD
which is more than 64 bytes for rte flow.

Signed-off-by: Suanming Mou <suanmingm@mellanox.com>
---
 drivers/net/mlx5/mlx5.c      | 10 ++++++++++
 drivers/net/mlx5/mlx5.h      |  1 +
 drivers/net/mlx5/mlx5_flow.c | 24 ++++++++++++++++++------
 drivers/net/mlx5/mlx5_flow.h |  1 +
 4 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index 3ca2ed0..1493d25 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -277,6 +277,16 @@ struct mlx5_dev_spawn_data {
 		.free = rte_free,
 		.type = "mlx5_flow_handle_ipool",
 	},
+	{
+		.size = (sizeof(struct rte_flow) +
+			RTE_ALIGN_CEIL(sizeof(uint16_t), sizeof(void *))),
+		.trunk_size = 4096,
+		.need_lock = 1,
+		.release_mem_en = 1,
+		.malloc = rte_malloc_socket,
+		.free = rte_free,
+		.type = "rte_flow_ipool",
+	},
 };
 
 
diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index 41c87ec..378a13f 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -61,6 +61,7 @@ enum mlx5_ipool_index {
 #endif
 	MLX5_IPOOL_HRXQ, /* Pool for hrxq resource. */
 	MLX5_IPOOL_MLX5_FLOW, /* Pool for mlx5 flow handle. */
+	MLX5_IPOOL_RTE_FLOW, /* Pool for rte_flow. */
 	MLX5_IPOOL_MAX,
 };
 
diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 9f79031..cc2b207 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -4225,6 +4225,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 	struct mlx5_flow_rss_meta *rss_meta = priv->rss_meta;
 	const struct rte_flow_action *p_actions_rx = actions;
 	uint32_t i;
+	uint32_t idx = 0;
 	uint32_t flow_size;
 	int hairpin_flow = 0;
 	uint32_t hairpin_id = 0;
@@ -4247,17 +4248,21 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 	}
 	flow_size = sizeof(struct rte_flow);
 	rss = flow_get_rss_action(p_actions_rx);
-	if (rss)
+	if (rss) {
 		flow_size += RTE_ALIGN_CEIL(rss->queue_num * sizeof(uint16_t),
 					    sizeof(void *));
-	else
-		flow_size += RTE_ALIGN_CEIL(sizeof(uint16_t), sizeof(void *));
-	flow = rte_calloc(__func__, 1, flow_size, 0);
+		flow = rte_calloc(__func__, 1, flow_size, 0);
+	} else {
+		flow = mlx5_ipool_zmalloc(priv->sh->ipool[MLX5_IPOOL_RTE_FLOW],
+		       &idx);
+	}
 	if (!flow) {
 		rte_errno = ENOMEM;
 		goto error_before_flow;
 	}
 	flow->drv_type = flow_get_drv_type(dev, attr);
+	if (idx)
+		flow->idx = idx;
 	if (hairpin_id != 0)
 		flow->hairpin_flow_id = hairpin_id;
 	MLX5_ASSERT(flow->drv_type > MLX5_FLOW_TYPE_MIN &&
@@ -4368,7 +4373,10 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 	ret = rte_errno; /* Save rte_errno before cleanup. */
 	flow_mreg_del_copy_action(dev, flow);
 	flow_drv_destroy(dev, flow);
-	rte_free(flow);
+	if (idx)
+		mlx5_ipool_free(priv->sh->ipool[MLX5_IPOOL_RTE_FLOW], idx);
+	else
+		rte_free(flow);
 	rte_errno = ret; /* Restore rte_errno. */
 error_before_flow:
 	ret = rte_errno;
@@ -4492,7 +4500,11 @@ struct rte_flow *
 		TAILQ_REMOVE(list, flow, next);
 	flow_mreg_del_copy_action(dev, flow);
 	rte_free(flow->fdir);
-	rte_free(flow);
+	if (flow->idx)
+		mlx5_ipool_free(priv->sh->ipool[MLX5_IPOOL_RTE_FLOW],
+				flow->idx);
+	else
+		rte_free(flow);
 }
 
 /**
diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index 4516b51..e220647 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -765,6 +765,7 @@ struct rte_flow {
 	struct mlx5_fdir *fdir; /**< Pointer to associated FDIR if any. */
 	uint32_t hairpin_flow_id; /**< The flow id used for hairpin. */
 	uint32_t copy_applied:1; /**< The MARK copy Flow os applied. */
+	uint32_t idx; /**< Index to the rte flow allocated from indexed pool. */
 };
 
 typedef int (*mlx5_flow_validate_t)(struct rte_eth_dev *dev,
-- 
1.8.3.1


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

* [dpdk-dev] [PATCH 06/10] net/mlx5: optimize flow meter handle type
  2020-04-15  6:39 [dpdk-dev] [PATCH 00/10] net/mlx5: optimize flow structure Suanming Mou
                   ` (4 preceding siblings ...)
  2020-04-15  6:39 ` [dpdk-dev] [PATCH 05/10] net/mlx5: allocate rte flow from indexed pool Suanming Mou
@ 2020-04-15  6:39 ` Suanming Mou
  2020-04-15  6:39 ` [dpdk-dev] [PATCH 07/10] net/mlx5: allocate meter from indexed pool Suanming Mou
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Suanming Mou @ 2020-04-15  6:39 UTC (permalink / raw)
  To: Matan Azrad, Shahaf Shuler, Viacheslav Ovsiienko; +Cc: wentaoc, rasland, dev

While flow attaches the meter handle, the meter id can be the unique tag
for the flow to get the meter handle. It's no need for flow to save the
pointer of the meter handle.

Save the meter id instead of pointer helps reduce the size for rte flow
structure.

As the supported maximum meter rule is 4K, uint16_t type is selected for
the meter id.

Signed-off-by: Suanming Mou <suanmingm@mellanox.com>
---
 drivers/net/mlx5/mlx5_flow.h    |  2 +-
 drivers/net/mlx5/mlx5_flow_dv.c | 29 ++++++++++++++++++++++-------
 2 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index e220647..983ccc1 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -759,7 +759,7 @@ struct rte_flow {
 	uint32_t counter; /**< Holds flow counter. */
 	struct mlx5_flow_mreg_copy_resource *mreg_copy;
 	/**< pointer to metadata register copy table resource. */
-	struct mlx5_flow_meter *meter; /**< Holds flow meter. */
+	uint16_t meter; /**< Holds flow meter id. */
 	uint32_t dev_handles;
 	/**< Device flow handles that are part of the flow. */
 	struct mlx5_fdir *fdir; /**< Pointer to associated FDIR if any. */
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 26733e0..28010ca 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -7396,6 +7396,7 @@ struct field_modify_info modify_tcp[] = {
 		struct mlx5_flow_dv_port_id_action_resource port_id_resource;
 		int action_type = actions->type;
 		const struct rte_flow_action *found_action = NULL;
+		struct mlx5_flow_meter *fm = NULL;
 
 		switch (action_type) {
 		case RTE_FLOW_ACTION_TYPE_VOID:
@@ -7772,20 +7773,30 @@ struct field_modify_info modify_tcp[] = {
 		case RTE_FLOW_ACTION_TYPE_METER:
 			mtr = actions->conf;
 			if (!flow->meter) {
-				flow->meter = mlx5_flow_meter_attach(priv,
-							mtr->mtr_id, attr,
-							error);
-				if (!flow->meter)
+				fm = mlx5_flow_meter_attach(priv, mtr->mtr_id,
+							    attr, error);
+				if (!fm)
 					return rte_flow_error_set(error,
 						rte_errno,
 						RTE_FLOW_ERROR_TYPE_ACTION,
 						NULL,
 						"meter not found "
 						"or invalid parameters");
+				flow->meter = fm->meter_id;
 			}
 			/* Set the meter action. */
+			if (!fm) {
+				fm = mlx5_flow_meter_find(priv, flow->meter);
+				if (!fm)
+					return rte_flow_error_set(error,
+						rte_errno,
+						RTE_FLOW_ERROR_TYPE_ACTION,
+						NULL,
+						"meter not found "
+						"or invalid parameters");
+			}
 			dev_flow->dv.actions[actions_n++] =
-				flow->meter->mfts->meter_action;
+				fm->mfts->meter_action;
 			action_flags |= MLX5_FLOW_ACTION_METER;
 			break;
 		case RTE_FLOW_ACTION_TYPE_SET_IPV4_DSCP:
@@ -8472,8 +8483,12 @@ struct field_modify_info modify_tcp[] = {
 		flow->counter = 0;
 	}
 	if (flow->meter) {
-		mlx5_flow_meter_detach(flow->meter);
-		flow->meter = NULL;
+		struct mlx5_flow_meter *fm;
+
+		fm  = mlx5_flow_meter_find(priv, flow->meter);
+		if (fm)
+			mlx5_flow_meter_detach(fm);
+		flow->meter = 0;
 	}
 	while (flow->dev_handles) {
 		uint32_t tmp_idx = flow->dev_handles;
-- 
1.8.3.1


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

* [dpdk-dev] [PATCH 07/10] net/mlx5: allocate meter from indexed pool
  2020-04-15  6:39 [dpdk-dev] [PATCH 00/10] net/mlx5: optimize flow structure Suanming Mou
                   ` (5 preceding siblings ...)
  2020-04-15  6:39 ` [dpdk-dev] [PATCH 06/10] net/mlx5: optimize flow meter handle type Suanming Mou
@ 2020-04-15  6:39 ` Suanming Mou
  2020-04-15  6:39 ` [dpdk-dev] [PATCH 08/10] net/mlx5: allocate metadata object " Suanming Mou
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Suanming Mou @ 2020-04-15  6:39 UTC (permalink / raw)
  To: Matan Azrad, Shahaf Shuler, Viacheslav Ovsiienko; +Cc: wentaoc, rasland, dev

This patch allocate the meter object memory from indexed memory pool
which will help to save the MALLOC_ELEM_OVERHEAD memory taken by
rte_malloc().

Signed-off-by: Suanming Mou <suanmingm@mellanox.com>
---
 drivers/net/mlx5/mlx5.c            | 11 +++++++++++
 drivers/net/mlx5/mlx5.h            |  1 +
 drivers/net/mlx5/mlx5_flow.h       |  1 +
 drivers/net/mlx5/mlx5_flow_meter.c | 11 ++++++-----
 4 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index 1493d25..81a2e94 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -256,6 +256,17 @@ struct mlx5_dev_spawn_data {
 	},
 #endif
 	{
+		.size = sizeof(struct mlx5_flow_meter),
+		.trunk_size = 64,
+		.grow_trunk = 3,
+		.grow_shift = 2,
+		.need_lock = 0,
+		.release_mem_en = 1,
+		.malloc = rte_malloc_socket,
+		.free = rte_free,
+		.type = "mlx5_meter_ipool",
+	},
+	{
 		.size = (sizeof(struct mlx5_hrxq) + MLX5_RSS_HASH_KEY_LEN),
 		.trunk_size = 64,
 		.grow_trunk = 3,
diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index 378a13f..cf785f5 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -59,6 +59,7 @@ enum mlx5_ipool_index {
 	MLX5_IPOOL_PORT_ID, /* Pool for port id resource. */
 	MLX5_IPOOL_JUMP, /* Pool for jump resource. */
 #endif
+	MLX5_IPOOL_MTR, /* Pool for meter resource. */
 	MLX5_IPOOL_HRXQ, /* Pool for hrxq resource. */
 	MLX5_IPOOL_MLX5_FLOW, /* Pool for mlx5 flow handle. */
 	MLX5_IPOOL_RTE_FLOW, /* Pool for rte_flow. */
diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index 983ccc1..c8b2d20 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -705,6 +705,7 @@ struct mlx5_meter_domains_infos {
 struct mlx5_flow_meter {
 	TAILQ_ENTRY(mlx5_flow_meter) next;
 	/**< Pointer to the next flow meter structure. */
+	uint32_t idx; /* Index to meter object. */
 	uint32_t meter_id;
 	/**< Meter id. */
 	struct rte_mtr_params params;
diff --git a/drivers/net/mlx5/mlx5_flow_meter.c b/drivers/net/mlx5/mlx5_flow_meter.c
index aa0fd7a..b5fbf5d 100644
--- a/drivers/net/mlx5/mlx5_flow_meter.c
+++ b/drivers/net/mlx5/mlx5_flow_meter.c
@@ -631,6 +631,7 @@
 			};
 	int ret;
 	unsigned int i;
+	uint32_t idx = 0;
 
 	if (!priv->mtr_en)
 		return -rte_mtr_error_set(error, ENOTSUP,
@@ -647,12 +648,12 @@
 					  RTE_MTR_ERROR_TYPE_METER_PROFILE_ID,
 					  NULL, "Meter profile id not valid.");
 	/* Allocate the flow meter memory. */
-	fm = rte_calloc(__func__, 1,
-			sizeof(struct mlx5_flow_meter), RTE_CACHE_LINE_SIZE);
+	fm = mlx5_ipool_zmalloc(priv->sh->ipool[MLX5_IPOOL_MTR], &idx);
 	if (fm == NULL)
 		return -rte_mtr_error_set(error, ENOMEM,
 					  RTE_MTR_ERROR_TYPE_UNSPECIFIED, NULL,
 					  "Memory alloc failed for meter.");
+	fm->idx = idx;
 	/* Fill the flow meter parameters. */
 	fm->meter_id = meter_id;
 	fm->profile = fmp;
@@ -683,7 +684,7 @@
 	for (i = 0; i < RTE_DIM(fm->policer_stats.cnt); i++)
 		if (fm->policer_stats.cnt[i])
 			mlx5_counter_free(dev, fm->policer_stats.cnt[i]);
-	rte_free(fm);
+	mlx5_ipool_free(priv->sh->ipool[MLX5_IPOOL_MTR], idx);
 	return -rte_mtr_error_set(error, -ret,
 				  RTE_MTR_ERROR_TYPE_UNSPECIFIED,
 				  NULL, "Failed to create devx meter.");
@@ -746,7 +747,7 @@
 	/* Free meter flow table */
 	mlx5_flow_destroy_policer_rules(dev, fm, &attr);
 	mlx5_flow_destroy_mtr_tbls(dev, fm->mfts);
-	rte_free(fm);
+	mlx5_ipool_free(priv->sh->ipool[MLX5_IPOOL_MTR], fm->idx);
 	return 0;
 }
 
@@ -1274,7 +1275,7 @@ struct mlx5_flow_meter *
 		/* Free meter flow table. */
 		mlx5_flow_destroy_policer_rules(dev, fm, &attr);
 		mlx5_flow_destroy_mtr_tbls(dev, fm->mfts);
-		rte_free(fm);
+		mlx5_ipool_free(priv->sh->ipool[MLX5_IPOOL_MTR], fm->idx);
 	}
 	TAILQ_FOREACH_SAFE(fmp, fmps, next, tmp) {
 		/* Check unused. */
-- 
1.8.3.1


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

* [dpdk-dev] [PATCH 08/10] net/mlx5: allocate metadata object from indexed pool
  2020-04-15  6:39 [dpdk-dev] [PATCH 00/10] net/mlx5: optimize flow structure Suanming Mou
                   ` (6 preceding siblings ...)
  2020-04-15  6:39 ` [dpdk-dev] [PATCH 07/10] net/mlx5: allocate meter from indexed pool Suanming Mou
@ 2020-04-15  6:39 ` Suanming Mou
  2020-04-15  6:39 ` [dpdk-dev] [PATCH 09/10] net/mlx5: optimize flow director filter memory Suanming Mou
  2020-04-15  6:39 ` [dpdk-dev] [PATCH 10/10] net/mlx5: reorganize rte flow structure Suanming Mou
  9 siblings, 0 replies; 11+ messages in thread
From: Suanming Mou @ 2020-04-15  6:39 UTC (permalink / raw)
  To: Matan Azrad, Shahaf Shuler, Viacheslav Ovsiienko; +Cc: wentaoc, rasland, dev

Allocate metadata object from indexed pool helps rte flow saves the 4
bytes index instead of 8 bytes pointer. For metadata object itself, it
helps save MALLOC_ELEM_OVERHEAD bytes from rte_malloc().

Signed-off-by: Suanming Mou <suanmingm@mellanox.com>
---
 drivers/net/mlx5/mlx5.c      | 11 +++++++++++
 drivers/net/mlx5/mlx5.h      |  1 +
 drivers/net/mlx5/mlx5_flow.c | 40 ++++++++++++++++++++++++++++------------
 drivers/net/mlx5/mlx5_flow.h |  5 +++--
 4 files changed, 43 insertions(+), 14 deletions(-)

diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index 81a2e94..767fd9c 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -267,6 +267,17 @@ struct mlx5_dev_spawn_data {
 		.type = "mlx5_meter_ipool",
 	},
 	{
+		.size = sizeof(struct mlx5_flow_meter),
+		.trunk_size = 64,
+		.grow_trunk = 3,
+		.grow_shift = 2,
+		.need_lock = 0,
+		.release_mem_en = 1,
+		.malloc = rte_malloc_socket,
+		.free = rte_free,
+		.type = "mlx5_mcp_ipool",
+	},
+	{
 		.size = (sizeof(struct mlx5_hrxq) + MLX5_RSS_HASH_KEY_LEN),
 		.trunk_size = 64,
 		.grow_trunk = 3,
diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index cf785f5..1141935 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -60,6 +60,7 @@ enum mlx5_ipool_index {
 	MLX5_IPOOL_JUMP, /* Pool for jump resource. */
 #endif
 	MLX5_IPOOL_MTR, /* Pool for meter resource. */
+	MLX5_IPOOL_MCP, /* Pool for metadata resource. */
 	MLX5_IPOOL_HRXQ, /* Pool for hrxq resource. */
 	MLX5_IPOOL_MLX5_FLOW, /* Pool for mlx5 flow handle. */
 	MLX5_IPOOL_RTE_FLOW, /* Pool for rte_flow. */
diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index cc2b207..0d2e3df 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -2961,6 +2961,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 		[3] = { .type = RTE_FLOW_ACTION_TYPE_END, },
 	};
 	struct mlx5_flow_mreg_copy_resource *mcp_res;
+	uint32_t idx = 0;
 	int ret;
 
 	/* Fill the register fileds in the flow. */
@@ -3029,11 +3030,12 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 		};
 	}
 	/* Build a new entry. */
-	mcp_res = rte_zmalloc(__func__, sizeof(*mcp_res), 0);
+	mcp_res = mlx5_ipool_zmalloc(priv->sh->ipool[MLX5_IPOOL_MCP], &idx);
 	if (!mcp_res) {
 		rte_errno = ENOMEM;
 		return NULL;
 	}
+	mcp_res->idx = idx;
 	/*
 	 * The copy Flows are not included in any list. There
 	 * ones are referenced from other Flows and can not
@@ -3055,7 +3057,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 error:
 	if (mcp_res->flow)
 		flow_list_destroy(dev, NULL, mcp_res->flow);
-	rte_free(mcp_res);
+	mlx5_ipool_free(priv->sh->ipool[MLX5_IPOOL_MCP], mcp_res->idx);
 	return NULL;
 }
 
@@ -3071,9 +3073,13 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 flow_mreg_del_copy_action(struct rte_eth_dev *dev,
 			  struct rte_flow *flow)
 {
-	struct mlx5_flow_mreg_copy_resource *mcp_res = flow->mreg_copy;
+	struct mlx5_flow_mreg_copy_resource *mcp_res;
 	struct mlx5_priv *priv = dev->data->dev_private;
 
+	if (!flow->mreg_copy)
+		return;
+	mcp_res = mlx5_ipool_get(priv->sh->ipool[MLX5_IPOOL_MCP],
+				 flow->mreg_copy);
 	if (!mcp_res || !priv->mreg_cp_tbl)
 		return;
 	if (flow->copy_applied) {
@@ -3092,8 +3098,8 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 	MLX5_ASSERT(mcp_res->flow);
 	flow_list_destroy(dev, NULL, mcp_res->flow);
 	mlx5_hlist_remove(priv->mreg_cp_tbl, &mcp_res->hlist_ent);
-	rte_free(mcp_res);
-	flow->mreg_copy = NULL;
+	mlx5_ipool_free(priv->sh->ipool[MLX5_IPOOL_MCP], mcp_res->idx);
+	flow->mreg_copy = 0;
 }
 
 /**
@@ -3111,10 +3117,15 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 flow_mreg_start_copy_action(struct rte_eth_dev *dev,
 			    struct rte_flow *flow)
 {
-	struct mlx5_flow_mreg_copy_resource *mcp_res = flow->mreg_copy;
+	struct mlx5_flow_mreg_copy_resource *mcp_res;
+	struct mlx5_priv *priv = dev->data->dev_private;
 	int ret;
 
-	if (!mcp_res || flow->copy_applied)
+	if (!flow->mreg_copy || flow->copy_applied)
+		return 0;
+	mcp_res = mlx5_ipool_get(priv->sh->ipool[MLX5_IPOOL_MCP],
+				 flow->mreg_copy);
+	if (!mcp_res)
 		return 0;
 	if (!mcp_res->appcnt) {
 		ret = flow_drv_apply(dev, mcp_res->flow, NULL);
@@ -3138,9 +3149,14 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 flow_mreg_stop_copy_action(struct rte_eth_dev *dev,
 			   struct rte_flow *flow)
 {
-	struct mlx5_flow_mreg_copy_resource *mcp_res = flow->mreg_copy;
+	struct mlx5_flow_mreg_copy_resource *mcp_res;
+	struct mlx5_priv *priv = dev->data->dev_private;
 
-	if (!mcp_res || !flow->copy_applied)
+	if (!flow->mreg_copy || !flow->copy_applied)
+		return;
+	mcp_res = mlx5_ipool_get(priv->sh->ipool[MLX5_IPOOL_MCP],
+				 flow->mreg_copy);
+	if (!mcp_res)
 		return;
 	MLX5_ASSERT(mcp_res->appcnt);
 	--mcp_res->appcnt;
@@ -3171,7 +3187,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 	MLX5_ASSERT(mcp_res->flow);
 	flow_list_destroy(dev, NULL, mcp_res->flow);
 	mlx5_hlist_remove(priv->mreg_cp_tbl, &mcp_res->hlist_ent);
-	rte_free(mcp_res);
+	mlx5_ipool_free(priv->sh->ipool[MLX5_IPOOL_MCP], mcp_res->idx);
 }
 
 /**
@@ -3263,7 +3279,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 				(dev, MLX5_FLOW_MARK_DEFAULT, error);
 			if (!mcp_res)
 				return -rte_errno;
-			flow->mreg_copy = mcp_res;
+			flow->mreg_copy = mcp_res->idx;
 			if (dev->data->dev_started) {
 				mcp_res->appcnt++;
 				flow->copy_applied = 1;
@@ -3276,7 +3292,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 				flow_mreg_add_copy_action(dev, mark->id, error);
 			if (!mcp_res)
 				return -rte_errno;
-			flow->mreg_copy = mcp_res;
+			flow->mreg_copy = mcp_res->idx;
 			if (dev->data->dev_started) {
 				mcp_res->appcnt++;
 				flow->copy_applied = 1;
diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index c8b2d20..7611410 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -460,6 +460,7 @@ struct mlx5_flow_mreg_copy_resource {
 	/* List entry for device flows. */
 	uint32_t refcnt; /* Reference counter. */
 	uint32_t appcnt; /* Apply/Remove counter. */
+	uint32_t idx;
 	struct rte_flow *flow; /* Built flow for copy. */
 };
 
@@ -758,8 +759,8 @@ struct rte_flow {
 	enum mlx5_flow_drv_type drv_type; /**< Driver type. */
 	struct mlx5_flow_rss_queue rss; /**< RSS queue context. */
 	uint32_t counter; /**< Holds flow counter. */
-	struct mlx5_flow_mreg_copy_resource *mreg_copy;
-	/**< pointer to metadata register copy table resource. */
+	uint32_t mreg_copy;
+	/**< Index to metadata register copy table resource. */
 	uint16_t meter; /**< Holds flow meter id. */
 	uint32_t dev_handles;
 	/**< Device flow handles that are part of the flow. */
-- 
1.8.3.1


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

* [dpdk-dev] [PATCH 09/10] net/mlx5: optimize flow director filter memory
  2020-04-15  6:39 [dpdk-dev] [PATCH 00/10] net/mlx5: optimize flow structure Suanming Mou
                   ` (7 preceding siblings ...)
  2020-04-15  6:39 ` [dpdk-dev] [PATCH 08/10] net/mlx5: allocate metadata object " Suanming Mou
@ 2020-04-15  6:39 ` Suanming Mou
  2020-04-15  6:39 ` [dpdk-dev] [PATCH 10/10] net/mlx5: reorganize rte flow structure Suanming Mou
  9 siblings, 0 replies; 11+ messages in thread
From: Suanming Mou @ 2020-04-15  6:39 UTC (permalink / raw)
  To: Matan Azrad, Shahaf Shuler, Viacheslav Ovsiienko; +Cc: wentaoc, rasland, dev

From: Wentao Cui <wentaoc@mellanox.com>

This commit is for mlx5 fdir flow memory optimization.

Currently for the fdir member in rte_flow structure. It saves the fidr
memory pointer directly. As fidr is fading away, use one bit help to
indicate the function in the flow and add the content to an extra list
save the memory for the other widely usage cases.

Signed-off-by: Wentao Cui <wentaoc@mellanox.com>
Signed-off-by: Suanming Mou <suanmingm@mellanox.com>
---
 drivers/net/mlx5/mlx5.h      |  1 +
 drivers/net/mlx5/mlx5_flow.c | 62 ++++++++++++++++++++++++++++++++++++--------
 drivers/net/mlx5/mlx5_flow.h |  9 ++++++-
 3 files changed, 60 insertions(+), 12 deletions(-)

diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index 1141935..41be692 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -600,6 +600,7 @@ struct mlx5_priv {
 #endif
 	uint8_t skip_default_rss_reta; /* Skip configuration of default reta. */
 	uint8_t fdb_def_rule; /* Whether fdb jump to table 1 is configured. */
+	LIST_HEAD(fdir, mlx5_fdir_flow) fdir_flows; /* fdir flows. */
 };
 
 #define PORT_ID(priv) ((priv)->dev_data->port_id)
diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 0d2e3df..007e5c4 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -4501,6 +4501,7 @@ struct rte_flow *
 		  struct rte_flow *flow)
 {
 	struct mlx5_priv *priv = dev->data->dev_private;
+	struct mlx5_fdir_flow *priv_fdir_flow = NULL;
 
 	/*
 	 * Update RX queue flags only if port is started, otherwise it is
@@ -4515,7 +4516,17 @@ struct rte_flow *
 	if (list)
 		TAILQ_REMOVE(list, flow, next);
 	flow_mreg_del_copy_action(dev, flow);
-	rte_free(flow->fdir);
+	if (flow->fdir) {
+		LIST_FOREACH(priv_fdir_flow, &priv->fdir_flows, next) {
+			if (priv_fdir_flow->flow == flow)
+				break;
+		}
+		if (priv_fdir_flow) {
+			LIST_REMOVE(priv_fdir_flow, next);
+			rte_free(priv_fdir_flow->flow);
+			rte_free(priv_fdir_flow);
+		}
+	}
 	if (flow->idx)
 		mlx5_ipool_free(priv->sh->ipool[MLX5_IPOOL_RTE_FLOW],
 				flow->idx);
@@ -5211,12 +5222,14 @@ struct rte_flow *
 {
 	struct mlx5_priv *priv = dev->data->dev_private;
 	struct rte_flow *flow = NULL;
+	struct mlx5_fdir_flow *priv_fdir_flow = NULL;
 
 	MLX5_ASSERT(fdir_flow);
-	TAILQ_FOREACH(flow, &priv->flows, next) {
-		if (flow->fdir && !flow_fdir_cmp(flow->fdir, fdir_flow)) {
+	LIST_FOREACH(priv_fdir_flow, &priv->fdir_flows, next) {
+		if (!flow_fdir_cmp(priv_fdir_flow->fdir, fdir_flow)) {
 			DRV_LOG(DEBUG, "port %u found FDIR flow %p",
 				dev->data->port_id, (void *)flow);
+			flow = priv_fdir_flow->flow;
 			break;
 		}
 	}
@@ -5241,6 +5254,7 @@ struct rte_flow *
 	struct mlx5_priv *priv = dev->data->dev_private;
 	struct mlx5_fdir *fdir_flow;
 	struct rte_flow *flow;
+	struct mlx5_fdir_flow *priv_fdir_flow = NULL;
 	int ret;
 
 	fdir_flow = rte_zmalloc(__func__, sizeof(*fdir_flow), 0);
@@ -5256,17 +5270,26 @@ struct rte_flow *
 		rte_errno = EEXIST;
 		goto error;
 	}
+	priv_fdir_flow = rte_zmalloc(__func__, sizeof(struct mlx5_fdir_flow),
+				     0);
+	if (!priv_fdir_flow) {
+		rte_errno = ENOMEM;
+		goto error;
+	}
 	flow = flow_list_create(dev, &priv->flows, &fdir_flow->attr,
 				fdir_flow->items, fdir_flow->actions, true,
 				NULL);
 	if (!flow)
 		goto error;
-	MLX5_ASSERT(!flow->fdir);
-	flow->fdir = fdir_flow;
+	flow->fdir = 1;
+	priv_fdir_flow->fdir = fdir_flow;
+	priv_fdir_flow->flow = flow;
+	LIST_INSERT_HEAD(&priv->fdir_flows, priv_fdir_flow, next);
 	DRV_LOG(DEBUG, "port %u created FDIR flow %p",
 		dev->data->port_id, (void *)flow);
 	return 0;
 error:
+	rte_free(priv_fdir_flow);
 	rte_free(fdir_flow);
 	return -rte_errno;
 }
@@ -5291,17 +5314,26 @@ struct rte_flow *
 	struct mlx5_fdir fdir_flow = {
 		.attr.group = 0,
 	};
+	struct mlx5_fdir_flow *priv_fdir_flow = NULL;
 	int ret;
 
 	ret = flow_fdir_filter_convert(dev, fdir_filter, &fdir_flow);
 	if (ret)
 		return -rte_errno;
-	flow = flow_fdir_filter_lookup(dev, &fdir_flow);
-	if (!flow) {
-		rte_errno = ENOENT;
-		return -rte_errno;
+	LIST_FOREACH(priv_fdir_flow, &priv->fdir_flows, next) {
+		/* Find the fdir in priv list */
+		if (!flow_fdir_cmp(priv_fdir_flow->fdir, &fdir_flow))
+			break;
 	}
+	if (!priv_fdir_flow)
+		return 0;
+	LIST_REMOVE(priv_fdir_flow, next);
+	flow = priv_fdir_flow->flow;
+	/* Fdir resource will be releasd after flow destroy. */
+	flow->fdir = 0;
 	flow_list_destroy(dev, &priv->flows, flow);
+	rte_free(priv_fdir_flow->fdir);
+	rte_free(priv_fdir_flow);
 	DRV_LOG(DEBUG, "port %u deleted FDIR flow %p",
 		dev->data->port_id, (void *)flow);
 	return 0;
@@ -5340,8 +5372,16 @@ struct rte_flow *
 flow_fdir_filter_flush(struct rte_eth_dev *dev)
 {
 	struct mlx5_priv *priv = dev->data->dev_private;
-
-	mlx5_flow_list_flush(dev, &priv->flows, false);
+	struct mlx5_fdir_flow *priv_fdir_flow = NULL;
+
+	while (!LIST_EMPTY(&priv->fdir_flows)) {
+		priv_fdir_flow = LIST_FIRST(&priv->fdir_flows);
+		LIST_REMOVE(priv_fdir_flow, next);
+		priv_fdir_flow->flow->fdir = 0;
+		flow_list_destroy(dev, &priv->flows, priv_fdir_flow->flow);
+		rte_free(priv_fdir_flow->fdir);
+		rte_free(priv_fdir_flow);
+	}
 }
 
 /**
diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index 7611410..911007b 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -753,6 +753,13 @@ struct mlx5_flow_meter_profile {
 	uint32_t ref_cnt; /**< Use count. */
 };
 
+/* Fdir flow struture */
+struct mlx5_fdir_flow {
+	LIST_ENTRY(mlx5_fdir_flow) next; /* Pointer to the next element. */
+	struct mlx5_fdir *fdir; /* Pointer to fdir. */
+	struct rte_flow *flow; /* Pointer to flow. */
+};
+
 /* Flow structure. */
 struct rte_flow {
 	TAILQ_ENTRY(rte_flow) next; /**< Pointer to the next flow structure. */
@@ -764,7 +771,7 @@ struct rte_flow {
 	uint16_t meter; /**< Holds flow meter id. */
 	uint32_t dev_handles;
 	/**< Device flow handles that are part of the flow. */
-	struct mlx5_fdir *fdir; /**< Pointer to associated FDIR if any. */
+	uint32_t fdir:1; /**< Identifier of associated FDIR if any. */
 	uint32_t hairpin_flow_id; /**< The flow id used for hairpin. */
 	uint32_t copy_applied:1; /**< The MARK copy Flow os applied. */
 	uint32_t idx; /**< Index to the rte flow allocated from indexed pool. */
-- 
1.8.3.1


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

* [dpdk-dev] [PATCH 10/10] net/mlx5: reorganize rte flow structure
  2020-04-15  6:39 [dpdk-dev] [PATCH 00/10] net/mlx5: optimize flow structure Suanming Mou
                   ` (8 preceding siblings ...)
  2020-04-15  6:39 ` [dpdk-dev] [PATCH 09/10] net/mlx5: optimize flow director filter memory Suanming Mou
@ 2020-04-15  6:39 ` Suanming Mou
  9 siblings, 0 replies; 11+ messages in thread
From: Suanming Mou @ 2020-04-15  6:39 UTC (permalink / raw)
  To: Matan Azrad, Shahaf Shuler, Viacheslav Ovsiienko; +Cc: wentaoc, rasland, dev

Currently, the rte flow structure is not fully aligned and has some
bits wasted. The members can be optimized and reorganized to save
memory.

1. The drv_type uses only limited bits, change the type to 2 bits what
it needs.

2. Align the hairpin_flow_id, drv_type, fdir, copy_applied to 32 bits.
As hairpin never uses the full 32 bits.

3. The RSS queue number type is 16 bits, combine it with the 16 bits
meter id has a better organize for the struct.

4. __rte_packed helps tight up the structure memory layout.

The optimization totally helps save 20 bytes for the structure.

Signed-off-by: Suanming Mou <suanmingm@mellanox.com>
---
 drivers/net/mlx5/mlx5.c      |  3 ++-
 drivers/net/mlx5/mlx5_flow.h | 19 +++++++++++--------
 2 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index 767fd9c..8f4843e 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -764,7 +764,8 @@ struct mlx5_flow_id_pool *
 			goto error;
 		}
 	}
-	sh->flow_id_pool = mlx5_flow_id_pool_alloc(UINT32_MAX);
+	sh->flow_id_pool = mlx5_flow_id_pool_alloc
+					((1 << HAIRPIN_FLOW_ID_BITS) - 1);
 	if (!sh->flow_id_pool) {
 		DRV_LOG(ERR, "can't create flow id pool");
 		err = ENOMEM;
diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index 911007b..0d32896 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -760,22 +760,25 @@ struct mlx5_fdir_flow {
 	struct rte_flow *flow; /* Pointer to flow. */
 };
 
+#define HAIRPIN_FLOW_ID_BITS 28
+
 /* Flow structure. */
 struct rte_flow {
 	TAILQ_ENTRY(rte_flow) next; /**< Pointer to the next flow structure. */
-	enum mlx5_flow_drv_type drv_type; /**< Driver type. */
-	struct mlx5_flow_rss_queue rss; /**< RSS queue context. */
-	uint32_t counter; /**< Holds flow counter. */
-	uint32_t mreg_copy;
-	/**< Index to metadata register copy table resource. */
-	uint16_t meter; /**< Holds flow meter id. */
 	uint32_t dev_handles;
 	/**< Device flow handles that are part of the flow. */
+	uint32_t drv_type:2; /**< Driver type. */
 	uint32_t fdir:1; /**< Identifier of associated FDIR if any. */
-	uint32_t hairpin_flow_id; /**< The flow id used for hairpin. */
+	uint32_t hairpin_flow_id:HAIRPIN_FLOW_ID_BITS;
+	/**< The flow id used for hairpin. */
 	uint32_t copy_applied:1; /**< The MARK copy Flow os applied. */
+	uint32_t mreg_copy;
+	/**< Index to metadata register copy table resource. */
+	uint32_t counter; /**< Holds flow counter. */
+	uint16_t meter; /**< Holds flow meter id. */
+	struct mlx5_flow_rss_queue rss; /**< RSS queue context. */
 	uint32_t idx; /**< Index to the rte flow allocated from indexed pool. */
-};
+} __rte_packed;
 
 typedef int (*mlx5_flow_validate_t)(struct rte_eth_dev *dev,
 				    const struct rte_flow_attr *attr,
-- 
1.8.3.1


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

end of thread, other threads:[~2020-04-15  6:41 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-15  6:39 [dpdk-dev] [PATCH 00/10] net/mlx5: optimize flow structure Suanming Mou
2020-04-15  6:39 ` [dpdk-dev] [PATCH 01/10] net/mlx5: reorganize fate actions as union Suanming Mou
2020-04-15  6:39 ` [dpdk-dev] [PATCH 02/10] net/mlx5: optimize action flags in flow handle Suanming Mou
2020-04-15  6:39 ` [dpdk-dev] [PATCH 03/10] net/mlx5: reorganize the mlx5 flow handle struct Suanming Mou
2020-04-15  6:39 ` [dpdk-dev] [PATCH 04/10] net/mlx5: optimize mlx5 flow RSS struct Suanming Mou
2020-04-15  6:39 ` [dpdk-dev] [PATCH 05/10] net/mlx5: allocate rte flow from indexed pool Suanming Mou
2020-04-15  6:39 ` [dpdk-dev] [PATCH 06/10] net/mlx5: optimize flow meter handle type Suanming Mou
2020-04-15  6:39 ` [dpdk-dev] [PATCH 07/10] net/mlx5: allocate meter from indexed pool Suanming Mou
2020-04-15  6:39 ` [dpdk-dev] [PATCH 08/10] net/mlx5: allocate metadata object " Suanming Mou
2020-04-15  6:39 ` [dpdk-dev] [PATCH 09/10] net/mlx5: optimize flow director filter memory Suanming Mou
2020-04-15  6:39 ` [dpdk-dev] [PATCH 10/10] net/mlx5: reorganize rte flow structure Suanming Mou

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