* [dpdk-dev] [PATCH 0/6] support yellow color policy in mlx5
@ 2021-07-05 15:57 Bing Zhao
2021-07-05 15:57 ` [dpdk-dev] [PATCH 1/6] net/mlx5: add yellow color default policy Bing Zhao
` (7 more replies)
0 siblings, 8 replies; 24+ messages in thread
From: Bing Zhao @ 2021-07-05 15:57 UTC (permalink / raw)
To: viacheslavo, matan; +Cc: dev, orika, rasland
When creating a meter policy, the actions for yellow color can be
specified together with green color. The mlx5 PMD now supports to
set the policy for yellow.
The actions list that is supported for yellow is the same as that
for green.
Bing Zhao (6):
net/mlx5: add yellow color default policy
net/mlx5: support yellow in meter policy validation
net/mlx5: enable meter bucket overflow for yellow color
net/mlx5: added support for yellow policy rules
net/mlx5: split policies handling of colors
doc: update mlx5 metering policy part
doc/guides/rel_notes/release_21_08.rst | 5 +
drivers/net/mlx5/mlx5.h | 4 +-
drivers/net/mlx5/mlx5_flow.c | 41 +--
drivers/net/mlx5/mlx5_flow_aso.c | 3 +
drivers/net/mlx5/mlx5_flow_dv.c | 343 +++++++++++++++----------
drivers/net/mlx5/mlx5_flow_meter.c | 56 ++--
6 files changed, 271 insertions(+), 181 deletions(-)
--
2.27.0
^ permalink raw reply [flat|nested] 24+ messages in thread* [dpdk-dev] [PATCH 1/6] net/mlx5: add yellow color default policy 2021-07-05 15:57 [dpdk-dev] [PATCH 0/6] support yellow color policy in mlx5 Bing Zhao @ 2021-07-05 15:57 ` Bing Zhao 2021-07-05 15:57 ` [dpdk-dev] [PATCH 2/6] net/mlx5: support yellow in meter policy validation Bing Zhao ` (6 subsequent siblings) 7 siblings, 0 replies; 24+ messages in thread From: Bing Zhao @ 2021-07-05 15:57 UTC (permalink / raw) To: viacheslavo, matan; +Cc: dev, orika, rasland To support the yellow color for the default meter policy, the default policy action for yellow should be created together with the green policy. The default policy action for yellow action is the same as that for green. In the same table, the same matcher will be reused for yellow and the destination group will be the same. Signed-off-by: Bing Zhao <bingz@nvidia.com> --- drivers/net/mlx5/mlx5_flow_dv.c | 88 +++++++++++++++++++++------------ 1 file changed, 57 insertions(+), 31 deletions(-) diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c index a04a3c2bb8..c6a06d9def 100644 --- a/drivers/net/mlx5/mlx5_flow_dv.c +++ b/drivers/net/mlx5/mlx5_flow_dv.c @@ -15365,19 +15365,18 @@ __flow_dv_create_policy_flow(struct rte_eth_dev *dev, if (!is_default_policy && (priv->representor || priv->master)) { if (flow_dv_translate_item_port_id(dev, matcher.buf, value.buf, NULL, attr)) { - DRV_LOG(ERR, - "Failed to create meter policy flow with port."); + DRV_LOG(ERR, "Failed to create meter policy%d flow's" + " value with port.", color); return -1; } } flow_dv_match_meta_reg(matcher.buf, value.buf, - (enum modify_reg)color_reg_c_idx, - rte_col_2_mlx5_col(color), - UINT32_MAX); - ret = mlx5_flow_os_create_flow(matcher_object, - (void *)&value, actions_n, actions, rule); + (enum modify_reg)color_reg_c_idx, + rte_col_2_mlx5_col(color), UINT32_MAX); + ret = mlx5_flow_os_create_flow(matcher_object, (void *)&value, + actions_n, actions, rule); if (ret) { - DRV_LOG(ERR, "Failed to create meter policy flow."); + DRV_LOG(ERR, "Failed to create meter policy%d flow.", color); return -1; } return 0; @@ -15416,8 +15415,8 @@ __flow_dv_create_policy_matcher(struct rte_eth_dev *dev, if (!is_default_policy && (priv->representor || priv->master)) { if (flow_dv_translate_item_port_id(dev, matcher.mask.buf, value.buf, NULL, attr)) { - DRV_LOG(ERR, - "Failed to register meter drop matcher with port."); + DRV_LOG(ERR, "Failed to register meter policy%d matcher" + " with port.", priority); return -1; } } @@ -15425,9 +15424,11 @@ __flow_dv_create_policy_matcher(struct rte_eth_dev *dev, if (priority < RTE_COLOR_RED) flow_dv_match_meta_reg(matcher.mask.buf, value.buf, (enum modify_reg)color_reg_c_idx, 0, color_mask); - matcher.priority = priority; + /* Adjust the priority */ + matcher.priority = (priority == RTE_COLOR_YELLOW) ? + RTE_COLOR_GREEN : priority; matcher.crc = rte_raw_cksum((const void *)matcher.mask.buf, - matcher.mask.size); + matcher.mask.size); entry = mlx5_cache_register(&tbl_data->matchers, &ctx); if (!entry) { DRV_LOG(ERR, "Failed to register meter drop matcher."); @@ -15490,15 +15491,18 @@ __flow_dv_create_domain_policy_rules(struct rte_eth_dev *dev, /* Prepare matchers. */ color_reg_c_idx = ret; for (i = 0; i < RTE_COLORS; i++) { - if (i == RTE_COLOR_YELLOW || !acts[i].actions_n) + if (!acts[i].actions_n) continue; attr.priority = i; if (!sub_policy->color_matcher[i]) { /* Create matchers for Color. */ if (__flow_dv_create_policy_matcher(dev, color_reg_c_idx, i, sub_policy, - &attr, is_default_policy, &flow_err)) + &attr, is_default_policy, &flow_err)) { + DRV_LOG(ERR, + "Failed to create color%u matcher.", i); return -1; + } } /* Create flow, matching color. */ if (acts[i].actions_n) @@ -15509,8 +15513,11 @@ __flow_dv_create_domain_policy_rules(struct rte_eth_dev *dev, acts[i].dv_actions, is_default_policy, &sub_policy->color_rule[i], - &attr)) + &attr)) { + DRV_LOG(ERR, + "Failed to create color%u rule.", i); return -1; + } } return 0; } @@ -15665,8 +15672,7 @@ __flow_dv_create_domain_def_policy(struct rte_eth_dev *dev, uint32_t domain) sizeof(struct mlx5_flow_meter_def_policy), RTE_CACHE_LINE_SIZE, SOCKET_ID_ANY); if (!def_policy) { - DRV_LOG(ERR, "Failed to alloc " - "default policy table."); + DRV_LOG(ERR, "Failed to alloc default policy table."); goto def_policy_error; } mtrmng->def_policy[domain] = def_policy; @@ -15682,26 +15688,46 @@ __flow_dv_create_domain_def_policy(struct rte_eth_dev *dev, uint32_t domain) } def_policy->sub_policy.jump_tbl[RTE_COLOR_GREEN] = jump_tbl; tbl_data = container_of(jump_tbl, - struct mlx5_flow_tbl_data_entry, tbl); + struct mlx5_flow_tbl_data_entry, tbl); def_policy->dr_jump_action[RTE_COLOR_GREEN] = tbl_data->jump.action; - acts[RTE_COLOR_GREEN].dv_actions[0] = - tbl_data->jump.action; + acts[RTE_COLOR_GREEN].dv_actions[0] = tbl_data->jump.action; acts[RTE_COLOR_GREEN].actions_n = 1; + /* + * YELLOW has the same default policy as GREEN does. + * G & Y share the same table and action. + */ + jump_tbl = flow_dv_tbl_resource_get(dev, + MLX5_FLOW_TABLE_LEVEL_METER, + egress, transfer, false, NULL, 0, + 0, MLX5_MTR_TABLE_ID_SUFFIX, &error); + if (!jump_tbl) { + DRV_LOG(ERR, + "Failed to get meter suffix table."); + goto def_policy_error; + } + def_policy->sub_policy.jump_tbl[RTE_COLOR_YELLOW] = jump_tbl; + tbl_data = container_of(jump_tbl, + struct mlx5_flow_tbl_data_entry, tbl); + def_policy->dr_jump_action[RTE_COLOR_YELLOW] = + tbl_data->jump.action; + acts[RTE_COLOR_YELLOW].dv_actions[0] = tbl_data->jump.action; + acts[RTE_COLOR_YELLOW].actions_n = 1; /* Create jump action to the drop table. */ if (!mtrmng->drop_tbl[domain]) { mtrmng->drop_tbl[domain] = flow_dv_tbl_resource_get (dev, MLX5_FLOW_TABLE_LEVEL_METER, - egress, transfer, false, NULL, 0, - 0, MLX5_MTR_TABLE_ID_DROP, &error); + egress, transfer, false, NULL, 0, + 0, MLX5_MTR_TABLE_ID_DROP, &error); if (!mtrmng->drop_tbl[domain]) { - DRV_LOG(ERR, "Failed to create " - "meter drop table for default policy."); + DRV_LOG(ERR, "Failed to create meter " + "drop table for default policy."); goto def_policy_error; } } + /* all RED: unique Drop table for jump action. */ tbl_data = container_of(mtrmng->drop_tbl[domain], - struct mlx5_flow_tbl_data_entry, tbl); + struct mlx5_flow_tbl_data_entry, tbl); def_policy->dr_jump_action[RTE_COLOR_RED] = tbl_data->jump.action; acts[RTE_COLOR_RED].dv_actions[0] = tbl_data->jump.action; @@ -15711,15 +15737,14 @@ __flow_dv_create_domain_def_policy(struct rte_eth_dev *dev, uint32_t domain) &def_policy->sub_policy, egress, transfer, true, acts); if (ret) { - DRV_LOG(ERR, "Failed to create " - "default policy rules."); - goto def_policy_error; + DRV_LOG(ERR, "Failed to create default policy rules."); + goto def_policy_error; } } return 0; def_policy_error: __flow_dv_destroy_domain_def_policy(dev, - (enum mlx5_meter_domain)domain); + (enum mlx5_meter_domain)domain); return -1; } @@ -15742,8 +15767,9 @@ flow_dv_create_def_policy(struct rte_eth_dev *dev) if (!priv->config.dv_esw_en && i == MLX5_MTR_DOMAIN_TRANSFER) continue; if (__flow_dv_create_domain_def_policy(dev, i)) { - DRV_LOG(ERR, - "Failed to create default policy"); + DRV_LOG(ERR, "Failed to create default policy"); + /* Rollback the created default policies for others. */ + flow_dv_destroy_def_policy(dev); return -1; } } -- 2.27.0 ^ permalink raw reply [flat|nested] 24+ messages in thread
* [dpdk-dev] [PATCH 2/6] net/mlx5: support yellow in meter policy validation 2021-07-05 15:57 [dpdk-dev] [PATCH 0/6] support yellow color policy in mlx5 Bing Zhao 2021-07-05 15:57 ` [dpdk-dev] [PATCH 1/6] net/mlx5: add yellow color default policy Bing Zhao @ 2021-07-05 15:57 ` Bing Zhao 2021-07-05 15:57 ` [dpdk-dev] [PATCH 3/6] net/mlx5: enable meter bucket overflow for yellow color Bing Zhao ` (5 subsequent siblings) 7 siblings, 0 replies; 24+ messages in thread From: Bing Zhao @ 2021-07-05 15:57 UTC (permalink / raw) To: viacheslavo, matan; +Cc: dev, orika, rasland In the previous implementation, the policy for yellow color was not supported. The action validation for yellow was skipped. Since the yellow color policy needs to be supported, the validation should also be done for yellow color. In the meanwhile, due to the fact that color policies of one meter should be used for the same flow(s), the domains supported of both colors shall be the same. If both of the colors have RSS as the fate actions, except the queues, all other parameters of RSS should be the same. Signed-off-by: Bing Zhao <bingz@nvidia.com> --- drivers/net/mlx5/mlx5.h | 4 +- drivers/net/mlx5/mlx5_flow.c | 4 +- drivers/net/mlx5/mlx5_flow_dv.c | 151 +++++++++++++++++------------ drivers/net/mlx5/mlx5_flow_meter.c | 3 +- 4 files changed, 97 insertions(+), 65 deletions(-) diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h index 1b2dc8f815..1d2e003900 100644 --- a/drivers/net/mlx5/mlx5.h +++ b/drivers/net/mlx5/mlx5.h @@ -608,8 +608,8 @@ struct mlx5_dev_shared_port { /*ASO flow meter structures*/ /* Modify this value if enum rte_mtr_color changes. */ #define RTE_MTR_DROPPED RTE_COLORS -/* Yellow is not supported. */ -#define MLX5_MTR_RTE_COLORS (RTE_COLOR_GREEN + 1) +/* Yellow is now supported. */ +#define MLX5_MTR_RTE_COLORS (RTE_COLOR_YELLOW + 1) /* table_id 22 bits in mlx5_flow_tbl_key so limit policy number. */ #define MLX5_MAX_SUB_POLICY_TBL_NUM 0x3FFFFF #define MLX5_INVALID_POLICY_ID UINT32_MAX diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c index 3b7c94d92f..61c72da2aa 100644 --- a/drivers/net/mlx5/mlx5_flow.c +++ b/drivers/net/mlx5/mlx5_flow.c @@ -7093,8 +7093,8 @@ mlx5_flow_validate_mtr_acts(struct rte_eth_dev *dev, const struct mlx5_flow_driver_ops *fops; fops = flow_get_drv_ops(MLX5_FLOW_TYPE_DV); - return fops->validate_mtr_acts(dev, actions, attr, - is_rss, domain_bitmap, is_def_policy, error); + return fops->validate_mtr_acts(dev, actions, attr, is_rss, + domain_bitmap, is_def_policy, error); } /** diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c index c6a06d9def..f53cf65e87 100644 --- a/drivers/net/mlx5/mlx5_flow_dv.c +++ b/drivers/net/mlx5/mlx5_flow_dv.c @@ -16430,6 +16430,19 @@ flow_dv_action_validate(struct rte_eth_dev *dev, } } +static inline int +flow_dv_mtr_policy_rss_compare(const struct rte_flow_action_rss *r1, + const struct rte_flow_action_rss *r2) +{ + if (!r1 || !r2) + return 0; + if (r1->func != r2->func || r1->level != r2->level || + r1->types != r2->types || r1->key_len != r2->key_len || + memcmp(r1->key, r2->key, r1->key_len)) + return 1; + return 0; +} + /** * Validate meter policy actions. * Dispatcher for action type specific validation. @@ -16459,42 +16472,46 @@ flow_dv_validate_mtr_policy_acts(struct rte_eth_dev *dev, struct mlx5_priv *priv = dev->data->dev_private; struct mlx5_dev_config *dev_conf = &priv->config; const struct rte_flow_action *act; - uint64_t action_flags = 0; + uint64_t action_flags[RTE_COLORS] = {0}; int actions_n; int i, ret; struct rte_flow_error flow_err; uint8_t domain_color[RTE_COLORS] = {0}; uint8_t def_domain = MLX5_MTR_ALL_DOMAIN_BIT; + bool def_green = false; + bool def_yellow = false; + const struct rte_flow_action_rss *rss_color[RTE_COLORS] = {NULL}; if (!priv->config.dv_esw_en) def_domain &= ~MLX5_MTR_DOMAIN_TRANSFER_BIT; *domain_bitmap = def_domain; - if (actions[RTE_COLOR_YELLOW] && - actions[RTE_COLOR_YELLOW]->type != RTE_FLOW_ACTION_TYPE_END) - return -rte_mtr_error_set(error, ENOTSUP, - RTE_MTR_ERROR_TYPE_METER_POLICY, - NULL, - "Yellow color does not support any action."); - if (actions[RTE_COLOR_YELLOW] && - actions[RTE_COLOR_YELLOW]->type != RTE_FLOW_ACTION_TYPE_DROP) + if (actions[RTE_COLOR_RED] && + actions[RTE_COLOR_RED]->type != RTE_FLOW_ACTION_TYPE_DROP) return -rte_mtr_error_set(error, ENOTSUP, RTE_MTR_ERROR_TYPE_METER_POLICY, NULL, "Red color only supports drop action."); /* * Check default policy actions: - * Green/Yellow: no action, Red: drop action + * Green / Yellow: no action, Red: drop action + * Either G or Y will trigger default policy actions to be created. */ - if ((!actions[RTE_COLOR_GREEN] || - actions[RTE_COLOR_GREEN]->type == RTE_FLOW_ACTION_TYPE_END)) { + if (!actions[RTE_COLOR_GREEN] || + actions[RTE_COLOR_GREEN]->type == RTE_FLOW_ACTION_TYPE_END) + def_green = true; + if (!actions[RTE_COLOR_YELLOW] || + actions[RTE_COLOR_YELLOW]->type == RTE_FLOW_ACTION_TYPE_END) + def_yellow = true; + if (def_green && def_yellow) { *is_def_policy = true; return 0; } - flow_err.message = NULL; + /* Set to empty string in case of NULL pointer access by user. */ + flow_err.message = ""; for (i = 0; i < RTE_COLORS; i++) { act = actions[i]; - for (action_flags = 0, actions_n = 0; - act && act->type != RTE_FLOW_ACTION_TYPE_END; - act++) { + for (action_flags[i] = 0, actions_n = 0; + act && act->type != RTE_FLOW_ACTION_TYPE_END; + act++) { if (actions_n == MLX5_DV_MAX_NUMBER_OF_ACTIONS) return -rte_mtr_error_set(error, ENOTSUP, RTE_MTR_ERROR_TYPE_METER_POLICY, @@ -16508,7 +16525,7 @@ flow_dv_validate_mtr_policy_acts(struct rte_eth_dev *dev, NULL, "PORT action validate check" " fail for ESW disable"); ret = flow_dv_validate_action_port_id(dev, - action_flags, + action_flags[i], act, attr, &flow_err); if (ret) return -rte_mtr_error_set(error, @@ -16518,11 +16535,11 @@ flow_dv_validate_mtr_policy_acts(struct rte_eth_dev *dev, flow_err.message : "PORT action validate check fail"); ++actions_n; - action_flags |= MLX5_FLOW_ACTION_PORT_ID; + action_flags[i] |= MLX5_FLOW_ACTION_PORT_ID; break; case RTE_FLOW_ACTION_TYPE_MARK: ret = flow_dv_validate_action_mark(dev, act, - action_flags, + action_flags[i], attr, &flow_err); if (ret < 0) return -rte_mtr_error_set(error, @@ -16539,12 +16556,12 @@ flow_dv_validate_mtr_policy_acts(struct rte_eth_dev *dev, NULL, "Extend MARK action is " "not supported. Please try use " "default policy for meter."); - action_flags |= MLX5_FLOW_ACTION_MARK; + action_flags[i] |= MLX5_FLOW_ACTION_MARK; ++actions_n; break; case RTE_FLOW_ACTION_TYPE_SET_TAG: ret = flow_dv_validate_action_set_tag(dev, - act, action_flags, + act, action_flags[i], attr, &flow_err); if (ret) return -rte_mtr_error_set(error, @@ -16557,15 +16574,14 @@ flow_dv_validate_mtr_policy_acts(struct rte_eth_dev *dev, * Count all modify-header actions * as one action. */ - if (!(action_flags & - MLX5_FLOW_MODIFY_HDR_ACTIONS)) + if (!(action_flags[i] & + MLX5_FLOW_MODIFY_HDR_ACTIONS)) ++actions_n; - action_flags |= MLX5_FLOW_ACTION_SET_TAG; + action_flags[i] |= MLX5_FLOW_ACTION_SET_TAG; break; case RTE_FLOW_ACTION_TYPE_DROP: ret = mlx5_flow_validate_action_drop - (action_flags, - attr, &flow_err); + (action_flags[i], attr, &flow_err); if (ret < 0) return -rte_mtr_error_set(error, ENOTSUP, @@ -16573,7 +16589,7 @@ flow_dv_validate_mtr_policy_acts(struct rte_eth_dev *dev, NULL, flow_err.message ? flow_err.message : "Drop action validate check fail"); - action_flags |= MLX5_FLOW_ACTION_DROP; + action_flags[i] |= MLX5_FLOW_ACTION_DROP; ++actions_n; break; case RTE_FLOW_ACTION_TYPE_QUEUE: @@ -16582,9 +16598,9 @@ flow_dv_validate_mtr_policy_acts(struct rte_eth_dev *dev, * metadata feature is engaged. */ if (dev_conf->dv_flow_en && - (dev_conf->dv_xmeta_en != - MLX5_XMETA_MODE_LEGACY) && - mlx5_flow_ext_mreg_supported(dev)) + (dev_conf->dv_xmeta_en != + MLX5_XMETA_MODE_LEGACY) && + mlx5_flow_ext_mreg_supported(dev)) return -rte_mtr_error_set(error, ENOTSUP, RTE_MTR_ERROR_TYPE_METER_POLICY, @@ -16592,7 +16608,7 @@ flow_dv_validate_mtr_policy_acts(struct rte_eth_dev *dev, "is not supported. Please try use " "default policy for meter."); ret = mlx5_flow_validate_action_queue(act, - action_flags, dev, + action_flags[i], dev, attr, &flow_err); if (ret < 0) return -rte_mtr_error_set(error, @@ -16601,14 +16617,14 @@ flow_dv_validate_mtr_policy_acts(struct rte_eth_dev *dev, NULL, flow_err.message ? flow_err.message : "Queue action validate check fail"); - action_flags |= MLX5_FLOW_ACTION_QUEUE; + action_flags[i] |= MLX5_FLOW_ACTION_QUEUE; ++actions_n; break; case RTE_FLOW_ACTION_TYPE_RSS: if (dev_conf->dv_flow_en && - (dev_conf->dv_xmeta_en != - MLX5_XMETA_MODE_LEGACY) && - mlx5_flow_ext_mreg_supported(dev)) + (dev_conf->dv_xmeta_en != + MLX5_XMETA_MODE_LEGACY) && + mlx5_flow_ext_mreg_supported(dev)) return -rte_mtr_error_set(error, ENOTSUP, RTE_MTR_ERROR_TYPE_METER_POLICY, @@ -16624,13 +16640,14 @@ flow_dv_validate_mtr_policy_acts(struct rte_eth_dev *dev, NULL, flow_err.message ? flow_err.message : "RSS action validate check fail"); - action_flags |= MLX5_FLOW_ACTION_RSS; + action_flags[i] |= MLX5_FLOW_ACTION_RSS; ++actions_n; - *is_rss = true; + /* Either G or Y will set the RSS. */ + rss_color[i] = act->conf; break; case RTE_FLOW_ACTION_TYPE_JUMP: ret = flow_dv_validate_action_jump(dev, - NULL, act, action_flags, + NULL, act, action_flags[i], attr, true, &flow_err); if (ret) return -rte_mtr_error_set(error, @@ -16640,7 +16657,7 @@ flow_dv_validate_mtr_policy_acts(struct rte_eth_dev *dev, flow_err.message : "Jump action validate check fail"); ++actions_n; - action_flags |= MLX5_FLOW_ACTION_JUMP; + action_flags[i] |= MLX5_FLOW_ACTION_JUMP; break; default: return -rte_mtr_error_set(error, ENOTSUP, @@ -16649,17 +16666,14 @@ flow_dv_validate_mtr_policy_acts(struct rte_eth_dev *dev, "Doesn't support optional action"); } } - /* Yellow is not supported, just skip. */ - if (i == RTE_COLOR_YELLOW) - continue; - if (action_flags & MLX5_FLOW_ACTION_PORT_ID) + if (action_flags[i] & MLX5_FLOW_ACTION_PORT_ID) domain_color[i] = MLX5_MTR_DOMAIN_TRANSFER_BIT; - else if ((action_flags & - (MLX5_FLOW_ACTION_RSS | MLX5_FLOW_ACTION_QUEUE)) || - (action_flags & MLX5_FLOW_ACTION_MARK)) + else if ((action_flags[i] & + (MLX5_FLOW_ACTION_RSS | MLX5_FLOW_ACTION_QUEUE)) || + (action_flags[i] & MLX5_FLOW_ACTION_MARK)) /* * Only support MLX5_XMETA_MODE_LEGACY - * so MARK action only in ingress domain. + * so MARK action is only in ingress domain. */ domain_color[i] = MLX5_MTR_DOMAIN_INGRESS_BIT; else @@ -16669,8 +16683,8 @@ flow_dv_validate_mtr_policy_acts(struct rte_eth_dev *dev, * with other actions. Drop action is mutually-exclusive * with any other action, except for Count action. */ - if ((action_flags & MLX5_FLOW_ACTION_DROP) && - (action_flags & ~MLX5_FLOW_ACTION_DROP)) { + if ((action_flags[i] & MLX5_FLOW_ACTION_DROP) && + (action_flags[i] & ~MLX5_FLOW_ACTION_DROP)) { return -rte_mtr_error_set(error, ENOTSUP, RTE_MTR_ERROR_TYPE_METER_POLICY, NULL, "Drop action is mutually-exclusive " @@ -16679,30 +16693,29 @@ flow_dv_validate_mtr_policy_acts(struct rte_eth_dev *dev, /* Eswitch has few restrictions on using items and actions */ if (domain_color[i] & MLX5_MTR_DOMAIN_TRANSFER_BIT) { if (!mlx5_flow_ext_mreg_supported(dev) && - action_flags & MLX5_FLOW_ACTION_MARK) + action_flags[i] & MLX5_FLOW_ACTION_MARK) return -rte_mtr_error_set(error, ENOTSUP, RTE_MTR_ERROR_TYPE_METER_POLICY, NULL, "unsupported action MARK"); - if (action_flags & MLX5_FLOW_ACTION_QUEUE) + if (action_flags[i] & MLX5_FLOW_ACTION_QUEUE) return -rte_mtr_error_set(error, ENOTSUP, RTE_MTR_ERROR_TYPE_METER_POLICY, NULL, "unsupported action QUEUE"); - if (action_flags & MLX5_FLOW_ACTION_RSS) + if (action_flags[i] & MLX5_FLOW_ACTION_RSS) return -rte_mtr_error_set(error, ENOTSUP, RTE_MTR_ERROR_TYPE_METER_POLICY, NULL, "unsupported action RSS"); - if (!(action_flags & MLX5_FLOW_FATE_ESWITCH_ACTIONS)) + if (!(action_flags[i] & MLX5_FLOW_FATE_ESWITCH_ACTIONS)) return -rte_mtr_error_set(error, ENOTSUP, RTE_MTR_ERROR_TYPE_METER_POLICY, NULL, "no fate action is found"); } else { - if (!(action_flags & MLX5_FLOW_FATE_ACTIONS) && - (domain_color[i] & - MLX5_MTR_DOMAIN_INGRESS_BIT)) { + if (!(action_flags[i] & MLX5_FLOW_FATE_ACTIONS) && + (domain_color[i] & MLX5_MTR_DOMAIN_INGRESS_BIT)) { if ((domain_color[i] & - MLX5_MTR_DOMAIN_EGRESS_BIT)) + MLX5_MTR_DOMAIN_EGRESS_BIT)) domain_color[i] = - MLX5_MTR_DOMAIN_EGRESS_BIT; + MLX5_MTR_DOMAIN_EGRESS_BIT; else return -rte_mtr_error_set(error, ENOTSUP, @@ -16710,9 +16723,27 @@ flow_dv_validate_mtr_policy_acts(struct rte_eth_dev *dev, NULL, "no fate action is found"); } } - if (domain_color[i] != def_domain) - *domain_bitmap = domain_color[i]; } + /* If both colors have RSS, the attributes should be the same. */ + if (flow_dv_mtr_policy_rss_compare(rss_color[RTE_COLOR_GREEN], + rss_color[RTE_COLOR_YELLOW])) + return -rte_mtr_error_set(error, EINVAL, + RTE_MTR_ERROR_TYPE_METER_POLICY, + NULL, "policy RSS attr conflict"); + if (rss_color[RTE_COLOR_GREEN] || rss_color[RTE_COLOR_YELLOW]) + *is_rss = true; + /* "domain_color[C]" is non-zero for each color, default is ALL. */ + if (!def_green && !def_yellow && + domain_color[RTE_COLOR_GREEN] != domain_color[RTE_COLOR_YELLOW]) + return -rte_mtr_error_set(error, EINVAL, + RTE_MTR_ERROR_TYPE_METER_POLICY, + NULL, "policy domains conflict"); + /* + * At least one color policy is listed in the actions, the domains + * to be supported should be the intersection. + */ + *domain_bitmap = domain_color[RTE_COLOR_GREEN] & + domain_color[RTE_COLOR_YELLOW]; return 0; } diff --git a/drivers/net/mlx5/mlx5_flow_meter.c b/drivers/net/mlx5/mlx5_flow_meter.c index d7ce5cd2f6..d313786eb3 100644 --- a/drivers/net/mlx5/mlx5_flow_meter.c +++ b/drivers/net/mlx5/mlx5_flow_meter.c @@ -660,7 +660,8 @@ mlx5_flow_meter_policy_add(struct rte_eth_dev *dev, 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); + &is_rss, &domain_bitmap, + &is_def_policy, error); if (ret) return ret; if (!domain_bitmap) -- 2.27.0 ^ permalink raw reply [flat|nested] 24+ messages in thread
* [dpdk-dev] [PATCH 3/6] net/mlx5: enable meter bucket overflow for yellow color 2021-07-05 15:57 [dpdk-dev] [PATCH 0/6] support yellow color policy in mlx5 Bing Zhao 2021-07-05 15:57 ` [dpdk-dev] [PATCH 1/6] net/mlx5: add yellow color default policy Bing Zhao 2021-07-05 15:57 ` [dpdk-dev] [PATCH 2/6] net/mlx5: support yellow in meter policy validation Bing Zhao @ 2021-07-05 15:57 ` Bing Zhao 2021-07-05 15:57 ` [dpdk-dev] [PATCH 4/6] net/mlx5: added support for yellow policy rules Bing Zhao ` (4 subsequent siblings) 7 siblings, 0 replies; 24+ messages in thread From: Bing Zhao @ 2021-07-05 15:57 UTC (permalink / raw) To: viacheslavo, matan; +Cc: dev, orika, rasland To support the meter policy for yellow action, the prerequisite is that the hardware needs to support the EBS and then the yellow color of packets can be marked for the further steering. In the current implementation EBS and overflow were ignored when creating a meter profile. With this commit, if EBS is set by the application, the generation of yellow color will be enabled in the hardware for flow rules steering of packets. Signed-off-by: Bing Zhao <bingz@nvidia.com> --- drivers/net/mlx5/mlx5_flow_aso.c | 3 +++ drivers/net/mlx5/mlx5_flow_meter.c | 10 +++++----- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/drivers/net/mlx5/mlx5_flow_aso.c b/drivers/net/mlx5/mlx5_flow_aso.c index 64631ffc29..a8cb590b57 100644 --- a/drivers/net/mlx5/mlx5_flow_aso.c +++ b/drivers/net/mlx5/mlx5_flow_aso.c @@ -747,6 +747,9 @@ mlx5_aso_mtr_sq_enqueue_single(struct mlx5_aso_sq *sq, wqe->aso_dseg.mtrs[dseg_idx].v_bo_sc_bbog_mm = RTE_BE32((1 << ASO_DSEG_VALID_OFFSET) | (MLX5_FLOW_COLOR_GREEN << ASO_DSEG_SC_OFFSET)); + if (fm->profile->srtcm_prm.ebs_eir) + wqe->aso_dseg.mtrs[dseg_idx].v_bo_sc_bbog_mm |= + RTE_BE32(1 << ASO_DSEG_BO_OFFSET); sq->head++; sq->pi += 2;/* Each WQE contains 2 WQEBB's. */ rte_io_wmb(); diff --git a/drivers/net/mlx5/mlx5_flow_meter.c b/drivers/net/mlx5/mlx5_flow_meter.c index d313786eb3..68351db1ce 100644 --- a/drivers/net/mlx5/mlx5_flow_meter.c +++ b/drivers/net/mlx5/mlx5_flow_meter.c @@ -312,9 +312,9 @@ mlx5_flow_meter_param_fill(struct mlx5_flow_meter_profile *fmp, cbs_man = man; cbs_exp = exp; srtcm->cbs_cir = rte_cpu_to_be_32(cbs_exp << ASO_DSEG_CBS_EXP_OFFSET | - cbs_man << ASO_DSEG_CBS_MAN_OFFSET | - cir_exp << ASO_DSEG_CIR_EXP_OFFSET | - cir_man); + cbs_man << ASO_DSEG_CBS_MAN_OFFSET | + cir_exp << ASO_DSEG_CIR_EXP_OFFSET | + cir_man); mlx5_flow_meter_xbs_man_exp_calc(ebs, &man, &exp); /* Check if ebs mantissa is too large. */ if (exp > ASO_DSEG_EXP_MASK) @@ -325,7 +325,7 @@ mlx5_flow_meter_param_fill(struct mlx5_flow_meter_profile *fmp, ebs_man = man; ebs_exp = exp; srtcm->ebs_eir = rte_cpu_to_be_32(ebs_exp << ASO_DSEG_EBS_EXP_OFFSET | - ebs_man << ASO_DSEG_EBS_MAN_OFFSET); + ebs_man << ASO_DSEG_EBS_MAN_OFFSET); return 0; } @@ -414,7 +414,7 @@ mlx5_flow_meter_profile_add(struct rte_eth_dev *dev, return ret; /* Meter profile memory allocation. */ fmp = mlx5_malloc(MLX5_MEM_ZERO, sizeof(struct mlx5_flow_meter_profile), - RTE_CACHE_LINE_SIZE, SOCKET_ID_ANY); + RTE_CACHE_LINE_SIZE, SOCKET_ID_ANY); if (fmp == NULL) return -rte_mtr_error_set(error, ENOMEM, RTE_MTR_ERROR_TYPE_UNSPECIFIED, -- 2.27.0 ^ permalink raw reply [flat|nested] 24+ messages in thread
* [dpdk-dev] [PATCH 4/6] net/mlx5: added support for yellow policy rules 2021-07-05 15:57 [dpdk-dev] [PATCH 0/6] support yellow color policy in mlx5 Bing Zhao ` (2 preceding siblings ...) 2021-07-05 15:57 ` [dpdk-dev] [PATCH 3/6] net/mlx5: enable meter bucket overflow for yellow color Bing Zhao @ 2021-07-05 15:57 ` Bing Zhao 2021-07-05 15:57 ` [dpdk-dev] [PATCH 5/6] net/mlx5: split policies handling of colors Bing Zhao ` (3 subsequent siblings) 7 siblings, 0 replies; 24+ messages in thread From: Bing Zhao @ 2021-07-05 15:57 UTC (permalink / raw) To: viacheslavo, matan; +Cc: dev, orika, rasland 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 <bingz@nvidia.com> --- drivers/net/mlx5/mlx5_flow_dv.c | 43 +++++++++++++++++++----------- drivers/net/mlx5/mlx5_flow_meter.c | 43 ++++++++++++++++++------------ 2 files changed, 53 insertions(+), 33 deletions(-) diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c index f53cf65e87..c606fa9471 100644 --- a/drivers/net/mlx5/mlx5_flow_dv.c +++ b/drivers/net/mlx5/mlx5_flow_dv.c @@ -14702,22 +14702,34 @@ __flow_dv_create_domain_policy_acts(struct rte_eth_dev *dev, sizeof(struct mlx5_modification_cmd) * (MLX5_MAX_MODIFY_NUM + 1)]; } mhdr_dummy; + struct mlx5_flow_mtr_mng *mtrmng = priv->sh->mtrmng; egress = (domain == MLX5_MTR_DOMAIN_EGRESS) ? 1 : 0; transfer = (domain == MLX5_MTR_DOMAIN_TRANSFER) ? 1 : 0; 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)); 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]; - for (act = actions[i]; - act && act->type != RTE_FLOW_ACTION_TYPE_END; - act++) { + act = actions[i]; + /* No policy action, use default. */ + if (!act || act->type == RTE_FLOW_ACTION_TYPE_END) { + if (!mtrmng->def_policy[domain]) + return -rte_mtr_error_set(error, + ENOTSUP, + RTE_MTR_ERROR_TYPE_METER_POLICY, + NULL, + "Default policy not created."); + act_cnt->dr_jump_action[domain] = + mtrmng->def_policy[domain]->dr_jump_action[i]; + continue; + } + for (; act->type != RTE_FLOW_ACTION_TYPE_END; act++) { switch (act->type) { case RTE_FLOW_ACTION_TYPE_MARK: { @@ -14933,6 +14945,7 @@ __flow_dv_create_domain_policy_acts(struct rte_eth_dev *dev, action_flags |= MLX5_FLOW_ACTION_PORT_ID; break; } + /* G & Y can use the same table by default. */ case RTE_FLOW_ACTION_TYPE_JUMP: { uint32_t jump_group = 0; @@ -14947,7 +14960,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, @@ -15036,6 +15049,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); + /* Rollback is done in mlx5 layer. */ if (ret) return ret; } @@ -15538,12 +15552,10 @@ __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; } @@ -15555,13 +15567,12 @@ __flow_dv_create_policy_acts_rules(struct rte_eth_dev *dev, "mark action for policy."); return -1; } - 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) { @@ -15576,7 +15587,7 @@ __flow_dv_create_policy_acts_rules(struct rte_eth_dev *dev, return -1; } acts[i].dv_actions[acts[i].actions_n] = - port_action->action; + port_action->action; acts[i].actions_n++; break; case MLX5_FLOW_FATE_DROP: @@ -15588,15 +15599,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."); return -1; } acts[i].dv_actions[acts[i].actions_n] = - hrxq->action; + hrxq->action; acts[i].actions_n++; break; default: @@ -15610,7 +15621,7 @@ __flow_dv_create_policy_acts_rules(struct rte_eth_dev *dev, if (__flow_dv_create_domain_policy_rules(dev, sub_policy, egress, transfer, false, acts)) { DRV_LOG(ERR, - "Failed to create policy rules per domain."); + "Failed to create policy rules per domain."); return -1; } return 0; diff --git a/drivers/net/mlx5/mlx5_flow_meter.c b/drivers/net/mlx5/mlx5_flow_meter.c index 68351db1ce..025b469a1d 100644 --- a/drivers/net/mlx5/mlx5_flow_meter.c +++ b/drivers/net/mlx5/mlx5_flow_meter.c @@ -644,21 +644,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); @@ -666,8 +665,8 @@ mlx5_flow_meter_policy_add(struct rte_eth_dev *dev, return ret; if (!domain_bitmap) return -rte_mtr_error_set(error, ENOTSUP, - RTE_MTR_ERROR_TYPE_METER_POLICY, - NULL, "fail to find policy domain."); + RTE_MTR_ERROR_TYPE_METER_POLICY, + NULL, "fail to find policy domain."); if (is_def_policy) { if (priv->sh->mtrmng->def_policy_id != MLX5_INVALID_POLICY_ID) return -rte_mtr_error_set(error, EEXIST, @@ -689,16 +688,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, @@ -715,7 +720,7 @@ 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); + &sub_policy_idx); if (!sub_policy) goto policy_add_err; if (sub_policy_idx > MLX5_MAX_SUB_POLICY_TBL_NUM) @@ -727,7 +732,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 >> @@ -743,13 +748,17 @@ mlx5_flow_meter_policy_add(struct rte_eth_dev *dev, 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, policy->actions, error); if (ret) goto policy_add_err; + /* + * 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) { /* Create policy rules in HW. */ ret = mlx5_flow_create_policy_rules(dev, mtr_policy); -- 2.27.0 ^ permalink raw reply [flat|nested] 24+ messages in thread
* [dpdk-dev] [PATCH 5/6] net/mlx5: split policies handling of colors 2021-07-05 15:57 [dpdk-dev] [PATCH 0/6] support yellow color policy in mlx5 Bing Zhao ` (3 preceding siblings ...) 2021-07-05 15:57 ` [dpdk-dev] [PATCH 4/6] net/mlx5: added support for yellow policy rules Bing Zhao @ 2021-07-05 15:57 ` Bing Zhao 2021-07-05 15:57 ` [dpdk-dev] [PATCH 6/6] doc: update mlx5 metering policy part Bing Zhao ` (2 subsequent siblings) 7 siblings, 0 replies; 24+ messages in thread From: Bing Zhao @ 2021-07-05 15:57 UTC (permalink / raw) To: viacheslavo, matan; +Cc: dev, orika, rasland If the fate action is either RSS or Queue of a meter policy, the action will only be created in the flow splitting stage. With queue as the fate action, only one sub-policy is needed. And RSS will have more than one sub-policies if there is an expansion. Since the RSS parameters are the same for both green and yellow colors except the queues, the expansion result will be unique. Even if only one color has the RSS action, the checking and possible expansion will be done then. For each sub-policy, the action rules need to be created separately on its own policy table. Signed-off-by: Bing Zhao <bingz@nvidia.com> --- drivers/net/mlx5/mlx5_flow.c | 37 +++++++++++--------- drivers/net/mlx5/mlx5_flow_dv.c | 61 +++++++++++++++++---------------- 2 files changed, 51 insertions(+), 47 deletions(-) diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c index 61c72da2aa..89056ec45e 100644 --- a/drivers/net/mlx5/mlx5_flow.c +++ b/drivers/net/mlx5/mlx5_flow.c @@ -4604,7 +4604,7 @@ get_meter_sub_policy(struct rte_eth_dev *dev, uint32_t i; MLX5_ASSERT(wks); - /** + /* * This is a tmp dev_flow, * no need to register any matcher for it in translate. */ @@ -4612,18 +4612,19 @@ get_meter_sub_policy(struct rte_eth_dev *dev, for (i = 0; i < MLX5_MTR_RTE_COLORS; i++) { struct mlx5_flow dev_flow = {0}; struct mlx5_flow_handle dev_handle = { {0} }; + uint8_t fate = policy->act_cnt[i].fate_action; - if (policy->is_rss) { + if (fate == MLX5_FLOW_FATE_SHARED_RSS) { const void *rss_act = policy->act_cnt[i].rss->conf; struct rte_flow_action rss_actions[2] = { [0] = { .type = RTE_FLOW_ACTION_TYPE_RSS, - .conf = rss_act + .conf = rss_act, }, [1] = { .type = RTE_FLOW_ACTION_TYPE_END, - .conf = NULL + .conf = NULL, } }; @@ -4650,7 +4651,8 @@ get_meter_sub_policy(struct rte_eth_dev *dev, rss_desc_v[i].tunnel = !!(dev_flow.handle->layers & MLX5_FLOW_LAYER_TUNNEL); - } else { + rss_desc[i] = &rss_desc_v[i]; + } else if (fate == MLX5_FLOW_FATE_QUEUE) { /* This is queue action. */ rss_desc_v[i] = wks->rss_desc; rss_desc_v[i].key_len = 0; @@ -4658,24 +4660,24 @@ get_meter_sub_policy(struct rte_eth_dev *dev, rss_desc_v[i].queue = &policy->act_cnt[i].queue; rss_desc_v[i].queue_num = 1; + rss_desc[i] = &rss_desc_v[i]; + } else { + rss_desc[i] = NULL; } - rss_desc[i] = &rss_desc_v[i]; } sub_policy = flow_drv_meter_sub_policy_rss_prepare(dev, flow, policy, rss_desc); } else { enum mlx5_meter_domain mtr_domain = attr->transfer ? MLX5_MTR_DOMAIN_TRANSFER : - attr->egress ? MLX5_MTR_DOMAIN_EGRESS : - MLX5_MTR_DOMAIN_INGRESS; + (attr->egress ? MLX5_MTR_DOMAIN_EGRESS : + MLX5_MTR_DOMAIN_INGRESS); sub_policy = policy->sub_policys[mtr_domain][0]; } - if (!sub_policy) { + if (!sub_policy) rte_flow_error_set(error, EINVAL, - RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL, - "Failed to get meter sub-policy."); - goto exit; - } + RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL, + "Failed to get meter sub-policy."); exit: return sub_policy; } @@ -4810,6 +4812,7 @@ flow_meter_split_prep(struct rte_eth_dev *dev, struct mlx5_flow_meter_sub_policy *sub_policy; struct mlx5_flow_tbl_data_entry *tbl_data; + /* Either G or Y can still use default policy. */ if (!fm->def_policy) { sub_policy = get_meter_sub_policy(dev, flow, fm->policy_id, attr, @@ -4819,8 +4822,8 @@ flow_meter_split_prep(struct rte_eth_dev *dev, } else { enum mlx5_meter_domain mtr_domain = attr->transfer ? MLX5_MTR_DOMAIN_TRANSFER : - attr->egress ? MLX5_MTR_DOMAIN_EGRESS : - MLX5_MTR_DOMAIN_INGRESS; + (attr->egress ? MLX5_MTR_DOMAIN_EGRESS : + MLX5_MTR_DOMAIN_INGRESS); sub_policy = &priv->sh->mtrmng->def_policy[mtr_domain]->sub_policy; @@ -4836,8 +4839,8 @@ flow_meter_split_prep(struct rte_eth_dev *dev, actions_pre++; if (!tag_action) return rte_flow_error_set(error, ENOMEM, - RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL, - "No tag action space."); + RTE_FLOW_ERROR_TYPE_UNSPECIFIED, + NULL, "No tag action space."); if (!mtr_flow_id) { tag_action->type = RTE_FLOW_ACTION_TYPE_VOID; goto exit; diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c index c606fa9471..0f62537938 100644 --- a/drivers/net/mlx5/mlx5_flow_dv.c +++ b/drivers/net/mlx5/mlx5_flow_dv.c @@ -14550,7 +14550,7 @@ __flow_dv_destroy_sub_policy_rules(struct rte_eth_dev *dev, } if (sub_policy->color_matcher[i]) { tbl = container_of(sub_policy->color_matcher[i]->tbl, - typeof(*tbl), tbl); + typeof(*tbl), tbl); mlx5_cache_unregister(&tbl->matchers, &sub_policy->color_matcher[i]->entry); sub_policy->color_matcher[i] = NULL; @@ -14563,13 +14563,13 @@ __flow_dv_destroy_sub_policy_rules(struct rte_eth_dev *dev, } if (sub_policy->jump_tbl[i]) { flow_dv_tbl_resource_release(MLX5_SH(dev), - sub_policy->jump_tbl[i]); + sub_policy->jump_tbl[i]); sub_policy->jump_tbl[i] = NULL; } } if (sub_policy->tbl_rsc) { flow_dv_tbl_resource_release(MLX5_SH(dev), - sub_policy->tbl_rsc); + sub_policy->tbl_rsc); sub_policy->tbl_rsc = NULL; } } @@ -14586,7 +14586,7 @@ __flow_dv_destroy_sub_policy_rules(struct rte_eth_dev *dev, */ static void flow_dv_destroy_policy_rules(struct rte_eth_dev *dev, - struct mlx5_flow_meter_policy *mtr_policy) + struct mlx5_flow_meter_policy *mtr_policy) { uint32_t i, j; struct mlx5_flow_meter_sub_policy *sub_policy; @@ -14599,8 +14599,8 @@ flow_dv_destroy_policy_rules(struct rte_eth_dev *dev, for (j = 0; j < sub_policy_num; j++) { sub_policy = mtr_policy->sub_policys[i][j]; if (sub_policy) - __flow_dv_destroy_sub_policy_rules - (dev, sub_policy); + __flow_dv_destroy_sub_policy_rules(dev, + sub_policy); } } } @@ -15550,6 +15550,7 @@ __flow_dv_create_policy_acts_rules(struct rte_eth_dev *dev, uint8_t egress, transfer; int i; + /* If RSS or Queue, no previous actions / rules is created. */ for (i = 0; i < RTE_COLORS; i++) { acts[i].actions_n = 0; if (i == RTE_COLOR_RED) { @@ -15994,36 +15995,35 @@ flow_dv_meter_sub_policy_rss_prepare(struct rte_eth_dev *dev, sub_policy_num = (mtr_policy->sub_policy_num >> (MLX5_MTR_SUB_POLICY_NUM_SHIFT * domain)) & MLX5_MTR_SUB_POLICY_NUM_MASK; - for (i = 0; i < sub_policy_num; - i++) { - for (j = 0; j < MLX5_MTR_RTE_COLORS; j++) { - if (rss_desc[j] && - hrxq_idx[j] != - mtr_policy->sub_policys[domain][i]->rix_hrxq[j]) + for (j = 0; j < sub_policy_num; j++) { + for (i = 0; i < MLX5_MTR_RTE_COLORS; i++) { + if (rss_desc[i] && + hrxq_idx[i] != + mtr_policy->sub_policys[domain][j]->rix_hrxq[i]) break; } - if (j >= MLX5_MTR_RTE_COLORS) { + if (i >= MLX5_MTR_RTE_COLORS) { /* * Found the sub policy table with - * the same queue per color + * the same queue per color. */ rte_spinlock_unlock(&mtr_policy->sl); - for (j = 0; j < MLX5_MTR_RTE_COLORS; j++) - mlx5_hrxq_release(dev, hrxq_idx[j]); - return mtr_policy->sub_policys[domain][i]; + for (i = 0; i < MLX5_MTR_RTE_COLORS; i++) + mlx5_hrxq_release(dev, hrxq_idx[i]); + return mtr_policy->sub_policys[domain][j]; } } /* Create sub policy. */ if (!mtr_policy->sub_policys[domain][0]->rix_hrxq[0]) { - /* Reuse the first dummy sub_policy*/ + /* Reuse the first dummy sub_policy. */ sub_policy = mtr_policy->sub_policys[domain][0]; sub_policy_idx = sub_policy->idx; } else { sub_policy = mlx5_ipool_zmalloc (priv->sh->ipool[MLX5_IPOOL_MTR_POLICY], - &sub_policy_idx); + &sub_policy_idx); if (!sub_policy || - sub_policy_idx > MLX5_MAX_SUB_POLICY_TBL_NUM) { + sub_policy_idx > MLX5_MAX_SUB_POLICY_TBL_NUM) { for (i = 0; i < MLX5_MTR_RTE_COLORS; i++) mlx5_hrxq_release(dev, hrxq_idx[i]); goto rss_sub_policy_error; @@ -16040,9 +16040,9 @@ flow_dv_meter_sub_policy_rss_prepare(struct rte_eth_dev *dev, * RSS action to Queue action. */ hrxq = mlx5_ipool_get(priv->sh->ipool[MLX5_IPOOL_HRXQ], - hrxq_idx[i]); + hrxq_idx[i]); if (!hrxq) { - DRV_LOG(ERR, "Failed to create policy hrxq"); + DRV_LOG(ERR, "Failed to get policy hrxq."); goto rss_sub_policy_error; } act_cnt = &mtr_policy->act_cnt[i]; @@ -16056,19 +16056,21 @@ flow_dv_meter_sub_policy_rss_prepare(struct rte_eth_dev *dev, } } if (__flow_dv_create_policy_acts_rules(dev, mtr_policy, - sub_policy, domain)) { + sub_policy, domain)) { DRV_LOG(ERR, "Failed to create policy " - "rules per domain."); + "rules for ingress domain."); goto rss_sub_policy_error; } if (sub_policy != mtr_policy->sub_policys[domain][0]) { i = (mtr_policy->sub_policy_num >> (MLX5_MTR_SUB_POLICY_NUM_SHIFT * domain)) & MLX5_MTR_SUB_POLICY_NUM_MASK; + if (i >= MLX5_MTR_RSS_MAX_SUB_POLICY) { + DRV_LOG(ERR, "No free sub-policy slot."); + goto rss_sub_policy_error; + } mtr_policy->sub_policys[domain][i] = sub_policy; i++; - if (i > MLX5_MTR_RSS_MAX_SUB_POLICY) - goto rss_sub_policy_error; mtr_policy->sub_policy_num &= ~(MLX5_MTR_SUB_POLICY_NUM_MASK << (MLX5_MTR_SUB_POLICY_NUM_SHIFT * domain)); mtr_policy->sub_policy_num |= @@ -16085,8 +16087,7 @@ flow_dv_meter_sub_policy_rss_prepare(struct rte_eth_dev *dev, (MLX5_MTR_SUB_POLICY_NUM_SHIFT * domain)) & MLX5_MTR_SUB_POLICY_NUM_MASK; mtr_policy->sub_policys[domain][i] = NULL; - mlx5_ipool_free - (priv->sh->ipool[MLX5_IPOOL_MTR_POLICY], + mlx5_ipool_free(priv->sh->ipool[MLX5_IPOOL_MTR_POLICY], sub_policy->idx); } } @@ -16108,7 +16109,7 @@ flow_dv_meter_sub_policy_rss_prepare(struct rte_eth_dev *dev, */ static void flow_dv_destroy_sub_policy_with_rxq(struct rte_eth_dev *dev, - struct mlx5_flow_meter_policy *mtr_policy) + struct mlx5_flow_meter_policy *mtr_policy) { struct mlx5_priv *priv = dev->data->dev_private; struct mlx5_flow_meter_sub_policy *sub_policy = NULL; @@ -16154,7 +16155,7 @@ flow_dv_destroy_sub_policy_with_rxq(struct rte_eth_dev *dev, case MLX5_FLOW_FATE_QUEUE: sub_policy = mtr_policy->sub_policys[domain][0]; __flow_dv_destroy_sub_policy_rules(dev, - sub_policy); + sub_policy); break; default: /*Other actions without queue and do nothing*/ -- 2.27.0 ^ permalink raw reply [flat|nested] 24+ messages in thread
* [dpdk-dev] [PATCH 6/6] doc: update mlx5 metering policy part 2021-07-05 15:57 [dpdk-dev] [PATCH 0/6] support yellow color policy in mlx5 Bing Zhao ` (4 preceding siblings ...) 2021-07-05 15:57 ` [dpdk-dev] [PATCH 5/6] net/mlx5: split policies handling of colors Bing Zhao @ 2021-07-05 15:57 ` Bing Zhao 2021-07-18 17:18 ` [dpdk-dev] [PATCH v2 0/7] support yellow color policy in mlx5 Bing Zhao 2021-07-21 8:54 ` [dpdk-dev] [PATCH v3 0/7] support yellow color policy in mlx5 Bing Zhao 7 siblings, 0 replies; 24+ messages in thread From: Bing Zhao @ 2021-07-05 15:57 UTC (permalink / raw) To: viacheslavo, matan; +Cc: dev, orika, rasland Added support for yellow color. Signed-off-by: Bing Zhao <bingz@nvidia.com> --- doc/guides/rel_notes/release_21_08.rst | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/doc/guides/rel_notes/release_21_08.rst b/doc/guides/rel_notes/release_21_08.rst index a6ecfdf3ce..93566a8500 100644 --- a/doc/guides/rel_notes/release_21_08.rst +++ b/doc/guides/rel_notes/release_21_08.rst @@ -55,6 +55,11 @@ New Features Also, make sure to start the actual text at the margin. ======================================================= +* **Updated Mellanox mlx5 driver.** + + Updated the Mellanox mlx5 driver with new features and improvements, including: + + * Added support for metering policy actions of yellow color. Removed Items ------------- -- 2.27.0 ^ permalink raw reply [flat|nested] 24+ messages in thread
* [dpdk-dev] [PATCH v2 0/7] support yellow color policy in mlx5 2021-07-05 15:57 [dpdk-dev] [PATCH 0/6] support yellow color policy in mlx5 Bing Zhao ` (5 preceding siblings ...) 2021-07-05 15:57 ` [dpdk-dev] [PATCH 6/6] doc: update mlx5 metering policy part Bing Zhao @ 2021-07-18 17:18 ` Bing Zhao 2021-07-18 17:18 ` [dpdk-dev] [PATCH v2 1/7] net/mlx5: handle yellow case in default meter policy Bing Zhao ` (6 more replies) 2021-07-21 8:54 ` [dpdk-dev] [PATCH v3 0/7] support yellow color policy in mlx5 Bing Zhao 7 siblings, 7 replies; 24+ messages in thread From: Bing Zhao @ 2021-07-18 17:18 UTC (permalink / raw) To: viacheslavo, matan; +Cc: dev, orika, rasland, thomas, lizh, shunh When creating a meter policy, the actions for yellow color can be specified together with green color. The mlx5 PMD now supports to set the policy actions for yellow color. The actions list that is supported for yellow is the same as that for green. --- v2: * bug fixes * add policy and profile consistency checking * add trTCM RFC2698 and RFC4115 support --- Acked-by: Matan Azrad <matan@nvidia.com> Bing Zhao (7): net/mlx5: handle yellow case in default meter policy net/mlx5: enable meter bucket overflow for yellow color net/mlx5: added support for yellow policy rules net/mlx5: split policies handling of colors net/mlx5: support yellow in meter policy validation net/mlx5: check consistency of meter policy and profile net/mlx5: add meter support for trTCM profiles doc/guides/nics/mlx5.rst | 7 +- doc/guides/rel_notes/release_21_08.rst | 2 + drivers/common/mlx5/mlx5_prm.h | 5 +- drivers/net/mlx5/mlx5.h | 20 +- drivers/net/mlx5/mlx5_flow.c | 46 +-- drivers/net/mlx5/mlx5_flow.h | 4 +- drivers/net/mlx5/mlx5_flow_aso.c | 21 ++ drivers/net/mlx5/mlx5_flow_dv.c | 470 +++++++++++++++---------- drivers/net/mlx5/mlx5_flow_meter.c | 203 +++++++---- 9 files changed, 490 insertions(+), 288 deletions(-) -- 2.27.0 ^ permalink raw reply [flat|nested] 24+ messages in thread
* [dpdk-dev] [PATCH v2 1/7] net/mlx5: handle yellow case in default meter policy 2021-07-18 17:18 ` [dpdk-dev] [PATCH v2 0/7] support yellow color policy in mlx5 Bing Zhao @ 2021-07-18 17:18 ` Bing Zhao 2021-07-18 17:18 ` [dpdk-dev] [PATCH v2 2/7] net/mlx5: enable meter bucket overflow for yellow color Bing Zhao ` (5 subsequent siblings) 6 siblings, 0 replies; 24+ messages in thread From: Bing Zhao @ 2021-07-18 17:18 UTC (permalink / raw) To: viacheslavo, matan; +Cc: dev, orika, rasland, thomas, lizh, shunh In order to support the yellow color for the default meter policy, the default policy action for yellow should be created together with the green policy. The default policy action for yellow action is the same as that for green. In the same table, the same matcher will be reused for yellow and the destination group will be the same. Signed-off-by: Bing Zhao <bingz@nvidia.com> --- drivers/net/mlx5/mlx5.h | 6 ++ drivers/net/mlx5/mlx5_flow_dv.c | 144 +++++++++++++++++++------------- 2 files changed, 93 insertions(+), 57 deletions(-) diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h index 94618e10fa..791696caf4 100644 --- a/drivers/net/mlx5/mlx5.h +++ b/drivers/net/mlx5/mlx5.h @@ -641,6 +641,12 @@ struct mlx5_dev_shared_port { #define MLX5_MTR_TABLE_ID_SUFFIX 1 /* Drop table_id on MLX5_FLOW_TABLE_LEVEL_METER. */ #define MLX5_MTR_TABLE_ID_DROP 2 +/* Priority of the meter policy matcher. */ +#define MLX5_MTR_POLICY_MATCHER_PRIO 0 +/* Default policy. */ +#define MLX5_MTR_POLICY_MODE_DEF 1 +/* Only green color valid. */ +#define MLX5_MTR_POLICY_MODE_OG 2 enum mlx5_meter_domain { MLX5_MTR_DOMAIN_INGRESS, diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c index d250486950..cfc646c5e5 100644 --- a/drivers/net/mlx5/mlx5_flow_dv.c +++ b/drivers/net/mlx5/mlx5_flow_dv.c @@ -188,7 +188,7 @@ flow_dv_attr_init(const struct rte_flow_item *item, union flow_dv_attr *attr, attr->valid = 1; } -/** +/* * Convert rte_mtr_color to mlx5 color. * * @param[in] rcol @@ -197,7 +197,7 @@ flow_dv_attr_init(const struct rte_flow_item *item, union flow_dv_attr *attr, * @return * mlx5 color. */ -static int +static inline int rte_col_2_mlx5_col(enum rte_color rcol) { switch (rcol) { @@ -15892,7 +15892,7 @@ flow_dv_destroy_mtr_drop_tbls(struct rte_eth_dev *dev) static void __flow_dv_destroy_domain_def_policy(struct rte_eth_dev *dev, - enum mlx5_meter_domain domain) + enum mlx5_meter_domain domain) { struct mlx5_priv *priv = dev->data->dev_private; struct mlx5_flow_meter_def_policy *def_policy = @@ -15943,21 +15943,20 @@ __flow_dv_create_policy_flow(struct rte_eth_dev *dev, if (match_src_port && (priv->representor || priv->master)) { if (flow_dv_translate_item_port_id(dev, matcher.buf, value.buf, item, attr)) { - DRV_LOG(ERR, - "Failed to create meter policy flow with port."); + DRV_LOG(ERR, "Failed to create meter policy%d flow's" + " value with port.", color); return -1; } } flow_dv_match_meta_reg(matcher.buf, value.buf, - (enum modify_reg)color_reg_c_idx, - rte_col_2_mlx5_col(color), - UINT32_MAX); + (enum modify_reg)color_reg_c_idx, + rte_col_2_mlx5_col(color), UINT32_MAX); misc_mask = flow_dv_matcher_enable(value.buf); __flow_dv_adjust_buf_size(&value.size, misc_mask); - ret = mlx5_flow_os_create_flow(matcher_object, - (void *)&value, actions_n, actions, rule); + ret = mlx5_flow_os_create_flow(matcher_object, (void *)&value, + actions_n, actions, rule); if (ret) { - DRV_LOG(ERR, "Failed to create meter policy flow."); + DRV_LOG(ERR, "Failed to create meter policy%d flow.", color); return -1; } return 0; @@ -15991,13 +15990,13 @@ __flow_dv_create_policy_matcher(struct rte_eth_dev *dev, }; struct mlx5_flow_tbl_data_entry *tbl_data; struct mlx5_priv *priv = dev->data->dev_private; - uint32_t color_mask = (UINT32_C(1) << MLX5_MTR_COLOR_BITS) - 1; + const uint32_t color_mask = (UINT32_C(1) << MLX5_MTR_COLOR_BITS) - 1; if (match_src_port && (priv->representor || priv->master)) { if (flow_dv_translate_item_port_id(dev, matcher.mask.buf, value.buf, item, attr)) { - DRV_LOG(ERR, - "Failed to register meter drop matcher with port."); + DRV_LOG(ERR, "Failed to register meter policy%d matcher" + " with port.", priority); return -1; } } @@ -16007,7 +16006,7 @@ __flow_dv_create_policy_matcher(struct rte_eth_dev *dev, (enum modify_reg)color_reg_c_idx, 0, color_mask); matcher.priority = priority; matcher.crc = rte_raw_cksum((const void *)matcher.mask.buf, - matcher.mask.size); + matcher.mask.size); entry = mlx5_list_register(tbl_data->matchers, &ctx); if (!entry) { DRV_LOG(ERR, "Failed to register meter drop matcher."); @@ -16055,6 +16054,8 @@ __flow_dv_create_domain_policy_rules(struct rte_eth_dev *dev, int i; int ret = mlx5_flow_get_reg_id(dev, MLX5_MTR_COLOR, 0, &flow_err); struct mlx5_sub_policy_color_rule *color_rule; + bool svport_match; + struct mlx5_sub_policy_color_rule *tmp_rules[RTE_COLORS] = {NULL}; if (ret < 0) return -1; @@ -16073,7 +16074,7 @@ __flow_dv_create_domain_policy_rules(struct rte_eth_dev *dev, color_reg_c_idx = ret; for (i = 0; i < RTE_COLORS; i++) { TAILQ_INIT(&sub_policy->color_rules[i]); - if (i == RTE_COLOR_YELLOW || !acts[i].actions_n) + if (!acts[i].actions_n) continue; color_rule = mlx5_malloc(MLX5_MEM_ZERO, sizeof(struct mlx5_sub_policy_color_rule), @@ -16082,45 +16083,52 @@ __flow_dv_create_domain_policy_rules(struct rte_eth_dev *dev, DRV_LOG(ERR, "No memory to create color rule."); goto err_exit; } + tmp_rules[i] = color_rule; + TAILQ_INSERT_TAIL(&sub_policy->color_rules[i], + color_rule, next_port); color_rule->src_port = priv->representor_id; + /* No use. */ attr.priority = i; - /* Create matchers for Color. */ - if (__flow_dv_create_policy_matcher(dev, - color_reg_c_idx, i, sub_policy, &attr, - (i != RTE_COLOR_RED ? match_src_port : false), - NULL, &color_rule->matcher, &flow_err)) { - DRV_LOG(ERR, "Failed to create color matcher."); + /* Create matchers for colors. */ + svport_match = (i != RTE_COLOR_RED) ? match_src_port : false; + if (__flow_dv_create_policy_matcher(dev, color_reg_c_idx, + MLX5_MTR_POLICY_MATCHER_PRIO, sub_policy, + &attr, svport_match, NULL, + &color_rule->matcher, &flow_err)) { + DRV_LOG(ERR, "Failed to create color%u matcher.", i); goto err_exit; } /* Create flow, matching color. */ if (__flow_dv_create_policy_flow(dev, color_reg_c_idx, (enum rte_color)i, color_rule->matcher->matcher_object, - acts[i].actions_n, - acts[i].dv_actions, - (i != RTE_COLOR_RED ? match_src_port : false), - NULL, &color_rule->rule, + acts[i].actions_n, acts[i].dv_actions, + svport_match, NULL, &color_rule->rule, &attr)) { - DRV_LOG(ERR, "Failed to create color rule."); + DRV_LOG(ERR, "Failed to create color%u rule.", i); goto err_exit; } - TAILQ_INSERT_TAIL(&sub_policy->color_rules[i], - color_rule, next_port); } return 0; err_exit: - if (color_rule) { - if (color_rule->rule) - mlx5_flow_os_destroy_flow(color_rule->rule); - if (color_rule->matcher) { - struct mlx5_flow_tbl_data_entry *tbl = - container_of(color_rule->matcher->tbl, - typeof(*tbl), tbl); - mlx5_list_unregister(tbl->matchers, + /* All the policy rules will be cleared. */ + do { + color_rule = tmp_rules[i]; + if (color_rule) { + if (color_rule->rule) + mlx5_flow_os_destroy_flow(color_rule->rule); + if (color_rule->matcher) { + struct mlx5_flow_tbl_data_entry *tbl = + container_of(color_rule->matcher->tbl, + typeof(*tbl), tbl); + mlx5_list_unregister(tbl->matchers, &color_rule->matcher->entry); + } + TAILQ_REMOVE(&sub_policy->color_rules[i], + color_rule, next_port); + mlx5_free(color_rule); } - mlx5_free(color_rule); - } + } while (i--); return -1; } @@ -16342,8 +16350,7 @@ __flow_dv_create_domain_def_policy(struct rte_eth_dev *dev, uint32_t domain) sizeof(struct mlx5_flow_meter_def_policy), RTE_CACHE_LINE_SIZE, SOCKET_ID_ANY); if (!def_policy) { - DRV_LOG(ERR, "Failed to alloc " - "default policy table."); + DRV_LOG(ERR, "Failed to alloc default policy table."); goto def_policy_error; } mtrmng->def_policy[domain] = def_policy; @@ -16359,26 +16366,48 @@ __flow_dv_create_domain_def_policy(struct rte_eth_dev *dev, uint32_t domain) } def_policy->sub_policy.jump_tbl[RTE_COLOR_GREEN] = jump_tbl; tbl_data = container_of(jump_tbl, - struct mlx5_flow_tbl_data_entry, tbl); + struct mlx5_flow_tbl_data_entry, tbl); def_policy->dr_jump_action[RTE_COLOR_GREEN] = tbl_data->jump.action; - acts[RTE_COLOR_GREEN].dv_actions[0] = - tbl_data->jump.action; + acts[RTE_COLOR_GREEN].dv_actions[0] = tbl_data->jump.action; acts[RTE_COLOR_GREEN].actions_n = 1; + /* + * YELLOW has the same default policy as GREEN does. + * G & Y share the same table and action. The 2nd time of table + * resource getting is just to update the reference count for + * the releasing stage. + */ + jump_tbl = flow_dv_tbl_resource_get(dev, + MLX5_FLOW_TABLE_LEVEL_METER, + egress, transfer, false, NULL, 0, + 0, MLX5_MTR_TABLE_ID_SUFFIX, &error); + if (!jump_tbl) { + DRV_LOG(ERR, + "Failed to get meter suffix table."); + goto def_policy_error; + } + def_policy->sub_policy.jump_tbl[RTE_COLOR_YELLOW] = jump_tbl; + tbl_data = container_of(jump_tbl, + struct mlx5_flow_tbl_data_entry, tbl); + def_policy->dr_jump_action[RTE_COLOR_YELLOW] = + tbl_data->jump.action; + acts[RTE_COLOR_YELLOW].dv_actions[0] = tbl_data->jump.action; + acts[RTE_COLOR_YELLOW].actions_n = 1; /* Create jump action to the drop table. */ if (!mtrmng->drop_tbl[domain]) { mtrmng->drop_tbl[domain] = flow_dv_tbl_resource_get (dev, MLX5_FLOW_TABLE_LEVEL_METER, - egress, transfer, false, NULL, 0, - 0, MLX5_MTR_TABLE_ID_DROP, &error); + egress, transfer, false, NULL, 0, + 0, MLX5_MTR_TABLE_ID_DROP, &error); if (!mtrmng->drop_tbl[domain]) { - DRV_LOG(ERR, "Failed to create " - "meter drop table for default policy."); + DRV_LOG(ERR, "Failed to create meter " + "drop table for default policy."); goto def_policy_error; } } + /* all RED: unique Drop table for jump action. */ tbl_data = container_of(mtrmng->drop_tbl[domain], - struct mlx5_flow_tbl_data_entry, tbl); + struct mlx5_flow_tbl_data_entry, tbl); def_policy->dr_jump_action[RTE_COLOR_RED] = tbl_data->jump.action; acts[RTE_COLOR_RED].dv_actions[0] = tbl_data->jump.action; @@ -16388,15 +16417,14 @@ __flow_dv_create_domain_def_policy(struct rte_eth_dev *dev, uint32_t domain) &def_policy->sub_policy, egress, transfer, false, acts); if (ret) { - DRV_LOG(ERR, "Failed to create " - "default policy rules."); - goto def_policy_error; + DRV_LOG(ERR, "Failed to create default policy rules."); + goto def_policy_error; } } return 0; def_policy_error: __flow_dv_destroy_domain_def_policy(dev, - (enum mlx5_meter_domain)domain); + (enum mlx5_meter_domain)domain); return -1; } @@ -16419,8 +16447,9 @@ flow_dv_create_def_policy(struct rte_eth_dev *dev) if (!priv->config.dv_esw_en && i == MLX5_MTR_DOMAIN_TRANSFER) continue; if (__flow_dv_create_domain_def_policy(dev, i)) { - DRV_LOG(ERR, - "Failed to create default policy"); + DRV_LOG(ERR, "Failed to create default policy"); + /* Rollback the created default policies for others. */ + flow_dv_destroy_def_policy(dev); return -1; } } @@ -16934,8 +16963,9 @@ flow_dv_meter_hierarchy_rule_create(struct rte_eth_dev *dev, goto err_exit; } if (__flow_dv_create_policy_matcher(dev, color_reg_c_idx, - i, sub_policy, &attr, true, item, - &color_rule->matcher, error)) { + MLX5_MTR_POLICY_MATCHER_PRIO, sub_policy, + &attr, true, item, + &color_rule->matcher, error)) { rte_flow_error_set(error, errno, RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL, "Failed to create hierarchy meter matcher."); -- 2.27.0 ^ permalink raw reply [flat|nested] 24+ messages in thread
* [dpdk-dev] [PATCH v2 2/7] net/mlx5: enable meter bucket overflow for yellow color 2021-07-18 17:18 ` [dpdk-dev] [PATCH v2 0/7] support yellow color policy in mlx5 Bing Zhao 2021-07-18 17:18 ` [dpdk-dev] [PATCH v2 1/7] net/mlx5: handle yellow case in default meter policy Bing Zhao @ 2021-07-18 17:18 ` Bing Zhao 2021-07-18 17:18 ` [dpdk-dev] [PATCH v2 3/7] net/mlx5: added support for yellow policy rules Bing Zhao ` (4 subsequent siblings) 6 siblings, 0 replies; 24+ messages in thread From: Bing Zhao @ 2021-07-18 17:18 UTC (permalink / raw) To: viacheslavo, matan; +Cc: dev, orika, rasland, thomas, lizh, shunh To support the meter policy for yellow action, the prerequisite is that the hardware needs to support the EBS, as defined in the RFC2697. https://datatracker.ietf.org/doc/html/rfc2697 Then some of the packets can be marked as yellow if the tokens of C bucket is not enough but enough in E bucket. The color could be used for the further steering of the packets. In the current implementation EBS and overflow were ignored when creating a meter profile. With this commit, if EBS is set by the application, the generation of yellow color will be enabled in the hardware for flow rules steering of packets. Signed-off-by: Bing Zhao <bingz@nvidia.com> --- drivers/net/mlx5/mlx5_flow_aso.c | 4 ++++ drivers/net/mlx5/mlx5_flow_meter.c | 10 +++++----- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/drivers/net/mlx5/mlx5_flow_aso.c b/drivers/net/mlx5/mlx5_flow_aso.c index 64631ffc29..23e22e560a 100644 --- a/drivers/net/mlx5/mlx5_flow_aso.c +++ b/drivers/net/mlx5/mlx5_flow_aso.c @@ -747,6 +747,10 @@ mlx5_aso_mtr_sq_enqueue_single(struct mlx5_aso_sq *sq, wqe->aso_dseg.mtrs[dseg_idx].v_bo_sc_bbog_mm = RTE_BE32((1 << ASO_DSEG_VALID_OFFSET) | (MLX5_FLOW_COLOR_GREEN << ASO_DSEG_SC_OFFSET)); + /* Only needed for RFC2697. */ + if (fm->profile->srtcm_prm.ebs_eir) + wqe->aso_dseg.mtrs[dseg_idx].v_bo_sc_bbog_mm |= + RTE_BE32(1 << ASO_DSEG_BO_OFFSET); sq->head++; sq->pi += 2;/* Each WQE contains 2 WQEBB's. */ rte_io_wmb(); diff --git a/drivers/net/mlx5/mlx5_flow_meter.c b/drivers/net/mlx5/mlx5_flow_meter.c index 78eb2a60f9..73eba0dabd 100644 --- a/drivers/net/mlx5/mlx5_flow_meter.c +++ b/drivers/net/mlx5/mlx5_flow_meter.c @@ -319,9 +319,9 @@ mlx5_flow_meter_param_fill(struct mlx5_flow_meter_profile *fmp, cbs_man = man; cbs_exp = exp; srtcm->cbs_cir = rte_cpu_to_be_32(cbs_exp << ASO_DSEG_CBS_EXP_OFFSET | - cbs_man << ASO_DSEG_CBS_MAN_OFFSET | - cir_exp << ASO_DSEG_CIR_EXP_OFFSET | - cir_man); + cbs_man << ASO_DSEG_CBS_MAN_OFFSET | + cir_exp << ASO_DSEG_CIR_EXP_OFFSET | + cir_man); mlx5_flow_meter_xbs_man_exp_calc(ebs, &man, &exp); /* Check if ebs mantissa is too large. */ if (exp > ASO_DSEG_EXP_MASK) @@ -332,7 +332,7 @@ mlx5_flow_meter_param_fill(struct mlx5_flow_meter_profile *fmp, ebs_man = man; ebs_exp = exp; srtcm->ebs_eir = rte_cpu_to_be_32(ebs_exp << ASO_DSEG_EBS_EXP_OFFSET | - ebs_man << ASO_DSEG_EBS_MAN_OFFSET); + ebs_man << ASO_DSEG_EBS_MAN_OFFSET); return 0; } @@ -421,7 +421,7 @@ mlx5_flow_meter_profile_add(struct rte_eth_dev *dev, return ret; /* Meter profile memory allocation. */ fmp = mlx5_malloc(MLX5_MEM_ZERO, sizeof(struct mlx5_flow_meter_profile), - RTE_CACHE_LINE_SIZE, SOCKET_ID_ANY); + RTE_CACHE_LINE_SIZE, SOCKET_ID_ANY); if (fmp == NULL) return -rte_mtr_error_set(error, ENOMEM, RTE_MTR_ERROR_TYPE_UNSPECIFIED, -- 2.27.0 ^ permalink raw reply [flat|nested] 24+ messages in thread
* [dpdk-dev] [PATCH v2 3/7] net/mlx5: added support for yellow policy rules 2021-07-18 17:18 ` [dpdk-dev] [PATCH v2 0/7] support yellow color policy in mlx5 Bing Zhao 2021-07-18 17:18 ` [dpdk-dev] [PATCH v2 1/7] net/mlx5: handle yellow case in default meter policy Bing Zhao 2021-07-18 17:18 ` [dpdk-dev] [PATCH v2 2/7] net/mlx5: enable meter bucket overflow for yellow color Bing Zhao @ 2021-07-18 17:18 ` Bing Zhao 2021-07-18 17:18 ` [dpdk-dev] [PATCH v2 4/7] net/mlx5: split policies handling of colors Bing Zhao ` (3 subsequent siblings) 6 siblings, 0 replies; 24+ messages in thread From: Bing Zhao @ 2021-07-18 17:18 UTC (permalink / raw) To: viacheslavo, matan; +Cc: dev, orika, rasland, thomas, lizh, shunh 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 <bingz@nvidia.com> --- drivers/net/mlx5/mlx5.h | 4 +++ drivers/net/mlx5/mlx5_flow_dv.c | 50 +++++++++++++++------------ drivers/net/mlx5/mlx5_flow_meter.c | 54 ++++++++++++++++++++---------- 3 files changed, 68 insertions(+), 40 deletions(-) diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h index 791696caf4..c3e1298ebd 100644 --- a/drivers/net/mlx5/mlx5.h +++ b/drivers/net/mlx5/mlx5.h @@ -744,6 +744,10 @@ struct mlx5_flow_meter_policy { /* Is queue action in policy table. */ uint32_t is_hierarchy:1; /* Is meter action in policy table. */ + uint32_t skip_y:1; + /* If yellow color policy is skipped. */ + uint32_t skip_g:1; + /* If green color policy is skipped. */ rte_spinlock_t sl; uint32_t ref_cnt; /* Use count. */ diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c index cfc646c5e5..ffe97d453a 100644 --- a/drivers/net/mlx5/mlx5_flow_dv.c +++ b/drivers/net/mlx5/mlx5_flow_dv.c @@ -15214,7 +15214,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; @@ -15234,21 +15234,24 @@ __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]; + /* Skip the color policy actions creation. */ + if ((i == RTE_COLOR_YELLOW && mtr_policy->skip_y) || + (i == RTE_COLOR_GREEN && mtr_policy->skip_g)) + continue; + 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: { @@ -15456,7 +15459,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, @@ -15500,6 +15503,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; @@ -15615,6 +15622,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; } @@ -16156,16 +16164,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 }; @@ -16199,13 +16206,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) { @@ -16220,7 +16226,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; @@ -16234,15 +16240,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: @@ -16284,7 +16290,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; @@ -16321,8 +16327,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..a6bcb8dbb5 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,20 +729,30 @@ 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, "Memory alloc failed for meter policy."); + if (policy_mode == MLX5_MTR_POLICY_MODE_OG) + mtr_policy->skip_y = 1; + if (policy_mode == MLX5_MTR_POLICY_MODE_OY) + mtr_policy->skip_g = 1; policy_size = sizeof(struct mlx5_flow_meter_policy); for (i = 0; i < MLX5_MTR_DOMAIN_MAX; i++) { if (!(domain_bitmap & (1 << i))) @@ -756,10 +765,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 +776,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 +788,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 +814,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.27.0 ^ permalink raw reply [flat|nested] 24+ messages in thread
* [dpdk-dev] [PATCH v2 4/7] net/mlx5: split policies handling of colors 2021-07-18 17:18 ` [dpdk-dev] [PATCH v2 0/7] support yellow color policy in mlx5 Bing Zhao ` (2 preceding siblings ...) 2021-07-18 17:18 ` [dpdk-dev] [PATCH v2 3/7] net/mlx5: added support for yellow policy rules Bing Zhao @ 2021-07-18 17:18 ` Bing Zhao 2021-07-18 17:18 ` [dpdk-dev] [PATCH v2 5/7] net/mlx5: support yellow in meter policy validation Bing Zhao ` (2 subsequent siblings) 6 siblings, 0 replies; 24+ messages in thread From: Bing Zhao @ 2021-07-18 17:18 UTC (permalink / raw) To: viacheslavo, matan; +Cc: dev, orika, rasland, thomas, lizh, shunh If the fate action is either RSS or Queue of a meter policy, the action will only be created in the flow splitting stage. With queue as the fate action, only one sub-policy is needed. And RSS will have more than one sub-policies if there is an expansion. Since the RSS parameters are the same for both green and yellow colors except the queues, the expansion result will be unique. Even if only one color has the RSS action, the checking and possible expansion will be done then. For each sub-policy, the action rules need to be created separately on its own policy table. Signed-off-by: Bing Zhao <bingz@nvidia.com> --- drivers/net/mlx5/mlx5_flow.c | 40 ++++++++++---------- drivers/net/mlx5/mlx5_flow_dv.c | 67 +++++++++++++++++---------------- 2 files changed, 55 insertions(+), 52 deletions(-) diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c index 347e8c1a09..d90c8cd314 100644 --- a/drivers/net/mlx5/mlx5_flow.c +++ b/drivers/net/mlx5/mlx5_flow.c @@ -4687,7 +4687,7 @@ get_meter_sub_policy(struct rte_eth_dev *dev, struct mlx5_flow_rss_desc *rss_desc[MLX5_MTR_RTE_COLORS] = {0}; uint32_t i; - /** + /* * This is a tmp dev_flow, * no need to register any matcher for it in translate. */ @@ -4695,18 +4695,19 @@ get_meter_sub_policy(struct rte_eth_dev *dev, for (i = 0; i < MLX5_MTR_RTE_COLORS; i++) { struct mlx5_flow dev_flow = {0}; struct mlx5_flow_handle dev_handle = { {0} }; + uint8_t fate = final_policy->act_cnt[i].fate_action; - if (final_policy->is_rss) { + if (fate == MLX5_FLOW_FATE_SHARED_RSS) { const void *rss_act = final_policy->act_cnt[i].rss->conf; struct rte_flow_action rss_actions[2] = { [0] = { .type = RTE_FLOW_ACTION_TYPE_RSS, - .conf = rss_act + .conf = rss_act, }, [1] = { .type = RTE_FLOW_ACTION_TYPE_END, - .conf = NULL + .conf = NULL, } }; @@ -4731,9 +4732,10 @@ get_meter_sub_policy(struct rte_eth_dev *dev, rss_desc_v[i].hash_fields ? rss_desc_v[i].queue_num : 1; rss_desc_v[i].tunnel = - !!(dev_flow.handle->layers & - MLX5_FLOW_LAYER_TUNNEL); - } else { + !!(dev_flow.handle->layers & + MLX5_FLOW_LAYER_TUNNEL); + rss_desc[i] = &rss_desc_v[i]; + } else if (fate == MLX5_FLOW_FATE_QUEUE) { /* This is queue action. */ rss_desc_v[i] = wks->rss_desc; rss_desc_v[i].key_len = 0; @@ -4741,24 +4743,24 @@ get_meter_sub_policy(struct rte_eth_dev *dev, rss_desc_v[i].queue = &final_policy->act_cnt[i].queue; rss_desc_v[i].queue_num = 1; + rss_desc[i] = &rss_desc_v[i]; + } else { + rss_desc[i] = NULL; } - rss_desc[i] = &rss_desc_v[i]; } sub_policy = flow_drv_meter_sub_policy_rss_prepare(dev, flow, policy, rss_desc); } else { enum mlx5_meter_domain mtr_domain = attr->transfer ? MLX5_MTR_DOMAIN_TRANSFER : - attr->egress ? MLX5_MTR_DOMAIN_EGRESS : - MLX5_MTR_DOMAIN_INGRESS; + (attr->egress ? MLX5_MTR_DOMAIN_EGRESS : + MLX5_MTR_DOMAIN_INGRESS); sub_policy = policy->sub_policys[mtr_domain][0]; } - if (!sub_policy) { + if (!sub_policy) rte_flow_error_set(error, EINVAL, - RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL, - "Failed to get meter sub-policy."); - goto exit; - } + RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL, + "Failed to get meter sub-policy."); exit: return sub_policy; } @@ -4956,8 +4958,8 @@ flow_meter_split_prep(struct rte_eth_dev *dev, } else { enum mlx5_meter_domain mtr_domain = attr->transfer ? MLX5_MTR_DOMAIN_TRANSFER : - attr->egress ? MLX5_MTR_DOMAIN_EGRESS : - MLX5_MTR_DOMAIN_INGRESS; + (attr->egress ? MLX5_MTR_DOMAIN_EGRESS : + MLX5_MTR_DOMAIN_INGRESS); sub_policy = &priv->sh->mtrmng->def_policy[mtr_domain]->sub_policy; @@ -4973,8 +4975,8 @@ flow_meter_split_prep(struct rte_eth_dev *dev, actions_pre++; if (!tag_action) return rte_flow_error_set(error, ENOMEM, - RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL, - "No tag action space."); + RTE_FLOW_ERROR_TYPE_UNSPECIFIED, + NULL, "No tag action space."); if (!mtr_flow_id) { tag_action->type = RTE_FLOW_ACTION_TYPE_VOID; goto exit; diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c index ffe97d453a..c617e8801a 100644 --- a/drivers/net/mlx5/mlx5_flow_dv.c +++ b/drivers/net/mlx5/mlx5_flow_dv.c @@ -15070,11 +15070,11 @@ __flow_dv_destroy_sub_policy_rules(struct rte_eth_dev *dev, next_port, tmp) { claim_zero(mlx5_flow_os_destroy_flow(color_rule->rule)); tbl = container_of(color_rule->matcher->tbl, - typeof(*tbl), tbl); + typeof(*tbl), tbl); mlx5_list_unregister(tbl->matchers, - &color_rule->matcher->entry); + &color_rule->matcher->entry); TAILQ_REMOVE(&sub_policy->color_rules[i], - color_rule, next_port); + color_rule, next_port); mlx5_free(color_rule); if (next_fm) mlx5_flow_meter_detach(priv, next_fm); @@ -15088,13 +15088,13 @@ __flow_dv_destroy_sub_policy_rules(struct rte_eth_dev *dev, } if (sub_policy->jump_tbl[i]) { flow_dv_tbl_resource_release(MLX5_SH(dev), - sub_policy->jump_tbl[i]); + sub_policy->jump_tbl[i]); sub_policy->jump_tbl[i] = NULL; } } if (sub_policy->tbl_rsc) { flow_dv_tbl_resource_release(MLX5_SH(dev), - sub_policy->tbl_rsc); + sub_policy->tbl_rsc); sub_policy->tbl_rsc = NULL; } } @@ -15111,7 +15111,7 @@ __flow_dv_destroy_sub_policy_rules(struct rte_eth_dev *dev, */ static void flow_dv_destroy_policy_rules(struct rte_eth_dev *dev, - struct mlx5_flow_meter_policy *mtr_policy) + struct mlx5_flow_meter_policy *mtr_policy) { uint32_t i, j; struct mlx5_flow_meter_sub_policy *sub_policy; @@ -15124,8 +15124,8 @@ flow_dv_destroy_policy_rules(struct rte_eth_dev *dev, for (j = 0; j < sub_policy_num; j++) { sub_policy = mtr_policy->sub_policys[i][j]; if (sub_policy) - __flow_dv_destroy_sub_policy_rules - (dev, sub_policy); + __flow_dv_destroy_sub_policy_rules(dev, + sub_policy); } } } @@ -16162,6 +16162,7 @@ __flow_dv_create_policy_acts_rules(struct rte_eth_dev *dev, bool match_src_port = false; int i; + /* If RSS or Queue, no previous actions / rules is created. */ for (i = 0; i < RTE_COLORS; i++) { acts[i].actions_n = 0; if (i == RTE_COLOR_RED) { @@ -16661,37 +16662,36 @@ __flow_dv_meter_get_rss_sub_policy(struct rte_eth_dev *dev, sub_policy_num = (mtr_policy->sub_policy_num >> (MLX5_MTR_SUB_POLICY_NUM_SHIFT * domain)) & MLX5_MTR_SUB_POLICY_NUM_MASK; - for (i = 0; i < sub_policy_num; - i++) { - for (j = 0; j < MLX5_MTR_RTE_COLORS; j++) { - if (rss_desc[j] && - hrxq_idx[j] != - mtr_policy->sub_policys[domain][i]->rix_hrxq[j]) + for (j = 0; j < sub_policy_num; j++) { + for (i = 0; i < MLX5_MTR_RTE_COLORS; i++) { + if (rss_desc[i] && + hrxq_idx[i] != + mtr_policy->sub_policys[domain][j]->rix_hrxq[i]) break; } - if (j >= MLX5_MTR_RTE_COLORS) { + if (i >= MLX5_MTR_RTE_COLORS) { /* * Found the sub policy table with - * the same queue per color + * the same queue per color. */ rte_spinlock_unlock(&mtr_policy->sl); - for (j = 0; j < MLX5_MTR_RTE_COLORS; j++) - mlx5_hrxq_release(dev, hrxq_idx[j]); + for (i = 0; i < MLX5_MTR_RTE_COLORS; i++) + mlx5_hrxq_release(dev, hrxq_idx[i]); *is_reuse = true; - return mtr_policy->sub_policys[domain][i]; + return mtr_policy->sub_policys[domain][j]; } } /* Create sub policy. */ if (!mtr_policy->sub_policys[domain][0]->rix_hrxq[0]) { - /* Reuse the first dummy sub_policy*/ + /* Reuse the first pre-allocated sub_policy. */ sub_policy = mtr_policy->sub_policys[domain][0]; sub_policy_idx = sub_policy->idx; } else { sub_policy = mlx5_ipool_zmalloc (priv->sh->ipool[MLX5_IPOOL_MTR_POLICY], - &sub_policy_idx); + &sub_policy_idx); if (!sub_policy || - sub_policy_idx > MLX5_MAX_SUB_POLICY_TBL_NUM) { + sub_policy_idx > MLX5_MAX_SUB_POLICY_TBL_NUM) { for (i = 0; i < MLX5_MTR_RTE_COLORS; i++) mlx5_hrxq_release(dev, hrxq_idx[i]); goto rss_sub_policy_error; @@ -16713,9 +16713,9 @@ __flow_dv_meter_get_rss_sub_policy(struct rte_eth_dev *dev, * RSS action to Queue action. */ hrxq = mlx5_ipool_get(priv->sh->ipool[MLX5_IPOOL_HRXQ], - hrxq_idx[i]); + hrxq_idx[i]); if (!hrxq) { - DRV_LOG(ERR, "Failed to create policy hrxq"); + DRV_LOG(ERR, "Failed to get policy hrxq"); goto rss_sub_policy_error; } act_cnt = &mtr_policy->act_cnt[i]; @@ -16730,19 +16730,21 @@ __flow_dv_meter_get_rss_sub_policy(struct rte_eth_dev *dev, } } if (__flow_dv_create_policy_acts_rules(dev, mtr_policy, - sub_policy, domain)) { + sub_policy, domain)) { DRV_LOG(ERR, "Failed to create policy " - "rules per domain."); + "rules for ingress domain."); goto rss_sub_policy_error; } if (sub_policy != mtr_policy->sub_policys[domain][0]) { i = (mtr_policy->sub_policy_num >> (MLX5_MTR_SUB_POLICY_NUM_SHIFT * domain)) & MLX5_MTR_SUB_POLICY_NUM_MASK; + if (i >= MLX5_MTR_RSS_MAX_SUB_POLICY) { + DRV_LOG(ERR, "No free sub-policy slot."); + goto rss_sub_policy_error; + } mtr_policy->sub_policys[domain][i] = sub_policy; i++; - if (i > MLX5_MTR_RSS_MAX_SUB_POLICY) - goto rss_sub_policy_error; mtr_policy->sub_policy_num &= ~(MLX5_MTR_SUB_POLICY_NUM_MASK << (MLX5_MTR_SUB_POLICY_NUM_SHIFT * domain)); mtr_policy->sub_policy_num |= @@ -16760,8 +16762,7 @@ __flow_dv_meter_get_rss_sub_policy(struct rte_eth_dev *dev, (MLX5_MTR_SUB_POLICY_NUM_SHIFT * domain)) & MLX5_MTR_SUB_POLICY_NUM_MASK; mtr_policy->sub_policys[domain][i] = NULL; - mlx5_ipool_free - (priv->sh->ipool[MLX5_IPOOL_MTR_POLICY], + mlx5_ipool_free(priv->sh->ipool[MLX5_IPOOL_MTR_POLICY], sub_policy->idx); } } @@ -16822,7 +16823,7 @@ flow_dv_meter_sub_policy_rss_prepare(struct rte_eth_dev *dev, while (i) { /** * From last policy to the first one in hierarchy, - * create/get the sub policy for each of them. + * create / get the sub policy for each of them. */ sub_policy = __flow_dv_meter_get_rss_sub_policy(dev, policies[--i], @@ -17026,7 +17027,7 @@ flow_dv_meter_hierarchy_rule_create(struct rte_eth_dev *dev, */ static void flow_dv_destroy_sub_policy_with_rxq(struct rte_eth_dev *dev, - struct mlx5_flow_meter_policy *mtr_policy) + struct mlx5_flow_meter_policy *mtr_policy) { struct mlx5_priv *priv = dev->data->dev_private; struct mlx5_flow_meter_sub_policy *sub_policy = NULL; @@ -17072,7 +17073,7 @@ flow_dv_destroy_sub_policy_with_rxq(struct rte_eth_dev *dev, case MLX5_FLOW_FATE_QUEUE: sub_policy = mtr_policy->sub_policys[domain][0]; __flow_dv_destroy_sub_policy_rules(dev, - sub_policy); + sub_policy); break; default: /*Other actions without queue and do nothing*/ -- 2.27.0 ^ permalink raw reply [flat|nested] 24+ messages in thread
* [dpdk-dev] [PATCH v2 5/7] net/mlx5: support yellow in meter policy validation 2021-07-18 17:18 ` [dpdk-dev] [PATCH v2 0/7] support yellow color policy in mlx5 Bing Zhao ` (3 preceding siblings ...) 2021-07-18 17:18 ` [dpdk-dev] [PATCH v2 4/7] net/mlx5: split policies handling of colors Bing Zhao @ 2021-07-18 17:18 ` Bing Zhao 2021-07-18 17:18 ` [dpdk-dev] [PATCH v2 6/7] net/mlx5: check consistency of meter policy and profile Bing Zhao 2021-07-18 17:18 ` [dpdk-dev] [PATCH v2 7/7] net/mlx5: add meter support for trTCM profiles Bing Zhao 6 siblings, 0 replies; 24+ messages in thread From: Bing Zhao @ 2021-07-18 17:18 UTC (permalink / raw) To: viacheslavo, matan; +Cc: dev, orika, rasland, thomas, lizh, shunh In the previous implementation, the policy for yellow color was not supported. The action validation for yellow was skipped. Since the yellow color policy needs to be supported, the validation should also be done for the yellow color. In the meanwhile, due to the fact that color policies of one meter should be used for the same flow(s), the domains supported of both colors should be the same. If both of the colors have RSS as the termination actions, except the queues, all other parameters of RSS should be the same. Signed-off-by: Bing Zhao <bingz@nvidia.com> --- doc/guides/nics/mlx5.rst | 6 +- doc/guides/rel_notes/release_21_08.rst | 1 + drivers/net/mlx5/mlx5.h | 8 +- drivers/net/mlx5/mlx5_flow.c | 6 +- drivers/net/mlx5/mlx5_flow.h | 4 +- drivers/net/mlx5/mlx5_flow_dv.c | 209 ++++++++++++++++--------- drivers/net/mlx5/mlx5_flow_meter.c | 15 +- 7 files changed, 156 insertions(+), 93 deletions(-) diff --git a/doc/guides/nics/mlx5.rst b/doc/guides/nics/mlx5.rst index f5b727c1ee..9396074b5a 100644 --- a/doc/guides/nics/mlx5.rst +++ b/doc/guides/nics/mlx5.rst @@ -421,18 +421,18 @@ Limitations - All the meter colors with drop action will be counted only by the global drop statistics. - Green color is not supported with drop action. - - Yellow detection is not supported. + - Yellow detection is only supported with ASO metering. - Red color must be with drop action. - Meter statistics are supported only for drop case. - - Meter yellow color detection is not supported. - A meter action created with pre-defined policy must be the last action in the flow except single case where the policy actions are: - green: NULL or END. - yellow: NULL or END. - RED: DROP / END. - The only supported meter policy actions: - green: QUEUE, RSS, PORT_ID, JUMP, MARK and SET_TAG. - - yellow: must be empty. + - yellow: QUEUE, RSS, PORT_ID, JUMP, MARK and SET_TAG. - RED: must be DROP. + - Policy actions of RSS for green and yellow should have the same configuration except queues. - meter profile packet mode is supported. - Integrity: diff --git a/doc/guides/rel_notes/release_21_08.rst b/doc/guides/rel_notes/release_21_08.rst index 1b38b1aa51..03d4fd059a 100644 --- a/doc/guides/rel_notes/release_21_08.rst +++ b/doc/guides/rel_notes/release_21_08.rst @@ -91,6 +91,7 @@ New Features * Added matching on IPv4 Internet Header Length (IHL). * Added support for matching on VXLAN header last 8-bits reserved field. * Optimized multi-thread flow rule insertion rate. + * Added support for metering policy actions of yellow color. * **Added Wangxun ngbe PMD.** diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h index c3e1298ebd..9832ed4189 100644 --- a/drivers/net/mlx5/mlx5.h +++ b/drivers/net/mlx5/mlx5.h @@ -629,11 +629,11 @@ struct mlx5_dev_shared_port { */ #define MLX5_DV_MAX_NUMBER_OF_ACTIONS 8 -/*ASO flow meter structures*/ +/* ASO flow meter structures */ /* Modify this value if enum rte_mtr_color changes. */ #define RTE_MTR_DROPPED RTE_COLORS -/* Yellow is not supported. */ -#define MLX5_MTR_RTE_COLORS (RTE_COLOR_GREEN + 1) +/* Yellow is now supported. */ +#define MLX5_MTR_RTE_COLORS (RTE_COLOR_YELLOW + 1) /* table_id 22 bits in mlx5_flow_tbl_key so limit policy number. */ #define MLX5_MAX_SUB_POLICY_TBL_NUM 0x3FFFFF #define MLX5_INVALID_POLICY_ID UINT32_MAX @@ -647,6 +647,8 @@ struct mlx5_dev_shared_port { #define MLX5_MTR_POLICY_MODE_DEF 1 /* Only green color valid. */ #define MLX5_MTR_POLICY_MODE_OG 2 +/* Only yellow color valid. */ +#define MLX5_MTR_POLICY_MODE_OY 3 enum mlx5_meter_domain { MLX5_MTR_DOMAIN_INGRESS, diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c index d90c8cd314..549b3058c2 100644 --- a/drivers/net/mlx5/mlx5_flow.c +++ b/drivers/net/mlx5/mlx5_flow.c @@ -7199,14 +7199,14 @@ mlx5_flow_validate_mtr_acts(struct rte_eth_dev *dev, struct rte_flow_attr *attr, bool *is_rss, uint8_t *domain_bitmap, - bool *is_def_policy, + uint8_t *policy_mode, struct rte_mtr_error *error) { const struct mlx5_flow_driver_ops *fops; fops = flow_get_drv_ops(MLX5_FLOW_TYPE_DV); - return fops->validate_mtr_acts(dev, actions, attr, - is_rss, domain_bitmap, is_def_policy, error); + return fops->validate_mtr_acts(dev, actions, attr, is_rss, + domain_bitmap, policy_mode, error); } /** diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h index 8f0521aa72..3724293d26 100644 --- a/drivers/net/mlx5/mlx5_flow.h +++ b/drivers/net/mlx5/mlx5_flow.h @@ -1209,7 +1209,7 @@ typedef int (*mlx5_flow_validate_mtr_acts_t) struct rte_flow_attr *attr, bool *is_rss, uint8_t *domain_bitmap, - bool *is_def_policy, + uint8_t *policy_mode, struct rte_mtr_error *error); typedef int (*mlx5_flow_create_mtr_acts_t) (struct rte_eth_dev *dev, @@ -1690,7 +1690,7 @@ int mlx5_flow_validate_mtr_acts(struct rte_eth_dev *dev, struct rte_flow_attr *attr, bool *is_rss, uint8_t *domain_bitmap, - bool *is_def_policy, + uint8_t *policy_mode, struct rte_mtr_error *error); void mlx5_flow_destroy_mtr_acts(struct rte_eth_dev *dev, struct mlx5_flow_meter_policy *mtr_policy); diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c index c617e8801a..1d673596a5 100644 --- a/drivers/net/mlx5/mlx5_flow_dv.c +++ b/drivers/net/mlx5/mlx5_flow_dv.c @@ -17362,6 +17362,31 @@ flow_dv_action_validate(struct rte_eth_dev *dev, } } +/* + * Check if the RSS configurations for colors of a meter policy match + * each other, except the queues. + * + * @param[in] r1 + * Pointer to the first RSS flow action. + * @param[in] r2 + * Pointer to the second RSS flow action. + * + * @return + * 0 on match, 1 on conflict. + */ +static inline int +flow_dv_mtr_policy_rss_compare(const struct rte_flow_action_rss *r1, + const struct rte_flow_action_rss *r2) +{ + if (!r1 || !r2) + return 0; + if (r1->func != r2->func || r1->level != r2->level || + r1->types != r2->types || r1->key_len != r2->key_len || + memcmp(r1->key, r2->key, r1->key_len)) + return 1; + return 0; +} + /** * Validate the meter hierarchy chain for meter policy. * @@ -17457,13 +17482,13 @@ flow_dv_validate_mtr_policy_acts(struct rte_eth_dev *dev, struct rte_flow_attr *attr, bool *is_rss, uint8_t *domain_bitmap, - bool *is_def_policy, + uint8_t *policy_mode, struct rte_mtr_error *error) { struct mlx5_priv *priv = dev->data->dev_private; struct mlx5_dev_config *dev_conf = &priv->config; const struct rte_flow_action *act; - uint64_t action_flags = 0; + uint64_t action_flags[RTE_COLORS] = {0}; int actions_n; int i, ret; struct rte_flow_error flow_err; @@ -17471,36 +17496,44 @@ flow_dv_validate_mtr_policy_acts(struct rte_eth_dev *dev, uint8_t def_domain = MLX5_MTR_ALL_DOMAIN_BIT; uint8_t hierarchy_domain = 0; const struct rte_flow_action_meter *mtr; + bool def_green = false; + bool def_yellow = false; + const struct rte_flow_action_rss *rss_color[RTE_COLORS] = {NULL}; if (!priv->config.dv_esw_en) def_domain &= ~MLX5_MTR_DOMAIN_TRANSFER_BIT; *domain_bitmap = def_domain; - if (actions[RTE_COLOR_YELLOW] && - actions[RTE_COLOR_YELLOW]->type != RTE_FLOW_ACTION_TYPE_END) - return -rte_mtr_error_set(error, ENOTSUP, - RTE_MTR_ERROR_TYPE_METER_POLICY, - NULL, - "Yellow color does not support any action."); - if (actions[RTE_COLOR_YELLOW] && - actions[RTE_COLOR_YELLOW]->type != RTE_FLOW_ACTION_TYPE_DROP) + if (actions[RTE_COLOR_RED] && + actions[RTE_COLOR_RED]->type != RTE_FLOW_ACTION_TYPE_DROP) return -rte_mtr_error_set(error, ENOTSUP, RTE_MTR_ERROR_TYPE_METER_POLICY, NULL, "Red color only supports drop action."); /* * Check default policy actions: - * Green/Yellow: no action, Red: drop action + * Green / Yellow: no action, Red: drop action + * Either G or Y will trigger default policy actions to be created. */ - if ((!actions[RTE_COLOR_GREEN] || - actions[RTE_COLOR_GREEN]->type == RTE_FLOW_ACTION_TYPE_END)) { - *is_def_policy = true; + if (!actions[RTE_COLOR_GREEN] || + actions[RTE_COLOR_GREEN]->type == RTE_FLOW_ACTION_TYPE_END) + def_green = true; + if (!actions[RTE_COLOR_YELLOW] || + actions[RTE_COLOR_YELLOW]->type == RTE_FLOW_ACTION_TYPE_END) + def_yellow = true; + if (def_green && def_yellow) { + *policy_mode = MLX5_MTR_POLICY_MODE_DEF; return 0; + } else if (!def_green && def_yellow) { + *policy_mode = MLX5_MTR_POLICY_MODE_OG; + } else if (def_green && !def_yellow) { + *policy_mode = MLX5_MTR_POLICY_MODE_OY; } - flow_err.message = NULL; + /* Set to empty string in case of NULL pointer access by user. */ + flow_err.message = ""; for (i = 0; i < RTE_COLORS; i++) { act = actions[i]; - for (action_flags = 0, actions_n = 0; - act && act->type != RTE_FLOW_ACTION_TYPE_END; - act++) { + for (action_flags[i] = 0, actions_n = 0; + act && act->type != RTE_FLOW_ACTION_TYPE_END; + act++) { if (actions_n == MLX5_DV_MAX_NUMBER_OF_ACTIONS) return -rte_mtr_error_set(error, ENOTSUP, RTE_MTR_ERROR_TYPE_METER_POLICY, @@ -17514,7 +17547,7 @@ flow_dv_validate_mtr_policy_acts(struct rte_eth_dev *dev, NULL, "PORT action validate check" " fail for ESW disable"); ret = flow_dv_validate_action_port_id(dev, - action_flags, + action_flags[i], act, attr, &flow_err); if (ret) return -rte_mtr_error_set(error, @@ -17524,11 +17557,11 @@ flow_dv_validate_mtr_policy_acts(struct rte_eth_dev *dev, flow_err.message : "PORT action validate check fail"); ++actions_n; - action_flags |= MLX5_FLOW_ACTION_PORT_ID; + action_flags[i] |= MLX5_FLOW_ACTION_PORT_ID; break; case RTE_FLOW_ACTION_TYPE_MARK: ret = flow_dv_validate_action_mark(dev, act, - action_flags, + action_flags[i], attr, &flow_err); if (ret < 0) return -rte_mtr_error_set(error, @@ -17545,12 +17578,12 @@ flow_dv_validate_mtr_policy_acts(struct rte_eth_dev *dev, NULL, "Extend MARK action is " "not supported. Please try use " "default policy for meter."); - action_flags |= MLX5_FLOW_ACTION_MARK; + action_flags[i] |= MLX5_FLOW_ACTION_MARK; ++actions_n; break; case RTE_FLOW_ACTION_TYPE_SET_TAG: ret = flow_dv_validate_action_set_tag(dev, - act, action_flags, + act, action_flags[i], attr, &flow_err); if (ret) return -rte_mtr_error_set(error, @@ -17559,19 +17592,12 @@ flow_dv_validate_mtr_policy_acts(struct rte_eth_dev *dev, NULL, flow_err.message ? flow_err.message : "Set tag action validate check fail"); - /* - * Count all modify-header actions - * as one action. - */ - if (!(action_flags & - MLX5_FLOW_MODIFY_HDR_ACTIONS)) - ++actions_n; - action_flags |= MLX5_FLOW_ACTION_SET_TAG; + action_flags[i] |= MLX5_FLOW_ACTION_SET_TAG; + ++actions_n; break; case RTE_FLOW_ACTION_TYPE_DROP: ret = mlx5_flow_validate_action_drop - (action_flags, - attr, &flow_err); + (action_flags[i], attr, &flow_err); if (ret < 0) return -rte_mtr_error_set(error, ENOTSUP, @@ -17579,7 +17605,7 @@ flow_dv_validate_mtr_policy_acts(struct rte_eth_dev *dev, NULL, flow_err.message ? flow_err.message : "Drop action validate check fail"); - action_flags |= MLX5_FLOW_ACTION_DROP; + action_flags[i] |= MLX5_FLOW_ACTION_DROP; ++actions_n; break; case RTE_FLOW_ACTION_TYPE_QUEUE: @@ -17588,9 +17614,9 @@ flow_dv_validate_mtr_policy_acts(struct rte_eth_dev *dev, * metadata feature is engaged. */ if (dev_conf->dv_flow_en && - (dev_conf->dv_xmeta_en != - MLX5_XMETA_MODE_LEGACY) && - mlx5_flow_ext_mreg_supported(dev)) + (dev_conf->dv_xmeta_en != + MLX5_XMETA_MODE_LEGACY) && + mlx5_flow_ext_mreg_supported(dev)) return -rte_mtr_error_set(error, ENOTSUP, RTE_MTR_ERROR_TYPE_METER_POLICY, @@ -17598,7 +17624,7 @@ flow_dv_validate_mtr_policy_acts(struct rte_eth_dev *dev, "is not supported. Please try use " "default policy for meter."); ret = mlx5_flow_validate_action_queue(act, - action_flags, dev, + action_flags[i], dev, attr, &flow_err); if (ret < 0) return -rte_mtr_error_set(error, @@ -17607,14 +17633,14 @@ flow_dv_validate_mtr_policy_acts(struct rte_eth_dev *dev, NULL, flow_err.message ? flow_err.message : "Queue action validate check fail"); - action_flags |= MLX5_FLOW_ACTION_QUEUE; + action_flags[i] |= MLX5_FLOW_ACTION_QUEUE; ++actions_n; break; case RTE_FLOW_ACTION_TYPE_RSS: if (dev_conf->dv_flow_en && - (dev_conf->dv_xmeta_en != - MLX5_XMETA_MODE_LEGACY) && - mlx5_flow_ext_mreg_supported(dev)) + (dev_conf->dv_xmeta_en != + MLX5_XMETA_MODE_LEGACY) && + mlx5_flow_ext_mreg_supported(dev)) return -rte_mtr_error_set(error, ENOTSUP, RTE_MTR_ERROR_TYPE_METER_POLICY, @@ -17622,7 +17648,7 @@ flow_dv_validate_mtr_policy_acts(struct rte_eth_dev *dev, "is not supported. Please try use " "default policy for meter."); ret = mlx5_validate_action_rss(dev, act, - &flow_err); + &flow_err); if (ret < 0) return -rte_mtr_error_set(error, ENOTSUP, @@ -17630,13 +17656,14 @@ flow_dv_validate_mtr_policy_acts(struct rte_eth_dev *dev, NULL, flow_err.message ? flow_err.message : "RSS action validate check fail"); - action_flags |= MLX5_FLOW_ACTION_RSS; + action_flags[i] |= MLX5_FLOW_ACTION_RSS; ++actions_n; - *is_rss = true; + /* Either G or Y will set the RSS. */ + rss_color[i] = act->conf; break; case RTE_FLOW_ACTION_TYPE_JUMP: ret = flow_dv_validate_action_jump(dev, - NULL, act, action_flags, + NULL, act, action_flags[i], attr, true, &flow_err); if (ret) return -rte_mtr_error_set(error, @@ -17646,8 +17673,13 @@ flow_dv_validate_mtr_policy_acts(struct rte_eth_dev *dev, flow_err.message : "Jump action validate check fail"); ++actions_n; - action_flags |= MLX5_FLOW_ACTION_JUMP; + action_flags[i] |= MLX5_FLOW_ACTION_JUMP; break; + /* + * Only the last meter in the hierarchy will support + * the YELLOW color steering. Then in the meter policy + * actions list, there should be no other meter inside. + */ case RTE_FLOW_ACTION_TYPE_METER: if (i != RTE_COLOR_GREEN) return -rte_mtr_error_set(error, @@ -17659,14 +17691,14 @@ flow_dv_validate_mtr_policy_acts(struct rte_eth_dev *dev, mtr = act->conf; ret = flow_dv_validate_policy_mtr_hierarchy(dev, mtr->mtr_id, - action_flags, + action_flags[i], is_rss, &hierarchy_domain, error); if (ret) return ret; ++actions_n; - action_flags |= + action_flags[i] |= MLX5_FLOW_ACTION_METER_WITH_TERMINATED_POLICY; break; default: @@ -17676,31 +17708,38 @@ flow_dv_validate_mtr_policy_acts(struct rte_eth_dev *dev, "Doesn't support optional action"); } } - /* Yellow is not supported, just skip. */ - if (i == RTE_COLOR_YELLOW) - continue; - if (action_flags & MLX5_FLOW_ACTION_PORT_ID) + if (action_flags[i] & MLX5_FLOW_ACTION_PORT_ID) domain_color[i] = MLX5_MTR_DOMAIN_TRANSFER_BIT; - else if ((action_flags & - (MLX5_FLOW_ACTION_RSS | MLX5_FLOW_ACTION_QUEUE)) || - (action_flags & MLX5_FLOW_ACTION_MARK)) + else if ((action_flags[i] & + (MLX5_FLOW_ACTION_RSS | MLX5_FLOW_ACTION_QUEUE)) || + (action_flags[i] & MLX5_FLOW_ACTION_MARK)) /* * Only support MLX5_XMETA_MODE_LEGACY - * so MARK action only in ingress domain. + * so MARK action is only in ingress domain. */ domain_color[i] = MLX5_MTR_DOMAIN_INGRESS_BIT; - else if (action_flags & - MLX5_FLOW_ACTION_METER_WITH_TERMINATED_POLICY) + else if (action_flags[i] & + MLX5_FLOW_ACTION_METER_WITH_TERMINATED_POLICY) domain_color[i] = hierarchy_domain; else domain_color[i] = def_domain; + /* + * Non-termination actions only support NIC Tx domain. + * The adjustion should be skipped when there is no + * action or only END is provided. The default domains + * bit-mask is set to find the MIN intersection. + * The action flags checking should also be skipped. + */ + if ((def_green && i == RTE_COLOR_GREEN) || + (def_yellow && i == RTE_COLOR_YELLOW)) + continue; /* * Validate the drop action mutual exclusion * with other actions. Drop action is mutually-exclusive * with any other action, except for Count action. */ - if ((action_flags & MLX5_FLOW_ACTION_DROP) && - (action_flags & ~MLX5_FLOW_ACTION_DROP)) { + if ((action_flags[i] & MLX5_FLOW_ACTION_DROP) && + (action_flags[i] & ~MLX5_FLOW_ACTION_DROP)) { return -rte_mtr_error_set(error, ENOTSUP, RTE_MTR_ERROR_TYPE_METER_POLICY, NULL, "Drop action is mutually-exclusive " @@ -17709,40 +17748,60 @@ flow_dv_validate_mtr_policy_acts(struct rte_eth_dev *dev, /* Eswitch has few restrictions on using items and actions */ if (domain_color[i] & MLX5_MTR_DOMAIN_TRANSFER_BIT) { if (!mlx5_flow_ext_mreg_supported(dev) && - action_flags & MLX5_FLOW_ACTION_MARK) + action_flags[i] & MLX5_FLOW_ACTION_MARK) return -rte_mtr_error_set(error, ENOTSUP, RTE_MTR_ERROR_TYPE_METER_POLICY, NULL, "unsupported action MARK"); - if (action_flags & MLX5_FLOW_ACTION_QUEUE) + if (action_flags[i] & MLX5_FLOW_ACTION_QUEUE) return -rte_mtr_error_set(error, ENOTSUP, RTE_MTR_ERROR_TYPE_METER_POLICY, NULL, "unsupported action QUEUE"); - if (action_flags & MLX5_FLOW_ACTION_RSS) + if (action_flags[i] & MLX5_FLOW_ACTION_RSS) return -rte_mtr_error_set(error, ENOTSUP, RTE_MTR_ERROR_TYPE_METER_POLICY, NULL, "unsupported action RSS"); - if (!(action_flags & MLX5_FLOW_FATE_ESWITCH_ACTIONS)) + if (!(action_flags[i] & MLX5_FLOW_FATE_ESWITCH_ACTIONS)) return -rte_mtr_error_set(error, ENOTSUP, RTE_MTR_ERROR_TYPE_METER_POLICY, NULL, "no fate action is found"); } else { - if (!(action_flags & MLX5_FLOW_FATE_ACTIONS) && - (domain_color[i] & - MLX5_MTR_DOMAIN_INGRESS_BIT)) { + if (!(action_flags[i] & MLX5_FLOW_FATE_ACTIONS) && + (domain_color[i] & MLX5_MTR_DOMAIN_INGRESS_BIT)) { if ((domain_color[i] & - MLX5_MTR_DOMAIN_EGRESS_BIT)) + MLX5_MTR_DOMAIN_EGRESS_BIT)) domain_color[i] = - MLX5_MTR_DOMAIN_EGRESS_BIT; + MLX5_MTR_DOMAIN_EGRESS_BIT; else return -rte_mtr_error_set(error, - ENOTSUP, - RTE_MTR_ERROR_TYPE_METER_POLICY, - NULL, "no fate action is found"); + ENOTSUP, + RTE_MTR_ERROR_TYPE_METER_POLICY, + NULL, + "no fate action is found"); } } - if (domain_color[i] != def_domain) - *domain_bitmap = domain_color[i]; } + /* If both colors have RSS, the attributes should be the same. */ + if (flow_dv_mtr_policy_rss_compare(rss_color[RTE_COLOR_GREEN], + rss_color[RTE_COLOR_YELLOW])) + return -rte_mtr_error_set(error, EINVAL, + RTE_MTR_ERROR_TYPE_METER_POLICY, + NULL, "policy RSS attr conflict"); + if (rss_color[RTE_COLOR_GREEN] || rss_color[RTE_COLOR_YELLOW]) + *is_rss = true; + /* "domain_color[C]" is non-zero for each color, default is ALL. */ + if (!def_green && !def_yellow && + domain_color[RTE_COLOR_GREEN] != domain_color[RTE_COLOR_YELLOW] && + !(action_flags[RTE_COLOR_GREEN] & MLX5_FLOW_ACTION_DROP) && + !(action_flags[RTE_COLOR_YELLOW] & MLX5_FLOW_ACTION_DROP)) + return -rte_mtr_error_set(error, EINVAL, + RTE_MTR_ERROR_TYPE_METER_POLICY, + NULL, "policy domains conflict"); + /* + * At least one color policy is listed in the actions, the domains + * to be supported should be the intersection. + */ + *domain_bitmap = domain_color[RTE_COLOR_GREEN] & + domain_color[RTE_COLOR_YELLOW]; return 0; } diff --git a/drivers/net/mlx5/mlx5_flow_meter.c b/drivers/net/mlx5/mlx5_flow_meter.c index a6bcb8dbb5..e9b9b22fb2 100644 --- a/drivers/net/mlx5/mlx5_flow_meter.c +++ b/drivers/net/mlx5/mlx5_flow_meter.c @@ -582,7 +582,7 @@ mlx5_flow_meter_policy_validate(struct rte_eth_dev *dev, struct rte_flow_attr attr = { .transfer = priv->config.dv_esw_en ? 1 : 0}; bool is_rss = false; - bool is_def_policy = false; + uint8_t policy_mode; uint8_t domain_bitmap; int ret; @@ -591,7 +591,7 @@ mlx5_flow_meter_policy_validate(struct rte_eth_dev *dev, RTE_MTR_ERROR_TYPE_METER_POLICY, NULL, "meter policy unsupported."); ret = mlx5_flow_validate_mtr_acts(dev, policy->actions, &attr, - &is_rss, &domain_bitmap, &is_def_policy, error); + &is_rss, &domain_bitmap, &policy_mode, error); if (ret) return ret; return 0; @@ -674,7 +674,7 @@ mlx5_flow_meter_policy_add(struct rte_eth_dev *dev, struct mlx5_flow_meter_policy *mtr_policy = NULL; struct mlx5_flow_meter_sub_policy *sub_policy; bool is_rss = false; - bool is_def_policy = false; + uint8_t policy_mode; uint32_t i; int ret; uint32_t policy_size = sizeof(struct mlx5_flow_meter_policy); @@ -701,14 +701,15 @@ mlx5_flow_meter_policy_add(struct rte_eth_dev *dev, 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); + &is_rss, &domain_bitmap, + &policy_mode, error); if (ret) return ret; if (!domain_bitmap) return -rte_mtr_error_set(error, ENOTSUP, - RTE_MTR_ERROR_TYPE_METER_POLICY, - NULL, "fail to find policy domain."); - if (is_def_policy) { + RTE_MTR_ERROR_TYPE_METER_POLICY, + NULL, "fail to find policy domain."); + if (policy_mode == MLX5_MTR_POLICY_MODE_DEF) { if (priv->sh->mtrmng->def_policy_id != MLX5_INVALID_POLICY_ID) return -rte_mtr_error_set(error, EEXIST, RTE_MTR_ERROR_TYPE_METER_POLICY_ID, -- 2.27.0 ^ permalink raw reply [flat|nested] 24+ messages in thread
* [dpdk-dev] [PATCH v2 6/7] net/mlx5: check consistency of meter policy and profile 2021-07-18 17:18 ` [dpdk-dev] [PATCH v2 0/7] support yellow color policy in mlx5 Bing Zhao ` (4 preceding siblings ...) 2021-07-18 17:18 ` [dpdk-dev] [PATCH v2 5/7] net/mlx5: support yellow in meter policy validation Bing Zhao @ 2021-07-18 17:18 ` Bing Zhao 2021-07-18 17:18 ` [dpdk-dev] [PATCH v2 7/7] net/mlx5: add meter support for trTCM profiles Bing Zhao 6 siblings, 0 replies; 24+ messages in thread From: Bing Zhao @ 2021-07-18 17:18 UTC (permalink / raw) To: viacheslavo, matan; +Cc: dev, orika, rasland, thomas, lizh, shunh In the previous implementation, only green color policy was supported in mlx5 PMD. Since yellow color policy is supported now, the consistency of meter policy and profile should be checked. 1. If the profile supports yellow but the policy doesn't, an error should be returned when creating the meter. Or else, there is no explicit steering action for the packets marked with yellow. 2. If the policy supports yellow but the profile doesn't, it will be considered as a valid case. Even if no packet will be handled with the yellow steering action, it is just like that only the green policy presents. Usually the green color is supported by default, but when it is disabled intentionally with setting the CBS to a small value like zero in the profile, the similar checking on green policy and profile should also be done. Signed-off-by: Bing Zhao <bingz@nvidia.com> --- drivers/net/mlx5/mlx5.h | 2 ++ drivers/net/mlx5/mlx5_flow_meter.c | 16 ++++++++++++++-- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h index 9832ed4189..3a8587b7cf 100644 --- a/drivers/net/mlx5/mlx5.h +++ b/drivers/net/mlx5/mlx5.h @@ -870,6 +870,8 @@ struct mlx5_flow_meter_profile { /**< srtcm_rfc2697 struct. */ }; uint32_t ref_cnt; /**< Use count. */ + uint32_t g_support:1; /**< If G color will be generated. */ + uint32_t y_support:1; /**< If Y color will be generated. */ }; /* 2 meters in each ASO cache line */ diff --git a/drivers/net/mlx5/mlx5_flow_meter.c b/drivers/net/mlx5/mlx5_flow_meter.c index e9b9b22fb2..cf3fb8aa9d 100644 --- a/drivers/net/mlx5/mlx5_flow_meter.c +++ b/drivers/net/mlx5/mlx5_flow_meter.c @@ -333,6 +333,10 @@ mlx5_flow_meter_param_fill(struct mlx5_flow_meter_profile *fmp, ebs_exp = exp; srtcm->ebs_eir = rte_cpu_to_be_32(ebs_exp << ASO_DSEG_EBS_EXP_OFFSET | ebs_man << ASO_DSEG_EBS_MAN_OFFSET); + if (srtcm->cbs_cir) + fmp->g_support = 1; + if (srtcm->ebs_eir) + fmp->y_support = 1; return 0; } @@ -1136,13 +1140,13 @@ mlx5_flow_meter_create(struct rte_eth_dev *dev, uint32_t meter_id, if (!priv->config.dv_esw_en) domain_bitmap &= ~MLX5_MTR_DOMAIN_TRANSFER_BIT; } else { - mtr_policy = mlx5_flow_meter_policy_find(dev, - params->meter_policy_id, &policy_idx); if (!priv->sh->meter_aso_en) return -rte_mtr_error_set(error, ENOTSUP, RTE_MTR_ERROR_TYPE_UNSPECIFIED, NULL, "Part of the policies cannot be " "supported without ASO "); + mtr_policy = mlx5_flow_meter_policy_find(dev, + params->meter_policy_id, &policy_idx); if (!mtr_policy) return -rte_mtr_error_set(error, ENOENT, RTE_MTR_ERROR_TYPE_METER_POLICY_ID, @@ -1153,6 +1157,14 @@ mlx5_flow_meter_create(struct rte_eth_dev *dev, uint32_t meter_id, MLX5_MTR_DOMAIN_EGRESS_BIT : 0) | (mtr_policy->transfer ? MLX5_MTR_DOMAIN_TRANSFER_BIT : 0); + if (fmp->g_support && mtr_policy->skip_g) + return -rte_mtr_error_set(error, ENOTSUP, + RTE_MTR_ERROR_TYPE_METER_POLICY_ID, + NULL, "Meter green policy is empty."); + if (fmp->y_support && mtr_policy->skip_y) + return -rte_mtr_error_set(error, ENOTSUP, + RTE_MTR_ERROR_TYPE_METER_POLICY_ID, + NULL, "Meter yellow policy is empty."); } /* Allocate the flow meter memory. */ if (priv->sh->meter_aso_en) { -- 2.27.0 ^ permalink raw reply [flat|nested] 24+ messages in thread
* [dpdk-dev] [PATCH v2 7/7] net/mlx5: add meter support for trTCM profiles 2021-07-18 17:18 ` [dpdk-dev] [PATCH v2 0/7] support yellow color policy in mlx5 Bing Zhao ` (5 preceding siblings ...) 2021-07-18 17:18 ` [dpdk-dev] [PATCH v2 6/7] net/mlx5: check consistency of meter policy and profile Bing Zhao @ 2021-07-18 17:18 ` Bing Zhao 6 siblings, 0 replies; 24+ messages in thread From: Bing Zhao @ 2021-07-18 17:18 UTC (permalink / raw) To: viacheslavo, matan; +Cc: dev, orika, rasland, thomas, lizh, shunh The support of RFC2698 and RFC4115 are added in mlx5 PMD. Only the ASO metering supports these two profiles. Signed-off-by: Bing Zhao <bingz@nvidia.com> --- doc/guides/nics/mlx5.rst | 1 + doc/guides/rel_notes/release_21_08.rst | 1 + drivers/common/mlx5/mlx5_prm.h | 5 +- drivers/net/mlx5/mlx5_flow_aso.c | 23 ++++- drivers/net/mlx5/mlx5_flow_meter.c | 112 ++++++++++++++++--------- 5 files changed, 98 insertions(+), 44 deletions(-) diff --git a/doc/guides/nics/mlx5.rst b/doc/guides/nics/mlx5.rst index 9396074b5a..e20d02d607 100644 --- a/doc/guides/nics/mlx5.rst +++ b/doc/guides/nics/mlx5.rst @@ -434,6 +434,7 @@ Limitations - RED: must be DROP. - Policy actions of RSS for green and yellow should have the same configuration except queues. - meter profile packet mode is supported. + - meter profiles of RFC2697, RFC2698 and RFC4115 are supported. - Integrity: diff --git a/doc/guides/rel_notes/release_21_08.rst b/doc/guides/rel_notes/release_21_08.rst index 03d4fd059a..e159615deb 100644 --- a/doc/guides/rel_notes/release_21_08.rst +++ b/doc/guides/rel_notes/release_21_08.rst @@ -92,6 +92,7 @@ New Features * Added support for matching on VXLAN header last 8-bits reserved field. * Optimized multi-thread flow rule insertion rate. * Added support for metering policy actions of yellow color. + * Added support for metering trTCM RFC2698 and RFC4115. * **Added Wangxun ngbe PMD.** diff --git a/drivers/common/mlx5/mlx5_prm.h b/drivers/common/mlx5/mlx5_prm.h index 7950070976..88705be9d6 100644 --- a/drivers/common/mlx5/mlx5_prm.h +++ b/drivers/common/mlx5/mlx5_prm.h @@ -3031,11 +3031,12 @@ struct mlx5_aso_mtr_dseg { #define ASO_DSEG_VALID_OFFSET 31 #define ASO_DSEG_BO_OFFSET 30 #define ASO_DSEG_SC_OFFSET 28 +#define ASO_DSEG_BBOG_OFFSET 27 #define ASO_DSEG_MTR_MODE 24 #define ASO_DSEG_CBS_EXP_OFFSET 24 #define ASO_DSEG_CBS_MAN_OFFSET 16 -#define ASO_DSEG_CIR_EXP_MASK 0x1F -#define ASO_DSEG_CIR_EXP_OFFSET 8 +#define ASO_DSEG_XIR_EXP_MASK 0x1F +#define ASO_DSEG_XIR_EXP_OFFSET 8 #define ASO_DSEG_EBS_EXP_OFFSET 24 #define ASO_DSEG_EBS_MAN_OFFSET 16 #define ASO_DSEG_EXP_MASK 0x1F diff --git a/drivers/net/mlx5/mlx5_flow_aso.c b/drivers/net/mlx5/mlx5_flow_aso.c index 23e22e560a..e11327a11b 100644 --- a/drivers/net/mlx5/mlx5_flow_aso.c +++ b/drivers/net/mlx5/mlx5_flow_aso.c @@ -747,10 +747,27 @@ mlx5_aso_mtr_sq_enqueue_single(struct mlx5_aso_sq *sq, wqe->aso_dseg.mtrs[dseg_idx].v_bo_sc_bbog_mm = RTE_BE32((1 << ASO_DSEG_VALID_OFFSET) | (MLX5_FLOW_COLOR_GREEN << ASO_DSEG_SC_OFFSET)); - /* Only needed for RFC2697. */ - if (fm->profile->srtcm_prm.ebs_eir) + switch (fmp->profile.alg) { + case RTE_MTR_SRTCM_RFC2697: + /* Only needed for RFC2697. */ + if (fm->profile->srtcm_prm.ebs_eir) + wqe->aso_dseg.mtrs[dseg_idx].v_bo_sc_bbog_mm |= + RTE_BE32(1 << ASO_DSEG_BO_OFFSET); + break; + case RTE_MTR_TRTCM_RFC2698: wqe->aso_dseg.mtrs[dseg_idx].v_bo_sc_bbog_mm |= - RTE_BE32(1 << ASO_DSEG_BO_OFFSET); + RTE_BE32(1 << ASO_DSEG_BBOG_OFFSET); + break; + case RTE_MTR_TRTCM_RFC4115: + default: + break; + } + /* + * Note: + * Due to software performance reason, the token fields will not be + * set when posting the WQE to ASO SQ. It will be filled by the HW + * automatically. + */ sq->head++; sq->pi += 2;/* Each WQE contains 2 WQEBB's. */ rte_io_wmb(); diff --git a/drivers/net/mlx5/mlx5_flow_meter.c b/drivers/net/mlx5/mlx5_flow_meter.c index cf3fb8aa9d..b077a01896 100644 --- a/drivers/net/mlx5/mlx5_flow_meter.c +++ b/drivers/net/mlx5/mlx5_flow_meter.c @@ -55,7 +55,7 @@ mlx5_flow_meter_action_create(struct mlx5_priv *priv, MLX5_SET(flow_meter_parameters, fmp, cbs_exponent, val); val = (cbs_cir >> ASO_DSEG_CBS_MAN_OFFSET) & ASO_DSEG_MAN_MASK; MLX5_SET(flow_meter_parameters, fmp, cbs_mantissa, val); - val = (cbs_cir >> ASO_DSEG_CIR_EXP_OFFSET) & ASO_DSEG_EXP_MASK; + val = (cbs_cir >> ASO_DSEG_XIR_EXP_OFFSET) & ASO_DSEG_EXP_MASK; MLX5_SET(flow_meter_parameters, fmp, cir_exponent, val); val = (cbs_cir & ASO_DSEG_MAN_MASK); MLX5_SET(flow_meter_parameters, fmp, cir_mantissa, val); @@ -194,18 +194,18 @@ mlx5_flow_meter_profile_validate(struct rte_eth_dev *dev, NULL, "Metering algorithm not supported."); } -/** - * Calculate mantissa and exponent for cir. +/* + * Calculate mantissa and exponent for cir / eir. * - * @param[in] cir + * @param[in] xir * Value to be calculated. * @param[out] man * Pointer to the mantissa. * @param[out] exp * Pointer to the exp. */ -static void -mlx5_flow_meter_cir_man_exp_calc(int64_t cir, uint8_t *man, uint8_t *exp) +static inline void +mlx5_flow_meter_xir_man_exp_calc(int64_t xir, uint8_t *man, uint8_t *exp) { int64_t _cir; int64_t delta = INT64_MAX; @@ -216,8 +216,8 @@ mlx5_flow_meter_cir_man_exp_calc(int64_t cir, uint8_t *man, uint8_t *exp) for (m = 0; m <= 0xFF; m++) { /* man width 8 bit */ for (e = 0; e <= 0x1F; e++) { /* exp width 5bit */ _cir = (1000000000ULL * m) >> e; - if (llabs(cir - _cir) <= delta) { - delta = llabs(cir - _cir); + if (llabs(xir - _cir) <= delta) { + delta = llabs(xir - _cir); _man = m; _exp = e; } @@ -227,7 +227,7 @@ mlx5_flow_meter_cir_man_exp_calc(int64_t cir, uint8_t *man, uint8_t *exp) *exp = _exp; } -/** +/* * Calculate mantissa and exponent for xbs. * * @param[in] xbs @@ -237,7 +237,7 @@ mlx5_flow_meter_cir_man_exp_calc(int64_t cir, uint8_t *man, uint8_t *exp) * @param[out] exp * Pointer to the exp. */ -static void +static inline void mlx5_flow_meter_xbs_man_exp_calc(uint64_t xbs, uint8_t *man, uint8_t *exp) { int _exp; @@ -275,37 +275,63 @@ mlx5_flow_meter_param_fill(struct mlx5_flow_meter_profile *fmp, struct mlx5_flow_meter_srtcm_rfc2697_prm *srtcm = &fmp->srtcm_prm; uint8_t man, exp; uint32_t cbs_exp, cbs_man, cir_exp, cir_man; - uint32_t ebs_exp, ebs_man; - uint64_t cir, cbs, ebs; + uint32_t eir_exp, eir_man, ebs_exp, ebs_man; + uint64_t cir, cbs, eir, ebs; - if (fmp->profile.alg != RTE_MTR_SRTCM_RFC2697) - return -rte_mtr_error_set(error, ENOTSUP, + if (!priv->sh->meter_aso_en) { + /* Legacy FW metering will only support srTCM. */ + if (fmp->profile.alg != RTE_MTR_SRTCM_RFC2697) + return -rte_mtr_error_set(error, ENOTSUP, RTE_MTR_ERROR_TYPE_METER_PROFILE, - NULL, "Metering algorithm not supported."); - if (!priv->sh->meter_aso_en && fmp->profile.packet_mode) - return -rte_mtr_error_set(error, ENOTSUP, - RTE_MTR_ERROR_TYPE_METER_PROFILE, - NULL, "Metering algorithm packet mode not supported."); - if (priv->sh->meter_aso_en && fmp->profile.packet_mode) { - cir = fmp->profile.srtcm_rfc2697.cir << - MLX5_MTRS_PPS_MAP_BPS_SHIFT; - cbs = fmp->profile.srtcm_rfc2697.cbs << - MLX5_MTRS_PPS_MAP_BPS_SHIFT; - ebs = fmp->profile.srtcm_rfc2697.ebs << - MLX5_MTRS_PPS_MAP_BPS_SHIFT; - } else { + NULL, "Metering algorithm is not supported."); + if (fmp->profile.packet_mode) + return -rte_mtr_error_set(error, ENOTSUP, + RTE_MTR_ERROR_TYPE_METER_PROFILE, NULL, + "Metering algorithm packet mode is not supported."); + } + switch (fmp->profile.alg) { + case RTE_MTR_SRTCM_RFC2697: cir = fmp->profile.srtcm_rfc2697.cir; cbs = fmp->profile.srtcm_rfc2697.cbs; + eir = 0; ebs = fmp->profile.srtcm_rfc2697.ebs; + break; + case RTE_MTR_TRTCM_RFC2698: + MLX5_ASSERT(fmp->profile.trtcm_rfc2698.pir > + fmp->profile.trtcm_rfc2698.cir && + fmp->profile.trtcm_rfc2698.pbs > + fmp->profile.trtcm_rfc2698.cbs); + cir = fmp->profile.trtcm_rfc2698.cir; + cbs = fmp->profile.trtcm_rfc2698.cbs; + /* EIR / EBS are filled with PIR / PBS. */ + eir = fmp->profile.trtcm_rfc2698.pir; + ebs = fmp->profile.trtcm_rfc2698.pbs; + break; + case RTE_MTR_TRTCM_RFC4115: + cir = fmp->profile.trtcm_rfc4115.cir; + cbs = fmp->profile.trtcm_rfc4115.cbs; + eir = fmp->profile.trtcm_rfc4115.eir; + ebs = fmp->profile.trtcm_rfc4115.ebs; + break; + default: + return -rte_mtr_error_set(error, EINVAL, + RTE_MTR_ERROR_TYPE_METER_PROFILE, NULL, + "Metering algorithm mode is invalid"); + } + /* Adjust the values for PPS mode. */ + if (fmp->profile.packet_mode) { + cir <<= MLX5_MTRS_PPS_MAP_BPS_SHIFT; + cbs <<= MLX5_MTRS_PPS_MAP_BPS_SHIFT; + eir <<= MLX5_MTRS_PPS_MAP_BPS_SHIFT; + ebs <<= MLX5_MTRS_PPS_MAP_BPS_SHIFT; } /* cir = 8G * cir_mantissa * 1/(2^cir_exponent)) Bytes/Sec */ - mlx5_flow_meter_cir_man_exp_calc(cir, &man, &exp); + mlx5_flow_meter_xir_man_exp_calc(cir, &man, &exp); /* Check if cir mantissa is too large. */ - if (exp > ASO_DSEG_CIR_EXP_MASK) + if (exp > ASO_DSEG_XIR_EXP_MASK) return -rte_mtr_error_set(error, ENOTSUP, RTE_MTR_ERROR_TYPE_MTR_PARAMS, NULL, - "meter profile parameter cir is" - " not supported."); + "meter profile parameter cir is not supported."); cir_man = man; cir_exp = exp; /* cbs = cbs_mantissa * 2^cbs_exponent */ @@ -314,25 +340,33 @@ mlx5_flow_meter_param_fill(struct mlx5_flow_meter_profile *fmp, if (exp > ASO_DSEG_EXP_MASK) return -rte_mtr_error_set(error, ENOTSUP, RTE_MTR_ERROR_TYPE_MTR_PARAMS, NULL, - "meter profile parameter cbs is" - " not supported."); + "meter profile parameter cbs is not supported."); cbs_man = man; cbs_exp = exp; srtcm->cbs_cir = rte_cpu_to_be_32(cbs_exp << ASO_DSEG_CBS_EXP_OFFSET | cbs_man << ASO_DSEG_CBS_MAN_OFFSET | - cir_exp << ASO_DSEG_CIR_EXP_OFFSET | + cir_exp << ASO_DSEG_XIR_EXP_OFFSET | cir_man); + mlx5_flow_meter_xir_man_exp_calc(eir, &man, &exp); + /* Check if eir mantissa is too large. */ + if (exp > ASO_DSEG_XIR_EXP_MASK) + return -rte_mtr_error_set(error, ENOTSUP, + RTE_MTR_ERROR_TYPE_MTR_PARAMS, NULL, + "meter profile parameter eir is not supported."); + eir_man = man; + eir_exp = exp; mlx5_flow_meter_xbs_man_exp_calc(ebs, &man, &exp); /* Check if ebs mantissa is too large. */ if (exp > ASO_DSEG_EXP_MASK) return -rte_mtr_error_set(error, ENOTSUP, RTE_MTR_ERROR_TYPE_MTR_PARAMS, NULL, - "meter profile parameter ebs is" - " not supported."); + "meter profile parameter ebs is not supported."); ebs_man = man; ebs_exp = exp; srtcm->ebs_eir = rte_cpu_to_be_32(ebs_exp << ASO_DSEG_EBS_EXP_OFFSET | - ebs_man << ASO_DSEG_EBS_MAN_OFFSET); + ebs_man << ASO_DSEG_EBS_MAN_OFFSET | + eir_exp << ASO_DSEG_XIR_EXP_OFFSET | + eir_man); if (srtcm->cbs_cir) fmp->g_support = 1; if (srtcm->ebs_eir) @@ -1008,7 +1042,7 @@ mlx5_flow_meter_action_modify(struct mlx5_priv *priv, cbs_mantissa, val); } if (modify_bits & MLX5_FLOW_METER_OBJ_MODIFY_FIELD_CIR) { - val = (cbs_cir >> ASO_DSEG_CIR_EXP_OFFSET) & + val = (cbs_cir >> ASO_DSEG_XIR_EXP_OFFSET) & ASO_DSEG_EXP_MASK; MLX5_SET(flow_meter_parameters, attr, cir_exponent, val); @@ -1389,7 +1423,7 @@ mlx5_flow_meter_modify_state(struct mlx5_priv *priv, &srtcm, modify_bits, 0, 0); else ret = mlx5_flow_meter_action_modify(priv, fm, - &fm->profile->srtcm_prm, + &fm->profile->srtcm_prm, modify_bits, 0, 1); if (ret) return -rte_mtr_error_set(error, -ret, -- 2.27.0 ^ permalink raw reply [flat|nested] 24+ messages in thread
* [dpdk-dev] [PATCH v3 0/7] support yellow color policy in mlx5 2021-07-05 15:57 [dpdk-dev] [PATCH 0/6] support yellow color policy in mlx5 Bing Zhao ` (6 preceding siblings ...) 2021-07-18 17:18 ` [dpdk-dev] [PATCH v2 0/7] support yellow color policy in mlx5 Bing Zhao @ 2021-07-21 8:54 ` Bing Zhao 2021-07-21 8:54 ` [dpdk-dev] [PATCH v3 1/7] net/mlx5: handle yellow case in default meter policy Bing Zhao ` (7 more replies) 7 siblings, 8 replies; 24+ messages in thread From: Bing Zhao @ 2021-07-21 8:54 UTC (permalink / raw) To: viacheslavo, matan; +Cc: dev, orika, rasland, thomas, lizh, shunh When creating a meter policy, the actions for yellow color can be specified together with green color. The mlx5 PMD now supports to set the policy actions for yellow color. The actions list that is supported for yellow is the same as that for green. --- v3: * patch set building fix * bug fixes and document update v2: * bug fixes * add policy and profile consistency checking * add trTCM RFC2698 and RFC4115 support --- Acked-by: Matan Azrad <matan@nvidia.com> Bing Zhao (7): net/mlx5: handle yellow case in default meter policy net/mlx5: enable meter bucket overflow for yellow color net/mlx5: added support for yellow policy rules net/mlx5: split policies handling of colors net/mlx5: support yellow in meter policy validation net/mlx5: check consistency of meter policy and profile net/mlx5: add meter support for trTCM profiles doc/guides/nics/mlx5.rst | 10 +- doc/guides/rel_notes/release_21_08.rst | 2 + drivers/common/mlx5/mlx5_prm.h | 5 +- drivers/net/mlx5/mlx5.h | 20 +- drivers/net/mlx5/mlx5_flow.c | 46 +-- drivers/net/mlx5/mlx5_flow.h | 4 +- drivers/net/mlx5/mlx5_flow_aso.c | 21 ++ drivers/net/mlx5/mlx5_flow_dv.c | 471 +++++++++++++++---------- drivers/net/mlx5/mlx5_flow_meter.c | 203 +++++++---- 9 files changed, 492 insertions(+), 290 deletions(-) -- 2.27.0 ^ permalink raw reply [flat|nested] 24+ messages in thread
* [dpdk-dev] [PATCH v3 1/7] net/mlx5: handle yellow case in default meter policy 2021-07-21 8:54 ` [dpdk-dev] [PATCH v3 0/7] support yellow color policy in mlx5 Bing Zhao @ 2021-07-21 8:54 ` Bing Zhao 2021-07-21 8:54 ` [dpdk-dev] [PATCH v3 2/7] net/mlx5: enable meter bucket overflow for yellow color Bing Zhao ` (6 subsequent siblings) 7 siblings, 0 replies; 24+ messages in thread From: Bing Zhao @ 2021-07-21 8:54 UTC (permalink / raw) To: viacheslavo, matan; +Cc: dev, orika, rasland, thomas, lizh, shunh In order to support the yellow color for the default meter policy, the default policy action for yellow should be created together with the green policy. The default policy action for yellow action is the same as that for green. In the same table, the same matcher will be reused for yellow and the destination group will be the same. Signed-off-by: Bing Zhao <bingz@nvidia.com> --- drivers/net/mlx5/mlx5.h | 6 +- drivers/net/mlx5/mlx5_flow_dv.c | 144 +++++++++++++++++++------------- 2 files changed, 91 insertions(+), 59 deletions(-) diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h index 94618e10fa..a2fe9b90c7 100644 --- a/drivers/net/mlx5/mlx5.h +++ b/drivers/net/mlx5/mlx5.h @@ -632,8 +632,8 @@ struct mlx5_dev_shared_port { /*ASO flow meter structures*/ /* Modify this value if enum rte_mtr_color changes. */ #define RTE_MTR_DROPPED RTE_COLORS -/* Yellow is not supported. */ -#define MLX5_MTR_RTE_COLORS (RTE_COLOR_GREEN + 1) +/* Yellow is now supported. */ +#define MLX5_MTR_RTE_COLORS (RTE_COLOR_YELLOW + 1) /* table_id 22 bits in mlx5_flow_tbl_key so limit policy number. */ #define MLX5_MAX_SUB_POLICY_TBL_NUM 0x3FFFFF #define MLX5_INVALID_POLICY_ID UINT32_MAX @@ -641,6 +641,8 @@ struct mlx5_dev_shared_port { #define MLX5_MTR_TABLE_ID_SUFFIX 1 /* Drop table_id on MLX5_FLOW_TABLE_LEVEL_METER. */ #define MLX5_MTR_TABLE_ID_DROP 2 +/* Priority of the meter policy matcher. */ +#define MLX5_MTR_POLICY_MATCHER_PRIO 0 enum mlx5_meter_domain { MLX5_MTR_DOMAIN_INGRESS, diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c index d250486950..cfc646c5e5 100644 --- a/drivers/net/mlx5/mlx5_flow_dv.c +++ b/drivers/net/mlx5/mlx5_flow_dv.c @@ -188,7 +188,7 @@ flow_dv_attr_init(const struct rte_flow_item *item, union flow_dv_attr *attr, attr->valid = 1; } -/** +/* * Convert rte_mtr_color to mlx5 color. * * @param[in] rcol @@ -197,7 +197,7 @@ flow_dv_attr_init(const struct rte_flow_item *item, union flow_dv_attr *attr, * @return * mlx5 color. */ -static int +static inline int rte_col_2_mlx5_col(enum rte_color rcol) { switch (rcol) { @@ -15892,7 +15892,7 @@ flow_dv_destroy_mtr_drop_tbls(struct rte_eth_dev *dev) static void __flow_dv_destroy_domain_def_policy(struct rte_eth_dev *dev, - enum mlx5_meter_domain domain) + enum mlx5_meter_domain domain) { struct mlx5_priv *priv = dev->data->dev_private; struct mlx5_flow_meter_def_policy *def_policy = @@ -15943,21 +15943,20 @@ __flow_dv_create_policy_flow(struct rte_eth_dev *dev, if (match_src_port && (priv->representor || priv->master)) { if (flow_dv_translate_item_port_id(dev, matcher.buf, value.buf, item, attr)) { - DRV_LOG(ERR, - "Failed to create meter policy flow with port."); + DRV_LOG(ERR, "Failed to create meter policy%d flow's" + " value with port.", color); return -1; } } flow_dv_match_meta_reg(matcher.buf, value.buf, - (enum modify_reg)color_reg_c_idx, - rte_col_2_mlx5_col(color), - UINT32_MAX); + (enum modify_reg)color_reg_c_idx, + rte_col_2_mlx5_col(color), UINT32_MAX); misc_mask = flow_dv_matcher_enable(value.buf); __flow_dv_adjust_buf_size(&value.size, misc_mask); - ret = mlx5_flow_os_create_flow(matcher_object, - (void *)&value, actions_n, actions, rule); + ret = mlx5_flow_os_create_flow(matcher_object, (void *)&value, + actions_n, actions, rule); if (ret) { - DRV_LOG(ERR, "Failed to create meter policy flow."); + DRV_LOG(ERR, "Failed to create meter policy%d flow.", color); return -1; } return 0; @@ -15991,13 +15990,13 @@ __flow_dv_create_policy_matcher(struct rte_eth_dev *dev, }; struct mlx5_flow_tbl_data_entry *tbl_data; struct mlx5_priv *priv = dev->data->dev_private; - uint32_t color_mask = (UINT32_C(1) << MLX5_MTR_COLOR_BITS) - 1; + const uint32_t color_mask = (UINT32_C(1) << MLX5_MTR_COLOR_BITS) - 1; if (match_src_port && (priv->representor || priv->master)) { if (flow_dv_translate_item_port_id(dev, matcher.mask.buf, value.buf, item, attr)) { - DRV_LOG(ERR, - "Failed to register meter drop matcher with port."); + DRV_LOG(ERR, "Failed to register meter policy%d matcher" + " with port.", priority); return -1; } } @@ -16007,7 +16006,7 @@ __flow_dv_create_policy_matcher(struct rte_eth_dev *dev, (enum modify_reg)color_reg_c_idx, 0, color_mask); matcher.priority = priority; matcher.crc = rte_raw_cksum((const void *)matcher.mask.buf, - matcher.mask.size); + matcher.mask.size); entry = mlx5_list_register(tbl_data->matchers, &ctx); if (!entry) { DRV_LOG(ERR, "Failed to register meter drop matcher."); @@ -16055,6 +16054,8 @@ __flow_dv_create_domain_policy_rules(struct rte_eth_dev *dev, int i; int ret = mlx5_flow_get_reg_id(dev, MLX5_MTR_COLOR, 0, &flow_err); struct mlx5_sub_policy_color_rule *color_rule; + bool svport_match; + struct mlx5_sub_policy_color_rule *tmp_rules[RTE_COLORS] = {NULL}; if (ret < 0) return -1; @@ -16073,7 +16074,7 @@ __flow_dv_create_domain_policy_rules(struct rte_eth_dev *dev, color_reg_c_idx = ret; for (i = 0; i < RTE_COLORS; i++) { TAILQ_INIT(&sub_policy->color_rules[i]); - if (i == RTE_COLOR_YELLOW || !acts[i].actions_n) + if (!acts[i].actions_n) continue; color_rule = mlx5_malloc(MLX5_MEM_ZERO, sizeof(struct mlx5_sub_policy_color_rule), @@ -16082,45 +16083,52 @@ __flow_dv_create_domain_policy_rules(struct rte_eth_dev *dev, DRV_LOG(ERR, "No memory to create color rule."); goto err_exit; } + tmp_rules[i] = color_rule; + TAILQ_INSERT_TAIL(&sub_policy->color_rules[i], + color_rule, next_port); color_rule->src_port = priv->representor_id; + /* No use. */ attr.priority = i; - /* Create matchers for Color. */ - if (__flow_dv_create_policy_matcher(dev, - color_reg_c_idx, i, sub_policy, &attr, - (i != RTE_COLOR_RED ? match_src_port : false), - NULL, &color_rule->matcher, &flow_err)) { - DRV_LOG(ERR, "Failed to create color matcher."); + /* Create matchers for colors. */ + svport_match = (i != RTE_COLOR_RED) ? match_src_port : false; + if (__flow_dv_create_policy_matcher(dev, color_reg_c_idx, + MLX5_MTR_POLICY_MATCHER_PRIO, sub_policy, + &attr, svport_match, NULL, + &color_rule->matcher, &flow_err)) { + DRV_LOG(ERR, "Failed to create color%u matcher.", i); goto err_exit; } /* Create flow, matching color. */ if (__flow_dv_create_policy_flow(dev, color_reg_c_idx, (enum rte_color)i, color_rule->matcher->matcher_object, - acts[i].actions_n, - acts[i].dv_actions, - (i != RTE_COLOR_RED ? match_src_port : false), - NULL, &color_rule->rule, + acts[i].actions_n, acts[i].dv_actions, + svport_match, NULL, &color_rule->rule, &attr)) { - DRV_LOG(ERR, "Failed to create color rule."); + DRV_LOG(ERR, "Failed to create color%u rule.", i); goto err_exit; } - TAILQ_INSERT_TAIL(&sub_policy->color_rules[i], - color_rule, next_port); } return 0; err_exit: - if (color_rule) { - if (color_rule->rule) - mlx5_flow_os_destroy_flow(color_rule->rule); - if (color_rule->matcher) { - struct mlx5_flow_tbl_data_entry *tbl = - container_of(color_rule->matcher->tbl, - typeof(*tbl), tbl); - mlx5_list_unregister(tbl->matchers, + /* All the policy rules will be cleared. */ + do { + color_rule = tmp_rules[i]; + if (color_rule) { + if (color_rule->rule) + mlx5_flow_os_destroy_flow(color_rule->rule); + if (color_rule->matcher) { + struct mlx5_flow_tbl_data_entry *tbl = + container_of(color_rule->matcher->tbl, + typeof(*tbl), tbl); + mlx5_list_unregister(tbl->matchers, &color_rule->matcher->entry); + } + TAILQ_REMOVE(&sub_policy->color_rules[i], + color_rule, next_port); + mlx5_free(color_rule); } - mlx5_free(color_rule); - } + } while (i--); return -1; } @@ -16342,8 +16350,7 @@ __flow_dv_create_domain_def_policy(struct rte_eth_dev *dev, uint32_t domain) sizeof(struct mlx5_flow_meter_def_policy), RTE_CACHE_LINE_SIZE, SOCKET_ID_ANY); if (!def_policy) { - DRV_LOG(ERR, "Failed to alloc " - "default policy table."); + DRV_LOG(ERR, "Failed to alloc default policy table."); goto def_policy_error; } mtrmng->def_policy[domain] = def_policy; @@ -16359,26 +16366,48 @@ __flow_dv_create_domain_def_policy(struct rte_eth_dev *dev, uint32_t domain) } def_policy->sub_policy.jump_tbl[RTE_COLOR_GREEN] = jump_tbl; tbl_data = container_of(jump_tbl, - struct mlx5_flow_tbl_data_entry, tbl); + struct mlx5_flow_tbl_data_entry, tbl); def_policy->dr_jump_action[RTE_COLOR_GREEN] = tbl_data->jump.action; - acts[RTE_COLOR_GREEN].dv_actions[0] = - tbl_data->jump.action; + acts[RTE_COLOR_GREEN].dv_actions[0] = tbl_data->jump.action; acts[RTE_COLOR_GREEN].actions_n = 1; + /* + * YELLOW has the same default policy as GREEN does. + * G & Y share the same table and action. The 2nd time of table + * resource getting is just to update the reference count for + * the releasing stage. + */ + jump_tbl = flow_dv_tbl_resource_get(dev, + MLX5_FLOW_TABLE_LEVEL_METER, + egress, transfer, false, NULL, 0, + 0, MLX5_MTR_TABLE_ID_SUFFIX, &error); + if (!jump_tbl) { + DRV_LOG(ERR, + "Failed to get meter suffix table."); + goto def_policy_error; + } + def_policy->sub_policy.jump_tbl[RTE_COLOR_YELLOW] = jump_tbl; + tbl_data = container_of(jump_tbl, + struct mlx5_flow_tbl_data_entry, tbl); + def_policy->dr_jump_action[RTE_COLOR_YELLOW] = + tbl_data->jump.action; + acts[RTE_COLOR_YELLOW].dv_actions[0] = tbl_data->jump.action; + acts[RTE_COLOR_YELLOW].actions_n = 1; /* Create jump action to the drop table. */ if (!mtrmng->drop_tbl[domain]) { mtrmng->drop_tbl[domain] = flow_dv_tbl_resource_get (dev, MLX5_FLOW_TABLE_LEVEL_METER, - egress, transfer, false, NULL, 0, - 0, MLX5_MTR_TABLE_ID_DROP, &error); + egress, transfer, false, NULL, 0, + 0, MLX5_MTR_TABLE_ID_DROP, &error); if (!mtrmng->drop_tbl[domain]) { - DRV_LOG(ERR, "Failed to create " - "meter drop table for default policy."); + DRV_LOG(ERR, "Failed to create meter " + "drop table for default policy."); goto def_policy_error; } } + /* all RED: unique Drop table for jump action. */ tbl_data = container_of(mtrmng->drop_tbl[domain], - struct mlx5_flow_tbl_data_entry, tbl); + struct mlx5_flow_tbl_data_entry, tbl); def_policy->dr_jump_action[RTE_COLOR_RED] = tbl_data->jump.action; acts[RTE_COLOR_RED].dv_actions[0] = tbl_data->jump.action; @@ -16388,15 +16417,14 @@ __flow_dv_create_domain_def_policy(struct rte_eth_dev *dev, uint32_t domain) &def_policy->sub_policy, egress, transfer, false, acts); if (ret) { - DRV_LOG(ERR, "Failed to create " - "default policy rules."); - goto def_policy_error; + DRV_LOG(ERR, "Failed to create default policy rules."); + goto def_policy_error; } } return 0; def_policy_error: __flow_dv_destroy_domain_def_policy(dev, - (enum mlx5_meter_domain)domain); + (enum mlx5_meter_domain)domain); return -1; } @@ -16419,8 +16447,9 @@ flow_dv_create_def_policy(struct rte_eth_dev *dev) if (!priv->config.dv_esw_en && i == MLX5_MTR_DOMAIN_TRANSFER) continue; if (__flow_dv_create_domain_def_policy(dev, i)) { - DRV_LOG(ERR, - "Failed to create default policy"); + DRV_LOG(ERR, "Failed to create default policy"); + /* Rollback the created default policies for others. */ + flow_dv_destroy_def_policy(dev); return -1; } } @@ -16934,8 +16963,9 @@ flow_dv_meter_hierarchy_rule_create(struct rte_eth_dev *dev, goto err_exit; } if (__flow_dv_create_policy_matcher(dev, color_reg_c_idx, - i, sub_policy, &attr, true, item, - &color_rule->matcher, error)) { + MLX5_MTR_POLICY_MATCHER_PRIO, sub_policy, + &attr, true, item, + &color_rule->matcher, error)) { rte_flow_error_set(error, errno, RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL, "Failed to create hierarchy meter matcher."); -- 2.27.0 ^ permalink raw reply [flat|nested] 24+ messages in thread
* [dpdk-dev] [PATCH v3 2/7] net/mlx5: enable meter bucket overflow for yellow color 2021-07-21 8:54 ` [dpdk-dev] [PATCH v3 0/7] support yellow color policy in mlx5 Bing Zhao 2021-07-21 8:54 ` [dpdk-dev] [PATCH v3 1/7] net/mlx5: handle yellow case in default meter policy Bing Zhao @ 2021-07-21 8:54 ` Bing Zhao 2021-07-21 8:54 ` [dpdk-dev] [PATCH v3 3/7] net/mlx5: added support for yellow policy rules Bing Zhao ` (5 subsequent siblings) 7 siblings, 0 replies; 24+ messages in thread From: Bing Zhao @ 2021-07-21 8:54 UTC (permalink / raw) To: viacheslavo, matan; +Cc: dev, orika, rasland, thomas, lizh, shunh To support the meter policy for yellow action, the prerequisite is that the hardware needs to support the EBS, as defined in the RFC2697. https://datatracker.ietf.org/doc/html/rfc2697 Then some of the packets can be marked as yellow if the tokens of C bucket is not enough but enough in E bucket. The color could be used for the further steering of the packets. In the current implementation EBS and overflow were ignored when creating a meter profile. With this commit, if EBS is set by the application, the generation of yellow color will be enabled in the hardware for flow rules steering of packets. Signed-off-by: Bing Zhao <bingz@nvidia.com> --- drivers/net/mlx5/mlx5_flow_aso.c | 4 ++++ drivers/net/mlx5/mlx5_flow_meter.c | 10 +++++----- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/drivers/net/mlx5/mlx5_flow_aso.c b/drivers/net/mlx5/mlx5_flow_aso.c index 64631ffc29..23e22e560a 100644 --- a/drivers/net/mlx5/mlx5_flow_aso.c +++ b/drivers/net/mlx5/mlx5_flow_aso.c @@ -747,6 +747,10 @@ mlx5_aso_mtr_sq_enqueue_single(struct mlx5_aso_sq *sq, wqe->aso_dseg.mtrs[dseg_idx].v_bo_sc_bbog_mm = RTE_BE32((1 << ASO_DSEG_VALID_OFFSET) | (MLX5_FLOW_COLOR_GREEN << ASO_DSEG_SC_OFFSET)); + /* Only needed for RFC2697. */ + if (fm->profile->srtcm_prm.ebs_eir) + wqe->aso_dseg.mtrs[dseg_idx].v_bo_sc_bbog_mm |= + RTE_BE32(1 << ASO_DSEG_BO_OFFSET); sq->head++; sq->pi += 2;/* Each WQE contains 2 WQEBB's. */ rte_io_wmb(); diff --git a/drivers/net/mlx5/mlx5_flow_meter.c b/drivers/net/mlx5/mlx5_flow_meter.c index 78eb2a60f9..73eba0dabd 100644 --- a/drivers/net/mlx5/mlx5_flow_meter.c +++ b/drivers/net/mlx5/mlx5_flow_meter.c @@ -319,9 +319,9 @@ mlx5_flow_meter_param_fill(struct mlx5_flow_meter_profile *fmp, cbs_man = man; cbs_exp = exp; srtcm->cbs_cir = rte_cpu_to_be_32(cbs_exp << ASO_DSEG_CBS_EXP_OFFSET | - cbs_man << ASO_DSEG_CBS_MAN_OFFSET | - cir_exp << ASO_DSEG_CIR_EXP_OFFSET | - cir_man); + cbs_man << ASO_DSEG_CBS_MAN_OFFSET | + cir_exp << ASO_DSEG_CIR_EXP_OFFSET | + cir_man); mlx5_flow_meter_xbs_man_exp_calc(ebs, &man, &exp); /* Check if ebs mantissa is too large. */ if (exp > ASO_DSEG_EXP_MASK) @@ -332,7 +332,7 @@ mlx5_flow_meter_param_fill(struct mlx5_flow_meter_profile *fmp, ebs_man = man; ebs_exp = exp; srtcm->ebs_eir = rte_cpu_to_be_32(ebs_exp << ASO_DSEG_EBS_EXP_OFFSET | - ebs_man << ASO_DSEG_EBS_MAN_OFFSET); + ebs_man << ASO_DSEG_EBS_MAN_OFFSET); return 0; } @@ -421,7 +421,7 @@ mlx5_flow_meter_profile_add(struct rte_eth_dev *dev, return ret; /* Meter profile memory allocation. */ fmp = mlx5_malloc(MLX5_MEM_ZERO, sizeof(struct mlx5_flow_meter_profile), - RTE_CACHE_LINE_SIZE, SOCKET_ID_ANY); + RTE_CACHE_LINE_SIZE, SOCKET_ID_ANY); if (fmp == NULL) return -rte_mtr_error_set(error, ENOMEM, RTE_MTR_ERROR_TYPE_UNSPECIFIED, -- 2.27.0 ^ permalink raw reply [flat|nested] 24+ messages in thread
* [dpdk-dev] [PATCH v3 3/7] net/mlx5: added support for yellow policy rules 2021-07-21 8:54 ` [dpdk-dev] [PATCH v3 0/7] support yellow color policy in mlx5 Bing Zhao 2021-07-21 8:54 ` [dpdk-dev] [PATCH v3 1/7] net/mlx5: handle yellow case in default meter policy Bing Zhao 2021-07-21 8:54 ` [dpdk-dev] [PATCH v3 2/7] net/mlx5: enable meter bucket overflow for yellow color Bing Zhao @ 2021-07-21 8:54 ` Bing Zhao 2021-07-21 8:54 ` [dpdk-dev] [PATCH v3 4/7] net/mlx5: split policies handling of colors Bing Zhao ` (4 subsequent siblings) 7 siblings, 0 replies; 24+ messages in thread From: Bing Zhao @ 2021-07-21 8:54 UTC (permalink / raw) To: viacheslavo, matan; +Cc: dev, orika, rasland, thomas, lizh, shunh 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 <bingz@nvidia.com> --- 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 cfc646c5e5..2400565232 100644 --- a/drivers/net/mlx5/mlx5_flow_dv.c +++ b/drivers/net/mlx5/mlx5_flow_dv.c @@ -15214,7 +15214,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; @@ -15234,21 +15234,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: { @@ -15456,7 +15455,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, @@ -15500,6 +15499,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; @@ -15615,6 +15618,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; } @@ -16156,16 +16160,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 }; @@ -16199,13 +16202,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) { @@ -16220,7 +16222,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; @@ -16234,15 +16236,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: @@ -16284,7 +16286,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; @@ -16321,8 +16323,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.27.0 ^ permalink raw reply [flat|nested] 24+ messages in thread
* [dpdk-dev] [PATCH v3 4/7] net/mlx5: split policies handling of colors 2021-07-21 8:54 ` [dpdk-dev] [PATCH v3 0/7] support yellow color policy in mlx5 Bing Zhao ` (2 preceding siblings ...) 2021-07-21 8:54 ` [dpdk-dev] [PATCH v3 3/7] net/mlx5: added support for yellow policy rules Bing Zhao @ 2021-07-21 8:54 ` Bing Zhao 2021-07-21 8:54 ` [dpdk-dev] [PATCH v3 5/7] net/mlx5: support yellow in meter policy validation Bing Zhao ` (3 subsequent siblings) 7 siblings, 0 replies; 24+ messages in thread From: Bing Zhao @ 2021-07-21 8:54 UTC (permalink / raw) To: viacheslavo, matan; +Cc: dev, orika, rasland, thomas, lizh, shunh If the fate action is either RSS or Queue of a meter policy, the action will only be created in the flow splitting stage. With queue as the fate action, only one sub-policy is needed. And RSS will have more than one sub-policies if there is an expansion. Since the RSS parameters are the same for both green and yellow colors except the queues, the expansion result will be unique. Even if only one color has the RSS action, the checking and possible expansion will be done then. For each sub-policy, the action rules need to be created separately on its own policy table. Signed-off-by: Bing Zhao <bingz@nvidia.com> --- drivers/net/mlx5/mlx5_flow.c | 40 ++++++++++---------- drivers/net/mlx5/mlx5_flow_dv.c | 67 +++++++++++++++++---------------- 2 files changed, 55 insertions(+), 52 deletions(-) diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c index 347e8c1a09..d90c8cd314 100644 --- a/drivers/net/mlx5/mlx5_flow.c +++ b/drivers/net/mlx5/mlx5_flow.c @@ -4687,7 +4687,7 @@ get_meter_sub_policy(struct rte_eth_dev *dev, struct mlx5_flow_rss_desc *rss_desc[MLX5_MTR_RTE_COLORS] = {0}; uint32_t i; - /** + /* * This is a tmp dev_flow, * no need to register any matcher for it in translate. */ @@ -4695,18 +4695,19 @@ get_meter_sub_policy(struct rte_eth_dev *dev, for (i = 0; i < MLX5_MTR_RTE_COLORS; i++) { struct mlx5_flow dev_flow = {0}; struct mlx5_flow_handle dev_handle = { {0} }; + uint8_t fate = final_policy->act_cnt[i].fate_action; - if (final_policy->is_rss) { + if (fate == MLX5_FLOW_FATE_SHARED_RSS) { const void *rss_act = final_policy->act_cnt[i].rss->conf; struct rte_flow_action rss_actions[2] = { [0] = { .type = RTE_FLOW_ACTION_TYPE_RSS, - .conf = rss_act + .conf = rss_act, }, [1] = { .type = RTE_FLOW_ACTION_TYPE_END, - .conf = NULL + .conf = NULL, } }; @@ -4731,9 +4732,10 @@ get_meter_sub_policy(struct rte_eth_dev *dev, rss_desc_v[i].hash_fields ? rss_desc_v[i].queue_num : 1; rss_desc_v[i].tunnel = - !!(dev_flow.handle->layers & - MLX5_FLOW_LAYER_TUNNEL); - } else { + !!(dev_flow.handle->layers & + MLX5_FLOW_LAYER_TUNNEL); + rss_desc[i] = &rss_desc_v[i]; + } else if (fate == MLX5_FLOW_FATE_QUEUE) { /* This is queue action. */ rss_desc_v[i] = wks->rss_desc; rss_desc_v[i].key_len = 0; @@ -4741,24 +4743,24 @@ get_meter_sub_policy(struct rte_eth_dev *dev, rss_desc_v[i].queue = &final_policy->act_cnt[i].queue; rss_desc_v[i].queue_num = 1; + rss_desc[i] = &rss_desc_v[i]; + } else { + rss_desc[i] = NULL; } - rss_desc[i] = &rss_desc_v[i]; } sub_policy = flow_drv_meter_sub_policy_rss_prepare(dev, flow, policy, rss_desc); } else { enum mlx5_meter_domain mtr_domain = attr->transfer ? MLX5_MTR_DOMAIN_TRANSFER : - attr->egress ? MLX5_MTR_DOMAIN_EGRESS : - MLX5_MTR_DOMAIN_INGRESS; + (attr->egress ? MLX5_MTR_DOMAIN_EGRESS : + MLX5_MTR_DOMAIN_INGRESS); sub_policy = policy->sub_policys[mtr_domain][0]; } - if (!sub_policy) { + if (!sub_policy) rte_flow_error_set(error, EINVAL, - RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL, - "Failed to get meter sub-policy."); - goto exit; - } + RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL, + "Failed to get meter sub-policy."); exit: return sub_policy; } @@ -4956,8 +4958,8 @@ flow_meter_split_prep(struct rte_eth_dev *dev, } else { enum mlx5_meter_domain mtr_domain = attr->transfer ? MLX5_MTR_DOMAIN_TRANSFER : - attr->egress ? MLX5_MTR_DOMAIN_EGRESS : - MLX5_MTR_DOMAIN_INGRESS; + (attr->egress ? MLX5_MTR_DOMAIN_EGRESS : + MLX5_MTR_DOMAIN_INGRESS); sub_policy = &priv->sh->mtrmng->def_policy[mtr_domain]->sub_policy; @@ -4973,8 +4975,8 @@ flow_meter_split_prep(struct rte_eth_dev *dev, actions_pre++; if (!tag_action) return rte_flow_error_set(error, ENOMEM, - RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL, - "No tag action space."); + RTE_FLOW_ERROR_TYPE_UNSPECIFIED, + NULL, "No tag action space."); if (!mtr_flow_id) { tag_action->type = RTE_FLOW_ACTION_TYPE_VOID; goto exit; diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c index 2400565232..ee593a7001 100644 --- a/drivers/net/mlx5/mlx5_flow_dv.c +++ b/drivers/net/mlx5/mlx5_flow_dv.c @@ -15070,11 +15070,11 @@ __flow_dv_destroy_sub_policy_rules(struct rte_eth_dev *dev, next_port, tmp) { claim_zero(mlx5_flow_os_destroy_flow(color_rule->rule)); tbl = container_of(color_rule->matcher->tbl, - typeof(*tbl), tbl); + typeof(*tbl), tbl); mlx5_list_unregister(tbl->matchers, - &color_rule->matcher->entry); + &color_rule->matcher->entry); TAILQ_REMOVE(&sub_policy->color_rules[i], - color_rule, next_port); + color_rule, next_port); mlx5_free(color_rule); if (next_fm) mlx5_flow_meter_detach(priv, next_fm); @@ -15088,13 +15088,13 @@ __flow_dv_destroy_sub_policy_rules(struct rte_eth_dev *dev, } if (sub_policy->jump_tbl[i]) { flow_dv_tbl_resource_release(MLX5_SH(dev), - sub_policy->jump_tbl[i]); + sub_policy->jump_tbl[i]); sub_policy->jump_tbl[i] = NULL; } } if (sub_policy->tbl_rsc) { flow_dv_tbl_resource_release(MLX5_SH(dev), - sub_policy->tbl_rsc); + sub_policy->tbl_rsc); sub_policy->tbl_rsc = NULL; } } @@ -15111,7 +15111,7 @@ __flow_dv_destroy_sub_policy_rules(struct rte_eth_dev *dev, */ static void flow_dv_destroy_policy_rules(struct rte_eth_dev *dev, - struct mlx5_flow_meter_policy *mtr_policy) + struct mlx5_flow_meter_policy *mtr_policy) { uint32_t i, j; struct mlx5_flow_meter_sub_policy *sub_policy; @@ -15124,8 +15124,8 @@ flow_dv_destroy_policy_rules(struct rte_eth_dev *dev, for (j = 0; j < sub_policy_num; j++) { sub_policy = mtr_policy->sub_policys[i][j]; if (sub_policy) - __flow_dv_destroy_sub_policy_rules - (dev, sub_policy); + __flow_dv_destroy_sub_policy_rules(dev, + sub_policy); } } } @@ -16158,6 +16158,7 @@ __flow_dv_create_policy_acts_rules(struct rte_eth_dev *dev, bool match_src_port = false; int i; + /* If RSS or Queue, no previous actions / rules is created. */ for (i = 0; i < RTE_COLORS; i++) { acts[i].actions_n = 0; if (i == RTE_COLOR_RED) { @@ -16657,37 +16658,36 @@ __flow_dv_meter_get_rss_sub_policy(struct rte_eth_dev *dev, sub_policy_num = (mtr_policy->sub_policy_num >> (MLX5_MTR_SUB_POLICY_NUM_SHIFT * domain)) & MLX5_MTR_SUB_POLICY_NUM_MASK; - for (i = 0; i < sub_policy_num; - i++) { - for (j = 0; j < MLX5_MTR_RTE_COLORS; j++) { - if (rss_desc[j] && - hrxq_idx[j] != - mtr_policy->sub_policys[domain][i]->rix_hrxq[j]) + for (j = 0; j < sub_policy_num; j++) { + for (i = 0; i < MLX5_MTR_RTE_COLORS; i++) { + if (rss_desc[i] && + hrxq_idx[i] != + mtr_policy->sub_policys[domain][j]->rix_hrxq[i]) break; } - if (j >= MLX5_MTR_RTE_COLORS) { + if (i >= MLX5_MTR_RTE_COLORS) { /* * Found the sub policy table with - * the same queue per color + * the same queue per color. */ rte_spinlock_unlock(&mtr_policy->sl); - for (j = 0; j < MLX5_MTR_RTE_COLORS; j++) - mlx5_hrxq_release(dev, hrxq_idx[j]); + for (i = 0; i < MLX5_MTR_RTE_COLORS; i++) + mlx5_hrxq_release(dev, hrxq_idx[i]); *is_reuse = true; - return mtr_policy->sub_policys[domain][i]; + return mtr_policy->sub_policys[domain][j]; } } /* Create sub policy. */ if (!mtr_policy->sub_policys[domain][0]->rix_hrxq[0]) { - /* Reuse the first dummy sub_policy*/ + /* Reuse the first pre-allocated sub_policy. */ sub_policy = mtr_policy->sub_policys[domain][0]; sub_policy_idx = sub_policy->idx; } else { sub_policy = mlx5_ipool_zmalloc (priv->sh->ipool[MLX5_IPOOL_MTR_POLICY], - &sub_policy_idx); + &sub_policy_idx); if (!sub_policy || - sub_policy_idx > MLX5_MAX_SUB_POLICY_TBL_NUM) { + sub_policy_idx > MLX5_MAX_SUB_POLICY_TBL_NUM) { for (i = 0; i < MLX5_MTR_RTE_COLORS; i++) mlx5_hrxq_release(dev, hrxq_idx[i]); goto rss_sub_policy_error; @@ -16709,9 +16709,9 @@ __flow_dv_meter_get_rss_sub_policy(struct rte_eth_dev *dev, * RSS action to Queue action. */ hrxq = mlx5_ipool_get(priv->sh->ipool[MLX5_IPOOL_HRXQ], - hrxq_idx[i]); + hrxq_idx[i]); if (!hrxq) { - DRV_LOG(ERR, "Failed to create policy hrxq"); + DRV_LOG(ERR, "Failed to get policy hrxq"); goto rss_sub_policy_error; } act_cnt = &mtr_policy->act_cnt[i]; @@ -16726,19 +16726,21 @@ __flow_dv_meter_get_rss_sub_policy(struct rte_eth_dev *dev, } } if (__flow_dv_create_policy_acts_rules(dev, mtr_policy, - sub_policy, domain)) { + sub_policy, domain)) { DRV_LOG(ERR, "Failed to create policy " - "rules per domain."); + "rules for ingress domain."); goto rss_sub_policy_error; } if (sub_policy != mtr_policy->sub_policys[domain][0]) { i = (mtr_policy->sub_policy_num >> (MLX5_MTR_SUB_POLICY_NUM_SHIFT * domain)) & MLX5_MTR_SUB_POLICY_NUM_MASK; + if (i >= MLX5_MTR_RSS_MAX_SUB_POLICY) { + DRV_LOG(ERR, "No free sub-policy slot."); + goto rss_sub_policy_error; + } mtr_policy->sub_policys[domain][i] = sub_policy; i++; - if (i > MLX5_MTR_RSS_MAX_SUB_POLICY) - goto rss_sub_policy_error; mtr_policy->sub_policy_num &= ~(MLX5_MTR_SUB_POLICY_NUM_MASK << (MLX5_MTR_SUB_POLICY_NUM_SHIFT * domain)); mtr_policy->sub_policy_num |= @@ -16756,8 +16758,7 @@ __flow_dv_meter_get_rss_sub_policy(struct rte_eth_dev *dev, (MLX5_MTR_SUB_POLICY_NUM_SHIFT * domain)) & MLX5_MTR_SUB_POLICY_NUM_MASK; mtr_policy->sub_policys[domain][i] = NULL; - mlx5_ipool_free - (priv->sh->ipool[MLX5_IPOOL_MTR_POLICY], + mlx5_ipool_free(priv->sh->ipool[MLX5_IPOOL_MTR_POLICY], sub_policy->idx); } } @@ -16818,7 +16819,7 @@ flow_dv_meter_sub_policy_rss_prepare(struct rte_eth_dev *dev, while (i) { /** * From last policy to the first one in hierarchy, - * create/get the sub policy for each of them. + * create / get the sub policy for each of them. */ sub_policy = __flow_dv_meter_get_rss_sub_policy(dev, policies[--i], @@ -17022,7 +17023,7 @@ flow_dv_meter_hierarchy_rule_create(struct rte_eth_dev *dev, */ static void flow_dv_destroy_sub_policy_with_rxq(struct rte_eth_dev *dev, - struct mlx5_flow_meter_policy *mtr_policy) + struct mlx5_flow_meter_policy *mtr_policy) { struct mlx5_priv *priv = dev->data->dev_private; struct mlx5_flow_meter_sub_policy *sub_policy = NULL; @@ -17068,7 +17069,7 @@ flow_dv_destroy_sub_policy_with_rxq(struct rte_eth_dev *dev, case MLX5_FLOW_FATE_QUEUE: sub_policy = mtr_policy->sub_policys[domain][0]; __flow_dv_destroy_sub_policy_rules(dev, - sub_policy); + sub_policy); break; default: /*Other actions without queue and do nothing*/ -- 2.27.0 ^ permalink raw reply [flat|nested] 24+ messages in thread
* [dpdk-dev] [PATCH v3 5/7] net/mlx5: support yellow in meter policy validation 2021-07-21 8:54 ` [dpdk-dev] [PATCH v3 0/7] support yellow color policy in mlx5 Bing Zhao ` (3 preceding siblings ...) 2021-07-21 8:54 ` [dpdk-dev] [PATCH v3 4/7] net/mlx5: split policies handling of colors Bing Zhao @ 2021-07-21 8:54 ` Bing Zhao 2021-07-21 8:54 ` [dpdk-dev] [PATCH v3 6/7] net/mlx5: check consistency of meter policy and profile Bing Zhao ` (2 subsequent siblings) 7 siblings, 0 replies; 24+ messages in thread From: Bing Zhao @ 2021-07-21 8:54 UTC (permalink / raw) To: viacheslavo, matan; +Cc: dev, orika, rasland, thomas, lizh, shunh In the previous implementation, the policy for yellow color was not supported. The action validation for yellow was skipped. Since the yellow color policy needs to be supported, the validation should also be done for the yellow color. In the meanwhile, due to the fact that color policies of one meter should be used for the same flow(s), the domains supported of both colors should be the same. If both of the colors have RSS as the termination actions, except the queues, all other parameters of RSS should be the same. Signed-off-by: Bing Zhao <bingz@nvidia.com> --- doc/guides/nics/mlx5.rst | 9 +- doc/guides/rel_notes/release_21_08.rst | 1 + drivers/net/mlx5/mlx5.h | 8 +- drivers/net/mlx5/mlx5_flow.c | 6 +- drivers/net/mlx5/mlx5_flow.h | 4 +- drivers/net/mlx5/mlx5_flow_dv.c | 210 ++++++++++++++++--------- drivers/net/mlx5/mlx5_flow_meter.c | 15 +- 7 files changed, 160 insertions(+), 93 deletions(-) diff --git a/doc/guides/nics/mlx5.rst b/doc/guides/nics/mlx5.rst index f5b727c1ee..1f5b6fb954 100644 --- a/doc/guides/nics/mlx5.rst +++ b/doc/guides/nics/mlx5.rst @@ -420,19 +420,18 @@ Limitations - Meter: - All the meter colors with drop action will be counted only by the global drop statistics. - - Green color is not supported with drop action. - - Yellow detection is not supported. + - Yellow detection is only supported with ASO metering. - Red color must be with drop action. - Meter statistics are supported only for drop case. - - Meter yellow color detection is not supported. - A meter action created with pre-defined policy must be the last action in the flow except single case where the policy actions are: - green: NULL or END. - yellow: NULL or END. - RED: DROP / END. - The only supported meter policy actions: - - green: QUEUE, RSS, PORT_ID, JUMP, MARK and SET_TAG. - - yellow: must be empty. + - green: QUEUE, RSS, PORT_ID, JUMP, DROP, MARK and SET_TAG. + - yellow: QUEUE, RSS, PORT_ID, JUMP, DROP, MARK and SET_TAG. - RED: must be DROP. + - Policy actions of RSS for green and yellow should have the same configuration except queues. - meter profile packet mode is supported. - Integrity: diff --git a/doc/guides/rel_notes/release_21_08.rst b/doc/guides/rel_notes/release_21_08.rst index 1b38b1aa51..03d4fd059a 100644 --- a/doc/guides/rel_notes/release_21_08.rst +++ b/doc/guides/rel_notes/release_21_08.rst @@ -91,6 +91,7 @@ New Features * Added matching on IPv4 Internet Header Length (IHL). * Added support for matching on VXLAN header last 8-bits reserved field. * Optimized multi-thread flow rule insertion rate. + * Added support for metering policy actions of yellow color. * **Added Wangxun ngbe PMD.** diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h index a2fe9b90c7..ea16109972 100644 --- a/drivers/net/mlx5/mlx5.h +++ b/drivers/net/mlx5/mlx5.h @@ -629,7 +629,7 @@ struct mlx5_dev_shared_port { */ #define MLX5_DV_MAX_NUMBER_OF_ACTIONS 8 -/*ASO flow meter structures*/ +/* ASO flow meter structures */ /* Modify this value if enum rte_mtr_color changes. */ #define RTE_MTR_DROPPED RTE_COLORS /* Yellow is now supported. */ @@ -643,6 +643,12 @@ struct mlx5_dev_shared_port { #define MLX5_MTR_TABLE_ID_DROP 2 /* Priority of the meter policy matcher. */ #define MLX5_MTR_POLICY_MATCHER_PRIO 0 +/* Default policy. */ +#define MLX5_MTR_POLICY_MODE_DEF 1 +/* Only green color valid. */ +#define MLX5_MTR_POLICY_MODE_OG 2 +/* Only yellow color valid. */ +#define MLX5_MTR_POLICY_MODE_OY 3 enum mlx5_meter_domain { MLX5_MTR_DOMAIN_INGRESS, diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c index d90c8cd314..549b3058c2 100644 --- a/drivers/net/mlx5/mlx5_flow.c +++ b/drivers/net/mlx5/mlx5_flow.c @@ -7199,14 +7199,14 @@ mlx5_flow_validate_mtr_acts(struct rte_eth_dev *dev, struct rte_flow_attr *attr, bool *is_rss, uint8_t *domain_bitmap, - bool *is_def_policy, + uint8_t *policy_mode, struct rte_mtr_error *error) { const struct mlx5_flow_driver_ops *fops; fops = flow_get_drv_ops(MLX5_FLOW_TYPE_DV); - return fops->validate_mtr_acts(dev, actions, attr, - is_rss, domain_bitmap, is_def_policy, error); + return fops->validate_mtr_acts(dev, actions, attr, is_rss, + domain_bitmap, policy_mode, error); } /** diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h index 8f0521aa72..3724293d26 100644 --- a/drivers/net/mlx5/mlx5_flow.h +++ b/drivers/net/mlx5/mlx5_flow.h @@ -1209,7 +1209,7 @@ typedef int (*mlx5_flow_validate_mtr_acts_t) struct rte_flow_attr *attr, bool *is_rss, uint8_t *domain_bitmap, - bool *is_def_policy, + uint8_t *policy_mode, struct rte_mtr_error *error); typedef int (*mlx5_flow_create_mtr_acts_t) (struct rte_eth_dev *dev, @@ -1690,7 +1690,7 @@ int mlx5_flow_validate_mtr_acts(struct rte_eth_dev *dev, struct rte_flow_attr *attr, bool *is_rss, uint8_t *domain_bitmap, - bool *is_def_policy, + uint8_t *policy_mode, struct rte_mtr_error *error); void mlx5_flow_destroy_mtr_acts(struct rte_eth_dev *dev, struct mlx5_flow_meter_policy *mtr_policy); diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c index ee593a7001..97e297d5c2 100644 --- a/drivers/net/mlx5/mlx5_flow_dv.c +++ b/drivers/net/mlx5/mlx5_flow_dv.c @@ -17358,6 +17358,31 @@ flow_dv_action_validate(struct rte_eth_dev *dev, } } +/* + * Check if the RSS configurations for colors of a meter policy match + * each other, except the queues. + * + * @param[in] r1 + * Pointer to the first RSS flow action. + * @param[in] r2 + * Pointer to the second RSS flow action. + * + * @return + * 0 on match, 1 on conflict. + */ +static inline int +flow_dv_mtr_policy_rss_compare(const struct rte_flow_action_rss *r1, + const struct rte_flow_action_rss *r2) +{ + if (!r1 || !r2) + return 0; + if (r1->func != r2->func || r1->level != r2->level || + r1->types != r2->types || r1->key_len != r2->key_len || + memcmp(r1->key, r2->key, r1->key_len)) + return 1; + return 0; +} + /** * Validate the meter hierarchy chain for meter policy. * @@ -17453,13 +17478,13 @@ flow_dv_validate_mtr_policy_acts(struct rte_eth_dev *dev, struct rte_flow_attr *attr, bool *is_rss, uint8_t *domain_bitmap, - bool *is_def_policy, + uint8_t *policy_mode, struct rte_mtr_error *error) { struct mlx5_priv *priv = dev->data->dev_private; struct mlx5_dev_config *dev_conf = &priv->config; const struct rte_flow_action *act; - uint64_t action_flags = 0; + uint64_t action_flags[RTE_COLORS] = {0}; int actions_n; int i, ret; struct rte_flow_error flow_err; @@ -17467,36 +17492,45 @@ flow_dv_validate_mtr_policy_acts(struct rte_eth_dev *dev, uint8_t def_domain = MLX5_MTR_ALL_DOMAIN_BIT; uint8_t hierarchy_domain = 0; const struct rte_flow_action_meter *mtr; + bool def_green = false; + bool def_yellow = false; + const struct rte_flow_action_rss *rss_color[RTE_COLORS] = {NULL}; if (!priv->config.dv_esw_en) def_domain &= ~MLX5_MTR_DOMAIN_TRANSFER_BIT; *domain_bitmap = def_domain; - if (actions[RTE_COLOR_YELLOW] && - actions[RTE_COLOR_YELLOW]->type != RTE_FLOW_ACTION_TYPE_END) - return -rte_mtr_error_set(error, ENOTSUP, - RTE_MTR_ERROR_TYPE_METER_POLICY, - NULL, - "Yellow color does not support any action."); - if (actions[RTE_COLOR_YELLOW] && - actions[RTE_COLOR_YELLOW]->type != RTE_FLOW_ACTION_TYPE_DROP) + /* Red color could only support DROP action. */ + if (!actions[RTE_COLOR_RED] || + actions[RTE_COLOR_RED]->type != RTE_FLOW_ACTION_TYPE_DROP) return -rte_mtr_error_set(error, ENOTSUP, RTE_MTR_ERROR_TYPE_METER_POLICY, NULL, "Red color only supports drop action."); /* * Check default policy actions: - * Green/Yellow: no action, Red: drop action + * Green / Yellow: no action, Red: drop action + * Either G or Y will trigger default policy actions to be created. */ - if ((!actions[RTE_COLOR_GREEN] || - actions[RTE_COLOR_GREEN]->type == RTE_FLOW_ACTION_TYPE_END)) { - *is_def_policy = true; + if (!actions[RTE_COLOR_GREEN] || + actions[RTE_COLOR_GREEN]->type == RTE_FLOW_ACTION_TYPE_END) + def_green = true; + if (!actions[RTE_COLOR_YELLOW] || + actions[RTE_COLOR_YELLOW]->type == RTE_FLOW_ACTION_TYPE_END) + def_yellow = true; + if (def_green && def_yellow) { + *policy_mode = MLX5_MTR_POLICY_MODE_DEF; return 0; + } else if (!def_green && def_yellow) { + *policy_mode = MLX5_MTR_POLICY_MODE_OG; + } else if (def_green && !def_yellow) { + *policy_mode = MLX5_MTR_POLICY_MODE_OY; } - flow_err.message = NULL; + /* Set to empty string in case of NULL pointer access by user. */ + flow_err.message = ""; for (i = 0; i < RTE_COLORS; i++) { act = actions[i]; - for (action_flags = 0, actions_n = 0; - act && act->type != RTE_FLOW_ACTION_TYPE_END; - act++) { + for (action_flags[i] = 0, actions_n = 0; + act && act->type != RTE_FLOW_ACTION_TYPE_END; + act++) { if (actions_n == MLX5_DV_MAX_NUMBER_OF_ACTIONS) return -rte_mtr_error_set(error, ENOTSUP, RTE_MTR_ERROR_TYPE_METER_POLICY, @@ -17510,7 +17544,7 @@ flow_dv_validate_mtr_policy_acts(struct rte_eth_dev *dev, NULL, "PORT action validate check" " fail for ESW disable"); ret = flow_dv_validate_action_port_id(dev, - action_flags, + action_flags[i], act, attr, &flow_err); if (ret) return -rte_mtr_error_set(error, @@ -17520,11 +17554,11 @@ flow_dv_validate_mtr_policy_acts(struct rte_eth_dev *dev, flow_err.message : "PORT action validate check fail"); ++actions_n; - action_flags |= MLX5_FLOW_ACTION_PORT_ID; + action_flags[i] |= MLX5_FLOW_ACTION_PORT_ID; break; case RTE_FLOW_ACTION_TYPE_MARK: ret = flow_dv_validate_action_mark(dev, act, - action_flags, + action_flags[i], attr, &flow_err); if (ret < 0) return -rte_mtr_error_set(error, @@ -17541,12 +17575,12 @@ flow_dv_validate_mtr_policy_acts(struct rte_eth_dev *dev, NULL, "Extend MARK action is " "not supported. Please try use " "default policy for meter."); - action_flags |= MLX5_FLOW_ACTION_MARK; + action_flags[i] |= MLX5_FLOW_ACTION_MARK; ++actions_n; break; case RTE_FLOW_ACTION_TYPE_SET_TAG: ret = flow_dv_validate_action_set_tag(dev, - act, action_flags, + act, action_flags[i], attr, &flow_err); if (ret) return -rte_mtr_error_set(error, @@ -17555,19 +17589,12 @@ flow_dv_validate_mtr_policy_acts(struct rte_eth_dev *dev, NULL, flow_err.message ? flow_err.message : "Set tag action validate check fail"); - /* - * Count all modify-header actions - * as one action. - */ - if (!(action_flags & - MLX5_FLOW_MODIFY_HDR_ACTIONS)) - ++actions_n; - action_flags |= MLX5_FLOW_ACTION_SET_TAG; + action_flags[i] |= MLX5_FLOW_ACTION_SET_TAG; + ++actions_n; break; case RTE_FLOW_ACTION_TYPE_DROP: ret = mlx5_flow_validate_action_drop - (action_flags, - attr, &flow_err); + (action_flags[i], attr, &flow_err); if (ret < 0) return -rte_mtr_error_set(error, ENOTSUP, @@ -17575,7 +17602,7 @@ flow_dv_validate_mtr_policy_acts(struct rte_eth_dev *dev, NULL, flow_err.message ? flow_err.message : "Drop action validate check fail"); - action_flags |= MLX5_FLOW_ACTION_DROP; + action_flags[i] |= MLX5_FLOW_ACTION_DROP; ++actions_n; break; case RTE_FLOW_ACTION_TYPE_QUEUE: @@ -17584,9 +17611,9 @@ flow_dv_validate_mtr_policy_acts(struct rte_eth_dev *dev, * metadata feature is engaged. */ if (dev_conf->dv_flow_en && - (dev_conf->dv_xmeta_en != - MLX5_XMETA_MODE_LEGACY) && - mlx5_flow_ext_mreg_supported(dev)) + (dev_conf->dv_xmeta_en != + MLX5_XMETA_MODE_LEGACY) && + mlx5_flow_ext_mreg_supported(dev)) return -rte_mtr_error_set(error, ENOTSUP, RTE_MTR_ERROR_TYPE_METER_POLICY, @@ -17594,7 +17621,7 @@ flow_dv_validate_mtr_policy_acts(struct rte_eth_dev *dev, "is not supported. Please try use " "default policy for meter."); ret = mlx5_flow_validate_action_queue(act, - action_flags, dev, + action_flags[i], dev, attr, &flow_err); if (ret < 0) return -rte_mtr_error_set(error, @@ -17603,14 +17630,14 @@ flow_dv_validate_mtr_policy_acts(struct rte_eth_dev *dev, NULL, flow_err.message ? flow_err.message : "Queue action validate check fail"); - action_flags |= MLX5_FLOW_ACTION_QUEUE; + action_flags[i] |= MLX5_FLOW_ACTION_QUEUE; ++actions_n; break; case RTE_FLOW_ACTION_TYPE_RSS: if (dev_conf->dv_flow_en && - (dev_conf->dv_xmeta_en != - MLX5_XMETA_MODE_LEGACY) && - mlx5_flow_ext_mreg_supported(dev)) + (dev_conf->dv_xmeta_en != + MLX5_XMETA_MODE_LEGACY) && + mlx5_flow_ext_mreg_supported(dev)) return -rte_mtr_error_set(error, ENOTSUP, RTE_MTR_ERROR_TYPE_METER_POLICY, @@ -17618,7 +17645,7 @@ flow_dv_validate_mtr_policy_acts(struct rte_eth_dev *dev, "is not supported. Please try use " "default policy for meter."); ret = mlx5_validate_action_rss(dev, act, - &flow_err); + &flow_err); if (ret < 0) return -rte_mtr_error_set(error, ENOTSUP, @@ -17626,13 +17653,14 @@ flow_dv_validate_mtr_policy_acts(struct rte_eth_dev *dev, NULL, flow_err.message ? flow_err.message : "RSS action validate check fail"); - action_flags |= MLX5_FLOW_ACTION_RSS; + action_flags[i] |= MLX5_FLOW_ACTION_RSS; ++actions_n; - *is_rss = true; + /* Either G or Y will set the RSS. */ + rss_color[i] = act->conf; break; case RTE_FLOW_ACTION_TYPE_JUMP: ret = flow_dv_validate_action_jump(dev, - NULL, act, action_flags, + NULL, act, action_flags[i], attr, true, &flow_err); if (ret) return -rte_mtr_error_set(error, @@ -17642,8 +17670,13 @@ flow_dv_validate_mtr_policy_acts(struct rte_eth_dev *dev, flow_err.message : "Jump action validate check fail"); ++actions_n; - action_flags |= MLX5_FLOW_ACTION_JUMP; + action_flags[i] |= MLX5_FLOW_ACTION_JUMP; break; + /* + * Only the last meter in the hierarchy will support + * the YELLOW color steering. Then in the meter policy + * actions list, there should be no other meter inside. + */ case RTE_FLOW_ACTION_TYPE_METER: if (i != RTE_COLOR_GREEN) return -rte_mtr_error_set(error, @@ -17655,14 +17688,14 @@ flow_dv_validate_mtr_policy_acts(struct rte_eth_dev *dev, mtr = act->conf; ret = flow_dv_validate_policy_mtr_hierarchy(dev, mtr->mtr_id, - action_flags, + action_flags[i], is_rss, &hierarchy_domain, error); if (ret) return ret; ++actions_n; - action_flags |= + action_flags[i] |= MLX5_FLOW_ACTION_METER_WITH_TERMINATED_POLICY; break; default: @@ -17672,31 +17705,38 @@ flow_dv_validate_mtr_policy_acts(struct rte_eth_dev *dev, "Doesn't support optional action"); } } - /* Yellow is not supported, just skip. */ - if (i == RTE_COLOR_YELLOW) - continue; - if (action_flags & MLX5_FLOW_ACTION_PORT_ID) + if (action_flags[i] & MLX5_FLOW_ACTION_PORT_ID) domain_color[i] = MLX5_MTR_DOMAIN_TRANSFER_BIT; - else if ((action_flags & - (MLX5_FLOW_ACTION_RSS | MLX5_FLOW_ACTION_QUEUE)) || - (action_flags & MLX5_FLOW_ACTION_MARK)) + else if ((action_flags[i] & + (MLX5_FLOW_ACTION_RSS | MLX5_FLOW_ACTION_QUEUE)) || + (action_flags[i] & MLX5_FLOW_ACTION_MARK)) /* * Only support MLX5_XMETA_MODE_LEGACY - * so MARK action only in ingress domain. + * so MARK action is only in ingress domain. */ domain_color[i] = MLX5_MTR_DOMAIN_INGRESS_BIT; - else if (action_flags & - MLX5_FLOW_ACTION_METER_WITH_TERMINATED_POLICY) + else if (action_flags[i] & + MLX5_FLOW_ACTION_METER_WITH_TERMINATED_POLICY) domain_color[i] = hierarchy_domain; else domain_color[i] = def_domain; + /* + * Non-termination actions only support NIC Tx domain. + * The adjustion should be skipped when there is no + * action or only END is provided. The default domains + * bit-mask is set to find the MIN intersection. + * The action flags checking should also be skipped. + */ + if ((def_green && i == RTE_COLOR_GREEN) || + (def_yellow && i == RTE_COLOR_YELLOW)) + continue; /* * Validate the drop action mutual exclusion * with other actions. Drop action is mutually-exclusive * with any other action, except for Count action. */ - if ((action_flags & MLX5_FLOW_ACTION_DROP) && - (action_flags & ~MLX5_FLOW_ACTION_DROP)) { + if ((action_flags[i] & MLX5_FLOW_ACTION_DROP) && + (action_flags[i] & ~MLX5_FLOW_ACTION_DROP)) { return -rte_mtr_error_set(error, ENOTSUP, RTE_MTR_ERROR_TYPE_METER_POLICY, NULL, "Drop action is mutually-exclusive " @@ -17705,40 +17745,60 @@ flow_dv_validate_mtr_policy_acts(struct rte_eth_dev *dev, /* Eswitch has few restrictions on using items and actions */ if (domain_color[i] & MLX5_MTR_DOMAIN_TRANSFER_BIT) { if (!mlx5_flow_ext_mreg_supported(dev) && - action_flags & MLX5_FLOW_ACTION_MARK) + action_flags[i] & MLX5_FLOW_ACTION_MARK) return -rte_mtr_error_set(error, ENOTSUP, RTE_MTR_ERROR_TYPE_METER_POLICY, NULL, "unsupported action MARK"); - if (action_flags & MLX5_FLOW_ACTION_QUEUE) + if (action_flags[i] & MLX5_FLOW_ACTION_QUEUE) return -rte_mtr_error_set(error, ENOTSUP, RTE_MTR_ERROR_TYPE_METER_POLICY, NULL, "unsupported action QUEUE"); - if (action_flags & MLX5_FLOW_ACTION_RSS) + if (action_flags[i] & MLX5_FLOW_ACTION_RSS) return -rte_mtr_error_set(error, ENOTSUP, RTE_MTR_ERROR_TYPE_METER_POLICY, NULL, "unsupported action RSS"); - if (!(action_flags & MLX5_FLOW_FATE_ESWITCH_ACTIONS)) + if (!(action_flags[i] & MLX5_FLOW_FATE_ESWITCH_ACTIONS)) return -rte_mtr_error_set(error, ENOTSUP, RTE_MTR_ERROR_TYPE_METER_POLICY, NULL, "no fate action is found"); } else { - if (!(action_flags & MLX5_FLOW_FATE_ACTIONS) && - (domain_color[i] & - MLX5_MTR_DOMAIN_INGRESS_BIT)) { + if (!(action_flags[i] & MLX5_FLOW_FATE_ACTIONS) && + (domain_color[i] & MLX5_MTR_DOMAIN_INGRESS_BIT)) { if ((domain_color[i] & - MLX5_MTR_DOMAIN_EGRESS_BIT)) + MLX5_MTR_DOMAIN_EGRESS_BIT)) domain_color[i] = - MLX5_MTR_DOMAIN_EGRESS_BIT; + MLX5_MTR_DOMAIN_EGRESS_BIT; else return -rte_mtr_error_set(error, - ENOTSUP, - RTE_MTR_ERROR_TYPE_METER_POLICY, - NULL, "no fate action is found"); + ENOTSUP, + RTE_MTR_ERROR_TYPE_METER_POLICY, + NULL, + "no fate action is found"); } } - if (domain_color[i] != def_domain) - *domain_bitmap = domain_color[i]; } + /* If both colors have RSS, the attributes should be the same. */ + if (flow_dv_mtr_policy_rss_compare(rss_color[RTE_COLOR_GREEN], + rss_color[RTE_COLOR_YELLOW])) + return -rte_mtr_error_set(error, EINVAL, + RTE_MTR_ERROR_TYPE_METER_POLICY, + NULL, "policy RSS attr conflict"); + if (rss_color[RTE_COLOR_GREEN] || rss_color[RTE_COLOR_YELLOW]) + *is_rss = true; + /* "domain_color[C]" is non-zero for each color, default is ALL. */ + if (!def_green && !def_yellow && + domain_color[RTE_COLOR_GREEN] != domain_color[RTE_COLOR_YELLOW] && + !(action_flags[RTE_COLOR_GREEN] & MLX5_FLOW_ACTION_DROP) && + !(action_flags[RTE_COLOR_YELLOW] & MLX5_FLOW_ACTION_DROP)) + return -rte_mtr_error_set(error, EINVAL, + RTE_MTR_ERROR_TYPE_METER_POLICY, + NULL, "policy domains conflict"); + /* + * At least one color policy is listed in the actions, the domains + * to be supported should be the intersection. + */ + *domain_bitmap = domain_color[RTE_COLOR_GREEN] & + domain_color[RTE_COLOR_YELLOW]; return 0; } diff --git a/drivers/net/mlx5/mlx5_flow_meter.c b/drivers/net/mlx5/mlx5_flow_meter.c index 19b2665558..32ad4ea133 100644 --- a/drivers/net/mlx5/mlx5_flow_meter.c +++ b/drivers/net/mlx5/mlx5_flow_meter.c @@ -582,7 +582,7 @@ mlx5_flow_meter_policy_validate(struct rte_eth_dev *dev, struct rte_flow_attr attr = { .transfer = priv->config.dv_esw_en ? 1 : 0}; bool is_rss = false; - bool is_def_policy = false; + uint8_t policy_mode; uint8_t domain_bitmap; int ret; @@ -591,7 +591,7 @@ mlx5_flow_meter_policy_validate(struct rte_eth_dev *dev, RTE_MTR_ERROR_TYPE_METER_POLICY, NULL, "meter policy unsupported."); ret = mlx5_flow_validate_mtr_acts(dev, policy->actions, &attr, - &is_rss, &domain_bitmap, &is_def_policy, error); + &is_rss, &domain_bitmap, &policy_mode, error); if (ret) return ret; return 0; @@ -674,7 +674,7 @@ mlx5_flow_meter_policy_add(struct rte_eth_dev *dev, struct mlx5_flow_meter_policy *mtr_policy = NULL; struct mlx5_flow_meter_sub_policy *sub_policy; bool is_rss = false; - bool is_def_policy = false; + uint8_t policy_mode; uint32_t i; int ret; uint32_t policy_size = sizeof(struct mlx5_flow_meter_policy); @@ -701,14 +701,15 @@ mlx5_flow_meter_policy_add(struct rte_eth_dev *dev, 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); + &is_rss, &domain_bitmap, + &policy_mode, error); if (ret) return ret; if (!domain_bitmap) return -rte_mtr_error_set(error, ENOTSUP, - RTE_MTR_ERROR_TYPE_METER_POLICY, - NULL, "fail to find policy domain."); - if (is_def_policy) { + RTE_MTR_ERROR_TYPE_METER_POLICY, + NULL, "fail to find policy domain."); + if (policy_mode == MLX5_MTR_POLICY_MODE_DEF) { if (priv->sh->mtrmng->def_policy_id != MLX5_INVALID_POLICY_ID) return -rte_mtr_error_set(error, EEXIST, RTE_MTR_ERROR_TYPE_METER_POLICY_ID, -- 2.27.0 ^ permalink raw reply [flat|nested] 24+ messages in thread
* [dpdk-dev] [PATCH v3 6/7] net/mlx5: check consistency of meter policy and profile 2021-07-21 8:54 ` [dpdk-dev] [PATCH v3 0/7] support yellow color policy in mlx5 Bing Zhao ` (4 preceding siblings ...) 2021-07-21 8:54 ` [dpdk-dev] [PATCH v3 5/7] net/mlx5: support yellow in meter policy validation Bing Zhao @ 2021-07-21 8:54 ` Bing Zhao 2021-07-21 8:54 ` [dpdk-dev] [PATCH v3 7/7] net/mlx5: add meter support for trTCM profiles Bing Zhao 2021-07-22 11:28 ` [dpdk-dev] [PATCH v3 0/7] support yellow color policy in mlx5 Thomas Monjalon 7 siblings, 0 replies; 24+ messages in thread From: Bing Zhao @ 2021-07-21 8:54 UTC (permalink / raw) To: viacheslavo, matan; +Cc: dev, orika, rasland, thomas, lizh, shunh In the previous implementation, only green color policy was supported in mlx5 PMD. Since yellow color policy is supported now, the consistency of meter policy and profile should be checked. 1. If the profile supports yellow but the policy doesn't, an error should be returned when creating the meter. Or else, there is no explicit steering action for the packets marked with yellow. 2. If the policy supports yellow but the profile doesn't, it will be considered as a valid case. Even if no packet will be handled with the yellow steering action, it is just like that only the green policy presents. Usually the green color is supported by default, but when it is disabled intentionally with setting the CBS to a small value like zero in the profile, the similar checking on green policy and profile should also be done. Signed-off-by: Bing Zhao <bingz@nvidia.com> --- drivers/net/mlx5/mlx5.h | 6 ++++++ drivers/net/mlx5/mlx5_flow_dv.c | 4 ++++ drivers/net/mlx5/mlx5_flow_meter.c | 20 ++++++++++++++++++-- 3 files changed, 28 insertions(+), 2 deletions(-) diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h index ea16109972..3a8587b7cf 100644 --- a/drivers/net/mlx5/mlx5.h +++ b/drivers/net/mlx5/mlx5.h @@ -746,6 +746,10 @@ struct mlx5_flow_meter_policy { /* Is queue action in policy table. */ uint32_t is_hierarchy:1; /* Is meter action in policy table. */ + uint32_t skip_y:1; + /* If yellow color policy is skipped. */ + uint32_t skip_g:1; + /* If green color policy is skipped. */ rte_spinlock_t sl; uint32_t ref_cnt; /* Use count. */ @@ -866,6 +870,8 @@ struct mlx5_flow_meter_profile { /**< srtcm_rfc2697 struct. */ }; uint32_t ref_cnt; /**< Use count. */ + uint32_t g_support:1; /**< If G color will be generated. */ + uint32_t y_support:1; /**< If Y color will be generated. */ }; /* 2 meters in each ASO cache line */ diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c index 97e297d5c2..7ea04ba6e5 100644 --- a/drivers/net/mlx5/mlx5_flow_dv.c +++ b/drivers/net/mlx5/mlx5_flow_dv.c @@ -15245,6 +15245,10 @@ __flow_dv_create_domain_policy_acts(struct rte_eth_dev *dev, for (i = 0; i < RTE_COLORS; i++) { if (i < MLX5_MTR_RTE_COLORS) act_cnt = &mtr_policy->act_cnt[i]; + /* Skip the color policy actions creation. */ + if ((i == RTE_COLOR_YELLOW && mtr_policy->skip_y) || + (i == RTE_COLOR_GREEN && mtr_policy->skip_g)) + continue; action_flags = 0; for (act = actions[i]; act && act->type != RTE_FLOW_ACTION_TYPE_END; act++) { diff --git a/drivers/net/mlx5/mlx5_flow_meter.c b/drivers/net/mlx5/mlx5_flow_meter.c index 32ad4ea133..4f57b7e04e 100644 --- a/drivers/net/mlx5/mlx5_flow_meter.c +++ b/drivers/net/mlx5/mlx5_flow_meter.c @@ -333,6 +333,10 @@ mlx5_flow_meter_param_fill(struct mlx5_flow_meter_profile *fmp, ebs_exp = exp; srtcm->ebs_eir = rte_cpu_to_be_32(ebs_exp << ASO_DSEG_EBS_EXP_OFFSET | ebs_man << ASO_DSEG_EBS_MAN_OFFSET); + if (srtcm->cbs_cir) + fmp->g_support = 1; + if (srtcm->ebs_eir) + fmp->y_support = 1; return 0; } @@ -750,6 +754,10 @@ mlx5_flow_meter_policy_add(struct rte_eth_dev *dev, return -rte_mtr_error_set(error, ENOMEM, RTE_MTR_ERROR_TYPE_METER_POLICY, NULL, "Memory alloc failed for meter policy."); + if (policy_mode == MLX5_MTR_POLICY_MODE_OG) + mtr_policy->skip_y = 1; + else if (policy_mode == MLX5_MTR_POLICY_MODE_OY) + mtr_policy->skip_g = 1; policy_size = sizeof(struct mlx5_flow_meter_policy); for (i = 0; i < MLX5_MTR_DOMAIN_MAX; i++) { if (!(domain_bitmap & (1 << i))) @@ -1132,13 +1140,13 @@ mlx5_flow_meter_create(struct rte_eth_dev *dev, uint32_t meter_id, if (!priv->config.dv_esw_en) domain_bitmap &= ~MLX5_MTR_DOMAIN_TRANSFER_BIT; } else { - mtr_policy = mlx5_flow_meter_policy_find(dev, - params->meter_policy_id, &policy_idx); if (!priv->sh->meter_aso_en) return -rte_mtr_error_set(error, ENOTSUP, RTE_MTR_ERROR_TYPE_UNSPECIFIED, NULL, "Part of the policies cannot be " "supported without ASO "); + mtr_policy = mlx5_flow_meter_policy_find(dev, + params->meter_policy_id, &policy_idx); if (!mtr_policy) return -rte_mtr_error_set(error, ENOENT, RTE_MTR_ERROR_TYPE_METER_POLICY_ID, @@ -1149,6 +1157,14 @@ mlx5_flow_meter_create(struct rte_eth_dev *dev, uint32_t meter_id, MLX5_MTR_DOMAIN_EGRESS_BIT : 0) | (mtr_policy->transfer ? MLX5_MTR_DOMAIN_TRANSFER_BIT : 0); + if (fmp->g_support && mtr_policy->skip_g) + return -rte_mtr_error_set(error, ENOTSUP, + RTE_MTR_ERROR_TYPE_METER_POLICY_ID, + NULL, "Meter green policy is empty."); + if (fmp->y_support && mtr_policy->skip_y) + return -rte_mtr_error_set(error, ENOTSUP, + RTE_MTR_ERROR_TYPE_METER_POLICY_ID, + NULL, "Meter yellow policy is empty."); } /* Allocate the flow meter memory. */ if (priv->sh->meter_aso_en) { -- 2.27.0 ^ permalink raw reply [flat|nested] 24+ messages in thread
* [dpdk-dev] [PATCH v3 7/7] net/mlx5: add meter support for trTCM profiles 2021-07-21 8:54 ` [dpdk-dev] [PATCH v3 0/7] support yellow color policy in mlx5 Bing Zhao ` (5 preceding siblings ...) 2021-07-21 8:54 ` [dpdk-dev] [PATCH v3 6/7] net/mlx5: check consistency of meter policy and profile Bing Zhao @ 2021-07-21 8:54 ` Bing Zhao 2021-07-22 11:28 ` [dpdk-dev] [PATCH v3 0/7] support yellow color policy in mlx5 Thomas Monjalon 7 siblings, 0 replies; 24+ messages in thread From: Bing Zhao @ 2021-07-21 8:54 UTC (permalink / raw) To: viacheslavo, matan; +Cc: dev, orika, rasland, thomas, lizh, shunh The support of RFC2698 and RFC4115 are added in mlx5 PMD. Only the ASO metering supports these two profiles. Signed-off-by: Bing Zhao <bingz@nvidia.com> --- doc/guides/nics/mlx5.rst | 1 + doc/guides/rel_notes/release_21_08.rst | 1 + drivers/common/mlx5/mlx5_prm.h | 5 +- drivers/net/mlx5/mlx5_flow_aso.c | 23 ++++- drivers/net/mlx5/mlx5_flow_meter.c | 112 ++++++++++++++++--------- 5 files changed, 98 insertions(+), 44 deletions(-) diff --git a/doc/guides/nics/mlx5.rst b/doc/guides/nics/mlx5.rst index 1f5b6fb954..ae267d315f 100644 --- a/doc/guides/nics/mlx5.rst +++ b/doc/guides/nics/mlx5.rst @@ -433,6 +433,7 @@ Limitations - RED: must be DROP. - Policy actions of RSS for green and yellow should have the same configuration except queues. - meter profile packet mode is supported. + - meter profiles of RFC2697, RFC2698 and RFC4115 are supported. - Integrity: diff --git a/doc/guides/rel_notes/release_21_08.rst b/doc/guides/rel_notes/release_21_08.rst index 03d4fd059a..e159615deb 100644 --- a/doc/guides/rel_notes/release_21_08.rst +++ b/doc/guides/rel_notes/release_21_08.rst @@ -92,6 +92,7 @@ New Features * Added support for matching on VXLAN header last 8-bits reserved field. * Optimized multi-thread flow rule insertion rate. * Added support for metering policy actions of yellow color. + * Added support for metering trTCM RFC2698 and RFC4115. * **Added Wangxun ngbe PMD.** diff --git a/drivers/common/mlx5/mlx5_prm.h b/drivers/common/mlx5/mlx5_prm.h index 7950070976..88705be9d6 100644 --- a/drivers/common/mlx5/mlx5_prm.h +++ b/drivers/common/mlx5/mlx5_prm.h @@ -3031,11 +3031,12 @@ struct mlx5_aso_mtr_dseg { #define ASO_DSEG_VALID_OFFSET 31 #define ASO_DSEG_BO_OFFSET 30 #define ASO_DSEG_SC_OFFSET 28 +#define ASO_DSEG_BBOG_OFFSET 27 #define ASO_DSEG_MTR_MODE 24 #define ASO_DSEG_CBS_EXP_OFFSET 24 #define ASO_DSEG_CBS_MAN_OFFSET 16 -#define ASO_DSEG_CIR_EXP_MASK 0x1F -#define ASO_DSEG_CIR_EXP_OFFSET 8 +#define ASO_DSEG_XIR_EXP_MASK 0x1F +#define ASO_DSEG_XIR_EXP_OFFSET 8 #define ASO_DSEG_EBS_EXP_OFFSET 24 #define ASO_DSEG_EBS_MAN_OFFSET 16 #define ASO_DSEG_EXP_MASK 0x1F diff --git a/drivers/net/mlx5/mlx5_flow_aso.c b/drivers/net/mlx5/mlx5_flow_aso.c index 23e22e560a..e11327a11b 100644 --- a/drivers/net/mlx5/mlx5_flow_aso.c +++ b/drivers/net/mlx5/mlx5_flow_aso.c @@ -747,10 +747,27 @@ mlx5_aso_mtr_sq_enqueue_single(struct mlx5_aso_sq *sq, wqe->aso_dseg.mtrs[dseg_idx].v_bo_sc_bbog_mm = RTE_BE32((1 << ASO_DSEG_VALID_OFFSET) | (MLX5_FLOW_COLOR_GREEN << ASO_DSEG_SC_OFFSET)); - /* Only needed for RFC2697. */ - if (fm->profile->srtcm_prm.ebs_eir) + switch (fmp->profile.alg) { + case RTE_MTR_SRTCM_RFC2697: + /* Only needed for RFC2697. */ + if (fm->profile->srtcm_prm.ebs_eir) + wqe->aso_dseg.mtrs[dseg_idx].v_bo_sc_bbog_mm |= + RTE_BE32(1 << ASO_DSEG_BO_OFFSET); + break; + case RTE_MTR_TRTCM_RFC2698: wqe->aso_dseg.mtrs[dseg_idx].v_bo_sc_bbog_mm |= - RTE_BE32(1 << ASO_DSEG_BO_OFFSET); + RTE_BE32(1 << ASO_DSEG_BBOG_OFFSET); + break; + case RTE_MTR_TRTCM_RFC4115: + default: + break; + } + /* + * Note: + * Due to software performance reason, the token fields will not be + * set when posting the WQE to ASO SQ. It will be filled by the HW + * automatically. + */ sq->head++; sq->pi += 2;/* Each WQE contains 2 WQEBB's. */ rte_io_wmb(); diff --git a/drivers/net/mlx5/mlx5_flow_meter.c b/drivers/net/mlx5/mlx5_flow_meter.c index 4f57b7e04e..2d91a6fcf0 100644 --- a/drivers/net/mlx5/mlx5_flow_meter.c +++ b/drivers/net/mlx5/mlx5_flow_meter.c @@ -55,7 +55,7 @@ mlx5_flow_meter_action_create(struct mlx5_priv *priv, MLX5_SET(flow_meter_parameters, fmp, cbs_exponent, val); val = (cbs_cir >> ASO_DSEG_CBS_MAN_OFFSET) & ASO_DSEG_MAN_MASK; MLX5_SET(flow_meter_parameters, fmp, cbs_mantissa, val); - val = (cbs_cir >> ASO_DSEG_CIR_EXP_OFFSET) & ASO_DSEG_EXP_MASK; + val = (cbs_cir >> ASO_DSEG_XIR_EXP_OFFSET) & ASO_DSEG_EXP_MASK; MLX5_SET(flow_meter_parameters, fmp, cir_exponent, val); val = (cbs_cir & ASO_DSEG_MAN_MASK); MLX5_SET(flow_meter_parameters, fmp, cir_mantissa, val); @@ -194,18 +194,18 @@ mlx5_flow_meter_profile_validate(struct rte_eth_dev *dev, NULL, "Metering algorithm not supported."); } -/** - * Calculate mantissa and exponent for cir. +/* + * Calculate mantissa and exponent for cir / eir. * - * @param[in] cir + * @param[in] xir * Value to be calculated. * @param[out] man * Pointer to the mantissa. * @param[out] exp * Pointer to the exp. */ -static void -mlx5_flow_meter_cir_man_exp_calc(int64_t cir, uint8_t *man, uint8_t *exp) +static inline void +mlx5_flow_meter_xir_man_exp_calc(int64_t xir, uint8_t *man, uint8_t *exp) { int64_t _cir; int64_t delta = INT64_MAX; @@ -216,8 +216,8 @@ mlx5_flow_meter_cir_man_exp_calc(int64_t cir, uint8_t *man, uint8_t *exp) for (m = 0; m <= 0xFF; m++) { /* man width 8 bit */ for (e = 0; e <= 0x1F; e++) { /* exp width 5bit */ _cir = (1000000000ULL * m) >> e; - if (llabs(cir - _cir) <= delta) { - delta = llabs(cir - _cir); + if (llabs(xir - _cir) <= delta) { + delta = llabs(xir - _cir); _man = m; _exp = e; } @@ -227,7 +227,7 @@ mlx5_flow_meter_cir_man_exp_calc(int64_t cir, uint8_t *man, uint8_t *exp) *exp = _exp; } -/** +/* * Calculate mantissa and exponent for xbs. * * @param[in] xbs @@ -237,7 +237,7 @@ mlx5_flow_meter_cir_man_exp_calc(int64_t cir, uint8_t *man, uint8_t *exp) * @param[out] exp * Pointer to the exp. */ -static void +static inline void mlx5_flow_meter_xbs_man_exp_calc(uint64_t xbs, uint8_t *man, uint8_t *exp) { int _exp; @@ -275,37 +275,63 @@ mlx5_flow_meter_param_fill(struct mlx5_flow_meter_profile *fmp, struct mlx5_flow_meter_srtcm_rfc2697_prm *srtcm = &fmp->srtcm_prm; uint8_t man, exp; uint32_t cbs_exp, cbs_man, cir_exp, cir_man; - uint32_t ebs_exp, ebs_man; - uint64_t cir, cbs, ebs; + uint32_t eir_exp, eir_man, ebs_exp, ebs_man; + uint64_t cir, cbs, eir, ebs; - if (fmp->profile.alg != RTE_MTR_SRTCM_RFC2697) - return -rte_mtr_error_set(error, ENOTSUP, + if (!priv->sh->meter_aso_en) { + /* Legacy FW metering will only support srTCM. */ + if (fmp->profile.alg != RTE_MTR_SRTCM_RFC2697) + return -rte_mtr_error_set(error, ENOTSUP, RTE_MTR_ERROR_TYPE_METER_PROFILE, - NULL, "Metering algorithm not supported."); - if (!priv->sh->meter_aso_en && fmp->profile.packet_mode) - return -rte_mtr_error_set(error, ENOTSUP, - RTE_MTR_ERROR_TYPE_METER_PROFILE, - NULL, "Metering algorithm packet mode not supported."); - if (priv->sh->meter_aso_en && fmp->profile.packet_mode) { - cir = fmp->profile.srtcm_rfc2697.cir << - MLX5_MTRS_PPS_MAP_BPS_SHIFT; - cbs = fmp->profile.srtcm_rfc2697.cbs << - MLX5_MTRS_PPS_MAP_BPS_SHIFT; - ebs = fmp->profile.srtcm_rfc2697.ebs << - MLX5_MTRS_PPS_MAP_BPS_SHIFT; - } else { + NULL, "Metering algorithm is not supported."); + if (fmp->profile.packet_mode) + return -rte_mtr_error_set(error, ENOTSUP, + RTE_MTR_ERROR_TYPE_METER_PROFILE, NULL, + "Metering algorithm packet mode is not supported."); + } + switch (fmp->profile.alg) { + case RTE_MTR_SRTCM_RFC2697: cir = fmp->profile.srtcm_rfc2697.cir; cbs = fmp->profile.srtcm_rfc2697.cbs; + eir = 0; ebs = fmp->profile.srtcm_rfc2697.ebs; + break; + case RTE_MTR_TRTCM_RFC2698: + MLX5_ASSERT(fmp->profile.trtcm_rfc2698.pir > + fmp->profile.trtcm_rfc2698.cir && + fmp->profile.trtcm_rfc2698.pbs > + fmp->profile.trtcm_rfc2698.cbs); + cir = fmp->profile.trtcm_rfc2698.cir; + cbs = fmp->profile.trtcm_rfc2698.cbs; + /* EIR / EBS are filled with PIR / PBS. */ + eir = fmp->profile.trtcm_rfc2698.pir; + ebs = fmp->profile.trtcm_rfc2698.pbs; + break; + case RTE_MTR_TRTCM_RFC4115: + cir = fmp->profile.trtcm_rfc4115.cir; + cbs = fmp->profile.trtcm_rfc4115.cbs; + eir = fmp->profile.trtcm_rfc4115.eir; + ebs = fmp->profile.trtcm_rfc4115.ebs; + break; + default: + return -rte_mtr_error_set(error, EINVAL, + RTE_MTR_ERROR_TYPE_METER_PROFILE, NULL, + "Metering algorithm mode is invalid"); + } + /* Adjust the values for PPS mode. */ + if (fmp->profile.packet_mode) { + cir <<= MLX5_MTRS_PPS_MAP_BPS_SHIFT; + cbs <<= MLX5_MTRS_PPS_MAP_BPS_SHIFT; + eir <<= MLX5_MTRS_PPS_MAP_BPS_SHIFT; + ebs <<= MLX5_MTRS_PPS_MAP_BPS_SHIFT; } /* cir = 8G * cir_mantissa * 1/(2^cir_exponent)) Bytes/Sec */ - mlx5_flow_meter_cir_man_exp_calc(cir, &man, &exp); + mlx5_flow_meter_xir_man_exp_calc(cir, &man, &exp); /* Check if cir mantissa is too large. */ - if (exp > ASO_DSEG_CIR_EXP_MASK) + if (exp > ASO_DSEG_XIR_EXP_MASK) return -rte_mtr_error_set(error, ENOTSUP, RTE_MTR_ERROR_TYPE_MTR_PARAMS, NULL, - "meter profile parameter cir is" - " not supported."); + "meter profile parameter cir is not supported."); cir_man = man; cir_exp = exp; /* cbs = cbs_mantissa * 2^cbs_exponent */ @@ -314,25 +340,33 @@ mlx5_flow_meter_param_fill(struct mlx5_flow_meter_profile *fmp, if (exp > ASO_DSEG_EXP_MASK) return -rte_mtr_error_set(error, ENOTSUP, RTE_MTR_ERROR_TYPE_MTR_PARAMS, NULL, - "meter profile parameter cbs is" - " not supported."); + "meter profile parameter cbs is not supported."); cbs_man = man; cbs_exp = exp; srtcm->cbs_cir = rte_cpu_to_be_32(cbs_exp << ASO_DSEG_CBS_EXP_OFFSET | cbs_man << ASO_DSEG_CBS_MAN_OFFSET | - cir_exp << ASO_DSEG_CIR_EXP_OFFSET | + cir_exp << ASO_DSEG_XIR_EXP_OFFSET | cir_man); + mlx5_flow_meter_xir_man_exp_calc(eir, &man, &exp); + /* Check if eir mantissa is too large. */ + if (exp > ASO_DSEG_XIR_EXP_MASK) + return -rte_mtr_error_set(error, ENOTSUP, + RTE_MTR_ERROR_TYPE_MTR_PARAMS, NULL, + "meter profile parameter eir is not supported."); + eir_man = man; + eir_exp = exp; mlx5_flow_meter_xbs_man_exp_calc(ebs, &man, &exp); /* Check if ebs mantissa is too large. */ if (exp > ASO_DSEG_EXP_MASK) return -rte_mtr_error_set(error, ENOTSUP, RTE_MTR_ERROR_TYPE_MTR_PARAMS, NULL, - "meter profile parameter ebs is" - " not supported."); + "meter profile parameter ebs is not supported."); ebs_man = man; ebs_exp = exp; srtcm->ebs_eir = rte_cpu_to_be_32(ebs_exp << ASO_DSEG_EBS_EXP_OFFSET | - ebs_man << ASO_DSEG_EBS_MAN_OFFSET); + ebs_man << ASO_DSEG_EBS_MAN_OFFSET | + eir_exp << ASO_DSEG_XIR_EXP_OFFSET | + eir_man); if (srtcm->cbs_cir) fmp->g_support = 1; if (srtcm->ebs_eir) @@ -1008,7 +1042,7 @@ mlx5_flow_meter_action_modify(struct mlx5_priv *priv, cbs_mantissa, val); } if (modify_bits & MLX5_FLOW_METER_OBJ_MODIFY_FIELD_CIR) { - val = (cbs_cir >> ASO_DSEG_CIR_EXP_OFFSET) & + val = (cbs_cir >> ASO_DSEG_XIR_EXP_OFFSET) & ASO_DSEG_EXP_MASK; MLX5_SET(flow_meter_parameters, attr, cir_exponent, val); @@ -1389,7 +1423,7 @@ mlx5_flow_meter_modify_state(struct mlx5_priv *priv, &srtcm, modify_bits, 0, 0); else ret = mlx5_flow_meter_action_modify(priv, fm, - &fm->profile->srtcm_prm, + &fm->profile->srtcm_prm, modify_bits, 0, 1); if (ret) return -rte_mtr_error_set(error, -ret, -- 2.27.0 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [dpdk-dev] [PATCH v3 0/7] support yellow color policy in mlx5 2021-07-21 8:54 ` [dpdk-dev] [PATCH v3 0/7] support yellow color policy in mlx5 Bing Zhao ` (6 preceding siblings ...) 2021-07-21 8:54 ` [dpdk-dev] [PATCH v3 7/7] net/mlx5: add meter support for trTCM profiles Bing Zhao @ 2021-07-22 11:28 ` Thomas Monjalon 7 siblings, 0 replies; 24+ messages in thread From: Thomas Monjalon @ 2021-07-22 11:28 UTC (permalink / raw) To: Bing Zhao; +Cc: viacheslavo, matan, dev, orika, rasland, lizh, shunh, asafp 21/07/2021 10:54, Bing Zhao: > When creating a meter policy, the actions for yellow color can be > specified together with green color. The mlx5 PMD now supports to > set the policy actions for yellow color. > > The actions list that is supported for yellow is the same as that > for green. > > Acked-by: Matan Azrad <matan@nvidia.com> > > Bing Zhao (7): > net/mlx5: handle yellow case in default meter policy > net/mlx5: enable meter bucket overflow for yellow color > net/mlx5: added support for yellow policy rules > net/mlx5: split policies handling of colors > net/mlx5: support yellow in meter policy validation > net/mlx5: check consistency of meter policy and profile > net/mlx5: add meter support for trTCM profiles Applied, thanks ^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2021-07-22 11:28 UTC | newest] Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-07-05 15:57 [dpdk-dev] [PATCH 0/6] support yellow color policy in mlx5 Bing Zhao 2021-07-05 15:57 ` [dpdk-dev] [PATCH 1/6] net/mlx5: add yellow color default policy Bing Zhao 2021-07-05 15:57 ` [dpdk-dev] [PATCH 2/6] net/mlx5: support yellow in meter policy validation Bing Zhao 2021-07-05 15:57 ` [dpdk-dev] [PATCH 3/6] net/mlx5: enable meter bucket overflow for yellow color Bing Zhao 2021-07-05 15:57 ` [dpdk-dev] [PATCH 4/6] net/mlx5: added support for yellow policy rules Bing Zhao 2021-07-05 15:57 ` [dpdk-dev] [PATCH 5/6] net/mlx5: split policies handling of colors Bing Zhao 2021-07-05 15:57 ` [dpdk-dev] [PATCH 6/6] doc: update mlx5 metering policy part Bing Zhao 2021-07-18 17:18 ` [dpdk-dev] [PATCH v2 0/7] support yellow color policy in mlx5 Bing Zhao 2021-07-18 17:18 ` [dpdk-dev] [PATCH v2 1/7] net/mlx5: handle yellow case in default meter policy Bing Zhao 2021-07-18 17:18 ` [dpdk-dev] [PATCH v2 2/7] net/mlx5: enable meter bucket overflow for yellow color Bing Zhao 2021-07-18 17:18 ` [dpdk-dev] [PATCH v2 3/7] net/mlx5: added support for yellow policy rules Bing Zhao 2021-07-18 17:18 ` [dpdk-dev] [PATCH v2 4/7] net/mlx5: split policies handling of colors Bing Zhao 2021-07-18 17:18 ` [dpdk-dev] [PATCH v2 5/7] net/mlx5: support yellow in meter policy validation Bing Zhao 2021-07-18 17:18 ` [dpdk-dev] [PATCH v2 6/7] net/mlx5: check consistency of meter policy and profile Bing Zhao 2021-07-18 17:18 ` [dpdk-dev] [PATCH v2 7/7] net/mlx5: add meter support for trTCM profiles Bing Zhao 2021-07-21 8:54 ` [dpdk-dev] [PATCH v3 0/7] support yellow color policy in mlx5 Bing Zhao 2021-07-21 8:54 ` [dpdk-dev] [PATCH v3 1/7] net/mlx5: handle yellow case in default meter policy Bing Zhao 2021-07-21 8:54 ` [dpdk-dev] [PATCH v3 2/7] net/mlx5: enable meter bucket overflow for yellow color Bing Zhao 2021-07-21 8:54 ` [dpdk-dev] [PATCH v3 3/7] net/mlx5: added support for yellow policy rules Bing Zhao 2021-07-21 8:54 ` [dpdk-dev] [PATCH v3 4/7] net/mlx5: split policies handling of colors Bing Zhao 2021-07-21 8:54 ` [dpdk-dev] [PATCH v3 5/7] net/mlx5: support yellow in meter policy validation Bing Zhao 2021-07-21 8:54 ` [dpdk-dev] [PATCH v3 6/7] net/mlx5: check consistency of meter policy and profile Bing Zhao 2021-07-21 8:54 ` [dpdk-dev] [PATCH v3 7/7] net/mlx5: add meter support for trTCM profiles Bing Zhao 2021-07-22 11:28 ` [dpdk-dev] [PATCH v3 0/7] support yellow color policy in mlx5 Thomas Monjalon
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).