From: Dekel Peled <dekelp@nvidia.com>
To: matan@nvidia.com, shahafs@nvidia.com, viacheslavo@nvidia.com
Cc: dev@dpdk.org, stable@dpdk.org
Subject: [dpdk-dev] [PATCH] net/mlx5: fix shared RSS translation and cleanup
Date: Mon, 1 Feb 2021 11:28:57 +0200 [thread overview]
Message-ID: <ef797b9b97e64be8cc4d731f087a0a3029f8af6d.1612171517.git.dekelp@nvidia.com> (raw)
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
next reply other threads:[~2021-02-01 9:29 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-02-01 9:28 Dekel Peled [this message]
2021-02-01 13:24 ` 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=ef797b9b97e64be8cc4d731f087a0a3029f8af6d.1612171517.git.dekelp@nvidia.com \
--to=dekelp@nvidia.com \
--cc=dev@dpdk.org \
--cc=matan@nvidia.com \
--cc=shahafs@nvidia.com \
--cc=stable@dpdk.org \
--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).