DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] net/mlx5: fix validation of push VLAN without full mask
@ 2020-04-02 10:21 Xiaoyu Min
  2020-04-13  3:32 ` [dpdk-dev] [PATCH v2] " Xiaoyu Min
  0 siblings, 1 reply; 3+ messages in thread
From: Xiaoyu Min @ 2020-04-02 10:21 UTC (permalink / raw)
  To: Matan Azrad, Shahaf Shuler, Viacheslav Ovsiienko; +Cc: dev, stable, Dekel Peled

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: stable@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 <jackmin@mellanox.com>
Reviewed-by: Dekel Peled <dekelp@mellanox.com>
---
 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 6aa6e8383d..eb7d499f7e 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;
 }
@@ -4580,6 +4606,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;
@@ -4637,6 +4664,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,
@@ -4952,7 +4982,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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* [dpdk-dev] [PATCH v2] net/mlx5: fix validation of push VLAN without full mask
  2020-04-02 10:21 [dpdk-dev] [PATCH] net/mlx5: fix validation of push VLAN without full mask Xiaoyu Min
@ 2020-04-13  3:32 ` Xiaoyu Min
  2020-04-15 15:35   ` Raslan Darawsheh
  0 siblings, 1 reply; 3+ messages in thread
From: Xiaoyu Min @ 2020-04-13  3:32 UTC (permalink / raw)
  To: rasland, Matan Azrad, Shahaf Shuler, Viacheslav Ovsiienko
  Cc: dev, stable, Dekel Peled

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: stable@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 <jackmin@mellanox.com>
Reviewed-by: Dekel Peled <dekelp@mellanox.com>
Acked-by: Viacheslav Ovsiienko <viacheslavo@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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [dpdk-dev] [PATCH v2] net/mlx5: fix validation of push VLAN without full mask
  2020-04-13  3:32 ` [dpdk-dev] [PATCH v2] " Xiaoyu Min
@ 2020-04-15 15:35   ` Raslan Darawsheh
  0 siblings, 0 replies; 3+ messages in thread
From: Raslan Darawsheh @ 2020-04-15 15:35 UTC (permalink / raw)
  To: Jack Min, Matan Azrad, Shahaf Shuler, Slava Ovsiienko
  Cc: dev, stable, Dekel Peled

Hi,
> -----Original Message-----
> From: Xiaoyu Min <jackmin@mellanox.com>
> Sent: Monday, April 13, 2020 6:33 AM
> To: Raslan Darawsheh <rasland@mellanox.com>; Matan Azrad
> <matan@mellanox.com>; Shahaf Shuler <shahafs@mellanox.com>; Slava
> Ovsiienko <viacheslavo@mellanox.com>
> Cc: dev@dpdk.org; stable@dpdk.org; Dekel Peled <dekelp@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: stable@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 <jackmin@mellanox.com>
> Reviewed-by: Dekel Peled <dekelp@mellanox.com>
> Acked-by: Viacheslav Ovsiienko <viacheslavo@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


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2020-04-15 15:35 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-02 10:21 [dpdk-dev] [PATCH] net/mlx5: fix validation of push VLAN without full mask Xiaoyu Min
2020-04-13  3:32 ` [dpdk-dev] [PATCH v2] " Xiaoyu Min
2020-04-15 15:35   ` 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).