[PATCH next-next v4 0/2] MPLS: Add limited GSO support

[PATCH next-next v4 0/2] MPLS: Add limited GSO support

[Simon Horman]

In the case where a non-MPLS packet is received 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 short series is to provide GSO in software for MPLS packets
whose skbs are GSO.

Change since v4:

Update first patch of the series to use 16 bits for all *_headers
rather than just inner_*_headers

Simon Horman (2):
  net: Use 16bits for *_headers fields of struct skbuff
  MPLS: Add limited GSO support

 include/linux/netdev_features.h |   4 +-
 include/linux/netdevice.h       |   2 +
 include/linux/skbuff.h          | 123 ++++------------------------------------
 net/Kconfig                     |   1 +
 net/Makefile                    |   1 +
 net/core/dev.c                  |   4 ++
 net/core/ethtool.c              |   1 +
 net/ipv4/af_inet.c              |   1 +
 net/ipv4/tcp.c                  |   1 +
 net/ipv4/udp.c                  |   2 +-
 net/ipv6/ip6_offload.c          |   1 +
 net/ipv6/udp_offload.c          |   3 +-
 net/mpls/Kconfig                |   9 +++
 net/mpls/Makefile               |   4 ++
 net/mpls/mpls_gso.c             | 108 +++++++++++++++++++++++++++++++++++
 15 files changed, 149 insertions(+), 116 deletions(-)
 create mode 100644 net/mpls/Kconfig
 create mode 100644 net/mpls/Makefile
 create mode 100644 net/mpls/mpls_gso.c

-- 
1.8.2.1

[PATCH net-next v5 1/2] net: Use 16bits for *_headers fields of struct skbuff

[Simon Horman]

In order to mitigate ongoing incresase in the size of struct skbuff
use 16 bit integer offsets rather than pointers for inner_*_headers.

This appears to reduce the size of struct skbuff from 0xd0 to 0xc0
bytes on x86_64 with the following all unset.

	CONFIG_XFRM
	CONFIG_NF_CONNTRACK
	CONFIG_NF_CONNTRACK_MODULE
	NET_SKBUFF_NF_DEFRAG_NEEDED
	CONFIG_BRIDGE_NETFILTER
	CONFIG_NET_SCHED
	CONFIG_IPV6_NDISC_NODETYPE
	CONFIG_NET_DMA
	CONFIG_NETWORK_SECMARK

Signed-off-by: Simon Horman <horms@verge.net.au>
---
 include/linux/skbuff.h | 119 +++----------------------------------------------
 1 file changed, 6 insertions(+), 113 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 2e0ced1..5663e35 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -509,12 +509,12 @@ struct sk_buff {
 		__u32		reserved_tailroom;
 	};
 
-	sk_buff_data_t		inner_transport_header;
-	sk_buff_data_t		inner_network_header;
-	sk_buff_data_t		inner_mac_header;
-	sk_buff_data_t		transport_header;
-	sk_buff_data_t		network_header;
-	sk_buff_data_t		mac_header;
+	__u16			inner_transport_header;
+	__u16			inner_network_header;
+	__u16			inner_mac_header;
+	__u16			transport_header;
+	__u16			network_header;
+	__u16			mac_header;
 	/* These elements must be at the end, see alloc_skb() for details.  */
 	sk_buff_data_t		tail;
 	sk_buff_data_t		end;
@@ -1527,7 +1527,6 @@ static inline void skb_reset_mac_len(struct sk_buff *skb)
 	skb->mac_len = skb->network_header - skb->mac_header;
 }
 
-#ifdef NET_SKBUFF_DATA_USES_OFFSET
 static inline unsigned char *skb_inner_transport_header(const struct sk_buff
 							*skb)
 {
@@ -1638,112 +1637,6 @@ static inline void skb_set_mac_header(struct sk_buff *skb, const int offset)
 	skb->mac_header += offset;
 }
 
