From 89a8e3c4f84bc053d916666048f7ff9f8b62edcf Mon Sep 17 00:00:00 2001 From: Suanming Mou Date: Wed, 28 Oct 2020 17:33:26 +0800 Subject: [PATCH] net/mlx5: make meter action thread safe This commit adds the spinlock for the meter action to make it be thread safe. Atomic reference counter in all is not enough as the meter action should be created synchronized with reference counter increment. With only atomic reference counter, even the counter is increased, the action may still not be created. Signed-off-by: Suanming Mou Acked-by: Matan Azrad --- drivers/net/mlx5/mlx5_flow.h | 2 + drivers/net/mlx5/mlx5_flow_meter.c | 72 +++++++++++++++--------------- 2 files changed, 39 insertions(+), 35 deletions(-) diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h index 6cac3ab033..cb5b77f4d7 100644 --- a/drivers/net/mlx5/mlx5_flow.h +++ b/drivers/net/mlx5/mlx5_flow.h @@ -845,6 +845,8 @@ struct mlx5_flow_meter { struct mlx5_flow_meter_profile *profile; /**< Meter profile parameters. */ + rte_spinlock_t sl; /**< Meter action spinlock. */ + /** Policer actions (per meter output color). */ enum rte_mtr_policer_action action[RTE_COLORS]; diff --git a/drivers/net/mlx5/mlx5_flow_meter.c b/drivers/net/mlx5/mlx5_flow_meter.c index b36bc7bac3..03a5e79eb8 100644 --- a/drivers/net/mlx5/mlx5_flow_meter.c +++ b/drivers/net/mlx5/mlx5_flow_meter.c @@ -679,6 +679,7 @@ mlx5_flow_meter_create(struct rte_eth_dev *dev, uint32_t meter_id, fm->shared = !!shared; fm->policer_stats.stats_mask = params->stats_mask; fm->profile->ref_cnt++; + rte_spinlock_init(&fm->sl); return 0; error: mlx5_flow_destroy_policer_rules(dev, fm, &attr); @@ -1167,49 +1168,49 @@ mlx5_flow_meter_attach(struct mlx5_priv *priv, uint32_t meter_id, struct rte_flow_error *error) { struct mlx5_flow_meter *fm; + int ret = 0; fm = mlx5_flow_meter_find(priv, meter_id); if (fm == NULL) { rte_flow_error_set(error, ENOENT, RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL, "Meter object id not valid"); - goto error; - } - if (!fm->shared && fm->ref_cnt) { - DRV_LOG(ERR, "Cannot share a non-shared meter."); - rte_flow_error_set(error, EINVAL, - RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL, - "Meter can't be shared"); - goto error; + return fm; } - if (!fm->ref_cnt++) { - MLX5_ASSERT(!fm->mfts->meter_action); + rte_spinlock_lock(&fm->sl); + if (fm->mfts->meter_action) { + if (fm->shared && + attr->transfer == fm->transfer && + attr->ingress == fm->ingress && + attr->egress == fm->egress) + fm->ref_cnt++; + else + ret = -1; + } else { fm->ingress = attr->ingress; fm->egress = attr->egress; fm->transfer = attr->transfer; + fm->ref_cnt = 1; /* This also creates the meter object. */ fm->mfts->meter_action = mlx5_flow_meter_action_create(priv, fm); - if (!fm->mfts->meter_action) - goto error_detach; - } else { - MLX5_ASSERT(fm->mfts->meter_action); - if (attr->transfer != fm->transfer || - attr->ingress != fm->ingress || - attr->egress != fm->egress) { - DRV_LOG(ERR, "meter I/O attributes do not " - "match flow I/O attributes."); - goto error_detach; + if (!fm->mfts->meter_action) { + fm->ref_cnt = 0; + fm->ingress = 0; + fm->egress = 0; + fm->transfer = 0; + ret = -1; + DRV_LOG(ERR, "Meter action create failed."); } } - return fm; -error_detach: - mlx5_flow_meter_detach(fm); - rte_flow_error_set(error, EINVAL, RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL, - fm->mfts->meter_action ? "Meter attr not match" : - "Meter action create failed"); -error: - return NULL; + rte_spinlock_unlock(&fm->sl); + if (ret) + rte_flow_error_set(error, EINVAL, + RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL, + fm->mfts->meter_action ? + "Meter attr not match" : + "Meter action create failed"); + return ret ? NULL : fm; } /** @@ -1222,15 +1223,16 @@ void mlx5_flow_meter_detach(struct mlx5_flow_meter *fm) { #ifdef HAVE_MLX5_DR_CREATE_ACTION_FLOW_METER + rte_spinlock_lock(&fm->sl); MLX5_ASSERT(fm->ref_cnt); - if (--fm->ref_cnt) - return; - if (fm->mfts->meter_action) + if (--fm->ref_cnt == 0) { mlx5_glue->destroy_flow_action(fm->mfts->meter_action); - fm->mfts->meter_action = NULL; - fm->ingress = 0; - fm->egress = 0; - fm->transfer = 0; + fm->mfts->meter_action = NULL; + fm->ingress = 0; + fm->egress = 0; + fm->transfer = 0; + } + rte_spinlock_unlock(&fm->sl); #else (void)fm; #endif -- 2.20.1