From fa31a5ed0196a3d0ea61c27030d7fd515cc8c62d Mon Sep 17 00:00:00 2001 From: Bing Zhao Date: Wed, 21 Jul 2021 11:54:17 +0300 Subject: [PATCH] net/mlx5: support yellow meter policy rules When creating a meter policy, both / either of the action rules for green and yellow colors may be provided. After validation, usually the actions are created before the meter is using by a flow rule. If there is action specified for the yellow color, the action rules should be created together with green color in the same time. The action of green / yellow color can be empty, then the default behavior is the jump action of the rule, just the same as that of the default policy. If the fate action of either one color is queue / RSS, all the actions rules will be created on the flow splitting stage instead of the policy adding stage. Signed-off-by: Bing Zhao Acked-by: Matan Azrad --- drivers/net/mlx5/mlx5_flow_dv.c | 46 ++++++++++++++------------- drivers/net/mlx5/mlx5_flow_meter.c | 50 +++++++++++++++++++----------- 2 files changed, 56 insertions(+), 40 deletions(-) diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c index 873a3724c7..cd77835076 100644 --- a/drivers/net/mlx5/mlx5_flow_dv.c +++ b/drivers/net/mlx5/mlx5_flow_dv.c @@ -15206,7 +15206,7 @@ __flow_dv_create_domain_policy_acts(struct rte_eth_dev *dev, struct mlx5_priv *priv = dev->data->dev_private; struct rte_flow_error flow_err; const struct rte_flow_action *act; - uint64_t action_flags = 0; + uint64_t action_flags; struct mlx5_flow_handle dh; struct mlx5_flow dev_flow; struct mlx5_flow_dv_port_id_action_resource port_id_action; @@ -15226,21 +15226,20 @@ __flow_dv_create_domain_policy_acts(struct rte_eth_dev *dev, memset(&dh, 0, sizeof(struct mlx5_flow_handle)); memset(&dev_flow, 0, sizeof(struct mlx5_flow)); memset(&port_id_action, 0, - sizeof(struct mlx5_flow_dv_port_id_action_resource)); + sizeof(struct mlx5_flow_dv_port_id_action_resource)); memset(mhdr_res, 0, sizeof(*mhdr_res)); mhdr_res->ft_type = transfer ? MLX5DV_FLOW_TABLE_TYPE_FDB : - egress ? - MLX5DV_FLOW_TABLE_TYPE_NIC_TX : - MLX5DV_FLOW_TABLE_TYPE_NIC_RX; + (egress ? MLX5DV_FLOW_TABLE_TYPE_NIC_TX : + MLX5DV_FLOW_TABLE_TYPE_NIC_RX); dev_flow.handle = &dh; dev_flow.dv.port_id_action = &port_id_action; dev_flow.external = true; for (i = 0; i < RTE_COLORS; i++) { if (i < MLX5_MTR_RTE_COLORS) act_cnt = &mtr_policy->act_cnt[i]; + action_flags = 0; for (act = actions[i]; - act && act->type != RTE_FLOW_ACTION_TYPE_END; - act++) { + act && act->type != RTE_FLOW_ACTION_TYPE_END; act++) { switch (act->type) { case RTE_FLOW_ACTION_TYPE_MARK: { @@ -15448,7 +15447,7 @@ __flow_dv_create_domain_policy_acts(struct rte_eth_dev *dev, (1 << MLX5_SCALE_FLOW_GROUP_BIT), }; struct mlx5_flow_meter_sub_policy *sub_policy = - mtr_policy->sub_policys[domain][0]; + mtr_policy->sub_policys[domain][0]; if (i >= MLX5_MTR_RTE_COLORS) return -rte_mtr_error_set(error, @@ -15492,6 +15491,10 @@ __flow_dv_create_domain_policy_acts(struct rte_eth_dev *dev, action_flags |= MLX5_FLOW_ACTION_JUMP; break; } + /* + * No need to check meter hierarchy for Y or R colors + * here since it is done in the validation stage. + */ case RTE_FLOW_ACTION_TYPE_METER: { const struct rte_flow_action_meter *mtr; @@ -15607,6 +15610,7 @@ flow_dv_create_mtr_policy_acts(struct rte_eth_dev *dev, ret = __flow_dv_create_domain_policy_acts(dev, mtr_policy, actions, (enum mlx5_meter_domain)i, error); + /* Cleaning resource is done in the caller level. */ if (ret) return ret; } @@ -16148,16 +16152,15 @@ __flow_dv_create_policy_acts_rules(struct rte_eth_dev *dev, for (i = 0; i < RTE_COLORS; i++) { acts[i].actions_n = 0; - if (i == RTE_COLOR_YELLOW) - continue; if (i == RTE_COLOR_RED) { /* Only support drop on red. */ acts[i].dv_actions[0] = - mtr_policy->dr_drop_action[domain]; + mtr_policy->dr_drop_action[domain]; acts[i].actions_n = 1; continue; } - if (mtr_policy->act_cnt[i].fate_action == MLX5_FLOW_FATE_MTR) { + if (i == RTE_COLOR_GREEN && + mtr_policy->act_cnt[i].fate_action == MLX5_FLOW_FATE_MTR) { struct rte_flow_attr attr = { .transfer = transfer }; @@ -16191,13 +16194,12 @@ __flow_dv_create_policy_acts_rules(struct rte_eth_dev *dev, "mark action for policy."); goto err_exit; } - acts[i].dv_actions[acts[i].actions_n] = - tag->action; + acts[i].dv_actions[acts[i].actions_n] = tag->action; acts[i].actions_n++; } if (mtr_policy->act_cnt[i].modify_hdr) { acts[i].dv_actions[acts[i].actions_n] = - mtr_policy->act_cnt[i].modify_hdr->action; + mtr_policy->act_cnt[i].modify_hdr->action; acts[i].actions_n++; } if (mtr_policy->act_cnt[i].fate_action) { @@ -16212,7 +16214,7 @@ __flow_dv_create_policy_acts_rules(struct rte_eth_dev *dev, goto err_exit; } acts[i].dv_actions[acts[i].actions_n] = - port_action->action; + port_action->action; acts[i].actions_n++; mtr_policy->dev = dev; match_src_port = true; @@ -16226,15 +16228,15 @@ __flow_dv_create_policy_acts_rules(struct rte_eth_dev *dev, case MLX5_FLOW_FATE_SHARED_RSS: case MLX5_FLOW_FATE_QUEUE: hrxq = mlx5_ipool_get - (priv->sh->ipool[MLX5_IPOOL_HRXQ], - sub_policy->rix_hrxq[i]); + (priv->sh->ipool[MLX5_IPOOL_HRXQ], + sub_policy->rix_hrxq[i]); if (!hrxq) { DRV_LOG(ERR, "Failed to find " "queue action for policy."); goto err_exit; } acts[i].dv_actions[acts[i].actions_n] = - hrxq->action; + hrxq->action; acts[i].actions_n++; break; case MLX5_FLOW_FATE_MTR: @@ -16276,7 +16278,7 @@ __flow_dv_create_policy_acts_rules(struct rte_eth_dev *dev, if (__flow_dv_create_domain_policy_rules(dev, sub_policy, egress, transfer, match_src_port, acts)) { DRV_LOG(ERR, - "Failed to create policy rules per domain."); + "Failed to create policy rules per domain."); goto err_exit; } return 0; @@ -16313,8 +16315,8 @@ flow_dv_create_policy_rules(struct rte_eth_dev *dev, /* Prepare actions list and create policy rules. */ if (__flow_dv_create_policy_acts_rules(dev, mtr_policy, mtr_policy->sub_policys[i][0], i)) { - DRV_LOG(ERR, - "Failed to create policy action list per domain."); + DRV_LOG(ERR, "Failed to create policy action " + "list per domain."); return -1; } } diff --git a/drivers/net/mlx5/mlx5_flow_meter.c b/drivers/net/mlx5/mlx5_flow_meter.c index 73eba0dabd..19b2665558 100644 --- a/drivers/net/mlx5/mlx5_flow_meter.c +++ b/drivers/net/mlx5/mlx5_flow_meter.c @@ -686,21 +686,20 @@ mlx5_flow_meter_policy_add(struct rte_eth_dev *dev, if (!priv->mtr_en) return -rte_mtr_error_set(error, ENOTSUP, RTE_MTR_ERROR_TYPE_METER_POLICY, - NULL, "meter policy unsupported."); + NULL, "meter policy unsupported. "); if (policy_id == MLX5_INVALID_POLICY_ID) return -rte_mtr_error_set(error, ENOTSUP, - RTE_MTR_ERROR_TYPE_METER_POLICY_ID, NULL, - "policy ID is invalid. "); + RTE_MTR_ERROR_TYPE_METER_POLICY_ID, + NULL, "policy ID is invalid. "); if (policy_id == priv->sh->mtrmng->def_policy_id) return -rte_mtr_error_set(error, EEXIST, - RTE_MTR_ERROR_TYPE_METER_POLICY_ID, NULL, - "policy ID exists. "); - mtr_policy = mlx5_flow_meter_policy_find(dev, policy_id, - &policy_idx); + RTE_MTR_ERROR_TYPE_METER_POLICY_ID, + NULL, "default policy ID exists. "); + mtr_policy = mlx5_flow_meter_policy_find(dev, policy_id, &policy_idx); if (mtr_policy) return -rte_mtr_error_set(error, EEXIST, - RTE_MTR_ERROR_TYPE_METER_POLICY_ID, NULL, - "policy ID exists. "); + RTE_MTR_ERROR_TYPE_METER_POLICY_ID, + NULL, "policy ID exists. "); ret = mlx5_flow_validate_mtr_acts(dev, policy->actions, &attr, &is_rss, &domain_bitmap, &is_def_policy, error); if (ret) @@ -730,16 +729,22 @@ mlx5_flow_meter_policy_add(struct rte_eth_dev *dev, for (i = 0; i < MLX5_MTR_DOMAIN_MAX; i++) { if (!(domain_bitmap & (1 << i))) continue; + /* + * If RSS is found, it means that only the ingress domain can + * be supported. It is invalid to support RSS for one color + * and egress / transfer domain actions for another. Drop and + * jump action should have no impact. + */ if (is_rss) { policy_size += - sizeof(struct mlx5_flow_meter_sub_policy *) * - MLX5_MTR_RSS_MAX_SUB_POLICY; + sizeof(struct mlx5_flow_meter_sub_policy *) * + MLX5_MTR_RSS_MAX_SUB_POLICY; break; } policy_size += sizeof(struct mlx5_flow_meter_sub_policy *); } mtr_policy = mlx5_malloc(MLX5_MEM_ZERO, policy_size, - RTE_CACHE_LINE_SIZE, SOCKET_ID_ANY); + RTE_CACHE_LINE_SIZE, SOCKET_ID_ANY); if (!mtr_policy) return -rte_mtr_error_set(error, ENOMEM, RTE_MTR_ERROR_TYPE_METER_POLICY, NULL, @@ -756,10 +761,9 @@ mlx5_flow_meter_policy_add(struct rte_eth_dev *dev, mtr_policy->transfer = 1; sub_policy = mlx5_ipool_zmalloc (priv->sh->ipool[MLX5_IPOOL_MTR_POLICY], - &sub_policy_idx); - if (!sub_policy) - goto policy_add_err; - if (sub_policy_idx > MLX5_MAX_SUB_POLICY_TBL_NUM) + &sub_policy_idx); + if (!sub_policy || + sub_policy_idx > MLX5_MAX_SUB_POLICY_TBL_NUM) goto policy_add_err; sub_policy->idx = sub_policy_idx; sub_policy->main_policy = mtr_policy; @@ -768,7 +772,7 @@ mlx5_flow_meter_policy_add(struct rte_eth_dev *dev, sub_policy->main_policy_id = 1; } mtr_policy->sub_policys[i] = - (struct mlx5_flow_meter_sub_policy **) + (struct mlx5_flow_meter_sub_policy **) ((uint8_t *)mtr_policy + policy_size); mtr_policy->sub_policys[i][0] = sub_policy; sub_policy_num = (mtr_policy->sub_policy_num >> @@ -780,11 +784,17 @@ mlx5_flow_meter_policy_add(struct rte_eth_dev *dev, mtr_policy->sub_policy_num |= (sub_policy_num & MLX5_MTR_SUB_POLICY_NUM_MASK) << (MLX5_MTR_SUB_POLICY_NUM_SHIFT * i); + /* + * If RSS is found, it means that only the ingress domain can + * be supported. It is invalid to support RSS for one color + * and egress / transfer domain actions for another. Drop and + * jump action should have no impact. + */ if (is_rss) { mtr_policy->is_rss = 1; break; } - policy_size += sizeof(struct mlx5_flow_meter_sub_policy *); + policy_size += sizeof(struct mlx5_flow_meter_sub_policy *); } rte_spinlock_init(&mtr_policy->sl); ret = mlx5_flow_create_mtr_acts(dev, mtr_policy, @@ -800,6 +810,10 @@ mlx5_flow_meter_policy_add(struct rte_eth_dev *dev, goto policy_add_err; skip_rule = (final_policy->is_rss || final_policy->is_queue); } + /* + * If either Green or Yellow has queue / RSS action, all the policy + * rules will be created later in the flow splitting stage. + */ if (!is_rss && !mtr_policy->is_queue && !skip_rule) { /* Create policy rules in HW. */ ret = mlx5_flow_create_policy_rules(dev, mtr_policy); -- 2.20.1