On 18 July 2017 at 17:40, Joe Stringer <[email protected]> wrote: > On 18 July 2017 at 10:49, Sugesh Chandran <[email protected]> wrote: >> +static bool >> +validate_and_combine_post_tnl_actions(struct xlate_ctx *ctx, >> + const struct xport *xport, >> + struct xport *out_dev, >> + struct ovs_action_push_tnl >> tnl_push_data) >> +{ > ... >> + if (ctx->odp_actions->size > push_action_size) { >> + /* Update the CLONE action only when combined. */ >> + nl_msg_end_nested(ctx->odp_actions, clone_ofs); >> + } else { >> + /* No actions after the tunnel, no need of clone. */ >> + nl_msg_cancel_nested(ctx->odp_actions, clone_ofs); >> + odp_put_tnl_push_action(ctx->odp_actions, &tnl_push_data); > > I looked at this line again for this version since I didn't understand > it last time around, and I'm still a bit confused. If there's no > actions to run on the second bridge, then the copy of the packet which > is tunneled is effectively dropped. If that copy of the packet is > dropped, then why do we need the tunnel action at all? If I follow > correctly, then this means that if you have two bridges, where the > first bridge has output(tunnel),output(other_device) then the second > bridge where the tunneling occurs has no flows, then the datapath flow > will end up as something like: > > push_tnl(...),output(other_device). > > I realise that the testsuite breaks if you remove this line, but maybe > the testsuite needs fixing for these cases?
On second thought, this should probably be a logically separate follow-up patch even if we agree to change it. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
