On Thu, Jan 5, 2017 at 4:28 PM, Ben Pfaff <b...@ovn.org> wrote: > This is a design decision but it seems conceptually cleaner than having > them leak through into the clone. >
I agree that saving and restoring is a good thing. Whether to clear, as Jarno brought up, that is a good question. I guess it would be more flexible to allow a clone to pop things off the stack, at its own risk? We cannot really predict all of the places where clone might be used in the future. In most cases the cloned packet will not pop off more than it pushes, so I don't think leaving the stack there can do much damage. > > Reported-by: Mickey Spiegel <mickeys....@gmail.com> > Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2017- > January/326981.html > Fixes: 7ae62a676d3a ("ofp-actions: Add clone action.") > Signed-off-by: Ben Pfaff <b...@ovn.org> > Acked-by: Mickey Spiegel <mickeys....@gmail.com> > --- > ofproto/ofproto-dpif-xlate.c | 14 ++++++++++++++ > utilities/ovs-ofctl.8.in | 3 ++- > 2 files changed, 16 insertions(+), 1 deletion(-) > > diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c > index b02e317..6736c12 100644 > --- a/ofproto/ofproto-dpif-xlate.c > +++ b/ofproto/ofproto-dpif-xlate.c > @@ -4334,8 +4334,22 @@ compose_clone_action(struct xlate_ctx *ctx, const > struct ofpact_nest *oc) > bool old_conntracked = ctx->conntracked; > struct flow old_flow = ctx->xin->flow; > > + struct ofpbuf old_stack = ctx->stack; > + union mf_subvalue new_stack[1024 / sizeof(union mf_subvalue)]; > + ofpbuf_use_stub(&ctx->stack, new_stack, sizeof new_stack); > + > + struct ofpbuf old_action_set = ctx->action_set; > + uint64_t actset_stub[1024 / 8]; > + ofpbuf_use_stub(&ctx->action_set, actset_stub, sizeof actset_stub); > + > do_xlate_actions(oc->actions, ofpact_nest_get_action_len(oc), ctx); > > + ofpbuf_uninit(&ctx->action_set); > + ctx->action_set = old_action_set; > + > + ofpbuf_uninit(&ctx->stack); > + ctx->stack = old_stack; > + > ctx->xin->flow = old_flow; > > /* The clone's conntrack execution should have no effect on the > original > diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in > index 72aa89a..2f9606b 100644 > --- a/utilities/ovs-ofctl.8.in > +++ b/utilities/ovs-ofctl.8.in > @@ -2628,7 +2628,8 @@ in Open vSwitch 2.4. > Executes each nested \fIaction\fR, saving much of the packet and > pipeline state beforehand and then restoring it afterward. The state > that is saved and restored includes all flow data and metadata > -(including, for example, \fBct_state\fR). > +(including, for example, \fBct_state\fR), the stack accessed by > +\fBpush\fR and \fBpop\fR actions, and the OpenFlow action set. > .IP > This action was added in Open vSwitch 2.6.90. > .RE > -- > 2.10.2 > > _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
