On Mon, May 23, 2016 at 08:56:46PM -0700, Ben Pfaff wrote:
> On Tue, May 24, 2016 at 12:43:59PM +0900, Simon Horman wrote:
> > Hi Ben,
> > 
> > On Tue, Apr 12, 2016 at 12:39:51PM -0700, Ben Pfaff wrote:
> > > On Mon, Apr 04, 2016 at 05:16:35PM +0900, Simon Horman 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 changes outside of do_xlate_actions().
> > > > 
> > > > Signed-off-by: Simon Horman <simon.hor...@netronome.com>
> > > > ---
> > > > * Lightly tested using test-suite portion of this patch
> > > 
> > > I think that recirculation is necessary for output to patch ports.
> > 
> > I believe this is already handled in my patch as recirculation
> > is triggered in xlate_table_action().
> > 
> > As part of the incremental patch below I have included a test
> > to exercise this.
> > 
> > > I think that recirculation is necessary for output to a group that
> > > chooses a bucket based on L3+ fields, even if the actions in the group
> > > do not otherwise require recirculation.
> > 
> > Thanks, I missed that one. I think that the best thing to do at
> > this time is to trigger recirculation for select groups - other groups
> > don't access fields as you describe and should already be handled correctly.
> > 
> > The incremental patch below updates the code to do this as
> > well as providing a test to exercise this case.
> 
> Thanks.  Will you submit the squashed patch, then?

Yes of course, will do.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to