[ovs-dev] [xlate v3 3/8] ofproto-dpif: Ditch SLOW_MATCH slow path reason.
Ethan Jackson
ethan at nicira.com
Tue May 28 21:36:06 UTC 2013
Before this patch, datapath keys with ODP_FIT_TO_LITTLE, would be
assigned subfacets and installed in the kernel with a SLOW_MATCH
slow path reason. This is problematic, because these flow keys
can't be reliable converted into a 'struct flow' thus breaking a
fundamental assumption of ofproto-dpif. This patch circumvents the
issue by skipping facet creation for these flows altogether. This
approach has the added benefit of simplifying the code for future
patches.
Signed-off-by: Ethan Jackson <ethan at nicira.com>
---
lib/odp-util.c | 2 --
lib/odp-util.h | 4 ----
ofproto/ofproto-dpif.c | 34 +++++++++++++---------------------
tests/odp.at | 1 -
4 files changed, 13 insertions(+), 28 deletions(-)
diff --git a/lib/odp-util.c b/lib/odp-util.c
index 5bf94fe..aa50333 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -187,8 +187,6 @@ slow_path_reason_to_string(uint32_t data)
return "bfd";
case SLOW_CONTROLLER:
return "controller";
- case SLOW_MATCH:
- return "match";
default:
return NULL;
}
diff --git a/lib/odp-util.h b/lib/odp-util.h
index a981d17..a2f2b14 100644
--- a/lib/odp-util.h
+++ b/lib/odp-util.h
@@ -184,10 +184,6 @@ enum slow_path_reason {
/* Mutually exclusive with SLOW_BFD, SLOW_CFM, SLOW_LACP, SLOW_STP.
* Could possibly appear with SLOW_IN_BAND. */
SLOW_CONTROLLER = 1 << 5, /* Packets must go to OpenFlow controller. */
-
- /* This can appear on its own, or, theoretically at least, along with any
- * other combination of reasons. */
- SLOW_MATCH = 1 << 6, /* Datapath can't match specifically enough. */
};
#endif /* odp-util.h */
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 22544f1..9a196f4 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -3808,7 +3808,13 @@ handle_flow_miss(struct flow_miss *miss, struct flow_miss_op *ops,
if (!facet) {
struct rule_dpif *rule = rule_dpif_lookup(ofproto, &miss->flow);
- if (!flow_miss_should_make_facet(ofproto, miss, hash)) {
+ /* There does not exist a bijection between 'struct flow' and datapath
+ * flow keys with fitness ODP_FIT_TO_LITTLE. This breaks a fundamental
+ * assumption used throughout the facet and subfacet handling code.
+ * Since we have to handle these misses in userspace anyway, we simply
+ * skip facet creation, avoiding the problem alltogether. */
+ if (miss->key_fitness == ODP_FIT_TOO_LITTLE
+ || !flow_miss_should_make_facet(ofproto, miss, hash)) {
handle_flow_miss_without_facet(miss, rule, ops, n_ops);
return;
}
@@ -5122,19 +5128,16 @@ facet_revalidate(struct facet *facet)
memset(&ctx, 0, sizeof ctx);
ofpbuf_use_stub(&odp_actions, odp_actions_stub, sizeof odp_actions_stub);
LIST_FOR_EACH (subfacet, list_node, &facet->subfacets) {
- enum slow_path_reason slow;
-
action_xlate_ctx_init(&ctx, ofproto, &facet->flow,
&subfacet->initial_vals, new_rule, 0, NULL);
xlate_actions(&ctx, new_rule->up.ofpacts, new_rule->up.ofpacts_len,
&odp_actions);
- slow = (subfacet->slow & SLOW_MATCH) | ctx.slow;
- if (subfacet_should_install(subfacet, slow, &odp_actions)) {
+ if (subfacet_should_install(subfacet, ctx.slow, &odp_actions)) {
struct dpif_flow_stats stats;
- subfacet_install(subfacet,
- odp_actions.data, odp_actions.size, &stats, slow);
+ subfacet_install(subfacet, odp_actions.data, odp_actions.size,
+ &stats, ctx.slow);
subfacet_update_stats(subfacet, &stats);
if (!new_actions) {
@@ -5164,7 +5167,7 @@ facet_revalidate(struct facet *facet)
i = 0;
LIST_FOR_EACH (subfacet, list_node, &facet->subfacets) {
- subfacet->slow = (subfacet->slow & SLOW_MATCH) | ctx.slow;
+ subfacet->slow = ctx.slow;
if (new_actions && new_actions[i].odp_actions) {
free(subfacet->actions);
@@ -5363,9 +5366,7 @@ subfacet_create(struct facet *facet, struct flow_miss *miss,
subfacet->dp_byte_count = 0;
subfacet->actions_len = 0;
subfacet->actions = NULL;
- subfacet->slow = (subfacet->key_fitness == ODP_FIT_TOO_LITTLE
- ? SLOW_MATCH
- : 0);
+ subfacet->slow = 0;
subfacet->path = SF_NOT_INSTALLED;
subfacet->initial_vals = miss->initial_vals;
subfacet->odp_in_port = miss->odp_in_port;
@@ -5460,7 +5461,7 @@ subfacet_make_actions(struct subfacet *subfacet, const struct ofpbuf *packet,
facet->nf_flow.output_iface = ctx.nf_output_iface;
facet->mirrors = ctx.mirrors;
- subfacet->slow = (subfacet->slow & SLOW_MATCH) | ctx.slow;
+ subfacet->slow = ctx.slow;
if (subfacet->actions_len != odp_actions->size
|| memcmp(subfacet->actions, odp_actions->data, odp_actions->size)) {
free(subfacet->actions);
@@ -8327,19 +8328,10 @@ ofproto_trace(struct ofproto_dpif *ofproto, const struct flow *flow,
ds_put_cstr(ds, "\n\t- Sends \"packet-in\" messages "
"to the OpenFlow controller.");
break;
- case SLOW_MATCH:
- ds_put_cstr(ds, "\n\t- Needs more specific matching "
- "than the datapath supports.");
- break;
}
slow &= ~bit;
}
-
- if (slow & ~SLOW_MATCH) {
- ds_put_cstr(ds, "\nThe datapath actions above do not reflect "
- "the special slow-path processing.");
- }
}
}
}
diff --git a/tests/odp.at b/tests/odp.at
index a6bcdf5..fd6a192 100644
--- a/tests/odp.at
+++ b/tests/odp.at
@@ -87,7 +87,6 @@ userspace(pid=555666777)
userspace(pid=6633,sFlow(vid=9,pcp=7,output=10))
userspace(pid=9765,slow_path())
userspace(pid=9765,slow_path(cfm))
-userspace(pid=9765,slow_path(cfm,match))
userspace(pid=1234567,userdata(0102030405060708090a0b0c0d0e0f))
userspace(pid=6633,flow_sample(probability=123,collector_set_id=1234,obs_domain_id=2345,obs_point_id=3456))
userspace(pid=6633,ipfix)
--
1.7.9.5
More information about the dev
mailing list