recirculation: Do not drop packet when there is no match from

internal table.

In current recirculation implementation, the flow misses (with
'recirc_id' set) are always looked up on the receiving bridge's
internal flow table.  However, the bond port may actually reside
on another bridge which gets connected to the receiving bridge
via patch port.  Since the recirculation rules are pushed to the
other bridge's internal table, the flow lookup on the receiving
bridge will match nothing but the drop rule, causing unexpected
packet drops.

This commit fixes the above bug via keeping lookup the misses
(with 'recirc_id' set) in default table (table 0) and processing
it until reaching the bridge that owns the bond port.  Then,
the misses can hit the post recirculation flows as expected.

VMware-BZ: 1362178

Reported-by: Ansis Atteka <aatteka@nicira.com>
Signed-off-by: Alex Wang <alexw@nicira.com>
Acked-by: Andy Zhou <azhou@nicira.com>

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 9f0fdce..5a2b883 100644 (file)
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -2655,6 +2655,7 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,
         const struct xport *peer = xport->peer;
         struct flow old_flow = ctx->xin->flow;
         enum slow_path_reason special;
+        uint8_t table_id = rule_dpif_lookup_get_init_table_id(&ctx->xin->flow);
 
         ctx->xbridge = peer->xbridge;
         flow->in_port.ofp_port = peer->ofp_port;
@@ -2669,14 +2670,16 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,
             ctx->xout->slow |= special;
         } else if (may_receive(peer, ctx)) {
             if (xport_stp_forward_state(peer) && xport_rstp_forward_state(peer)) {
-                xlate_table_action(ctx, flow->in_port.ofp_port, 0, true, true);
+                xlate_table_action(ctx, flow->in_port.ofp_port, table_id,
+                                   true, true);
             } else {
                 /* Forwarding is disabled by STP and RSTP.  Let OFPP_NORMAL and
                  * the learning action look at the packet, then drop it. */
                 struct flow old_base_flow = ctx->base_flow;
                 size_t old_size = ofpbuf_size(ctx->xout->odp_actions);
                 mirror_mask_t old_mirrors = ctx->xout->mirrors;
-                xlate_table_action(ctx, flow->in_port.ofp_port, 0, true, true);
+                xlate_table_action(ctx, flow->in_port.ofp_port, table_id,
+                                   true, true);
                 ctx->xout->mirrors = old_mirrors;
                 ctx->base_flow = old_base_flow;
                 ofpbuf_set_size(ctx->xout->odp_actions, old_size);

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index ffaffe4..72859c2 100644 (file)
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -1316,29 +1316,28 @@ add_internal_flows(struct ofproto_dpif *ofproto)
         return error;
     }
 
-    /* Continue non-recirculation rule lookups from table 0.
+    /* Drop any run away non-recirc rule lookups. Recirc_id has to be
+     * zero when reaching this rule.
      *
-     * (priority=2), recirc=0, actions=resubmit(, 0)
+     * (priority=2), recirc_id=0, actions=drop
      */
-    resubmit = ofpact_put_RESUBMIT(&ofpacts);
-    resubmit->in_port = OFPP_IN_PORT;
-    resubmit->table_id = 0;
-
+    ofpbuf_clear(&ofpacts);
     match_init_catchall(&match);
     match_set_recirc_id(&match, 0);
-
     error = ofproto_dpif_add_internal_flow(ofproto, &match, 2, 0, &ofpacts,
                                            &unused_rulep);
     if (error) {
         return error;
     }
 
-    /* Drop any run away recirc rule lookups. Recirc_id has to be
-     * non-zero when reaching this rule.
+    /* Continue rule lookups for not-matched recirc rules from table 0.
      *
-     * (priority=1), *, actions=drop
+     * (priority=1), actions=resubmit(, 0)
      */
-    ofpbuf_clear(&ofpacts);
+    resubmit = ofpact_put_RESUBMIT(&ofpacts);
+    resubmit->in_port = OFPP_IN_PORT;
+    resubmit->table_id = 0;
+
     match_init_catchall(&match);
     error = ofproto_dpif_add_internal_flow(ofproto, &match, 1, 0, &ofpacts,
                                            &unused_rulep);
@@ -3645,11 +3644,7 @@ rule_dpif_lookup(struct ofproto_dpif *ofproto, struct flow *flow,
         if (wc) {
             wc->masks.recirc_id = UINT32_MAX;
         }
-        if (flow->recirc_id) {
-            /* Start looking up from internal table for post recirculation
-             * flows or packets. */
-            *table_id = TBL_INTERNAL;
-        }
+        *table_id = rule_dpif_lookup_get_init_table_id(flow);
     }
 
     return rule_dpif_lookup_from_table(ofproto, flow, wc, take_ref, stats,

diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h
index 48e1b30..2192ea3 100644 (file)
--- a/ofproto/ofproto-dpif.h
+++ b/ofproto/ofproto-dpif.h
@@ -40,6 +40,11 @@ struct dpif_backer;
 struct OVS_LOCKABLE rule_dpif;
 struct OVS_LOCKABLE group_dpif;
 
+/* Number of implemented OpenFlow tables. */
+enum { N_TABLES = 255 };
+enum { TBL_INTERNAL = N_TABLES - 1 };    /* Used for internal hidden rules. */
+BUILD_ASSERT_DECL(N_TABLES >= 2 && N_TABLES <= 255);
+
 /* Ofproto-dpif -- DPIF based ofproto implementation.
  *
  * Ofproto-dpif provides an ofproto implementation for those platforms which
@@ -86,6 +91,14 @@ struct rule_dpif *rule_dpif_lookup_from_table(struct ofproto_dpif *,
                                               bool may_packet_in,
                                               bool honor_table_miss);
 
+/* If 'recirc_id' is set, starts looking up from internal table for
+ * post recirculation flows or packets.  Otherwise, starts from table 0. */
+static inline uint8_t
+rule_dpif_lookup_get_init_table_id(const struct flow *flow)
+{
+    return flow->recirc_id ? TBL_INTERNAL : 0;
+}
+
 static inline void rule_dpif_ref(struct rule_dpif *);
 static inline void rule_dpif_unref(struct rule_dpif *);
 
@@ -214,12 +227,6 @@ int ofproto_dpif_add_internal_flow(struct ofproto_dpif *,
                                    struct rule **rulep);
 int ofproto_dpif_delete_internal_flow(struct ofproto_dpif *, struct match *,
                                       int priority);
-
-/* Number of implemented OpenFlow tables. */
-enum { N_TABLES = 255 };
-enum { TBL_INTERNAL = N_TABLES - 1 };    /* Used for internal hidden rules. */
-BUILD_ASSERT_DECL(N_TABLES >= 2 && N_TABLES <= 255);
-
 \f
 /* struct rule_dpif has struct rule as it's first member. */
 #define RULE_CAST(RULE) ((struct rule *)RULE)