[ovs-dev] [mpls v2 1/2] Implement OpenFlow support for MPLS, for up to 3 labels.

[Simon Horman]

On Thu, Jan 02, 2014 at 11:51:15AM -0500, Jesse Gross wrote:
> On Sun, Dec 29, 2013 at 2:50 AM, Ben Pfaff <blp at nicira.com> wrote:
> > I've been a little frustrated with the current approach to MPLS, because it
> > seems quite difficult to understand.  One particularly difficult bit for
> > me is the variables used during translation, e.g. mpls_depth_delta and
> > pre_push_mpls_lse.  And what we end up with is support for a single MPLS
> > label, which I don't think is going to make any real-world users happy.
> >
> > This commit attempts to implement something easier to understand and more
> > powerful by just keeping track of all the labels in struct flow.
> >
> > Signed-off-by: Ben Pfaff <blp at nicira.com>
> > Co-authored-by: Simon Horman <horms at verge.net.au>
> > Signed-off-by: Simon Horman <horms at verge.net.au>
> 
> I have some general comments:
> 
> * I'm not sure that flow_count_mpls_labels() is megaflow-safe since it
> reads fields without updating the wildcards. I think the only real way
> to handle this is with automatic tracking.
> * It seems like we might want a harder failure mechanism for cases
> where we exceed the number of labels that we are able to handle. I
> don't think that this is the same as trying to change L4 ports when
> you don't have an L4 header because an MPLS packet could still be
> interpreted, just differently.

Are you specifically referring to pushing and popping MPLS labels when
there are more labels present than we can handle? If so, do you think it
would be appropriate to send a controller action and stop processing
actions for the current table, the behaviour that occurs when an action
tries to decrement the IP TTL when it is already one or less.

> Other things, assuming that the goal is to eventually hook this up to
> the kernel patches:
> 
> * How do we handle cases where userspace supports parsing more labels
> than the kernel (which is actually true at the moment)? I think that
> it would just fail at flow installation time right now.
> * Somewhat related to the above, the kernel's action validator doesn't
> trust userspace to provide a valid EtherType for pop actions, so it
> doesn't allow multiple consecutive pops in cases where there are more
> tags than it knows about (which is 1).

Prior to Ben publishing this patch my assumption was that such cases
would always be handled by recirculation. Which I believe implies
that the kernel would trust itself to extract the flow from a packet
after a pop, based on the ether type included in the pop action.

Furthermore, the same assumption was made to allow poping to
IP and then allowing IP actions to occur after recirculation.

Perhaps that assumption was invalid too.

I may be missing the point but I'm a little unsure how much can occur after
an mpls pop action if the kernel doesn't trust the ethertype supplied in
the action.