[ovs-dev] [PATCH 1/2] datapath: IP fragments should include L4 header in flow length.

[Jesse Gross]

If we can't parse a header because it is invalid or not present due to
fragmentation, we still need to include the length of that header when
comparing the flow key.  The value of the field will be zero to
indicate that header was not present, rather than effectively
wildcarding the value.  However, this was not done with fragments on
flow extract but is effectively done on flow setup.  Since the flow
length also changes the hash, it caused all fragments to miss the
hash table and be sent to useerspace.

Reported-by: Ben Pfaff <blp at nicira.com>
Signed-off-by: Jesse Gross <jesse at nicira.com>
---
 datapath/flow.c |   56 +++++++++++++++++++++++++++---------------------------
 1 files changed, 28 insertions(+), 28 deletions(-)

diff --git a/datapath/flow.c b/datapath/flow.c
index d181cde..7bc34be 100644
--- a/datapath/flow.c
+++ b/datapath/flow.c
@@ -489,36 +489,36 @@ int flow_extract(struct sk_buff *skb, u16 in_port, struct sw_flow_key *key,
 		key->ip.nw_proto = nh->protocol;
 
 		/* Transport layer. */
-		if (!(nh->frag_off & htons(IP_MF | IP_OFFSET)) &&
-		    !(skb_shinfo(skb)->gso_type & SKB_GSO_UDP)) {
-			if (key->ip.nw_proto == IPPROTO_TCP) {
-				key_len = SW_FLOW_KEY_OFFSET(ipv4.tp);
-				if (tcphdr_ok(skb)) {
-					struct tcphdr *tcp = tcp_hdr(skb);
-					key->ipv4.tp.src = tcp->source;
-					key->ipv4.tp.dst = tcp->dest;
-				}
-			} else if (key->ip.nw_proto == IPPROTO_UDP) {
-				key_len = SW_FLOW_KEY_OFFSET(ipv4.tp);
-				if (udphdr_ok(skb)) {
-					struct udphdr *udp = udp_hdr(skb);
-					key->ipv4.tp.src = udp->source;
-					key->ipv4.tp.dst = udp->dest;
-				}
-			} else if (key->ip.nw_proto == IPPROTO_ICMP) {
-				key_len = SW_FLOW_KEY_OFFSET(ipv4.tp);
-				if (icmphdr_ok(skb)) {
-					struct icmphdr *icmp = icmp_hdr(skb);
-					/* The ICMP type and code fields use the 16-bit
-					 * transport port fields, so we need to store them
-					 * in 16-bit network byte order. */
-					key->ipv4.tp.src = htons(icmp->type);
-					key->ipv4.tp.dst = htons(icmp->code);
-				}
-			}
-		} else
+		if ((nh->frag_off & htons(IP_MF | IP_OFFSET)) ||
+		    (skb_shinfo(skb)->gso_type & SKB_GSO_UDP))
 			*is_frag = true;
 
+		if (key->ip.nw_proto == IPPROTO_TCP) {
+			key_len = SW_FLOW_KEY_OFFSET(ipv4.tp);
+			if (!*is_frag && tcphdr_ok(skb)) {
+				struct tcphdr *tcp = tcp_hdr(skb);
+				key->ipv4.tp.src = tcp->source;
+				key->ipv4.tp.dst = tcp->dest;
+			}
+		} else if (key->ip.nw_proto == IPPROTO_UDP) {
+			key_len = SW_FLOW_KEY_OFFSET(ipv4.tp);
+			if (!*is_frag && udphdr_ok(skb)) {
+				struct udphdr *udp = udp_hdr(skb);
+				key->ipv4.tp.src = udp->source;
+				key->ipv4.tp.dst = udp->dest;
+			}
+		} else if (key->ip.nw_proto == IPPROTO_ICMP) {
+			key_len = SW_FLOW_KEY_OFFSET(ipv4.tp);
+			if (!*is_frag && icmphdr_ok(skb)) {
+				struct icmphdr *icmp = icmp_hdr(skb);
+				/* The ICMP type and code fields use the 16-bit
+				 * transport port fields, so we need to store them
+				 * in 16-bit network byte order. */
+				key->ipv4.tp.src = htons(icmp->type);
+				key->ipv4.tp.dst = htons(icmp->code);
+			}
+		}
+
 	} else if (key->eth.type == htons(ETH_P_ARP) && arphdr_ok(skb)) {
 		struct arp_eth_header *arp;
 
-- 
1.7.4.1