[ovs-dev] [PATCH 3/3] datapath: Add basic MPLS support to kernel

[Simon Horman]

On Wed, Mar 06, 2013 at 09:12:53AM -0800, Jesse Gross wrote:
> On Tue, Mar 5, 2013 at 10:03 PM, Simon Horman <horms at verge.net.au> wrote:
> > On Tue, Mar 05, 2013 at 11:35:41AM -0800, Jesse Gross wrote:
> >> On Tue, Mar 5, 2013 at 10:23 AM, Ben Pfaff <blp at nicira.com> wrote:
> >> > On Thu, Feb 28, 2013 at 06:15:09PM +0900, Simon Horman wrote:
> >> >> Allow datapath to recognize and extract MPLS labels into flow keys
> >> >> and execute actions which push, pop, and set labels on packets.
> >> >>
> >> >> Based heavily on work by Leo Alterman and Ravi K.
> >> >>
> >> >> Cc: Ravi K <rkerur at gmail.com>
> >> >> Cc: Leo Alterman <lalterman at nicira.com>
> >> >> Reviewed-by: Isaku Yamahata <yamahata at valinux.co.jp>
> >> >> Signed-off-by: Simon Horman <horms at verge.net.au>
> >> >
> >> > Jesse needs to review this.
> >>
> >> I was hoping that Simon would update the patch based on the discussion
> >> in the thread before I take another look at it.
> >
> > Hi Jesse,
> >
> > I'm not quite sure which issues you would like me to look at.
> >
> > There are number of changes in v2.20 (this version) of the patch
> > which address the very helpful review you supplied.
> >
> > On my todo list I have the following:
> >
> > * Exercise checksum code in pop_mpls(), push_mpls() and set_mpls()
> >   I am unsure how to pass through or simulate skbs with CHECKSUM_COMPLETE.
> > * Enhance core kernel code to handle GSO for MPLS or
> >   otherwise deal with accelerations. (Linux network core)
> > * Add ETH_TYPE_MIN or similar to Linux network core
> > * Enhance ovs-vswitchd to output actions in order to
> >   allow use of network_header in the datapath to track the top
> >   of the MPLS stack.
> >
> > I'd appreciate some guidance on how to achieve the first item on the list.
> 
> I wouldn't worry that much about it.  There are a few NICs that will
> generate packets like this but they are fairly rare at this point and
> simulating it seems like more trouble than it is worth.  I'll review
> it and that should be good enough.

Thanks.

> > And the rest of the items seem like future work to me.
> >
> > Am I missing some items on my list?
> 
> The main one that I was referring to is the last one on the list since
> I think that it will allow us to simplify things somewhat.  GSO and
> other offloads are also somewhat problematic since it means that until
> we have that core kernel support OVS will create invalid packets in
> some situations.  The exact same situation happened previously with
> vlans, so there is some GSO emulation code in vport-netdev.c.

Sure, I can look into it.

Perhaps Ben has some ideas, but it seems to me that the
way that execute_* and compose_* interact needs to be changed
around in order to preserve the order of actions.