[ovs-dev] [arp-rw 11/13] odp-util: Make it possible to combine slow path reasons.

Ben Pfaff blp at nicira.com
Mon Sep 23 17:49:57 UTC 2013


It will soon be possible for a single flow to be slow pathed for multiple
reasons.  This commit makes it possible to indicate more than one reason
to slow path a flow.

This commit is logically a revert of commit 98f0520fb2 (odp-util: Make
slow_path_reasons mutually exclusive.) but details have changed.

Signed-off-by: Ben Pfaff <blp at nicira.com>
---
 lib/odp-util.c               |   70 +++++++++++++++++-------------------------
 lib/odp-util.h               |   32 ++++++++++++++-----
 ofproto/ofproto-dpif-xlate.c |    7 ++---
 ofproto/ofproto-dpif.c       |   30 +++++++-----------
 4 files changed, 68 insertions(+), 71 deletions(-)

diff --git a/lib/odp-util.c b/lib/odp-util.c
index 5ca8baf..0edbbf0 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -175,37 +175,27 @@ format_odp_sample_action(struct ds *ds, const struct nlattr *attr)
 }
 
 static const char *
-slow_path_reason_to_string(enum slow_path_reason reason)
+slow_path_reason_to_string(uint32_t reason)
 {
-    switch (reason) {
-    case SLOW_CFM:
-        return "cfm";
-    case SLOW_LACP:
-        return "lacp";
-    case SLOW_STP:
-        return "stp";
-    case SLOW_BFD:
-        return "bfd";
-    case SLOW_CONTROLLER:
-        return "controller";
-    case __SLOW_MAX:
-    default:
-        return NULL;
+    switch ((enum slow_path_reason) reason) {
+#define SPR(ENUM, STRING, EXPLANATION) case ENUM: return STRING;
+        SLOW_PATH_REASONS
+#undef SPR
     }
+
+    return NULL;
 }
 
-static enum slow_path_reason
-string_to_slow_path_reason(const char *string)
+const char *
+slow_path_reason_to_explanation(enum slow_path_reason reason)
 {
-    enum slow_path_reason i;
-
-    for (i = 1; i < __SLOW_MAX; i++) {
-        if (!strcmp(string, slow_path_reason_to_string(i))) {
-            return i;
-        }
+    switch (reason) {
+#define SPR(ENUM, STRING, EXPLANATION) case ENUM: return EXPLANATION;
+        SLOW_PATH_REASONS
+#undef SPR
     }
 
-    return 0;
+    return "<unknown>";
 }
 
 static int
@@ -302,10 +292,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) {
-                const char *reason;
-                reason = slow_path_reason_to_string(cookie.slow_path.reason);
-                reason = reason ? reason : "";
-                ds_put_format(ds, ",slow_path(%s)", reason);
+                ds_put_cstr(ds, ",slow_path(");
+                format_flags(ds, slow_path_reason_to_string,
+                             cookie.slow_path.reason, ',');
+                ds_put_format(ds, ")");
             } else if (userdata_len == sizeof cookie.flow_sample
                        && cookie.type == USER_ACTION_COOKIE_FLOW_SAMPLE) {
                 ds_put_format(ds, ",flow_sample(probability=%"PRIu16
@@ -535,27 +525,25 @@ 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;
-            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;
-            }
+            int res;
 
             cookie.type = USER_ACTION_COOKIE_SLOW_PATH;
             cookie.slow_path.unused = 0;
-            cookie.slow_path.reason = string_to_slow_path_reason(reason);
+            cookie.slow_path.reason = 0;
 
-            if (reason[0] && !cookie.slow_path.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] != ')') {
                 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 192cfa0..6e06f7f 100644
--- a/lib/odp-util.h
+++ b/lib/odp-util.h
@@ -187,14 +187,32 @@ void odp_put_tunnel_action(const struct flow_tnl *tunnel,
 void odp_put_pkt_mark_action(const uint32_t pkt_mark,
                              struct ofpbuf *odp_actions);
 
-/* Reasons why a subfacet might not be fast-pathable. */
+#define SLOW_PATH_REASONS                                               \
+    /* These reasons are mutually exclusive. */                         \
+    SPR(SLOW_CFM,        "cfm",        "Consists of CFM packets")       \
+    SPR(SLOW_BFD,        "bfd",        "Consists of BFD packets")       \
+    SPR(SLOW_LACP,       "lacp",       "Consists of LACP packets")      \
+    SPR(SLOW_STP,        "stp",        "Consists of STP packets")       \
+    SPR(SLOW_CONTROLLER, "controller",                                  \
+        "Sends \"packet-in\" messages to the OpenFlow controller")
+
+/* Indexes for slow-path reasons.  Client code uses "enum slow_path_reason"
+ * values instead of these, these are just a way to construct those. */
+enum {
+#define SPR(ENUM, STRING, EXPLANATION) ENUM##_INDEX,
+    SLOW_PATH_REASONS
+#undef SPR
+};
+
+/* Reasons why a subfacet might not be fast-pathable.
+ *
+ * Each reason is a separate bit to allow reasons to be combined. */
 enum slow_path_reason {
-    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
+#define SPR(ENUM, STRING, EXPLANATION) ENUM = 1 << ENUM##_INDEX,
+    SLOW_PATH_REASONS
+#undef SPR
 };
 
+const char *slow_path_reason_to_explanation(enum slow_path_reason);
+
 #endif /* odp-util.h */
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index a5b6814..a38a33b 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -1566,7 +1566,7 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,
         special = process_special(ctx, &ctx->xin->flow, peer,
                                   ctx->xin->packet);
         if (special) {
-            ctx->xout->slow = special;
+            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);
@@ -1787,8 +1787,7 @@ execute_controller_action(struct xlate_ctx *ctx, int len,
     struct ofpbuf *packet;
     struct flow key;
 
-    ovs_assert(!ctx->xout->slow || ctx->xout->slow == SLOW_CONTROLLER);
-    ctx->xout->slow = SLOW_CONTROLLER;
+    ctx->xout->slow |= SLOW_CONTROLLER;
     if (!ctx->xin->packet) {
         return;
     }
@@ -2644,7 +2643,7 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout)
     in_port = get_ofp_port(ctx.xbridge, flow->in_port.ofp_port);
     special = process_special(&ctx, flow, in_port, ctx.xin->packet);
     if (special) {
-        ctx.xout->slow = special;
+        ctx.xout->slow |= special;
     } else {
         size_t sample_actions_len;
 
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 1c82318..01e6881 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -5474,27 +5474,19 @@ ofproto_trace(struct ofproto_dpif *ofproto, const struct flow *flow,
                            trace.xout.odp_actions.size);
 
         if (trace.xout.slow) {
+            enum slow_path_reason slow;
+
             ds_put_cstr(ds, "\nThis flow is handled by the userspace "
                         "slow path because it:");
-            switch (trace.xout.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();
+
+            slow = trace.xout.slow;
+            while (slow) {
+                enum slow_path_reason bit = rightmost_1bit(slow);
+
+                ds_put_format(ds, "\n\t- %s.",
+                              slow_path_reason_to_explanation(bit));
+
+                slow &= ~bit;
             }
         }
 
-- 
1.7.10.4




More information about the dev mailing list