This code was a twisty maze of tiny functions, but what it actually needed to do was simple. This makes it look that simple.
Among more stylistic changes, this removes 'user_cookie_offset' from xlate_ctx. This member was used to communicate between two sections of code that are both in xlate_actions() and close together, so it's better to simply use a local variable than to put it into a shared context structure. Signed-off-by: Ben Pfaff <b...@nicira.com> --- ofproto/ofproto-dpif-xlate.c | 255 ++++++++++++++++--------------------------- 1 file changed, 97 insertions(+), 158 deletions(-) diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index 03bca1b..7726a40 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -205,7 +205,6 @@ struct xlate_ctx { uint32_t orig_skb_priority; /* Priority when packet arrived. */ uint32_t sflow_n_outputs; /* Number of output ports. */ odp_port_t sflow_odp_port; /* Output port for composing sFlow action. */ - uint16_t user_cookie_offset;/* Used for user_action_cookie fixup. */ bool exit; /* No further actions should be processed. */ /* These are used for non-bond recirculation. The recirculation IDs are @@ -2440,211 +2439,147 @@ xlate_normal(struct xlate_ctx *ctx) } } -/* Compose SAMPLE action for sFlow or IPFIX. The given probability is - * the number of packets out of UINT32_MAX to sample. The given - * cookie is passed back in the callback for each sampled packet. +/* Appends a "sample" action for sFlow or IPFIX to 'ctx->odp_actions'. The + * 'probability' is the number of packets out of UINT32_MAX to sample. The + * 'cookie' (of length 'cookie_size' bytes) is passed back in the callback for + * each sampled packet. 'tunnel_out_port', if not ODPP_NONE, is added as the + * OVS_USERSPACE_ATTR_EGRESS_TUN_PORT attribute. If 'include_actions', an + * OVS_USERSPACE_ATTR_ACTIONS attribute is added. */ static size_t -compose_sample_action(const struct xbridge *xbridge, - struct ofpbuf *odp_actions, - const struct flow *flow, +compose_sample_action(struct xlate_ctx *ctx, const uint32_t probability, const union user_action_cookie *cookie, const size_t cookie_size, const odp_port_t tunnel_out_port, bool include_actions) { - size_t sample_offset, actions_offset; - odp_port_t odp_port; - int cookie_offset; - uint32_t pid; + size_t sample_offset = nl_msg_start_nested(ctx->odp_actions, + OVS_ACTION_ATTR_SAMPLE); - sample_offset = nl_msg_start_nested(odp_actions, OVS_ACTION_ATTR_SAMPLE); + nl_msg_put_u32(ctx->odp_actions, OVS_SAMPLE_ATTR_PROBABILITY, probability); - nl_msg_put_u32(odp_actions, OVS_SAMPLE_ATTR_PROBABILITY, probability); + size_t actions_offset = nl_msg_start_nested(ctx->odp_actions, + OVS_SAMPLE_ATTR_ACTIONS); - actions_offset = nl_msg_start_nested(odp_actions, OVS_SAMPLE_ATTR_ACTIONS); + odp_port_t odp_port = ofp_port_to_odp_port( + ctx->xbridge, ctx->xin->flow.in_port.ofp_port); + uint32_t pid = dpif_port_get_pid(ctx->xbridge->dpif, odp_port, + flow_hash_5tuple(&ctx->xin->flow, 0)); + int cookie_offset = odp_put_userspace_action(pid, cookie, cookie_size, + tunnel_out_port, + include_actions, + ctx->odp_actions); - odp_port = ofp_port_to_odp_port(xbridge, flow->in_port.ofp_port); - pid = dpif_port_get_pid(xbridge->dpif, odp_port, - flow_hash_5tuple(flow, 0)); - cookie_offset = odp_put_userspace_action(pid, cookie, cookie_size, - tunnel_out_port, - include_actions, - odp_actions); + nl_msg_end_nested(ctx->odp_actions, actions_offset); + nl_msg_end_nested(ctx->odp_actions, sample_offset); - nl_msg_end_nested(odp_actions, actions_offset); - nl_msg_end_nested(odp_actions, sample_offset); return cookie_offset; } -static void -compose_sflow_cookie(const struct xbridge *xbridge, ovs_be16 vlan_tci, - odp_port_t odp_port, unsigned int n_outputs, - union user_action_cookie *cookie) -{ - int ifindex; - - cookie->type = USER_ACTION_COOKIE_SFLOW; - cookie->sflow.vlan_tci = vlan_tci; - - /* See http://www.sflow.org/sflow_version_5.txt (search for "Input/output - * port information") for the interpretation of cookie->output. */ - switch (n_outputs) { - case 0: - /* 0x40000000 | 256 means "packet dropped for unknown reason". */ - cookie->sflow.output = 0x40000000 | 256; - break; - - case 1: - ifindex = dpif_sflow_odp_port_to_ifindex(xbridge->sflow, odp_port); - if (ifindex) { - cookie->sflow.output = ifindex; - break; - } - /* Fall through. */ - default: - /* 0x80000000 means "multiple output ports. */ - cookie->sflow.output = 0x80000000 | n_outputs; - break; - } -} - -/* Compose SAMPLE action for sFlow bridge sampling. */ +/* If sFLow is not enabled, returns 0 without doing anything. + * + * If sFlow is enabled, appends a template "sample" action to the ODP actions + * in 'ctx'. This action is a template because some of the information needed + * to fill it out is not available until flow translation is complete. In this + * case, this functions returns an offset, which is always nonzero, to pass + * later to fix_sflow_action() to fill in the rest of the template. */ static size_t -compose_sflow_action(const struct xbridge *xbridge, - struct ofpbuf *odp_actions, - const struct flow *flow, - odp_port_t odp_port) +compose_sflow_action(struct xlate_ctx *ctx) { - uint32_t probability; - union user_action_cookie cookie; - - if (!xbridge->sflow || flow->in_port.ofp_port == OFPP_NONE) { + struct dpif_sflow *sflow = ctx->xbridge->sflow; + if (!sflow || ctx->xin->flow.in_port.ofp_port == OFPP_NONE) { return 0; } - probability = dpif_sflow_get_probability(xbridge->sflow); - compose_sflow_cookie(xbridge, htons(0), odp_port, - odp_port == ODPP_NONE ? 0 : 1, &cookie); - - return compose_sample_action(xbridge, odp_actions, flow, probability, + union user_action_cookie cookie = { .type = USER_ACTION_COOKIE_SFLOW }; + return compose_sample_action(ctx, dpif_sflow_get_probability(sflow), &cookie, sizeof cookie.sflow, ODPP_NONE, true); } +/* If IPFIX is enabled, this appends a "sample" action to implement IPFIX to + * 'ctx->odp_actions'. */ static void -compose_flow_sample_cookie(uint16_t probability, uint32_t collector_set_id, - uint32_t obs_domain_id, uint32_t obs_point_id, - union user_action_cookie *cookie) -{ - cookie->type = USER_ACTION_COOKIE_FLOW_SAMPLE; - cookie->flow_sample.probability = probability; - cookie->flow_sample.collector_set_id = collector_set_id; - cookie->flow_sample.obs_domain_id = obs_domain_id; - cookie->flow_sample.obs_point_id = obs_point_id; -} - -static void -compose_ipfix_cookie(union user_action_cookie *cookie, - odp_port_t output_odp_port) -{ - cookie->type = USER_ACTION_COOKIE_IPFIX; - cookie->ipfix.output_odp_port = output_odp_port; -} - -/* Compose SAMPLE action for IPFIX bridge sampling. */ -static void -compose_ipfix_action(const struct xbridge *xbridge, - struct ofpbuf *odp_actions, - const struct flow *flow, - odp_port_t output_odp_port) +compose_ipfix_action(struct xlate_ctx *ctx, odp_port_t output_odp_port) { - uint32_t probability; - union user_action_cookie cookie; + struct dpif_ipfix *ipfix = ctx->xbridge->ipfix; odp_port_t tunnel_out_port = ODPP_NONE; - if (!xbridge->ipfix || flow->in_port.ofp_port == OFPP_NONE) { + if (!ipfix || ctx->xin->flow.in_port.ofp_port == OFPP_NONE) { return; } /* For input case, output_odp_port is ODPP_NONE, which is an invalid port * number. */ if (output_odp_port == ODPP_NONE && - !dpif_ipfix_get_bridge_exporter_input_sampling(xbridge->ipfix)) { + !dpif_ipfix_get_bridge_exporter_input_sampling(ipfix)) { return; } /* For output case, output_odp_port is valid*/ if (output_odp_port != ODPP_NONE) { - if (!dpif_ipfix_get_bridge_exporter_output_sampling(xbridge->ipfix)) { + if (!dpif_ipfix_get_bridge_exporter_output_sampling(ipfix)) { return; } /* If tunnel sampling is enabled, put an additional option attribute: * OVS_USERSPACE_ATTR_TUNNEL_OUT_PORT */ - if (dpif_ipfix_get_bridge_exporter_tunnel_sampling(xbridge->ipfix) && - dpif_ipfix_get_tunnel_port(xbridge->ipfix, output_odp_port) ) { + if (dpif_ipfix_get_bridge_exporter_tunnel_sampling(ipfix) && + dpif_ipfix_get_tunnel_port(ipfix, output_odp_port) ) { tunnel_out_port = output_odp_port; } } - probability = dpif_ipfix_get_bridge_exporter_probability(xbridge->ipfix); - compose_ipfix_cookie(&cookie, output_odp_port); - - compose_sample_action(xbridge, odp_actions, flow, probability, + union user_action_cookie cookie = { + .ipfix = { + .type = USER_ACTION_COOKIE_IPFIX, + .output_odp_port = output_odp_port, + } + }; + compose_sample_action(ctx, + dpif_ipfix_get_bridge_exporter_probability(ipfix), &cookie, sizeof cookie.ipfix, tunnel_out_port, false); } -/* SAMPLE action for sFlow must be first action in any given list of - * actions. At this point we do not have all information required to - * build it. So try to build sample action as complete as possible. */ -static void -add_sflow_action(struct xlate_ctx *ctx) -{ - ctx->user_cookie_offset = compose_sflow_action(ctx->xbridge, - ctx->odp_actions, - &ctx->xin->flow, ODPP_NONE); - ctx->sflow_odp_port = 0; - ctx->sflow_n_outputs = 0; -} - -/* SAMPLE action for IPFIX must be 1st or 2nd action in any given list - * of actions, eventually after the SAMPLE action for sFlow. */ -static void -add_ipfix_action(struct xlate_ctx *ctx) -{ - compose_ipfix_action(ctx->xbridge, ctx->odp_actions, - &ctx->xin->flow, ODPP_NONE); -} - -static void -add_ipfix_output_action(struct xlate_ctx *ctx, odp_port_t port) -{ - compose_ipfix_action(ctx->xbridge, ctx->odp_actions, - &ctx->xin->flow, port); -} - -/* Fix SAMPLE action according to data collected while composing ODP actions. - * We need to fix SAMPLE actions OVS_SAMPLE_ATTR_ACTIONS attribute, i.e. nested - * USERSPACE action's user-cookie which is required for sflow. */ +/* Fix "sample" action according to data collected while composing ODP actions, + * as described in compose_sflow_action(). + * + * 'user_cookie_offset' must be the offset returned by add_sflow_action(). */ static void -fix_sflow_action(struct xlate_ctx *ctx) +fix_sflow_action(struct xlate_ctx *ctx, unsigned int user_cookie_offset) { const struct flow *base = &ctx->base_flow; union user_action_cookie *cookie; - if (!ctx->user_cookie_offset) { - return; - } - - cookie = ofpbuf_at(ctx->odp_actions, ctx->user_cookie_offset, + cookie = ofpbuf_at(ctx->odp_actions, user_cookie_offset, sizeof cookie->sflow); ovs_assert(cookie->type == USER_ACTION_COOKIE_SFLOW); - compose_sflow_cookie(ctx->xbridge, base->vlan_tci, - ctx->sflow_odp_port, ctx->sflow_n_outputs, cookie); + cookie->type = USER_ACTION_COOKIE_SFLOW; + cookie->sflow.vlan_tci = base->vlan_tci; + + /* See http://www.sflow.org/sflow_version_5.txt (search for "Input/output + * port information") for the interpretation of cookie->output. */ + switch (ctx->sflow_n_outputs) { + case 0: + /* 0x40000000 | 256 means "packet dropped for unknown reason". */ + cookie->sflow.output = 0x40000000 | 256; + break; + + case 1: + cookie->sflow.output = dpif_sflow_odp_port_to_ifindex( + ctx->xbridge->sflow, ctx->sflow_odp_port); + if (cookie->sflow.output) { + break; + } + /* Fall through. */ + default: + /* 0x80000000 means "multiple output ports. */ + cookie->sflow.output = 0x80000000 | ctx->sflow_n_outputs; + break; + } } static bool @@ -3096,7 +3031,7 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port, } else { /* Tunnel push-pop action is not compatible with * IPFIX action. */ - add_ipfix_output_action(ctx, out_port); + compose_ipfix_action(ctx, out_port); nl_msg_put_odp_port(ctx->odp_actions, OVS_ACTION_ATTR_OUTPUT, out_port); @@ -3955,7 +3890,6 @@ static void xlate_sample_action(struct xlate_ctx *ctx, const struct ofpact_sample *os) { - union user_action_cookie cookie; /* Scale the probability from 16-bit to 32-bit while representing * the same percentage. */ uint32_t probability = (os->probability << 16) | os->probability; @@ -3975,12 +3909,17 @@ xlate_sample_action(struct xlate_ctx *ctx, ctx->odp_actions, ctx->wc, use_masked); - compose_flow_sample_cookie(os->probability, os->collector_set_id, - os->obs_domain_id, os->obs_point_id, &cookie); - compose_sample_action(ctx->xbridge, ctx->odp_actions, - &ctx->xin->flow, probability, &cookie, - sizeof cookie.flow_sample, ODPP_NONE, - false); + union user_action_cookie cookie = { + .flow_sample = { + .type = USER_ACTION_COOKIE_FLOW_SAMPLE, + .probability = os->probability, + .collector_set_id = os->collector_set_id, + .obs_domain_id = os->obs_domain_id, + .obs_point_id = os->obs_point_id, + } + }; + compose_sample_action(ctx, probability, &cookie, sizeof cookie.flow_sample, + ODPP_NONE, false); } static bool @@ -4821,7 +4760,6 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout) .orig_skb_priority = flow->skb_priority, .sflow_n_outputs = 0, .sflow_odp_port = 0, - .user_cookie_offset = 0, .exit = false, .recirc_action_offset = -1, @@ -4996,9 +4934,10 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout) } /* Sampling is done only for packets really received by the bridge. */ + unsigned int user_cookie_offset = 0; if (!xin->recirc) { - add_sflow_action(&ctx); - add_ipfix_action(&ctx); + user_cookie_offset = compose_sflow_action(&ctx); + compose_ipfix_action(&ctx, ODPP_NONE); } size_t sample_actions_len = ctx.odp_actions->size; @@ -5056,8 +4995,8 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout) compose_output_action(&ctx, OFPP_LOCAL, NULL); } - if (!xin->recirc) { - fix_sflow_action(&ctx); + if (user_cookie_offset) { + fix_sflow_action(&ctx, user_cookie_offset); } /* Only mirror fully processed packets. */ if (!exit_recirculates(&ctx) -- 2.1.3 _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev