[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