Re: [PATCH net-next 2/3] net: mpls: Fixups for GSO

On 8/17/16 7:06 PM, Alexander Duyck wrote:

On Wed, Aug 17, 2016 at 4:23 PM, David Ahern [off-list ref] wrote:

quoted

On 8/17/16 5:16 PM, Alexander Duyck wrote:

quoted

quoted

diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
index 1ecbd7715f6d..6d78f162a88b 100644
--- a/net/openvswitch/actions.c
+++ b/net/openvswitch/actions.c

@@ -167,6 +167,12 @@ static int push_mpls(struct sk_buff *skb, struct sw_flow_key *key,
                skb->mac_len);
        skb_reset_mac_header(skb);

+       /* for GSO: set MPLS as network header and encapsulated protocol
+        * header as inner network header
+        */
+       skb_set_network_header(skb, skb->mac_len);
+       skb_set_inner_network_header(skb, skb->mac_len + MPLS_HLEN);
+
        new_mpls_lse = (__be32 *)skb_mpls_header(skb);
        *new_mpls_lse = mpls->mpls_lse;

So the one question I would have about this is how attached are you to
using the network_header to record the offset for the MPLS header?  I
ask because I think from a hardware offloading perspective it would
make it much easier if instead you used the inner_mac_header to
represent the offset for the MPLS header.  This way device drivers
could just skip over it like a VLAN and just use network and transport
header values like they would otherwise.

Where does the network_header relate to if I change the marker to inner_mac_header? Would it be skipped?

No, the network header would still be the network header.

If core MPLS code (ie., non-OVS) does not do skb_reset_network_header(skb) after adding the MPLS label nothing works. Not even ping with small packets. tcpdump shows a completely mangled packet. Right now resetting the network_header to mpls is required.