netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [patch net-next 0/5] make flow dissector great again
@ 2017-03-06 15:39 Jiri Pirko
  2017-03-06 15:39 ` [patch net-next 1/5] flow_dissector: Move ARP dissection into a separate function Jiri Pirko
                   ` (5 more replies)
  0 siblings, 6 replies; 7+ messages in thread
From: Jiri Pirko @ 2017-03-06 15:39 UTC (permalink / raw)
  To: netdev
  Cc: davem, tom, simon.horman, eric.dumazet, dinan.gunawardena, hadarh,
	e, fgao, mlxsw

From: Jiri Pirko <jiri@mellanox.com>

This patchset follows-up the discussion about future extensions of flow
dissector and tries to address the mentioned concerns. Some parts are
cut out into sub-functions. Also, the processing of the code (ARP, MPLS)
is made dependent on user actually requiring the bisected values.
This prepares the code for future extensions to bisect IPv6 ND messages,
TCP flags, etc.

Jiri Pirko (5):
  flow_dissector: Move ARP dissection into a separate function
  flow_dissector: Move MPLS dissection into a separate function
  flow_dissector: Fix GRE header error path
  flow_dissector: rename "proto again" goto label
  flow_dissector: Move GRE dissection into a separate function

 net/core/flow_dissector.c | 426 ++++++++++++++++++++++++++--------------------
 1 file changed, 238 insertions(+), 188 deletions(-)

-- 
2.7.4

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

* [patch net-next 1/5] flow_dissector: Move ARP dissection into a separate function
  2017-03-06 15:39 [patch net-next 0/5] make flow dissector great again Jiri Pirko
@ 2017-03-06 15:39 ` Jiri Pirko
  2017-03-06 15:39 ` [patch net-next 2/5] flow_dissector: Move MPLS " Jiri Pirko
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Jiri Pirko @ 2017-03-06 15:39 UTC (permalink / raw)
  To: netdev
  Cc: davem, tom, simon.horman, eric.dumazet, dinan.gunawardena, hadarh,
	e, fgao, mlxsw

From: Jiri Pirko <jiri@mellanox.com>

Make the main flow_dissect function a bit smaller and move the ARP
dissection into a separate function. Along with that, do the ARP header
processing only in case the flow dissection user requires it.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 net/core/flow_dissector.c | 120 ++++++++++++++++++++++++++--------------------
 1 file changed, 67 insertions(+), 53 deletions(-)

diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index c35aae1..d79fb8f 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -113,6 +113,66 @@ __be32 __skb_flow_get_ports(const struct sk_buff *skb, int thoff, u8 ip_proto,
 }
 EXPORT_SYMBOL(__skb_flow_get_ports);
 
