[ovs-dev] [PATCH v2.39 0/7] MPLS actions and matches

[Simon Horman]

On Fri, Sep 13, 2013 at 03:15:17PM -0700, Jesse Gross wrote:
> On Thu, Sep 12, 2013 at 3:54 PM, Ben Pfaff <blp at nicira.com> wrote:
> > On Fri, Sep 13, 2013 at 07:56:14AM +0900, Simon Horman wrote:
> >> On Thu, Sep 12, 2013 at 12:06:36PM -0700, Ben Pfaff wrote:
> >> > I've totally lost track of the status of this patch series.  I assume it
> >> > needs Jesse's review.  Jesse, if I'm wrong about that, let me know and
> >> > I'll take a pass at it.
> >>
> >> My understanding is that you have looked over the approach
> >> taken for the non-datapath code and were happy with it in
> >> the context that it needed review from Jesse along with the
> >> datapath code.
> >>
> >> I believe it was a few revisions ago that you looked over
> >> the series but I don't believe the non-datapath code has changed
> >> in a meaningful way since then.
> >
> > That sounds plausible, thanks for refreshing my memory.
> 
> I haven't really reviewed the userspace code but there is one thing in
> particular that concerns me: mpls_depth in the flow structure. We
> obviously can't be making flow-level decisions on information that the
> kernel doesn't include in the flow and I think that it is mostly
> vestigial at this point. However, at best the name seems misleading
> and at worst could result in someone trying to use information that we
> don't really have. Can we fix this somehow? Maybe using the BoS bit?

I believe we discussed this a long time ago.

The recirculation series includes a patch to allow multiple levels of pull
and push and that patch removes the mpls_depth field from the flow
structure. The approach taken is to track changes in the mpls depth during
translation, without knowing what the original depth was, and to
recirculate when the depth changes by more than one.  So in a nutshell my
strategy was to use recirculation to resolve this problem.

However, I do entirely agree with your concerns and I don't believe that we
need mpls_depth at all even at this stage.  I will take a look at removing
it by tracking the depth during translation without a dependency on
recirculation.