On 09/12/2017 12:49 PM, Andy Zhou wrote:
To patch_port_output(). The original function name does not make
much sense.
I think the commit message is a little mangled. Perhaps like this:
Rename apply_nested_clone_actions() to patch_port_output(). The
original function name does not make much sense.
Having the commit title extend into the commit message can be
confusing.
Other than that nitpick the rest looks good.
Signed-off-by: Andy Zhou <[email protected]>
---
ofproto/ofproto-dpif-xlate.c | 33 +++++++++++++++++----------------
1 file changed, 17 insertions(+), 16 deletions(-)
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 81853c29afbf..94aa071a37cd 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -432,8 +432,8 @@ static void xlate_action_set(struct xlate_ctx *ctx);
static void xlate_commit_actions(struct xlate_ctx *ctx);
static void
-apply_nested_clone_actions(struct xlate_ctx *ctx, const struct xport *in_dev,
- struct xport *out_dev);
+patch_port_output(struct xlate_ctx *ctx, const struct xport *in_dev,
+ struct xport *out_dev);
static void
ctx_trigger_freeze(struct xlate_ctx *ctx)
@@ -3317,7 +3317,7 @@ validate_and_combine_post_tnl_actions(struct xlate_ctx
*ctx,
entry->tunnel_hdr.hdr_size = tnl_push_data.header_len;
entry->tunnel_hdr.operation = ADD;
- apply_nested_clone_actions(ctx, xport, out_dev);
+ patch_port_output(ctx, xport, out_dev);
nested_act_flag = is_tunnel_actions_clone_ready(ctx);
if (nested_act_flag) {
@@ -3509,21 +3509,22 @@ xlate_flow_is_protected(const struct xlate_ctx *ctx,
const struct flow *flow, co
xport_in->xbundle->protected && xport_out->xbundle->protected);
}
-/* Function to combine actions from following device/port with the current
- * device actions in openflow pipeline. Mainly used for the translation of
- * patch/tunnel port output actions. It pushes the openflow state into a stack
- * first, clear out to execute the packet through the device and finally pop
- * the openflow state back from the stack. This is equivalent to cloning
- * a packet in translation for the duration of execution.
+/* Function handles when a packet is sent from one bridge to another bridge.
*
- * On output to a patch port, the output action will be replaced with set of
- * nested actions on the peer patch port.
- * Similarly on output to a tunnel port, the post nested actions on
- * tunnel are chained up with the tunnel-push action.
+ * The bridges are internally connected, either with patch ports or with
+ * tunnel ports.
+ *
+ * The output action to another bridge causes translation to continue within
+ * the next bridge. This process can be recursive; the next bridge can
+ * output yet to another bridge.
+ *
+ * The translated actions from the second bridge onwards are enclosed within
+ * the clone action, so that any modification to the packet will not be visible
+ * to the remaining actions of the originating bridge.
I like the improvement in the comments here. Thanks!
*/
static void
-apply_nested_clone_actions(struct xlate_ctx *ctx, const struct xport *in_dev,
- struct xport *out_dev)
+patch_port_output(struct xlate_ctx *ctx, const struct xport *in_dev,
+ struct xport *out_dev)
{
struct flow *flow = &ctx->xin->flow;
struct flow old_flow = ctx->xin->flow;
@@ -3760,7 +3761,7 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t
ofp_port,
}
if (xport->peer) {
- apply_nested_clone_actions(ctx, xport, xport->peer);
+ patch_port_output(ctx, xport, xport->peer);
return;
}
Tested-by: Greg Rose <[email protected]>
Reviewed-by: Greg Rose <[email protected]>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