+enum flow_dissect_ret {
+	FLOW_DISSECT_RET_OUT_GOOD,
+	FLOW_DISSECT_RET_OUT_BAD,
+};
+
+static enum flow_dissect_ret
+__skb_flow_dissect_arp(const struct sk_buff *skb,
+		       struct flow_dissector *flow_dissector,
+		       void *target_container, void *data, int nhoff, int hlen)
+{
+	struct flow_dissector_key_arp *key_arp;
+	struct {
+		unsigned char ar_sha[ETH_ALEN];
+		unsigned char ar_sip[4];
+		unsigned char ar_tha[ETH_ALEN];
+		unsigned char ar_tip[4];
+	} *arp_eth, _arp_eth;
+	const struct arphdr *arp;
+	struct arphdr *_arp;
+
+	if (!dissector_uses_key(flow_dissector, FLOW_DISSECTOR_KEY_ARP))
+		return FLOW_DISSECT_RET_OUT_GOOD;
+
+	arp = __skb_header_pointer(skb, nhoff, sizeof(_arp), data,
+				   hlen, &_arp);
+	if (!arp)
+		return FLOW_DISSECT_RET_OUT_BAD;
+
+	if (arp->ar_hrd != htons(ARPHRD_ETHER) ||
+	    arp->ar_pro != htons(ETH_P_IP) ||
+	    arp->ar_hln != ETH_ALEN ||
+	    arp->ar_pln != 4 ||
+	    (arp->ar_op != htons(ARPOP_REPLY) &&
+	     arp->ar_op != htons(ARPOP_REQUEST)))
+		return FLOW_DISSECT_RET_OUT_BAD;
+
+	arp_eth = __skb_header_pointer(skb, nhoff + sizeof(_arp),
+				       sizeof(_arp_eth), data,
+				       hlen, &_arp_eth);
+	if (!arp_eth)
+		return FLOW_DISSECT_RET_OUT_BAD;
+
+	key_arp = skb_flow_dissector_target(flow_dissector,
+					    FLOW_DISSECTOR_KEY_ARP,
+					    target_container);
+
+	memcpy(&key_arp->sip, arp_eth->ar_sip, sizeof(key_arp->sip));
+	memcpy(&key_arp->tip, arp_eth->ar_tip, sizeof(key_arp->tip));
+
+	/* Only store the lower byte of the opcode;
+	 * this covers ARPOP_REPLY and ARPOP_REQUEST.
+	 */
+	key_arp->op = ntohs(arp->ar_op) & 0xff;
+
+	ether_addr_copy(key_arp->sha, arp_eth->ar_sha);
+	ether_addr_copy(key_arp->tha, arp_eth->ar_tha);
+
+	return FLOW_DISSECT_RET_OUT_GOOD;
+}
+
 /**
  * __skb_flow_dissect - extract the flow_keys struct and return it
  * @skb: sk_buff to extract the flow from, can be NULL if the rest are specified
@@ -138,7 +198,6 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
 	struct flow_dissector_key_control *key_control;
 	struct flow_dissector_key_basic *key_basic;
 	struct flow_dissector_key_addrs *key_addrs;
-	struct flow_dissector_key_arp *key_arp;
 	struct flow_dissector_key_ports *key_ports;
 	struct flow_dissector_key_icmp *key_icmp;
 	struct flow_dissector_key_tags *key_tags;
@@ -382,60 +441,15 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
 		goto out_good;
 
 	case htons(ETH_P_ARP):
-	case htons(ETH_P_RARP): {
-		struct {
-			unsigned char ar_sha[ETH_ALEN];
-			unsigned char ar_sip[4];
-			unsigned char ar_tha[ETH_ALEN];
-			unsigned char ar_tip[4];
-		} *arp_eth, _arp_eth;
-		const struct arphdr *arp;
-		struct arphdr *_arp;
-
-		arp = __skb_header_pointer(skb, nhoff, sizeof(_arp), data,
-					   hlen, &_arp);
-		if (!arp)
-			goto out_bad;
-
-		if (arp->ar_hrd != htons(ARPHRD_ETHER) ||
-		    arp->ar_pro != htons(ETH_P_IP) ||
-		    arp->ar_hln != ETH_ALEN ||
-		    arp->ar_pln != 4 ||
-		    (arp->ar_op != htons(ARPOP_REPLY) &&
-		     arp->ar_op != htons(ARPOP_REQUEST)))
-			goto out_bad;
-
-		arp_eth = __skb_header_pointer(skb, nhoff + sizeof(_arp),
-					       sizeof(_arp_eth), data,
-					       hlen,
-					       &_arp_eth);
-		if (!arp_eth)
+	case htons(ETH_P_RARP):
+		switch (__skb_flow_dissect_arp(skb, flow_dissector,
+					       target_container, data,
+					       nhoff, hlen)) {
+		case FLOW_DISSECT_RET_OUT_GOOD:
+			goto out_good;
+		case FLOW_DISSECT_RET_OUT_BAD:
 			goto out_bad;
-
-		if (dissector_uses_key(flow_dissector,
-				       FLOW_DISSECTOR_KEY_ARP)) {
-
-			key_arp = skb_flow_dissector_target(flow_dissector,
-							    FLOW_DISSECTOR_KEY_ARP,
-							    target_container);
-
-			memcpy(&key_arp->sip, arp_eth->ar_sip,
-			       sizeof(key_arp->sip));
-			memcpy(&key_arp->tip, arp_eth->ar_tip,
-			       sizeof(key_arp->tip));
-
-			/* Only store the lower byte of the opcode;
-			 * this covers ARPOP_REPLY and ARPOP_REQUEST.
-			 */
-			key_arp->op = ntohs(arp->ar_op) & 0xff;
-
-			ether_addr_copy(key_arp->sha, arp_eth->ar_sha);
-			ether_addr_copy(key_arp->tha, arp_eth->ar_tha);
 		}