-#else /* NET_SKBUFF_DATA_USES_OFFSET */
-static inline unsigned char *skb_inner_transport_header(const struct sk_buff
-							*skb)
-{
-	return skb->inner_transport_header;
-}
-
-static inline void skb_reset_inner_transport_header(struct sk_buff *skb)
-{
-	skb->inner_transport_header = skb->data;
-}
-
-static inline void skb_set_inner_transport_header(struct sk_buff *skb,
-						   const int offset)
-{
-	skb->inner_transport_header = skb->data + offset;
-}
-
-static inline unsigned char *skb_inner_network_header(const struct sk_buff *skb)
-{
-	return skb->inner_network_header;
-}
-
-static inline void skb_reset_inner_network_header(struct sk_buff *skb)
-{
-	skb->inner_network_header = skb->data;
-}
-
-static inline void skb_set_inner_network_header(struct sk_buff *skb,
-						const int offset)
-{
-	skb->inner_network_header = skb->data + offset;
-}
-
-static inline unsigned char *skb_inner_mac_header(const struct sk_buff *skb)
-{
-	return skb->inner_mac_header;
-}
-
-static inline void skb_reset_inner_mac_header(struct sk_buff *skb)
-{
-	skb->inner_mac_header = skb->data;
-}
-
-static inline void skb_set_inner_mac_header(struct sk_buff *skb,
-						const int offset)
-{
-	skb->inner_mac_header = skb->data + offset;
-}
-static inline bool skb_transport_header_was_set(const struct sk_buff *skb)
-{
-	return skb->transport_header != NULL;
-}
-
-static inline unsigned char *skb_transport_header(const struct sk_buff *skb)
-{
-	return skb->transport_header;
-}
-
-static inline void skb_reset_transport_header(struct sk_buff *skb)
-{
-	skb->transport_header = skb->data;
-}
-
-static inline void skb_set_transport_header(struct sk_buff *skb,
-					    const int offset)
-{
-	skb->transport_header = skb->data + offset;
-}
-
-static inline unsigned char *skb_network_header(const struct sk_buff *skb)
-{
-	return skb->network_header;
-}
-
-static inline void skb_reset_network_header(struct sk_buff *skb)
-{
-	skb->network_header = skb->data;
-}
-
-static inline void skb_set_network_header(struct sk_buff *skb, const int offset)
-{
-	skb->network_header = skb->data + offset;
-}
-
-static inline unsigned char *skb_mac_header(const struct sk_buff *skb)
-{
-	return skb->mac_header;
-}
-
-static inline int skb_mac_header_was_set(const struct sk_buff *skb)
-{
-	return skb->mac_header != NULL;
-}
-
-static inline void skb_reset_mac_header(struct sk_buff *skb)
-{
-	skb->mac_header = skb->data;
-}
-
-static inline void skb_set_mac_header(struct sk_buff *skb, const int offset)
-{
-	skb->mac_header = skb->data + offset;
-}
-#endif /* NET_SKBUFF_DATA_USES_OFFSET */
-
 static inline void skb_probe_transport_header(struct sk_buff *skb,
 					      const int offset_hint)
 {
-- 
1.8.2.1

[PATCH net-next v5 2/2] MPLS: Add limited GSO support

[Simon Horman]

In the case where a non-MPLS packet is received 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.

SKB Usage:

When an implementation adds an MPLS stack to a non-MPLS packet it should do
the following to skb metadata:

* Set skb->inner_protocol to the old non-MPLS ethertype of the packet.
  skb->inner_protocol is added by this patch.

* Set skb->protocol to the new MPLS ethertype of the packet.

* Set skb->network_header to correspond to the
  end of the L3 header, including the MPLS label stack.

I have posted a patch, "[PATCH v3.29] 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:push_mpls()
and was used to exercise this 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.

Features:

I believe that the approach that I have taken is at least partially
consistent with the handling of other protocols.  Jesse, I understand that
you have some ideas here.  I am more than happy to change my implementation.

This patch adds dev->mpls_features which may be used by devices
to advertise features supported for MPLS packets.

A new NETIF_F_MPLS_GSO feature is added for devices which support
hardware MPLS GSO offload.  Currently no devices support this
and MPLS GSO always falls back to software.

Alternate Implementation:

One possible alternate implementation is to teach netif_skb_features()
and skb_network_protocol() about MPLS, in a similar way to their
understanding of VLANs. I believe this would avoid the need
for net/mpls/mpls_gso.c and in particular the calls to
__skb_push() and __skb_push() in mpls_gso_segment().

I have decided on the implementation in this patch as it should
not introduce any overhead in the case where mpls_gso is not compiled
into the kernel or inserted as a module.

MPLS GSO suggested by Jesse Gross.
Based in part on "v4 GRE: Add TCP segmentation offload for GRE"
by Pravin B Shelar.

Cc: Jesse Gross <jesse@nicira.com>
Cc: Pravin B Shelar <pshelar@nicira.com>
Signed-off-by: Simon Horman <horms@verge.net.au>

---

MPLS GSO also requires "net: Loosen constraints for recalculating checksum
in skb_segment()" in order for segmentation of TCP/IP inner packets to
correctly recalculate checksums.

Tested using the bnx2 driver.

v5
* Move inner_protocol from below inner_mac_header to
  above inner_transport header to allow all *_header fields
  to be grouped together.
* Remove trailing blank line from mpls_gso.c

v4
* Add inner_protocol to kernel doc for struct skbuff
* As suggested by Jesse Gross
  - Refer to CONFIG_NET_MPLS_GSO rather than CONFIG_NET_GRE_GSO in
    change log
  - Reword the changelog to reflect that mac_len is not set to correspond
    to the end of l3 data. Its value is not important to the function of
    GSO as a call to skb_reset_mac_len() is made in __skb_gso_segment().

* I considered guarding the presence of inner_protocol in struct skbuff with
  #ifdef CONFIG_NET_MPLS_GSO as it is otherwise unused. However, this
  makes accessing inner_protocol from out-of tree code somewhat tedious.
  And the reference usage of this code is the out-of tree version
  of the Open vSwitch datapath.

v3
* As suggested by Ben Hutchings
  - Update NETIF_F_GSO_LAST
* Update SKB usage
  - Add and use skb->inner_protocol
  - Set skb->protocol to MPLS ethertype of packet
  - Set skb->mac_len and skb->network_header to correspond to the
    end of the L3 header.

v2
* As suggested by Jarno Rajahalme
  - Update NETIF_F_GSO_LAST
  - mpls_mc_offload to use ETH_P_MPLS_MC
* Remove NETIF_F_iP_CSUM|NETIF_F_IPV6_CSUM features hack in
  mpls_gso_segment()
---
 include/linux/netdev_features.h |   4 +-
 include/linux/netdevice.h       |   2 +
 include/linux/skbuff.h          |   4 ++
 net/Kconfig                     |   1 +
 net/Makefile                    |   1 +
 net/core/dev.c                  |   4 ++
 net/core/ethtool.c              |   1 +
 net/ipv4/af_inet.c              |   1 +
 net/ipv4/tcp.c                  |   1 +
 net/ipv4/udp.c                  |   2 +-
 net/ipv6/ip6_offload.c          |   1 +
 net/ipv6/udp_offload.c          |   3 +-
 net/mpls/Kconfig                |   9 ++++
 net/mpls/Makefile               |   4 ++
 net/mpls/mpls_gso.c             | 108 ++++++++++++++++++++++++++++++++++++++++
 15 files changed, 143 insertions(+), 3 deletions(-)
 create mode 100644 net/mpls/Kconfig
 create mode 100644 net/mpls/Makefile
 create mode 100644 net/mpls/mpls_gso.c

diff --git a/include/linux/netdev_features.h b/include/linux/netdev_features.h
index 09906b7..a2a89a5 100644
--- a/include/linux/netdev_features.h
+++ b/include/linux/netdev_features.h
@@ -43,8 +43,9 @@ enum {
 	NETIF_F_FSO_BIT,		/* ... FCoE segmentation */
 	NETIF_F_GSO_GRE_BIT,		/* ... GRE with TSO */
 	NETIF_F_GSO_UDP_TUNNEL_BIT,	/* ... UDP TUNNEL with TSO */
+	NETIF_F_GSO_MPLS_BIT,		/* ... MPLS segmentation */
 	/**/NETIF_F_GSO_LAST =		/* last bit, see GSO_MASK */
-		NETIF_F_GSO_UDP_TUNNEL_BIT,
+		NETIF_F_GSO_MPLS_BIT,
 
 	NETIF_F_FCOE_CRC_BIT,		/* FCoE CRC32 */
 	NETIF_F_SCTP_CSUM_BIT,		/* SCTP checksum offload */
@@ -107,6 +108,7 @@ enum {
 #define NETIF_F_RXALL		__NETIF_F(RXALL)
 #define NETIF_F_GSO_GRE		__NETIF_F(GSO_GRE)
 #define NETIF_F_GSO_UDP_TUNNEL	__NETIF_F(GSO_UDP_TUNNEL)
+#define NETIF_F_GSO_MPLS	__NETIF_F(GSO_MPLS)
 #define NETIF_F_HW_VLAN_STAG_FILTER __NETIF_F(HW_VLAN_STAG_FILTER)
 #define NETIF_F_HW_VLAN_STAG_RX	__NETIF_F(HW_VLAN_STAG_RX)
 #define NETIF_F_HW_VLAN_STAG_TX	__NETIF_F(HW_VLAN_STAG_TX)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 7dd535d..c7b608b 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1088,6 +1088,8 @@ struct net_device {
 	 * need to set them appropriately.
 	 */
 	netdev_features_t	hw_enc_features;
+	/* mask of fetures inheritable by MPLS */
+	netdev_features_t	mpls_features;
 
 	/* Interface index. Unique device identifier	*/
 	int			ifindex;
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 5663e35..8f2b830 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -319,6 +319,8 @@ enum {
 	SKB_GSO_GRE = 1 << 6,
 
 	SKB_GSO_UDP_TUNNEL = 1 << 7,
+
+	SKB_GSO_MPLS = 1 << 8,
 };
 
 #if BITS_PER_LONG > 32
@@ -389,6 +391,7 @@ typedef unsigned char *sk_buff_data_t;
  *	@dropcount: total number of sk_receive_queue overflows
  *	@vlan_proto: vlan encapsulation protocol
  *	@vlan_tci: vlan tag control information
+ *	@inner_protocol: Protocol (encapsulation)
  *	@inner_transport_header: Inner transport layer header (encapsulation)
  *	@inner_network_header: Network layer header (encapsulation)
  *	@inner_mac_header: Link layer header (encapsulation)
@@ -509,6 +512,7 @@ struct sk_buff {
 		__u32		reserved_tailroom;
 	};
 
+	__be16			inner_protocol;
 	__u16			inner_transport_header;
 	__u16			inner_network_header;
 	__u16			inner_mac_header;
diff --git a/net/Kconfig b/net/Kconfig
index 08de901..523e43e 100644
--- a/net/Kconfig
+++ b/net/Kconfig
@@ -218,6 +218,7 @@ source "net/batman-adv/Kconfig"
 source "net/openvswitch/Kconfig"
 source "net/vmw_vsock/Kconfig"
 source "net/netlink/Kconfig"
+source "net/mpls/Kconfig"
 
 config RPS
 	boolean
diff --git a/net/Makefile b/net/Makefile
index 091e7b04..9492e8c 100644
--- a/net/Makefile
+++ b/net/Makefile
@@ -70,3 +70,4 @@ obj-$(CONFIG_BATMAN_ADV)	+= batman-adv/
 obj-$(CONFIG_NFC)		+= nfc/
 obj-$(CONFIG_OPENVSWITCH)	+= openvswitch/
 obj-$(CONFIG_VSOCKETS)	+= vmw_vsock/
+obj-$(CONFIG_NET_MPLS_GSO)	+= mpls/
diff --git a/net/core/dev.c b/net/core/dev.c
index 7229bc3..3cb489f 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5276,6 +5276,10 @@ int register_netdevice(struct net_device *dev)
 	 */
 	dev->hw_enc_features |= NETIF_F_SG;
 
+	/* Make NETIF_F_SG inheritable to MPLS.
+	 */
+	dev->mpls_features |= NETIF_F_SG;
+
 	ret = call_netdevice_notifiers(NETDEV_POST_INIT, dev);
 	ret = notifier_to_errno(ret);
 	if (ret)
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 22efdaa..4e6f63a 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -82,6 +82,7 @@ static const char netdev_features_strings[NETDEV_FEATURE_COUNT][ETH_GSTRING_LEN]
 	[NETIF_F_FSO_BIT] =              "tx-fcoe-segmentation",
 	[NETIF_F_GSO_GRE_BIT] =		 "tx-gre-segmentation",
 	[NETIF_F_GSO_UDP_TUNNEL_BIT] =	 "tx-udp_tnl-segmentation",
+	[NETIF_F_GSO_MPLS_BIT] =	 "tx-mpls-segmentation",
 
 	[NETIF_F_FCOE_CRC_BIT] =         "tx-checksum-fcoe-crc",
 	[NETIF_F_SCTP_CSUM_BIT] =        "tx-checksum-sctp",
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index d01be2a..b05ae96 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -1295,6 +1295,7 @@ static struct sk_buff *inet_gso_segment(struct sk_buff *skb,
 		       SKB_GSO_GRE |
 		       SKB_GSO_TCPV6 |
 		       SKB_GSO_UDP_TUNNEL |
+		       SKB_GSO_MPLS |
 		       0)))
 		goto out;
 
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 53d9c12..6201e57 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -2916,6 +2916,7 @@ struct sk_buff *tcp_tso_segment(struct sk_buff *skb,
 			       SKB_GSO_TCP_ECN |
 			       SKB_GSO_TCPV6 |
 			       SKB_GSO_GRE |
+			       SKB_GSO_MPLS |
 			       SKB_GSO_UDP_TUNNEL |
 			       0) ||
 			     !(type & (SKB_GSO_TCPV4 | SKB_GSO_TCPV6))))
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 0bf5d39..aa5eff4 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -2381,7 +2381,7 @@ struct sk_buff *udp4_ufo_fragment(struct sk_buff *skb,
 
 		if (unlikely(type & ~(SKB_GSO_UDP | SKB_GSO_DODGY |
 				      SKB_GSO_UDP_TUNNEL |
-				      SKB_GSO_GRE) ||
+				      SKB_GSO_GRE | SKB_GSO_MPLS) ||
 			     !(type & (SKB_GSO_UDP))))
 			goto out;
 
