[ovs-dev] [PATCH] xlate: Skip recirculation for output and set actions

[Simon Horman]

On Tue, May 24, 2016 at 12:36:02PM -0700, Jarno Rajahalme wrote:
> One comment below, otherwise looks good,
> 
> Acked-by: Jarno Rajahalme <jarno at ovn.org>

[...]

> > On May 24, 2016, at 12:29 AM, Simon Horman <simon.horman at netronome.com> wrote:
> > 
> > Until 8bf009bf8ab4 ("xlate: Always recirculate after an MPLS POP to a
> > non-MPLS ethertype.") the translation code took some care to only
> > recirculate as a result of a pop_mpls action if necessary. This was
> > implemented using per-action checks and resulted in some maintenance
> > burden.
> > 
> > Unfortunately recirculation is a relatively expensive operation and a
> > performance degradation of up to 35% has been observed with the above
> > mentioned patch applied for the arguably common case of:
> > 
> > 	pop_mpls,set(l2 field),output
> > 
> > This patch attempts to strike a balance between performance and
> > maintainability by special casing set and output actions such
> > that recirculation may be avoided.
> > 
> > This partially reverts the above mentioned commit. In particular most
> > of the C code outside of do_xlate_actions().
> > 
> > Signed-off-by: Simon Horman <simon.horman at netronome.com>

[...]

> > @@ -4500,6 +4612,8 @@ do_xlate_actions(const struct ofpact *ofpacts, size_t ofpacts_len,
> >             break;
> >         }
> > 
> > +        recirc_for_mpls(a, ctx);
> > +
> 
> IMO this should not be done when 'ctx->exit' has already been set, as it may trigger an unnecessary recirculation in that case.

Thanks, good point.

I will make recirc_for_mpls return early if ctx->exit is already set.