Hi,
> -----Original Message-----
> From: Xiaoyu Min <jack...@mellanox.com>
> Sent: Monday, April 13, 2020 6:33 AM
> To: Raslan Darawsheh <rasl...@mellanox.com>; Matan Azrad
> <ma...@mellanox.com>; Shahaf Shuler <shah...@mellanox.com>; Slava
> Ovsiienko <viachesl...@mellanox.com>
> Cc: dev@dpdk.org; sta...@dpdk.org; Dekel Peled <dek...@mellanox.com>
> Subject: [PATCH v2] net/mlx5: fix validation of push VLAN without full mask
>
> Due the limitation of HW, when PMD create push VLAN action it needs to
> know what exactly the value of VID/PCP.
>
> PMD try to figure out them via:
> - of_set_vlan_vid/pcp actions
> - VLAN item in pattern
> If none of above is provided, default value - zero is used.
>
> However user will write rule like [1] which match on a range of VID and
> without of_set_vlan_vid action and expect the VID will inherit from
> original packet. This is not supported by HW currently. PMD will set VID
> to default value - zero because it cannot figure out the exact value of VID
> from VLAN item.
>
> This is sort of misleading for some users.
>
> In order to avoid this, PMD will spit out error for rule like [1] to
> force user to provide explicit VID/PCP for new pushed VLAN headers.
>
> Fixes: 9aee7a8418d4 ("net/mlx5: support push flow action on VLAN header")
> Cc: sta...@dpdk.org
>
> [1]: testpmd> flow create 2 ingress transfer group 0 priority 3 pattern
> eth / vlan vid spec 2859 vid prefix 4 / ipv4 / end
> actcions of_push_vlan ethertype 0x88A8 /
> of_set_vlan_pcp vlan_pcp 6 / port_id id 0 / end
>
> Signed-off-by: Xiaoyu Min <jack...@mellanox.com>
> Reviewed-by: Dekel Peled <dek...@mellanox.com>
> Acked-by: Viacheslav Ovsiienko <viachesl...@mellanox.com>
> ---
> v2:
> - rebased
> - added Acked-by tag
>
> drivers/net/mlx5/mlx5_flow_dv.c | 34
> +++++++++++++++++++++++++++++++--
> 1 file changed, 32 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/mlx5/mlx5_flow_dv.c
> b/drivers/net/mlx5/mlx5_flow_dv.c
> index 18ea577f8c..b1d6bf6e6c 100644
> --- a/drivers/net/mlx5/mlx5_flow_dv.c
> +++ b/drivers/net/mlx5/mlx5_flow_dv.c
> @@ -1829,7 +1829,7 @@ flow_dev_get_vlan_info_from_items(const struct
> rte_flow_item *items,
> static int
> 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_item_vlan *vlan_m,
> const struct rte_flow_action *action,
> const struct rte_flow_attr *attr,
> struct rte_flow_error *error)
> @@ -1863,6 +1863,32 @@ flow_dv_validate_action_push_vlan(struct
> rte_eth_dev *dev,
>
> RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
> "push vlan action for VF representor
> "
> "not supported on NIC table");
> + if (vlan_m &&
> + (vlan_m->tci & MLX5DV_FLOW_VLAN_PCP_MASK_BE) &&
> + (vlan_m->tci & MLX5DV_FLOW_VLAN_PCP_MASK_BE) !=
> + MLX5DV_FLOW_VLAN_PCP_MASK_BE &&
> + !(action_flags & MLX5_FLOW_ACTION_OF_SET_VLAN_PCP) &&
> + !(mlx5_flow_find_action
> + (action + 1, RTE_FLOW_ACTION_TYPE_OF_SET_VLAN_PCP)))
> + return rte_flow_error_set(error, EINVAL,
> + RTE_FLOW_ERROR_TYPE_ACTION,
> action,
> + "not full match mask on VLAN PCP
> and "
> + "there is no of_set_vlan_pcp action,
> "
> + "push VLAN action cannot figure out
> "
> + "PCP value");
> + if (vlan_m &&
> + (vlan_m->tci & MLX5DV_FLOW_VLAN_VID_MASK_BE) &&
> + (vlan_m->tci & MLX5DV_FLOW_VLAN_VID_MASK_BE) !=
> + MLX5DV_FLOW_VLAN_VID_MASK_BE &&
> + !(action_flags & MLX5_FLOW_ACTION_OF_SET_VLAN_VID) &&
> + !(mlx5_flow_find_action
> + (action + 1, RTE_FLOW_ACTION_TYPE_OF_SET_VLAN_VID)))
> + return rte_flow_error_set(error, EINVAL,
> + RTE_FLOW_ERROR_TYPE_ACTION,
> action,
> + "not full match mask on VLAN VID
> and "
> + "there is no of_set_vlan_vid action,
> "
> + "push VLAN action cannot figure out
> "
> + "VID value");
> (void)attr;
> return 0;
> }
> @@ -4563,6 +4589,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const
> struct rte_flow_attr *attr,
> struct mlx5_priv *priv = dev->data->dev_private;
> struct mlx5_dev_config *dev_conf = &priv->config;
> uint16_t queue_index = 0xFFFF;
> + const struct rte_flow_item_vlan *vlan_m = NULL;
>
> if (items == NULL)
> return -1;
> @@ -4620,6 +4647,9 @@ flow_dv_validate(struct rte_eth_dev *dev, const
> struct rte_flow_attr *attr,
> } else {
> ether_type = 0;
> }
> + /* Store outer VLAN mask for of_push_vlan action.
> */
> + if (!tunnel)
> + vlan_m = items->mask;
> break;
> case RTE_FLOW_ITEM_TYPE_IPV4:
> mlx5_flow_tunnel_ip_check(items, next_protocol,
> @@ -4935,7 +4965,7 @@ flow_dv_validate(struct rte_eth_dev *dev, const
> struct rte_flow_attr *attr,
> case RTE_FLOW_ACTION_TYPE_OF_PUSH_VLAN:
> ret = flow_dv_validate_action_push_vlan(dev,
> action_flags,
> - item_flags,
> + vlan_m,
> actions, attr,
> error);
> if (ret < 0)
> --
> 2.26.0
Patch applied to next-net-mlx,
Kindest regards,
Raslan Darawsheh
