On Tue, Feb 05, 2013 at 09:19:06AM -0800, Ben Pfaff wrote:
> On Tue, Feb 05, 2013 at 10:13:18AM +0900, Simon Horman wrote:
> > On Tue, Feb 05, 2013 at 09:00:35AM +0900, Simon Horman wrote:
> > > 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 <rke...@gmail.com>
> > > > > Reviewed-by: Isaku Yamahata <yamah...@valinux.co.jp>
> > > > > Signed-off-by: Simon Horman <ho...@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 reviewed all the changes and they look good to me.
> > Please feel free to apply. I can then rebase the remaining patches on
> > master.
> 
> I fixed a few indentation errors I noticed at the last minute and
> applied this to master.  Thanks!

Great, thanks!

I'll rebase the remaining patches ASAP and...

> > > > 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.
> > 
> > I'll start looking into this in more depth. But I'd rather handle any
> > updates as a subsequent change to this patch.
> 
> Thanks, that sounds fine.

... investigate this.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to