Regards _Sugesh
> -----Original Message----- > From: Joe Stringer [mailto:[email protected]] > Sent: Wednesday, July 19, 2017 2:34 AM > To: Chandran, Sugesh <[email protected]> > Cc: ovs dev <[email protected]>; Andy Zhou <[email protected]>; Zoltán > Balogh <[email protected]> > Subject: Re: [PATCH v3 3/3] tunneling: Avoid datapath-recirc by combining > recirc actions at xlate. > > 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. [Sugesh] That make sense. Perhaps we can add comment like /* * XXX : Only needed for the unit test cases. Few tests in 'make check' * doesn’t have any actions to handle encaped packets. */ Is that OK? _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
