* [PATCH] net/mlx5: forbid direction attributes in transfer flow rules
@ 2022-11-03 10:10 Dariusz Sosnowski
2022-11-06 11:07 ` Raslan Darawsheh
0 siblings, 1 reply; 2+ messages in thread
From: Dariusz Sosnowski @ 2022-11-03 10:10 UTC (permalink / raw)
To: Matan Azrad, Viacheslav Ovsiienko; +Cc: dev, Raslan Darawsheh
Since [1] flow API forbids usage of direction attributes in transfer
flow rules. This patch adapts mlx5 PMD to this requirement.
From this patch, flow rule validation in mxl5 PMD will reject transfer
flow rules with any of the direction attributes set
(i.e. 'ingress' or 'egress').
As a result flow rule can only have one of 'ingress', 'egress' or
'transfer' attributes set.
This patch also changes the following:
- Control flow rules used in FDB are 'transfer' only.
- Checks which assumed that 'transfer' can be used
with 'ingress' and 'egress' are reduced to just checking
for direction attributes, since all attributes are exclusive.
- Flow rules for updating flow_tag are created for both ingress
and transfer flow rules which have MARK action.
- Moves mlx5_flow_validate_attributes() function from generic flow
implementation to legacy Verbs flow engine implementation,
since it was used only there. Function is renamed accordingly.
Also removes checking if E-Switch uses DV in that
function, since if legacy Verbs flow engine is used,
then that is always not the case.
[1] commit bd2a4d4b2e3a ("ethdev: forbid direction attribute in
transfer flow rules")
Signed-off-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
Acked-by: Matan Azrad <matan@nvidia.com>
---
drivers/net/mlx5/mlx5_flow.c | 59 +++---------------------------
drivers/net/mlx5/mlx5_flow.h | 3 --
drivers/net/mlx5/mlx5_flow_dv.c | 42 ++++++++-------------
drivers/net/mlx5/mlx5_flow_verbs.c | 50 ++++++++++++++++++++++++-
4 files changed, 71 insertions(+), 83 deletions(-)
diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 8e7d649d15..921e419d05 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -2299,53 +2299,6 @@ mlx5_validate_action_ct(struct rte_eth_dev *dev,
return 0;
}
-/**
- * Verify the @p attributes will be correctly understood by the NIC and store
- * them in the @p flow if everything is correct.
- *
- * @param[in] dev
- * Pointer to the Ethernet device structure.
- * @param[in] attributes
- * Pointer to flow attributes
- * @param[out] error
- * Pointer to error structure.
- *
- * @return
- * 0 on success, a negative errno value otherwise and rte_errno is set.
- */
-int
-mlx5_flow_validate_attributes(struct rte_eth_dev *dev,
- const struct rte_flow_attr *attributes,
- struct rte_flow_error *error)
-{
- struct mlx5_priv *priv = dev->data->dev_private;
- uint32_t priority_max = priv->sh->flow_max_priority - 1;
-
- if (attributes->group)
- return rte_flow_error_set(error, ENOTSUP,
- RTE_FLOW_ERROR_TYPE_ATTR_GROUP,
- NULL, "groups is not supported");
- if (attributes->priority != MLX5_FLOW_LOWEST_PRIO_INDICATOR &&
- attributes->priority >= priority_max)
- return rte_flow_error_set(error, ENOTSUP,
- RTE_FLOW_ERROR_TYPE_ATTR_PRIORITY,
- NULL, "priority out of range");
- if (attributes->egress)
- return rte_flow_error_set(error, ENOTSUP,
- RTE_FLOW_ERROR_TYPE_ATTR_EGRESS, NULL,
- "egress is not supported");
- if (attributes->transfer && !priv->sh->config.dv_esw_en)
- return rte_flow_error_set(error, ENOTSUP,
- RTE_FLOW_ERROR_TYPE_ATTR_TRANSFER,
- NULL, "transfer is not supported");
- if (!attributes->ingress)
- return rte_flow_error_set(error, EINVAL,
- RTE_FLOW_ERROR_TYPE_ATTR_INGRESS,
- NULL,
- "ingress attribute is mandatory");
- return 0;
-}
-
/**
* Validate ICMP6 item.
*
@@ -6342,7 +6295,7 @@ flow_create_split_metadata(struct rte_eth_dev *dev,
ret = -rte_errno;
goto exit;
}
- } else if (attr->egress && !attr->transfer) {
+ } else if (attr->egress) {
/*
* All the actions on NIC Tx should have a metadata register
* copy action to copy reg_a from WQE to reg_c[meta]
@@ -7061,7 +7014,7 @@ flow_list_create(struct rte_eth_dev *dev, enum mlx5_flow_type type,
flow->drv_type < MLX5_FLOW_TYPE_MAX);
memset(rss_desc, 0, offsetof(struct mlx5_flow_rss_desc, queue));
/* RSS Action only works on NIC RX domain */
- if (attr->ingress && !attr->transfer)
+ if (attr->ingress)
rss = flow_get_rss_action(dev, p_actions_rx);
if (rss) {
if (flow_rss_workspace_adjust(wks, rss_desc, rss->queue_num))
@@ -7159,11 +7112,11 @@ flow_list_create(struct rte_eth_dev *dev, enum mlx5_flow_type type,
* we might create the extra rte_flow for each unique
* MARK/FLAG action ID.
*
- * The table is updated for ingress Flows only, because
+ * The table is updated for ingress and transfer flows only, because
* the egress Flows belong to the different device and
* copy table should be updated in peer NIC Rx domain.
*/
- if (attr->ingress &&
+ if ((attr->ingress || attr->transfer) &&
(external || attr->group != MLX5_FLOW_MREG_CP_TABLE_GROUP)) {
ret = flow_mreg_update_copy_table(dev, flow, actions, error);
if (ret)
@@ -7235,7 +7188,7 @@ mlx5_flow_create_esw_table_zero_flow(struct rte_eth_dev *dev)
const struct rte_flow_attr attr = {
.group = 0,
.priority = 0,
- .ingress = 1,
+ .ingress = 0,
.egress = 0,
.transfer = 1,
};
@@ -7279,7 +7232,7 @@ mlx5_flow_create_devx_sq_miss_flow(struct rte_eth_dev *dev, uint32_t sq_num)
struct rte_flow_attr attr = {
.group = 0,
.priority = MLX5_FLOW_LOWEST_PRIO_INDICATOR,
- .ingress = 1,
+ .ingress = 0,
.egress = 0,
.transfer = 1,
};
diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index da9b65d8fe..2398e44ea0 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -2228,9 +2228,6 @@ int mlx5_flow_validate_action_rss(const struct rte_flow_action *action,
int mlx5_flow_validate_action_default_miss(uint64_t action_flags,
const struct rte_flow_attr *attr,
struct rte_flow_error *error);
-int mlx5_flow_validate_attributes(struct rte_eth_dev *dev,
- const struct rte_flow_attr *attributes,
- struct rte_flow_error *error);
int mlx5_flow_item_acceptable(const struct rte_flow_item *item,
const uint8_t *mask,
const uint8_t *nic_mask,
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 1e52278191..2be62cbdcb 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -3384,8 +3384,7 @@ flow_dv_validate_action_set_tag(struct rte_eth_dev *dev,
ret = mlx5_flow_get_reg_id(dev, MLX5_APP_TAG, conf->index, error);
if (ret < 0)
return ret;
- if (!attr->transfer && attr->ingress &&
- (action_flags & terminal_action_flags))
+ if (attr->ingress && (action_flags & terminal_action_flags))
return rte_flow_error_set(error, EINVAL,
RTE_FLOW_ERROR_TYPE_ACTION, action,
"set_tag has no effect"
@@ -5942,7 +5941,7 @@ flow_dv_validate_action_sample(uint64_t *action_flags,
"action");
}
}
- if (attr->ingress && !attr->transfer) {
+ if (attr->ingress) {
if (!(sub_action_flags & (MLX5_FLOW_ACTION_QUEUE |
MLX5_FLOW_ACTION_RSS)))
return rte_flow_error_set(error, EINVAL,
@@ -5950,7 +5949,7 @@ flow_dv_validate_action_sample(uint64_t *action_flags,
NULL,
"Ingress must has a dest "
"QUEUE for Sample");
- } else if (attr->egress && !attr->transfer) {
+ } else if (attr->egress) {
return rte_flow_error_set(error, ENOTSUP,
RTE_FLOW_ERROR_TYPE_ACTION,
NULL,
@@ -5995,8 +5994,7 @@ flow_dv_validate_action_sample(uint64_t *action_flags,
NULL, "encap and decap "
"combination aren't "
"supported");
- if (!attr->transfer && attr->ingress && (sub_action_flags &
- MLX5_FLOW_ACTION_ENCAP))
+ if (attr->ingress && (sub_action_flags & MLX5_FLOW_ACTION_ENCAP))
return rte_flow_error_set(error, ENOTSUP,
RTE_FLOW_ERROR_TYPE_ACTION,
NULL, "encap is not supported"
@@ -6820,23 +6818,16 @@ flow_dv_validate_attributes(struct rte_eth_dev *dev,
RTE_FLOW_ERROR_TYPE_ATTR_PRIORITY,
NULL,
"priority out of range");
- if (attributes->transfer) {
- if (!priv->sh->config.dv_esw_en)
- return rte_flow_error_set
- (error, ENOTSUP,
- RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
- "E-Switch dr is not supported");
- if (attributes->egress)
- return rte_flow_error_set
- (error, ENOTSUP,
- RTE_FLOW_ERROR_TYPE_ATTR_EGRESS, attributes,
- "egress is not supported");
- }
- if (!(attributes->egress ^ attributes->ingress))
+ if (attributes->transfer && !priv->sh->config.dv_esw_en)
return rte_flow_error_set(error, ENOTSUP,
+ RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
+ "E-Switch dr is not supported");
+ if (attributes->ingress + attributes->egress + attributes->transfer != 1) {
+ return rte_flow_error_set(error, EINVAL,
RTE_FLOW_ERROR_TYPE_ATTR, NULL,
"must specify exactly one of "
- "ingress or egress");
+ "ingress, egress or transfer");
+ }
return ret;
}
@@ -8153,11 +8144,11 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr,
RTE_FLOW_ERROR_TYPE_ACTION, NULL,
"tunnel set decap rule must terminate "
"with JUMP");
- if (!attr->ingress)
+ if (attr->egress)
return rte_flow_error_set
(error, EINVAL,
RTE_FLOW_ERROR_TYPE_ACTION, NULL,
- "tunnel flows for ingress traffic only");
+ "tunnel flows for ingress and transfer traffic only");
}
if (action_flags & MLX5_FLOW_ACTION_TUNNEL_MATCH) {
uint64_t bad_actions_mask = MLX5_FLOW_ACTION_JUMP |
@@ -8262,7 +8253,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr,
"push VLAN action not supported "
"for ingress");
}
- if (!attr->transfer && attr->ingress) {
+ if (attr->ingress) {
if (action_flags & MLX5_FLOW_ACTION_ENCAP)
return rte_flow_error_set
(error, ENOTSUP,
@@ -8350,8 +8341,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const struct rte_flow_attr *attr,
* hairpin default egress flow with TX_QUEUE item, other flows not
* work due to metadata regC0 mismatch.
*/
- if ((!attr->transfer && attr->egress) && priv->representor &&
- !(item_flags & MLX5_FLOW_ITEM_SQ))
+ if (attr->egress && priv->representor && !(item_flags & MLX5_FLOW_ITEM_SQ))
return rte_flow_error_set(error, EINVAL,
RTE_FLOW_ERROR_TYPE_ITEM,
NULL,
@@ -13673,7 +13663,7 @@ flow_dv_translate_items_sws(struct rte_eth_dev *dev,
!(wks.item_flags & MLX5_FLOW_ITEM_REPRESENTED_PORT) &&
!(wks.item_flags & MLX5_FLOW_ITEM_PORT_REPRESENTOR) &&
priv->sh->esw_mode &&
- !(attr->egress && !attr->transfer) &&
+ !attr->egress &&
attr->group != MLX5_FLOW_MREG_CP_TABLE_GROUP) {
if (flow_dv_translate_item_port_id_all(dev, match_mask,
match_value, NULL, attr))
diff --git a/drivers/net/mlx5/mlx5_flow_verbs.c b/drivers/net/mlx5/mlx5_flow_verbs.c
index 81a33ddf09..8da2bd1f78 100644
--- a/drivers/net/mlx5/mlx5_flow_verbs.c
+++ b/drivers/net/mlx5/mlx5_flow_verbs.c
@@ -1207,6 +1207,54 @@ flow_verbs_translate_action_count(struct mlx5_flow *dev_flow,
return 0;
}
+/**
+ * Validates @p attributes of the flow rule.
+ *
+ * This function is used if and only if legacy Verbs flow engine is used.
+ *
+ * @param[in] dev
+ * Pointer to the Ethernet device structure.
+ * @param[in] attributes
+ * Pointer to flow attributes
+ * @param[out] error
+ * Pointer to error structure.
+ *
+ * @return
+ * 0 on success, a negative errno value otherwise and rte_errno is set.
+ */
+static int
+flow_verbs_validate_attributes(struct rte_eth_dev *dev,
+ const struct rte_flow_attr *attributes,
+ struct rte_flow_error *error)
+{
+ struct mlx5_priv *priv = dev->data->dev_private;
+ uint32_t priority_max = priv->sh->flow_max_priority - 1;
+
+ if (attributes->group)
+ return rte_flow_error_set(error, ENOTSUP,
+ RTE_FLOW_ERROR_TYPE_ATTR_GROUP,
+ NULL, "groups is not supported");
+ if (attributes->priority != MLX5_FLOW_LOWEST_PRIO_INDICATOR &&
+ attributes->priority >= priority_max)
+ return rte_flow_error_set(error, ENOTSUP,
+ RTE_FLOW_ERROR_TYPE_ATTR_PRIORITY,
+ NULL, "priority out of range");
+ if (attributes->egress)
+ return rte_flow_error_set(error, ENOTSUP,
+ RTE_FLOW_ERROR_TYPE_ATTR_EGRESS, NULL,
+ "egress is not supported");
+ if (attributes->transfer)
+ return rte_flow_error_set(error, ENOTSUP,
+ RTE_FLOW_ERROR_TYPE_ATTR_TRANSFER,
+ NULL, "transfer is not supported");
+ if (!attributes->ingress)
+ return rte_flow_error_set(error, EINVAL,
+ RTE_FLOW_ERROR_TYPE_ATTR_INGRESS,
+ NULL,
+ "ingress attribute is mandatory");
+ return 0;
+}
+
/**
* Internal validation function. For validating both actions and items.
*
@@ -1249,7 +1297,7 @@ flow_verbs_validate(struct rte_eth_dev *dev,
if (items == NULL)
return -1;
- ret = mlx5_flow_validate_attributes(dev, attr, error);
+ ret = flow_verbs_validate_attributes(dev, attr, error);
if (ret < 0)
return ret;
is_root = ret;
--
2.25.1
^ permalink raw reply [flat|nested] 2+ messages in thread
* RE: [PATCH] net/mlx5: forbid direction attributes in transfer flow rules
2022-11-03 10:10 [PATCH] net/mlx5: forbid direction attributes in transfer flow rules Dariusz Sosnowski
@ 2022-11-06 11:07 ` Raslan Darawsheh
0 siblings, 0 replies; 2+ messages in thread
From: Raslan Darawsheh @ 2022-11-06 11:07 UTC (permalink / raw)
To: Dariusz Sosnowski, Matan Azrad, Slava Ovsiienko; +Cc: dev
Hi,
> -----Original Message-----
> From: Dariusz Sosnowski <dsosnowski@nvidia.com>
> Sent: Thursday, November 3, 2022 12:10 PM
> To: Matan Azrad <matan@nvidia.com>; Slava Ovsiienko
> <viacheslavo@nvidia.com>
> Cc: dev@dpdk.org; Raslan Darawsheh <rasland@nvidia.com>
> Subject: [PATCH] net/mlx5: forbid direction attributes in transfer flow rules
>
> Since [1] flow API forbids usage of direction attributes in transfer
> flow rules. This patch adapts mlx5 PMD to this requirement.
>
> From this patch, flow rule validation in mxl5 PMD will reject transfer
> flow rules with any of the direction attributes set
> (i.e. 'ingress' or 'egress').
> As a result flow rule can only have one of 'ingress', 'egress' or
> 'transfer' attributes set.
>
> This patch also changes the following:
>
> - Control flow rules used in FDB are 'transfer' only.
> - Checks which assumed that 'transfer' can be used
> with 'ingress' and 'egress' are reduced to just checking
> for direction attributes, since all attributes are exclusive.
> - Flow rules for updating flow_tag are created for both ingress
> and transfer flow rules which have MARK action.
> - Moves mlx5_flow_validate_attributes() function from generic flow
> implementation to legacy Verbs flow engine implementation,
> since it was used only there. Function is renamed accordingly.
> Also removes checking if E-Switch uses DV in that
> function, since if legacy Verbs flow engine is used,
> then that is always not the case.
>
> [1] commit bd2a4d4b2e3a ("ethdev: forbid direction attribute in
> transfer flow rules")
>
> Signed-off-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
> Acked-by: Matan Azrad <matan@nvidia.com>
Patch applied to next-net-mlx,
Kindest regards,
Raslan Darawsheh
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-11-06 11:07 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-03 10:10 [PATCH] net/mlx5: forbid direction attributes in transfer flow rules Dariusz Sosnowski
2022-11-06 11:07 ` Raslan Darawsheh
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).