This should be less confusing now.
Reported-by: Han Zhou <hz...@ovn.org>
Signed-off-by: Ben Pfaff <b...@ovn.org>
---
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
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
