[dpdk-stable] [PATCH] [18.11] net/mlx5: fix match on empty VLAN item in DV mode

[dpdk-stable] [PATCH] [18.11] net/mlx5: fix match on empty VLAN item in DV mode

[Viacheslav Ovsiienko]

From: Dekel Peled <dekelp@mellanox.com>

From: Dekel Peled <dekelp@mellanox.com>

In existing implementation, using wild card VLAN item is not allowed.
A VLAN item in flow pattern must include VLAN ID (vid) value.
This obligation contradict the flow API specification.

This patch updates the VLAN item validation and translation, to allow
wild card VLAN item, without VLAN ID value.
User guide and release notes are updated accordingly.

Fixes: 3d79e4f2d563 ("net/mlx5: fix VLAN match for DV mode")

Signed-off-by: Dekel Peled <dekelp@mellanox.com>
Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
---
 doc/guides/nics/mlx5.rst        | 24 +++++++++++++
 drivers/net/mlx5/mlx5_flow_dv.c | 74 ++++++++++++++++++++++++++++++++++++-----
 2 files changed, 90 insertions(+), 8 deletions(-)

diff --git a/doc/guides/nics/mlx5.rst b/doc/guides/nics/mlx5.rst
index e566f44..189d67c 100644
--- a/doc/guides/nics/mlx5.rst
+++ b/doc/guides/nics/mlx5.rst
@@ -110,6 +110,30 @@ Limitations
   is set to multi-packet send or Enhanced multi-packet send. Otherwise it must have
   less than 50 segments.
 
+- When using DV flow engine (``dv_flow_en`` = 1), flow pattern without VLAN item
+  will match untagged packets only.
+  The flow rule::
+
+        flow create 0 ingress pattern eth / ipv4 / end ...
+
+  Will match untagged packets only.
+  The flow rule::
+
+        flow create 0 ingress pattern eth / vlan / ipv4 / end ...
+
+  Will match tagged packets only, with any VLAN ID value.
+  The flow rule::
+
+        flow create 0 ingress pattern eth / vlan vid is 3 / ipv4 / end ...
+
+  Will only match tagged packets with VLAN ID 3.
+
+- VLAN pop offload command:
+
+  - Flow rules having a VLAN pop offload command as one of their actions and
+    are lacking a match on VLAN as one of their items are not supported.
+  - The command is not supported on egress traffic.
+
 - Count action for RTE flow is **only supported in Mellanox OFED**.
 
 - Flows with a VXLAN Network Identifier equal (or ends to be equal)
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index aee0546..17a298e 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -38,6 +38,60 @@
 #ifdef HAVE_IBV_FLOW_DV_SUPPORT
 
 /**
+ * Validate VLAN item.
+ *
+ * @param[in] item
+ *   Item specification.
+ * @param[in] item_flags
+ *   Bit-fields that holds the items detected until now.
+ * @param[in] dev
+ *   Ethernet device flow is being created on.
+ * @param[out] error
+ *   Pointer to error structure.
+ *
+ * @return
+ *   0 on success, a negative errno value otherwise and rte_errno is set.
+ */
+static int
+flow_dv_validate_item_vlan(const struct rte_flow_item *item,
+			  uint64_t item_flags,
+			  struct rte_eth_dev *dev __rte_unused,
+			  struct rte_flow_error *error)
+{
+	const struct rte_flow_item_vlan *mask = item->mask;
+	const struct rte_flow_item_vlan nic_mask = {
+		.tci = RTE_BE16(UINT16_MAX),
+		.inner_type = RTE_BE16(UINT16_MAX),
+	};
+	const int tunnel = !!(item_flags & MLX5_FLOW_LAYER_TUNNEL);
+	int ret;
+	const uint64_t l34m = tunnel ? (MLX5_FLOW_LAYER_INNER_L3 |
+					MLX5_FLOW_LAYER_INNER_L4) :
+				       (MLX5_FLOW_LAYER_OUTER_L3 |
+					MLX5_FLOW_LAYER_OUTER_L4);
+	const uint64_t vlanm = tunnel ? MLX5_FLOW_LAYER_INNER_VLAN :
+					MLX5_FLOW_LAYER_OUTER_VLAN;
+
+	if (item_flags & vlanm)
+		return rte_flow_error_set(error, EINVAL,
+					  RTE_FLOW_ERROR_TYPE_ITEM, item,
+					  "multiple VLAN layers not supported");
+	else if ((item_flags & l34m) != 0)
+		return rte_flow_error_set(error, EINVAL,
+					  RTE_FLOW_ERROR_TYPE_ITEM, item,
+					  "VLAN cannot follow L3/L4 layer");
+	if (!mask)
+		mask = &rte_flow_item_vlan_mask;
+	ret = mlx5_flow_item_acceptable(item, (const uint8_t *)mask,
+					(const uint8_t *)&nic_mask,
+					sizeof(struct rte_flow_item_vlan),
+					error);
+	if (ret)
+		return ret;
+	return 0;
+}
+
+/**
  * Validate META item.
  *
  * @param[in] dev
@@ -814,8 +868,8 @@
 			}
 			break;
 		case RTE_FLOW_ITEM_TYPE_VLAN:
-			ret = mlx5_flow_validate_item_vlan(items, item_flags,
-							   error);
+			ret = flow_dv_validate_item_vlan(items, item_flags,
+							 dev, error);
 			if (ret < 0)
 				return ret;
 			last_item = tunnel ? MLX5_FLOW_LAYER_INNER_VLAN :
@@ -1229,10 +1283,6 @@
 	uint16_t tci_m;
 	uint16_t tci_v;
 
-	if (!vlan_v)
-		return;
-	if (!vlan_m)
-		vlan_m = &rte_flow_item_vlan_mask;
 	if (inner) {
 		headers_m = MLX5_ADDR_OF(fte_match_param, matcher,
 					 inner_headers);
@@ -1242,10 +1292,18 @@
 					 outer_headers);
 		headers_v = MLX5_ADDR_OF(fte_match_param, key, outer_headers);
 	}
-	tci_m = rte_be_to_cpu_16(vlan_m->tci);
-	tci_v = rte_be_to_cpu_16(vlan_m->tci & vlan_v->tci);
+	/*
+	 * When VLAN item exists in flow, mark packet as tagged,
+	 * even if TCI is not specified.
+	 */
 	MLX5_SET(fte_match_set_lyr_2_4, headers_m, cvlan_tag, 1);
 	MLX5_SET(fte_match_set_lyr_2_4, headers_v, cvlan_tag, 1);
