On Fri, Jan 29, 2016 at 05:26:47PM -0800, Jarno Rajahalme wrote: > > > On Jan 29, 2016, at 5:09 PM, Ben Pfaff <b...@ovn.org> wrote: > > > > On Fri, Jan 29, 2016 at 03:38:12PM -0800, Jarno Rajahalme wrote: > >> Handle implicit recirculation explicitly for each action type, so that > >> it is easier to follow what is happening. > >> > >> Signed-off-by: Jarno Rajahalme <ja...@ovn.org> > > > > I think this would benefit from passing 'end' instead of 'rem_len' to > > xlate_implicit_recirculation(). I think the code would be clearer. > > Another argument is microoptimization: the compiler could in theory > > factor calculating 'rem_len' out into xlate_implicit_recirculation() > > anyway, but that would take a fair amount of insight, and it might even > > conclude that it is commonly needed and always calculate it because it > > is used so much. > > > > I think this is the third way we've dealt with deciding when MPLS needs > > recirculation. The first way was, if I recall correctly, an MPLS > > specific switch function in the spirit of recirc_unroll_actions(). The > > second way is the current one. But both of them make me unhappy. > > Really, I find myself wondering whether this optimization is worth it. > > How common do you think it is that a flow table pops off the last MPLS > > label and then directly outputs to a port, without any resubmit or > > goto_table intervening? That's basically all you can do. So I'm > > inclined to drop this optimization and make compose_mpls_pop_action() > > always recirculate if it pops off the last MPLS label. It will be > > a code cleanup and totally more maintainable. > > > > What do you think? > > I think you are right. I did not stop to think about the actual cases > that benefit from postponing the recirculation. I’ll propose a patch > to always recirculate after pop when the packet type changes (== > ‘was_mpls’).
Sweet. Thank you. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
