[dpdk-stable] [PATCH v2 02/13] net/enic: fix flow director SCTP matching

[dpdk-stable] [PATCH v2 02/13] net/enic: fix flow director SCTP matching

[Hyong Youb Kim]

The firmware filter API does not have flags indicating "match SCTP
packet". Instead, the driver needs to explicitly add an IP match and
set the protocol number (132 for SCTP) in the IP header.

The existing code (copy_fltr_v2) has two bugs.

1. It sets the protocol number (132) in the match value, but not the
mask. The mask remains 0, so the match becomes a wildcard match. The
NIC ends up matching all protocol numbers (i.e. thinks non-SCTP
packets are SCTP).

2. It modifies the input argument (rte_eth_fdir_input). The driver
tracks filters using rte_hash_{add,del}_key(input). So, addding
(RTE_ETH_FILTER_ADD) and deleting (RTE_ETH_FILTER_DELETE) must use the
same input argument for the same filter. But, overwriting the protocol
number while adding the filter breaks this assumption, and causes
delete operation to fail.

So, set the mask as well as protocol value. Do not modify the input
argument, and use const in function signatures to make the intention
clear. Also move a couple function declarations to enic_clsf.c from
enic.h as they are strictly local.

Fixes: dfbd6a9cb504 ("net/enic: extend flow director support for 1300 series")
Cc: stable@dpdk.org

Signed-off-by: Hyong Youb Kim <hyonkim@cisco.com>
Reviewed-by: John Daley <johndale@cisco.com>
---
 drivers/net/enic/enic.h      |  8 ++------
 drivers/net/enic/enic_clsf.c | 38 ++++++++++++++++++++++++++------------
 2 files changed, 28 insertions(+), 18 deletions(-)