diff --git a/net/ipv6/ip6_offload.c b/net/ipv6/ip6_offload.c
index 71b766e..a263b99 100644
--- a/net/ipv6/ip6_offload.c
+++ b/net/ipv6/ip6_offload.c
@@ -98,6 +98,7 @@ static struct sk_buff *ipv6_gso_segment(struct sk_buff *skb,
 		       SKB_GSO_TCP_ECN |
 		       SKB_GSO_GRE |
 		       SKB_GSO_UDP_TUNNEL |
+		       SKB_GSO_MPLS |
 		       SKB_GSO_TCPV6 |
 		       0)))
 		goto out;
diff --git a/net/ipv6/udp_offload.c b/net/ipv6/udp_offload.c
index 3bb3a89..76d401a 100644
--- a/net/ipv6/udp_offload.c
+++ b/net/ipv6/udp_offload.c
@@ -63,7 +63,8 @@ static struct sk_buff *udp6_ufo_fragment(struct sk_buff *skb,
 		if (unlikely(type & ~(SKB_GSO_UDP |
 				      SKB_GSO_DODGY |
 				      SKB_GSO_UDP_TUNNEL |
-				      SKB_GSO_GRE) ||
+				      SKB_GSO_GRE |
+				      SKB_GSO_MPLS) ||
 			     !(type & (SKB_GSO_UDP))))
 			goto out;
 
diff --git a/net/mpls/Kconfig b/net/mpls/Kconfig
new file mode 100644
index 0000000..37421db
--- /dev/null
+++ b/net/mpls/Kconfig
@@ -0,0 +1,9 @@
+#
+# MPLS configuration
+#
+config NET_MPLS_GSO
+	tristate "MPLS: GSO support"
+	help
+	 This is helper module to allow segmentation of non-MPLS GSO packets
+	 that have had MPLS stack entries pushed onto them and thus
+	 become MPLS GSO packets.
diff --git a/net/mpls/Makefile b/net/mpls/Makefile
new file mode 100644
index 0000000..0a3c171
--- /dev/null
+++ b/net/mpls/Makefile
@@ -0,0 +1,4 @@
+#
+# Makefile for MPLS.
+#
+obj-y += mpls_gso.o
diff --git a/net/mpls/mpls_gso.c b/net/mpls/mpls_gso.c
new file mode 100644
index 0000000..1bec121
--- /dev/null
+++ b/net/mpls/mpls_gso.c
@@ -0,0 +1,108 @@
+/*
+ *	MPLS GSO Support
+ *
+ *	Authors: Simon Horman (horms@verge.net.au)
+ *
+ *	This program is free software; you can redistribute it and/or
+ *	modify it under the terms of the GNU General Public License
+ *	as published by the Free Software Foundation; either version
+ *	2 of the License, or (at your option) any later version.
+ *
+ *	Based on: GSO portions of net/ipv4/gre.c
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/err.h>
+#include <linux/module.h>
+#include <linux/netdev_features.h>
+#include <linux/netdevice.h>
+#include <linux/skbuff.h>
+
+static struct sk_buff *mpls_gso_segment(struct sk_buff *skb,
+				       netdev_features_t features)
+{
+	struct sk_buff *segs = ERR_PTR(-EINVAL);
+	netdev_features_t mpls_features;
+	__be16 mpls_protocol;
+
+	if (unlikely(skb_shinfo(skb)->gso_type &
+				~(SKB_GSO_TCPV4 |
+				  SKB_GSO_TCPV6 |
+				  SKB_GSO_UDP |
+				  SKB_GSO_DODGY |
+				  SKB_GSO_TCP_ECN |
+				  SKB_GSO_GRE |
+				  SKB_GSO_MPLS)))
+		goto out;
+
+	/* Setup inner SKB. */
+	mpls_protocol = skb->protocol;
+	skb->protocol = skb->inner_protocol;
+
+	/* Push back the mac header that skb_mac_gso_segment() has pulled.
+	 * It will be re-pulled by the call to skb_mac_gso_segment() below
+	 */
+	__skb_push(skb, skb->mac_len);
+
+	/* Segment inner packet. */
+	mpls_features = skb->dev->mpls_features & netif_skb_features(skb);
+	segs = skb_mac_gso_segment(skb, mpls_features);
+
+
+	/* Restore outer protocol. */
+	skb->protocol = mpls_protocol;
+
+	/* Re-pull the mac header that the call to skb_mac_gso_segment()
+	 * above pulled.  It will be re-pushed after returning
+	 * skb_mac_gso_segment(), an indirect caller of this function.
+	 */
+	__skb_push(skb, skb->data - skb_mac_header(skb));
+
+out:
+	return segs;
+}
+
+static int mpls_gso_send_check(struct sk_buff *skb)
+{
+	return 0;
+}
+
+static struct packet_offload mpls_mc_offload = {
+	.type = cpu_to_be16(ETH_P_MPLS_MC),
+	.callbacks = {
+		.gso_send_check =	mpls_gso_send_check,
+		.gso_segment    =	mpls_gso_segment,
+	},
+};
+
+static struct packet_offload mpls_uc_offload = {
+	.type = cpu_to_be16(ETH_P_MPLS_UC),
+	.callbacks = {
+		.gso_send_check =	mpls_gso_send_check,
+		.gso_segment    =	mpls_gso_segment,
+	},
+};
+
+static int __init mpls_gso_init(void)
+{
+	pr_info("MPLS GSO support\n");
+
+	dev_add_offload(&mpls_uc_offload);
+	dev_add_offload(&mpls_mc_offload);
+
+	return 0;
+}
+
+static void __exit mpls_gso_exit(void)
+{
+	dev_remove_offload(&mpls_uc_offload);
+	dev_remove_offload(&mpls_mc_offload);
+}
+
+module_init(mpls_gso_init);
+module_exit(mpls_gso_exit);
+
+MODULE_DESCRIPTION("MPLS GSO support");
+MODULE_AUTHOR("Simon Horman (horms@verge.net.au)");
+MODULE_LICENSE("GPL");
-- 
1.8.2.1

