On Thu, Oct 20, 2011 at 03:56:41PM -0700, Jesse Gross wrote:
> On Wed, Oct 19, 2011 at 10:56 PM, Ben Pfaff <b...@nicira.com> wrote:
> > On Wed, Oct 19, 2011 at 06:09:30PM -0700, Jesse Gross wrote:
> >> Otherwise, the incremental looks good. ??However, I realized that there
> >> is one more issue: when we pass up the flow key for UDP GSO packets,
> >> they will all reflect the first fragment and not the later packets.
> >
> > Hmm, good point.
> >
> > Here's a build-tested-only try at this. ??It needs testing and comments,
> > at least. ??Is it on the right track?
>
> It looks good to me (actually I think the new separation is much
> cleaner even independent of this change).
OK. I added a comment:
@@ -411,6 +411,10 @@ static int queue_gso_packets(int dp_ifindex, struct
sk_buff *skb,
break;
if (skb == segs && skb_shinfo(skb)->gso_type & SKB_GSO_UDP) {
+ /* The initial flow key extracted by flow_extract() in
+ * this case is for a first fragment, so we need to
+ * properly mark later fragments.
+ */
later_key = *upcall_info->key;
later_key.ip.tos_frag &= ~OVS_FRAG_TYPE_MASK;
later_key.ip.tos_frag |= OVS_FRAG_TYPE_LATER;
> For the whole thing then:
> Acked-by: Jesse Gross <je...@nicira.com>
Thanks.
In testing, I found a bug: "later" fragments were being extracted with
a key_len that covered the transport ports, but the key sent to
and received back from userspace omitted it, so that the flow that
userspace set up didn't actually match, so that every "later" packet
went to userspace.
Please verify my fix:
diff --git a/datapath/flow.c b/datapath/flow.c
index 7c602bf..ad20c26 100644
--- a/datapath/flow.c
+++ b/datapath/flow.c
@@ -689,30 +689,32 @@ int flow_extract(struct sk_buff *skb, u16 in_port, struct
sw_flow_key *key,
key->ip.tos_frag = nh->tos & ~INET_ECN_MASK;
offset = nh->frag_off & htons(IP_OFFSET);
- if (offset)
+ if (offset) {
key->ip.tos_frag |= OVS_FRAG_TYPE_LATER;
- else if (nh->frag_off & htons(IP_MF) ||
+ goto out;
+ }
+ if (nh->frag_off & htons(IP_MF) ||
skb_shinfo(skb)->gso_type & SKB_GSO_UDP)
key->ip.tos_frag |= OVS_FRAG_TYPE_FIRST;
/* Transport layer. */
if (key->ip.proto == IPPROTO_TCP) {
key_len = SW_FLOW_KEY_OFFSET(ipv4.tp);
- if (!offset && tcphdr_ok(skb)) {
+ 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.proto == IPPROTO_UDP) {
key_len = SW_FLOW_KEY_OFFSET(ipv4.tp);
- if (!offset && udphdr_ok(skb)) {
+ 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.proto == IPPROTO_ICMP) {
key_len = SW_FLOW_KEY_OFFSET(ipv4.tp);
- if (!offset && icmphdr_ok(skb)) {
+ 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
@@ -757,6 +759,8 @@ int flow_extract(struct sk_buff *skb, u16 in_port, struct
sw_flow_key *key,
goto out;
}
+ if ((key->ip.tos_frag & OVS_FRAG_TYPE_MASK) ==
OVS_FRAG_TYPE_LATER)
+ goto out;
if (skb_shinfo(skb)->gso_type & SKB_GSO_UDP)
key->ip.tos_frag |= OVS_FRAG_TYPE_FIRST;
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