diff --git a/drivers/net/enic/enic.h b/drivers/net/enic/enic.h
index 6c497e9a2..fa4d5590e 100644
--- a/drivers/net/enic/enic.h
+++ b/drivers/net/enic/enic.h
@@ -76,8 +76,8 @@ struct enic_fdir {
 	u32 modes;
 	u32 types_mask;
 	void (*copy_fltr_fn)(struct filter_v2 *filt,
-			     struct rte_eth_fdir_input *input,
-			     struct rte_eth_fdir_masks *masks);
+			     const struct rte_eth_fdir_input *input,
+			     const struct rte_eth_fdir_masks *masks);
 };
 
 struct enic_soft_stats {
@@ -342,9 +342,5 @@ int enic_link_update(struct enic *enic);
 bool enic_use_vector_rx_handler(struct enic *enic);
 void enic_fdir_info(struct enic *enic);
 void enic_fdir_info_get(struct enic *enic, struct rte_eth_fdir_info *stats);
-void copy_fltr_v1(struct filter_v2 *fltr, struct rte_eth_fdir_input *input,
-		  struct rte_eth_fdir_masks *masks);
-void copy_fltr_v2(struct filter_v2 *fltr, struct rte_eth_fdir_input *input,
-		  struct rte_eth_fdir_masks *masks);
 extern const struct rte_flow_ops enic_flow_ops;
 #endif /* _ENIC_H_ */
diff --git a/drivers/net/enic/enic_clsf.c b/drivers/net/enic/enic_clsf.c
index 9e9e548c2..48c8e6264 100644
--- a/drivers/net/enic/enic_clsf.c
+++ b/drivers/net/enic/enic_clsf.c
@@ -36,6 +36,13 @@
 
 #define ENICPMD_CLSF_HASH_ENTRIES       ENICPMD_FDIR_MAX
 
+static void copy_fltr_v1(struct filter_v2 *fltr,
+		const struct rte_eth_fdir_input *input,
+		const struct rte_eth_fdir_masks *masks);
+static void copy_fltr_v2(struct filter_v2 *fltr,
+		const struct rte_eth_fdir_input *input,
+		const struct rte_eth_fdir_masks *masks);
+
 void enic_fdir_stats_get(struct enic *enic, struct rte_eth_fdir_stats *stats)
 {
 	*stats = enic->fdir.stats;
@@ -79,9 +86,9 @@ enic_set_layer(struct filter_generic_1 *gp, unsigned int flag,
 /* Copy Flow Director filter to a VIC ipv4 filter (for Cisco VICs
  * without advanced filter support.
  */
-void
-copy_fltr_v1(struct filter_v2 *fltr, struct rte_eth_fdir_input *input,
-	     __rte_unused struct rte_eth_fdir_masks *masks)
+static void
+copy_fltr_v1(struct filter_v2 *fltr, const struct rte_eth_fdir_input *input,
+	     __rte_unused const struct rte_eth_fdir_masks *masks)
 {
 	fltr->type = FILTER_IPV4_5TUPLE;
 	fltr->u.ipv4.src_addr = rte_be_to_cpu_32(
@@ -104,9 +111,9 @@ copy_fltr_v1(struct filter_v2 *fltr, struct rte_eth_fdir_input *input,
 /* Copy Flow Director filter to a VIC generic filter (requires advanced
  * filter support.
  */
-void
-copy_fltr_v2(struct filter_v2 *fltr, struct rte_eth_fdir_input *input,
-	     struct rte_eth_fdir_masks *masks)
+static void
+copy_fltr_v2(struct filter_v2 *fltr, const struct rte_eth_fdir_input *input,
+	     const struct rte_eth_fdir_masks *masks)
 {
 	struct filter_generic_1 *gp = &fltr->u.generic_1;
 
@@ -163,9 +170,11 @@ copy_fltr_v2(struct filter_v2 *fltr, struct rte_eth_fdir_input *input,
 			sctp_val.tag = input->flow.sctp4_flow.verify_tag;
 		}
 
-		/* v4 proto should be 132, override ip4_flow.proto */
-		input->flow.ip4_flow.proto = 132;
-
+		/*
+		 * Unlike UDP/TCP (FILTER_GENERIC_1_{UDP,TCP}), the firmware
+		 * has no "packet is SCTP" flag. Use flag=0 (generic L4) and
+		 * manually set proto_id=sctp below.
+		 */
 		enic_set_layer(gp, 0, FILTER_GENERIC_1_L4, &sctp_mask,
 			       &sctp_val, sizeof(struct sctp_hdr));
 	}
@@ -189,6 +198,10 @@ copy_fltr_v2(struct filter_v2 *fltr, struct rte_eth_fdir_input *input,
 		if (input->flow.ip4_flow.proto) {
 			ip4_mask.next_proto_id = masks->ipv4_mask.proto;
 			ip4_val.next_proto_id = input->flow.ip4_flow.proto;
+		} else if (input->flow_type == RTE_ETH_FLOW_NONFRAG_IPV4_SCTP) {
+			/* Explicitly match the SCTP protocol number */
+			ip4_mask.next_proto_id = 0xff;
+			ip4_val.next_proto_id = IPPROTO_SCTP;
 		}
 		if (input->flow.ip4_flow.src_ip) {
 			ip4_mask.src_addr =  masks->ipv4_mask.src_ip;
@@ -251,9 +264,6 @@ copy_fltr_v2(struct filter_v2 *fltr, struct rte_eth_fdir_input *input,
 			sctp_val.tag = input->flow.sctp6_flow.verify_tag;
 		}
 
-		/* v4 proto should be 132, override ipv6_flow.proto */
-		input->flow.ipv6_flow.proto = 132;
-
 		enic_set_layer(gp, 0, FILTER_GENERIC_1_L4, &sctp_mask,
 			       &sctp_val, sizeof(struct sctp_hdr));
 	}
@@ -269,6 +279,10 @@ copy_fltr_v2(struct filter_v2 *fltr, struct rte_eth_fdir_input *input,
 		if (input->flow.ipv6_flow.proto) {
 			ipv6_mask.proto = masks->ipv6_mask.proto;
 			ipv6_val.proto = input->flow.ipv6_flow.proto;
+		} else if (input->flow_type == RTE_ETH_FLOW_NONFRAG_IPV6_SCTP) {
+			/* See comments for IPv4 SCTP above. */
+			ipv6_mask.proto = 0xff;
+			ipv6_val.proto = IPPROTO_SCTP;
 		}
 		memcpy(ipv6_mask.src_addr, masks->ipv6_mask.src_ip,
 		       sizeof(ipv6_mask.src_addr));
-- 
2.16.2

[dpdk-stable] [PATCH v2 03/13] net/enic: fix SCTP match for flow API

[Hyong Youb Kim]

The driver needs to explicitly set the protocol number (132) in the IP
header pattern, as the current firmware filter API lacks "match SCTP
packet" flag. Otherwise, the resulting NIC filter may lead to false
positives (i.e. NIC reporting non-SCTP packets as SCTP packets). The
flow director handler does the same (enic_clsf.c).

Fixes: 6ced137607d0 ("net/enic: flow API for NICs with advanced filters enabled")
Cc: stable@dpdk.org

Signed-off-by: Hyong Youb Kim <hyonkim@cisco.com>
Reviewed-by: John Daley <johndale@cisco.com>
---
 drivers/net/enic/enic_flow.c | 28 ++++++++++++++++++++++++++--
 1 file changed, 26 insertions(+), 2 deletions(-)

diff --git a/drivers/net/enic/enic_flow.c b/drivers/net/enic/enic_flow.c
index bb9ed037a..55d8d50a1 100644
--- a/drivers/net/enic/enic_flow.c
+++ b/drivers/net/enic/enic_flow.c
@@ -70,7 +70,6 @@ static enic_copy_item_fn enic_copy_item_ipv6_v2;
 static enic_copy_item_fn enic_copy_item_udp_v2;
 static enic_copy_item_fn enic_copy_item_tcp_v2;
 static enic_copy_item_fn enic_copy_item_sctp_v2;
-static enic_copy_item_fn enic_copy_item_sctp_v2;
 static enic_copy_item_fn enic_copy_item_vxlan_v2;
 static copy_action_fn enic_copy_action_v1;
 static copy_action_fn enic_copy_action_v2;
@@ -237,7 +236,7 @@ static const struct enic_items enic_items_v3[] = {
 	},
 	[RTE_FLOW_ITEM_TYPE_SCTP] = {
 		.copy_item = enic_copy_item_sctp_v2,
-		.valid_start_item = 1,
+		.valid_start_item = 0,
 		.prev_items = (const enum rte_flow_item_type[]) {
 			       RTE_FLOW_ITEM_TYPE_IPV4,
 			       RTE_FLOW_ITEM_TYPE_IPV6,
@@ -819,12 +818,37 @@ enic_copy_item_sctp_v2(const struct rte_flow_item *item,
 	const struct rte_flow_item_sctp *spec = item->spec;
 	const struct rte_flow_item_sctp *mask = item->mask;
 	struct filter_generic_1 *gp = &enic_filter->u.generic_1;
+	uint8_t *ip_proto_mask = NULL;
+	uint8_t *ip_proto = NULL;
 
 	FLOW_TRACE();
 
 	if (*inner_ofst)
 		return ENOTSUP;
 
+	/*
+	 * The NIC filter API has no flags for "match sctp", so explicitly set
+	 * the protocol number in the IP pattern.
+	 */
+	if (gp->val_flags & FILTER_GENERIC_1_IPV4) {
+		struct ipv4_hdr *ip;
+		ip = (struct ipv4_hdr *)gp->layer[FILTER_GENERIC_1_L3].mask;
+		ip_proto_mask = &ip->next_proto_id;
+		ip = (struct ipv4_hdr *)gp->layer[FILTER_GENERIC_1_L3].val;
+		ip_proto = &ip->next_proto_id;
+	} else if (gp->val_flags & FILTER_GENERIC_1_IPV6) {
+		struct ipv6_hdr *ip;
+		ip = (struct ipv6_hdr *)gp->layer[FILTER_GENERIC_1_L3].mask;
+		ip_proto_mask = &ip->proto;
+		ip = (struct ipv6_hdr *)gp->layer[FILTER_GENERIC_1_L3].val;
+		ip_proto = &ip->proto;
+	} else {
+		/* Need IPv4/IPv6 pattern first */
+		return EINVAL;
+	}
+	*ip_proto = IPPROTO_SCTP;
+	*ip_proto_mask = 0xff;
+
 	/* Match all if no spec */
 	if (!spec)
 		return 0;
-- 
2.16.2

[dpdk-stable] [PATCH v2 04/13] net/enic: allow flow mark ID 0

[Hyong Youb Kim]

The driver currently accepts mark ID 0 but does not report it in
matching packet's mbuf. For example, the following testpmd command
succeeds. But, the mbuf of a matching IPv4 UDP packet does not have
PKT_RX_FDIR_ID set.

flow create 0 ingress pattern ... actions mark id 0 / queue index 0 / end

The problem has to do with mapping mark IDs (32-bit) to NIC filter
IDs. Filter ID is currently 16-bit, so values greater than 0xffff are
rejected. The firmware reserves filter ID 0 for filters that do not
mark (e.g. steer w/o mark). And, the driver reserves 0xffff for the
flag action. This leaves 1...0xfffe for app use.

It is possible to simply reject mark ID 0 as unsupported. But, 0 is
commonly used (e.g. OVS-DPDK and VPP). So, when adding a filter, set
filter ID = mark ID + 1 to support mark ID 0. The receive handler
subtracts 1 from filter ID to get back the original mark ID.

Fixes: dfbd6a9cb504 ("net/enic: extend flow director support for 1300 series")
Cc: stable@dpdk.org

Signed-off-by: Hyong Youb Kim <hyonkim@cisco.com>
Reviewed-by: John Daley <johndale@cisco.com>
---
 doc/guides/nics/enic.rst            |  1 +
 drivers/net/enic/enic_flow.c        | 15 +++++++++++----
 drivers/net/enic/enic_rxtx_common.h |  3 ++-
 3 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/doc/guides/nics/enic.rst b/doc/guides/nics/enic.rst
index bc38f51aa..e456e6c2d 100644
--- a/doc/guides/nics/enic.rst
+++ b/doc/guides/nics/enic.rst
@@ -450,6 +450,7 @@ PKT_RX_VLAN_STRIPPED mbuf flags would not be set. This mode is enabled with the
     1000 for 1300 series VICs). Filters are checked for matching in the order they
     were added. Since there currently is no grouping or priority support,
     'catch-all' filters should be added last.
+  - The supported range of IDs for the 'MARK' action is 0 - 0xFFFD.
 
 - **Statistics**
 
diff --git a/drivers/net/enic/enic_flow.c b/drivers/net/enic/enic_flow.c
index 55d8d50a1..e12a6ec73 100644
--- a/drivers/net/enic/enic_flow.c
+++ b/drivers/net/enic/enic_flow.c
@@ -1081,12 +1081,18 @@ enic_copy_action_v2(const struct rte_flow_action actions[],
 			if (overlap & MARK)
 				return ENOTSUP;
 			overlap |= MARK;
-			/* ENIC_MAGIC_FILTER_ID is reserved and is the highest
-			 * in the range of allows mark ids.
+			/*
+			 * Map mark ID (32-bit) to filter ID (16-bit):
+			 * - Reject values > 16 bits
+			 * - Filter ID 0 is reserved for filters that steer
+			 *   but not mark. So add 1 to the mark ID to avoid
+			 *   using 0.
+			 * - Filter ID (ENIC_MAGIC_FILTER_ID = 0xffff) is
+			 *   reserved for the "flag" action below.
 			 */
-			if (mark->id >= ENIC_MAGIC_FILTER_ID)
+			if (mark->id >= ENIC_MAGIC_FILTER_ID - 1)
 				return EINVAL;
-			enic_action->filter_id = mark->id;
+			enic_action->filter_id = mark->id + 1;
 			enic_action->flags |= FILTER_ACTION_FILTER_ID_FLAG;
 			break;
 		}
@@ -1094,6 +1100,7 @@ enic_copy_action_v2(const struct rte_flow_action actions[],
 			if (overlap & MARK)
 				return ENOTSUP;
 			overlap |= MARK;
+			/* ENIC_MAGIC_FILTER_ID is reserved for flagging */
 			enic_action->filter_id = ENIC_MAGIC_FILTER_ID;
 			enic_action->flags |= FILTER_ACTION_FILTER_ID_FLAG;
 			break;
diff --git a/drivers/net/enic/enic_rxtx_common.h b/drivers/net/enic/enic_rxtx_common.h
index bfbb4909e..66f631dfe 100644
--- a/drivers/net/enic/enic_rxtx_common.h
+++ b/drivers/net/enic/enic_rxtx_common.h
@@ -226,7 +226,8 @@ enic_cq_rx_to_pkt_flags(struct cq_desc *cqd, struct rte_mbuf *mbuf)
 		if (filter_id) {
 			pkt_flags |= PKT_RX_FDIR;
 			if (filter_id != ENIC_MAGIC_FILTER_ID) {
-				mbuf->hash.fdir.hi = clsf_cqd->filter_id;
+				/* filter_id = mark id + 1, so subtract 1 */
+				mbuf->hash.fdir.hi = filter_id - 1;
 				pkt_flags |= PKT_RX_FDIR_ID;
 			}
 		}
-- 
2.16.2

[dpdk-stable] [PATCH v2 05/13] net/enic: check for unsupported flow item types

[Hyong Youb Kim]

Currently a pattern with an unsupported item type causes segfault,
because the flow handler is using the type as an array index without
checking bounds. Add an explicit check for unsupported item types and
avoid out-of-bound accesses.

Fixes: 6ced137607d0 ("net/enic: flow API for NICs with advanced filters enabled")
Cc: stable@dpdk.org

Signed-off-by: Hyong Youb Kim <hyonkim@cisco.com>
Reviewed-by: John Daley <johndale@cisco.com>
---
 drivers/net/enic/enic_flow.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/net/enic/enic_flow.c b/drivers/net/enic/enic_flow.c
index e12a6ec73..c60476c8c 100644
--- a/drivers/net/enic/enic_flow.c
+++ b/drivers/net/enic/enic_flow.c
@@ -40,6 +40,8 @@ struct enic_items {
 struct enic_filter_cap {
 	/** list of valid items and their handlers and attributes. */
 	const struct enic_items *item_info;
+	/* Max type in the above list, used to detect unsupported types */
+	enum rte_flow_item_type max_item_type;
 };
 
 /* functions for copying flow actions into enic actions */
@@ -257,12 +259,15 @@ static const struct enic_items enic_items_v3[] = {
 static const struct enic_filter_cap enic_filter_cap[] = {
 	[FILTER_IPV4_5TUPLE] = {
 		.item_info = enic_items_v1,
+		.max_item_type = RTE_FLOW_ITEM_TYPE_TCP,
 	},
 	[FILTER_USNIC_IP] = {
 		.item_info = enic_items_v2,
+		.max_item_type = RTE_FLOW_ITEM_TYPE_VXLAN,
 	},
 	[FILTER_DPDK_1] = {
 		.item_info = enic_items_v3,
+		.max_item_type = RTE_FLOW_ITEM_TYPE_VXLAN,
 	},
 };
 
@@ -946,7 +951,7 @@ item_stacking_valid(enum rte_flow_item_type prev_item,
  */
 static int
 enic_copy_filter(const struct rte_flow_item pattern[],
-		 const struct enic_items *items_info,
+		 const struct enic_filter_cap *cap,
 		 struct filter_v2 *enic_filter,
 		 struct rte_flow_error *error)
 {
@@ -969,7 +974,14 @@ enic_copy_filter(const struct rte_flow_item pattern[],
 		if (item->type == RTE_FLOW_ITEM_TYPE_VOID)
 			continue;
 
-		item_info = &items_info[item->type];
+		item_info = &cap->item_info[item->type];
+		if (item->type > cap->max_item_type ||
+		    item_info->copy_item == NULL) {
+			rte_flow_error_set(error, ENOTSUP,
+				RTE_FLOW_ERROR_TYPE_ITEM,
+				NULL, "Unsupported item.");
+			return -rte_errno;
+		}
 
 		/* check to see if item stacking is valid */
 		if (!item_stacking_valid(prev_item, item_info, is_first_item))
@@ -1423,7 +1435,7 @@ enic_flow_parse(struct rte_eth_dev *dev,
 		return -rte_errno;
 	}
 	enic_filter->type = enic->flow_filter_mode;
-	ret = enic_copy_filter(pattern, enic_filter_cap->item_info,
+	ret = enic_copy_filter(pattern, enic_filter_cap,
 				       enic_filter, error);
 	return ret;
 }
-- 
2.16.2

[dpdk-stable] [PATCH v2 10/13] net/enic: reset VXLAN port regardless of overlay offload

[Hyong Youb Kim]

Currently, the driver resets the vxlan port register only if overlay
offload is enabled. But, the register is actually tied to hardware
vxlan parsing, which is an independent feature and is always enabled
even if overlay offload is disabled. If left uninitialized, it can
affect flow rules that match vxlan. So always reset the port number
when HW vxlan parsing is available.

Fixes: 8a4efd17410c ("net/enic: add handlers to add/delete vxlan port number")
Cc: stable@dpdk.org

Signed-off-by: Hyong Youb Kim <hyonkim@cisco.com>
---
 drivers/net/enic/enic_main.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/net/enic/enic_main.c b/drivers/net/enic/enic_main.c
index 2652949a2..ea9eb2edf 100644
--- a/drivers/net/enic/enic_main.c
+++ b/drivers/net/enic/enic_main.c
@@ -1714,8 +1714,15 @@ static int enic_dev_init(struct enic *enic)
 			PKT_TX_OUTER_IP_CKSUM |
 			PKT_TX_TUNNEL_MASK;
 		enic->overlay_offload = true;
-		enic->vxlan_port = ENIC_DEFAULT_VXLAN_PORT;
 		dev_info(enic, "Overlay offload is enabled\n");
+	}
+	/*
+	 * Reset the vxlan port if HW vxlan parsing is available. It
+	 * is always enabled regardless of overlay offload
+	 * enable/disable.
+	 */
+	if (enic->vxlan) {
+		enic->vxlan_port = ENIC_DEFAULT_VXLAN_PORT;
 		/*
 		 * Reset the vxlan port to the default, as the NIC firmware
 		 * does not reset it automatically and keeps the old setting.
-- 
2.16.2

[dpdk-stable] [PATCH v2 11/13] net/enic: fix a couple issues with VXLAN match

[Hyong Youb Kim]

The filter API does not have flags for "match VXLAN". Explicitly set
the UDP destination port and mask in the L4 pattern. Otherwise, UDP
packets with non-VXLAN ports may be falsely reported as VXLAN.

1400 series VIC adapters have hardware VXLAN parsing. The L5 buffer on
the NIC starts with the inner Ethernet header, and the VXLAN header is
now in the L4 buffer following the UDP header. So the VXLAN spec/mask
needs to be in the L4 pattern, not L5. Older models still expect the
VXLAN spec/mask in the L5 pattern. Fix up the L4/L5 patterns
accordingly.

Fixes: 6ced137607d0 ("net/enic: flow API for NICs with advanced filters enabled")
Cc: stable@dpdk.org

Signed-off-by: Hyong Youb Kim <hyonkim@cisco.com>
---
 drivers/net/enic/enic_flow.c | 46 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 45 insertions(+), 1 deletion(-)

diff --git a/drivers/net/enic/enic_flow.c b/drivers/net/enic/enic_flow.c
index ffc6ce1da..da43b31dc 100644
--- a/drivers/net/enic/enic_flow.c
+++ b/drivers/net/enic/enic_flow.c
@@ -830,12 +830,23 @@ enic_copy_item_vxlan_v2(struct copy_item_args *arg)
 	const struct rte_flow_item_vxlan *spec = item->spec;
 	const struct rte_flow_item_vxlan *mask = item->mask;
 	struct filter_generic_1 *gp = &enic_filter->u.generic_1;
+	struct udp_hdr *udp;
 
 	FLOW_TRACE();
 
 	if (*inner_ofst)
 		return EINVAL;
 
+	/*
+	 * The NIC filter API has no flags for "match vxlan". Set UDP port to
+	 * avoid false positives.
+	 */
+	gp->mask_flags |= FILTER_GENERIC_1_UDP;
+	gp->val_flags |= FILTER_GENERIC_1_UDP;
+	udp = (struct udp_hdr *)gp->layer[FILTER_GENERIC_1_L4].mask;
+	udp->dst_port = 0xffff;
+	udp = (struct udp_hdr *)gp->layer[FILTER_GENERIC_1_L4].val;
+	udp->dst_port = RTE_BE16(4789);
 	/* Match all if no spec */
 	if (!spec)
 		return 0;
@@ -931,6 +942,36 @@ item_stacking_valid(enum rte_flow_item_type prev_item,
 	return 0;
 }
 
+/*
+ * Fix up the L5 layer.. HW vxlan parsing removes vxlan header from L5.
+ * Instead it is in L4 following the UDP header. Append the vxlan
+ * pattern to L4 (udp) and shift any inner packet pattern in L5.
+ */
+static void
+fixup_l5_layer(struct enic *enic, struct filter_generic_1 *gp,
+	       uint8_t inner_ofst)
+{
+	uint8_t layer[FILTER_GENERIC_1_KEY_LEN];
+	uint8_t inner;
+	uint8_t vxlan;
+
+	if (!(inner_ofst > 0 && enic->vxlan))
+		return;
+	FLOW_TRACE();
+	vxlan = sizeof(struct vxlan_hdr);
+	memcpy(gp->layer[FILTER_GENERIC_1_L4].mask + sizeof(struct udp_hdr),
+	       gp->layer[FILTER_GENERIC_1_L5].mask, vxlan);
+	memcpy(gp->layer[FILTER_GENERIC_1_L4].val + sizeof(struct udp_hdr),
+	       gp->layer[FILTER_GENERIC_1_L5].val, vxlan);
+	inner = inner_ofst - vxlan;
+	memset(layer, 0, sizeof(layer));
+	memcpy(layer, gp->layer[FILTER_GENERIC_1_L5].mask + vxlan, inner);
+	memcpy(gp->layer[FILTER_GENERIC_1_L5].mask, layer, sizeof(layer));
+	memset(layer, 0, sizeof(layer));
+	memcpy(layer, gp->layer[FILTER_GENERIC_1_L5].val + vxlan, inner);
+	memcpy(gp->layer[FILTER_GENERIC_1_L5].val, layer, sizeof(layer));
+}
+
 /**
  * Build the intenal enic filter structure from the provided pattern. The
  * pattern is validated as the items are copied.
@@ -945,6 +986,7 @@ item_stacking_valid(enum rte_flow_item_type prev_item,
 static int
 enic_copy_filter(const struct rte_flow_item pattern[],
 		 const struct enic_filter_cap *cap,
+		 struct enic *enic,
 		 struct filter_v2 *enic_filter,
 		 struct rte_flow_error *error)
 {
@@ -989,6 +1031,8 @@ enic_copy_filter(const struct rte_flow_item pattern[],
 		prev_item = item->type;
 		is_first_item = 0;
 	}
+	fixup_l5_layer(enic, &enic_filter->u.generic_1, inner_ofst);
+
 	return 0;
 
 item_not_supported:
@@ -1481,7 +1525,7 @@ enic_flow_parse(struct rte_eth_dev *dev,
 		return -rte_errno;
 	}
 	enic_filter->type = enic->flow_filter_mode;
-	ret = enic_copy_filter(pattern, enic_filter_cap,
+	ret = enic_copy_filter(pattern, enic_filter_cap, enic,
 				       enic_filter, error);
 	return ret;
 }
-- 
2.16.2

[dpdk-stable] [PATCH v2 12/13] net/enic: fix an endian bug in VLAN match

[Hyong Youb Kim]

The VLAN fields in the NIC filter use little endian. The VLAN item is
in big endian, so swap bytes.

Fixes: 6ced137607d0 ("net/enic: flow API for NICs with advanced filters enabled")
Cc: stable@dpdk.org

Signed-off-by: Hyong Youb Kim <hyonkim@cisco.com>
---
 doc/guides/nics/enic.rst     | 10 ++++++++--
 drivers/net/enic/enic_flow.c | 12 ++++++++----
 2 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/doc/guides/nics/enic.rst b/doc/guides/nics/enic.rst
index c1415dc0d..d4241ef45 100644
--- a/doc/guides/nics/enic.rst
+++ b/doc/guides/nics/enic.rst
@@ -247,7 +247,7 @@ Generic Flow API is supported. The baseline support is:
   in the pattern.
 
   - Attributes: ingress
-  - Items: eth, ipv4, ipv6, udp, tcp, vxlan, inner eth, ipv4, ipv6, udp, tcp
+  - Items: eth, vlan, ipv4, ipv6, udp, tcp, vxlan, inner eth, vlan, ipv4, ipv6, udp, tcp
   - Actions: queue and void
   - Selectors: 'is', 'spec' and 'mask'. 'last' is not supported
   - In total, up to 64 bytes of mask is allowed across all headers
@@ -255,7 +255,7 @@ Generic Flow API is supported. The baseline support is:
 - **1300 and later series VICS with advanced filters enabled**
 
   - Attributes: ingress
-  - Items: eth, ipv4, ipv6, udp, tcp, vxlan, raw, inner eth, ipv4, ipv6, udp, tcp
+  - Items: eth, vlan, ipv4, ipv6, udp, tcp, vxlan, raw, inner eth, vlan, ipv4, ipv6, udp, tcp
   - Actions: queue, mark, drop, flag, rss, passthru, and void
   - Selectors: 'is', 'spec' and 'mask'. 'last' is not supported
   - In total, up to 64 bytes of mask is allowed across all headers
@@ -266,6 +266,12 @@ Generic Flow API is supported. The baseline support is:
 
   - Action: count
 
+The VIC performs packet matching after applying VLAN strip. If VLAN
+stripping is enabled, EtherType in the ETH item corresponds to the
+stripped VLAN header's EtherType. Stripping does not affect the VLAN
+item. TCI and EtherType in the VLAN item are matched against those in
+the (stripped) VLAN header whether stripping is enabled or disabled.
+
 More features may be added in future firmware and new versions of the VIC.
 Please refer to the release notes.
 
diff --git a/drivers/net/enic/enic_flow.c b/drivers/net/enic/enic_flow.c
index da43b31dc..b3172e7be 100644
--- a/drivers/net/enic/enic_flow.c
+++ b/drivers/net/enic/enic_flow.c
@@ -579,12 +579,16 @@ enic_copy_item_vlan_v2(struct copy_item_args *arg)
 		/* Outer TPID cannot be matched */
 		if (eth_mask->ether_type)
 			return ENOTSUP;
+		/*
+		 * When packet matching, the VIC always compares vlan-stripped
+		 * L2, regardless of vlan stripping settings. So, the inner type
+		 * from vlan becomes the ether type of the eth header.
+		 */
 		eth_mask->ether_type = mask->inner_type;
 		eth_val->ether_type = spec->inner_type;
-
-		/* Outer header. Use the vlan mask/val fields */
-		gp->mask_vlan = mask->tci;
-		gp->val_vlan = spec->tci;
+		/* For TCI, use the vlan mask/val fields (little endian). */
+		gp->mask_vlan = rte_be_to_cpu_16(mask->tci);
+		gp->val_vlan = rte_be_to_cpu_16(spec->tci);
 	} else {
 		/* Inner header. Mask/Val start at *inner_ofst into L5 */
 		if ((*inner_ofst + sizeof(struct vlan_hdr)) >
-- 
2.16.2

Re: [dpdk-stable] [dpdk-dev] [PATCH v2 13/13] net/enic: fix several issues with inner packet matching

[Kevin Traynor]

On 04/03/2019 16:58, Ferruh Yigit wrote:
> On 3/2/2019 10:42 AM, Hyong Youb Kim wrote:
>> Inner packet matching is currently buggy in many cases.
>>
>> 1. Mishandling null spec ("match any").
>> The copy_item functions do nothing if spec is null. This is incorrect,
>> as all patterns should be appended to the L5 pattern buffer even for
>> null spec (treated as all zeros).
>>
>> 2. Accessing null spec causing segfault.
>>
>> 3. Not setting protocol fields.
>> The NIC filter API currently has no flags for "match inner IPv4, IPv6,
>> UDP, TCP, and so on". So, the driver needs to explicitly set EtherType
>> and IP protocol fields in the L5 pattern buffer to avoid false
>> positives (e.g. reporting IPv6 as IPv4).
>>
>> Instead of keep adding "if inner, do something differently" cases to
>> the existing copy_item functions, introduce separate functions for
>> inner packet patterns and address the above issues in those
>> functions. The changes to the previous outer-packet copy_item
>> functions are mechanical, due to reduced indentation.
>>
>> Fixes: 6ced137607d0 ("net/enic: flow API for NICs with advanced filters enabled")
>>
>> Signed-off-by: Hyong Youb Kim <hyonkim@cisco.com>
> 
> <...>
> 
> I have added "Cc: stable@dpdk.org" tag while merging. If the tag explicitly left
> out to prevent backport please let me know to remove the tag back.
> 

Hi Hyong,

I can't apply this patch with confidence on the 18.11 LTS branch due to
the amount of changes and conflicts. The other relevant patches in the
series for 18.11 were ok.

If you want the fixes from this patch on stable branches, can you please
send a backport for them
(http://doc.dpdk.org/guides/contributing/patches.html?highlight=stable#backporting-patches-for-stable-releases).

Otherwise, please let us know that you don't want them on stable branches.

thanks,
Kevin.