From: Andy Zhou <azhou@nicira.com>
Date: Wed, 11 Dec 2013 07:32:51 +0000 (-0800)
Subject: ofproto-dpif: Ignore non-packet field masks during flow revalidation
X-Git-Tag: sliver-openvswitch-2.1.90-1~10^2~214
X-Git-Url: http://git.onelab.eu/?p=sliver-openvswitch.git;a=commitdiff_plain;h=c11c6faa557fb7f363375f988731fff19562dee5

ofproto-dpif: Ignore non-packet field masks during flow revalidation

Commit bcd2633a5be(ofproto-dpif: Store relevant fields for wildcarding
in facet) implements flow revalidation by comparing the newly looked
up flow mask with that of the existing facet.

The non-packet fields, such as register masks, are always cleared
by xlate_actions in the masks stored within facets, but they are not
cleared in the newly looked up flow masks, causing otherwise valid
flows to be declared as invalid flows and be removed from the datapath.

This patch provides a fix. I was able to verify the fix on a system set
up by Ying Chen where the bug can be reproduced.

Bug #21680
Reported by: Ying Chen <yingchen@vmware.com>

Acked-by: Justin Pettit <jpettit@nicira.com>
Signed-off-by: Andy Zhou <azhou@nicira.com>
---

diff --git a/lib/flow.c b/lib/flow.c
index 63efab1bc..3633dffa3 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -630,6 +630,15 @@ flow_wildcards_init_catchall(struct flow_wildcards *wc)
     memset(&wc->masks, 0, sizeof wc->masks);
 }
 
+/* Clear the metadata and register wildcard masks. They are not packet
+ * header fields. */
+void
+flow_wildcards_clear_non_packet_fields(struct flow_wildcards *wc)
+{
+    memset(&wc->masks.metadata, 0, sizeof wc->masks.metadata);
+    memset(&wc->masks.regs, 0, sizeof wc->masks.regs);
+}
+
 /* Returns true if 'wc' matches every packet, false if 'wc' fixes any bits or
  * fields. */
 bool
diff --git a/lib/flow.h b/lib/flow.h
index 5c0007ddf..44d96adc6 100644
--- a/lib/flow.h
+++ b/lib/flow.h
@@ -284,6 +284,8 @@ struct flow_wildcards {
 
 void flow_wildcards_init_catchall(struct flow_wildcards *);
 
+void flow_wildcards_clear_non_packet_fields(struct flow_wildcards *);
+
 bool flow_wildcards_is_catchall(const struct flow_wildcards *);
 
 void flow_wildcards_set_reg_mask(struct flow_wildcards *,
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index b0f61ca12..aec17c014 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -3203,8 +3203,7 @@ xlate_actions__(struct xlate_in *xin, struct xlate_out *xout)
 
     /* Clear the metadata and register wildcard masks, because we won't
      * use non-header fields as part of the cache. */
-    memset(&wc->masks.metadata, 0, sizeof wc->masks.metadata);
-    memset(&wc->masks.regs, 0, sizeof wc->masks.regs);
+    flow_wildcards_clear_non_packet_fields(wc);
 
 out:
     rule_actions_unref(actions);
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index bfa9a9a56..1b4127f33 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -4219,6 +4219,10 @@ facet_revalidate(struct facet *facet)
     xlate_in_init(&xin, ofproto, &facet->flow, new_rule, 0, NULL);
     xlate_actions(&xin, &xout);
     flow_wildcards_or(&xout.wc, &xout.wc, &wc);
+    /* Make sure non -packet fields are not masked. If not cleared,
+     * the memcmp() below may fail, causing an otherwise valid facet
+     * to be removed. */
+    flow_wildcards_clear_non_packet_fields(&xout.wc);
 
     /* A facet's slow path reason should only change under dramatic
      * circumstances.  Rather than try to update everything, it's simpler to