Skip to content

Commit

Permalink
ofproto: Honour Table Mod settings for table-miss handling
Browse files Browse the repository at this point in the history 
This reworks lookup of rules for both table 0 and table action translation.
The result is that Table Mod settings, which can alter the miss-behaviour
of tables, including table 0, on a per-table basis may be honoured.

Previous patches proposed by myself which build on earlier merged patches
by Andy Zhou implement the ofproto side of Table Mod. So with this patch
the feature should be complete.

Neither this patch, nor any other patches it builds on, alter the default
behaviour of Open vSwitch. And in particular the OpenFlow1.1 behaviour is
the default regardless of which OpenFlow version is negotiated between the
switch and the controller.

An implementation detail, which lends itself to future work, is the
handling of OFPTC_TABLE_MISS_CONTINUE. If a table has this behaviour set by
Table Mod and a miss occurs then a loop is created, skipping to the next
table. It is quite easy to create a situation where this loop covers ~255
tables which is very expensive as the lookup for each table involves taking
locks, amongst other things.

Cc: Andy Zhou <[email protected]>
Signed-off-by: Simon Horman <[email protected]>
[[email protected] updated comments and refactored]
Signed-off-by: Ben Pfaff <[email protected]>
  • Loading branch information
@horms @blp
horms authored and blp committed Mar 20, 2014
1 parent 5bb8a63 commit 6d328fa
Show file tree
Hide file tree
Showing 7 changed files with 562 additions and 87 deletions.
5 changes: 0 additions & 5 deletions OPENFLOW-1.1+
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,6 @@ OpenFlow 1.1
The list of remaining work items for OpenFlow 1.1 is below. It is
probably incomplete.

* OFPT_TABLE_MOD message. This is new in OF1.1, so we need to
implement it. It should be implemented so that the default OVS
behavior does not change. Simon Horman has posted a patch.
[required for OF1.1 and OF1.2]

* MPLS. Simon Horman maintains a patch series that adds this
feature. This is partially merged.
[optional for OF1.1+]
Expand Down
71 changes: 44 additions & 27 deletions ofproto/ofproto-dpif-xlate.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -228,8 +228,9 @@ static void xlate_actions__(struct xlate_in *, struct xlate_out *)
OVS_REQ_RDLOCK(xlate_rwlock);
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);
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);
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 *,
Expand Down Expand Up @@ -1736,14 +1737,14 @@ 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);
xlate_table_action(ctx, flow->in_port.ofp_port, 0, 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 = ctx->xout->odp_actions.size;
mirror_mask_t old_mirrors = ctx->xout->mirrors;
xlate_table_action(ctx, flow->in_port.ofp_port, 0, true);
xlate_table_action(ctx, flow->in_port.ofp_port, 0, true, true);
ctx->xout->mirrors = old_mirrors;
ctx->base_flow = old_base_flow;
ctx->xout->odp_actions.size = old_size;
Expand Down Expand Up @@ -1878,44 +1879,59 @@ 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)
xlate_table_action(struct xlate_ctx *ctx, ofp_port_t in_port, uint8_t table_id,
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;
bool skip_wildcards = ctx->xin->skip_wildcards;
uint8_t old_table_id = ctx->table_id;
struct rule_dpif *rule;
enum rule_dpif_lookup_verdict verdict;
enum ofputil_port_config config = 0;

ctx->table_id = table_id;

/* Look up a flow with 'in_port' as the input port. Then restore the
* original input port (otherwise OFPP_NORMAL and OFPP_IN_PORT will
* have surprising behavior). */
ctx->xin->flow.in_port.ofp_port = in_port;
rule_dpif_lookup_in_table(ctx->xbridge->ofproto, &ctx->xin->flow,
!skip_wildcards ? &ctx->xout->wc : NULL,
table_id, &rule);
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->flow.in_port.ofp_port = old_in_port;

if (ctx->xin->resubmit_hook) {
ctx->xin->resubmit_hook(ctx->xin, rule, ctx->recurse);
}

if (!rule && may_packet_in) {
struct xport *xport;

/* XXX
* check if table configuration flags
* OFPTC11_TABLE_MISS_CONTROLLER, default.
* OFPTC11_TABLE_MISS_CONTINUE,
* OFPTC11_TABLE_MISS_DROP
* When OF1.0, OFPTC11_TABLE_MISS_CONTINUE is used. What to do? */
xport = get_ofp_port(ctx->xbridge, ctx->xin->flow.in_port.ofp_port);
choose_miss_rule(xport ? xport->config : 0,
ctx->xbridge->miss_rule,
ctx->xbridge->no_packet_in_rule, &rule);
switch (verdict) {
case RULE_DPIF_LOOKUP_VERDICT_MATCH:
goto match;
case RULE_DPIF_LOOKUP_VERDICT_CONTROLLER:
if (may_packet_in) {
struct xport *xport;

xport = get_ofp_port(ctx->xbridge,
ctx->xin->flow.in_port.ofp_port);
config = xport ? xport->config : 0;
break;
}
/* Fall through to drop */
case RULE_DPIF_LOOKUP_VERDICT_DROP:
config = OFPUTIL_PC_NO_PACKET_IN;
break;
default:
OVS_NOT_REACHED();
}

choose_miss_rule(config, ctx->xbridge->miss_rule,
ctx->xbridge->no_packet_in_rule, &rule);

match:
if (rule) {
xlate_recursively(ctx, rule);
rule_dpif_unref(rule);
Expand Down Expand Up @@ -2076,7 +2092,7 @@ xlate_ofpact_resubmit(struct xlate_ctx *ctx,
table_id = ctx->table_id;
}

xlate_table_action(ctx, in_port, table_id, false);
xlate_table_action(ctx, in_port, table_id, false, false);
}

static void
Expand Down Expand Up @@ -2284,7 +2300,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);
0, may_packet_in, true);
break;
case OFPP_NORMAL:
xlate_normal(ctx);
Expand Down Expand Up @@ -2797,7 +2813,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);
ogt->table_id, true, true);
break;
}

