On Wed, Apr 03, 2013 at 04:51:38PM -0700, Jesse Gross wrote: > On Wed, Apr 3, 2013 at 2:11 AM, Simon Horman <ho...@verge.net.au> wrote: > > In the case where a non-MPLS packet is recieved and an MPLS stack is > > added it may well be the case that the original skb is GSO but the > > NIC used for transmit does not support GSO of MPLS packets. > > > > The aim of this code is to provide GSO in software for MPLS packets > > whose skbs are GSO. > > > > When an implementation adds an MPLS stack to a non-MPLS packet it should do > > the following to skb metadata: > > > > * Set skb_mac_header(skb)->protocol to the new MPLS ethertype. > > That is, either ETH_P_MPLS_UC or ETH_P_MPLS_MC. > > > > * Leave skb->protocol as the old non-MPLS ethertype. > > > > * Set skb->encapsulation = 1. > > > > This may not strictly be necessary as I believe that checking > > skb_mac_header(skb)->protocol and skb->protocol should be necessary and > > sufficient. > > > > However, it does seem to fit nicely with the current implementation of > > dev_hard_start_xmit() where the more expensive check of > > skb_mac_header(skb)->protocol may be guarded by an existing check of > > skb->encapsulation. > > > > One aspect of this patch that I am unsure about is the modification I have > > made to skb_segment(). This seems to be necessary as checskum accelearation > > may no longer be possible as the packet has changed to be MPLS from some > > other packet type which may have been supported by the hardware in use. > > > > I will post a patch, "[PATCH v3.24] datapath: Add basic MPLS support to > > kernel" which adds MPLS support to the kernel datapath of Open vSwtich. > > That patch sets the above requirements in > > datapath/actions.c:set_ethertype() and was used to exercise the MPLS GSO > > code. The datapath patch is against the Open vSwtich tree but it is > > intended that it be added to the Open vSwtich code present in the mainline > > Linux kernel at some point. > > > > Suggested by Jesse Gross. Based heavily on "v4 GRE: Add TCP segmentation > > offload for GRE" by Pravin B Shelar. > > > > Cc: Jesse Gross <je...@nicira.com> > > Cc: Pravin B Shelar <pshe...@nicira.com> > > Signed-off-by: Simon Horman <ho...@verge.net.au> > > MPLS is very similar to both the Ethernet header and vlans in that GSO > only requires replication without any modification. That means that > if we look at the mac_len as containing all three then we can just > copy it without any special knowledge. I don't know that we carefully > maintain mac_len in all places but you are already doing that in your > MPLS patches.
At least for the cases that I am aware of I think that mac_len is predictable. But I'm a little unsure of what you are getting at here. > The other piece at that point is getting the inner protocol. I'm > worried that using skb->protocol for this will break things that try > to use it for filtering. It also means that depending on whether an > MPLS packet is locally sourced or not skb->protocol may be different > because we won't always be able to find the inner header. I think > this will require a careful definition of that field to make it > consistent. Yes, I agree. I was hoping that posting the patch to netdev would result in some analysis of this. Another idea I had would be to a new element to struct sk_buff to explicitly hold the encapsulated protocol for (cases like) MPLS. But it seems that it would be nicer to re-use the protocol element if possible. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
