[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