-
-		goto out_good;
-	}
-
 	default:
 		goto out_bad;
 	}
-- 
2.7.4

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

* [patch net-next 2/5] flow_dissector: Move MPLS dissection into a separate function
  2017-03-06 15:39 [patch net-next 0/5] make flow dissector great again Jiri Pirko
  2017-03-06 15:39 ` [patch net-next 1/5] flow_dissector: Move ARP dissection into a separate function Jiri Pirko
@ 2017-03-06 15:39 ` Jiri Pirko
  2017-03-06 15:39 ` [patch net-next 3/5] flow_dissector: Fix GRE header error path Jiri Pirko
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Jiri Pirko @ 2017-03-06 15:39 UTC (permalink / raw)
  To: netdev
  Cc: davem, tom, simon.horman, eric.dumazet, dinan.gunawardena, hadarh,
	e, fgao, mlxsw

From: Jiri Pirko <jiri@mellanox.com>

Make the main flow_dissect function a bit smaller and move the MPLS
dissection into a separate function. Along with that, do the MPLS header
processing only in case the flow dissection user requires it.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 net/core/flow_dissector.c | 56 ++++++++++++++++++++++++++++-------------------
 1 file changed, 34 insertions(+), 22 deletions(-)

diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index d79fb8f..8d01298 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -119,6 +119,33 @@ enum flow_dissect_ret {
 };
 
 static enum flow_dissect_ret
+__skb_flow_dissect_mpls(const struct sk_buff *skb,
+			struct flow_dissector *flow_dissector,
+			void *target_container, void *data, int nhoff, int hlen)
+{
+	struct flow_dissector_key_keyid *key_keyid;
+	struct mpls_label *hdr, _hdr[2];
+
+	if (!dissector_uses_key(flow_dissector,
+				FLOW_DISSECTOR_KEY_MPLS_ENTROPY))
+		return FLOW_DISSECT_RET_OUT_GOOD;
+
+	hdr = __skb_header_pointer(skb, nhoff, sizeof(_hdr), data,
+				   hlen, &_hdr);
+	if (!hdr)
+		return FLOW_DISSECT_RET_OUT_BAD;
+
+	if ((ntohl(hdr[0].entry) & MPLS_LS_LABEL_MASK) >>
+	    MPLS_LS_LABEL_SHIFT == MPLS_LABEL_ENTROPY) {
+		key_keyid = skb_flow_dissector_target(flow_dissector,
+						      FLOW_DISSECTOR_KEY_MPLS_ENTROPY,
+						      target_container);
+		key_keyid->keyid = hdr[1].entry & htonl(MPLS_LS_LABEL_MASK);
+	}
+	return FLOW_DISSECT_RET_OUT_GOOD;
+}
+
+static enum flow_dissect_ret
 __skb_flow_dissect_arp(const struct sk_buff *skb,
 		       struct flow_dissector *flow_dissector,
 		       void *target_container, void *data, int nhoff, int hlen)
@@ -408,31 +435,16 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
 	}
 
 	case htons(ETH_P_MPLS_UC):
-	case htons(ETH_P_MPLS_MC): {
-		struct mpls_label *hdr, _hdr[2];
+	case htons(ETH_P_MPLS_MC):
 mpls:
-		hdr = __skb_header_pointer(skb, nhoff, sizeof(_hdr), data,
-					   hlen, &_hdr);
-		if (!hdr)
-			goto out_bad;
-
-		if ((ntohl(hdr[0].entry) & MPLS_LS_LABEL_MASK) >>
-		     MPLS_LS_LABEL_SHIFT == MPLS_LABEL_ENTROPY) {
-			if (dissector_uses_key(flow_dissector,
-					       FLOW_DISSECTOR_KEY_MPLS_ENTROPY)) {
-				key_keyid = skb_flow_dissector_target(flow_dissector,
-								      FLOW_DISSECTOR_KEY_MPLS_ENTROPY,
-								      target_container);
-				key_keyid->keyid = hdr[1].entry &
-					htonl(MPLS_LS_LABEL_MASK);
-			}
-
+		switch (__skb_flow_dissect_mpls(skb, flow_dissector,
+						target_container, data,
+						nhoff, hlen)) {
+		case FLOW_DISSECT_RET_OUT_GOOD:
 			goto out_good;
+		case FLOW_DISSECT_RET_OUT_BAD:
+			goto out_bad;
 		}
-
-		goto out_good;
-	}
-
 	case htons(ETH_P_FCOE):
 		if ((hlen - nhoff) < FCOE_HEADER_LEN)
 			goto out_bad;
-- 
2.7.4

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

* [patch net-next 3/5] flow_dissector: Fix GRE header error path
  2017-03-06 15:39 [patch net-next 0/5] make flow dissector great again Jiri Pirko
  2017-03-06 15:39 ` [patch net-next 1/5] flow_dissector: Move ARP dissection into a separate function Jiri Pirko
  2017-03-06 15:39 ` [patch net-next 2/5] flow_dissector: Move MPLS " Jiri Pirko
@ 2017-03-06 15:39 ` Jiri Pirko
  2017-03-06 15:39 ` [patch net-next 4/5] flow_dissector: rename "proto again" goto label Jiri Pirko
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Jiri Pirko @ 2017-03-06 15:39 UTC (permalink / raw)
  To: netdev
  Cc: davem, tom, simon.horman, eric.dumazet, dinan.gunawardena, hadarh,
	e, fgao, mlxsw

From: Jiri Pirko <jiri@mellanox.com>

Now, when an unexpected element in the GRE header appears, we break so
the l4 ports are processed. But since the ports are processed
unconditionally, there will be certainly random values dissected. Fix
this by just bailing out in such situations.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 net/core/flow_dissector.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index 8d01298..cefaf23 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -479,18 +479,18 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
 
 		/* Only look inside GRE without routing */
 		if (hdr->flags & GRE_ROUTING)
-			break;
+			goto out_good;
 
 		/* Only look inside GRE for version 0 and 1 */
 		gre_ver = ntohs(hdr->flags & GRE_VERSION);
 		if (gre_ver > 1)
-			break;
+			goto out_good;
 
 		proto = hdr->protocol;
 		if (gre_ver) {
 			/* Version1 must be PPTP, and check the flags */
 			if (!(proto == GRE_PROTO_PPP && (hdr->flags & GRE_KEY)))
-				break;
+				goto out_good;
 		}
 
 		offset += sizeof(struct gre_base_hdr);
-- 
2.7.4

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

* [patch net-next 4/5] flow_dissector: rename "proto again" goto label
  2017-03-06 15:39 [patch net-next 0/5] make flow dissector great again Jiri Pirko
                   ` (2 preceding siblings ...)
  2017-03-06 15:39 ` [patch net-next 3/5] flow_dissector: Fix GRE header error path Jiri Pirko
@ 2017-03-06 15:39 ` Jiri Pirko
  2017-03-06 15:39 ` [patch net-next 5/5] flow_dissector: Move GRE dissection into a separate function Jiri Pirko
  2017-03-09  7:09 ` [patch net-next 0/5] flow dissector improvements David Miller
  5 siblings, 0 replies; 7+ messages in thread
