On Fri, Sep 30, 2016 at 1:06 AM, Jiri Benc [off-list ref] wrote:

On Thu, 29 Sep 2016 16:03:19 -0700, pravin shelar wrote:

quoted

quoted

--- a/include/net/mpls.h
+++ b/include/net/mpls.h

@@ -25,15 +25,4 @@ static inline bool eth_p_mpls(__be16 eth_type)
                eth_type == htons(ETH_P_MPLS_MC);
 }

-/*
- * For non-MPLS skbs this will correspond to the network header.
- * For MPLS skbs it will be before the network_header as the MPLS
- * label stack lies between the end of the mac header and the network
- * header. That is, for MPLS skbs the end of the mac header
- * is the top of the MPLS label stack.
- */
-static inline unsigned char *skb_mpls_header(struct sk_buff *skb)
-{
-       return skb_mac_header(skb) + skb->mac_len;
-}

I think we should keep this API, so that it is clear that MPLS header
mapped skb network header.

I was pondering this but I don't think it really gains us anything.
Wrappers like ip_hdr() are useful as they do type conversion; it's much
nicer to write
        ip_hdr(skb)->daddr
than
        (struct iphdr *)skb_network_header(skb)->daddr.

But we don't really have a good type to return from skb_mpls_header, it
boils down to be 100% equivalent to skb_network_header. In fact, you
could just do:
#define skb_mpls_header skb_network_header

I am fine with this too.

In the code, I don't think there's much benefit from calling the
wrapper, meaning of skb_network_header itself is clear enough. It *is*
the network header, after all. In the whole openvswitch code, MPLS is
treated as L3 header - no dissection is performed after the MPLS
headers, the network header and mac_len is set as expected now, etc.

It makes it easier to locate which modules are using this API. That
also helps if in future we decide to change MPLS header mapping again.