[dpdk-dev] [PATCH 2/6] net/mlx5: support yellow in meter policy validation
Bing Zhao
bingz at nvidia.com
Mon Jul 5 17:57:52 CEST 2021
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 at 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
More information about the dev
mailing list