DPDK patches and discussions
 help / color / mirror / Atom feed
From: Hyong Youb Kim <hyonkim@cisco.com>
To: Ferruh Yigit <ferruh.yigit@intel.com>
Cc: dev@dpdk.org, John Daley <johndale@cisco.com>,
	Hyong Youb Kim <hyonkim@cisco.com>,
	stable@dpdk.org
Subject: [dpdk-dev] [PATCH 02/15] net/enic: fix flow director SCTP matching
Date: Wed, 27 Feb 2019 23:03:04 -0800	[thread overview]
Message-ID: <20190228070317.17002-3-hyonkim@cisco.com> (raw)
In-Reply-To: <20190228070317.17002-1-hyonkim@cisco.com>

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

  parent reply	other threads:[~2019-02-28  7:04 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-28  7:03 [dpdk-dev] [PATCH 00/15] net/enic: 19.05 updates Hyong Youb Kim
2019-02-28  7:03 ` [dpdk-dev] [PATCH 01/15] net/enic: remove unused code Hyong Youb Kim
2019-02-28  7:03 ` Hyong Youb Kim [this message]
2019-02-28  7:03 ` [dpdk-dev] [PATCH 03/15] net/enic: fix SCTP match for flow API Hyong Youb Kim
2019-02-28  7:03 ` [dpdk-dev] [PATCH 04/15] net/enic: allow flow mark ID 0 Hyong Youb Kim
2019-02-28  7:03 ` [dpdk-dev] [PATCH 05/15] net/enic: check for unsupported flow item types Hyong Youb Kim
2019-02-28  7:03 ` [dpdk-dev] [PATCH 06/15] net/enic: enable limited RSS flow action Hyong Youb Kim
2019-02-28  7:03 ` [dpdk-dev] [PATCH 07/15] net/enic: enable limited PASSTHRU " Hyong Youb Kim
2019-02-28  7:03 ` [dpdk-dev] [PATCH 08/15] net/enic: move arguments into struct Hyong Youb Kim
2019-02-28  7:03 ` [dpdk-dev] [PATCH 09/15] net/enic: enable limited support for RAW flow item Hyong Youb Kim
2019-02-28  7:03 ` [dpdk-dev] [PATCH 10/15] net/enic: initialize VXLAN port regardless of overlay offload Hyong Youb Kim
2019-02-28  7:03 ` [dpdk-dev] [PATCH 11/15] net/enic: fix a couple issues with VXLAN match Hyong Youb Kim
2019-02-28  7:03 ` [dpdk-dev] [PATCH 12/15] net/enic: fix an endian bug in VLAN match Hyong Youb Kim
2019-02-28  7:03 ` [dpdk-dev] [PATCH 13/15] net/enic: fix several issues with inner packet matching Hyong Youb Kim
2019-02-28  7:03 ` [dpdk-dev] [PATCH 14/15] doc: update enic guide Hyong Youb Kim
2019-02-28  7:03 ` [dpdk-dev] [PATCH 15/15] doc: update release notes for enic Hyong Youb Kim
2019-03-01 14:26 ` [dpdk-dev] [PATCH 00/15] net/enic: 19.05 updates Thomas Monjalon
2019-03-01 14:56   ` Hyong Youb Kim

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190228070317.17002-3-hyonkim@cisco.com \
    --to=hyonkim@cisco.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=johndale@cisco.com \
    --cc=stable@dpdk.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).