Re: [PATCH next-next v4 0/2] MPLS: Add limited GSO support

[Simon Horman]

[ Sorry, I messed up the subject, I will re-post ]

On Fri, May 24, 2013 at 03:51:33PM +0900, Simon Horman wrote:
> In the case where a non-MPLS packet is received 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 short series is to provide GSO in software for MPLS packets
> whose skbs are GSO.
> 
> Change since v4:
> 
> Update first patch of the series to use 16 bits for all *_headers
> rather than just inner_*_headers
> 
> Simon Horman (2):
>   net: Use 16bits for *_headers fields of struct skbuff
>   MPLS: Add limited GSO support
> 
>  include/linux/netdev_features.h |   4 +-
>  include/linux/netdevice.h       |   2 +
>  include/linux/skbuff.h          | 123 ++++------------------------------------
>  net/Kconfig                     |   1 +
>  net/Makefile                    |   1 +
>  net/core/dev.c                  |   4 ++
>  net/core/ethtool.c              |   1 +
>  net/ipv4/af_inet.c              |   1 +
>  net/ipv4/tcp.c                  |   1 +
>  net/ipv4/udp.c                  |   2 +-
>  net/ipv6/ip6_offload.c          |   1 +
>  net/ipv6/udp_offload.c          |   3 +-
>  net/mpls/Kconfig                |   9 +++
>  net/mpls/Makefile               |   4 ++
>  net/mpls/mpls_gso.c             | 108 +++++++++++++++++++++++++++++++++++
>  15 files changed, 149 insertions(+), 116 deletions(-)
>  create mode 100644 net/mpls/Kconfig
>  create mode 100644 net/mpls/Makefile
>  create mode 100644 net/mpls/mpls_gso.c
> 
> -- 
> 1.8.2.1
> 

Re: [PATCH next-next v4 0/2] MPLS: Add limited GSO support

[Zhi Yong Wu]

HI,

   By the way, will this label is allocated statically or via
auto-negotiation between two switches such as LDP?

On Fri, May 24, 2013 at 2:51 PM, Simon Horman <horms@verge.net.au> wrote:
> In the case where a non-MPLS packet is received 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 short series is to provide GSO in software for MPLS packets
> whose skbs are GSO.
>
> Change since v4:
>
> Update first patch of the series to use 16 bits for all *_headers
> rather than just inner_*_headers
>
> Simon Horman (2):
>   net: Use 16bits for *_headers fields of struct skbuff
>   MPLS: Add limited GSO support
>
>  include/linux/netdev_features.h |   4 +-
>  include/linux/netdevice.h       |   2 +
>  include/linux/skbuff.h          | 123 ++++------------------------------------
>  net/Kconfig                     |   1 +
>  net/Makefile                    |   1 +
>  net/core/dev.c                  |   4 ++
>  net/core/ethtool.c              |   1 +
>  net/ipv4/af_inet.c              |   1 +
>  net/ipv4/tcp.c                  |   1 +
>  net/ipv4/udp.c                  |   2 +-
>  net/ipv6/ip6_offload.c          |   1 +
>  net/ipv6/udp_offload.c          |   3 +-
>  net/mpls/Kconfig                |   9 +++
>  net/mpls/Makefile               |   4 ++
>  net/mpls/mpls_gso.c             | 108 +++++++++++++++++++++++++++++++++++
>  15 files changed, 149 insertions(+), 116 deletions(-)
>  create mode 100644 net/mpls/Kconfig
>  create mode 100644 net/mpls/Makefile
>  create mode 100644 net/mpls/mpls_gso.c
>
> --
> 1.8.2.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Regards,

Zhi Yong Wu

Re: [PATCH next-next v4 0/2] MPLS: Add limited GSO support

[Simon Horman]

On Fri, May 24, 2013 at 03:12:33PM +0800, Zhi Yong Wu wrote:
> HI,
> 
>    By the way, will this label is allocated statically or via
> auto-negotiation between two switches such as LDP?

Hi,

The support that I have been working on is for Open vSwtich.
As such the label is added by Open vSwtich which may be configured in
a variety of ways including static configuration using ovs-odpctl
and dynamic configuration using an Open Flow controller.

However, this small patch-set only adds limited GSO support for MPLS.
It does not handle manipulation of labels.

Re: [PATCH net-next v5 1/2] net: Use 16bits for *_headers fields of struct skbuff

[Olof Johansson]

Simon,

On Thu, May 23, 2013 at 11:51 PM, Simon Horman <horms@verge.net.au> wrote:
> In order to mitigate ongoing incresase in the size of struct skbuff
> use 16 bit integer offsets rather than pointers for inner_*_headers.
>
> This appears to reduce the size of struct skbuff from 0xd0 to 0xc0
> bytes on x86_64 with the following all unset.
>
>         CONFIG_XFRM
>         CONFIG_NF_CONNTRACK
>         CONFIG_NF_CONNTRACK_MODULE
>         NET_SKBUFF_NF_DEFRAG_NEEDED
>         CONFIG_BRIDGE_NETFILTER
>         CONFIG_NET_SCHED
>         CONFIG_IPV6_NDISC_NODETYPE
>         CONFIG_NET_DMA
>         CONFIG_NETWORK_SECMARK
>
> Signed-off-by: Simon Horman <horms@verge.net.au>

