On Wed, Nov 20, 2019 at 12:46:55PM -0800, Han Zhou wrote: > On Wed, Nov 20, 2019 at 11:53 AM Ben Pfaff <[email protected]> wrote: > > > > This should be less confusing now. > > > > Reported-by: Han Zhou <[email protected]> > > Signed-off-by: Ben Pfaff <[email protected]> > > --- > > lib/ovs-actions.xml | 31 ++++++++++++++++++++++++------- > > ofproto/ofproto-dpif-xlate.c | 8 ++++---- > > 2 files changed, 28 insertions(+), 11 deletions(-) > > > > diff --git a/lib/ovs-actions.xml b/lib/ovs-actions.xml > > index e52cd849e37b..ab8e08b84d8b 100644 > > --- a/lib/ovs-actions.xml > > +++ b/lib/ovs-actions.xml > > @@ -933,15 +933,32 @@ for <var>i</var> in [1,<var>n_slaves</var>]: > > based on which bucket was selected. An obvious design would be > for the > > bucket to communicate the value via <code>set_field</code> on a > > register. This does not work because registers are part of the > > - metadata that <code>group</code> saves and restores. A design > that > > - would work would be for the bucket to recursively invoke the > rest of > > - the pipeline with <code>resubmit</code> rather than to attempt to > > - return it. Another possibility is for the bucket to use > > - <code>push</code> to put the value on the stack for the caller to > > - <code>pop</code> off, since <code>group</code> preserves only > packet > > - data and metadata, not the stack. > > + metadata that <code>group</code> saves and restores. The > following > > + alternative bucket designs do work: > > </p> > > > > + <ul> > > + <li> > > + Recursively invoke the rest of the pipeline with > > + <code>resubmit</code>. > > + </li> > > + > > + <li> > > + <p> > > + Use <code>resubmit</code> into a table that uses > <code>push</code> > > + to put the value on the stack for the caller to > <code>pop</code> > > + off. This works because <code>group</code> preserves only > packet > > + data and metadata, not the stack. > > + </p> > > + > > + <p> > > + (This design requires indirection through > <code>resubmit</code> > > + because actions sets may not contain <code>push</code> or > > + <code>pop</code> actions.) > > + </p> > > + </li> > > + </ul> > > + > > <p> > > An <code>exit</code> action within a group bucket terminates only > > execution of that bucket, not other buckets or the overall > pipeline. > > diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c > > index f92cb62c80ce..cebae7a5b2b3 100644 > > --- a/ofproto/ofproto-dpif-xlate.c > > +++ b/ofproto/ofproto-dpif-xlate.c > > @@ -4468,11 +4468,11 @@ xlate_group_bucket(struct xlate_ctx *ctx, struct > ofputil_bucket *bucket, > > * > > * Note that group buckets are action sets, hence they cannot modify > the > > * main action set. Also any stack actions are ignored when > executing an > > - * action set, so group buckets cannot change the stack either. > > + * action set, so group buckets cannot directly change the stack > either. > > * However, we do allow resubmit actions in group buckets, which > could > > - * break the above assumptions. It is up to the controller to not > mess up > > - * with the action_set and stack in the tables resubmitted to from > > - * group buckets. */ > > + * recursively execute actions that do modify the action set or > change the > > + * stack. The controller must be careful about what it does to the > > + * action_set and stack in the tables resubmitted to from group > buckets. */ > > ctx->xin->flow = old_flow; > > > > /* The group bucket popping MPLS should have no effect after bucket > > -- > > 2.21.0 > > > > Thanks Ben. > Acked-by: Han Zhou <[email protected]>
Thanks, applied to master. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
