[PATCH net-next RFC 0/8] udp and configurable gro

[PATCH net-next RFC 0/8] udp and configurable gro

[Willem de Bruijn]

From: Willem de Bruijn <willemb@google.com>

This is a *very rough* draft. Mainly for discussion while we also
look at another partially overlapping approach [1].

Reduce UDP receive cost for bulk traffic by enabling datagram
coalescing with GRO.

Before adding more GRO callbacks, make GRO configurable by the
administrator to optionally reduce the attack surface of this
early receive path. See also [2].

Introduce sysctls net.(core|ipv4|ipv6).gro that expose the table of
protocols for which GRO is support. Allow the administrator to disable
individual entries in the table.

To have a single infrastructure, convert dev_offloads to the
table-based approach to existing inet(6)_offloads. Additional small
benefit is that ipv6 will no longer take two list lookups to find.

Patch 1 converts dev_offloads to the infra of inet(6)_offloads
Patch 2 deduplicates gro_complete logic now that all share infra
Patch 3 does the same for gro_receive, in anticipation of adding
        a branch to check whether gro_receive is enabled
Patch 4 harmonizes ipv6 header opts, so that those, too can be
        optionally disabled.
Patch 5 makes inet(6)_offloads non-const to allow disabling a flag
Patch 6 introduces the administrative sysctl
Patch 7 avoids udp gro cost if no udp gro callback is register
Patch 8 introduces udp gro

[1] http://patchwork.ozlabs.org/project/netdev/list/?series=65741
[2] http://vger.kernel.org/netconf2017_files/rx_hardening_and_udp_gso.pdf

Willem de Bruijn (8):
  gro: convert device offloads to net_offload
  gro: deduplicate gro_complete
  gro: add net_gro_receive
  ipv6: remove offload exception for hopopts
  net: deconstify net_offload
  net: make gro configurable
  udp: gro behind static key
  udp: add gro

 drivers/net/geneve.c       |  11 +---
 drivers/net/vxlan.c        |   8 +++
 include/linux/netdevice.h  |  64 +++++++++++++++++++--
 include/net/protocol.h     |  19 ++-----
 include/net/udp.h          |   2 +
 include/uapi/linux/udp.h   |   1 +
 net/8021q/vlan.c           |  12 +---
 net/core/dev.c             | 112 ++++++++-----------------------------
 net/core/sysctl_net_core.c |  60 ++++++++++++++++++++
 net/ethernet/eth.c         |  13 +----
 net/ipv4/af_inet.c         |  21 ++-----
 net/ipv4/esp4_offload.c    |   2 +-
 net/ipv4/fou.c             |  41 ++++----------
 net/ipv4/gre_offload.c     |  26 ++++-----
 net/ipv4/protocol.c        |  10 ++--
 net/ipv4/sysctl_net_ipv4.c |   7 +++
 net/ipv4/tcp_offload.c     |   2 +-
 net/ipv4/udp.c             |  73 +++++++++++++++++++++++-
 net/ipv4/udp_offload.c     |  19 +++----
 net/ipv6/esp6_offload.c    |   2 +-
 net/ipv6/exthdrs_offload.c |  17 +++++-
 net/ipv6/ip6_offload.c     |  69 +++++++++--------------
 net/ipv6/protocol.c        |  10 ++--
 net/ipv6/sysctl_net_ipv6.c |   8 +++
 net/ipv6/tcpv6_offload.c   |   2 +-
 net/ipv6/udp.c             |   2 +-
 net/ipv6/udp_offload.c     |   4 +-
 net/sctp/offload.c         |   2 +-
 28 files changed, 344 insertions(+), 275 deletions(-)

-- 
2.19.0.397.gdd90340f6a-goog

[PATCH net-next RFC 1/8] gro: convert device offloads to net_offload

[Willem de Bruijn]

From: Willem de Bruijn <willemb@google.com>

In preparation of making GRO receive configurable, have all offloads
share the same infrastructure.

Signed-off-by: Willem de Bruijn <willemb@google.com>
---
 include/linux/netdevice.h |  17 +++++-
 include/net/protocol.h    |   7 ---
 net/core/dev.c            | 105 +++++++++++++-------------------------
 3 files changed, 51 insertions(+), 78 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index e2b3bd750c98..7425068fa249 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2366,13 +2366,18 @@ struct offload_callbacks {
 	int			(*gro_complete)(struct sk_buff *skb, int nhoff);
 };
 
