> 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’). Jarno _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
