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 <aatt...@nicira.com>
Signed-off-by: Alex Wang <al...@nicira.com>
---
ofproto/ofproto-dpif-xlate.c | 7 ++-
ofproto/ofproto-dpif.c | 35 ++++-------
ofproto/ofproto-dpif.h | 139 ++++++++++++++++++++++--------------------
3 files changed, 91 insertions(+), 90 deletions(-)
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 4f77ac5..0fc5443 100644
--- 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
--- 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..58e4f22 100644
--- a/ofproto/ofproto-dpif.h
+++ b/ofproto/ofproto-dpif.h
@@ -87,72 +87,6 @@ extern struct ovs_rwlock xlate_rwlock;
size_t ofproto_dpif_get_max_mpls_depth(const struct ofproto_dpif *);
bool ofproto_dpif_get_enable_recirc(const struct ofproto_dpif *);
-uint8_t rule_dpif_lookup(struct ofproto_dpif *, struct flow *,
- struct flow_wildcards *, struct rule_dpif **rule,
- bool take_ref);
-
-enum rule_dpif_lookup_verdict rule_dpif_lookup_from_table(struct ofproto_dpif
*,
- const struct flow *,
- struct
flow_wildcards *,
- bool
force_controller_on_miss,
- uint8_t *table_id,
- struct rule_dpif
**rule,
- bool take_ref);
-
-static inline void rule_dpif_ref(struct rule_dpif *);
-static inline void rule_dpif_unref(struct rule_dpif *);
-
-void rule_dpif_credit_stats(struct rule_dpif *rule ,
- const struct dpif_flow_stats *);
-
-static inline bool rule_dpif_is_fail_open(const struct rule_dpif *);
-static inline bool rule_dpif_is_table_miss(const struct rule_dpif *);
-static inline bool rule_dpif_is_internal(const struct rule_dpif *);
-
-uint8_t rule_dpif_get_table(const struct rule_dpif *);
-
-bool table_is_internal(uint8_t table_id);
-
-const struct rule_actions *rule_dpif_get_actions(const struct rule_dpif *);
-
-ovs_be64 rule_dpif_get_flow_cookie(const struct rule_dpif *rule);
-
-void rule_dpif_reduce_timeouts(struct rule_dpif *rule, uint16_t idle_timeout,
- uint16_t hard_timeout);
-
-void choose_miss_rule(enum ofputil_port_config,
- struct rule_dpif *miss_rule,
- struct rule_dpif *no_packet_in_rule,
- struct rule_dpif **rule, bool take_ref);
-
-bool group_dpif_lookup(struct ofproto_dpif *ofproto, uint32_t group_id,
- struct group_dpif **group);
-
-void group_dpif_release(struct group_dpif *group);
-
-void group_dpif_get_buckets(const struct group_dpif *group,
- const struct list **buckets);
-enum ofp11_group_type group_dpif_get_type(const struct group_dpif *group);
-
-bool ofproto_has_vlan_splinters(const struct ofproto_dpif *);
-ofp_port_t vsp_realdev_to_vlandev(const struct ofproto_dpif *,
- ofp_port_t realdev_ofp_port,
- ovs_be16 vlan_tci);
-bool vsp_adjust_flow(const struct ofproto_dpif *, struct flow *);
-
-int ofproto_dpif_execute_actions(struct ofproto_dpif *, const struct flow *,
- struct rule_dpif *, const struct ofpact *,
- size_t ofpacts_len, struct ofpbuf *)
- OVS_EXCLUDED(xlate_rwlock);
-void ofproto_dpif_send_packet_in(struct ofproto_dpif *,
- struct ofproto_packet_in *);
-bool ofproto_dpif_wants_packet_in_on_miss(struct ofproto_dpif *);
-int ofproto_dpif_send_packet(const struct ofport_dpif *, struct ofpbuf *);
-void ofproto_dpif_flow_mod(struct ofproto_dpif *, struct ofputil_flow_mod *);
-struct rule_dpif *ofproto_dpif_refresh_rule(struct rule_dpif *);
-
-struct ofport_dpif *odp_port_to_ofport(const struct dpif_backer *, odp_port_t);
-
/*
* Recirculation
* =============
@@ -229,6 +163,79 @@ enum { N_TABLES = 255 };
enum { TBL_INTERNAL = N_TABLES - 1 }; /* Used for internal hidden rules. */
BUILD_ASSERT_DECL(N_TABLES >= 2 && N_TABLES <= 255);
+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 *,
+ bool
force_controller_on_miss,
+ uint8_t *table_id,
+ struct rule_dpif
**rule,
+ bool take_ref);
+
+static inline void rule_dpif_ref(struct rule_dpif *);
+static inline void rule_dpif_unref(struct rule_dpif *);
+
+void rule_dpif_credit_stats(struct rule_dpif *rule ,
+ const struct dpif_flow_stats *);
+
+static inline bool rule_dpif_is_fail_open(const struct rule_dpif *);
+static inline bool rule_dpif_is_table_miss(const struct rule_dpif *);
+static inline bool rule_dpif_is_internal(const struct rule_dpif *);
+
+uint8_t rule_dpif_get_table(const struct rule_dpif *);
+
+bool table_is_internal(uint8_t table_id);
+
+const struct rule_actions *rule_dpif_get_actions(const struct rule_dpif *);
+
+ovs_be64 rule_dpif_get_flow_cookie(const struct rule_dpif *rule);
+
+void rule_dpif_reduce_timeouts(struct rule_dpif *rule, uint16_t idle_timeout,
+ uint16_t hard_timeout);
+
+void choose_miss_rule(enum ofputil_port_config,
+ struct rule_dpif *miss_rule,
+ struct rule_dpif *no_packet_in_rule,
+ struct rule_dpif **rule, bool take_ref);
+
+bool group_dpif_lookup(struct ofproto_dpif *ofproto, uint32_t group_id,
+ struct group_dpif **group);
+
+void group_dpif_release(struct group_dpif *group);
+
+void group_dpif_get_buckets(const struct group_dpif *group,
+ const struct list **buckets);
+enum ofp11_group_type group_dpif_get_type(const struct group_dpif *group);
+
+bool ofproto_has_vlan_splinters(const struct ofproto_dpif *);
+ofp_port_t vsp_realdev_to_vlandev(const struct ofproto_dpif *,
+ ofp_port_t realdev_ofp_port,
+ ovs_be16 vlan_tci);
+bool vsp_adjust_flow(const struct ofproto_dpif *, struct flow *);
+
+int ofproto_dpif_execute_actions(struct ofproto_dpif *, const struct flow *,
+ struct rule_dpif *, const struct ofpact *,
+ size_t ofpacts_len, struct ofpbuf *)
+ OVS_EXCLUDED(xlate_rwlock);
+void ofproto_dpif_send_packet_in(struct ofproto_dpif *,
+ struct ofproto_packet_in *);
+bool ofproto_dpif_wants_packet_in_on_miss(struct ofproto_dpif *);
+int ofproto_dpif_send_packet(const struct ofport_dpif *, struct ofpbuf *);
+void ofproto_dpif_flow_mod(struct ofproto_dpif *, struct ofputil_flow_mod *);
+struct rule_dpif *ofproto_dpif_refresh_rule(struct rule_dpif *);
+
+struct ofport_dpif *odp_port_to_ofport(const struct dpif_backer *, odp_port_t);
/* struct rule_dpif has struct rule as it's first member. */
#define RULE_CAST(RULE) ((struct rule *)RULE)
--
1.7.9.5
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