-struct packet_offload {
+struct net_offload {
 	__be16			 type;	/* This is really htons(ether_type). */
 	u16			 priority;
 	struct offload_callbacks callbacks;
-	struct list_head	 list;
+	unsigned int		 flags;	/* Flags used by IPv6 for now */
 };
 
+#define packet_offload	net_offload
+
+/* This should be set for any extension header which is compatible with GSO. */
+#define INET6_PROTO_GSO_EXTHDR	0x1
+
 /* often modified stats are per-CPU, other are shared (netdev->stats) */
 struct pcpu_sw_netstats {
 	u64     rx_packets;
@@ -3554,6 +3559,14 @@ gro_result_t napi_gro_frags(struct napi_struct *napi);
 struct packet_offload *gro_find_receive_by_type(__be16 type);
 struct packet_offload *gro_find_complete_by_type(__be16 type);
 
+static inline u8 net_offload_from_type(u16 type)
+{
+	/* Do not bother handling collisions. There are none.
+	 * If they do occur with new offloads, add a mapping function here.
+	 */
+	return type & 0xFF;
+}
+
 static inline void napi_free_frags(struct napi_struct *napi)
 {
 	kfree_skb(napi->skb);
diff --git a/include/net/protocol.h b/include/net/protocol.h
index 4fc75f7ae23b..53a0322ee545 100644
--- a/include/net/protocol.h
+++ b/include/net/protocol.h
@@ -69,13 +69,6 @@ struct inet6_protocol {
 #define INET6_PROTO_FINAL	0x2
 #endif
 
-struct net_offload {
-	struct offload_callbacks callbacks;
-	unsigned int		 flags;	/* Flags used by IPv6 for now */
-};
-/* This should be set for any extension header which is compatible with GSO. */
-#define INET6_PROTO_GSO_EXTHDR	0x1
-
 /* This is used to register socket interfaces for IP protocols.  */
 struct inet_protosw {
 	struct list_head list;
diff --git a/net/core/dev.c b/net/core/dev.c
index 0b2d777e5b9e..55f86b6d3182 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -154,7 +154,6 @@
 #define GRO_MAX_HEAD (MAX_HEADER + 128)
 
 static DEFINE_SPINLOCK(ptype_lock);
-static DEFINE_SPINLOCK(offload_lock);
 struct list_head ptype_base[PTYPE_HASH_SIZE] __read_mostly;
 struct list_head ptype_all __read_mostly;	/* Taps */
 static struct list_head offload_base __read_mostly;
@@ -467,6 +466,9 @@ void dev_remove_pack(struct packet_type *pt)
 EXPORT_SYMBOL(dev_remove_pack);
 
 
+const struct net_offload __rcu *dev_offloads[256] __read_mostly;
+EXPORT_SYMBOL(dev_offloads);
+
 /**
  *	dev_add_offload - register offload handlers
  *	@po: protocol offload declaration
@@ -481,15 +483,9 @@ EXPORT_SYMBOL(dev_remove_pack);
  */
 void dev_add_offload(struct packet_offload *po)
 {
-	struct packet_offload *elem;
-
-	spin_lock(&offload_lock);
-	list_for_each_entry(elem, &offload_base, list) {
-		if (po->priority < elem->priority)
-			break;
-	}
-	list_add_rcu(&po->list, elem->list.prev);
-	spin_unlock(&offload_lock);
+	cmpxchg((const struct net_offload **)
+		&dev_offloads[net_offload_from_type(po->type)],
+			NULL, po);
 }
 EXPORT_SYMBOL(dev_add_offload);
 
@@ -506,23 +502,11 @@ EXPORT_SYMBOL(dev_add_offload);
  *	and must not be freed until after all the CPU's have gone
  *	through a quiescent state.
  */
-static void __dev_remove_offload(struct packet_offload *po)
+static int __dev_remove_offload(struct packet_offload *po)
 {
-	struct list_head *head = &offload_base;
-	struct packet_offload *po1;
-
-	spin_lock(&offload_lock);
-
-	list_for_each_entry(po1, head, list) {
-		if (po == po1) {
-			list_del_rcu(&po->list);
-			goto out;
-		}
-	}
-
-	pr_warn("dev_remove_offload: %p not found\n", po);
-out:
-	spin_unlock(&offload_lock);
+	return (cmpxchg((const struct net_offload **)
+			&dev_offloads[net_offload_from_type(po->type)],
+		       po, NULL) == po) ? 0 : -1;
 }
 
 /**
@@ -2962,7 +2946,7 @@ struct sk_buff *skb_mac_gso_segment(struct sk_buff *skb,
 				    netdev_features_t features)
 {
 	struct sk_buff *segs = ERR_PTR(-EPROTONOSUPPORT);
-	struct packet_offload *ptype;
+	const struct net_offload *off;
 	int vlan_depth = skb->mac_len;
 	__be16 type = skb_network_protocol(skb, &vlan_depth);
 
@@ -2972,12 +2956,9 @@ struct sk_buff *skb_mac_gso_segment(struct sk_buff *skb,
 	__skb_pull(skb, vlan_depth);
 
 	rcu_read_lock();
-	list_for_each_entry_rcu(ptype, &offload_base, list) {
-		if (ptype->type == type && ptype->callbacks.gso_segment) {
-			segs = ptype->callbacks.gso_segment(skb, features);
-			break;
-		}
-	}
+	off = rcu_dereference(dev_offloads[net_offload_from_type(type)]);
+	if (off && off->type == type && off->callbacks.gso_segment)
+		segs = off->callbacks.gso_segment(skb, features);
 	rcu_read_unlock();
 
 	__skb_push(skb, skb->data - skb_mac_header(skb));
@@ -5254,9 +5235,8 @@ static void flush_all_backlogs(void)
 
 static int napi_gro_complete(struct sk_buff *skb)
 {
-	struct packet_offload *ptype;
+	const struct packet_offload *ptype;
 	__be16 type = skb->protocol;
-	struct list_head *head = &offload_base;
 	int err = -ENOENT;
 
 	BUILD_BUG_ON(sizeof(struct napi_gro_cb) > sizeof(skb->cb));
@@ -5267,17 +5247,12 @@ static int napi_gro_complete(struct sk_buff *skb)
 	}
 
 	rcu_read_lock();
-	list_for_each_entry_rcu(ptype, head, list) {
-		if (ptype->type != type || !ptype->callbacks.gro_complete)
-			continue;
-
+	ptype = dev_offloads[net_offload_from_type(type)];
+	if (ptype && ptype->callbacks.gro_complete)
 		err = ptype->callbacks.gro_complete(skb, 0);
-		break;
-	}
 	rcu_read_unlock();
 
 	if (err) {
-		WARN_ON(&ptype->list == head);
 		kfree_skb(skb);
 		return NET_RX_SUCCESS;
 	}
@@ -5417,8 +5392,7 @@ static void gro_flush_oldest(struct list_head *head)
 static enum gro_result dev_gro_receive(struct napi_struct *napi, struct sk_buff *skb)
 {
 	u32 hash = skb_get_hash_raw(skb) & (GRO_HASH_BUCKETS - 1);
-	struct list_head *head = &offload_base;
-	struct packet_offload *ptype;
+	const struct packet_offload *ptype;
 	__be16 type = skb->protocol;
 	struct list_head *gro_head;
 	struct sk_buff *pp = NULL;
@@ -5432,10 +5406,8 @@ static enum gro_result dev_gro_receive(struct napi_struct *napi, struct sk_buff
 	gro_head = gro_list_prepare(napi, skb);
 
 	rcu_read_lock();
-	list_for_each_entry_rcu(ptype, head, list) {
-		if (ptype->type != type || !ptype->callbacks.gro_receive)
-			continue;
-
+	ptype = dev_offloads[net_offload_from_type(type)];
+	if (ptype && ptype->callbacks.gro_receive) {
 		skb_set_network_header(skb, skb_gro_offset(skb));
 		skb_reset_mac_len(skb);
 		NAPI_GRO_CB(skb)->same_flow = 0;
@@ -5464,12 +5436,11 @@ static enum gro_result dev_gro_receive(struct napi_struct *napi, struct sk_buff
 		}
 
 		pp = ptype->callbacks.gro_receive(gro_head, skb);
-		break;
-	}
-	rcu_read_unlock();
-
-	if (&ptype->list == head)
+		rcu_read_unlock();
+	} else {
+		rcu_read_unlock();
 		goto normal;
+	}
 
 	if (IS_ERR(pp) && PTR_ERR(pp) == -EINPROGRESS) {
 		ret = GRO_CONSUMED;
@@ -5524,29 +5495,25 @@ static enum gro_result dev_gro_receive(struct napi_struct *napi, struct sk_buff
 
 struct packet_offload *gro_find_receive_by_type(__be16 type)
 {
-	struct list_head *offload_head = &offload_base;
-	struct packet_offload *ptype;
+	struct net_offload *off;
 
-	list_for_each_entry_rcu(ptype, offload_head, list) {
-		if (ptype->type != type || !ptype->callbacks.gro_receive)
-			continue;
-		return ptype;
-	}
-	return NULL;
+	off = (struct net_offload *) rcu_dereference(dev_offloads[type & 0xFF]);
+	if (off && off->type == type && off->callbacks.gro_receive)
+		return off;
+	else
+		return NULL;
 }
 EXPORT_SYMBOL(gro_find_receive_by_type);
 
 struct packet_offload *gro_find_complete_by_type(__be16 type)
 {
-	struct list_head *offload_head = &offload_base;
-	struct packet_offload *ptype;
+	struct net_offload *off;
 
-	list_for_each_entry_rcu(ptype, offload_head, list) {
-		if (ptype->type != type || !ptype->callbacks.gro_complete)
-			continue;
-		return ptype;
-	}
-	return NULL;
+	off = (struct net_offload *) rcu_dereference(dev_offloads[type & 0xFF]);
+	if (off && off->type == type && off->callbacks.gro_complete)
+		return off;
+	else
+		return NULL;
 }
 EXPORT_SYMBOL(gro_find_complete_by_type);
 
-- 
2.19.0.397.gdd90340f6a-goog

[PATCH net-next RFC 2/8] gro: deduplicate gro_complete

[Willem de Bruijn]

From: Willem de Bruijn <willemb@google.com>

The gro completion datapath is open coded for all protocols.
Deduplicate with new helper function net_gro_complete.

Signed-off-by: Willem de Bruijn <willemb@google.com>
---
 drivers/net/geneve.c      |  9 +--------
 include/linux/netdevice.h | 19 ++++++++++++++++++-
 net/8021q/vlan.c          | 10 +---------
 net/core/dev.c            | 24 +-----------------------
 net/ethernet/eth.c        | 11 +----------
 net/ipv4/af_inet.c        | 15 ++-------------
 net/ipv4/fou.c            | 25 +++----------------------
 net/ipv4/gre_offload.c    | 12 +++---------
 net/ipv6/ip6_offload.c    | 13 +------------
 9 files changed, 31 insertions(+), 107 deletions(-)

diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
index 6625fabe2c88..a3a4621d9bee 100644
--- a/drivers/net/geneve.c
+++ b/drivers/net/geneve.c
@@ -488,7 +488,6 @@ static int geneve_gro_complete(struct sock *sk, struct sk_buff *skb,
 			       int nhoff)
 {
 	struct genevehdr *gh;
-	struct packet_offload *ptype;
 	__be16 type;
 	int gh_len;
 	int err = -ENOSYS;
@@ -497,13 +496,7 @@ static int geneve_gro_complete(struct sock *sk, struct sk_buff *skb,
 	gh_len = geneve_hlen(gh);
 	type = gh->proto_type;
 
-	rcu_read_lock();
-	ptype = gro_find_complete_by_type(type);
-	if (ptype)
-		err = ptype->callbacks.gro_complete(skb, nhoff + gh_len);
-
-	rcu_read_unlock();
-
+	err = net_gro_complete(dev_offloads, type, skb, nhoff + gh_len);
 	skb_set_inner_mac_header(skb, nhoff + gh_len);
 
 	return err;
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 7425068fa249..0d292ea6716e 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3557,7 +3557,8 @@ void napi_gro_flush(struct napi_struct *napi, bool flush_old);
 struct sk_buff *napi_get_frags(struct napi_struct *napi);
 gro_result_t napi_gro_frags(struct napi_struct *napi);
 struct packet_offload *gro_find_receive_by_type(__be16 type);
-struct packet_offload *gro_find_complete_by_type(__be16 type);
+
+extern const struct net_offload __rcu *dev_offloads[256];
 
 static inline u8 net_offload_from_type(u16 type)
 {
@@ -3567,6 +3568,22 @@ static inline u8 net_offload_from_type(u16 type)
 	return type & 0xFF;
 }
 
+static inline int net_gro_complete(const struct net_offload __rcu **offs,
+				   u16 type, struct sk_buff *skb, int nhoff)
+{
+	const struct net_offload *off;
+	int ret = -ENOENT;
+
+	rcu_read_lock();
+	off = rcu_dereference(offs[net_offload_from_type(type)]);
+	if (off && off->callbacks.gro_complete &&
+	    (!off->type || off->type == type))
+		ret = off->callbacks.gro_complete(skb, nhoff);
+	rcu_read_unlock();
+
+	return ret;
+}
+
 static inline void napi_free_frags(struct napi_struct *napi)
 {
 	kfree_skb(napi->skb);
diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
index 5e9950453955..6ac27aa9f158 100644
--- a/net/8021q/vlan.c
+++ b/net/8021q/vlan.c
@@ -703,16 +703,8 @@ static int vlan_gro_complete(struct sk_buff *skb, int nhoff)
 {
 	struct vlan_hdr *vhdr = (struct vlan_hdr *)(skb->data + nhoff);
 	__be16 type = vhdr->h_vlan_encapsulated_proto;
-	struct packet_offload *ptype;
-	int err = -ENOENT;
 
-	rcu_read_lock();
-	ptype = gro_find_complete_by_type(type);
-	if (ptype)
-		err = ptype->callbacks.gro_complete(skb, nhoff + sizeof(*vhdr));
-
-	rcu_read_unlock();
-	return err;
+	return net_gro_complete(dev_offloads, type, skb, nhoff + sizeof(*vhdr));
 }
 
 static struct packet_offload vlan_packet_offloads[] __read_mostly = {
diff --git a/net/core/dev.c b/net/core/dev.c
index 55f86b6d3182..2c21e507291f 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5235,10 +5235,6 @@ static void flush_all_backlogs(void)
 
 static int napi_gro_complete(struct sk_buff *skb)
 {
-	const struct packet_offload *ptype;
-	__be16 type = skb->protocol;
-	int err = -ENOENT;
-
 	BUILD_BUG_ON(sizeof(struct napi_gro_cb) > sizeof(skb->cb));
 
 	if (NAPI_GRO_CB(skb)->count == 1) {
@@ -5246,13 +5242,7 @@ static int napi_gro_complete(struct sk_buff *skb)
 		goto out;
 	}
 
-	rcu_read_lock();
-	ptype = dev_offloads[net_offload_from_type(type)];
-	if (ptype && ptype->callbacks.gro_complete)
-		err = ptype->callbacks.gro_complete(skb, 0);
-	rcu_read_unlock();
-
-	if (err) {
+	if (net_gro_complete(dev_offloads, skb->protocol, skb, 0)) {
 		kfree_skb(skb);
 		return NET_RX_SUCCESS;
 	}
@@ -5505,18 +5495,6 @@ struct packet_offload *gro_find_receive_by_type(__be16 type)
 }
 EXPORT_SYMBOL(gro_find_receive_by_type);
 
-struct packet_offload *gro_find_complete_by_type(__be16 type)
-{
-	struct net_offload *off;
-
-	off = (struct net_offload *) rcu_dereference(dev_offloads[type & 0xFF]);
-	if (off && off->type == type && off->callbacks.gro_complete)
-		return off;
-	else
-		return NULL;
-}
-EXPORT_SYMBOL(gro_find_complete_by_type);
-
 static void napi_skb_free_stolen_head(struct sk_buff *skb)
 {
 	skb_dst_drop(skb);
diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c
index fd8faa0dfa61..fb17a13722e8 100644
--- a/net/ethernet/eth.c
+++ b/net/ethernet/eth.c
@@ -485,20 +485,11 @@ int eth_gro_complete(struct sk_buff *skb, int nhoff)
 {
 	struct ethhdr *eh = (struct ethhdr *)(skb->data + nhoff);
 	__be16 type = eh->h_proto;
-	struct packet_offload *ptype;
-	int err = -ENOSYS;
 
 	if (skb->encapsulation)
 		skb_set_inner_mac_header(skb, nhoff);
 
-	rcu_read_lock();
-	ptype = gro_find_complete_by_type(type);
-	if (ptype != NULL)
-		err = ptype->callbacks.gro_complete(skb, nhoff +
-						    sizeof(struct ethhdr));
-
-	rcu_read_unlock();
-	return err;
+	return net_gro_complete(dev_offloads, type, skb, nhoff + sizeof(*eh));
 }
 EXPORT_SYMBOL(eth_gro_complete);
 
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 1fbe2f815474..1b72ee4a7811 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -1560,9 +1560,7 @@ int inet_gro_complete(struct sk_buff *skb, int nhoff)
 {
 	__be16 newlen = htons(skb->len - nhoff);
 	struct iphdr *iph = (struct iphdr *)(skb->data + nhoff);
-	const struct net_offload *ops;
 	int proto = iph->protocol;
-	int err = -ENOSYS;
 
 	if (skb->encapsulation) {
 		skb_set_inner_protocol(skb, cpu_to_be16(ETH_P_IP));
@@ -1572,21 +1570,12 @@ int inet_gro_complete(struct sk_buff *skb, int nhoff)
 	csum_replace2(&iph->check, iph->tot_len, newlen);
 	iph->tot_len = newlen;
 
-	rcu_read_lock();
-	ops = rcu_dereference(inet_offloads[proto]);
-	if (WARN_ON(!ops || !ops->callbacks.gro_complete))
-		goto out_unlock;
-
 	/* Only need to add sizeof(*iph) to get to the next hdr below
 	 * because any hdr with option will have been flushed in
 	 * inet_gro_receive().
 	 */
-	err = ops->callbacks.gro_complete(skb, nhoff + sizeof(*iph));
-
-out_unlock:
-	rcu_read_unlock();
-
-	return err;
+	return net_gro_complete(inet_offloads, proto, skb,
+				nhoff + sizeof(*iph));
 }
 EXPORT_SYMBOL(inet_gro_complete);
 
diff --git a/net/ipv4/fou.c b/net/ipv4/fou.c
index 500a59906b87..c42a3ef17864 100644
--- a/net/ipv4/fou.c
+++ b/net/ipv4/fou.c
@@ -261,24 +261,14 @@ static struct sk_buff *fou_gro_receive(struct sock *sk,
 static int fou_gro_complete(struct sock *sk, struct sk_buff *skb,
 			    int nhoff)
 {
-	const struct net_offload *ops;
 	u8 proto = fou_from_sock(sk)->protocol;
-	int err = -ENOSYS;
 	const struct net_offload **offloads;
+	int err;
 
-	rcu_read_lock();
 	offloads = NAPI_GRO_CB(skb)->is_ipv6 ? inet6_offloads : inet_offloads;
-	ops = rcu_dereference(offloads[proto]);
-	if (WARN_ON(!ops || !ops->callbacks.gro_complete))
-		goto out_unlock;
-
-	err = ops->callbacks.gro_complete(skb, nhoff);
-
+	err = net_gro_complete(offloads, proto, skb, nhoff);
 	skb_set_inner_mac_header(skb, nhoff);
 
-out_unlock:
-	rcu_read_unlock();
-
 	return err;
 }
 
@@ -457,7 +447,6 @@ static int gue_gro_complete(struct sock *sk, struct sk_buff *skb, int nhoff)
 {
 	const struct net_offload **offloads;
 	struct guehdr *guehdr = (struct guehdr *)(skb->data + nhoff);
-	const struct net_offload *ops;
 	unsigned int guehlen = 0;
 	u8 proto;
 	int err = -ENOENT;
@@ -483,18 +472,10 @@ static int gue_gro_complete(struct sock *sk, struct sk_buff *skb, int nhoff)
 		return err;
 	}
 
-	rcu_read_lock();
 	offloads = NAPI_GRO_CB(skb)->is_ipv6 ? inet6_offloads : inet_offloads;
-	ops = rcu_dereference(offloads[proto]);
-	if (WARN_ON(!ops || !ops->callbacks.gro_complete))
-		goto out_unlock;
-
-	err = ops->callbacks.gro_complete(skb, nhoff + guehlen);
-
+	err = net_gro_complete(offloads, proto, skb, nhoff + guehlen);
 	skb_set_inner_mac_header(skb, nhoff + guehlen);
 
-out_unlock:
-	rcu_read_unlock();
 	return err;
 }
 
diff --git a/net/ipv4/gre_offload.c b/net/ipv4/gre_offload.c
index 6c63524f598a..fc8c99e4a058 100644
--- a/net/ipv4/gre_offload.c
+++ b/net/ipv4/gre_offload.c
@@ -231,10 +231,9 @@ static struct sk_buff *gre_gro_receive(struct list_head *head,
 static int gre_gro_complete(struct sk_buff *skb, int nhoff)
 {
 	struct gre_base_hdr *greh = (struct gre_base_hdr *)(skb->data + nhoff);
-	struct packet_offload *ptype;
 	unsigned int grehlen = sizeof(*greh);
-	int err = -ENOENT;
 	__be16 type;
+	int err;
 
 	skb->encapsulation = 1;
 	skb_shinfo(skb)->gso_type = SKB_GSO_GRE;
@@ -246,13 +245,8 @@ static int gre_gro_complete(struct sk_buff *skb, int nhoff)
 	if (greh->flags & GRE_CSUM)
 		grehlen += GRE_HEADER_SECTION;
 
-	rcu_read_lock();
-	ptype = gro_find_complete_by_type(type);
-	if (ptype)
-		err = ptype->callbacks.gro_complete(skb, nhoff + grehlen);
-
-	rcu_read_unlock();
-
+	err = net_gro_complete(dev_offloads, type, skb,
+				nhoff + sizeof(*greh));
 	skb_set_inner_mac_header(skb, nhoff + grehlen);
 
 	return err;
diff --git a/net/ipv6/ip6_offload.c b/net/ipv6/ip6_offload.c
index c7e495f12011..e8bf554ae611 100644
--- a/net/ipv6/ip6_offload.c
+++ b/net/ipv6/ip6_offload.c
@@ -298,7 +298,6 @@ static int ipv6_gro_complete(struct sk_buff *skb, int nhoff)
 {
 	const struct net_offload *ops;
 	struct ipv6hdr *iph = (struct ipv6hdr *)(skb->data + nhoff);
-	int err = -ENOSYS;
 
 	if (skb->encapsulation) {
 		skb_set_inner_protocol(skb, cpu_to_be16(ETH_P_IPV6));
@@ -307,18 +306,8 @@ static int ipv6_gro_complete(struct sk_buff *skb, int nhoff)
 
 	iph->payload_len = htons(skb->len - nhoff - sizeof(*iph));
 
-	rcu_read_lock();
-
 	nhoff += sizeof(*iph) + ipv6_exthdrs_len(iph, &ops);
-	if (WARN_ON(!ops || !ops->callbacks.gro_complete))
-		goto out_unlock;
-
-	err = ops->callbacks.gro_complete(skb, nhoff);
-
-out_unlock:
-	rcu_read_unlock();
-
-	return err;
+	return net_gro_complete(inet6_offloads, ops->type, skb, nhoff);
 }
 
 static int sit_gro_complete(struct sk_buff *skb, int nhoff)
-- 
2.19.0.397.gdd90340f6a-goog

[PATCH net-next RFC 3/8] gro: add net_gro_receive

[Willem de Bruijn]

From: Willem de Bruijn <willemb@google.com>

For configurable gro_receive all callsites need to be updated. Similar
to gro_complete, introduce a single shared helper, net_gro_receive.

Signed-off-by: Willem de Bruijn <willemb@google.com>
---
 drivers/net/geneve.c      |  2 +-
 include/linux/netdevice.h | 14 +++++++++++++-
 net/8021q/vlan.c          |  2 +-
 net/core/dev.c            | 20 ++++----------------
 net/ethernet/eth.c        |  2 +-
 net/ipv4/af_inet.c        |  4 ++--
 net/ipv4/fou.c            |  8 ++++----
 net/ipv4/gre_offload.c    | 12 ++++++------
 net/ipv6/ip6_offload.c    |  8 ++++----
 9 files changed, 36 insertions(+), 36 deletions(-)

diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
index a3a4621d9bee..a812a774e5fd 100644
--- a/drivers/net/geneve.c
+++ b/drivers/net/geneve.c
@@ -467,7 +467,7 @@ static struct sk_buff *geneve_gro_receive(struct sock *sk,
 	type = gh->proto_type;
 
 	rcu_read_lock();
-	ptype = gro_find_receive_by_type(type);
+	ptype = net_gro_receive(dev_offloads, type);
 	if (!ptype)
 		goto out_unlock;
 
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 0d292ea6716e..0be594f8d1ce 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3556,7 +3556,6 @@ gro_result_t napi_gro_receive(struct napi_struct *napi, struct sk_buff *skb);
 void napi_gro_flush(struct napi_struct *napi, bool flush_old);
 struct sk_buff *napi_get_frags(struct napi_struct *napi);
 gro_result_t napi_gro_frags(struct napi_struct *napi);
-struct packet_offload *gro_find_receive_by_type(__be16 type);
 
 extern const struct net_offload __rcu *dev_offloads[256];
 
@@ -3568,6 +3567,19 @@ static inline u8 net_offload_from_type(u16 type)
 	return type & 0xFF;
 }
 
+static inline const struct net_offload *
+net_gro_receive(const struct net_offload __rcu **offs, u16 type)
+{
+	const struct net_offload *off;
+
+	off = rcu_dereference(offs[net_offload_from_type(type)]);
+	if (off && off->callbacks.gro_receive &&
+	    (!off->type || off->type == type))
+		return off;
+	else
+		return NULL;
+}
+
 static inline int net_gro_complete(const struct net_offload __rcu **offs,
 				   u16 type, struct sk_buff *skb, int nhoff)
 {
diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
index 6ac27aa9f158..a106c5373b1d 100644
--- a/net/8021q/vlan.c
+++ b/net/8021q/vlan.c
@@ -670,7 +670,7 @@ static struct sk_buff *vlan_gro_receive(struct list_head *head,
 	type = vhdr->h_vlan_encapsulated_proto;
 
 	rcu_read_lock();
-	ptype = gro_find_receive_by_type(type);
+	ptype = net_gro_receive(dev_offloads, type);
 	if (!ptype)
 		goto out_unlock;
 
diff --git a/net/core/dev.c b/net/core/dev.c
index 2c21e507291f..ae5fbd4114d2 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5382,7 +5382,7 @@ static void gro_flush_oldest(struct list_head *head)
 static enum gro_result dev_gro_receive(struct napi_struct *napi, struct sk_buff *skb)
 {
 	u32 hash = skb_get_hash_raw(skb) & (GRO_HASH_BUCKETS - 1);
-	const struct packet_offload *ptype;
+	const struct net_offload *ops;
 	__be16 type = skb->protocol;
 	struct list_head *gro_head;
 	struct sk_buff *pp = NULL;
@@ -5396,8 +5396,8 @@ static enum gro_result dev_gro_receive(struct napi_struct *napi, struct sk_buff
 	gro_head = gro_list_prepare(napi, skb);
 
 	rcu_read_lock();
-	ptype = dev_offloads[net_offload_from_type(type)];
-	if (ptype && ptype->callbacks.gro_receive) {
+	ops = net_gro_receive(dev_offloads, type);
+	if (ops) {
 		skb_set_network_header(skb, skb_gro_offset(skb));
 		skb_reset_mac_len(skb);
 		NAPI_GRO_CB(skb)->same_flow = 0;
@@ -5425,7 +5425,7 @@ static enum gro_result dev_gro_receive(struct napi_struct *napi, struct sk_buff
 			NAPI_GRO_CB(skb)->csum_valid = 0;
 		}
 
-		pp = ptype->callbacks.gro_receive(gro_head, skb);
+		pp = ops->callbacks.gro_receive(gro_head, skb);
 		rcu_read_unlock();
 	} else {
 		rcu_read_unlock();
@@ -5483,18 +5483,6 @@ static enum gro_result dev_gro_receive(struct napi_struct *napi, struct sk_buff
 	goto pull;
 }
 
-struct packet_offload *gro_find_receive_by_type(__be16 type)
-{
-	struct net_offload *off;
-
-	off = (struct net_offload *) rcu_dereference(dev_offloads[type & 0xFF]);
-	if (off && off->type == type && off->callbacks.gro_receive)
-		return off;
-	else
-		return NULL;
-}
-EXPORT_SYMBOL(gro_find_receive_by_type);
-
 static void napi_skb_free_stolen_head(struct sk_buff *skb)
 {
 	skb_dst_drop(skb);
diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c
index fb17a13722e8..542dbc2ec956 100644
--- a/net/ethernet/eth.c
+++ b/net/ethernet/eth.c
@@ -462,7 +462,7 @@ struct sk_buff *eth_gro_receive(struct list_head *head, struct sk_buff *skb)
 	type = eh->h_proto;
 
 	rcu_read_lock();
-	ptype = gro_find_receive_by_type(type);
+	ptype = net_gro_receive(dev_offloads, type);
 	if (ptype == NULL) {
 		flush = 1;
 		goto out_unlock;
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 1b72ee4a7811..28b7c7671789 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -1409,8 +1409,8 @@ struct sk_buff *inet_gro_receive(struct list_head *head, struct sk_buff *skb)
 	proto = iph->protocol;
 
 	rcu_read_lock();
-	ops = rcu_dereference(inet_offloads[proto]);
-	if (!ops || !ops->callbacks.gro_receive)
+	ops = net_gro_receive(inet_offloads, proto);
+	if (!ops)
 		goto out_unlock;
 
 	if (*(u8 *)iph != 0x45)
diff --git a/net/ipv4/fou.c b/net/ipv4/fou.c
index c42a3ef17864..13401cb2e7a4 100644
--- a/net/ipv4/fou.c
+++ b/net/ipv4/fou.c
@@ -246,8 +246,8 @@ static struct sk_buff *fou_gro_receive(struct sock *sk,
 
 	rcu_read_lock();
 	offloads = NAPI_GRO_CB(skb)->is_ipv6 ? inet6_offloads : inet_offloads;
-	ops = rcu_dereference(offloads[proto]);
-	if (!ops || !ops->callbacks.gro_receive)
+	ops = net_gro_receive(offloads, proto);
+	if (!ops)
 		goto out_unlock;
 
 	pp = call_gro_receive(ops->callbacks.gro_receive, head, skb);
@@ -428,8 +428,8 @@ static struct sk_buff *gue_gro_receive(struct sock *sk,
 
 	rcu_read_lock();
 	offloads = NAPI_GRO_CB(skb)->is_ipv6 ? inet6_offloads : inet_offloads;
-	ops = rcu_dereference(offloads[proto]);
-	if (WARN_ON_ONCE(!ops || !ops->callbacks.gro_receive))
+	ops = net_gro_receive(offloads, proto);
+	if (WARN_ON_ONCE(!ops))
 		goto out_unlock;
 
 	pp = call_gro_receive(ops->callbacks.gro_receive, head, skb);
diff --git a/net/ipv4/gre_offload.c b/net/ipv4/gre_offload.c
index fc8c99e4a058..4f9237a4bea1 100644
--- a/net/ipv4/gre_offload.c
+++ b/net/ipv4/gre_offload.c
@@ -111,13 +111,13 @@ static struct sk_buff *gre_gso_segment(struct sk_buff *skb,
 static struct sk_buff *gre_gro_receive(struct list_head *head,
 				       struct sk_buff *skb)
 {
-	struct sk_buff *pp = NULL;
-	struct sk_buff *p;
 	const struct gre_base_hdr *greh;
+	const struct net_offload *ops;
 	unsigned int hlen, grehlen;
+	struct sk_buff *pp = NULL;
+	struct sk_buff *p;
 	unsigned int off;
 	int flush = 1;
-	struct packet_offload *ptype;
 	__be16 type;
 
 	if (NAPI_GRO_CB(skb)->encap_mark)
@@ -154,8 +154,8 @@ static struct sk_buff *gre_gro_receive(struct list_head *head,
 	type = greh->protocol;
 
 	rcu_read_lock();
-	ptype = gro_find_receive_by_type(type);
-	if (!ptype)
+	ops = net_gro_receive(dev_offloads, type);
+	if (!ops)
 		goto out_unlock;
 
 	grehlen = GRE_HEADER_SECTION;
@@ -217,7 +217,7 @@ static struct sk_buff *gre_gro_receive(struct list_head *head,
 	/* Adjusted NAPI_GRO_CB(skb)->csum after skb_gro_pull()*/
 	skb_gro_postpull_rcsum(skb, greh, grehlen);
 
-	pp = call_gro_receive(ptype->callbacks.gro_receive, head, skb);
+	pp = call_gro_receive(ops->callbacks.gro_receive, head, skb);
 	flush = 0;
 
 out_unlock:
diff --git a/net/ipv6/ip6_offload.c b/net/ipv6/ip6_offload.c
index e8bf554ae611..9d301bef0e23 100644
--- a/net/ipv6/ip6_offload.c
+++ b/net/ipv6/ip6_offload.c
@@ -194,8 +194,8 @@ static struct sk_buff *ipv6_gro_receive(struct list_head *head,
 
 	rcu_read_lock();
 	proto = iph->nexthdr;
-	ops = rcu_dereference(inet6_offloads[proto]);
-	if (!ops || !ops->callbacks.gro_receive) {
+	ops = net_gro_receive(inet6_offloads, proto);
+	if (!ops) {
 		__pskb_pull(skb, skb_gro_offset(skb));
 		skb_gro_frag0_invalidate(skb);
 		proto = ipv6_gso_pull_exthdrs(skb, proto);
@@ -203,8 +203,8 @@ static struct sk_buff *ipv6_gro_receive(struct list_head *head,
 		skb_reset_transport_header(skb);
 		__skb_push(skb, skb_gro_offset(skb));
 
-		ops = rcu_dereference(inet6_offloads[proto]);
-		if (!ops || !ops->callbacks.gro_receive)
+		ops = net_gro_receive(inet6_offloads, proto);
+		if (!ops)
 			goto out_unlock;
 
 		iph = ipv6_hdr(skb);
-- 
2.19.0.397.gdd90340f6a-goog

[PATCH net-next RFC 4/8] ipv6: remove offload exception for hopopts

[Willem de Bruijn]

From: Willem de Bruijn <willemb@google.com>

Extension headers in ipv6 are pulled without calling a callback
function. An inet6_offload signals this feature with flag
INET6_PROTO_GSO_EXTHDR.

Add net_has_flag helper to hide implementation details and in
prepartion for configurable gro.

Convert NEXTHDR_HOP from a special case branch to a standard
extension header offload.

Signed-off-by: Willem de Bruijn <willemb@google.com>
---
 include/linux/netdevice.h  |  9 +++++++++
 net/ipv6/exthdrs_offload.c | 17 ++++++++++++++---
 net/ipv6/ip6_offload.c     | 36 +++++++++++++-----------------------
 3 files changed, 36 insertions(+), 26 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 0be594f8d1ce..1c97a048506f 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3567,6 +3567,15 @@ static inline u8 net_offload_from_type(u16 type)
 	return type & 0xFF;
 }
 
+static inline bool net_offload_has_flag(const struct net_offload __rcu **offs,
+					u16 type, u16 flag)
+{
+	const struct net_offload *off;
+
+	off = offs ? rcu_dereference(offs[net_offload_from_type(type)]) : NULL;
+	return off && off->flags & flag;
+}
+
 static inline const struct net_offload *
 net_gro_receive(const struct net_offload __rcu **offs, u16 type)
 {
diff --git a/net/ipv6/exthdrs_offload.c b/net/ipv6/exthdrs_offload.c
index f5e2ba1c18bf..2230331c6012 100644
--- a/net/ipv6/exthdrs_offload.c
+++ b/net/ipv6/exthdrs_offload.c
@@ -12,11 +12,15 @@
 #include <net/protocol.h>
 #include "ip6_offload.h"
 
-static const struct net_offload rthdr_offload = {
+static struct net_offload hophdr_offload = {
 	.flags		=	INET6_PROTO_GSO_EXTHDR,
 };
 
-static const struct net_offload dstopt_offload = {
+static struct net_offload rthdr_offload = {
+	.flags		=	INET6_PROTO_GSO_EXTHDR,
+};
+
+static struct net_offload dstopt_offload = {
 	.flags		=	INET6_PROTO_GSO_EXTHDR,
 };
 
@@ -24,10 +28,14 @@ int __init ipv6_exthdrs_offload_init(void)
 {
 	int ret;
 
-	ret = inet6_add_offload(&rthdr_offload, IPPROTO_ROUTING);
+	ret = inet6_add_offload(&hophdr_offload, IPPROTO_HOPOPTS);
 	if (ret)
 		goto out;
 
+	ret = inet6_add_offload(&rthdr_offload, IPPROTO_ROUTING);
+	if (ret)
+		goto out_hop;
+
 	ret = inet6_add_offload(&dstopt_offload, IPPROTO_DSTOPTS);
 	if (ret)
 		goto out_rt;
@@ -37,5 +45,8 @@ int __init ipv6_exthdrs_offload_init(void)
 
 out_rt:
 	inet6_del_offload(&rthdr_offload, IPPROTO_ROUTING);
+
+out_hop:
+	inet6_del_offload(&rthdr_offload, IPPROTO_HOPOPTS);
 	goto out;
 }
diff --git a/net/ipv6/ip6_offload.c b/net/ipv6/ip6_offload.c
index 9d301bef0e23..4854509a2c5d 100644
--- a/net/ipv6/ip6_offload.c
+++ b/net/ipv6/ip6_offload.c
@@ -22,21 +22,13 @@
 
 static int ipv6_gso_pull_exthdrs(struct sk_buff *skb, int proto)
 {
-	const struct net_offload *ops = NULL;
-
 	for (;;) {
 		struct ipv6_opt_hdr *opth;
 		int len;
 
-		if (proto != NEXTHDR_HOP) {
-			ops = rcu_dereference(inet6_offloads[proto]);
-
-			if (unlikely(!ops))
-				break;
-
-			if (!(ops->flags & INET6_PROTO_GSO_EXTHDR))
-				break;
-		}
+		if (!net_offload_has_flag(inet6_offloads, proto,
+					  INET6_PROTO_GSO_EXTHDR))
+			break;
 
 		if (unlikely(!pskb_may_pull(skb, 8)))
 			break;
@@ -141,26 +133,24 @@ static struct sk_buff *ipv6_gso_segment(struct sk_buff *skb,
 /* Return the total length of all the extension hdrs, following the same
  * logic in ipv6_gso_pull_exthdrs() when parsing ext-hdrs.
  */
-static int ipv6_exthdrs_len(struct ipv6hdr *iph,
-			    const struct net_offload **opps)
+static int ipv6_exthdrs_len(struct ipv6hdr *iph, u8 *pproto)
 {
 	struct ipv6_opt_hdr *opth = (void *)iph;
 	int len = 0, proto, optlen = sizeof(*iph);
 
 	proto = iph->nexthdr;
 	for (;;) {
-		if (proto != NEXTHDR_HOP) {
-			*opps = rcu_dereference(inet6_offloads[proto]);
-			if (unlikely(!(*opps)))
-				break;
-			if (!((*opps)->flags & INET6_PROTO_GSO_EXTHDR))
-				break;
-		}
+		if (!net_offload_has_flag(inet6_offloads, proto,
+					  INET6_PROTO_GSO_EXTHDR))
+			break;
+
 		opth = (void *)opth + optlen;
 		optlen = ipv6_optlen(opth);
 		len += optlen;
 		proto = opth->nexthdr;
 	}
+
+	*pproto = proto;
 	return len;
 }
 
@@ -296,8 +286,8 @@ static struct sk_buff *ip4ip6_gro_receive(struct list_head *head,
 
 static int ipv6_gro_complete(struct sk_buff *skb, int nhoff)
 {
-	const struct net_offload *ops;
 	struct ipv6hdr *iph = (struct ipv6hdr *)(skb->data + nhoff);
+	u8 proto;
 
 	if (skb->encapsulation) {
 		skb_set_inner_protocol(skb, cpu_to_be16(ETH_P_IPV6));
@@ -306,8 +296,8 @@ static int ipv6_gro_complete(struct sk_buff *skb, int nhoff)
 
 	iph->payload_len = htons(skb->len - nhoff - sizeof(*iph));
 
-	nhoff += sizeof(*iph) + ipv6_exthdrs_len(iph, &ops);
-	return net_gro_complete(inet6_offloads, ops->type, skb, nhoff);
+	nhoff += sizeof(*iph) + ipv6_exthdrs_len(iph, &proto);
+	return net_gro_complete(inet6_offloads, proto, skb, nhoff);
 }
 
 static int sit_gro_complete(struct sk_buff *skb, int nhoff)
-- 
2.19.0.397.gdd90340f6a-goog

[PATCH net-next RFC 5/8] net: deconstify net_offload

[Willem de Bruijn]

From: Willem de Bruijn <willemb@google.com>

With configurable gro, the flags field in net_offloads may be changed.

Remove the const keyword. This is a noop otherwise.

Signed-off-by: Willem de Bruijn <willemb@google.com>
---
 include/linux/netdevice.h | 14 +++++++-------
 include/net/protocol.h    | 12 ++++++------
 net/core/dev.c            |  8 +++-----
 net/ipv4/af_inet.c        |  2 +-
 net/ipv4/esp4_offload.c   |  2 +-
 net/ipv4/fou.c            |  8 ++++----
 net/ipv4/gre_offload.c    |  2 +-
 net/ipv4/protocol.c       | 10 +++++-----
 net/ipv4/tcp_offload.c    |  2 +-
 net/ipv4/udp_offload.c    |  6 +++---
 net/ipv6/esp6_offload.c   |  2 +-
 net/ipv6/ip6_offload.c    |  6 +++---
 net/ipv6/protocol.c       | 10 +++++-----
 net/ipv6/tcpv6_offload.c  |  2 +-
 net/ipv6/udp_offload.c    |  2 +-
 net/sctp/offload.c        |  2 +-
 16 files changed, 44 insertions(+), 46 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 1c97a048506f..b9e671887fc2 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3557,7 +3557,7 @@ void napi_gro_flush(struct napi_struct *napi, bool flush_old);
 struct sk_buff *napi_get_frags(struct napi_struct *napi);
 gro_result_t napi_gro_frags(struct napi_struct *napi);
 
-extern const struct net_offload __rcu *dev_offloads[256];
+extern struct net_offload __rcu *dev_offloads[256];
 
 static inline u8 net_offload_from_type(u16 type)
 {
@@ -3567,19 +3567,19 @@ static inline u8 net_offload_from_type(u16 type)
 	return type & 0xFF;
 }
 
-static inline bool net_offload_has_flag(const struct net_offload __rcu **offs,
+static inline bool net_offload_has_flag(struct net_offload __rcu **offs,
 					u16 type, u16 flag)
 {
-	const struct net_offload *off;
+	struct net_offload *off;
 
 	off = offs ? rcu_dereference(offs[net_offload_from_type(type)]) : NULL;
 	return off && off->flags & flag;
 }
 
 static inline const struct net_offload *
-net_gro_receive(const struct net_offload __rcu **offs, u16 type)
+net_gro_receive(struct net_offload __rcu **offs, u16 type)
 {
-	const struct net_offload *off;
+	struct net_offload *off;
 
 	off = rcu_dereference(offs[net_offload_from_type(type)]);
 	if (off && off->callbacks.gro_receive &&
@@ -3589,10 +3589,10 @@ net_gro_receive(const struct net_offload __rcu **offs, u16 type)
 		return NULL;
 }
 
-static inline int net_gro_complete(const struct net_offload __rcu **offs,
+static inline int net_gro_complete(struct net_offload __rcu **offs,
 				   u16 type, struct sk_buff *skb, int nhoff)
 {
-	const struct net_offload *off;
+	struct net_offload *off;
 	int ret = -ENOENT;
 
 	rcu_read_lock();
diff --git a/include/net/protocol.h b/include/net/protocol.h
index 53a0322ee545..5e2c20b662d1 100644
--- a/include/net/protocol.h
+++ b/include/net/protocol.h
@@ -87,8 +87,8 @@ struct inet_protosw {
 #define INET_PROTOSW_ICSK      0x04  /* Is this an inet_connection_sock? */
 
 extern struct net_protocol __rcu *inet_protos[MAX_INET_PROTOS];
-extern const struct net_offload __rcu *inet_offloads[MAX_INET_PROTOS];
-extern const struct net_offload __rcu *inet6_offloads[MAX_INET_PROTOS];
+extern struct net_offload __rcu *inet_offloads[MAX_INET_PROTOS];
+extern struct net_offload __rcu *inet6_offloads[MAX_INET_PROTOS];
 
 #if IS_ENABLED(CONFIG_IPV6)
 extern struct inet6_protocol __rcu *inet6_protos[MAX_INET_PROTOS];
@@ -96,8 +96,8 @@ extern struct inet6_protocol __rcu *inet6_protos[MAX_INET_PROTOS];
 
 int inet_add_protocol(const struct net_protocol *prot, unsigned char num);
 int inet_del_protocol(const struct net_protocol *prot, unsigned char num);
-int inet_add_offload(const struct net_offload *prot, unsigned char num);
-int inet_del_offload(const struct net_offload *prot, unsigned char num);
+int inet_add_offload(struct net_offload *prot, unsigned char num);
+int inet_del_offload(struct net_offload *prot, unsigned char num);
 void inet_register_protosw(struct inet_protosw *p);
 void inet_unregister_protosw(struct inet_protosw *p);
 
@@ -107,7 +107,7 @@ int inet6_del_protocol(const struct inet6_protocol *prot, unsigned char num);
 int inet6_register_protosw(struct inet_protosw *p);
 void inet6_unregister_protosw(struct inet_protosw *p);
 #endif
-int inet6_add_offload(const struct net_offload *prot, unsigned char num);
-int inet6_del_offload(const struct net_offload *prot, unsigned char num);
+int inet6_add_offload(struct net_offload *prot, unsigned char num);
+int inet6_del_offload(struct net_offload *prot, unsigned char num);
 
 #endif	/* _PROTOCOL_H */
diff --git a/net/core/dev.c b/net/core/dev.c
index ae5fbd4114d2..20d9552afd38 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -466,7 +466,7 @@ void dev_remove_pack(struct packet_type *pt)
 EXPORT_SYMBOL(dev_remove_pack);
 
 
-const struct net_offload __rcu *dev_offloads[256] __read_mostly;
+struct net_offload __rcu *dev_offloads[256] __read_mostly;
 EXPORT_SYMBOL(dev_offloads);
 
 /**
@@ -483,8 +483,7 @@ EXPORT_SYMBOL(dev_offloads);
  */
 void dev_add_offload(struct packet_offload *po)
 {
-	cmpxchg((const struct net_offload **)
-		&dev_offloads[net_offload_from_type(po->type)],
+	cmpxchg(&dev_offloads[net_offload_from_type(po->type)],
 			NULL, po);
 }
 EXPORT_SYMBOL(dev_add_offload);
@@ -504,8 +503,7 @@ EXPORT_SYMBOL(dev_add_offload);
  */
 static int __dev_remove_offload(struct packet_offload *po)
 {
-	return (cmpxchg((const struct net_offload **)
-			&dev_offloads[net_offload_from_type(po->type)],
+	return (cmpxchg(&dev_offloads[net_offload_from_type(po->type)],
 		       po, NULL) == po) ? 0 : -1;
 }
 
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 28b7c7671789..f3ee6f4dfc0f 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -1839,7 +1839,7 @@ static struct packet_offload ip_packet_offload __read_mostly = {
 	},
 };
 
-static const struct net_offload ipip_offload = {
+static struct net_offload ipip_offload = {
 	.callbacks = {
 		.gso_segment	= inet_gso_segment,
 		.gro_receive	= ipip_gro_receive,
diff --git a/net/ipv4/esp4_offload.c b/net/ipv4/esp4_offload.c
index 58834a10c0be..e6d7a9be9244 100644
--- a/net/ipv4/esp4_offload.c
+++ b/net/ipv4/esp4_offload.c
@@ -240,7 +240,7 @@ static int esp_xmit(struct xfrm_state *x, struct sk_buff *skb,  netdev_features_
 	return 0;
 }
 
-static const struct net_offload esp4_offload = {
+static struct net_offload esp4_offload = {
 	.callbacks = {
 		.gro_receive = esp4_gro_receive,
 		.gso_segment = esp4_gso_segment,
diff --git a/net/ipv4/fou.c b/net/ipv4/fou.c
index 13401cb2e7a4..52e01dcaa417 100644
--- a/net/ipv4/fou.c
+++ b/net/ipv4/fou.c
@@ -229,7 +229,7 @@ static struct sk_buff *fou_gro_receive(struct sock *sk,
 				       struct sk_buff *skb)
 {
 	u8 proto = fou_from_sock(sk)->protocol;
-	const struct net_offload **offloads;
+	struct net_offload **offloads;
 	const struct net_offload *ops;
 	struct sk_buff *pp = NULL;
 
@@ -262,7 +262,7 @@ static int fou_gro_complete(struct sock *sk, struct sk_buff *skb,
 			    int nhoff)
 {
 	u8 proto = fou_from_sock(sk)->protocol;
-	const struct net_offload **offloads;
+	struct net_offload **offloads;
 	int err;
 
 	offloads = NAPI_GRO_CB(skb)->is_ipv6 ? inet6_offloads : inet_offloads;
@@ -299,7 +299,7 @@ static struct sk_buff *gue_gro_receive(struct sock *sk,
 				       struct list_head *head,
 				       struct sk_buff *skb)
 {
-	const struct net_offload **offloads;
+	struct net_offload **offloads;
 	const struct net_offload *ops;
 	struct sk_buff *pp = NULL;
 	struct sk_buff *p;
@@ -445,7 +445,7 @@ static struct sk_buff *gue_gro_receive(struct sock *sk,
 
 static int gue_gro_complete(struct sock *sk, struct sk_buff *skb, int nhoff)
 {
-	const struct net_offload **offloads;
+	struct net_offload **offloads;
 	struct guehdr *guehdr = (struct guehdr *)(skb->data + nhoff);
 	unsigned int guehlen = 0;
 	u8 proto;
diff --git a/net/ipv4/gre_offload.c b/net/ipv4/gre_offload.c
index 4f9237a4bea1..70910650d322 100644
--- a/net/ipv4/gre_offload.c
+++ b/net/ipv4/gre_offload.c
@@ -252,7 +252,7 @@ static int gre_gro_complete(struct sk_buff *skb, int nhoff)
 	return err;
 }
 
-static const struct net_offload gre_offload = {
+static struct net_offload gre_offload = {
 	.callbacks = {
 		.gso_segment = gre_gso_segment,
 		.gro_receive = gre_gro_receive,
diff --git a/net/ipv4/protocol.c b/net/ipv4/protocol.c
index 32a691b7ce2c..66948d77672e 100644
--- a/net/ipv4/protocol.c
+++ b/net/ipv4/protocol.c
@@ -29,7 +29,7 @@
 #include <net/protocol.h>
 
 struct net_protocol __rcu *inet_protos[MAX_INET_PROTOS] __read_mostly;
-const struct net_offload __rcu *inet_offloads[MAX_INET_PROTOS] __read_mostly;
+struct net_offload __rcu *inet_offloads[MAX_INET_PROTOS] __read_mostly;
 EXPORT_SYMBOL(inet_offloads);
 
 int inet_add_protocol(const struct net_protocol *prot, unsigned char protocol)
@@ -45,9 +45,9 @@ int inet_add_protocol(const struct net_protocol *prot, unsigned char protocol)
 }
 EXPORT_SYMBOL(inet_add_protocol);
 
-int inet_add_offload(const struct net_offload *prot, unsigned char protocol)
+int inet_add_offload(struct net_offload *prot, unsigned char protocol)
 {
-	return !cmpxchg((const struct net_offload **)&inet_offloads[protocol],
+	return !cmpxchg((struct net_offload **)&inet_offloads[protocol],
 			NULL, prot) ? 0 : -1;
 }
 EXPORT_SYMBOL(inet_add_offload);
@@ -65,11 +65,11 @@ int inet_del_protocol(const struct net_protocol *prot, unsigned char protocol)
 }
 EXPORT_SYMBOL(inet_del_protocol);
 
-int inet_del_offload(const struct net_offload *prot, unsigned char protocol)
+int inet_del_offload(struct net_offload *prot, unsigned char protocol)
 {
 	int ret;
 
-	ret = (cmpxchg((const struct net_offload **)&inet_offloads[protocol],
+	ret = (cmpxchg((struct net_offload **)&inet_offloads[protocol],
 		       prot, NULL) == prot) ? 0 : -1;
 
 	synchronize_net();
diff --git a/net/ipv4/tcp_offload.c b/net/ipv4/tcp_offload.c
index 870b0a335061..d670f2d008bc 100644
--- a/net/ipv4/tcp_offload.c
+++ b/net/ipv4/tcp_offload.c
@@ -333,7 +333,7 @@ static int tcp4_gro_complete(struct sk_buff *skb, int thoff)
 	return tcp_gro_complete(skb);
 }
 
-static const struct net_offload tcpv4_offload = {
+static struct net_offload tcpv4_offload = {
 	.callbacks = {
 		.gso_segment	=	tcp4_gso_segment,
 		.gro_receive	=	tcp4_gro_receive,
diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
index 0c0522b79b43..4f6aa95a9b12 100644
--- a/net/ipv4/udp_offload.c
+++ b/net/ipv4/udp_offload.c
@@ -153,8 +153,8 @@ struct sk_buff *skb_udp_tunnel_segment(struct sk_buff *skb,
 				       bool is_ipv6)
 {
 	__be16 protocol = skb->protocol;
-	const struct net_offload **offloads;
-	const struct net_offload *ops;
+	struct net_offload **offloads;
+	struct net_offload *ops;
 	struct sk_buff *segs = ERR_PTR(-EINVAL);
 	struct sk_buff *(*gso_inner_segment)(struct sk_buff *skb,
 					     netdev_features_t features);
@@ -472,7 +472,7 @@ static int udp4_gro_complete(struct sk_buff *skb, int nhoff)
 	return udp_gro_complete(skb, nhoff, udp4_lib_lookup_skb);
 }
 
-static const struct net_offload udpv4_offload = {
+static struct net_offload udpv4_offload = {
 	.callbacks = {
 		.gso_segment = udp4_ufo_fragment,
 		.gro_receive  =	udp4_gro_receive,
diff --git a/net/ipv6/esp6_offload.c b/net/ipv6/esp6_offload.c
index 6177e2171171..169dcd5c7135 100644
--- a/net/ipv6/esp6_offload.c
+++ b/net/ipv6/esp6_offload.c
@@ -268,7 +268,7 @@ static int esp6_xmit(struct xfrm_state *x, struct sk_buff *skb,  netdev_features
 	return 0;
 }
 
-static const struct net_offload esp6_offload = {
+static struct net_offload esp6_offload = {
 	.callbacks = {
 		.gro_receive = esp6_gro_receive,
 		.gso_segment = esp6_gso_segment,
diff --git a/net/ipv6/ip6_offload.c b/net/ipv6/ip6_offload.c
index 4854509a2c5d..2d0ea3f453f2 100644
--- a/net/ipv6/ip6_offload.c
+++ b/net/ipv6/ip6_offload.c
@@ -330,7 +330,7 @@ static struct packet_offload ipv6_packet_offload __read_mostly = {
 	},
 };
 
-static const struct net_offload sit_offload = {
+static struct net_offload sit_offload = {
 	.callbacks = {
 		.gso_segment	= ipv6_gso_segment,
 		.gro_receive    = sit_ip6ip6_gro_receive,
@@ -338,7 +338,7 @@ static const struct net_offload sit_offload = {
 	},
 };
 
-static const struct net_offload ip4ip6_offload = {
+static struct net_offload ip4ip6_offload = {
 	.callbacks = {
 		.gso_segment	= inet_gso_segment,
 		.gro_receive    = ip4ip6_gro_receive,
@@ -346,7 +346,7 @@ static const struct net_offload ip4ip6_offload = {
 	},
 };
 
-static const struct net_offload ip6ip6_offload = {
+static struct net_offload ip6ip6_offload = {
 	.callbacks = {
 		.gso_segment	= ipv6_gso_segment,
 		.gro_receive    = sit_ip6ip6_gro_receive,
diff --git a/net/ipv6/protocol.c b/net/ipv6/protocol.c
index b5d54d4f995c..06efcfc6d02b 100644
--- a/net/ipv6/protocol.c
+++ b/net/ipv6/protocol.c
@@ -50,21 +50,21 @@ int inet6_del_protocol(const struct inet6_protocol *prot, unsigned char protocol
 EXPORT_SYMBOL(inet6_del_protocol);
 #endif
 
-const struct net_offload __rcu *inet6_offloads[MAX_INET_PROTOS] __read_mostly;
+struct net_offload __rcu *inet6_offloads[MAX_INET_PROTOS] __read_mostly;
 EXPORT_SYMBOL(inet6_offloads);
 
-int inet6_add_offload(const struct net_offload *prot, unsigned char protocol)
+int inet6_add_offload(struct net_offload *prot, unsigned char protocol)
 {
-	return !cmpxchg((const struct net_offload **)&inet6_offloads[protocol],
+	return !cmpxchg((struct net_offload **)&inet6_offloads[protocol],
 			NULL, prot) ? 0 : -1;
 }
 EXPORT_SYMBOL(inet6_add_offload);
 
-int inet6_del_offload(const struct net_offload *prot, unsigned char protocol)
+int inet6_del_offload(struct net_offload *prot, unsigned char protocol)
 {
 	int ret;
 
-	ret = (cmpxchg((const struct net_offload **)&inet6_offloads[protocol],
+	ret = (cmpxchg((struct net_offload **)&inet6_offloads[protocol],
 		       prot, NULL) == prot) ? 0 : -1;
 
 	synchronize_net();
diff --git a/net/ipv6/tcpv6_offload.c b/net/ipv6/tcpv6_offload.c
index e72947c99454..a3c5010e1361 100644
--- a/net/ipv6/tcpv6_offload.c
+++ b/net/ipv6/tcpv6_offload.c
@@ -67,7 +67,7 @@ static struct sk_buff *tcp6_gso_segment(struct sk_buff *skb,
 
 	return tcp_gso_segment(skb, features);
 }
-static const struct net_offload tcpv6_offload = {
+static struct net_offload tcpv6_offload = {
 	.callbacks = {
 		.gso_segment	=	tcp6_gso_segment,
 		.gro_receive	=	tcp6_gro_receive,
diff --git a/net/ipv6/udp_offload.c b/net/ipv6/udp_offload.c
index 95dee9ca8d22..2a41da0dd33f 100644
--- a/net/ipv6/udp_offload.c
+++ b/net/ipv6/udp_offload.c
@@ -158,7 +158,7 @@ static int udp6_gro_complete(struct sk_buff *skb, int nhoff)
 	return udp_gro_complete(skb, nhoff, udp6_lib_lookup_skb);
 }
 
-static const struct net_offload udpv6_offload = {
+static struct net_offload udpv6_offload = {
 	.callbacks = {
 		.gso_segment	=	udp6_ufo_fragment,
 		.gro_receive	=	udp6_gro_receive,
diff --git a/net/sctp/offload.c b/net/sctp/offload.c
index 123e9f2dc226..ad504b83245d 100644
--- a/net/sctp/offload.c
+++ b/net/sctp/offload.c
@@ -90,7 +90,7 @@ static struct sk_buff *sctp_gso_segment(struct sk_buff *skb,
 	return segs;
 }
 
-static const struct net_offload sctp_offload = {
+static struct net_offload sctp_offload = {
 	.callbacks = {
 		.gso_segment = sctp_gso_segment,
 	},
-- 
2.19.0.397.gdd90340f6a-goog

Re: [PATCH net-next RFC 5/8] net: deconstify net_offload

[Subash Abhinov Kasiviswanathan]

On 2018-09-14 11:59, Willem de Bruijn wrote:
> From: Willem de Bruijn <willemb@google.com>
> 
> With configurable gro, the flags field in net_offloads may be changed.
> 
> Remove the const keyword. This is a noop otherwise.
> 
> Signed-off-by: Willem de Bruijn <willemb@google.com>
> diff --git a/net/sctp/offload.c b/net/sctp/offload.c
> index 123e9f2dc226..ad504b83245d 100644
> --- a/net/sctp/offload.c
> +++ b/net/sctp/offload.c
> @@ -90,7 +90,7 @@ static struct sk_buff *sctp_gso_segment(struct 
> sk_buff
> *skb,
>  	return segs;
>  }
> 
> -static const struct net_offload sctp_offload = {
> +static struct net_offload sctp_offload = {
>  	.callbacks = {
>  		.gso_segment = sctp_gso_segment,
>  	},

Hi Willem

sctp6 also needs to be deconstified.

diff --git a/net/sctp/offload.c b/net/sctp/offload.c
index ad504b8..4be7794 100644
--- a/net/sctp/offload.c
+++ b/net/sctp/offload.c
@@ -96,7 +96,7 @@ static struct sk_buff *sctp_gso_segment(struct sk_buff 
*skb,
         },
  };

-static const struct net_offload sctp6_offload = {
+static struct net_offload sctp6_offload = {
         .callbacks = {
                 .gso_segment = sctp_gso_segment,
         },


-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH net-next RFC 5/8] net: deconstify net_offload

[Willem de Bruijn]

On Fri, Sep 14, 2018 at 11:30 PM Subash Abhinov Kasiviswanathan
<subashab@codeaurora.org> wrote:
>
> On 2018-09-14 11:59, Willem de Bruijn wrote:
> > From: Willem de Bruijn <willemb@google.com>
> >
> > With configurable gro, the flags field in net_offloads may be changed.
> >
> > Remove the const keyword. This is a noop otherwise.
> >
> > Signed-off-by: Willem de Bruijn <willemb@google.com>
> > diff --git a/net/sctp/offload.c b/net/sctp/offload.c
> > index 123e9f2dc226..ad504b83245d 100644
> > --- a/net/sctp/offload.c
> > +++ b/net/sctp/offload.c
> > @@ -90,7 +90,7 @@ static struct sk_buff *sctp_gso_segment(struct
> > sk_buff
> > *skb,
> >       return segs;
> >  }
> >
> > -static const struct net_offload sctp_offload = {
> > +static struct net_offload sctp_offload = {
> >       .callbacks = {
> >               .gso_segment = sctp_gso_segment,
> >       },
>
> Hi Willem
>
> sctp6 also needs to be deconstified.
>
> diff --git a/net/sctp/offload.c b/net/sctp/offload.c
> index ad504b8..4be7794 100644
> --- a/net/sctp/offload.c
> +++ b/net/sctp/offload.c
> @@ -96,7 +96,7 @@ static struct sk_buff *sctp_gso_segment(struct sk_buff
> *skb,
>          },
>   };
>
> -static const struct net_offload sctp6_offload = {
> +static struct net_offload sctp6_offload = {
>          .callbacks = {
>                  .gso_segment = sctp_gso_segment,
>          },

Thanks! I'll update that.

[PATCH net-next RFC 6/8] net: make gro configurable

[Willem de Bruijn]

From: Willem de Bruijn <willemb@google.com>

Add net_offload flag NET_OFF_FLAG_GRO_OFF. If set, a net_offload will
not be used for gro receive processing.

Also add sysctl helper proc_do_net_offload that toggles this flag and
register sysctls net.{core,ipv4,ipv6}.gro

Signed-off-by: Willem de Bruijn <willemb@google.com>
---
 drivers/net/vxlan.c        |  8 +++++
 include/linux/netdevice.h  |  7 ++++-
 net/core/dev.c             |  1 +
 net/core/sysctl_net_core.c | 60 ++++++++++++++++++++++++++++++++++++++
 net/ipv4/sysctl_net_ipv4.c |  7 +++++
 net/ipv6/ip6_offload.c     | 10 +++++--
 net/ipv6/sysctl_net_ipv6.c |  8 +++++
 7 files changed, 97 insertions(+), 4 deletions(-)

diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index e5d236595206..8cb8e02c8ab6 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -572,6 +572,7 @@ static struct sk_buff *vxlan_gro_receive(struct sock *sk,
 					 struct list_head *head,
 					 struct sk_buff *skb)
 {
+	const struct net_offload *ops;
 	struct sk_buff *pp = NULL;
 	struct sk_buff *p;
 	struct vxlanhdr *vh, *vh2;
@@ -606,6 +607,12 @@ static struct sk_buff *vxlan_gro_receive(struct sock *sk,
 			goto out;
 	}
 
+	rcu_read_lock();
+	ops = net_gro_receive(dev_offloads, ETH_P_TEB);
+	rcu_read_unlock();
+	if (!ops)
+		goto out;
+
 	skb_gro_pull(skb, sizeof(struct vxlanhdr)); /* pull vxlan header */
 
 	list_for_each_entry(p, head, list) {
@@ -621,6 +628,7 @@ static struct sk_buff *vxlan_gro_receive(struct sock *sk,
 	}
 
 	pp = call_gro_receive(eth_gro_receive, head, skb);
+
 	flush = 0;
 
 out:
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index b9e671887fc2..93e8c9ade593 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2377,6 +2377,10 @@ struct net_offload {
 
 /* This should be set for any extension header which is compatible with GSO. */
 #define INET6_PROTO_GSO_EXTHDR	0x1
+#define NET_OFF_FLAG_GRO_OFF	0x2
+
+int proc_do_net_offload(struct ctl_table *ctl, int write, void __user *buffer,
+			size_t *lenp, loff_t *ppos);
 
 /* often modified stats are per-CPU, other are shared (netdev->stats) */
 struct pcpu_sw_netstats {
@@ -3583,7 +3587,8 @@ net_gro_receive(struct net_offload __rcu **offs, u16 type)
 
 	off = rcu_dereference(offs[net_offload_from_type(type)]);
 	if (off && off->callbacks.gro_receive &&
-	    (!off->type || off->type == type))
+	    (!off->type || off->type == type) &&
+	    !(off->flags & NET_OFF_FLAG_GRO_OFF))
 		return off;
 	else
 		return NULL;
diff --git a/net/core/dev.c b/net/core/dev.c
index 20d9552afd38..0fd5273bc931 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -154,6 +154,7 @@
 #define GRO_MAX_HEAD (MAX_HEADER + 128)
 
 static DEFINE_SPINLOCK(ptype_lock);
+DEFINE_SPINLOCK(offload_lock);
 struct list_head ptype_base[PTYPE_HASH_SIZE] __read_mostly;
 struct list_head ptype_all __read_mostly;	/* Taps */
 static struct list_head offload_base __read_mostly;
diff --git a/net/core/sysctl_net_core.c b/net/core/sysctl_net_core.c
index b1a2c5e38530..d2d72afdd9eb 100644
--- a/net/core/sysctl_net_core.c
+++ b/net/core/sysctl_net_core.c
@@ -15,6 +15,7 @@
 #include <linux/vmalloc.h>
 #include <linux/init.h>
 #include <linux/slab.h>
+#include <linux/bitmap.h>
 
 #include <net/ip.h>
 #include <net/sock.h>
@@ -34,6 +35,58 @@ static int net_msg_warn;	/* Unused, but still a sysctl */
 int sysctl_fb_tunnels_only_for_init_net __read_mostly = 0;
 EXPORT_SYMBOL(sysctl_fb_tunnels_only_for_init_net);
 
+extern spinlock_t offload_lock;
+
+#define NET_OFF_TBL_LEN	256
+
+int proc_do_net_offload(struct ctl_table *ctl, int write, void __user *buffer,
+			size_t *lenp, loff_t *ppos)
+{
+	unsigned long bitmap[NET_OFF_TBL_LEN / (sizeof(unsigned long) << 3)];
+	struct ctl_table tbl = { .maxlen = NET_OFF_TBL_LEN, .data = bitmap };
+	unsigned long flag = (unsigned long) ctl->extra2;
+	struct net_offload __rcu **offs = ctl->extra1;
+	struct net_offload *off;
+	int i, ret;
+
+	memset(bitmap, 0, sizeof(bitmap));
+
+	spin_lock(&offload_lock);
+
+	for (i = 0; i < tbl.maxlen; i++) {
+		off = rcu_dereference_protected(offs[i], lockdep_is_held(&offload_lock));
+		if (off && off->flags & flag) {
+			/* flag specific constraints */
+			if (flag == NET_OFF_FLAG_GRO_OFF) {
+				/* gro disable bit: only if can gro */
+				if (!off->callbacks.gro_receive &&
+				    !(off->flags & INET6_PROTO_GSO_EXTHDR))
+					continue;
+			}
+			set_bit(i, bitmap);
+		}
+	}
+
+	ret = proc_do_large_bitmap(&tbl, write, buffer, lenp, ppos);
+
+	if (write && !ret) {
+		for (i = 0; i < tbl.maxlen; i++) {
+			bool isset = test_bit(i, bitmap);
+
+			off = rcu_dereference_protected(offs[i], lockdep_is_held(&offload_lock));
+			if (!isset && (off->flags & flag))
+				off->flags &= ~flag;
+			else if (isset && !(off->flags & flag))
+				off->flags |= flag;
+		}
+	}
+
+	spin_unlock(&offload_lock);
+
+	return ret;
+}
+EXPORT_SYMBOL(proc_do_net_offload);
+
 #ifdef CONFIG_RPS
 static int rps_sock_flow_sysctl(struct ctl_table *table, int write,
 				void __user *buffer, size_t *lenp, loff_t *ppos)
@@ -435,6 +488,13 @@ static struct ctl_table net_core_table[] = {
 		.extra1		= &zero,
 		.extra2		= &one
 	},
+	{
+		.procname	= "gro",
+		.mode		= 0644,
+		.proc_handler	= proc_do_net_offload,
+		.extra1		= dev_offloads,
+		.extra2		= (void *) NET_OFF_FLAG_GRO_OFF,
+	},
 #ifdef CONFIG_RPS
 	{
 		.procname	= "rps_sock_flow_entries",
diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
index b92f422f2fa8..7a525039afb2 100644
--- a/net/ipv4/sysctl_net_ipv4.c
+++ b/net/ipv4/sysctl_net_ipv4.c
@@ -477,6 +477,13 @@ static struct ctl_table ipv4_table[] = {
 		.mode		= 0644,
 		.proc_handler	= proc_dointvec
 	},
+	{
+		.procname	= "gro",
+		.mode		= 0644,
+		.proc_handler	= proc_do_net_offload,
+		.extra1		= inet_offloads,
+		.extra2		= (void *) NET_OFF_FLAG_GRO_OFF,
+	},
 #ifdef CONFIG_NETLABEL
 	{
 		.procname	= "cipso_cache_enable",
diff --git a/net/ipv6/ip6_offload.c b/net/ipv6/ip6_offload.c
index 2d0ea3f453f2..6be5adbd2ce7 100644
--- a/net/ipv6/ip6_offload.c
+++ b/net/ipv6/ip6_offload.c
@@ -20,7 +20,7 @@
 
 #include "ip6_offload.h"
 
-static int ipv6_gso_pull_exthdrs(struct sk_buff *skb, int proto)
+static int ipv6_gso_pull_exthdrs(struct sk_buff *skb, int proto, bool is_gro)
 {
 	for (;;) {
 		struct ipv6_opt_hdr *opth;
@@ -30,6 +30,10 @@ static int ipv6_gso_pull_exthdrs(struct sk_buff *skb, int proto)
 					  INET6_PROTO_GSO_EXTHDR))
 			break;
 
+		if (is_gro && !net_offload_has_flag(inet6_offloads, proto,
+						    NET_OFF_FLAG_GRO_OFF))
+			break;
+
 		if (unlikely(!pskb_may_pull(skb, 8)))
 			break;
 
@@ -76,7 +80,7 @@ static struct sk_buff *ipv6_gso_segment(struct sk_buff *skb,
 	__skb_pull(skb, sizeof(*ipv6h));
 	segs = ERR_PTR(-EPROTONOSUPPORT);
 
-	proto = ipv6_gso_pull_exthdrs(skb, ipv6h->nexthdr);
+	proto = ipv6_gso_pull_exthdrs(skb, ipv6h->nexthdr, false);
 
 	if (skb->encapsulation &&
 	    skb_shinfo(skb)->gso_type & (SKB_GSO_IPXIP4 | SKB_GSO_IPXIP6))
@@ -188,7 +192,7 @@ static struct sk_buff *ipv6_gro_receive(struct list_head *head,
 	if (!ops) {
 		__pskb_pull(skb, skb_gro_offset(skb));
 		skb_gro_frag0_invalidate(skb);
-		proto = ipv6_gso_pull_exthdrs(skb, proto);
+		proto = ipv6_gso_pull_exthdrs(skb, proto, true);
 		skb_gro_pull(skb, -skb_transport_offset(skb));
 		skb_reset_transport_header(skb);
 		__skb_push(skb, skb_gro_offset(skb));
diff --git a/net/ipv6/sysctl_net_ipv6.c b/net/ipv6/sysctl_net_ipv6.c
index e15cd37024fd..83f14962a909 100644
--- a/net/ipv6/sysctl_net_ipv6.c
+++ b/net/ipv6/sysctl_net_ipv6.c
@@ -20,6 +20,7 @@
 #ifdef CONFIG_NETLABEL
 #include <net/calipso.h>
 #endif
+#include <net/protocol.h>
 
 static int zero;
 static int one = 1;
@@ -178,6 +179,13 @@ static struct ctl_table ipv6_rotable[] = {
 		.proc_handler	= proc_dointvec_minmax,
 		.extra1		= &one
 	},
+	{
+		.procname	= "gro",
+		.mode		= 0644,
+		.proc_handler	= proc_do_net_offload,
+		.extra1		= inet6_offloads,
+		.extra2		= (void *) NET_OFF_FLAG_GRO_OFF,
+	},
 #ifdef CONFIG_NETLABEL
 	{
 		.procname	= "calipso_cache_enable",
-- 
2.19.0.397.gdd90340f6a-goog

Re: [PATCH net-next RFC 6/8] net: make gro configurable

[Stephen Hemminger]

On Fri, 14 Sep 2018 13:59:39 -0400
Willem de Bruijn <willemdebruijn.kernel@gmail.com> wrote:

> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
> index e5d236595206..8cb8e02c8ab6 100644
> --- a/drivers/net/vxlan.c
> +++ b/drivers/net/vxlan.c
> @@ -572,6 +572,7 @@ static struct sk_buff *vxlan_gro_receive(struct sock *sk,
>  					 struct list_head *head,
>  					 struct sk_buff *skb)
>  {
> +	const struct net_offload *ops;
>  	struct sk_buff *pp = NULL;
>  	struct sk_buff *p;
>  	struct vxlanhdr *vh, *vh2;
> @@ -606,6 +607,12 @@ static struct sk_buff *vxlan_gro_receive(struct sock *sk,
>  			goto out;
>  	}
>  
> +	rcu_read_lock();
> +	ops = net_gro_receive(dev_offloads, ETH_P_TEB);
> +	rcu_read_unlock();
> +	if (!ops)
> +		goto out;

Isn't rcu_read_lock already held here?
RCU read lock is always held in the receive handler path

> +
>  	skb_gro_pull(skb, sizeof(struct vxlanhdr)); /* pull vxlan header */
>  
>  	list_for_each_entry(p, head, list) {
> @@ -621,6 +628,7 @@ static struct sk_buff *vxlan_gro_receive(struct sock *sk,
>  	}
>  
>  	pp = call_gro_receive(eth_gro_receive, head, skb);
> +
>  	flush = 0;

whitespace change crept into this patch.

Re: [PATCH net-next RFC 6/8] net: make gro configurable

[Willem de Bruijn]

On Fri, Sep 14, 2018 at 2:39 PM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Fri, 14 Sep 2018 13:59:39 -0400
> Willem de Bruijn <willemdebruijn.kernel@gmail.com> wrote:
>
> > diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
> > index e5d236595206..8cb8e02c8ab6 100644
> > --- a/drivers/net/vxlan.c
> > +++ b/drivers/net/vxlan.c
> > @@ -572,6 +572,7 @@ static struct sk_buff *vxlan_gro_receive(struct sock *sk,
> >                                        struct list_head *head,
> >                                        struct sk_buff *skb)
> >  {
> > +     const struct net_offload *ops;
> >       struct sk_buff *pp = NULL;
> >       struct sk_buff *p;
> >       struct vxlanhdr *vh, *vh2;
> > @@ -606,6 +607,12 @@ static struct sk_buff *vxlan_gro_receive(struct sock *sk,
> >                       goto out;
> >       }
> >
> > +     rcu_read_lock();
> > +     ops = net_gro_receive(dev_offloads, ETH_P_TEB);
> > +     rcu_read_unlock();
> > +     if (!ops)
> > +             goto out;
>
> Isn't rcu_read_lock already held here?
> RCU read lock is always held in the receive handler path

There is a critical section on receive, taken in
netif_receive_skb_core, but gro code runs before that. All the
existing gro handlers call rcu_read_lock.

> > +
> >       skb_gro_pull(skb, sizeof(struct vxlanhdr)); /* pull vxlan header */
> >
> >       list_for_each_entry(p, head, list) {
> > @@ -621,6 +628,7 @@ static struct sk_buff *vxlan_gro_receive(struct sock *sk,
> >       }
> >
> >       pp = call_gro_receive(eth_gro_receive, head, skb);
> > +
> >       flush = 0;
>
> whitespace change crept into this patch.

Oops, thanks.

Re: [PATCH net-next RFC 6/8] net: make gro configurable

[Willem de Bruijn]

On Fri, Sep 14, 2018 at 6:50 PM Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
>
> On Fri, Sep 14, 2018 at 2:39 PM Stephen Hemminger
> <stephen@networkplumber.org> wrote:
> >
> > On Fri, 14 Sep 2018 13:59:39 -0400
> > Willem de Bruijn <willemdebruijn.kernel@gmail.com> wrote:
> >
> > > diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
> > > index e5d236595206..8cb8e02c8ab6 100644
> > > --- a/drivers/net/vxlan.c
> > > +++ b/drivers/net/vxlan.c
> > > @@ -572,6 +572,7 @@ static struct sk_buff *vxlan_gro_receive(struct sock *sk,
> > >                                        struct list_head *head,
> > >                                        struct sk_buff *skb)
> > >  {
> > > +     const struct net_offload *ops;
> > >       struct sk_buff *pp = NULL;
> > >       struct sk_buff *p;
> > >       struct vxlanhdr *vh, *vh2;
> > > @@ -606,6 +607,12 @@ static struct sk_buff *vxlan_gro_receive(struct sock *sk,
> > >                       goto out;
> > >       }
> > >
> > > +     rcu_read_lock();
> > > +     ops = net_gro_receive(dev_offloads, ETH_P_TEB);
> > > +     rcu_read_unlock();
> > > +     if (!ops)
> > > +             goto out;
> >
> > Isn't rcu_read_lock already held here?
> > RCU read lock is always held in the receive handler path
>
> There is a critical section on receive, taken in
> netif_receive_skb_core, but gro code runs before that. All the
> existing gro handlers call rcu_read_lock.

Though if dev_gro_receive is the entry point for all of gro, then
all other handlers are ensured to be executed within its rcu
readside section.

Re: [PATCH net-next RFC 6/8] net: make gro configurable

[Willem de Bruijn]

On Fri, Sep 14, 2018 at 1:59 PM Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
>
> From: Willem de Bruijn <willemb@google.com>
>
> Add net_offload flag NET_OFF_FLAG_GRO_OFF. If set, a net_offload will
> not be used for gro receive processing.
>
> Also add sysctl helper proc_do_net_offload that toggles this flag and
> register sysctls net.{core,ipv4,ipv6}.gro
>
> Signed-off-by: Willem de Bruijn <willemb@google.com>
> ---
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 20d9552afd38..0fd5273bc931 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -154,6 +154,7 @@
>  #define GRO_MAX_HEAD (MAX_HEADER + 128)
>
>  static DEFINE_SPINLOCK(ptype_lock);
> +DEFINE_SPINLOCK(offload_lock);
>  struct list_head ptype_base[PTYPE_HASH_SIZE] __read_mostly;
>  struct list_head ptype_all __read_mostly;      /* Taps */
>  static struct list_head offload_base __read_mostly;
> diff --git a/net/core/sysctl_net_core.c b/net/core/sysctl_net_core.c
> index b1a2c5e38530..d2d72afdd9eb 100644
> --- a/net/core/sysctl_net_core.c
> +++ b/net/core/sysctl_net_core.c
> @@ -15,6 +15,7 @@
>  #include <linux/vmalloc.h>
>  #include <linux/init.h>
>  #include <linux/slab.h>
> +#include <linux/bitmap.h>
>
>  #include <net/ip.h>
>  #include <net/sock.h>
> @@ -34,6 +35,58 @@ static int net_msg_warn;     /* Unused, but still a sysctl */
>  int sysctl_fb_tunnels_only_for_init_net __read_mostly = 0;
>  EXPORT_SYMBOL(sysctl_fb_tunnels_only_for_init_net);
>
> +extern spinlock_t offload_lock;
> +
> +#define NET_OFF_TBL_LEN        256
> +
> +int proc_do_net_offload(struct ctl_table *ctl, int write, void __user *buffer,
> +                       size_t *lenp, loff_t *ppos)
> +{
> +       unsigned long bitmap[NET_OFF_TBL_LEN / (sizeof(unsigned long) << 3)];
> +       struct ctl_table tbl = { .maxlen = NET_OFF_TBL_LEN, .data = bitmap };
> +       unsigned long flag = (unsigned long) ctl->extra2;
> +       struct net_offload __rcu **offs = ctl->extra1;
> +       struct net_offload *off;
> +       int i, ret;
> +
> +       memset(bitmap, 0, sizeof(bitmap));
> +
> +       spin_lock(&offload_lock);
> +
> +       for (i = 0; i < tbl.maxlen; i++) {
> +               off = rcu_dereference_protected(offs[i], lockdep_is_held(&offload_lock));
> +               if (off && off->flags & flag) {

This does not actually work as is. No protocol will have this flag set
out of the box.

I was in the middle of rewriting some of this when it became topical,
so I sent it out for discussion. It's bound not to be the only bug of
the patchset as is. I'll work through them to get it back in shape.

[PATCH net-next RFC 7/8] udp: gro behind static key

[Willem de Bruijn]

From: Willem de Bruijn <willemb@google.com>

Avoid the socket lookup cost in udp_gro_receive if no socket has a
gro callback configured.

Signed-off-by: Willem de Bruijn <willemb@google.com>
---
 include/net/udp.h      | 2 ++
 net/ipv4/udp.c         | 2 +-
 net/ipv4/udp_offload.c | 2 +-
 net/ipv6/udp.c         | 2 +-
 net/ipv6/udp_offload.c | 2 +-
 5 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/include/net/udp.h b/include/net/udp.h
index 8482a990b0bb..9e82cb391dea 100644
--- a/include/net/udp.h
+++ b/include/net/udp.h
@@ -443,8 +443,10 @@ int udpv4_offload_init(void);
 
 void udp_init(void);
 
+DECLARE_STATIC_KEY_FALSE(udp_encap_needed_key);
 void udp_encap_enable(void);
 #if IS_ENABLED(CONFIG_IPV6)
+DECLARE_STATIC_KEY_FALSE(udpv6_encap_needed_key);
 void udpv6_encap_enable(void);
 #endif
 
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index f4e35b2ff8b8..bd873a5b8a86 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1889,7 +1889,7 @@ static int __udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
 	return 0;
 }
 
-static DEFINE_STATIC_KEY_FALSE(udp_encap_needed_key);
+DEFINE_STATIC_KEY_FALSE(udp_encap_needed_key);
 void udp_encap_enable(void)
 {
 	static_branch_enable(&udp_encap_needed_key);
diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
index 4f6aa95a9b12..f44fe328aa0f 100644
--- a/net/ipv4/udp_offload.c
+++ b/net/ipv4/udp_offload.c
@@ -405,7 +405,7 @@ static struct sk_buff *udp4_gro_receive(struct list_head *head,
 {
 	struct udphdr *uh = udp_gro_udphdr(skb);
 
-	if (unlikely(!uh))
+	if (unlikely(!uh) || !static_branch_unlikely(&udp_encap_needed_key))
 		goto flush;
 
 	/* Don't bother verifying checksum if we're going to flush anyway. */
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 83f4c77c79d8..d84672959f10 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -548,7 +548,7 @@ static __inline__ void udpv6_err(struct sk_buff *skb,
 	__udp6_lib_err(skb, opt, type, code, offset, info, &udp_table);
 }
 
-static DEFINE_STATIC_KEY_FALSE(udpv6_encap_needed_key);
+DEFINE_STATIC_KEY_FALSE(udpv6_encap_needed_key);
 void udpv6_encap_enable(void)
 {
 	static_branch_enable(&udpv6_encap_needed_key);
diff --git a/net/ipv6/udp_offload.c b/net/ipv6/udp_offload.c
index 2a41da0dd33f..e00f19c4a939 100644
--- a/net/ipv6/udp_offload.c
+++ b/net/ipv6/udp_offload.c
@@ -119,7 +119,7 @@ static struct sk_buff *udp6_gro_receive(struct list_head *head,
 {
 	struct udphdr *uh = udp_gro_udphdr(skb);
 
-	if (unlikely(!uh))
+	if (unlikely(!uh) || !static_branch_unlikely(&udpv6_encap_needed_key))
 		goto flush;
 
 	/* Don't bother verifying checksum if we're going to flush anyway. */
-- 
2.19.0.397.gdd90340f6a-goog

Re: [PATCH net-next RFC 7/8] udp: gro behind static key

[Subash Abhinov Kasiviswanathan]

On 2018-09-14 11:59, Willem de Bruijn wrote:
> From: Willem de Bruijn <willemb@google.com>
> 
> Avoid the socket lookup cost in udp_gro_receive if no socket has a
> gro callback configured.
> 
> Signed-off-by: Willem de Bruijn <willemb@google.com>
> diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
> index 4f6aa95a9b12..f44fe328aa0f 100644
> --- a/net/ipv4/udp_offload.c
> +++ b/net/ipv4/udp_offload.c
> @@ -405,7 +405,7 @@ static struct sk_buff *udp4_gro_receive(struct
> list_head *head,
>  {
>  	struct udphdr *uh = udp_gro_udphdr(skb);
> 
> -	if (unlikely(!uh))
> +	if (unlikely(!uh) ||
> !static_branch_unlikely(&udp_encap_needed_key))
>  		goto flush;
> 

Hi Willem

Does this need to be

diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
index 6dd3f0a..fcd5589 100644
--- a/net/ipv4/udp_offload.c
+++ b/net/ipv4/udp_offload.c
@@ -407,7 +407,7 @@ static struct sk_buff *udp4_gro_receive(struct 
list_head *head,
  {
         struct udphdr *uh = udp_gro_udphdr(skb);

-       if (unlikely(!uh) || 
!static_branch_unlikely(&udp_encap_needed_key))
+       if (unlikely(!uh) || 
static_branch_unlikely(&udp_encap_needed_key))
                 goto flush;

         /* Don't bother verifying checksum if we're going to flush 
anyway. */

I tried setting UDP_GRO socket option and I had to make this change to
exercise the udp_gro_receive_cb code path.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH net-next RFC 7/8] udp: gro behind static key

[Willem de Bruijn]

On Fri, Sep 14, 2018 at 11:37 PM Subash Abhinov Kasiviswanathan
<subashab@codeaurora.org> wrote:
>
> On 2018-09-14 11:59, Willem de Bruijn wrote:
> > From: Willem de Bruijn <willemb@google.com>
> >
> > Avoid the socket lookup cost in udp_gro_receive if no socket has a
> > gro callback configured.
> >
> > Signed-off-by: Willem de Bruijn <willemb@google.com>
> > diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
> > index 4f6aa95a9b12..f44fe328aa0f 100644
> > --- a/net/ipv4/udp_offload.c
> > +++ b/net/ipv4/udp_offload.c
> > @@ -405,7 +405,7 @@ static struct sk_buff *udp4_gro_receive(struct
> > list_head *head,
> >  {
> >       struct udphdr *uh = udp_gro_udphdr(skb);
> >
> > -     if (unlikely(!uh))
> > +     if (unlikely(!uh) ||
> > !static_branch_unlikely(&udp_encap_needed_key))
> >               goto flush;
> >
>
> Hi Willem
>
> Does this need to be
>
> diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
> index 6dd3f0a..fcd5589 100644
> --- a/net/ipv4/udp_offload.c
> +++ b/net/ipv4/udp_offload.c
> @@ -407,7 +407,7 @@ static struct sk_buff *udp4_gro_receive(struct
> list_head *head,
>   {
>          struct udphdr *uh = udp_gro_udphdr(skb);
>
> -       if (unlikely(!uh) ||
> !static_branch_unlikely(&udp_encap_needed_key))
> +       if (unlikely(!uh) ||
> static_branch_unlikely(&udp_encap_needed_key))
>                  goto flush;
>
>          /* Don't bother verifying checksum if we're going to flush
> anyway. */
>
> I tried setting UDP_GRO socket option and I had to make this change to
> exercise the udp_gro_receive_cb code path.

Thanks for trying it out, Subash.

The static_branch logic is correct as is. It skips the gro logic if
the static key is not enabled.

But there is indeed a bug. the UDP_GRO setsockopt has to enable
the static key. A full patch is more complete, but this should fix it for
now:

--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -2506,6 +2506,7 @@ int udp_lib_setsockopt(struct sock *sk, int
level, int optname,
                        if (!udp_sk(sk)->gro_receive) {
                                udp_sk(sk)->gro_complete = udp_gro_complete_cb;
                                udp_sk(sk)->gro_receive = udp_gro_receive_cb;
+                               udp_encap_enable();

sorry about that. I had tested the two patches independently, but
apparently not on top of each other.

Independent of udp gro, I tested the static key by running udp_rr
with perf record -a -g and looking for __udp4_lib_lookup. With the
patch, all calls are from __udp4_lib_rcv else udp_gro_receive also
shows up. I had to stress the porttable by opening ~32K ports.

Also, the simple patch to udpgso_bench_rx.c that I used to test
gro:

diff --git a/tools/testing/selftests/net/udpgso_bench_rx.c
b/tools/testing/selftests/net/udpgso_bench_rx.c
index 727cf67a3f75..35ba8567fc56 100644
--- a/tools/testing/selftests/net/udpgso_bench_rx.c
+++ b/tools/testing/selftests/net/udpgso_bench_rx.c
@@ -31,6 +31,11 @@
 #include <sys/wait.h>
 #include <unistd.h>

+#ifndef UDP_GRO
+#define UDP_GRO                104
+#endif
+
+static bool cfg_do_gro;
 static int  cfg_port           = 8000;
 static bool cfg_tcp;
 static bool cfg_verify;
@@ -89,6 +94,11 @@ static int do_socket(bool do_tcp)
        if (setsockopt(fd, SOL_SOCKET, SO_REUSEPORT, &val, sizeof(val)))
                error(1, errno, "setsockopt reuseport");

+       if (cfg_do_gro) {
+               if (setsockopt(fd, SOL_UDP, UDP_GRO, &val, sizeof(val)))
+                       error(1, errno, "setsockopt gro");
+       }
+
        addr.sin6_family =      PF_INET6;
        addr.sin6_port =        htons(cfg_port);
        addr.sin6_addr =        in6addr_any;
@@ -199,8 +209,11 @@ static void parse_opts(int argc, char **argv)
 {
        int c;

-       while ((c = getopt(argc, argv, "ptv")) != -1) {
+       while ((c = getopt(argc, argv, "gptv")) != -1) {
                switch (c) {
+               case 'g':
+                       cfg_do_gro = true;
+                       break;
                case 'p':
                        cfg_port = htons(strtoul(optarg, NULL, 0));
                        break;

Re: [PATCH net-next RFC 7/8] udp: gro behind static key

[Steffen Klassert]

On Fri, Sep 14, 2018 at 01:59:40PM -0400, Willem de Bruijn wrote:
> From: Willem de Bruijn <willemb@google.com>
> 
> Avoid the socket lookup cost in udp_gro_receive if no socket has a
> gro callback configured.
> 
> Signed-off-by: Willem de Bruijn <willemb@google.com>

...

> diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
> index 4f6aa95a9b12..f44fe328aa0f 100644
> --- a/net/ipv4/udp_offload.c
> +++ b/net/ipv4/udp_offload.c
> @@ -405,7 +405,7 @@ static struct sk_buff *udp4_gro_receive(struct list_head *head,
>  {
>  	struct udphdr *uh = udp_gro_udphdr(skb);
>  
> -	if (unlikely(!uh))
> +	if (unlikely(!uh) || !static_branch_unlikely(&udp_encap_needed_key))
>  		goto flush;

If you use udp_encap_needed_key to enalbe UDP GRO, then a UDP
encapsulation socket will enable it too. Not sure if this is
intentional.

That said, enabling UDP GRO on a UDP encapsulation socket
(ESP in UPD etc.) will fail badly as then encrypted ESP
packets might be merged together. So we somehow should
make sure that this does not happen.

Anyway, this reminds me that we can support GRO for
UDP encapsulation. It just requires separate GRO
callbacks for the different encapsulation types.

Re: [PATCH net-next RFC 7/8] udp: gro behind static key

[Willem de Bruijn]

On Mon, Sep 17, 2018 at 5:03 AM Steffen Klassert
<steffen.klassert@secunet.com> wrote:
>
> On Fri, Sep 14, 2018 at 01:59:40PM -0400, Willem de Bruijn wrote:
> > From: Willem de Bruijn <willemb@google.com>
> >
> > Avoid the socket lookup cost in udp_gro_receive if no socket has a
> > gro callback configured.
> >
> > Signed-off-by: Willem de Bruijn <willemb@google.com>
>
> ...
>
> > diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
> > index 4f6aa95a9b12..f44fe328aa0f 100644
> > --- a/net/ipv4/udp_offload.c
> > +++ b/net/ipv4/udp_offload.c
> > @@ -405,7 +405,7 @@ static struct sk_buff *udp4_gro_receive(struct list_head *head,
> >  {
> >       struct udphdr *uh = udp_gro_udphdr(skb);
> >
> > -     if (unlikely(!uh))
> > +     if (unlikely(!uh) || !static_branch_unlikely(&udp_encap_needed_key))
> >               goto flush;
>
> If you use udp_encap_needed_key to enalbe UDP GRO, then a UDP
> encapsulation socket will enable it too. Not sure if this is
> intentional.

Yes. That is already the case to a certain point. The function was
introduced with tunnels and is enabled by tunnels, but so far only
compiles out the encap_rcv() branch in udp_qeueue_rcv_skb.

With patch 7/8 it also toggles the GRO path. Critically, both are
enabled as soon as a tunnel is registered.

>
> That said, enabling UDP GRO on a UDP encapsulation socket
> (ESP in UPD etc.) will fail badly as then encrypted ESP
> packets might be merged together. So we somehow should
> make sure that this does not happen.

Absolutely. This initial implementation probably breaks UDP tunnels
badly. That needs to be addressed.

>
> Anyway, this reminds me that we can support GRO for
> UDP encapsulation. It just requires separate GRO
> callbacks for the different encapsulation types.

Re: [PATCH net-next RFC 7/8] udp: gro behind static key

[Paolo Abeni]

On Fri, 2018-09-14 at 13:59 -0400, Willem de Bruijn wrote:
> diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
> index 4f6aa95a9b12..f44fe328aa0f 100644
> --- a/net/ipv4/udp_offload.c
> +++ b/net/ipv4/udp_offload.c
> @@ -405,7 +405,7 @@ static struct sk_buff *udp4_gro_receive(struct list_head *head,
>  {
>  	struct udphdr *uh = udp_gro_udphdr(skb);
>  
> -	if (unlikely(!uh))
> +	if (unlikely(!uh) || !static_branch_unlikely(&udp_encap_needed_key))
>  		goto flush;
>  
>  	/* Don't bother verifying checksum if we're going to flush anyway. */

If I read this correctly, once udp_encap_needed_key is enabled, it will
never be turned off, because the tunnel and encap socket shut down does
not cope with udp_encap_needed_key.

Perhaps we should take care of that, too.

Cheers,

Paolo

Re: [PATCH net-next RFC 7/8] udp: gro behind static key

[Willem de Bruijn]

On Mon, Sep 17, 2018 at 6:24 AM Paolo Abeni <pabeni@redhat.com> wrote:
>
> On Fri, 2018-09-14 at 13:59 -0400, Willem de Bruijn wrote:
> > diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
> > index 4f6aa95a9b12..f44fe328aa0f 100644
> > --- a/net/ipv4/udp_offload.c
> > +++ b/net/ipv4/udp_offload.c
> > @@ -405,7 +405,7 @@ static struct sk_buff *udp4_gro_receive(struct list_head *head,
> >  {
> >       struct udphdr *uh = udp_gro_udphdr(skb);
> >
> > -     if (unlikely(!uh))
> > +     if (unlikely(!uh) || !static_branch_unlikely(&udp_encap_needed_key))
> >               goto flush;
> >
> >       /* Don't bother verifying checksum if we're going to flush anyway. */
>
> If I read this correctly, once udp_encap_needed_key is enabled, it will
> never be turned off, because the tunnel and encap socket shut down does
> not cope with udp_encap_needed_key.
>
> Perhaps we should take care of that, too.

Agreed. For now I reused what's already there, but I can extend
that with refcounting using static_branch_inc/static_branch_dec.

Re: [PATCH net-next RFC 7/8] udp: gro behind static key

[Steffen Klassert]

On Fri, Sep 14, 2018 at 01:59:40PM -0400, Willem de Bruijn wrote:
> From: Willem de Bruijn <willemb@google.com>
> 
> Avoid the socket lookup cost in udp_gro_receive if no socket has a
> gro callback configured.

It would be nice if we could do GRO not just for GRO configured
sockets, but also for flows that are going to be IPsec transformed
or directly forwarded.

Maybe in case that forwarding is enabled on the receiving device,
inet_gro_receive() could do a route lookup and allow GRO if the
route lookup returned at forwarding route.

For flows that are likely software segmented after that, it
would be worth to build packet chains insted of merging the
payload. Packets of the same flow could travel together, but
it would save the cost of the packet merging and segmenting.
This could be done similar to what I proposed for the list
receive case:

https://www.spinics.net/lists/netdev/msg522706.html

How GRO should be done could be even configured
by replacing the net_offload pointer similar
to what Paolo propsed in his pachset with
the inet_update_offload() function.

Re: [PATCH net-next RFC 7/8] udp: gro behind static key

[Willem de Bruijn]

On Mon, Sep 17, 2018 at 6:37 AM Steffen Klassert
<steffen.klassert@secunet.com> wrote:
>
> On Fri, Sep 14, 2018 at 01:59:40PM -0400, Willem de Bruijn wrote:
> > From: Willem de Bruijn <willemb@google.com>
> >
> > Avoid the socket lookup cost in udp_gro_receive if no socket has a
> > gro callback configured.
>
> It would be nice if we could do GRO not just for GRO configured
> sockets, but also for flows that are going to be IPsec transformed
> or directly forwarded.

I thought about that, as we have GSO. An egregious hack enables
GRO for all registered local sockets that support it and for any flow
for which no local socket is registered:

@@ -365,11 +369,13 @@ struct sk_buff *udp_gro_receive(struct list_head
*head, struct sk_buff *skb,
        rcu_read_lock();
        sk = (*lookup)(skb, uh->source, uh->dest);

-       if (sk && udp_sk(sk)->gro_receive)
-               goto unflush;
-       goto out_unlock;
+       if (!sk)
+               gro_receive_cb = udp_gro_receive_cb;
+       else if (!udp_sk(sk)->gro_receive)
+               goto out_unlock;
+       else
+               gro_receive_cb = udp_sk(sk)->gro_receive;

@@ -392,7 +398,7 @@ struct sk_buff *udp_gro_receive(struct list_head
*head, struct sk_buff *skb,

        skb_gro_pull(skb, sizeof(struct udphdr)); /* pull
encapsulating udp header */
        skb_gro_postpull_rcsum(skb, uh, sizeof(struct udphdr));
-       pp = call_gro_receive_sk(udp_sk(sk)->gro_receive, sk, head, skb);
+       pp = call_gro_receive_sk(gro_receive_cb, sk, head, skb);


But not having a local socket does not imply forwarding path, of course.

>
> Maybe in case that forwarding is enabled on the receiving device,
> inet_gro_receive() could do a route lookup and allow GRO if the
> route lookup returned at forwarding route.

That's a better solution, if the cost is acceptable. We do have to
be careful against increasing per packet cycle cost in this path
given that it's a possible vector for DoS attempts.

> For flows that are likely software segmented after that, it
> would be worth to build packet chains insted of merging the
> payload. Packets of the same flow could travel together, but
> it would save the cost of the packet merging and segmenting.

With software GSO that is faster, as it would have to allocate
all the separate segment skbs in skb_segment later. Though
there is some complexity if MTUs differ.

With hardware UDP GSO, having a single skb will be cheaper in
the forwarding path. Using napi_gro_frags, device drivers really
do only end up allocating one skb for the GSO packet.

> This could be done similar to what I proposed for the list
> receive case:
>
> https://www.spinics.net/lists/netdev/msg522706.html
>
> How GRO should be done could be even configured
> by replacing the net_offload pointer similar
> to what Paolo propsed in his pachset with
> the inet_update_offload() function.

Right. The above hack also already has to use two distinct
callback assignments.

Re: [PATCH net-next RFC 7/8] udp: gro behind static key

[Steffen Klassert]

On Mon, Sep 17, 2018 at 10:19:22AM -0400, Willem de Bruijn wrote:
> On Mon, Sep 17, 2018 at 6:37 AM Steffen Klassert
> <steffen.klassert@secunet.com> wrote:
> >
> > Maybe in case that forwarding is enabled on the receiving device,
> > inet_gro_receive() could do a route lookup and allow GRO if the
> > route lookup returned at forwarding route.
> 
> That's a better solution, if the cost is acceptable. We do have to
> be careful against increasing per packet cycle cost in this path
> given that it's a possible vector for DoS attempts.

I played with this already and I have not seen any significant
overhead when doing a route lookup in GRO. We have to do a route
lookup anyway, so it should not matter too much if we do it here
or later in the IP layer.

> 
> > For flows that are likely software segmented after that, it
> > would be worth to build packet chains insted of merging the
> > payload. Packets of the same flow could travel together, but
> > it would save the cost of the packet merging and segmenting.
> 
> With software GSO that is faster, as it would have to allocate
> all the separate segment skbs in skb_segment later. Though
> there is some complexity if MTUs differ.

This can be handled the same way as it is done for TCP GSO
packets. If the size of the original packets is bigger than
the outgoing MTU, we either signal ICMP_FRAG_NEEDED to the
sender, or split the chain back into single packets and do
fragmentation on them.

> 
> With hardware UDP GSO, having a single skb will be cheaper in
> the forwarding path. Using napi_gro_frags, device drivers really
> do only end up allocating one skb for the GSO packet.

Right, this is why it would be nice to have this
configurable.

[PATCH net-next RFC 8/8] udp: add gro

[Willem de Bruijn]

From: Willem de Bruijn <willemb@google.com>

Very rough initial version of udp gro, for discussion purpose only at
this point.

Among others it
- lacks the cmsg UDP_SEGMENT to return gso_size
- probably breaks udp tunnels
- hard breaks at 40 segments
- does not allow a last segment of unequal size

Signed-off-by: Willem de Bruijn <willemb@google.com>
---
 include/uapi/linux/udp.h |  1 +
 net/ipv4/udp.c           | 71 ++++++++++++++++++++++++++++++++++++++++
 net/ipv4/udp_offload.c   | 11 +++----
 3 files changed, 76 insertions(+), 7 deletions(-)

diff --git a/include/uapi/linux/udp.h b/include/uapi/linux/udp.h
index 09d00f8c442b..7fda3e8c7fcf 100644
--- a/include/uapi/linux/udp.h
+++ b/include/uapi/linux/udp.h
@@ -33,6 +33,7 @@ struct udphdr {
 #define UDP_NO_CHECK6_TX 101	/* Disable sending checksum for UDP6X */
 #define UDP_NO_CHECK6_RX 102	/* Disable accpeting checksum for UDP6 */
 #define UDP_SEGMENT	103	/* Set GSO segmentation size */
+#define UDP_GRO		104	/* Enable GRO */
 
 /* UDP encapsulation types */
 #define UDP_ENCAP_ESPINUDP_NON_IKE	1 /* draft-ietf-ipsec-nat-t-ike-00/01 */
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index bd873a5b8a86..ae49c08e6225 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -2387,6 +2387,51 @@ void udp_destroy_sock(struct sock *sk)
 	}
 }
 
+static struct sk_buff *udp_gro_receive_cb(struct sock *sk,
+					  struct list_head *head,
+					  struct sk_buff *skb)
+{
+	struct sk_buff *p;
+	unsigned int off;
+
+	off = skb_gro_offset(skb) - sizeof(struct udphdr);
+
+	list_for_each_entry(p, head, list) {
+		if (!NAPI_GRO_CB(p)->same_flow)
+			continue;
+
+		/* TODO: for UDP_GRO: match size unless last segment */
+		if (NAPI_GRO_CB(p)->flush)
+			break;
+
+		/* TODO: look into ip id check */
+		if (skb_gro_receive(p, skb)) {
+			NAPI_GRO_CB(skb)->flush = 1;
+			break;
+		}
+
+		if (NAPI_GRO_CB(skb)->count >= 40) {
+			return p;
+		}
+
+		return NULL;
+	}
+
+	return NULL;
+}
+
+static int udp_gro_complete_cb(struct sock *sk, struct sk_buff *skb,
+			       int nhoff)
+{
+	skb->csum_start = (unsigned char *)udp_hdr(skb) - skb->head;
+	skb->csum_offset = offsetof(struct udphdr, check);
+	skb->ip_summed = CHECKSUM_PARTIAL;
+
+	skb_shinfo(skb)->gso_segs = NAPI_GRO_CB(skb)->count;
+
+	return 0;
+}
+
 /*
  *	Socket option code for UDP
  */
@@ -2450,6 +2495,32 @@ int udp_lib_setsockopt(struct sock *sk, int level, int optname,
 		up->gso_size = val;
 		break;
 
+	case UDP_GRO:
+	{
+		if (val < 0 || val > 1)
+			return -EINVAL;
+
+		lock_sock(sk);
+		if (val) {
+
+			if (!udp_sk(sk)->gro_receive) {
+				udp_sk(sk)->gro_complete = udp_gro_complete_cb;
+				udp_sk(sk)->gro_receive = udp_gro_receive_cb;
+			} else {
+				err = -EALREADY;
+			}
+		} else {
+			if (udp_sk(sk)->gro_receive) {
+				udp_sk(sk)->gro_receive = NULL;
+				udp_sk(sk)->gro_complete = NULL;
+			} else {
+				err = -ENOENT;
+			}
+		}
+		release_sock(sk);
+		break;
+	}
+
 	/*
 	 * 	UDP-Lite's partial checksum coverage (RFC 3828).
 	 */
diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
index f44fe328aa0f..6dd3f0a28b5e 100644
--- a/net/ipv4/udp_offload.c
+++ b/net/ipv4/udp_offload.c
@@ -386,6 +386,8 @@ struct sk_buff *udp_gro_receive(struct list_head *head, struct sk_buff *skb,
 			NAPI_GRO_CB(p)->same_flow = 0;
 			continue;
 		}
+
+		/* TODO: for UDP_GRO: match size */
 	}
 
 	skb_gro_pull(skb, sizeof(struct udphdr)); /* pull encapsulating udp header */
@@ -437,11 +439,6 @@ int udp_gro_complete(struct sk_buff *skb, int nhoff,
 
 	uh->len = newlen;
 
-	/* Set encapsulation before calling into inner gro_complete() functions
-	 * to make them set up the inner offsets.
-	 */
-	skb->encapsulation = 1;
-
 	rcu_read_lock();
 	sk = (*lookup)(skb, uh->source, uh->dest);
 	if (sk && udp_sk(sk)->gro_complete)
@@ -462,11 +459,11 @@ static int udp4_gro_complete(struct sk_buff *skb, int nhoff)
 	struct udphdr *uh = (struct udphdr *)(skb->data + nhoff);
 
 	if (uh->check) {
-		skb_shinfo(skb)->gso_type |= SKB_GSO_UDP_TUNNEL_CSUM;
+		skb_shinfo(skb)->gso_type |= SKB_GSO_UDP_L4;
 		uh->check = ~udp_v4_check(skb->len - nhoff, iph->saddr,
 					  iph->daddr, 0);
 	} else {
-		skb_shinfo(skb)->gso_type |= SKB_GSO_UDP_TUNNEL;
+		skb_shinfo(skb)->gso_type |= SKB_GSO_UDP_L4;
 	}
 
 	return udp_gro_complete(skb, nhoff, udp4_lib_lookup_skb);
-- 
2.19.0.397.gdd90340f6a-goog

Re: [PATCH net-next RFC 0/8] udp and configurable gro

[Paolo Abeni]

Hi all,

On Fri, 2018-09-14 at 13:59 -0400, Willem de Bruijn wrote:
> This is a *very rough* draft. Mainly for discussion while we also
> look at another partially overlapping approach [1].

I'm wondering how we go on from this ? I'm fine with either approaches.

Also, I'm interested in [try to] enable GRO/GSO batching in the
forwarding path, as you outlined initially in the GSO series
submission. That should cover Steffen use-case, too, right? 

Cheers,

Paolo

Re: [PATCH net-next RFC 0/8] udp and configurable gro

[Willem de Bruijn]

On Fri, Oct 5, 2018 at 9:53 AM Paolo Abeni <pabeni@redhat.com> wrote:
>
> Hi all,
>
> On Fri, 2018-09-14 at 13:59 -0400, Willem de Bruijn wrote:
> > This is a *very rough* draft. Mainly for discussion while we also
> > look at another partially overlapping approach [1].
>
> I'm wondering how we go on from this ? I'm fine with either approaches.

Let me send the udp gro static_key patch. Then we don't need
the enable udp on demand logic (patch 2/4).

Your implementation of GRO is more fleshed out (patch 3/4) than
my quick hack. My only request would be to use a separate
UDP_GRO socket option instead of adding this to the existing
UDP_SEGMENT.

Sounds good?

> Also, I'm interested in [try to] enable GRO/GSO batching in the
> forwarding path, as you outlined initially in the GSO series
> submission. That should cover Steffen use-case, too, right?

Great. Indeed. Though there is some unresolved discussion on
one large gso skb vs frag list. There has been various concerns
around the use of frag lists for GSO in the past, and it does not
match h/w offload. So I think the answer would be the first unless
the second proves considerably faster (in which case it could also
be added later as optimization).

Re: [PATCH net-next RFC 0/8] udp and configurable gro

[Paolo Abeni]

Hi,

Thank you for the prompt reply!

On Fri, 2018-10-05 at 10:41 -0400, Willem de Bruijn wrote:
> On Fri, Oct 5, 2018 at 9:53 AM Paolo Abeni <pabeni@redhat.com> wrote:
> > 
> > Hi all,
> > 
> > On Fri, 2018-09-14 at 13:59 -0400, Willem de Bruijn wrote:
> > > This is a *very rough* draft. Mainly for discussion while we also
> > > look at another partially overlapping approach [1].
> > 
> > I'm wondering how we go on from this ? I'm fine with either approaches.
> 
> Let me send the udp gro static_key patch.

Would love that. We need to care of key decr, too (and possibly don't
be fooled by encap_rcv() users).

> Then we don't need the enable udp on demand logic (patch 2/4).

ok.

> Your implementation of GRO is more fleshed out (patch 3/4) than
> my quick hack. My only request would be to use a separate
> UDP_GRO socket option instead of adding this to the existing
> UDP_SEGMENT.
> 
> Sounds good?

Indeed!
I need also to add a cmsg to expose to the user the skb gro_size, and
some test cases. Locally I'm [ab-]using the GRO functionality
introduced recently on veth to test the code in a namespace pair
(attaching a dummy XDP program to the RX-side veth). I'm not sure if
that could fit a selftest.

> > Also, I'm interested in [try to] enable GRO/GSO batching in the
> > forwarding path, as you outlined initially in the GSO series
> > submission. That should cover Steffen use-case, too, right?
> 
> Great. Indeed. Though there is some unresolved discussion on
> one large gso skb vs frag list. There has been various concerns
> around the use of frag lists for GSO in the past, and it does not
> match h/w offload. So I think the answer would be the first unless
> the second proves considerably faster (in which case it could also
> be added later as optimization).

Agreed.

Let's try the first step first ;)

Final but relevant note: I'll try my best to avoid delaying this, but
lately I tend to be pre-empted by other tasks, it's difficult for me to
assure a deadline here.

Cheers,

Paolo

Re: [PATCH net-next RFC 0/8] udp and configurable gro

[Willem de Bruijn]

On Fri, Oct 5, 2018 at 11:30 AM Paolo Abeni <pabeni@redhat.com> wrote:
>
> Hi,
>
> Thank you for the prompt reply!
>

Not at all. Thanks for moving this forward :)

> On Fri, 2018-10-05 at 10:41 -0400, Willem de Bruijn wrote:
> > On Fri, Oct 5, 2018 at 9:53 AM Paolo Abeni <pabeni@redhat.com> wrote:
> > >
> > > Hi all,
> > >
> > > On Fri, 2018-09-14 at 13:59 -0400, Willem de Bruijn wrote:
> > > > This is a *very rough* draft. Mainly for discussion while we also
> > > > look at another partially overlapping approach [1].
> > >
> > > I'm wondering how we go on from this ? I'm fine with either approaches.
> >
> > Let me send the udp gro static_key patch.
>
> Would love that. We need to care of key decr, too (and possibly don't
> be fooled by encap_rcv() users).

I just sent  http://patchwork.ozlabs.org/patch/979525/

Right now all users are those that call setup_udp_tunnel_sock
to register encap_rcv.

If accepted, I'll add a separate patch to decrement the key. That's
probably in udp_tunnel_sock_release, but I need to take a closer
look.

> > Then we don't need the enable udp on demand logic (patch 2/4).
>
> ok.
>
> > Your implementation of GRO is more fleshed out (patch 3/4) than
> > my quick hack. My only request would be to use a separate
> > UDP_GRO socket option instead of adding this to the existing
> > UDP_SEGMENT.
> >
> > Sounds good?
>
> Indeed!
> I need also to add a cmsg to expose to the user the skb gro_size, and
> some test cases. Locally I'm [ab-]using the GRO functionality
> introduced recently on veth to test the code in a namespace pair
> (attaching a dummy XDP program to the RX-side veth). I'm not sure if
> that could fit a selftest.

Very nice. Yes, veth only implements napi in xdp mode.

>
> > > Also, I'm interested in [try to] enable GRO/GSO batching in the
> > > forwarding path, as you outlined initially in the GSO series
> > > submission. That should cover Steffen use-case, too, right?
> >
> > Great. Indeed. Though there is some unresolved discussion on
> > one large gso skb vs frag list. There has been various concerns
> > around the use of frag lists for GSO in the past, and it does not
> > match h/w offload. So I think the answer would be the first unless
> > the second proves considerably faster (in which case it could also
> > be added later as optimization).
>
> Agreed.
>
> Let's try the first step first ;)
>
> Final but relevant note: I'll try my best to avoid delaying this, but
> lately I tend to be pre-empted by other tasks, it's difficult for me to
> assure a deadline here.
>
> Cheers,
>
> Paolo
>

Re: [PATCH net-next RFC 0/8] udp and configurable gro

[Paolo Abeni]

On Fri, 2018-10-05 at 11:45 -0400, Willem de Bruijn wrote:
> On Fri, Oct 5, 2018 at 11:30 AM Paolo Abeni <pabeni@redhat.com> wrote:
> > Would love that. We need to care of key decr, too (and possibly don't
> > be fooled by encap_rcv() users).
> 
> I just sent  http://patchwork.ozlabs.org/patch/979525/
> 
> Right now all users are those that call setup_udp_tunnel_sock
> to register encap_rcv.

plus setsockopt(UDP_ENCAP)

> If accepted, I'll add a separate patch to decrement the key. That's
> probably in udp_tunnel_sock_release, but I need to take a closer
> look.

l2tp calls setup_udp_tunnel_sock() but don't use
udp_tunnel_sock_release(). Possibly it would be safer checking for:

up->encap_type || up(sk)->gro_receive

in udp_destroy_sock()

Cheers,

Paolo

Re: [PATCH net-next RFC 0/8] udp and configurable gro

[Willem de Bruijn]

On Fri, Oct 5, 2018 at 12:05 PM Paolo Abeni <pabeni@redhat.com> wrote:
>
> On Fri, 2018-10-05 at 11:45 -0400, Willem de Bruijn wrote:
> > On Fri, Oct 5, 2018 at 11:30 AM Paolo Abeni <pabeni@redhat.com> wrote:
> > > Would love that. We need to care of key decr, too (and possibly don't
> > > be fooled by encap_rcv() users).
> >
> > I just sent  http://patchwork.ozlabs.org/patch/979525/
> >
> > Right now all users are those that call setup_udp_tunnel_sock
> > to register encap_rcv.
>
> plus setsockopt(UDP_ENCAP)
>
> > If accepted, I'll add a separate patch to decrement the key. That's
> > probably in udp_tunnel_sock_release, but I need to take a closer
> > look.
>
> l2tp calls setup_udp_tunnel_sock() but don't use
> udp_tunnel_sock_release(). Possibly it would be safer checking for:
>
> up->encap_type || up(sk)->gro_receive
>
> in udp_destroy_sock()

Ah indeed. And gtp might be another example.

Thanks!

Re: [PATCH net-next RFC 0/8] udp and configurable gro

[Steffen Klassert]

On Fri, Oct 05, 2018 at 10:41:47AM -0400, Willem de Bruijn wrote:
> On Fri, Oct 5, 2018 at 9:53 AM Paolo Abeni <pabeni@redhat.com> wrote:
> >
> > Hi all,
> >
> > On Fri, 2018-09-14 at 13:59 -0400, Willem de Bruijn wrote:
> > > This is a *very rough* draft. Mainly for discussion while we also
> > > look at another partially overlapping approach [1].
> >
> > I'm wondering how we go on from this ? I'm fine with either approaches.
> 
> Let me send the udp gro static_key patch. Then we don't need
> the enable udp on demand logic (patch 2/4).
> 
> Your implementation of GRO is more fleshed out (patch 3/4) than
> my quick hack. My only request would be to use a separate
> UDP_GRO socket option instead of adding this to the existing
> UDP_SEGMENT.
> 
> Sounds good?
> 
> > Also, I'm interested in [try to] enable GRO/GSO batching in the
> > forwarding path, as you outlined initially in the GSO series
> > submission. That should cover Steffen use-case, too, right?
> 
> Great. Indeed. Though there is some unresolved discussion on
> one large gso skb vs frag list. There has been various concerns
> around the use of frag lists for GSO in the past, and it does not
> match h/w offload. So I think the answer would be the first unless
> the second proves considerably faster (in which case it could also
> be added later as optimization).

I think it depends a bit on the usecase and hardware etc.
if the first or the second approach is faster. So it would
be good if we can choose which one to use depending on that.

For local socket receiving, building big GSO packets
is likely faster than the chaining method.

But on forwarding the chaining method might be faster
because we don't have the overhead of creating GSO
packets and of segmenting them back to their native
form (at least as long as we don't have NICs that
support hardware UDP GSO). Same applies to packets
that undergo IPsec transformation.

Another thing where the chaining method could be intersting
is when we receive already big LRO or HW GRO packets from the
NIC. Packets of the same flow could still travel together
through the stack with the chaining method. I've never tried
this, though. For now it is just an idea.

I have the code for the chaining mehthod here, I'd just need
some method to hook it in. Maybe it could be done with
some sort of an inet_update_offload() as Paolo already
propsed in his pachset, or we could make it configurable
per device...