mlx5_fs_get_pr_encap_pool() inserts a new pool when the lookup misses. Two callers allocating the first reformat of a size at once both create one and the second fails with -EBUSY. TC serializes these allocations under encap_tbl_lock while RDMA flow actions on the FDB don't. Sashiko AI review of an earlier series spotted this issue. Tested for regressions on ConnectX-7 with firmware 28.48.1000. Fixes: aecd9d1020e3 ("net/mlx5: fs, add HWS packet reformat API function") Reviewed-by: Moshe Shemesh Signed-off-by: Alexandre Cassen --- .../mellanox/mlx5/core/steering/hws/fs_hws.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/fs_hws.c b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/fs_hws.c index 5a172c572a68..66edb42268cb 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/fs_hws.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/fs_hws.c @@ -1228,7 +1228,7 @@ static struct mlx5_fs_pool * mlx5_fs_get_pr_encap_pool(struct mlx5_core_dev *dev, struct xarray *pr_pools, enum mlx5hws_action_type reformat_type, size_t size) { - struct mlx5_fs_pool *pr_pool; + struct mlx5_fs_pool *pr_pool, *old; unsigned long index = size; int err; @@ -1242,13 +1242,14 @@ mlx5_fs_get_pr_encap_pool(struct mlx5_core_dev *dev, struct xarray *pr_pools, err = mlx5_fs_hws_pr_pool_init(pr_pool, dev, size, reformat_type); if (err) goto free_pr_pool; - err = xa_insert(pr_pools, index, pr_pool, GFP_KERNEL); - if (err) - goto cleanup_pr_pool; - return pr_pool; + old = xa_cmpxchg(pr_pools, index, NULL, pr_pool, GFP_KERNEL); + if (!old) + return pr_pool; -cleanup_pr_pool: mlx5_fs_hws_pr_pool_cleanup(pr_pool); + kfree(pr_pool); + return xa_is_err(old) ? ERR_PTR(xa_err(old)) : old; + free_pr_pool: kfree(pr_pool); return ERR_PTR(err); -- 2.43.0 mlx5_cmd_hws_modify_header_alloc() creates a pool at the index after the last one if no pool matches the pattern. TC and CT allocate modify headers concurrently, meaning two callers can take the same index and the second call fails with -EBUSY. Its error path also destroys the pool it just created although another caller may already use it. Keep such a pool since cleanup releases it. Pools now live until cleanup so the lookup needs no lock. A miss looks again under a mutex before creating the pool. Tested for regressions on ConnectX-7 with firmware 28.48.1000. Fixes: b36315ca69cb ("net/mlx5: fs, add HWS modify header API function") Signed-off-by: Alexandre Cassen --- .../mellanox/mlx5/core/steering/hws/fs_hws.c | 53 +++++++++++-------- .../mellanox/mlx5/core/steering/hws/fs_hws.h | 1 + 2 files changed, 33 insertions(+), 21 deletions(-) diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/fs_hws.c b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/fs_hws.c index 66edb42268cb..11cced847186 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/fs_hws.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/fs_hws.c @@ -63,6 +63,7 @@ static int mlx5_fs_init_hws_actions_pool(struct mlx5_core_dev *dev, xa_init(&hws_pool->el2tol3tnl_pools); xa_init(&hws_pool->el2tol2tnl_pools); xa_init(&hws_pool->mh_pools); + mutex_init(&hws_pool->mh_pools_lock); xa_init(&hws_pool->table_dests); xa_init(&hws_pool->vport_dests); xa_init(&hws_pool->vport_vhca_dests); @@ -111,6 +112,7 @@ static void mlx5_fs_cleanup_hws_actions_pool(struct mlx5_fs_hws_context *fs_ctx) xa_for_each(&hws_pool->mh_pools, i, pool) mlx5_fs_destroy_mh_pool(pool, &hws_pool->mh_pools, i); xa_destroy(&hws_pool->mh_pools); + mutex_destroy(&hws_pool->mh_pools_lock); xa_for_each(&hws_pool->el2tol2tnl_pools, i, pool) mlx5_fs_destroy_pr_pool(pool, &hws_pool->el2tol2tnl_pools, i); xa_destroy(&hws_pool->el2tol2tnl_pools); @@ -1457,6 +1459,23 @@ mlx5_fs_destroy_mh_pool(struct mlx5_fs_pool *pool, struct xarray *mh_pools, kfree(pool); } +static struct mlx5_fs_pool * +mlx5_fs_find_mh_pool(struct xarray *mh_pools, + struct mlx5hws_action_mh_pattern *pattern, + unsigned long *cnt) +{ + struct mlx5_fs_pool *pool; + unsigned long i; + + *cnt = 0; + xa_for_each(mh_pools, i, pool) { + if (mlx5_fs_hws_mh_pool_match(pool, pattern)) + return pool; + (*cnt)++; + } + return NULL; +} + static int mlx5_cmd_hws_modify_header_alloc(struct mlx5_flow_root_namespace *ns, u8 namespace, u8 num_actions, void *modify_actions, @@ -1467,33 +1486,28 @@ static int mlx5_cmd_hws_modify_header_alloc(struct mlx5_flow_root_namespace *ns, struct mlx5_fs_hws_mh *mh_data = NULL; struct mlx5hws_action *hws_action; struct mlx5_fs_pool *pool; - unsigned long i, cnt = 0; - bool known_pattern; + unsigned long cnt; int err; pattern.sz = MLX5_UN_SZ_BYTES(set_add_copy_action_in_auto) * num_actions; pattern.data = modify_actions; - known_pattern = false; - xa_for_each(&hws_pool->mh_pools, i, pool) { - if (mlx5_fs_hws_mh_pool_match(pool, &pattern)) { - known_pattern = true; - break; - } - cnt++; - } - - if (!known_pattern) { - pool = mlx5_fs_create_mh_pool(ns->dev, &pattern, - &hws_pool->mh_pools, cnt); + pool = mlx5_fs_find_mh_pool(&hws_pool->mh_pools, &pattern, &cnt); + if (!pool) { + mutex_lock(&hws_pool->mh_pools_lock); + pool = mlx5_fs_find_mh_pool(&hws_pool->mh_pools, &pattern, + &cnt); + if (!pool) + pool = mlx5_fs_create_mh_pool(ns->dev, &pattern, + &hws_pool->mh_pools, cnt); + mutex_unlock(&hws_pool->mh_pools_lock); if (IS_ERR(pool)) return PTR_ERR(pool); } + mh_data = mlx5_fs_hws_mh_pool_acquire_mh(pool); - if (IS_ERR(mh_data)) { - err = PTR_ERR(mh_data); - goto destroy_pool; - } + if (IS_ERR(mh_data)) + return PTR_ERR(mh_data); hws_action = mh_data->bulk->hws_action; mh_data->data = kmemdup(pattern.data, pattern.sz, GFP_KERNEL); if (!mh_data->data) { @@ -1509,9 +1523,6 @@ static int mlx5_cmd_hws_modify_header_alloc(struct mlx5_flow_root_namespace *ns, release_mh: mlx5_fs_hws_mh_pool_release_mh(pool, mh_data); -destroy_pool: - if (!known_pattern) - mlx5_fs_destroy_mh_pool(pool, &hws_pool->mh_pools, cnt); return err; } diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/fs_hws.h b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/fs_hws.h index 20cdacd8f12e..cf068fb5bdbf 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/fs_hws.h +++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/hws/fs_hws.h @@ -19,6 +19,7 @@ struct mlx5_fs_hws_actions_pool { struct xarray el2tol3tnl_pools; struct xarray el2tol2tnl_pools; struct xarray mh_pools; + struct mutex mh_pools_lock; /* serializes pool creation */ struct xarray table_dests; struct xarray vport_vhca_dests; struct xarray vport_dests; -- 2.43.0