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 4f77ac5..0fc5443 100644 (file)
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -1846,6 +1846,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;
@@ -1859,14 +1860,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)) {
-                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.  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 46595f8..7204ccf 100644 (file)
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -1236,31 +1236,31 @@ 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->ofpact.compat = 0;
-    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, &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->ofpact.compat = 0;
+    resubmit->in_port = OFPP_IN_PORT;
+    resubmit->table_id = 0;
+
     match_init_catchall(&match);
+
     error = ofproto_dpif_add_internal_flow(ofproto, &match, 1, &ofpacts,
                                            &unused_rulep);
 
@@ -3200,16 +3200,7 @@ rule_dpif_lookup(struct ofproto_dpif *ofproto, struct flow *flow,
         if (wc) {
             wc->masks.recirc_id = UINT32_MAX;
         }
-
-        /* Start looking up from internal table for post recirculation flows
-         * or packets. We can also simply send all, including normal flows
-         * or packets to the internal table. They will not match any post
-         * recirculation rules except the 'catch all' rule that resubmit
-         * them to table 0.
-         *
-         * As an optimization, we send normal flows and packets to table 0
-         * directly, saving one table lookup.  */
-        table_id = flow->recirc_id ? TBL_INTERNAL : 0;
+        table_id = rule_dpif_lookup_get_init_table_id(flow);
     } else {
         table_id = 0;
     }

diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h
index e49616e..6f77b1a 100644 (file)
--- a/ofproto/ofproto-dpif.h
+++ b/ofproto/ofproto-dpif.h
@@ -53,6 +53,11 @@ enum rule_dpif_lookup_verdict {
                                              * dropped. */
 };
 
+/* 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);
+
 /* For lock annotation below only. */
 extern struct ovs_rwlock xlate_rwlock;
 
@@ -91,6 +96,14 @@ uint8_t rule_dpif_lookup(struct ofproto_dpif *, struct flow *,
                          struct flow_wildcards *, struct rule_dpif **rule,
                          bool take_ref);
 
+/* 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;
+}
+
 enum rule_dpif_lookup_verdict rule_dpif_lookup_from_table(struct ofproto_dpif *,
                                                           const struct flow *,
                                                           struct flow_wildcards *,
@@ -223,12 +236,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)