DPDK patches and discussions
 help / color / mirror / Atom feed
From: Dariusz Sosnowski <dsosnowski@nvidia.com>
To: Viacheslav Ovsiienko <viacheslavo@nvidia.com>,
	Ori Kam <orika@nvidia.com>,  Suanming Mou <suanmingm@nvidia.com>,
	Matan Azrad <matan@nvidia.com>
Cc: <dev@dpdk.org>
Subject: [PATCH v3 4/4] net/mlx5: remove port from conntrack handle representation
Date: Tue, 27 Feb 2024 15:52:24 +0200	[thread overview]
Message-ID: <20240227135224.20066-5-dsosnowski@nvidia.com> (raw)
In-Reply-To: <20240227135224.20066-1-dsosnowski@nvidia.com>

This patch removes the owner port index from integer
representation of indirect action handle in mlx5 PMD for conntrack
flow actions.
This index is not needed when HW Steering flow engine is enabled,
because either:

- port references its own indirect actions or,
- port references indirect actions of the host port when sharing
  indirect actions was configured.

In both cases it is explicitly known which port owns the action.
Port index, included in action handle, introduced unnecessary
limitation and caused undefined behavior issues when application
used more than supported number of ports.

This patch removes the port index from indirect conntrack action handle
representation when HW steering flow engine is used.
It does not affect SW Steering flow engine.

Signed-off-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
Acked-by: Ori Kam <orika@nvidia.com>
---
 doc/guides/nics/mlx5.rst        |  2 +-
 drivers/net/mlx5/mlx5_flow.h    | 18 +++++++++++---
 drivers/net/mlx5/mlx5_flow_hw.c | 44 +++++++++++----------------------
 3 files changed, 29 insertions(+), 35 deletions(-)

diff --git a/doc/guides/nics/mlx5.rst b/doc/guides/nics/mlx5.rst
index db47d70b70..329b98f68f 100644
--- a/doc/guides/nics/mlx5.rst
+++ b/doc/guides/nics/mlx5.rst
@@ -815,7 +815,7 @@ Limitations
 
   - Cannot co-exist with ASO meter, ASO age action in a single flow rule.
   - Flow rules insertion rate and memory consumption need more optimization.
-  - 16 ports maximum.
+  - 16 ports maximum (with ``dv_flow_en=1``).
   - 32M connections maximum.
 
 - Multi-thread flow insertion:
diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index b4bf96cd64..187f440893 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -80,7 +80,12 @@ enum mlx5_indirect_type {
 #define MLX5_INDIRECT_ACT_CT_OWNER_SHIFT 25
 #define MLX5_INDIRECT_ACT_CT_OWNER_MASK (MLX5_INDIRECT_ACT_CT_MAX_PORT - 1)
 
-/* 29-31: type, 25-28: owner port, 0-24: index */
+/*
+ * When SW steering flow engine is used, the CT action handles are encoded in a following way:
+ * - bits 31:29 - type
+ * - bits 28:25 - port index of the action owner
+ * - bits 24:0 - action index
+ */
 #define MLX5_INDIRECT_ACT_CT_GEN_IDX(owner, index) \
 	((MLX5_INDIRECT_ACTION_TYPE_CT << MLX5_INDIRECT_ACTION_TYPE_OFFSET) | \
 	 (((owner) & MLX5_INDIRECT_ACT_CT_OWNER_MASK) << \
@@ -93,9 +98,14 @@ enum mlx5_indirect_type {
 #define MLX5_INDIRECT_ACT_CT_GET_IDX(index) \
 	((index) & ((1 << MLX5_INDIRECT_ACT_CT_OWNER_SHIFT) - 1))
 
-#define MLX5_ACTION_CTX_CT_GET_IDX  MLX5_INDIRECT_ACT_CT_GET_IDX
-#define MLX5_ACTION_CTX_CT_GET_OWNER MLX5_INDIRECT_ACT_CT_GET_OWNER
-#define MLX5_ACTION_CTX_CT_GEN_IDX MLX5_INDIRECT_ACT_CT_GEN_IDX
+/*
+ * When HW steering flow engine is used, the CT action handles are encoded in a following way:
+ * - bits 31:29 - type
+ * - bits 28:0 - action index
+ */
+#define MLX5_INDIRECT_ACT_HWS_CT_GEN_IDX(index) \
+	((struct rte_flow_action_handle *)(uintptr_t) \
+	 ((MLX5_INDIRECT_ACTION_TYPE_CT << MLX5_INDIRECT_ACTION_TYPE_OFFSET) | (index)))
 
 enum mlx5_indirect_list_type {
 	MLX5_INDIRECT_ACTION_LIST_TYPE_ERR = 0,
diff --git a/drivers/net/mlx5/mlx5_flow_hw.c b/drivers/net/mlx5/mlx5_flow_hw.c
index 2550e0604f..e48a927bf0 100644
--- a/drivers/net/mlx5/mlx5_flow_hw.c
+++ b/drivers/net/mlx5/mlx5_flow_hw.c
@@ -563,7 +563,7 @@ flow_hw_ct_compile(struct rte_eth_dev *dev,
 	struct mlx5_priv *priv = dev->data->dev_private;
 	struct mlx5_aso_ct_action *ct;
 
-	ct = mlx5_ipool_get(priv->hws_ctpool->cts, MLX5_ACTION_CTX_CT_GET_IDX(idx));
+	ct = mlx5_ipool_get(priv->hws_ctpool->cts, idx);
 	if (!ct || (!priv->shared_host && mlx5_aso_ct_available(priv->sh, queue, ct)))
 		return -1;
 	rule_act->action = priv->hws_ctpool->dr_action;
@@ -2462,8 +2462,7 @@ __flow_hw_actions_translate(struct rte_eth_dev *dev,
 			break;
 		case RTE_FLOW_ACTION_TYPE_CONNTRACK:
 			if (masks->conf) {
-				ct_idx = MLX5_ACTION_CTX_CT_GET_IDX
-					 ((uint32_t)(uintptr_t)actions->conf);
+				ct_idx = MLX5_INDIRECT_ACTION_IDX_GET(actions->conf);
 				if (flow_hw_ct_compile(dev, MLX5_HW_INV_QUEUE, ct_idx,
 						       &acts->rule_acts[dr_pos]))
 					goto err;
@@ -3180,8 +3179,7 @@ flow_hw_actions_construct(struct rte_eth_dev *dev,
 			job->flow->cnt_id = act_data->shared_counter.id;
 			break;
 		case RTE_FLOW_ACTION_TYPE_CONNTRACK:
-			ct_idx = MLX5_ACTION_CTX_CT_GET_IDX
-				 ((uint32_t)(uintptr_t)action->conf);
+			ct_idx = MLX5_INDIRECT_ACTION_IDX_GET(action->conf);
 			if (flow_hw_ct_compile(dev, queue, ct_idx,
 					       &rule_acts[act_data->action_dst]))
 				return -1;
@@ -3796,16 +3794,14 @@ flow_hw_pull_legacy_indirect_comp(struct rte_eth_dev *dev, struct mlx5_hw_q_job
 			aso_mtr = mlx5_ipool_get(priv->hws_mpool->idx_pool, idx);
 			aso_mtr->state = ASO_METER_READY;
 		} else if (type == MLX5_INDIRECT_ACTION_TYPE_CT) {
-			idx = MLX5_ACTION_CTX_CT_GET_IDX
-			((uint32_t)(uintptr_t)job->action);
+			idx = MLX5_INDIRECT_ACTION_IDX_GET(job->action);
 			aso_ct = mlx5_ipool_get(priv->hws_ctpool->cts, idx);
 			aso_ct->state = ASO_CONNTRACK_READY;
 		}
 	} else if (job->type == MLX5_HW_Q_JOB_TYPE_QUERY) {
 		type = MLX5_INDIRECT_ACTION_TYPE_GET(job->action);
 		if (type == MLX5_INDIRECT_ACTION_TYPE_CT) {
-			idx = MLX5_ACTION_CTX_CT_GET_IDX
-			((uint32_t)(uintptr_t)job->action);
+			idx = MLX5_INDIRECT_ACTION_IDX_GET(job->action);
 			aso_ct = mlx5_ipool_get(priv->hws_ctpool->cts, idx);
 			mlx5_aso_ct_obj_analyze(job->query.user,
 						job->query.hw);
@@ -10225,7 +10221,6 @@ flow_hw_conntrack_destroy(struct rte_eth_dev *dev,
 			  uint32_t idx,
 			  struct rte_flow_error *error)
 {
-	uint32_t ct_idx = MLX5_ACTION_CTX_CT_GET_IDX(idx);
 	struct mlx5_priv *priv = dev->data->dev_private;
 	struct mlx5_aso_ct_pool *pool = priv->hws_ctpool;
 	struct mlx5_aso_ct_action *ct;
@@ -10235,7 +10230,7 @@ flow_hw_conntrack_destroy(struct rte_eth_dev *dev,
 				RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
 				NULL,
 				"CT destruction is not allowed to guest port");
-	ct = mlx5_ipool_get(pool->cts, ct_idx);
+	ct = mlx5_ipool_get(pool->cts, idx);
 	if (!ct) {
 		return rte_flow_error_set(error, EINVAL,
 				RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
@@ -10244,7 +10239,7 @@ flow_hw_conntrack_destroy(struct rte_eth_dev *dev,
 	}
 	__atomic_store_n(&ct->state, ASO_CONNTRACK_FREE,
 				 __ATOMIC_RELAXED);
-	mlx5_ipool_free(pool->cts, ct_idx);
+	mlx5_ipool_free(pool->cts, idx);
 	return 0;
 }
 
@@ -10257,15 +10252,13 @@ flow_hw_conntrack_query(struct rte_eth_dev *dev, uint32_t queue, uint32_t idx,
 	struct mlx5_priv *priv = dev->data->dev_private;
 	struct mlx5_aso_ct_pool *pool = priv->hws_ctpool;
 	struct mlx5_aso_ct_action *ct;
-	uint32_t ct_idx;
 
 	if (priv->shared_host)
 		return rte_flow_error_set(error, ENOTSUP,
 				RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
 				NULL,
 				"CT query is not allowed to guest port");
-	ct_idx = MLX5_ACTION_CTX_CT_GET_IDX(idx);
-	ct = mlx5_ipool_get(pool->cts, ct_idx);
+	ct = mlx5_ipool_get(pool->cts, idx);
 	if (!ct) {
 		return rte_flow_error_set(error, EINVAL,
 				RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
@@ -10293,7 +10286,6 @@ flow_hw_conntrack_update(struct rte_eth_dev *dev, uint32_t queue,
 	struct mlx5_aso_ct_pool *pool = priv->hws_ctpool;
 	struct mlx5_aso_ct_action *ct;
 	const struct rte_flow_action_conntrack *new_prf;
-	uint32_t ct_idx;
 	int ret = 0;
 
 	if (priv->shared_host)
@@ -10301,8 +10293,7 @@ flow_hw_conntrack_update(struct rte_eth_dev *dev, uint32_t queue,
 				RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
 				NULL,
 				"CT update is not allowed to guest port");
-	ct_idx = MLX5_ACTION_CTX_CT_GET_IDX(idx);
-	ct = mlx5_ipool_get(pool->cts, ct_idx);
+	ct = mlx5_ipool_get(pool->cts, idx);
 	if (!ct) {
 		return rte_flow_error_set(error, EINVAL,
 				RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
@@ -10363,13 +10354,6 @@ flow_hw_conntrack_create(struct rte_eth_dev *dev, uint32_t queue,
 				   "CT is not enabled");
 		return 0;
 	}
-	if (dev->data->port_id >= MLX5_INDIRECT_ACT_CT_MAX_PORT) {
-		rte_flow_error_set(error, EINVAL,
-				   RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
-				   "CT supports port indexes up to "
-				   RTE_STR(MLX5_ACTION_CTX_CT_MAX_PORT));
-		return 0;
-	}
 	ct = mlx5_ipool_zmalloc(pool->cts, &ct_idx);
 	if (!ct) {
 		rte_flow_error_set(error, rte_errno,
@@ -10399,8 +10383,7 @@ flow_hw_conntrack_create(struct rte_eth_dev *dev, uint32_t queue,
 			return 0;
 		}
 	}
-	return (struct rte_flow_action_handle *)(uintptr_t)
-		MLX5_ACTION_CTX_CT_GEN_IDX(PORT_ID(priv), ct_idx);
+	return MLX5_INDIRECT_ACT_HWS_CT_GEN_IDX(ct_idx);
 }
 
 /**
@@ -10741,7 +10724,7 @@ flow_hw_action_handle_update(struct rte_eth_dev *dev, uint32_t queue,
 	case MLX5_INDIRECT_ACTION_TYPE_CT:
 		if (ct_conf->state)
 			aso = true;
-		ret = flow_hw_conntrack_update(dev, queue, update, act_idx,
+		ret = flow_hw_conntrack_update(dev, queue, update, idx,
 					       job, push, error);
 		break;
 	case MLX5_INDIRECT_ACTION_TYPE_METER_MARK:
@@ -10830,7 +10813,7 @@ flow_hw_action_handle_destroy(struct rte_eth_dev *dev, uint32_t queue,
 		mlx5_hws_cnt_shared_put(priv->hws_cpool, &act_idx);
 		break;
 	case MLX5_INDIRECT_ACTION_TYPE_CT:
-		ret = flow_hw_conntrack_destroy(dev, act_idx, error);
+		ret = flow_hw_conntrack_destroy(dev, idx, error);
 		break;
 	case MLX5_INDIRECT_ACTION_TYPE_METER_MARK:
 		aso_mtr = mlx5_ipool_get(pool->idx_pool, idx);
@@ -11116,6 +11099,7 @@ flow_hw_action_handle_query(struct rte_eth_dev *dev, uint32_t queue,
 	struct mlx5_hw_q_job *job = NULL;
 	uint32_t act_idx = (uint32_t)(uintptr_t)handle;
 	uint32_t type = act_idx >> MLX5_INDIRECT_ACTION_TYPE_OFFSET;
+	uint32_t idx = MLX5_INDIRECT_ACTION_IDX_GET(handle);
 	uint32_t age_idx = act_idx & MLX5_HWS_AGE_IDX_MASK;
 	int ret;
 	bool push = flow_hw_action_push(attr);
@@ -11139,7 +11123,7 @@ flow_hw_action_handle_query(struct rte_eth_dev *dev, uint32_t queue,
 		aso = true;
 		if (job)
 			job->query.user = data;
-		ret = flow_hw_conntrack_query(dev, queue, act_idx, data,
+		ret = flow_hw_conntrack_query(dev, queue, idx, data,
 					      job, push, error);
 		break;
 	case MLX5_INDIRECT_ACTION_TYPE_QUOTA:
-- 
2.25.1


  parent reply	other threads:[~2024-02-27 13:53 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-21 10:01 [PATCH 0/4] net/mlx5: connection tracking changes Dariusz Sosnowski
2024-02-21 10:01 ` [PATCH 1/4] net/mlx5: fix conntrack action handle representation Dariusz Sosnowski
2024-02-21 10:01 ` [PATCH 2/4] net/mlx5: fix connection tracking action validation Dariusz Sosnowski
2024-02-21 10:01 ` [PATCH 3/4] net/mlx5: add cross port CT object sharing Dariusz Sosnowski
2024-02-21 10:01 ` [PATCH 4/4] net/mlx5: remove port from conntrack handle representation Dariusz Sosnowski
2024-02-23 14:23 ` [PATCH v2 0/4] net/mlx5: connection tracking changes Dariusz Sosnowski
2024-02-23 14:23   ` [PATCH v2 1/4] net/mlx5: fix conntrack action handle representation Dariusz Sosnowski
2024-02-23 14:23   ` [PATCH v2 2/4] net/mlx5: fix connection tracking action validation Dariusz Sosnowski
2024-02-23 14:23   ` [PATCH v2 3/4] net/mlx5: add cross port CT object sharing Dariusz Sosnowski
2024-02-23 14:23   ` [PATCH v2 4/4] net/mlx5: remove port from conntrack handle representation Dariusz Sosnowski
2024-02-27 13:52   ` [PATCH v3 0/4] net/mlx5: connection tracking changes Dariusz Sosnowski
2024-02-27 13:52     ` [PATCH v3 1/4] net/mlx5: fix conntrack action handle representation Dariusz Sosnowski
2024-02-27 13:52     ` [PATCH v3 2/4] net/mlx5: fix connection tracking action validation Dariusz Sosnowski
2024-02-27 13:52     ` [PATCH v3 3/4] net/mlx5: add cross port CT object sharing Dariusz Sosnowski
2024-02-27 13:52     ` Dariusz Sosnowski [this message]
2024-02-28 10:12     ` [PATCH v3 0/4] net/mlx5: connection tracking changes Raslan Darawsheh

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240227135224.20066-5-dsosnowski@nvidia.com \
    --to=dsosnowski@nvidia.com \
    --cc=dev@dpdk.org \
    --cc=matan@nvidia.com \
    --cc=orika@nvidia.com \
    --cc=suanmingm@nvidia.com \
    --cc=viacheslavo@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).