[ovs-dev] [PATCH v2.0] Non-Datapath MPLS actions and matches

[Jesse Gross]

On Wed, Oct 3, 2012 at 5:40 PM, Simon Horman <horms at verge.net.au> wrote:
> On Wed, Oct 03, 2012 at 12:15:29PM -0700, Jesse Gross wrote:
>> On Tue, Oct 2, 2012 at 10:30 PM, Simon Horman <horms at verge.net.au> wrote:
>> > On Wed, Oct 03, 2012 at 12:19:09PM +0900, Simon Horman wrote:
>> >> On Fri, Sep 28, 2012 at 04:20:51PM -0700, Jesse Gross wrote:
>> >> > On Thu, Sep 27, 2012 at 5:44 PM, Simon Horman <horms at verge.net.au> wrote:
>> >> > > Do you think that adding support for multiple levels of tags to the kernel
>> >> > > is appropriate. Or should such cases be bounced to user-space?
>> >> >
>> >> > The kernel should support enough tags to handle the common cases.
>> >> > Initially I would make this 1 but I could see it becoming 2 in the
>> >> > future.  However, regardless of how many tags we support it's always
>> >> > possible that a greater number of tags are used than we support.  I
>> >> > can see three ways to handle this:
>> >> >
>> >> > 1. Don't support it.  You can run into a similar problem with vlan
>> >> > tags because it's possible to pop tags off and then do further
>> >> > operations but the kernel doesn't look deep enough into the packet.
>> >> > There isn't really a mechanism to deal with this today and to my
>> >> > knowledge nobody has actually complained.
>> >> >
>> >> > 2. Handle in userspace.  It's certainly a reasonable approach for corner cases.
>> >> >
>> >> > 3. Some form of recirculation.  Allowing the kernel to look at a
>> >> > packet, pop some tags off, and then look at it again allows processing
>> >> > an infinite number of tags in a manner that won't significantly affect
>> >> > throughput (although each round will result in more flow setups).  It
>> >> > also allows use cases like pop MPLS tag and do IP routing, which is
>> >> > both fairly common and can't be realistically achieved any other way
>> >> > in kernel.
>> >> >
>> >> > If the short term goal is a kernel implementation, I would do #1 since
>> >> > it's much simpler, is a strict increase in functionality from what we
>> >> > have today and doesn't preclude doing #3 in the future.  If you want
>> >> > something more complete then you can handle the extra cases using #2.
>> >> >
>> >> > If it's likely that we'll be using the userspace version for a while
>> >> > then it makes sense to handle all of the various cases upfront because
>> >> > we'll always have all of the packets and there's really no reason not
>> >> > to.
>> >> >
>> >> > So it depends what your goal is.
>> >>
>> >> My thinking is in terms of doing #1 then #2.
>> >>
>> >> #3 seems to add significant complexity to the kernel for corner cases.
>> >> If it is useful for a variety of cases then it may be worth-while,
>> >> but it seems to me to be overkill for MPLS alone.
>> >
>> > I have reconsidered and my current thinking is now in terms of doing #1
>> > and leaving the door open for #3.
>> >
>> > With that in mind I plan to post a revised patch which implements
>> > the non-datapath portions for MPLS actions and matches without attempting
>> > to provide non-datapath implementations of those actions or matches.
>> > That is, a user-space portion awaiting a data-path portion.
>> >
>> > I believe this is consistent with my new thinking not to pursue #2
>>
>> So after this patch there wouldn't be any new MPLS functionality at all?
>>
>> It seems like it would be very hard to review such a patch without a
>> clear plan of where it is going because many of the data structures
>> that it would need are different depending on what the datapath
>> portions look like.
>
> Understood.
>
>> Personally, I would just start adding the basic pieces both in kernel
>> in userspace.  A single MPLS label match certainly isn't controversial
>> and neither are basic push/pop actions.  They can be enabled
>> incrementally and anything that isn't implemented yet will just be
>> rejected, just as all of MPLS is today.
>>
>> I'm going to post the patch that Leo gave me.  It isn't complete and I
>> haven't reviewed it yet but I gave him the same advice so it should be
>> a reasonable starting point.
>
> So perhaps a good way forward would be to provide the non-datapath
> portions to match the (kernel) datapath portions in Leo's patch?

I think that makes sense.  Leo's patch will need a little more work
before it can go in, most notably offloading, but the basic operations
that it exposes in the interface are the ones that I would start with.