[ovs-dev] [PATCH v2.0] Non-Datapath MPLS actions and matches
horms at verge.net.au
Thu Oct 4 00:40:56 UTC 2012
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.
> 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?
More information about the dev