From: Jiri Pirko @ 2017-03-06 15:39 UTC (permalink / raw)
  To: netdev
  Cc: davem, tom, simon.horman, eric.dumazet, dinan.gunawardena, hadarh,
	e, fgao, mlxsw

From: Jiri Pirko <jiri@mellanox.com>

Align with "ip_proto_again" label used in the same function and rename
vague "again" to "proto_again".

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 net/core/flow_dissector.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index cefaf23..9120835 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -267,7 +267,7 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
 		memcpy(key_eth_addrs, &eth->h_dest, sizeof(*key_eth_addrs));
 	}
 
-again:
+proto_again:
 	switch (proto) {
 	case htons(ETH_P_IP): {
 		const struct iphdr *iph;
@@ -370,7 +370,7 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
 			proto = vlan->h_vlan_encapsulated_proto;
 			nhoff += sizeof(*vlan);
 			if (skip_vlan)
-				goto again;
+				goto proto_again;
 		}
 
 		skip_vlan = true;
@@ -393,7 +393,7 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
 			}
 		}
 
-		goto again;
+		goto proto_again;
 	}
 	case htons(ETH_P_PPP_SES): {
 		struct {
@@ -577,7 +577,7 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
 		if (flags & FLOW_DISSECTOR_F_STOP_AT_ENCAP)
 			goto out_good;
 
-		goto again;
+		goto proto_again;
 	}
 	case NEXTHDR_HOP:
 	case NEXTHDR_ROUTING:
-- 
2.7.4

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

* [patch net-next 5/5] flow_dissector: Move GRE dissection into a separate function
  2017-03-06 15:39 [patch net-next 0/5] make flow dissector great again Jiri Pirko
                   ` (3 preceding siblings ...)
  2017-03-06 15:39 ` [patch net-next 4/5] flow_dissector: rename "proto again" goto label Jiri Pirko
@ 2017-03-06 15:39 ` Jiri Pirko
  2017-03-09  7:09 ` [patch net-next 0/5] flow dissector improvements David Miller
  5 siblings, 0 replies; 7+ messages in thread
From: Jiri Pirko @ 2017-03-06 15:39 UTC (permalink / raw)
  To: netdev
  Cc: davem, tom, simon.horman, eric.dumazet, dinan.gunawardena, hadarh,
	e, fgao, mlxsw

From: Jiri Pirko <jiri@mellanox.com>

Make the main flow_dissect function a bit smaller and move the GRE
dissection into a separate function.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 net/core/flow_dissector.c | 244 +++++++++++++++++++++++++---------------------
 1 file changed, 134 insertions(+), 110 deletions(-)

diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index 9120835..5f3ae92 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -116,6 +116,7 @@ EXPORT_SYMBOL(__skb_flow_get_ports);
 enum flow_dissect_ret {
 	FLOW_DISSECT_RET_OUT_GOOD,
 	FLOW_DISSECT_RET_OUT_BAD,
+	FLOW_DISSECT_RET_OUT_PROTO_AGAIN,
 };
 
 static enum flow_dissect_ret
@@ -200,6 +201,128 @@ __skb_flow_dissect_arp(const struct sk_buff *skb,
 	return FLOW_DISSECT_RET_OUT_GOOD;
 }
 
+static enum flow_dissect_ret
+__skb_flow_dissect_gre(const struct sk_buff *skb,
+		       struct flow_dissector_key_control *key_control,
+		       struct flow_dissector *flow_dissector,
+		       void *target_container, void *data,
+		       __be16 *p_proto, int *p_nhoff, int *p_hlen,
+		       unsigned int flags)
+{
+	struct flow_dissector_key_keyid *key_keyid;
+	struct gre_base_hdr *hdr, _hdr;
+	int offset = 0;
+	u16 gre_ver;
+
+	hdr = __skb_header_pointer(skb, *p_nhoff, sizeof(_hdr),
+				   data, *p_hlen, &_hdr);
+	if (!hdr)
+		return FLOW_DISSECT_RET_OUT_BAD;
+
+	/* Only look inside GRE without routing */
+	if (hdr->flags & GRE_ROUTING)
+		return FLOW_DISSECT_RET_OUT_GOOD;
+
+	/* Only look inside GRE for version 0 and 1 */
+	gre_ver = ntohs(hdr->flags & GRE_VERSION);
+	if (gre_ver > 1)
+		return FLOW_DISSECT_RET_OUT_GOOD;
+
+	*p_proto = hdr->protocol;
+	if (gre_ver) {
+		/* Version1 must be PPTP, and check the flags */
+		if (!(*p_proto == GRE_PROTO_PPP && (hdr->flags & GRE_KEY)))
+			return FLOW_DISSECT_RET_OUT_GOOD;
+	}
+
+	offset += sizeof(struct gre_base_hdr);
+
+	if (hdr->flags & GRE_CSUM)
+		offset += sizeof(((struct gre_full_hdr *) 0)->csum) +
+			  sizeof(((struct gre_full_hdr *) 0)->reserved1);
+
+	if (hdr->flags & GRE_KEY) {
+		const __be32 *keyid;
+		__be32 _keyid;
+
+		keyid = __skb_header_pointer(skb, *p_nhoff + offset,
+					     sizeof(_keyid),
+					     data, *p_hlen, &_keyid);
+		if (!keyid)
+			return FLOW_DISSECT_RET_OUT_BAD;
+
+		if (dissector_uses_key(flow_dissector,
+				       FLOW_DISSECTOR_KEY_GRE_KEYID)) {
+			key_keyid = skb_flow_dissector_target(flow_dissector,
+							      FLOW_DISSECTOR_KEY_GRE_KEYID,
+							      target_container);
+			if (gre_ver == 0)
+				key_keyid->keyid = *keyid;
+			else
+				key_keyid->keyid = *keyid & GRE_PPTP_KEY_MASK;
+		}
+		offset += sizeof(((struct gre_full_hdr *) 0)->key);
+	}
+
+	if (hdr->flags & GRE_SEQ)
+		offset += sizeof(((struct pptp_gre_header *) 0)->seq);
+
+	if (gre_ver == 0) {
+		if (*p_proto == htons(ETH_P_TEB)) {
+			const struct ethhdr *eth;
+			struct ethhdr _eth;
+
+			eth = __skb_header_pointer(skb, *p_nhoff + offset,
+						   sizeof(_eth),
+						   data, *p_hlen, &_eth);
+			if (!eth)
+				return FLOW_DISSECT_RET_OUT_BAD;
+			*p_proto = eth->h_proto;
+			offset += sizeof(*eth);
+
+			/* Cap headers that we access via pointers at the
+			 * end of the Ethernet header as our maximum alignment
+			 * at that point is only 2 bytes.
+			 */
+			if (NET_IP_ALIGN)
+				*p_hlen = *p_nhoff + offset;
+		}
+	} else { /* version 1, must be PPTP */
+		u8 _ppp_hdr[PPP_HDRLEN];
+		u8 *ppp_hdr;
+
+		if (hdr->flags & GRE_ACK)
+			offset += sizeof(((struct pptp_gre_header *) 0)->ack);
+
+		ppp_hdr = __skb_header_pointer(skb, *p_nhoff + offset,
+					       sizeof(_ppp_hdr),
+					       data, *p_hlen, _ppp_hdr);
+		if (!ppp_hdr)
+			return FLOW_DISSECT_RET_OUT_BAD;
+
+		switch (PPP_PROTOCOL(ppp_hdr)) {
+		case PPP_IP:
+			*p_proto = htons(ETH_P_IP);
+			break;
+		case PPP_IPV6:
+			*p_proto = htons(ETH_P_IPV6);
+			break;
+		default:
+			/* Could probably catch some more like MPLS */
+			break;
+		}
+
+		offset += PPP_HDRLEN;
+	}
+
+	*p_nhoff += offset;
+	key_control->flags |= FLOW_DIS_ENCAPSULATION;
+	if (flags & FLOW_DISSECTOR_F_STOP_AT_ENCAP)
+		return FLOW_DISSECT_RET_OUT_GOOD;
+
+	return FLOW_DISSECT_RET_OUT_PROTO_AGAIN;
+}
+
 /**
  * __skb_flow_dissect - extract the flow_keys struct and return it
  * @skb: sk_buff to extract the flow from, can be NULL if the rest are specified
@@ -229,7 +352,6 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
 	struct flow_dissector_key_icmp *key_icmp;
 	struct flow_dissector_key_tags *key_tags;
 	struct flow_dissector_key_vlan *key_vlan;
-	struct flow_dissector_key_keyid *key_keyid;
 	bool skip_vlan = false;
 	u8 ip_proto = 0;
 	bool ret;
@@ -443,6 +565,7 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
 		case FLOW_DISSECT_RET_OUT_GOOD:
 			goto out_good;
 		case FLOW_DISSECT_RET_OUT_BAD:
+		default:
 			goto out_bad;
 		}
 	case htons(ETH_P_FCOE):
@@ -460,6 +583,7 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
 		case FLOW_DISSECT_RET_OUT_GOOD:
 			goto out_good;
 		case FLOW_DISSECT_RET_OUT_BAD:
+		default:
 			goto out_bad;
 		}
 	default:
@@ -468,117 +592,17 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
 
 ip_proto_again:
 	switch (ip_proto) {
-	case IPPROTO_GRE: {
-		struct gre_base_hdr *hdr, _hdr;
-		u16 gre_ver;
-		int offset = 0;
-
-		hdr = __skb_header_pointer(skb, nhoff, sizeof(_hdr), data, hlen, &_hdr);
-		if (!hdr)
-			goto out_bad;
-
-		/* Only look inside GRE without routing */
-		if (hdr->flags & GRE_ROUTING)
-			goto out_good;
-
-		/* Only look inside GRE for version 0 and 1 */
-		gre_ver = ntohs(hdr->flags & GRE_VERSION);
-		if (gre_ver > 1)
+	case IPPROTO_GRE:
+		switch (__skb_flow_dissect_gre(skb, key_control, flow_dissector,
+					       target_container, data,
+					       &proto, &nhoff, &hlen, flags)) {
+		case FLOW_DISSECT_RET_OUT_GOOD:
 			goto out_good;
-
-		proto = hdr->protocol;
-		if (gre_ver) {
-			/* Version1 must be PPTP, and check the flags */
-			if (!(proto == GRE_PROTO_PPP && (hdr->flags & GRE_KEY)))
-				goto out_good;
-		}
-
-		offset += sizeof(struct gre_base_hdr);
-
-		if (hdr->flags & GRE_CSUM)
-			offset += sizeof(((struct gre_full_hdr *)0)->csum) +
-				  sizeof(((struct gre_full_hdr *)0)->reserved1);
-
-		if (hdr->flags & GRE_KEY) {
-			const __be32 *keyid;
-			__be32 _keyid;
-
-			keyid = __skb_header_pointer(skb, nhoff + offset, sizeof(_keyid),
-						     data, hlen, &_keyid);
-			if (!keyid)
-				goto out_bad;
-
-			if (dissector_uses_key(flow_dissector,
-					       FLOW_DISSECTOR_KEY_GRE_KEYID)) {
-				key_keyid = skb_flow_dissector_target(flow_dissector,
-								      FLOW_DISSECTOR_KEY_GRE_KEYID,
-								      target_container);
-				if (gre_ver == 0)
-					key_keyid->keyid = *keyid;
-				else
-					key_keyid->keyid = *keyid & GRE_PPTP_KEY_MASK;
-			}
-			offset += sizeof(((struct gre_full_hdr *)0)->key);
-		}
-
-		if (hdr->flags & GRE_SEQ)
-			offset += sizeof(((struct pptp_gre_header *)0)->seq);
-
-		if (gre_ver == 0) {
-			if (proto == htons(ETH_P_TEB)) {
-				const struct ethhdr *eth;
-				struct ethhdr _eth;
-
-				eth = __skb_header_pointer(skb, nhoff + offset,
-							   sizeof(_eth),
-							   data, hlen, &_eth);
-				if (!eth)
-					goto out_bad;
-				proto = eth->h_proto;
-				offset += sizeof(*eth);
-
-				/* Cap headers that we access via pointers at the
-				 * end of the Ethernet header as our maximum alignment
-				 * at that point is only 2 bytes.
-				 */
-				if (NET_IP_ALIGN)
-					hlen = (nhoff + offset);
-			}
-		} else { /* version 1, must be PPTP */
-			u8 _ppp_hdr[PPP_HDRLEN];
-			u8 *ppp_hdr;
-
-			if (hdr->flags & GRE_ACK)
-				offset += sizeof(((struct pptp_gre_header *)0)->ack);
-
-			ppp_hdr = __skb_header_pointer(skb, nhoff + offset,
-						     sizeof(_ppp_hdr),
-						     data, hlen, _ppp_hdr);
-			if (!ppp_hdr)
-				goto out_bad;
-
-			switch (PPP_PROTOCOL(ppp_hdr)) {
-			case PPP_IP:
-				proto = htons(ETH_P_IP);
-				break;
-			case PPP_IPV6:
-				proto = htons(ETH_P_IPV6);
-				break;
-			default:
-				/* Could probably catch some more like MPLS */
-				break;
-			}
-
-			offset += PPP_HDRLEN;
+		case FLOW_DISSECT_RET_OUT_BAD:
+			goto out_bad;
+		case FLOW_DISSECT_RET_OUT_PROTO_AGAIN:
+			goto proto_again;
 		}
-
-		nhoff += offset;
-		key_control->flags |= FLOW_DIS_ENCAPSULATION;
-		if (flags & FLOW_DISSECTOR_F_STOP_AT_ENCAP)
-			goto out_good;
-
-		goto proto_again;
-	}
 	case NEXTHDR_HOP:
 	case NEXTHDR_ROUTING:
 	case NEXTHDR_DEST: {
-- 
2.7.4

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

* Re: [patch net-next 0/5] flow dissector improvements
  2017-03-06 15:39 [patch net-next 0/5] make flow dissector great again Jiri Pirko
                   ` (4 preceding siblings ...)
  2017-03-06 15:39 ` [patch net-next 5/5] flow_dissector: Move GRE dissection into a separate function Jiri Pirko
@ 2017-03-09  7:09 ` David Miller
  5 siblings, 0 replies; 7+ messages in thread
From: David Miller @ 2017-03-09  7:09 UTC (permalink / raw)
  To: jiri
  Cc: netdev, tom, simon.horman, eric.dumazet, dinan.gunawardena,
	hadarh, e, fgao, mlxsw

From: Jiri Pirko <jiri@resnulli.us>
Date: Mon,  6 Mar 2017 16:39:50 +0100

> This patchset follows-up the discussion about future extensions of flow
> dissector and tries to address the mentioned concerns. Some parts are
> cut out into sub-functions. Also, the processing of the code (ARP, MPLS)
> is made dependent on user actually requiring the bisected values.
> This prepares the code for future extensions to bisect IPv6 ND messages,
> TCP flags, etc.

Series applied, thanks Jiri.

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

end of thread, other threads:[~2017-03-09  7:09 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-03-06 15:39 [patch net-next 0/5] make flow dissector great again Jiri Pirko
2017-03-06 15:39 ` [patch net-next 1/5] flow_dissector: Move ARP dissection into a separate function Jiri Pirko
2017-03-06 15:39 ` [patch net-next 2/5] flow_dissector: Move MPLS " Jiri Pirko
2017-03-06 15:39 ` [patch net-next 3/5] flow_dissector: Fix GRE header error path Jiri Pirko
2017-03-06 15:39 ` [patch net-next 4/5] flow_dissector: rename "proto again" goto label Jiri Pirko
2017-03-06 15:39 ` [patch net-next 5/5] flow_dissector: Move GRE dissection into a separate function Jiri Pirko
2017-03-09  7:09 ` [patch net-next 0/5] flow dissector improvements David Miller

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).