[ovs-dev] [PATCH] Remove mpls_depth field from flow

Ben Pfaff blp at nicira.com
Wed Sep 25 00:13:34 UTC 2013


On Wed, Sep 25, 2013 at 09:02:10AM +0900, Simon Horman wrote:
> On Tue, Sep 24, 2013 at 08:42:50AM -0700, Ben Pfaff wrote:
> > On Tue, Sep 24, 2013 at 04:51:35PM +0900, Simon Horman wrote:
> > > Rather than tracking the MPLS depth as a field in the
> > > flow, which is an entirely poor place for it, just track
> > > the delta to the MPLS depth during translation.
> > > 
> > > This logic was developed while implementing recirculation
> > > and intended to be used to detect when recirculation should
> > > occur. This variant of the patch uses the logic to determine
> > > if processing of actions should stop due to an MPLS
> > > action which cannot be translated (without recirculation).
> > > 
> > > A side-effect of this patch is that it resolves a bug
> > > whereby ovs-vswitchd will abort due to to an assertion
> > > on eth_type_mpls(ctx->xin->flow.dl_type) in compose_mpls_pop_action(()
> > > if the actions of a flow include pop_mpls twice without
> > > a push_mpls in between.
> > > 
> > > Signed-off-by: Simon Horman <horms at verge.net.au>
> > 
> > pre_push_mpls_lse is a local variable in do_xlate_actions().  That means
> > that a push/pop sequence can't span a resubmit or cross two tables with
> > goto_table.  I think that if we put pre_push_mpls_lse in the context,
> > then this restriction would not exist.  Any particular reason to make it
> > a local variable?
> 
> Thanks, I had not considered that.
> 
> The only reason was that I wanted to keep the variable local
> as there seemed no reason not to. Now you have pointed out a reason
> I will move it into the context.
> 
> > It took me a minute to see that eth_mpls_depth() can be removed since it
> > had no callers in the existing tree.  Also, it looks like this patch
> > does not remove its prototype in packets.h.  It would be a no-brainer to
> > write a standalone patch to remove it and its prototype.
> 
> Thanks, I will clean that up.
> 
> > s/loosing/losing/ in comments in a couple of places.  Also
> > s/futher/further/ in the manpage.
> 
> Spelling is not my strong point...
> 
> I'll address the issues above and repost.

Thanks!



More information about the dev mailing list