Acked-by: Jarno Rajahalme <jrajaha...@nicira.com>
> On Jul 29, 2015, at 11:42 PM, Ben Pfaff <b...@nicira.com> wrote:
>
> As I see it, this has two benefits. First, by using an initializer
> rather than a series of assignment statements, the reader can be
> assured that everything in the structure is actually initialized.
> Second, previously the initialization of 'ctx' was scattered in
> a few places in this function, which made it a little harder to be
> sure that any given member was not just initialized but actually
> initialized before the statement that one was looking at.
>
> It's also nice to get rid of the stub members in xlate_ctx, since
> nothing outside of xlate_actions() itself needs direct access to
> them. (This is pretty much necessary if we're going to use an
> initializer for struct xlate_ctx, because otherwise the compiler
> would initialize the whole stub, which is too expensive.)
>
> Signed-off-by: Ben Pfaff <b...@nicira.com>
> ---
> ofproto/ofproto-dpif-xlate.c | 77 ++++++++++++++++++++++----------------------
> 1 file changed, 39 insertions(+), 38 deletions(-)
>
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index b6d5d15..643bde0 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -175,7 +175,6 @@ struct xlate_ctx {
>
> /* Stack for the push and pop actions. Each stack element is of type
> * "union mf_subvalue". */
> - union mf_subvalue init_stack[1024 / sizeof(union mf_subvalue)];
> struct ofpbuf stack;
>
> /* The rule that we are currently translating, or NULL. */
> @@ -294,7 +293,6 @@ struct xlate_ctx {
> * datapath actions. */
> bool action_set_has_group; /* Action set contains OFPACT_GROUP? */
> struct ofpbuf action_set; /* Action set. */
> - uint64_t action_set_stub[1024 / 8];
> };
>
> static void xlate_action_set(struct xlate_ctx *ctx);
> @@ -4732,16 +4730,53 @@ xlate_actions(struct xlate_in *xin, struct xlate_out
> *xout)
> };
>
> struct xlate_cfg *xcfg = ovsrcu_get(struct xlate_cfg *, &xcfgp);
> + struct xbridge *xbridge = xbridge_lookup(xcfg, xin->ofproto);
> + if (!xbridge) {
> + return;
> + }
> +
> struct flow_wildcards *wc = NULL;
> struct flow *flow = &xin->flow;
> struct rule_dpif *rule = NULL;
>
> + union mf_subvalue stack_stub[1024 / sizeof(union mf_subvalue)];
> + uint64_t action_set_stub[1024 / 8];
> + struct xlate_ctx ctx = {
> + .xin = xin,
> + .xout = xout,
> + .base_flow = *flow,
> + .orig_tunnel_ip_dst = flow->tunnel.ip_dst,
> + .xbridge = xbridge,
> + .stack = OFPBUF_STUB_INITIALIZER(stack_stub),
> + .rule = xin->rule,
> +
> + .recurse = 0,
> + .resubmits = 0,
> + .in_group = false,
> + .in_action_set = false,
> +
> + .table_id = 0,
> + .rule_cookie = OVS_BE64_MAX,
> + .orig_skb_priority = flow->skb_priority,
> + .sflow_n_outputs = 0,
> + .sflow_odp_port = 0,
> + .user_cookie_offset = 0,
> + .exit = false,
> +
> + .recirc_action_offset = -1,
> + .last_unroll_offset = -1,
> +
> + .was_mpls = false,
> +
> + .action_set_has_group = false,
> + .action_set = OFPBUF_STUB_INITIALIZER(action_set_stub),
> + };
> + memset(&ctx.base_flow.tunnel, 0, sizeof ctx.base_flow.tunnel);
> +
> enum slow_path_reason special;
> const struct ofpact *ofpacts;
> - struct xbridge *xbridge;
> struct xport *in_port;
> struct flow orig_flow;
> - struct xlate_ctx ctx;
> size_t ofpacts_len;
> bool tnl_may_send;
> bool is_icmp;
> @@ -4769,9 +4804,6 @@ xlate_actions(struct xlate_in *xin, struct xlate_out
> *xout)
> * kernel does. If we wish to maintain the original values an action
> * needs to be generated. */
>
> - ctx.xin = xin;
> - ctx.xout = xout;
> -
> xout->odp_actions = xin->odp_actions;
> if (!xout->odp_actions) {
> xout->odp_actions = &xout->odp_actions_buf;
> @@ -4780,19 +4812,6 @@ xlate_actions(struct xlate_in *xin, struct xlate_out
> *xout)
> }
> ofpbuf_reserve(xout->odp_actions, NL_A_U32_SIZE);
>
> - xbridge = xbridge_lookup(xcfg, xin->ofproto);
> - if (!xbridge) {
> - return;
> - }
> - /* 'ctx.xbridge' may be changed by action processing, whereas 'xbridge'
> - * will remain set on the original input bridge. */
> - ctx.xbridge = xbridge;
> - ctx.rule = xin->rule;
> -
> - ctx.base_flow = *flow;
> - memset(&ctx.base_flow.tunnel, 0, sizeof ctx.base_flow.tunnel);
> - ctx.orig_tunnel_ip_dst = flow->tunnel.ip_dst;
> -
> if (!xin->skip_wildcards) {
> wc = &xout->wc;
> flow_wildcards_init_catchall(wc);
> @@ -4814,24 +4833,6 @@ xlate_actions(struct xlate_in *xin, struct xlate_out
> *xout)
>
> tnl_may_send = tnl_xlate_init(flow, wc);
>
> - ctx.recurse = 0;
> - ctx.resubmits = 0;
> - ctx.in_group = false;
> - ctx.in_action_set = false;
> - ctx.orig_skb_priority = flow->skb_priority;
> - ctx.table_id = 0;
> - ctx.rule_cookie = OVS_BE64_MAX;
> - ctx.exit = false;
> - ctx.was_mpls = false;
> - ctx.recirc_action_offset = -1;
> - ctx.last_unroll_offset = -1;
> -
> - ctx.action_set_has_group = false;
> - ofpbuf_use_stub(&ctx.action_set,
> - ctx.action_set_stub, sizeof ctx.action_set_stub);
> -
> - ofpbuf_use_stub(&ctx.stack, ctx.init_stack, sizeof ctx.init_stack);
> -
> /* The in_port of the original packet before recirculation. */
> in_port = get_ofp_port(xbridge, flow->in_port.ofp_port);
>
> --
> 2.1.3
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
