datapath: IP fragments should include L4 header in flow length.
authorJesse Gross <jesse@nicira.com>
Wed, 8 Jun 2011 19:23:05 +0000 (12:23 -0700)
committerJesse Gross <jesse@nicira.com>
Wed, 8 Jun 2011 20:56:19 +0000 (13:56 -0700)
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@nicira.com>
Signed-off-by: Jesse Gross <jesse@nicira.com>
Tested-by: Ben Pfaff <blp@nicira.com>
datapath/flow.c

index d181cdefbd441b49e4752507238d50772e4cabdd..7bc34bec18440a154181f26bc9b1066c7c9f70ca 100644 (file)
@@ -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;