+	if (!vlan_v)
+		return;
+	if (!vlan_m)
+		vlan_m = &rte_flow_item_vlan_mask;
+	tci_m = rte_be_to_cpu_16(vlan_m->tci);
+	tci_v = rte_be_to_cpu_16(vlan_m->tci & vlan_v->tci);
 	MLX5_SET(fte_match_set_lyr_2_4, headers_m, first_vid, tci_m);
 	MLX5_SET(fte_match_set_lyr_2_4, headers_v, first_vid, tci_v);
 	MLX5_SET(fte_match_set_lyr_2_4, headers_m, first_cfi, tci_m >> 12);
-- 
1.8.3.1

Re: [dpdk-stable] [PATCH] [18.11] net/mlx5: fix match on empty VLAN item in DV mode

[Kevin Traynor]

On 08/12/2020 09:23, Viacheslav Ovsiienko wrote:
> From: Dekel Peled <dekelp@mellanox.com>
> 
> From: Dekel Peled <dekelp@mellanox.com>
> 

Removed duplicate From: ^ as it was embedding in commit msg.

Please add upstream commit info for backports. For this one I added

[ upstream commit 92818d839e8eb0ce479db826f00aa6d62384fc92 ]

> In existing implementation, using wild card VLAN item is not allowed.
> A VLAN item in flow pattern must include VLAN ID (vid) value.
> This obligation contradict the flow API specification.
> 
> This patch updates the VLAN item validation and translation, to allow
> wild card VLAN item, without VLAN ID value.
> User guide and release notes are updated accordingly.
> 
> Fixes: 3d79e4f2d563 ("net/mlx5: fix VLAN match for DV mode")
> 

Changed to fixed commit on main branch
Fixes: 00f75a40576b ("net/mlx5: fix VLAN match for DV mode")

If it's a backport where the main branch commit fixes something on main,
then we keep that Fixes: tag. If is fixing an LTS only issue then can
use the LTS branch commit id in the Fixes:.

> Signed-off-by: Dekel Peled <dekelp@mellanox.com>
> Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
> --->  doc/guides/nics/mlx5.rst        | 24 +++++++++++++
>  drivers/net/mlx5/mlx5_flow_dv.c | 74 ++++++++++++++++++++++++++++++++++++-----
>  2 files changed, 90 insertions(+), 8 deletions(-)
> 

Applied, thanks.