* [PATCH 21.11 1/2] net/mlx5: fix source port checking in sample flow rule
2022-11-26 16:02 [PATCH 21.11 0/2] net/mlx5: fix some sample/mirror issues Jiawei Wang
@ 2022-11-26 16:02 ` Jiawei Wang
2022-11-26 16:02 ` [PATCH 21.11 2/2] net/mlx5: fix mirror flow validation with ASO action Jiawei Wang
1 sibling, 0 replies; 3+ messages in thread
From: Jiawei Wang @ 2022-11-26 16:02 UTC (permalink / raw)
To: ktraynor, viacheslavo, Matan Azrad; +Cc: stable, rasland
[ upstream commit e9de8f33ca89876071fc3b0997a53c8268bfb593 ]
The metadata register C value was lost in FDB egress while doing the
flow sampler on ConnectX-5. The FDB direction checking was decided by
the source port in the flow creation. If there's additional port item
was added in the flow match, then the actual source port was changed.
This patch adds the checking for the port id item:
RTE_FLOW_ITEM_TYPE_PORT_ID, then updates FDB egress checking and
the source vport metadata from the port item, also updates the PUSH VLAN,
POP VLAN and flow sampler action validation.
Fixes: 8db2867c7996 ("net/mlx5: fix port matching in sample flow rule")
Fixes: 255b8f86eb6e ("net/mlx5: fix E-Switch egress mirror flow validation")
Signed-off-by: Jiawei Wang <jiaweiw@nvidia.com>
---
drivers/net/mlx5/mlx5_flow.c | 36 +++++++++++++++-----
drivers/net/mlx5/mlx5_flow.h | 20 +++++++++++
drivers/net/mlx5/mlx5_flow_dv.c | 59 +++++++++++++++++++++------------
3 files changed, 84 insertions(+), 31 deletions(-)
diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index df3a132ee4..36abc21639 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -5526,7 +5526,8 @@ flow_check_match_action(const struct rte_flow_action actions[],
ratio = sample->ratio;
sub_type = ((const struct rte_flow_action *)
(sample->actions))->type;
- if (ratio == 1 && attr->transfer)
+ if (ratio == 1 && attr->transfer &&
+ sub_type != RTE_FLOW_ACTION_TYPE_END)
fdb_mirror = 1;
break;
case RTE_FLOW_ACTION_TYPE_SET_MAC_SRC:
@@ -5698,9 +5699,11 @@ flow_sample_split_prep(struct rte_eth_dev *dev,
break;
case RTE_FLOW_ACTION_TYPE_OF_PUSH_VLAN:
case RTE_FLOW_ACTION_TYPE_OF_SET_VLAN_VID:
- push_vlan_idx = action_idx;
- if (push_vlan_idx < sample_action_pos)
+ if (action_idx < sample_action_pos &&
+ push_vlan_idx == -1) {
set_tag_idx = action_idx;
+ push_vlan_idx = action_idx;
+ }
break;
default:
break;
@@ -5763,17 +5766,18 @@ flow_sample_split_prep(struct rte_eth_dev *dev,
.data = tag_id,
};
/* Prepare the suffix subflow items. */
+ tag_spec = (void *)(sfx_items + SAMPLE_SUFFIX_ITEM);
+ tag_spec->data = tag_id;
+ tag_spec->id = set_tag->id;
+ tag_mask = tag_spec + 1;
+ tag_mask->data = UINT32_MAX;
for (; items->type != RTE_FLOW_ITEM_TYPE_END; items++) {
if (items->type == RTE_FLOW_ITEM_TYPE_PORT_ID) {
memcpy(sfx_items, items, sizeof(*sfx_items));
sfx_items++;
+ break;
}
}
- tag_spec = (void *)(sfx_items + SAMPLE_SUFFIX_ITEM);
- tag_spec->data = tag_id;
- tag_spec->id = set_tag->id;
- tag_mask = tag_spec + 1;
- tag_mask->data = UINT32_MAX;
sfx_items[0] = (struct rte_flow_item){
.type = (enum rte_flow_item_type)
MLX5_RTE_FLOW_ITEM_TYPE_TAG,
@@ -6351,6 +6355,8 @@ flow_create_split_sample(struct rte_eth_dev *dev,
uint16_t jump_table = 0;
const uint32_t next_ft_step = 1;
int ret = 0;
+ struct mlx5_priv *item_port_priv = NULL;
+ const struct rte_flow_item *item;
if (priv->sampler_en)
actions_n = flow_check_match_action(actions, attr,
@@ -6370,8 +6376,20 @@ flow_create_split_sample(struct rte_eth_dev *dev,
RTE_FLOW_ERROR_TYPE_ACTION,
NULL, "no memory to split "
"sample flow");
+ for (item = items; item->type != RTE_FLOW_ITEM_TYPE_END; item++) {
+ if (item->type == RTE_FLOW_ITEM_TYPE_PORT_ID) {
+ const struct rte_flow_item_port_id *spec;
+
+ spec = (const struct rte_flow_item_port_id *)item->spec;
+ if (spec)
+ item_port_priv =
+ mlx5_port_to_eswitch_info(spec->id, true);
+ break;
+ }
+ }
/* The representor_id is UINT16_MAX for uplink. */
- fdb_tx = (attr->transfer && priv->representor_id != UINT16_MAX);
+ fdb_tx = (attr->transfer &&
+ flow_source_vport_representor(priv, item_port_priv));
/*
* When reg_c_preserve is set, metadata registers Cx preserve
* their value even through packet duplication.
diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index 7b5fde4823..b8ffedddf7 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -1111,6 +1111,7 @@ struct mlx5_flow_workspace {
uint32_t skip_matcher_reg:1;
/* Indicates if need to skip matcher register in translate. */
uint32_t mark:1; /* Indicates if flow contains mark action. */
+ uint32_t vport_meta_tag; /* Used for vport index match. */
};
struct mlx5_flow_split_info {
@@ -1467,6 +1468,25 @@ mlx5_translate_tunnel_etypes(uint64_t pattern_flags)
return 0;
}
+/**
+ * Indicates whether flow source vport is representor port.
+ *
+ * @param[in] priv
+ * Pointer to device private context structure.
+ * @param[in] act_priv
+ * Pointer to actual device private context structure if have.
+ *
+ * @return
+ * True when the flow source vport is representor port, false otherwise.
+ */
+static inline bool
+flow_source_vport_representor(struct mlx5_priv *priv, struct mlx5_priv *act_priv)
+{
+ MLX5_ASSERT(priv);
+ return (!act_priv ? (priv->representor_id != UINT16_MAX) :
+ (act_priv->representor_id != UINT16_MAX));
+}
+
int mlx5_flow_group_to_table(struct rte_eth_dev *dev,
const struct mlx5_flow_tunnel *tunnel,
uint32_t group, uint32_t *table,
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 4603736c28..7dfae06aae 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -2145,6 +2145,7 @@ flow_dv_validate_item_port_id(struct rte_eth_dev *dev,
const struct rte_flow_item *item,
const struct rte_flow_attr *attr,
uint64_t item_flags,
+ struct mlx5_priv **act_priv,
struct rte_flow_error *error)
{
const struct rte_flow_item_port_id *spec = item->spec;
@@ -2203,6 +2204,7 @@ flow_dv_validate_item_port_id(struct rte_eth_dev *dev,
RTE_FLOW_ERROR_TYPE_ITEM_SPEC, spec,
"cannot match on a port from a"
" different E-Switch");
+ *act_priv = esw_priv;
return 0;
}
@@ -2722,30 +2724,12 @@ flow_dv_validate_action_pop_vlan(struct rte_eth_dev *dev,
struct rte_flow_error *error)
{
const struct mlx5_priv *priv = dev->data->dev_private;
- struct mlx5_dev_ctx_shared *sh = priv->sh;
- bool direction_error = false;
if (!priv->sh->pop_vlan_action)
return rte_flow_error_set(error, ENOTSUP,
RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
NULL,
"pop vlan action is not supported");
- /* Pop VLAN is not supported in egress except for CX6 FDB mode. */
- if (attr->transfer) {
- bool fdb_tx = priv->representor_id != UINT16_MAX;
- bool is_cx5 = sh->steering_format_version ==
- MLX5_STEERING_LOGIC_FORMAT_CONNECTX_5;
-
- if (fdb_tx && is_cx5)
- direction_error = true;
- } else if (attr->egress) {
- direction_error = true;
- }
- if (direction_error)
- return rte_flow_error_set(error, ENOTSUP,
- RTE_FLOW_ERROR_TYPE_ATTR_EGRESS,
- NULL,
- "pop vlan action not supported for egress");
if (action_flags & MLX5_FLOW_VLAN_ACTIONS)
return rte_flow_error_set(error, ENOTSUP,
RTE_FLOW_ERROR_TYPE_ACTION, action,
@@ -5568,6 +5552,7 @@ flow_dv_validate_action_sample(uint64_t *action_flags,
const struct rte_flow_action_rss **sample_rss,
const struct rte_flow_action_count **count,
int *fdb_mirror_limit,
+ struct mlx5_priv *act_priv,
struct rte_flow_error *error)
{
struct mlx5_priv *priv = dev->data->dev_private;
@@ -5755,7 +5740,7 @@ flow_dv_validate_action_sample(uint64_t *action_flags,
"E-Switch must has a dest "
"port for mirroring");
if (!priv->config.hca_attr.reg_c_preserve &&
- priv->representor_id != UINT16_MAX)
+ flow_source_vport_representor(priv, act_priv))
*fdb_mirror_limit = 1;
}
/* Continue validation for Xcap actions.*/
@@ -6913,6 +6898,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr,
uint32_t tag_id = 0;
const struct rte_flow_action_age *non_shared_age = NULL;
const struct rte_flow_action_count *count = NULL;
+ struct mlx5_priv *act_priv = NULL;
if (items == NULL)
return -1;
@@ -6954,7 +6940,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr,
break;
case RTE_FLOW_ITEM_TYPE_PORT_ID:
ret = flow_dv_validate_item_port_id
- (dev, items, attr, item_flags, error);
+ (dev, items, attr, item_flags, &act_priv, error);
if (ret < 0)
return ret;
last_item = MLX5_FLOW_ITEM_PORT_ID;
@@ -7817,6 +7803,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr,
rss, &sample_rss,
&sample_count,
&fdb_mirror_limit,
+ act_priv,
error);
if (ret < 0)
return ret;
@@ -7987,7 +7974,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr,
bool direction_error = false;
if (attr->transfer) {
- bool fdb_tx = priv->representor_id != UINT16_MAX;
+ bool fdb_tx = flow_source_vport_representor(priv, act_priv);
bool is_cx5 = sh->steering_format_version ==
MLX5_STEERING_LOGIC_FORMAT_CONNECTX_5;
@@ -8019,6 +8006,27 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr,
"multiple VLAN actions");
}
}
+ /* Pop VLAN is not supported in egress except for NICs newer than CX5. */
+ if (action_flags & MLX5_FLOW_ACTION_OF_POP_VLAN) {
+ struct mlx5_dev_ctx_shared *sh = priv->sh;
+ bool direction_error = false;
+
+ if (attr->transfer) {
+ bool fdb_tx = flow_source_vport_representor(priv, act_priv);
+ bool is_cx5 = sh->steering_format_version ==
+ MLX5_STEERING_LOGIC_FORMAT_CONNECTX_5;
+
+ if (fdb_tx && is_cx5)
+ direction_error = true;
+ } else if (attr->egress) {
+ direction_error = true;
+ }
+ if (direction_error)
+ return rte_flow_error_set(error, ENOTSUP,
+ RTE_FLOW_ERROR_TYPE_ATTR_EGRESS,
+ NULL,
+ "pop vlan action not supported for egress");
+ }
if (action_flags & MLX5_FLOW_ACTION_METER_WITH_TERMINATED_POLICY) {
if ((action_flags & (MLX5_FLOW_FATE_ACTIONS &
~MLX5_FLOW_ACTION_METER_WITH_TERMINATED_POLICY)) &&
@@ -8137,6 +8145,7 @@ flow_dv_prepare(struct rte_eth_dev *dev,
wks->skip_matcher_reg = 0;
wks->policy = NULL;
wks->final_policy = NULL;
+ wks->vport_meta_tag = 0;
/* In case of corrupting the memory. */
if (wks->flow_idx >= MLX5_NUM_MAX_DEV_FLOWS) {
rte_flow_error_set(error, ENOSPC,
@@ -9761,9 +9770,11 @@ flow_dv_translate_item_port_id(struct rte_eth_dev *dev, void *matcher,
{
const struct rte_flow_item_port_id *pid_m = item ? item->mask : NULL;
const struct rte_flow_item_port_id *pid_v = item ? item->spec : NULL;
+ struct mlx5_flow_workspace *wks = mlx5_flow_get_thread_workspace();
struct mlx5_priv *priv;
uint16_t mask, id;
+ MLX5_ASSERT(wks);
if (pid_v && pid_v->id == MLX5_PORT_ESW_MGR) {
flow_dv_translate_item_source_vport(matcher, key,
mlx5_flow_get_esw_manager_vport_id(dev), 0xffff);
@@ -9780,6 +9791,7 @@ flow_dv_translate_item_port_id(struct rte_eth_dev *dev, void *matcher,
* register.
*/
if (priv->vport_meta_mask) {
+ wks->vport_meta_tag = priv->vport_meta_tag;
/*
* Provide the hint for SW steering library
* to insert the flow into ingress domain and
@@ -11827,6 +11839,9 @@ flow_dv_translate_action_sample(struct rte_eth_dev *dev,
uint32_t action_in[MLX5_ST_SZ_DW(set_action_in)];
uint64_t set_action;
} action_ctx = { .set_action = 0 };
+ uint32_t vport_meta_tag = wks->vport_meta_tag ?
+ wks->vport_meta_tag :
+ priv->vport_meta_tag;
res->ft_type = MLX5DV_FLOW_TABLE_TYPE_FDB;
MLX5_SET(set_action_in, action_ctx.action_in, action_type,
@@ -11834,7 +11849,7 @@ flow_dv_translate_action_sample(struct rte_eth_dev *dev,
MLX5_SET(set_action_in, action_ctx.action_in, field,
MLX5_MODI_META_REG_C_0);
MLX5_SET(set_action_in, action_ctx.action_in, data,
- priv->vport_meta_tag);
+ vport_meta_tag);
res->set_action = action_ctx.set_action;
} else if (attr->ingress) {
res->ft_type = MLX5DV_FLOW_TABLE_TYPE_NIC_RX;
--
2.18.1
^ permalink raw reply [flat|nested] 3+ messages in thread* [PATCH 21.11 2/2] net/mlx5: fix mirror flow validation with ASO action
2022-11-26 16:02 [PATCH 21.11 0/2] net/mlx5: fix some sample/mirror issues Jiawei Wang
2022-11-26 16:02 ` [PATCH 21.11 1/2] net/mlx5: fix source port checking in sample flow rule Jiawei Wang
@ 2022-11-26 16:02 ` Jiawei Wang
1 sibling, 0 replies; 3+ messages in thread
From: Jiawei Wang @ 2022-11-26 16:02 UTC (permalink / raw)
To: ktraynor, viacheslavo, Matan Azrad, Dekel Peled; +Cc: stable, rasland
[ upstream commit 3e13a6ae95c52267852691f3114e26a7077a6b35 ]
While the ASO action(AGE, CT) with the sample action in the one
E-switch mirror flow, due to hardware limitation, the ASO action
after the sample action was not supported.
This patch adds the checking for this validation and reject the flows
with aso action after sample.
Fixes: f935ed4b645a ("net/mlx5: support flow hit action for aging")
Signed-off-by: Jiawei Wang <jiaweiw@nvidia.com>
---
drivers/net/mlx5/mlx5_flow_dv.c | 53 ++++++++++++++++++++-------------
1 file changed, 32 insertions(+), 21 deletions(-)
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 7dfae06aae..2d8a105678 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -5534,8 +5534,8 @@ flow_dv_modify_clone_free_cb(void *tool_ctx, struct mlx5_list_entry *entry)
* Pointer to the RSS action in sample action list.
* @param[out] count
* Pointer to the COUNT action in sample action list.
- * @param[out] fdb_mirror_limit
- * Pointer to the FDB mirror limitation flag.
+ * @param[out] fdb_mirror
+ * Pointer to the FDB mirror flag.
* @param[out] error
* Pointer to error structure.
*
@@ -5551,8 +5551,7 @@ flow_dv_validate_action_sample(uint64_t *action_flags,
const struct rte_flow_action_rss *rss,
const struct rte_flow_action_rss **sample_rss,
const struct rte_flow_action_count **count,
- int *fdb_mirror_limit,
- struct mlx5_priv *act_priv,
+ int *fdb_mirror,
struct rte_flow_error *error)
{
struct mlx5_priv *priv = dev->data->dev_private;
@@ -5739,9 +5738,7 @@ flow_dv_validate_action_sample(uint64_t *action_flags,
NULL,
"E-Switch must has a dest "
"port for mirroring");
- if (!priv->config.hca_attr.reg_c_preserve &&
- flow_source_vport_representor(priv, act_priv))
- *fdb_mirror_limit = 1;
+ *fdb_mirror = 1;
}
/* Continue validation for Xcap actions.*/
if ((sub_action_flags & MLX5_FLOW_XCAP_ACTIONS) &&
@@ -6833,7 +6830,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr,
uint16_t ether_type = 0;
int actions_n = 0;
uint8_t item_ipv6_proto = 0;
- int fdb_mirror_limit = 0;
+ int fdb_mirror = 0;
int modify_after_mirror = 0;
const struct rte_flow_item *geneve_item = NULL;
const struct rte_flow_item *gre_item = NULL;
@@ -6899,6 +6896,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr,
const struct rte_flow_action_age *non_shared_age = NULL;
const struct rte_flow_action_count *count = NULL;
struct mlx5_priv *act_priv = NULL;
+ int aso_after_sample = 0;
if (items == NULL)
return -1;
@@ -7648,12 +7646,6 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr,
error);
if (ret)
return ret;
- if ((action_flags & MLX5_FLOW_ACTION_SAMPLE) &&
- fdb_mirror_limit)
- return rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ACTION,
- NULL,
- "sample and jump action combination is not supported");
++actions_n;
action_flags |= MLX5_FLOW_ACTION_JUMP;
break;
@@ -7733,6 +7725,8 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr,
RTE_FLOW_ERROR_TYPE_ACTION,
NULL,
"duplicate age actions set");
+ if (action_flags & MLX5_FLOW_ACTION_SAMPLE)
+ aso_after_sample = 1;
action_flags |= MLX5_FLOW_ACTION_AGE;
++actions_n;
break;
@@ -7760,6 +7754,9 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr,
RTE_FLOW_ERROR_TYPE_ACTION,
NULL,
"old age action and count must be in the same sub flow");
+ } else {
+ if (action_flags & MLX5_FLOW_ACTION_SAMPLE)
+ aso_after_sample = 1;
}
action_flags |= MLX5_FLOW_ACTION_AGE;
++actions_n;
@@ -7802,8 +7799,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr,
attr, item_flags,
rss, &sample_rss,
&sample_count,
- &fdb_mirror_limit,
- act_priv,
+ &fdb_mirror,
error);
if (ret < 0)
return ret;
@@ -7837,6 +7833,8 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr,
error);
if (ret < 0)
return ret;
+ if (action_flags & MLX5_FLOW_ACTION_SAMPLE)
+ aso_after_sample = 1;
action_flags |= MLX5_FLOW_ACTION_CT;
break;
case MLX5_RTE_FLOW_ACTION_TYPE_TUNNEL_SET:
@@ -8089,11 +8087,24 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr,
NULL, "too many header modify"
" actions to support");
}
- /* Eswitch egress mirror and modify flow has limitation on CX5 */
- if (fdb_mirror_limit && modify_after_mirror)
- return rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ACTION, NULL,
- "sample before modify action is not supported");
+ if (fdb_mirror) {
+ if (!priv->sh->cdev->config.hca_attr.reg_c_preserve &&
+ flow_source_vport_representor(priv, act_priv)) {
+ /* Eswitch egress mirror and modify flow has limitation on CX5 */
+ if (modify_after_mirror)
+ return rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ACTION, NULL,
+ "sample before modify action is not supported");
+ if (action_flags & MLX5_FLOW_ACTION_JUMP)
+ return rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ACTION, NULL,
+ "sample and jump action combination is not supported");
+ }
+ if (aso_mask > 0 && aso_after_sample && fdb_mirror)
+ return rte_flow_error_set(error, ENOTSUP,
+ RTE_FLOW_ERROR_TYPE_ACTION, NULL,
+ "sample before ASO action is not supported");
+ }
/*
* Validation the NIC Egress flow on representor, except implicit
* hairpin default egress flow with TX_QUEUE item, other flows not
--
2.18.1
^ permalink raw reply [flat|nested] 3+ messages in thread