I'm getting crashes in csum_partial() on several ARM platforms that I
bisected down to this patch (and reverting this + "MPLS:
Add limited GSO support" due to conflicts) results in a working kernel.

The failures started with the 0529 linux-next, didn't exist in 0528.

Unfortunately I'm not getting a useful stack from the crashes:

[    6.495560] Unable to handle kernel paging request at virtual
address eb000000
[    6.502769] pgd = e9084000
[    6.505465] [eb000000] *pgd=00000000
[    6.509034] Internal error: Oops: 5 [#1] PREEMPT SMP ARM
[    6.514329] Modules linked in:
[    6.517378] CPU: 0 PID: 0 Comm: swapper/0 Not tainted
3.10.0-rc2-00483-g1a37e41 #27
[    6.525015] task: c06e1980 ti: c06d6000 task.ti: c06d6000
[    6.530402] PC is at csum_partial+0x40/0x130
[    6.534658] LR is at 0x0
[    6.537181] pc : [<c01e3150>]    lr : [<00000000>]    psr: 000f0113
[    6.537181] sp : c06d7d88  ip : e700c060  fp : c06d80c0
[    6.548631] r10: e90e2140  r9 : e9006050  r8 : 00000001
[    6.553839] r7 : e9006020  r6 : e9b7e400  r5 : 00000000  r4 : 00000000
[    6.560348] r3 : 00000000  r2 : 73129f9e  r1 : e900601c  r0 : eb000000
[    6.566857] Flags: nzcv  IRQs on  FIQs on  Mode SVC_32  ISA ARM
Segment kernel
[    6.574146] Control: 10c5387d  Table: 2908404a  DAC: 00000015
[    6.579874] Process swapper/0 (pid: 0, stack limit = 0xc06d6238)
[    6.585862] Stack: (0xc06d7d88 to 0xc06d8000)
[    6.590207] 7d80:                   c0714fc0 e900601c e9006050
c043a0b8 00000000 e92737b0
[    6.598365] 7da0: e9273740 c043c1d4 c0010208 c0f83cc0 c06d4cc0
c0f83cc0 00000000 e9b32000
[    6.606523] 7dc0: c06d4cc0 00000001 c06d80c0 e9273758 00989680
00000000 00000001 e9b7aec0
[    6.614680] 7de0: 00000000 e90e2140 e9b7af30 e9b7e400 e9b7e490
00000000 00000001 c043c5d4
[    6.622838] 7e00: 00000001 7cdfa980 00000001 e9b7e458 c06d6000
00000101 c06d6030 c043c414
[    6.630995] 7e20: 00200200 00000000 c06d80c0 c002fbe8 00000002
7c4716e8 e9b7e458 c073fb80
[    6.639153] 7e40: c043c414 e9b7e400 c06d7e68 c002fde8 00000000
c0f82678 00000000 c0740394
[    6.647310] 7e60: c0740594 c0740794 c06d7e68 c06d7e68 00000006
00000001 00000004 c06d8088
[    6.655468] 7e80: c06d8080 c06d6000 00000001 00000004 00000101
c002a57c 7c4716e8 00000001
[    6.663625] 7ea0: c06d37e0 00000000 c073f940 c06d80c0 ffff8d4f
c06e4970 00200000 c06d6018
[    6.671783] 7ec0: c02025e8 c06d6028 0000001d 00000000 fe000100
00000000 00000000 c06d6000
[    6.679940] 7ee0: c06de42c c002a9a4 c06d3fac c000eaa0 fe00010c
c06df074 c06d7f18 c00086f8
[    6.688098] 7f00: c0061288 c032b560 600f0013 ffffffff c06d7f4c
c000de60 c06d7f60 00000006
[    6.696255] 7f20: 7c471300 00000001 7c3bbca8 00000001 c0f83110
c06e3cd4 00000000 00000000
[    6.704413] 7f40: c06d6000 c06de42c 00000015 c06d7f60 c0061288
c032b560 600f0013 ffffffff
[    6.712571] 7f60: 7c471300 00000001 c0765ac8 c04fe16c c0f83110
c0765ac8 00000000 c06e3cd4
[    6.720728] 7f80: 00000000 c032b6b8 00004c9c c071ddc7 c06de490
c04fe16c c06d6000 c071ddc7
[    6.728886] 7fa0: c06d6000 c000edb4 00004c9c c0060b3c c04fe5fc
c0f7f9c0 00000000 c069f828
[    6.737043] 7fc0: ffffffff ffffffff c069f2e8 00000000 00000000
c06cbac0 00000000 10c5387d
[    6.745200] 7fe0: c06de3f4 c06cbabc c06e28e4 0000406a 411fc090
00008074 00000000 00000000
[    6.753359] Code: e0b22003 e0b22004 e0b22005 e0b2200e (e8b04038)
[    6.759438] ---[ end trace 40c34f89615c2c53 ]---
[    6.764041] Kernel panic - not syncing: Fatal exception in interrupt
[    6.770386] CPU1: stopping
[    6.773086] CPU: 1 PID: 0 Comm: swapper/1 Tainted: G      D
3.10.0-rc2-00483-g1a37e41 #27


-Olof

Re: [PATCH net-next v5 1/2] net: Use 16bits for *_headers fields of struct skbuff

[Olof Johansson]

Sorry, that was not a great bug report.

So, looks like it's the removal of NET_SKBUFF_DATA_USES_OFFSET that
does it for me.

The devices I've seen it with are with asix usb-ethernet adapters (on
Tegra seaboard) and with mv643xx_eth on cubox (dove).

I'll try to get a better stacktrace out of it. Simon, maybe you can
reproduce on shmobile hardware?



On Wed, May 29, 2013 at 2:21 PM, Olof Johansson <olof-nZhT3qVonbNeoWH0uzbU5w@public.gmane.org> wrote:
> Simon,
>
> On Thu, May 23, 2013 at 11:51 PM, Simon Horman <horms-/R6kz+dDXgpPR4JQBCEnsQ@public.gmane.org> wrote:
>> In order to mitigate ongoing incresase in the size of struct skbuff
>> use 16 bit integer offsets rather than pointers for inner_*_headers.
>>
>> This appears to reduce the size of struct skbuff from 0xd0 to 0xc0
>> bytes on x86_64 with the following all unset.
>>
>>         CONFIG_XFRM
>>         CONFIG_NF_CONNTRACK
>>         CONFIG_NF_CONNTRACK_MODULE
>>         NET_SKBUFF_NF_DEFRAG_NEEDED
>>         CONFIG_BRIDGE_NETFILTER
>>         CONFIG_NET_SCHED
>>         CONFIG_IPV6_NDISC_NODETYPE
>>         CONFIG_NET_DMA
>>         CONFIG_NETWORK_SECMARK
>>
>> Signed-off-by: Simon Horman <horms-/R6kz+dDXgpPR4JQBCEnsQ@public.gmane.org>
>
> I'm getting crashes in csum_partial() on several ARM platforms that I
> bisected down to this patch (and reverting this + "MPLS:
> Add limited GSO support" due to conflicts) results in a working kernel.
>
> The failures started with the 0529 linux-next, didn't exist in 0528.
>
> Unfortunately I'm not getting a useful stack from the crashes:
>
> [    6.495560] Unable to handle kernel paging request at virtual
> address eb000000
> [    6.502769] pgd = e9084000
> [    6.505465] [eb000000] *pgd=00000000
> [    6.509034] Internal error: Oops: 5 [#1] PREEMPT SMP ARM
> [    6.514329] Modules linked in:
> [    6.517378] CPU: 0 PID: 0 Comm: swapper/0 Not tainted
> 3.10.0-rc2-00483-g1a37e41 #27
> [    6.525015] task: c06e1980 ti: c06d6000 task.ti: c06d6000
> [    6.530402] PC is at csum_partial+0x40/0x130
> [    6.534658] LR is at 0x0
> [    6.537181] pc : [<c01e3150>]    lr : [<00000000>]    psr: 000f0113
> [    6.537181] sp : c06d7d88  ip : e700c060  fp : c06d80c0
> [    6.548631] r10: e90e2140  r9 : e9006050  r8 : 00000001
> [    6.553839] r7 : e9006020  r6 : e9b7e400  r5 : 00000000  r4 : 00000000
> [    6.560348] r3 : 00000000  r2 : 73129f9e  r1 : e900601c  r0 : eb000000
> [    6.566857] Flags: nzcv  IRQs on  FIQs on  Mode SVC_32  ISA ARM
> Segment kernel
> [    6.574146] Control: 10c5387d  Table: 2908404a  DAC: 00000015
> [    6.579874] Process swapper/0 (pid: 0, stack limit = 0xc06d6238)
> [    6.585862] Stack: (0xc06d7d88 to 0xc06d8000)
> [    6.590207] 7d80:                   c0714fc0 e900601c e9006050
> c043a0b8 00000000 e92737b0
> [    6.598365] 7da0: e9273740 c043c1d4 c0010208 c0f83cc0 c06d4cc0
> c0f83cc0 00000000 e9b32000
> [    6.606523] 7dc0: c06d4cc0 00000001 c06d80c0 e9273758 00989680
> 00000000 00000001 e9b7aec0
> [    6.614680] 7de0: 00000000 e90e2140 e9b7af30 e9b7e400 e9b7e490
> 00000000 00000001 c043c5d4
> [    6.622838] 7e00: 00000001 7cdfa980 00000001 e9b7e458 c06d6000
> 00000101 c06d6030 c043c414
> [    6.630995] 7e20: 00200200 00000000 c06d80c0 c002fbe8 00000002
> 7c4716e8 e9b7e458 c073fb80
> [    6.639153] 7e40: c043c414 e9b7e400 c06d7e68 c002fde8 00000000
> c0f82678 00000000 c0740394
> [    6.647310] 7e60: c0740594 c0740794 c06d7e68 c06d7e68 00000006
> 00000001 00000004 c06d8088
> [    6.655468] 7e80: c06d8080 c06d6000 00000001 00000004 00000101
> c002a57c 7c4716e8 00000001
> [    6.663625] 7ea0: c06d37e0 00000000 c073f940 c06d80c0 ffff8d4f
> c06e4970 00200000 c06d6018
> [    6.671783] 7ec0: c02025e8 c06d6028 0000001d 00000000 fe000100
> 00000000 00000000 c06d6000
> [    6.679940] 7ee0: c06de42c c002a9a4 c06d3fac c000eaa0 fe00010c
> c06df074 c06d7f18 c00086f8
> [    6.688098] 7f00: c0061288 c032b560 600f0013 ffffffff c06d7f4c
> c000de60 c06d7f60 00000006
> [    6.696255] 7f20: 7c471300 00000001 7c3bbca8 00000001 c0f83110
> c06e3cd4 00000000 00000000
> [    6.704413] 7f40: c06d6000 c06de42c 00000015 c06d7f60 c0061288
> c032b560 600f0013 ffffffff
> [    6.712571] 7f60: 7c471300 00000001 c0765ac8 c04fe16c c0f83110
> c0765ac8 00000000 c06e3cd4
> [    6.720728] 7f80: 00000000 c032b6b8 00004c9c c071ddc7 c06de490
> c04fe16c c06d6000 c071ddc7
> [    6.728886] 7fa0: c06d6000 c000edb4 00004c9c c0060b3c c04fe5fc
> c0f7f9c0 00000000 c069f828
> [    6.737043] 7fc0: ffffffff ffffffff c069f2e8 00000000 00000000
> c06cbac0 00000000 10c5387d
> [    6.745200] 7fe0: c06de3f4 c06cbabc c06e28e4 0000406a 411fc090
> 00008074 00000000 00000000
> [    6.753359] Code: e0b22003 e0b22004 e0b22005 e0b2200e (e8b04038)
> [    6.759438] ---[ end trace 40c34f89615c2c53 ]---
> [    6.764041] Kernel panic - not syncing: Fatal exception in interrupt
> [    6.770386] CPU1: stopping
> [    6.773086] CPU: 1 PID: 0 Comm: swapper/1 Tainted: G      D
> 3.10.0-rc2-00483-g1a37e41 #27
>
>
> -Olof

Re: [PATCH net-next v5 1/2] net: Use 16bits for *_headers fields of struct skbuff

[Simon Horman]

On Wed, May 29, 2013 at 02:46:05PM -0700, Olof Johansson wrote:
> Sorry, that was not a great bug report.
> 
> So, looks like it's the removal of NET_SKBUFF_DATA_USES_OFFSET that
> does it for me.
> 
> The devices I've seen it with are with asix usb-ethernet adapters (on
> Tegra seaboard) and with mv643xx_eth on cubox (dove).
> 
> I'll try to get a better stacktrace out of it. Simon, maybe you can
> reproduce on shmobile hardware?

Sure, I will try and reproduce it.

I did subsequently post some fixes for fallout resulting from
the patch listed below. Those changes are now net-next.
Are you seeing the crash with those patches?

The fix-up patches are:

net,ipv4,ipv6: Correct assignment of skb->network_header to skb->tail
sctp: Correct access to skb->{network,transport}_header
ipv4: Correct comparisons and calculations using skb->tail and skb-transport_header
ipv6: Correct comparisons and calculations using skb->tail and skb-transport_header
net: Correct comparisons and calculations using skb->tail and skb-transport_header
cxgb3: Correct comparisons and calculations using skb->tail and skb-transport_header
isdn: Correct comparison of skb->tail and skb-transport_header
net: Fix build warnings after mac_header and transport_header became __u16.


> 
> 
> 
> On Wed, May 29, 2013 at 2:21 PM, Olof Johansson <olof@lixom.net> wrote:
> > Simon,
> >
> > On Thu, May 23, 2013 at 11:51 PM, Simon Horman <horms@verge.net.au> wrote:
> >> In order to mitigate ongoing incresase in the size of struct skbuff
> >> use 16 bit integer offsets rather than pointers for inner_*_headers.
> >>
> >> This appears to reduce the size of struct skbuff from 0xd0 to 0xc0
> >> bytes on x86_64 with the following all unset.
> >>
> >>         CONFIG_XFRM
> >>         CONFIG_NF_CONNTRACK
> >>         CONFIG_NF_CONNTRACK_MODULE
> >>         NET_SKBUFF_NF_DEFRAG_NEEDED
> >>         CONFIG_BRIDGE_NETFILTER
> >>         CONFIG_NET_SCHED
> >>         CONFIG_IPV6_NDISC_NODETYPE
> >>         CONFIG_NET_DMA
> >>         CONFIG_NETWORK_SECMARK
> >>
> >> Signed-off-by: Simon Horman <horms@verge.net.au>
> >
> > I'm getting crashes in csum_partial() on several ARM platforms that I
> > bisected down to this patch (and reverting this + "MPLS:
> > Add limited GSO support" due to conflicts) results in a working kernel.
> >
> > The failures started with the 0529 linux-next, didn't exist in 0528.
> >
> > Unfortunately I'm not getting a useful stack from the crashes:
> >
> > [    6.495560] Unable to handle kernel paging request at virtual
> > address eb000000
> > [    6.502769] pgd = e9084000
> > [    6.505465] [eb000000] *pgd=00000000
> > [    6.509034] Internal error: Oops: 5 [#1] PREEMPT SMP ARM
> > [    6.514329] Modules linked in:
> > [    6.517378] CPU: 0 PID: 0 Comm: swapper/0 Not tainted
> > 3.10.0-rc2-00483-g1a37e41 #27
> > [    6.525015] task: c06e1980 ti: c06d6000 task.ti: c06d6000
> > [    6.530402] PC is at csum_partial+0x40/0x130
> > [    6.534658] LR is at 0x0
> > [    6.537181] pc : [<c01e3150>]    lr : [<00000000>]    psr: 000f0113
> > [    6.537181] sp : c06d7d88  ip : e700c060  fp : c06d80c0
> > [    6.548631] r10: e90e2140  r9 : e9006050  r8 : 00000001
> > [    6.553839] r7 : e9006020  r6 : e9b7e400  r5 : 00000000  r4 : 00000000
> > [    6.560348] r3 : 00000000  r2 : 73129f9e  r1 : e900601c  r0 : eb000000
> > [    6.566857] Flags: nzcv  IRQs on  FIQs on  Mode SVC_32  ISA ARM
> > Segment kernel
> > [    6.574146] Control: 10c5387d  Table: 2908404a  DAC: 00000015
> > [    6.579874] Process swapper/0 (pid: 0, stack limit = 0xc06d6238)
> > [    6.585862] Stack: (0xc06d7d88 to 0xc06d8000)
> > [    6.590207] 7d80:                   c0714fc0 e900601c e9006050
> > c043a0b8 00000000 e92737b0
> > [    6.598365] 7da0: e9273740 c043c1d4 c0010208 c0f83cc0 c06d4cc0
> > c0f83cc0 00000000 e9b32000
> > [    6.606523] 7dc0: c06d4cc0 00000001 c06d80c0 e9273758 00989680
> > 00000000 00000001 e9b7aec0
> > [    6.614680] 7de0: 00000000 e90e2140 e9b7af30 e9b7e400 e9b7e490
> > 00000000 00000001 c043c5d4
> > [    6.622838] 7e00: 00000001 7cdfa980 00000001 e9b7e458 c06d6000
> > 00000101 c06d6030 c043c414
> > [    6.630995] 7e20: 00200200 00000000 c06d80c0 c002fbe8 00000002
> > 7c4716e8 e9b7e458 c073fb80
> > [    6.639153] 7e40: c043c414 e9b7e400 c06d7e68 c002fde8 00000000
> > c0f82678 00000000 c0740394
> > [    6.647310] 7e60: c0740594 c0740794 c06d7e68 c06d7e68 00000006
> > 00000001 00000004 c06d8088
> > [    6.655468] 7e80: c06d8080 c06d6000 00000001 00000004 00000101
> > c002a57c 7c4716e8 00000001
> > [    6.663625] 7ea0: c06d37e0 00000000 c073f940 c06d80c0 ffff8d4f
> > c06e4970 00200000 c06d6018
> > [    6.671783] 7ec0: c02025e8 c06d6028 0000001d 00000000 fe000100
> > 00000000 00000000 c06d6000
> > [    6.679940] 7ee0: c06de42c c002a9a4 c06d3fac c000eaa0 fe00010c
> > c06df074 c06d7f18 c00086f8
> > [    6.688098] 7f00: c0061288 c032b560 600f0013 ffffffff c06d7f4c
> > c000de60 c06d7f60 00000006
> > [    6.696255] 7f20: 7c471300 00000001 7c3bbca8 00000001 c0f83110
> > c06e3cd4 00000000 00000000
> > [    6.704413] 7f40: c06d6000 c06de42c 00000015 c06d7f60 c0061288
> > c032b560 600f0013 ffffffff
> > [    6.712571] 7f60: 7c471300 00000001 c0765ac8 c04fe16c c0f83110
> > c0765ac8 00000000 c06e3cd4
> > [    6.720728] 7f80: 00000000 c032b6b8 00004c9c c071ddc7 c06de490
> > c04fe16c c06d6000 c071ddc7
> > [    6.728886] 7fa0: c06d6000 c000edb4 00004c9c c0060b3c c04fe5fc
> > c0f7f9c0 00000000 c069f828
> > [    6.737043] 7fc0: ffffffff ffffffff c069f2e8 00000000 00000000
> > c06cbac0 00000000 10c5387d
> > [    6.745200] 7fe0: c06de3f4 c06cbabc c06e28e4 0000406a 411fc090
> > 00008074 00000000 00000000
> > [    6.753359] Code: e0b22003 e0b22004 e0b22005 e0b2200e (e8b04038)
> > [    6.759438] ---[ end trace 40c34f89615c2c53 ]---
> > [    6.764041] Kernel panic - not syncing: Fatal exception in interrupt
> > [    6.770386] CPU1: stopping
> > [    6.773086] CPU: 1 PID: 0 Comm: swapper/1 Tainted: G      D
> > 3.10.0-rc2-00483-g1a37e41 #27
> >
> >
> > -Olof
> 

Re: [PATCH net-next v5 1/2] net: Use 16bits for *_headers fields of struct skbuff

[Olof Johansson]

Hi,

On Wed, May 29, 2013 at 3:54 PM, Simon Horman <horms@verge.net.au> wrote:
> On Wed, May 29, 2013 at 02:46:05PM -0700, Olof Johansson wrote:
>> Sorry, that was not a great bug report.
>>
>> So, looks like it's the removal of NET_SKBUFF_DATA_USES_OFFSET that
>> does it for me.
>>
>> The devices I've seen it with are with asix usb-ethernet adapters (on
>> Tegra seaboard) and with mv643xx_eth on cubox (dove).
>>
>> I'll try to get a better stacktrace out of it. Simon, maybe you can
>> reproduce on shmobile hardware?
>
> Sure, I will try and reproduce it.
>
> I did subsequently post some fixes for fallout resulting from
> the patch listed below. Those changes are now net-next.
> Are you seeing the crash with those patches?
>
> The fix-up patches are:
>
> net,ipv4,ipv6: Correct assignment of skb->network_header to skb->tail
> sctp: Correct access to skb->{network,transport}_header
> ipv4: Correct comparisons and calculations using skb->tail and skb-transport_header
> ipv6: Correct comparisons and calculations using skb->tail and skb-transport_header
> net: Correct comparisons and calculations using skb->tail and skb-transport_header
> cxgb3: Correct comparisons and calculations using skb->tail and skb-transport_header
> isdn: Correct comparison of skb->tail and skb-transport_header
> net: Fix build warnings after mac_header and transport_header became __u16.

Ah, no, looks like those didn't make it into the 0529 linux-next.

I've merged net-next on top and retested, and I no longer see the panics.


Thanks!


-Olof

Re: [PATCH net-next v5 1/2] net: Use 16bits for *_headers fields of struct skbuff

[Simon Horman]

On Wed, May 29, 2013 at 04:02:12PM -0700, Olof Johansson wrote:
> Hi,
> 
> On Wed, May 29, 2013 at 3:54 PM, Simon Horman <horms@verge.net.au> wrote:
> > On Wed, May 29, 2013 at 02:46:05PM -0700, Olof Johansson wrote:
> >> Sorry, that was not a great bug report.
> >>
> >> So, looks like it's the removal of NET_SKBUFF_DATA_USES_OFFSET that
> >> does it for me.
> >>
> >> The devices I've seen it with are with asix usb-ethernet adapters (on
> >> Tegra seaboard) and with mv643xx_eth on cubox (dove).
> >>
> >> I'll try to get a better stacktrace out of it. Simon, maybe you can
> >> reproduce on shmobile hardware?
> >
> > Sure, I will try and reproduce it.
> >
> > I did subsequently post some fixes for fallout resulting from
> > the patch listed below. Those changes are now net-next.
> > Are you seeing the crash with those patches?
> >
> > The fix-up patches are:
> >
> > net,ipv4,ipv6: Correct assignment of skb->network_header to skb->tail
> > sctp: Correct access to skb->{network,transport}_header
> > ipv4: Correct comparisons and calculations using skb->tail and skb-transport_header
> > ipv6: Correct comparisons and calculations using skb->tail and skb-transport_header
> > net: Correct comparisons and calculations using skb->tail and skb-transport_header
> > cxgb3: Correct comparisons and calculations using skb->tail and skb-transport_header
> > isdn: Correct comparison of skb->tail and skb-transport_header
> > net: Fix build warnings after mac_header and transport_header became __u16.
> 
> Ah, no, looks like those didn't make it into the 0529 linux-next.
> 
> I've merged net-next on top and retested, and I no longer see the panics.

Great news, sorry for the mess.

FWIW I did not have any luck reproducing the problem (without the fix-ups)
on the mackerel board.

[PATCH net-next v5 1/2] net: Use 16bits for *_headers fields of struct skbuff

[Simon Horman]

In order to mitigate ongoing incresase in the size of struct skbuff
use 16 bit integer offsets rather than pointers for inner_*_headers.

This appears to reduce the size of struct skbuff from 0xd0 to 0xc0
bytes on x86_64 with the following all unset.

	CONFIG_XFRM
	CONFIG_NF_CONNTRACK
	CONFIG_NF_CONNTRACK_MODULE
	NET_SKBUFF_NF_DEFRAG_NEEDED
	CONFIG_BRIDGE_NETFILTER
	CONFIG_NET_SCHED
	CONFIG_IPV6_NDISC_NODETYPE
	CONFIG_NET_DMA
	CONFIG_NETWORK_SECMARK

Signed-off-by: Simon Horman <horms-/R6kz+dDXgpPR4JQBCEnsQ@public.gmane.org>
---
 include/linux/skbuff.h | 119 +++----------------------------------------------
 1 file changed, 6 insertions(+), 113 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 2e0ced1..5663e35 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -509,12 +509,12 @@ struct sk_buff {
 		__u32		reserved_tailroom;
 	};
 
-	sk_buff_data_t		inner_transport_header;
-	sk_buff_data_t		inner_network_header;
-	sk_buff_data_t		inner_mac_header;
-	sk_buff_data_t		transport_header;
-	sk_buff_data_t		network_header;
-	sk_buff_data_t		mac_header;
+	__u16			inner_transport_header;
+	__u16			inner_network_header;
+	__u16			inner_mac_header;
+	__u16			transport_header;
+	__u16			network_header;
+	__u16			mac_header;
 	/* These elements must be at the end, see alloc_skb() for details.  */
 	sk_buff_data_t		tail;
 	sk_buff_data_t		end;
@@ -1527,7 +1527,6 @@ static inline void skb_reset_mac_len(struct sk_buff *skb)
 	skb->mac_len = skb->network_header - skb->mac_header;
 }
 
-#ifdef NET_SKBUFF_DATA_USES_OFFSET
 static inline unsigned char *skb_inner_transport_header(const struct sk_buff
 							*skb)
 {
@@ -1638,112 +1637,6 @@ static inline void skb_set_mac_header(struct sk_buff *skb, const int offset)
 	skb->mac_header += offset;
 }
 
-#else /* NET_SKBUFF_DATA_USES_OFFSET */
-static inline unsigned char *skb_inner_transport_header(const struct sk_buff
-							*skb)
-{
-	return skb->inner_transport_header;
-}
-
-static inline void skb_reset_inner_transport_header(struct sk_buff *skb)
-{
-	skb->inner_transport_header = skb->data;
-}
-
-static inline void skb_set_inner_transport_header(struct sk_buff *skb,
-						   const int offset)
-{
-	skb->inner_transport_header = skb->data + offset;
-}
-
-static inline unsigned char *skb_inner_network_header(const struct sk_buff *skb)
-{
-	return skb->inner_network_header;
-}
-
-static inline void skb_reset_inner_network_header(struct sk_buff *skb)
-{
-	skb->inner_network_header = skb->data;
-}
-
-static inline void skb_set_inner_network_header(struct sk_buff *skb,
-						const int offset)
-{
-	skb->inner_network_header = skb->data + offset;
-}
-
-static inline unsigned char *skb_inner_mac_header(const struct sk_buff *skb)
-{
-	return skb->inner_mac_header;
-}
-
-static inline void skb_reset_inner_mac_header(struct sk_buff *skb)
-{
-	skb->inner_mac_header = skb->data;
-}
-
-static inline void skb_set_inner_mac_header(struct sk_buff *skb,
-						const int offset)
-{
-	skb->inner_mac_header = skb->data + offset;
-}
-static inline bool skb_transport_header_was_set(const struct sk_buff *skb)
-{
-	return skb->transport_header != NULL;
-}
-
-static inline unsigned char *skb_transport_header(const struct sk_buff *skb)
-{
-	return skb->transport_header;
-}
-
-static inline void skb_reset_transport_header(struct sk_buff *skb)
-{
-	skb->transport_header = skb->data;
-}
-
-static inline void skb_set_transport_header(struct sk_buff *skb,
-					    const int offset)
-{
-	skb->transport_header = skb->data + offset;
-}
-
-static inline unsigned char *skb_network_header(const struct sk_buff *skb)
-{
-	return skb->network_header;
-}
-
-static inline void skb_reset_network_header(struct sk_buff *skb)
-{
-	skb->network_header = skb->data;
-}
-
-static inline void skb_set_network_header(struct sk_buff *skb, const int offset)
-{
-	skb->network_header = skb->data + offset;
-}
-
-static inline unsigned char *skb_mac_header(const struct sk_buff *skb)
-{
-	return skb->mac_header;
-}
-
-static inline int skb_mac_header_was_set(const struct sk_buff *skb)
-{
-	return skb->mac_header != NULL;
-}
-
-static inline void skb_reset_mac_header(struct sk_buff *skb)
-{
-	skb->mac_header = skb->data;
-}
-
-static inline void skb_set_mac_header(struct sk_buff *skb, const int offset)
-{
-	skb->mac_header = skb->data + offset;
-}
-#endif /* NET_SKBUFF_DATA_USES_OFFSET */
-
 static inline void skb_probe_transport_header(struct sk_buff *skb,
 					      const int offset_hint)
 {
-- 
1.8.2.1