odp-util: Make slow_path_reasons mutually exclusive.

It's no longer possible for a single datapath flow to be slow
pathed for two different reasons.  This patch updates the code to
reflect this fact (marginally simplifying it).

Signed-off-by: Ethan Jackson <ethan@nicira.com>

diff --git a/lib/odp-util.c b/lib/odp-util.c
index 4ff1f96..1c48e7b 100644 (file)
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -170,11 +170,9 @@ format_odp_sample_action(struct ds *ds, const struct nlattr *attr)
 }
 
 static const char *
-slow_path_reason_to_string(uint32_t data)
+slow_path_reason_to_string(enum slow_path_reason reason)
 {
-    enum slow_path_reason bit = (enum slow_path_reason) data;
-
-    switch (bit) {
+    switch (reason) {
     case SLOW_CFM:
         return "cfm";
     case SLOW_LACP:
@@ -185,11 +183,26 @@ slow_path_reason_to_string(uint32_t data)
         return "bfd";
     case SLOW_CONTROLLER:
         return "controller";
+    case __SLOW_MAX:
     default:
         return NULL;
     }
 }
 
+static enum slow_path_reason
+string_to_slow_path_reason(const char *string)
+{
+    enum slow_path_reason i;
+
+    for (i = 1; i < __SLOW_MAX; i++) {
+        if (!strcmp(string, slow_path_reason_to_string(i))) {
+            return i;
+        }
+    }
+
+    return 0;
+}
+
 static int
 parse_flags(const char *s, const char *(*bit_to_string)(uint32_t),
             uint32_t *res)
@@ -284,10 +297,10 @@ format_odp_userspace_action(struct ds *ds, const struct nlattr *attr)
                               cookie.sflow.output);
             } else if (userdata_len == sizeof cookie.slow_path
                        && cookie.type == USER_ACTION_COOKIE_SLOW_PATH) {
-                ds_put_cstr(ds, ",slow_path(");
-                format_flags(ds, slow_path_reason_to_string,
-                             cookie.slow_path.reason, ',');
-                ds_put_format(ds, ")");
+                const char *reason;
+                reason = slow_path_reason_to_string(cookie.slow_path.reason);
+                reason = reason ? reason : "";
+                ds_put_format(ds, ",slow_path(%s)", reason);
             } else if (userdata_len == sizeof cookie.flow_sample
                        && cookie.type == USER_ACTION_COOKIE_FLOW_SAMPLE) {
                 ds_put_format(ds, ",flow_sample(probability=%"PRIu16
@@ -498,25 +511,27 @@ parse_odp_action(const char *s, const struct simap *port_names,
             odp_put_userspace_action(pid, &cookie, sizeof cookie.sflow,
                                      actions);
             return n;
-        } else if (sscanf(s, "userspace(pid=%lli,slow_path%n", &pid, &n) > 0
+        } else if (sscanf(s, "userspace(pid=%lli,slow_path(%n", &pid, &n) > 0
                    && n > 0) {
             union user_action_cookie cookie;
-            int res;
+            char reason[32];
+
+            if (s[n] == ')' && s[n + 1] == ')') {
+                reason[0] = '\0';
+                n += 2;
+            } else if (sscanf(s + n, "%31[^)]))", reason) > 0) {
+                n += strlen(reason) + 2;
+            } else {
+                return -EINVAL;
+            }
 
             cookie.type = USER_ACTION_COOKIE_SLOW_PATH;
             cookie.slow_path.unused = 0;
-            cookie.slow_path.reason = 0;
+            cookie.slow_path.reason = string_to_slow_path_reason(reason);
 
-            res = parse_flags(&s[n], slow_path_reason_to_string,
-                              &cookie.slow_path.reason);
-            if (res < 0) {
-                return res;
-            }
-            n += res;
-            if (s[n] != ')') {
+            if (reason[0] && !cookie.slow_path.reason) {
                 return -EINVAL;
             }
-            n++;
 
             odp_put_userspace_action(pid, &cookie, sizeof cookie.slow_path,
                                      actions);

diff --git a/lib/odp-util.h b/lib/odp-util.h
index 0892cf7..0455370 100644 (file)
--- a/lib/odp-util.h
+++ b/lib/odp-util.h
@@ -174,12 +174,12 @@ void odp_put_skb_mark_action(const uint32_t skb_mark,
 
 /* Reasons why a subfacet might not be fast-pathable. */
 enum slow_path_reason {
-    /* These reasons are mutually exclusive. */
-    SLOW_CFM = 1 << 0,          /* CFM packets need per-packet processing. */
-    SLOW_LACP = 1 << 1,         /* LACP packets need per-packet processing. */
-    SLOW_STP = 1 << 2,          /* STP packets need per-packet processing. */
-    SLOW_BFD = 1 << 3,          /* BFD packets need per-packet processing. */
-    SLOW_CONTROLLER = 1 << 4,   /* Packets must go to OpenFlow controller. */
+    SLOW_CFM = 1,               /* CFM packets need per-packet processing. */
+    SLOW_LACP,                  /* LACP packets need per-packet processing. */
+    SLOW_STP,                   /* STP packets need per-packet processing. */
+    SLOW_BFD,                   /* BFD packets need per-packet processing. */
+    SLOW_CONTROLLER,            /* Packets must go to OpenFlow controller. */
+    __SLOW_MAX
 };
 
 #endif /* odp-util.h */

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 55d030f..a74702b 100644 (file)
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -6119,7 +6119,7 @@ compose_output_action__(struct action_xlate_ctx *ctx, uint16_t ofp_port,
         special = process_special(ctx->ofproto, &ctx->flow, in_port,
                                   ctx->packet);
         if (special) {
-            ctx->slow |= special;
+            ctx->slow = special;
         } else if (!in_port || may_receive(in_port, ctx)) {
             if (!in_port || stp_forward_in_state(in_port->stp_state)) {
                 xlate_table_action(ctx, ctx->flow.in_port, 0, true);
@@ -6341,7 +6341,8 @@ execute_controller_action(struct action_xlate_ctx *ctx, int len,
     struct ofputil_packet_in pin;
     struct ofpbuf *packet;
 
-    ctx->slow |= SLOW_CONTROLLER;
+    ovs_assert(!ctx->slow || ctx->slow == SLOW_CONTROLLER);
+    ctx->slow = SLOW_CONTROLLER;
     if (!ctx->packet) {
         return;
     }
@@ -7117,7 +7118,7 @@ xlate_actions(struct action_xlate_ctx *ctx,
     in_port = get_ofp_port(ctx->ofproto, ctx->flow.in_port);
     special = process_special(ctx->ofproto, &ctx->flow, in_port, ctx->packet);
     if (special) {
-        ctx->slow |= special;
+        ctx->slow = special;
     } else {
         static struct vlog_rate_limit trace_rl = VLOG_RATE_LIMIT_INIT(1, 1);
         struct initial_vals initial_vals;
@@ -8281,33 +8282,27 @@ ofproto_trace(struct ofproto_dpif *ofproto, const struct flow *flow,
         ofpbuf_uninit(&odp_actions);
 
         if (trace.ctx.slow) {
-            enum slow_path_reason slow;
-
             ds_put_cstr(ds, "\nThis flow is handled by the userspace "
                         "slow path because it:");
-            for (slow = trace.ctx.slow; slow; ) {
-                enum slow_path_reason bit = rightmost_1bit(slow);
-
-                switch (bit) {
-                case SLOW_CFM:
-                    ds_put_cstr(ds, "\n\t- Consists of CFM packets.");
-                    break;
-                case SLOW_LACP:
-                    ds_put_cstr(ds, "\n\t- Consists of LACP packets.");
-                    break;
-                case SLOW_STP:
-                    ds_put_cstr(ds, "\n\t- Consists of STP packets.");
-                    break;
-                case SLOW_BFD:
-                    ds_put_cstr(ds, "\n\t- Consists of BFD packets.");
-                    break;
-                case SLOW_CONTROLLER:
-                    ds_put_cstr(ds, "\n\t- Sends \"packet-in\" messages "
-                                "to the OpenFlow controller.");
-                    break;
-                }
-
-                slow &= ~bit;
+            switch (trace.ctx.slow) {
+            case SLOW_CFM:
+                ds_put_cstr(ds, "\n\t- Consists of CFM packets.");
+                break;
+            case SLOW_LACP:
+                ds_put_cstr(ds, "\n\t- Consists of LACP packets.");
+                break;
+            case SLOW_STP:
+                ds_put_cstr(ds, "\n\t- Consists of STP packets.");
+                break;
+            case SLOW_BFD:
+                ds_put_cstr(ds, "\n\t- Consists of BFD packets.");
+                break;
+            case SLOW_CONTROLLER:
+                ds_put_cstr(ds, "\n\t- Sends \"packet-in\" messages "
+                            "to the OpenFlow controller.");
+                break;
+            case __SLOW_MAX:
+                NOT_REACHED();
             }
         }
     }