[ovs-dev] [PATCH 01/16] User-Space MPLS actions and matches
horms at verge.net.au
Tue Feb 5 00:00:35 UTC 2013
On Mon, Feb 04, 2013 at 10:09:20AM -0800, Ben Pfaff wrote:
> On Fri, Jan 25, 2013 at 04:22:07PM +0900, Simon Horman wrote:
> > This patch implements use-space datapath and non-datapath code
> > to match and use the datapath API set out in Leo Alterman's patch
> > "user-space datapath: Add basic MPLS support to kernel".
> > The resulting MPLS implementation supports:
> > * Pushing a single MPLS label
> > * Poping a single MPLS label
> > * Modifying an MPLS lable using set-field or load actions
> > that act on the label value, tc and bos bit.
> > * There is no support for manipulating the TTL
> > this is considered future work.
> > The single-level push pop limitation is implemented by processing
> > push, pop and set-field/load actions in order and discarding information
> > that would require multiple levels of push/pop to be supported.
> > e.g.
> > push,push -> the first push is discarded
> > pop,pop -> the first pop is discarded
> > This patch is based heavily on work by Ravi K.
> > Cc: Ravi K <rkerur at gmail.com>
> > Reviewed-by: Isaku Yamahata <yamahata at valinux.co.jp>
> > Signed-off-by: Simon Horman <horms at verge.net.au>
> I resolved a few merge conflicts, then I went through this fairly
> carefully and made a few changes, mostly cosmetic. I'm appending an
> incremental and then a full revised patch.
> Here's a little commentary on the less trivial changes I made:
> - I think that eth_mpls_depth() was capable of counting an MPLS label
> that was off the end of the packet as part of the depth (for
> example, if there was one MPLS label without BOS followed by the
> end of the packet, I believe that it would count this as depth 2).
> This wasn't a problem for current callers but it seemed like a bad
> idea, so I changed it. Please check that it looks correct.
> - I think mf_random_value() should have used ->u8 for TC and BOS so I
> changed it.
Thanks, both of those changes seem reasonable to me. I'll review
these and the changes in your diff more thoroughly and get back to you ASAP.
> I have one question before I commit this: it seems that we could support
> a series of more than one pop_mpls action, why do we limit the number of
> pops to just one?
I think that I agree with you. I'll take a look and see how practical
it would be to remove that restriction.
More information about the dev