[ovs-dev] [branch-2.3 V2] recirculation: Do not drop packet when there is no match from internal table.
Alex Wang
alexw at nicira.com
Sat Dec 13 00:20:17 UTC 2014
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 at nicira.com>
Signed-off-by: Alex Wang <alexw at nicira.com>
---
ofproto/ofproto-dpif-xlate.c | 42 +++++++++++++++++++++++++++---------------
ofproto/ofproto-dpif.c | 5 ++---
2 files changed, 29 insertions(+), 18 deletions(-)
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 4f77ac5..79cb928 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -306,8 +306,8 @@ static void xlate_actions__(struct xlate_in *, struct xlate_out *)
static void xlate_normal(struct xlate_ctx *);
static void xlate_report(struct xlate_ctx *, const char *);
static void xlate_table_action(struct xlate_ctx *, ofp_port_t in_port,
- uint8_t table_id, bool may_packet_in,
- bool honor_table_miss);
+ uint8_t table_id, bool new_xbridge,
+ bool may_packet_in, bool honor_table_miss);
static bool input_vid_is_valid(uint16_t vid, struct xbundle *, bool warn);
static uint16_t input_vid_to_vlan(const struct xbundle *, uint16_t vid);
static void output_normal(struct xlate_ctx *, const struct xbundle *,
@@ -1859,14 +1859,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, 0, true,
+ 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, 0, true,
+ true, true);
ctx->xout->mirrors = old_mirrors;
ctx->base_flow = old_base_flow;
ofpbuf_set_size(&ctx->xout->odp_actions, old_size);
@@ -2033,7 +2035,7 @@ xlate_resubmit_resource_check(struct xlate_ctx *ctx)
static void
xlate_table_action(struct xlate_ctx *ctx, ofp_port_t in_port, uint8_t table_id,
- bool may_packet_in, bool honor_table_miss)
+ bool new_xbridge, bool may_packet_in, bool honor_table_miss)
{
if (xlate_resubmit_resource_check(ctx)) {
ofp_port_t old_in_port = ctx->xin->flow.in_port.ofp_port;
@@ -2049,13 +2051,23 @@ xlate_table_action(struct xlate_ctx *ctx, ofp_port_t in_port, uint8_t table_id,
* original input port (otherwise OFPP_NORMAL and OFPP_IN_PORT will
* have surprising behavior). */
ctx->xin->flow.in_port.ofp_port = in_port;
- verdict = rule_dpif_lookup_from_table(ctx->xbridge->ofproto,
- &ctx->xin->flow,
- !skip_wildcards
- ? &ctx->xout->wc : NULL,
- honor_table_miss,
- &ctx->table_id, &rule,
- ctx->xin->xcache != NULL);
+ /* If the action is to a new xbridge (e.g. via patch port), conducts
+ * generic lookup. */
+ if (new_xbridge) {
+ verdict = rule_dpif_lookup(ctx->xbridge->ofproto,
+ &ctx->xin->flow,
+ !skip_wildcards
+ ? &ctx->xout->wc : NULL,
+ &rule, ctx->xin->xcache != NULL);
+ } else {
+ verdict = rule_dpif_lookup_from_table(ctx->xbridge->ofproto,
+ &ctx->xin->flow,
+ !skip_wildcards
+ ? &ctx->xout->wc : NULL,
+ honor_table_miss,
+ &ctx->table_id, &rule,
+ ctx->xin->xcache != NULL);
+ }
ctx->xin->flow.in_port.ofp_port = old_in_port;
if (ctx->xin->resubmit_hook) {
@@ -2270,7 +2282,7 @@ xlate_ofpact_resubmit(struct xlate_ctx *ctx,
table_id = ctx->table_id;
}
- xlate_table_action(ctx, in_port, table_id, may_packet_in,
+ xlate_table_action(ctx, in_port, table_id, false, may_packet_in,
honor_table_miss);
}
@@ -2494,7 +2506,7 @@ xlate_output_action(struct xlate_ctx *ctx,
break;
case OFPP_TABLE:
xlate_table_action(ctx, ctx->xin->flow.in_port.ofp_port,
- 0, may_packet_in, true);
+ 0, false, may_packet_in, true);
break;
case OFPP_NORMAL:
xlate_normal(ctx);
@@ -3047,7 +3059,7 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
ovs_assert(ctx->table_id < ogt->table_id);
xlate_table_action(ctx, ctx->xin->flow.in_port.ofp_port,
- ogt->table_id, true, true);
+ ogt->table_id, false, true, true);
break;
}
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 46595f8..787fc3f 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -1236,9 +1236,9 @@ add_internal_flows(struct ofproto_dpif *ofproto)
return error;
}
- /* Continue non-recirculation rule lookups from table 0.
+ /* Continue rule lookups from table 0.
*
- * (priority=2), recirc=0, actions=resubmit(, 0)
+ * (priority=2), actions=resubmit(, 0)
*/
resubmit = ofpact_put_RESUBMIT(&ofpacts);
resubmit->ofpact.compat = 0;
@@ -1246,7 +1246,6 @@ add_internal_flows(struct ofproto_dpif *ofproto)
resubmit->table_id = 0;
match_init_catchall(&match);
- match_set_recirc_id(&match, 0);
error = ofproto_dpif_add_internal_flow(ofproto, &match, 2, &ofpacts,
&unused_rulep);
--
1.7.9.5
More information about the dev
mailing list