From 97e42c92a9b7d4642f0df49d217c3c2f782175f9 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Mon, 24 Oct 2011 09:58:35 -0700 Subject: [PATCH] ofproto-dpif: Fix uninitialized struct member in xlate_actions(). Commit 7257b535ab "Implement new fragment handling policy." moved around a bunch of initialization code in xlate_actions() so that the assignment to ctx->flow.tp_src and .tp_dst would not have to also assign to ctx->base_flow.tp_src and .tp_dst. However, this meant that the early-exit "return" in the new switch statement exited without initializing a lot of the context. In particular 'may_set_up_flow' didn't get initialized, so something the early-exit would produce a flow that couldn't be installed, which wasn't the intent. It seems that this optimization was a bad tradeoff, so this commit puts all of the initialization up front and just assigns to both copies of the tp_src and tp_dst members. Fixes a nondeterministic "make check" failure in the VLAN handling test. --- ofproto/ofproto-dpif.c | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index de600f0a..b53452d1 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -4289,13 +4289,24 @@ xlate_actions(struct action_xlate_ctx *ctx, ctx->odp_actions = ofpbuf_new(512); ofpbuf_reserve(ctx->odp_actions, NL_A_U32_SIZE); + ctx->tags = 0; + ctx->may_set_up_flow = true; + ctx->has_learn = false; + ctx->has_normal = false; + ctx->nf_output_iface = NF_OUT_DROP; + ctx->recurse = 0; + ctx->priority = 0; + ctx->base_priority = 0; + ctx->base_flow = ctx->flow; + ctx->base_flow.tun_id = 0; + ctx->table_id = 0; if (ctx->flow.tos_frag & FLOW_FRAG_ANY) { switch (ctx->ofproto->up.frag_handling) { case OFPC_FRAG_NORMAL: /* We must pretend that transport ports are unavailable. */ - ctx->flow.tp_src = htons(0); - ctx->flow.tp_dst = htons(0); + ctx->flow.tp_src = ctx->base_flow.tp_src = htons(0); + ctx->flow.tp_dst = ctx->base_flow.tp_dst = htons(0); break; case OFPC_FRAG_DROP: @@ -4310,18 +4321,6 @@ xlate_actions(struct action_xlate_ctx *ctx, } } - ctx->tags = 0; - ctx->may_set_up_flow = true; - ctx->has_learn = false; - ctx->has_normal = false; - ctx->nf_output_iface = NF_OUT_DROP; - ctx->recurse = 0; - ctx->priority = 0; - ctx->base_priority = 0; - ctx->base_flow = ctx->flow; - ctx->base_flow.tun_id = 0; - ctx->table_id = 0; - if (process_special(ctx->ofproto, &ctx->flow, ctx->packet)) { ctx->may_set_up_flow = false; return ctx->odp_actions; -- 2.30.2