On Wed, Apr 3, 2013 at 5:55 PM, Simon Horman <ho...@verge.net.au> wrote: > On Wed, Apr 03, 2013 at 05:44:17PM -0700, Jesse Gross wrote: >> On Wed, Apr 3, 2013 at 5:28 PM, Simon Horman <ho...@verge.net.au> wrote: >> > 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. >> >> If you have the MAC len then you don't need any new MPLS code here at >> all; just replicate the whole thing onto each packet. > > The MAC len is set to cover everything up to the top of the MPLS stack. > So it seems that something needs to be done to account for the length > of the MPLS stack. > > Are you suggesting that if Open vSwtich set up the MAC len to extend > to the end of the MPLS stack then mpls_gro_segment() would not be necessary?
Something along those lines. I think this is very similar to the skb->protocol discussion (and likely influenced by the outcome of that). MPLS is a little weird with respect to the existing layer pointers but if we can find a good definition then I think that the GSO code should be pretty minimal. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
