DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] net/mlx5: fix shared RSS translation and cleanup
@ 2021-02-01  9:28 Dekel Peled
  2021-02-01 13:24 ` Raslan Darawsheh
  0 siblings, 1 reply; 2+ messages in thread
From: Dekel Peled @ 2021-02-01  9:28 UTC (permalink / raw)
  To: matan, shahafs, viacheslavo; +Cc: dev, stable

This patch includes several updates of the shared RSS action:

(1)
The shared RSS action, introduced recently, uses existing definitions
of the regular RSS action.
The new defined value MLX5_RSS_HASH_IPV4_TCP uses existing definition
IBV_RX_HASH_SRC_PORT_TCP twice, instead of using
IBV_RX_HASH_SRC_PORT_TCP and IBV_RX_HASH_DST_PORT_TCP.
            ---                          ---
The same is true for IPv4-UDP, IPv6-TCP, IPv6-UDP.
As result, a shared RSS action with L4 type is specified as src-only.
Flow rule using such shared action, while specifying L4 item in flow
pattern, will fail to create.
This patch updates the new definitions, to use the existing values
correctly.

(2)
On shared RSS action destroy, in function __flow_dv_action_rss_release,
the indirection table shared_rss->ind_tbl was released before
shared_rss->refcnt was checked.
This order is incorrect, since the indirection table should be
released only when the shared RSS action is destroyed.
This patch puts release function calls in correct order.

(3)
Variables declared of type "struct mlx5_shared_action_rss" are named
"shared_rss", "action", and "shared_action".
To improve code readability, this patch renames all to "shared_rss".

Fixes: d7cfcddded61 ("net/mlx5: translate shared action for RSS action")
Fixes: d2046c09aa64 ("net/mlx5: support shared action for RSS")
Cc: stable@dpdk.org

Signed-off-by: Dekel Peled <dekelp@nvidia.com>
Acked-by: Matan Azrad <matan@nvidia.com>
---
 drivers/net/mlx5/mlx5_flow.c    |  4 +-
 drivers/net/mlx5/mlx5_flow.h    |  8 ++--
 drivers/net/mlx5/mlx5_flow_dv.c | 78 ++++++++++++++++-----------------
 3 files changed, 45 insertions(+), 45 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 0197a07209..632f46dfde 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -7452,12 +7452,12 @@ mlx5_shared_action_flush(struct rte_eth_dev *dev)
 {
 	struct rte_flow_error error;
 	struct mlx5_priv *priv = dev->data->dev_private;
-	struct mlx5_shared_action_rss *action;
+	struct mlx5_shared_action_rss *shared_rss;
 	int ret = 0;
 	uint32_t idx;
 
 	ILIST_FOREACH(priv->sh->ipool[MLX5_IPOOL_RSS_SHARED_ACTIONS],
-		      priv->rss_shared_actions, idx, action, next) {
+		      priv->rss_shared_actions, idx, shared_rss, next) {
 		ret |= mlx5_shared_action_destroy(dev,
 		       (struct rte_flow_shared_action *)(uintptr_t)idx, &error);
 	}
diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index a3a3c7f54f..8324e188e1 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -1095,17 +1095,17 @@ struct rte_flow {
 #define MLX5_RSS_HASH_IPV4 (IBV_RX_HASH_SRC_IPV4 | IBV_RX_HASH_DST_IPV4)
 #define MLX5_RSS_HASH_IPV4_TCP \
 	(MLX5_RSS_HASH_IPV4 | \
-	 IBV_RX_HASH_SRC_PORT_TCP | IBV_RX_HASH_SRC_PORT_TCP)
+	 IBV_RX_HASH_SRC_PORT_TCP | IBV_RX_HASH_DST_PORT_TCP)
 #define MLX5_RSS_HASH_IPV4_UDP \
 	(MLX5_RSS_HASH_IPV4 | \
-	 IBV_RX_HASH_SRC_PORT_UDP | IBV_RX_HASH_SRC_PORT_UDP)
+	 IBV_RX_HASH_SRC_PORT_UDP | IBV_RX_HASH_DST_PORT_UDP)
 #define MLX5_RSS_HASH_IPV6 (IBV_RX_HASH_SRC_IPV6 | IBV_RX_HASH_DST_IPV6)
 #define MLX5_RSS_HASH_IPV6_TCP \
 	(MLX5_RSS_HASH_IPV6 | \
-	 IBV_RX_HASH_SRC_PORT_TCP | IBV_RX_HASH_SRC_PORT_TCP)
+	 IBV_RX_HASH_SRC_PORT_TCP | IBV_RX_HASH_DST_PORT_TCP)
 #define MLX5_RSS_HASH_IPV6_UDP \
 	(MLX5_RSS_HASH_IPV6 | \
-	 IBV_RX_HASH_SRC_PORT_UDP | IBV_RX_HASH_SRC_PORT_UDP)
+	 IBV_RX_HASH_SRC_PORT_UDP | IBV_RX_HASH_DST_PORT_UDP)
 #define MLX5_RSS_HASH_NONE 0ULL
 
 /* array of valid combinations of RX Hash fields for RSS */
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 8c11ac306f..e0874e3f5f 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -12552,10 +12552,10 @@ __flow_dv_hrxqs_release(struct rte_eth_dev *dev,
  */
 static int
 __flow_dv_action_rss_hrxqs_release(struct rte_eth_dev *dev,
-				 struct mlx5_shared_action_rss *action)
+				 struct mlx5_shared_action_rss *shared_rss)
 {
-	return __flow_dv_hrxqs_release(dev, &action->hrxq) +
-		__flow_dv_hrxqs_release(dev, &action->hrxq_tunnel);
+	return __flow_dv_hrxqs_release(dev, &shared_rss->hrxq) +
+		__flow_dv_hrxqs_release(dev, &shared_rss->hrxq_tunnel);
 }
 
 /**
@@ -12579,25 +12579,25 @@ __flow_dv_action_rss_hrxqs_release(struct rte_eth_dev *dev,
 static int
 __flow_dv_action_rss_setup(struct rte_eth_dev *dev,
 			   uint32_t action_idx,
-			   struct mlx5_shared_action_rss *action,
+			   struct mlx5_shared_action_rss *shared_rss,
 			   struct rte_flow_error *error)
 {
 	struct mlx5_flow_rss_desc rss_desc = { 0 };
 	size_t i;
 	int err;
 
-	if (mlx5_ind_table_obj_setup(dev, action->ind_tbl)) {
+	if (mlx5_ind_table_obj_setup(dev, shared_rss->ind_tbl)) {
 		return rte_flow_error_set(error, rte_errno,
 					  RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
 					  "cannot setup indirection table");
 	}
-	memcpy(rss_desc.key, action->origin.key, MLX5_RSS_HASH_KEY_LEN);
+	memcpy(rss_desc.key, shared_rss->origin.key, MLX5_RSS_HASH_KEY_LEN);
 	rss_desc.key_len = MLX5_RSS_HASH_KEY_LEN;
-	rss_desc.const_q = action->origin.queue;
-	rss_desc.queue_num = action->origin.queue_num;
+	rss_desc.const_q = shared_rss->origin.queue;
+	rss_desc.queue_num = shared_rss->origin.queue_num;
 	/* Set non-zero value to indicate a shared RSS. */
 	rss_desc.shared_rss = action_idx;
-	rss_desc.ind_tbl = action->ind_tbl;
+	rss_desc.ind_tbl = shared_rss->ind_tbl;
 	for (i = 0; i < MLX5_RSS_HASH_FIELDS_LEN; i++) {
 		uint32_t hrxq_idx;
 		uint64_t hash_fields = mlx5_rss_hash_fields[i];
@@ -12615,16 +12615,16 @@ __flow_dv_action_rss_setup(struct rte_eth_dev *dev,
 				goto error_hrxq_new;
 			}
 			err = __flow_dv_action_rss_hrxq_set
-				(action, hash_fields, tunnel, hrxq_idx);
+				(shared_rss, hash_fields, tunnel, hrxq_idx);
 			MLX5_ASSERT(!err);
 		}
 	}
 	return 0;
 error_hrxq_new:
 	err = rte_errno;
-	__flow_dv_action_rss_hrxqs_release(dev, action);
-	if (!mlx5_ind_table_obj_release(dev, action->ind_tbl, true))
-		action->ind_tbl = NULL;
+	__flow_dv_action_rss_hrxqs_release(dev, shared_rss);
+	if (!mlx5_ind_table_obj_release(dev, shared_rss->ind_tbl, true))
+		shared_rss->ind_tbl = NULL;
 	rte_errno = err;
 	return -rte_errno;
 }
@@ -12653,7 +12653,7 @@ __flow_dv_action_rss_create(struct rte_eth_dev *dev,
 			    struct rte_flow_error *error)
 {
 	struct mlx5_priv *priv = dev->data->dev_private;
-	struct mlx5_shared_action_rss *shared_action = NULL;
+	struct mlx5_shared_action_rss *shared_rss = NULL;
 	void *queue = NULL;
 	struct rte_flow_action_rss *origin;
 	const uint8_t *rss_key;
@@ -12663,9 +12663,9 @@ __flow_dv_action_rss_create(struct rte_eth_dev *dev,
 	RTE_SET_USED(conf);
 	queue = mlx5_malloc(0, RTE_ALIGN_CEIL(queue_size, sizeof(void *)),
 			    0, SOCKET_ID_ANY);
-	shared_action = mlx5_ipool_zmalloc
+	shared_rss = mlx5_ipool_zmalloc
 			 (priv->sh->ipool[MLX5_IPOOL_RSS_SHARED_ACTIONS], &idx);
-	if (!shared_action || !queue) {
+	if (!shared_rss || !queue) {
 		rte_flow_error_set(error, ENOMEM,
 				   RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
 				   "cannot allocate resource memory");
@@ -12677,43 +12677,43 @@ __flow_dv_action_rss_create(struct rte_eth_dev *dev,
 				   "rss action number out of range");
 		goto error_rss_init;
 	}
-	shared_action->ind_tbl = mlx5_malloc(MLX5_MEM_ZERO,
-					     sizeof(*shared_action->ind_tbl),
-					     0, SOCKET_ID_ANY);
-	if (!shared_action->ind_tbl) {
+	shared_rss->ind_tbl = mlx5_malloc(MLX5_MEM_ZERO,
+					  sizeof(*shared_rss->ind_tbl),
+					  0, SOCKET_ID_ANY);
+	if (!shared_rss->ind_tbl) {
 		rte_flow_error_set(error, ENOMEM,
 				   RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
 				   "cannot allocate resource memory");
 		goto error_rss_init;
 	}
 	memcpy(queue, rss->queue, queue_size);
-	shared_action->ind_tbl->queues = queue;
-	shared_action->ind_tbl->queues_n = rss->queue_num;
-	origin = &shared_action->origin;
+	shared_rss->ind_tbl->queues = queue;
+	shared_rss->ind_tbl->queues_n = rss->queue_num;
+	origin = &shared_rss->origin;
 	origin->func = rss->func;
 	origin->level = rss->level;
 	/* RSS type 0 indicates default RSS type (ETH_RSS_IP). */
 	origin->types = !rss->types ? ETH_RSS_IP : rss->types;
 	/* NULL RSS key indicates default RSS key. */
 	rss_key = !rss->key ? rss_hash_default_key : rss->key;
-	memcpy(shared_action->key, rss_key, MLX5_RSS_HASH_KEY_LEN);
-	origin->key = &shared_action->key[0];
+	memcpy(shared_rss->key, rss_key, MLX5_RSS_HASH_KEY_LEN);
+	origin->key = &shared_rss->key[0];
 	origin->key_len = MLX5_RSS_HASH_KEY_LEN;
 	origin->queue = queue;
 	origin->queue_num = rss->queue_num;
-	if (__flow_dv_action_rss_setup(dev, idx, shared_action, error))
+	if (__flow_dv_action_rss_setup(dev, idx, shared_rss, error))
 		goto error_rss_init;
-	rte_spinlock_init(&shared_action->action_rss_sl);
-	__atomic_add_fetch(&shared_action->refcnt, 1, __ATOMIC_RELAXED);
+	rte_spinlock_init(&shared_rss->action_rss_sl);
+	__atomic_add_fetch(&shared_rss->refcnt, 1, __ATOMIC_RELAXED);
 	rte_spinlock_lock(&priv->shared_act_sl);
 	ILIST_INSERT(priv->sh->ipool[MLX5_IPOOL_RSS_SHARED_ACTIONS],
-		     &priv->rss_shared_actions, idx, shared_action, next);
+		     &priv->rss_shared_actions, idx, shared_rss, next);
 	rte_spinlock_unlock(&priv->shared_act_sl);
 	return idx;
 error_rss_init:
-	if (shared_action) {
-		if (shared_action->ind_tbl)
-			mlx5_free(shared_action->ind_tbl);
+	if (shared_rss) {
+		if (shared_rss->ind_tbl)
+			mlx5_free(shared_rss->ind_tbl);
 		mlx5_ipool_free(priv->sh->ipool[MLX5_IPOOL_RSS_SHARED_ACTIONS],
 				idx);
 	}
@@ -12758,6 +12758,13 @@ __flow_dv_action_rss_release(struct rte_eth_dev *dev, uint32_t idx,
 					  RTE_FLOW_ERROR_TYPE_ACTION,
 					  NULL,
 					  "shared rss hrxq has references");
+	if (!__atomic_compare_exchange_n(&shared_rss->refcnt, &old_refcnt,
+					 0, 0, __ATOMIC_ACQUIRE,
+					 __ATOMIC_RELAXED))
+		return rte_flow_error_set(error, EBUSY,
+					  RTE_FLOW_ERROR_TYPE_ACTION,
+					  NULL,
+					  "shared rss has references");
 	queue = shared_rss->ind_tbl->queues;
 	remaining = mlx5_ind_table_obj_release(dev, shared_rss->ind_tbl, true);
 	if (remaining)
@@ -12766,13 +12773,6 @@ __flow_dv_action_rss_release(struct rte_eth_dev *dev, uint32_t idx,
 					  NULL,
 					  "shared rss indirection table has"
 					  " references");
-	if (!__atomic_compare_exchange_n(&shared_rss->refcnt, &old_refcnt,
-					 0, 0, __ATOMIC_ACQUIRE,
-					 __ATOMIC_RELAXED))
-		return rte_flow_error_set(error, EBUSY,
-					  RTE_FLOW_ERROR_TYPE_ACTION,
-					  NULL,
-					  "shared rss has references");
 	mlx5_free(queue);
 	rte_spinlock_lock(&priv->shared_act_sl);
 	ILIST_REMOVE(priv->sh->ipool[MLX5_IPOOL_RSS_SHARED_ACTIONS],
-- 
2.25.1


^ permalink raw reply	[flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-02-01 13:25 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-01  9:28 [dpdk-dev] [PATCH] net/mlx5: fix shared RSS translation and cleanup Dekel Peled
2021-02-01 13:24 ` Raslan Darawsheh

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).