From 98f0520fb205eb619a386f9689b35dec3487fedf Mon Sep 17 00:00:00 2001 From: Ethan Jackson Date: Tue, 28 May 2013 11:43:43 -0700 Subject: [PATCH] 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 --- lib/odp-util.c | 53 +++++++++++++++++++++++++++--------------- lib/odp-util.h | 12 +++++----- ofproto/ofproto-dpif.c | 51 ++++++++++++++++++---------------------- 3 files changed, 63 insertions(+), 53 deletions(-) diff --git a/lib/odp-util.c b/lib/odp-util.c index 4ff1f963b..1c48e7b7b 100644 --- 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 0892cf7fd..0455370d3 100644 --- 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 55d030f68..a74702bed 100644 --- 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(); } } } -- 2.43.0