Hi,
> -----Original Message-----
> From: Dekel Peled <dek...@mellanox.com>
> Sent: Tuesday, March 24, 2020 2:58 PM
> To: Matan Azrad <ma...@mellanox.com>; Slava Ovsiienko
> <viachesl...@mellanox.com>; Raslan Darawsheh <rasl...@mellanox.com>
> Cc: dev@dpdk.org; sta...@dpdk.org
> Subject: [PATCH] net/mlx5: update VLAN and encap actions validation
>
> Flow rule in NIC table on VF representor should not contain VLAN pop
> or push actions, and encap or decap actions. Using these actions in
> NIC table on VF representor is not a valid use case.
> This patch updates the various validation functions to reject such
> rules.
>
> Cc: sta...@dpdk.org
>
> Signed-off-by: Dekel Peled <dek...@mellanox.com>
> Acked-by: Jack Min <jack...@mellanox.com>
> ---
> drivers/net/mlx5/mlx5_flow_dv.c | 82
> +++++++++++++++++++++++++++++++++--------
> 1 file changed, 67 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/net/mlx5/mlx5_flow_dv.c
> b/drivers/net/mlx5/mlx5_flow_dv.c
> index 2090631..7fe61c2 100644
> --- a/drivers/net/mlx5/mlx5_flow_dv.c
> +++ b/drivers/net/mlx5/mlx5_flow_dv.c
> @@ -1698,7 +1698,7 @@ struct field_modify_info modify_tcp[] = {
> const struct rte_flow_attr *attr,
> struct rte_flow_error *error)
> {
> - struct mlx5_priv *priv = dev->data->dev_private;
> + const struct mlx5_priv *priv = dev->data->dev_private;
>
> (void)action;
> (void)attr;
> @@ -1729,6 +1729,11 @@ struct field_modify_info modify_tcp[] = {
> RTE_FLOW_ERROR_TYPE_ACTION,
> action,
> "wrong action order, port_id should
> "
> "be after pop VLAN action");
> + if (!attr->transfer && priv->representor)
> + return rte_flow_error_set(error, ENOTSUP,
> +
> RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
> + "pop vlan action for VF representor "
> + "not supported on NIC table");
> return 0;
> }
>
> @@ -1792,6 +1797,8 @@ struct field_modify_info modify_tcp[] = {
> /**
> * Validate the push VLAN action.
> *
> + * @param[in] dev
> + * Pointer to the rte_eth_dev structure.
> * @param[in] action_flags
> * Holds the actions detected until now.
> * @param[in] item_flags
> @@ -1807,13 +1814,15 @@ struct field_modify_info modify_tcp[] = {
> * 0 on success, a negative errno value otherwise and rte_errno is set.
> */
> static int
> -flow_dv_validate_action_push_vlan(uint64_t action_flags,
> +flow_dv_validate_action_push_vlan(struct rte_eth_dev *dev,
> + uint64_t action_flags,
> uint64_t item_flags __rte_unused,
> const struct rte_flow_action *action,
> const struct rte_flow_attr *attr,
> struct rte_flow_error *error)
> {
> const struct rte_flow_action_of_push_vlan *push_vlan = action-
> >conf;
> + const struct mlx5_priv *priv = dev->data->dev_private;
>
> if (!attr->transfer && attr->ingress)
> return rte_flow_error_set(error, ENOTSUP,
> @@ -1836,6 +1845,11 @@ struct field_modify_info modify_tcp[] = {
> RTE_FLOW_ERROR_TYPE_ACTION,
> action,
> "wrong action order, port_id should
> "
> "be after push VLAN");
> + if (!attr->transfer && priv->representor)
> + return rte_flow_error_set(error, ENOTSUP,
> +
> RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
> + "push vlan action for VF representor
> "
> + "not supported on NIC table");
> (void)attr;
> return 0;
> }
> @@ -2201,10 +2215,14 @@ struct field_modify_info modify_tcp[] = {
> /**
> * Validate the L2 encap action.
> *
> + * @param[in] dev
> + * Pointer to the rte_eth_dev structure.
> * @param[in] action_flags
> * Holds the actions detected until now.
> * @param[in] action
> * Pointer to the action structure.
> + * @param[in] attr
> + * Pointer to flow attributes.
> * @param[out] error
> * Pointer to error structure.
> *
> @@ -2212,10 +2230,14 @@ struct field_modify_info modify_tcp[] = {
> * 0 on success, a negative errno value otherwise and rte_errno is set.
> */
> static int
> -flow_dv_validate_action_l2_encap(uint64_t action_flags,
> +flow_dv_validate_action_l2_encap(struct rte_eth_dev *dev,
> + uint64_t action_flags,
> const struct rte_flow_action *action,
> + const struct rte_flow_attr *attr,
> struct rte_flow_error *error)
> {
> + const struct mlx5_priv *priv = dev->data->dev_private;
> +
> if (!(action->conf))
> return rte_flow_error_set(error, EINVAL,
> RTE_FLOW_ERROR_TYPE_ACTION,
> action,
> @@ -2225,12 +2247,19 @@ struct field_modify_info modify_tcp[] = {
> RTE_FLOW_ERROR_TYPE_ACTION,
> NULL,
> "can only have a single encap action
> "
> "in a flow");
> + if (!attr->transfer && priv->representor)
> + return rte_flow_error_set(error, ENOTSUP,
> +
> RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
> + "encap action for VF representor "
> + "not supported on NIC table");
> return 0;
> }
>
> /**
> * Validate a decap action.
> *
> + * @param[in] dev
> + * Pointer to the rte_eth_dev structure.
> * @param[in] action_flags
> * Holds the actions detected until now.
> * @param[in] attr
> @@ -2242,10 +2271,13 @@ struct field_modify_info modify_tcp[] = {
> * 0 on success, a negative errno value otherwise and rte_errno is set.
> */
> static int
> -flow_dv_validate_action_decap(uint64_t action_flags,
> - const struct rte_flow_attr *attr,
> - struct rte_flow_error *error)
> +flow_dv_validate_action_decap(struct rte_eth_dev *dev,
> + uint64_t action_flags,
> + const struct rte_flow_attr *attr,
> + struct rte_flow_error *error)
> {
> + const struct mlx5_priv *priv = dev->data->dev_private;
> +
> if (action_flags & MLX5_FLOW_XCAP_ACTIONS)
> return rte_flow_error_set(error, ENOTSUP,
> RTE_FLOW_ERROR_TYPE_ACTION,
> NULL,
> @@ -2264,6 +2296,11 @@ struct field_modify_info modify_tcp[] = {
> NULL,
> "decap action not supported for "
> "egress");
> + if (!attr->transfer && priv->representor)
> + return rte_flow_error_set(error, ENOTSUP,
> +
> RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
> + "decap action for VF representor "
> + "not supported on NIC table");
> return 0;
> }
>
> @@ -2272,6 +2309,8 @@ struct field_modify_info modify_tcp[] = {
> /**
> * Validate the raw encap and decap actions.
> *
> + * @param[in] dev
> + * Pointer to the rte_eth_dev structure.
> * @param[in] decap
> * Pointer to the decap action.
> * @param[in] encap
> @@ -2290,11 +2329,13 @@ struct field_modify_info modify_tcp[] = {
> */
> static int
> flow_dv_validate_action_raw_encap_decap
> - (const struct rte_flow_action_raw_decap *decap,
> + (struct rte_eth_dev *dev,
> + const struct rte_flow_action_raw_decap *decap,
> const struct rte_flow_action_raw_encap *encap,
> const struct rte_flow_attr *attr, uint64_t *action_flags,
> int *actions_n, struct rte_flow_error *error)
> {
> + const struct mlx5_priv *priv = dev->data->dev_private;
> int ret;
>
> if (encap && (!encap->size || !encap->data))
> @@ -2327,7 +2368,8 @@ struct field_modify_info modify_tcp[] = {
> "encap combination");
> }
> if (decap) {
> - ret = flow_dv_validate_action_decap(*action_flags, attr,
> error);
> + ret = flow_dv_validate_action_decap(dev, *action_flags,
> attr,
> + error);
> if (ret < 0)
> return ret;
> *action_flags |= MLX5_FLOW_ACTION_DECAP;
> @@ -2344,6 +2386,12 @@ struct field_modify_info modify_tcp[] = {
>
> RTE_FLOW_ERROR_TYPE_ACTION,
> NULL,
> "more than one encap
> action");
> + if (!attr->transfer && priv->representor)
> + return rte_flow_error_set
> + (error, ENOTSUP,
> +
> RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
> + "encap action for VF representor "
> + "not supported on NIC table");
> *action_flags |= MLX5_FLOW_ACTION_ENCAP;
> ++(*actions_n);
> }
> @@ -4888,7 +4936,8 @@ struct field_modify_info modify_tcp[] = {
> ++actions_n;
> break;
> case RTE_FLOW_ACTION_TYPE_OF_PUSH_VLAN:
> - ret =
> flow_dv_validate_action_push_vlan(action_flags,
> + ret = flow_dv_validate_action_push_vlan(dev,
> + action_flags,
> item_flags,
> actions, attr,
> error);
> @@ -4916,8 +4965,10 @@ struct field_modify_info modify_tcp[] = {
> break;
> case RTE_FLOW_ACTION_TYPE_VXLAN_ENCAP:
> case RTE_FLOW_ACTION_TYPE_NVGRE_ENCAP:
> - ret =
> flow_dv_validate_action_l2_encap(action_flags,
> - actions, error);
> + ret = flow_dv_validate_action_l2_encap(dev,
> + action_flags,
> + actions, attr,
> + error);
> if (ret < 0)
> return ret;
> action_flags |= MLX5_FLOW_ACTION_ENCAP;
> @@ -4925,8 +4976,8 @@ struct field_modify_info modify_tcp[] = {
> break;
> case RTE_FLOW_ACTION_TYPE_VXLAN_DECAP:
> case RTE_FLOW_ACTION_TYPE_NVGRE_DECAP:
> - ret = flow_dv_validate_action_decap(action_flags,
> attr,
> - error);
> + ret = flow_dv_validate_action_decap(dev,
> action_flags,
> + attr, error);
> if (ret < 0)
> return ret;
> action_flags |= MLX5_FLOW_ACTION_DECAP;
> @@ -4934,7 +4985,7 @@ struct field_modify_info modify_tcp[] = {
> break;
> case RTE_FLOW_ACTION_TYPE_RAW_ENCAP:
> ret = flow_dv_validate_action_raw_encap_decap
> - (NULL, actions->conf, attr, &action_flags,
> + (dev, NULL, actions->conf, attr,
> &action_flags,
> &actions_n, error);
> if (ret < 0)
> return ret;
> @@ -4950,7 +5001,8 @@ struct field_modify_info modify_tcp[] = {
> encap = actions->conf;
> }
> ret = flow_dv_validate_action_raw_encap_decap
> - (decap ? decap : &empty_decap,
> encap,
> + (dev,
> + decap ? decap : &empty_decap,
> encap,
> attr, &action_flags, &actions_n,
> error);
> if (ret < 0)
> --
> 1.8.3.1
Patch applied to next-net-mlx,
Kindest regards,
Raslan Darawsheh