Expand Down Expand Up @@ -3024,8 +3040,9 @@ xlate_actions__(struct xlate_in *xin, struct xlate_out *xout)
ctx.exit = false;

if (!xin->ofpacts && !ctx.rule) {
rule_dpif_lookup(ctx.xbridge->ofproto, flow,
!xin->skip_wildcards ? wc : NULL, &rule);
ctx.table_id = rule_dpif_lookup(ctx.xbridge->ofproto, flow,
!xin->skip_wildcards ? wc : NULL,
&rule);
if (ctx.xin->resubmit_stats) {
rule_dpif_credit_stats(rule, ctx.xin->resubmit_stats);
}
Expand Down
179 changes: 131 additions & 48 deletions ofproto/ofproto-dpif.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1095,6 +1095,7 @@ add_internal_flow(struct ofproto_dpif *ofproto, int id,
const struct ofpbuf *ofpacts, struct rule_dpif **rulep)
{
struct ofputil_flow_mod fm;
struct classifier *cls;
int error;

match_init_catchall(&fm.match);
Expand All @@ -1121,12 +1122,12 @@ add_internal_flow(struct ofproto_dpif *ofproto, int id,
return error;
}

if (rule_dpif_lookup_in_table(ofproto, &fm.match.flow, NULL, TBL_INTERNAL,
rulep)) {
rule_dpif_unref(*rulep);
} else {
OVS_NOT_REACHED();
}
cls = &ofproto->up.tables[TBL_INTERNAL].cls;
fat_rwlock_rdlock(&cls->rwlock);
*rulep = rule_dpif_cast(rule_from_cls_rule(
classifier_lookup(cls, &fm.match.flow, NULL)));
ovs_assert(*rulep != NULL);
fat_rwlock_unlock(&cls->rwlock);

return 0;
}
Expand Down Expand Up @@ -3056,69 +3057,151 @@ rule_dpif_get_actions(const struct rule_dpif *rule)
return rule_get_actions(&rule->up);
}

