Skip to content

Commit

Permalink
recirculation: Do not drop packet when there is no match from
Browse files Browse the repository at this point in the history 
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 <[email protected]>
Signed-off-by: Alex Wang <[email protected]>
Acked-by: Andy Zhou <[email protected]>
  • Loading branch information
@yew011
yew011 committed Dec 13, 2014
1 parent bc78679 commit 0c7812e
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 24 deletions.
7 changes: 5 additions & 2 deletions ofproto/ofproto-dpif-xlate.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand Down
27 changes: 11 additions & 16 deletions ofproto/ofproto-dpif.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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,
Expand Down
19 changes: 13 additions & 6 deletions ofproto/ofproto-dpif.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 *);

Expand Down Expand Up @@ -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);


/* struct rule_dpif has struct rule as it's first member. */
#define RULE_CAST(RULE) ((struct rule *)RULE)
Expand Down

0 comments on commit 0c7812e

Please sign in to comment.