/* Lookup 'flow' in 'ofproto''s classifier. If 'wc' is non-null, sets
* the fields that were relevant as part of the lookup. */
void
/* Lookup 'flow' in table 0 of 'ofproto''s classifier.
* If 'wc' is non-null, sets the fields that were relevant as part of
* the lookup. Returns the table_id where a match or miss occurred.
*
* The return value will be zero unless there was a miss and
* OFPTC_TABLE_MISS_CONTINUE is in effect for the sequence of tables
* where misses occur. */
uint8_t
rule_dpif_lookup(struct ofproto_dpif *ofproto, const struct flow *flow,
struct flow_wildcards *wc, struct rule_dpif **rule)
{
struct ofport_dpif *port;
enum rule_dpif_lookup_verdict verdict;
enum ofputil_port_config config = 0;
uint8_t table_id = 0;

if (rule_dpif_lookup_in_table(ofproto, flow, wc, 0, rule)) {
return;
verdict = rule_dpif_lookup_from_table(ofproto, flow, wc, true,
&table_id, rule);

switch (verdict) {
case RULE_DPIF_LOOKUP_VERDICT_MATCH:
return table_id;
case RULE_DPIF_LOOKUP_VERDICT_CONTROLLER: {
struct ofport_dpif *port;

port = get_ofp_port(ofproto, flow->in_port.ofp_port);
if (!port) {
VLOG_WARN_RL(&rl, "packet-in on unknown OpenFlow port %"PRIu16,
flow->in_port.ofp_port);
}
config = port ? port->up.pp.config : 0;
break;
}
port = get_ofp_port(ofproto, flow->in_port.ofp_port);
if (!port) {
VLOG_WARN_RL(&rl, "packet-in on unknown OpenFlow port %"PRIu16,
flow->in_port.ofp_port);
case RULE_DPIF_LOOKUP_VERDICT_DROP:
config = OFPUTIL_PC_NO_PACKET_IN;
break;
default:
OVS_NOT_REACHED();
}

choose_miss_rule(port ? port->up.pp.config : 0, ofproto->miss_rule,
choose_miss_rule(config, ofproto->miss_rule,
ofproto->no_packet_in_rule, rule);
return table_id;
}

bool
rule_dpif_lookup_in_table(struct ofproto_dpif *ofproto,
const struct flow *flow, struct flow_wildcards *wc,
uint8_t table_id, struct rule_dpif **rule)
static struct rule_dpif *
rule_dpif_lookup_in_table(struct ofproto_dpif *ofproto, uint8_t table_id,
const struct flow *flow, struct flow_wildcards *wc)
{
struct classifier *cls = &ofproto->up.tables[table_id].cls;
const struct cls_rule *cls_rule;
struct classifier *cls;
bool frag;

*rule = NULL;
if (table_id >= N_TABLES) {
return false;
}
struct rule_dpif *rule;

if (wc) {
memset(&wc->masks.dl_type, 0xff, sizeof wc->masks.dl_type);
if (is_ip_any(flow)) {
wc->masks.nw_frag |= FLOW_NW_FRAG_MASK;
fat_rwlock_rdlock(&cls->rwlock);
if (ofproto->up.frag_handling != OFPC_FRAG_NX_MATCH) {
if (wc) {
memset(&wc->masks.dl_type, 0xff, sizeof wc->masks.dl_type);
if (is_ip_any(flow)) {
wc->masks.nw_frag |= FLOW_NW_FRAG_MASK;
}
}
}

cls = &ofproto->up.tables[table_id].cls;
fat_rwlock_rdlock(&cls->rwlock);
frag = (flow->nw_frag & FLOW_NW_FRAG_ANY) != 0;
if (frag && ofproto->up.frag_handling == OFPC_FRAG_NORMAL) {
/* We must pretend that transport ports are unavailable. */
struct flow ofpc_normal_flow = *flow;
ofpc_normal_flow.tp_src = htons(0);
ofpc_normal_flow.tp_dst = htons(0);
cls_rule = classifier_lookup(cls, &ofpc_normal_flow, wc);
} else if (frag && ofproto->up.frag_handling == OFPC_FRAG_DROP) {
cls_rule = &ofproto->drop_frags_rule->up.cr;
/* Frag mask in wc already set above. */
if (flow->nw_frag & FLOW_NW_FRAG_ANY) {
if (ofproto->up.frag_handling == OFPC_FRAG_NORMAL) {
/* We must pretend that transport ports are unavailable. */
struct flow ofpc_normal_flow = *flow;
ofpc_normal_flow.tp_src = htons(0);
ofpc_normal_flow.tp_dst = htons(0);
cls_rule = classifier_lookup(cls, &ofpc_normal_flow, wc);
} else {
/* Must be OFPC_FRAG_DROP (we don't have OFPC_FRAG_REASM). */
cls_rule = &ofproto->drop_frags_rule->up.cr;
}
} else {
cls_rule = classifier_lookup(cls, flow, wc);
}
} else {
cls_rule = classifier_lookup(cls, flow, wc);
}

*rule = rule_dpif_cast(rule_from_cls_rule(cls_rule));
rule_dpif_ref(*rule);
rule = rule_dpif_cast(rule_from_cls_rule(cls_rule));
rule_dpif_ref(rule);
fat_rwlock_unlock(&cls->rwlock);

return *rule != NULL;
return rule;
}

/* Look up 'flow' in 'ofproto''s classifier starting from table '*table_id'.
* Stores the rule that was found in '*rule', or NULL if none was found.
* Updates 'wc', if nonnull, to reflect the fields that were used during the
* lookup.
*
* If 'honor_table_miss' is true, the first lookup occurs in '*table_id', but
* if none is found then the table miss configuration for that table is
* honored, which can result in additional lookups in other OpenFlow tables.
* In this case the function updates '*table_id' to reflect the final OpenFlow
* table that was searched.
*
* If 'honor_table_miss' is false, then only one table lookup occurs, in
* '*table_id'.
*
* Returns:
*
* - RULE_DPIF_LOOKUP_VERDICT_MATCH if a rule (in '*rule') was found.
*
* - RULE_DPIF_LOOKUP_VERDICT_DROP if no rule was found and a table miss
* configuration specified that the packet should be dropped in this
* case. (This occurs only if 'honor_table_miss' is true, because only in
* this case does the table miss configuration matter.)
*
* - RULE_DPIF_LOOKUP_VERDICT_CONTROLLER if no rule was found otherwise. */
enum rule_dpif_lookup_verdict
rule_dpif_lookup_from_table(struct ofproto_dpif *ofproto,
const struct flow *flow,
struct flow_wildcards *wc,
bool honor_table_miss,
uint8_t *table_id, struct rule_dpif **rule)
{
uint8_t next_id;

for (next_id = *table_id;
next_id < ofproto->up.n_tables;
next_id++, next_id += (next_id == TBL_INTERNAL))
{
*table_id = next_id;
*rule = rule_dpif_lookup_in_table(ofproto, *table_id, flow, wc);
if (*rule) {
return RULE_DPIF_LOOKUP_VERDICT_MATCH;
} else if (!honor_table_miss) {
return RULE_DPIF_LOOKUP_VERDICT_CONTROLLER;
} else {
switch (table_get_config(&ofproto->up, *table_id)
& OFPTC11_TABLE_MISS_MASK) {
case OFPTC11_TABLE_MISS_CONTINUE:
break;

case OFPTC11_TABLE_MISS_CONTROLLER:
return RULE_DPIF_LOOKUP_VERDICT_CONTROLLER;

case OFPTC11_TABLE_MISS_DROP:
return RULE_DPIF_LOOKUP_VERDICT_DROP;
}
}
}

return RULE_DPIF_LOOKUP_VERDICT_CONTROLLER;
}

/* Given a port configuration (specified as zero if there's no port), chooses
Expand Down
Loading

0 comments on commit 6d328fa

Please sign in to comment.