[PATCH RFC v3 0/5] Support fraglist GRO/GSO

[PATCH RFC v3 0/5] Support fraglist GRO/GSO

[Steffen Klassert]

This patchset adds support to do GRO/GSO by chaining packets
of the same flow at the SKB frag_list pointer. This avoids
the overhead to merge payloads into one big packet, and
on the other end, if GSO is needed it avoids the overhead
of splitting the big packet back to the native form.

Patch 1 Enables UDP GRO by default.

Patch 2 adds a netdev feature flag to enable listifyed GRO,
this implements one of the configuration options discussed
at netconf 2019.

Patch 3 adds a netdev software feature set that defaults to off
and assigns the new listifyed GRO feature flag to it.

Patch 4 adds the core infrastructure to do fraglist GRO/GSO.

Patch 5 enables UDP to use fraglist GRO/GSO if configured and no
GRO supported socket is found.

I have only meaningful forwarding performance measurements.
I did some tests for the local receive path with netperf and iperf,
but in this case the sender that generates the packets is the
bottleneck. So the benchmarks are not that meaningful for the
receive path.

Paolo Abeni did some benchmarks of the local receive path for the v2
version of this pachset, results can be found here:

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

I used my IPsec forwarding test setup for the performance measurements:

           ------------         ------------
        -->| router 1 |-------->| router 2 |--
        |  ------------         ------------  |
        |                                     |
        |       --------------------          |
        --------|Spirent Testcenter|<----------
                --------------------

net-next (September 7th):

Single stream UDP frame size 1460 Bytes: 1.161.000 fps (13.5 Gbps).

----------------------------------------------------------------------

net-next (September 7th) + standard UDP GRO/GSO:

Single stream UDP frame size 1460 Bytes: 1.801.000 fps (21 Gbps).

----------------------------------------------------------------------

net-next (September 7th) + fraglist UDP GRO/GSO:

Single stream UDP frame size 1460 Bytes: 2.860.000 fps (33.4 Gbps).

-----------------------------------------------------------------------

Changes from v1:

- Add IPv6 support.
- Split patchset to enable UDP GRO by default before adding
  fraglist GRO support.
- Mark fraglist GRO packets as CHECKSUM_NONE.
- Take a refcount on the first segment skb when doing fraglist
  segmentation. With this we can use the same error handling
  path as with standard segmentation.

Changes from v2:

- Add a netdev feature flag to configure listifyed GRO.
- Fix UDP GRO enabling for IPv6.
- Fix a rcu_read_lock() imbalance.
- Fix error path in skb_segment_list().

[PATCH RFC v3 1/5] UDP: enable GRO by default.

[Steffen Klassert]

This patch enables UDP GRO regardless if a GRO capable
socket is present. With this GRO is done by default
for the local input and forwarding path.

Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 include/net/udp.h      |  2 +-
 net/ipv4/udp_offload.c | 33 ++++++++++++++-------------------
 net/ipv6/udp_offload.c | 10 ++++++++--
 3 files changed, 23 insertions(+), 22 deletions(-)

diff --git a/include/net/udp.h b/include/net/udp.h
index bad74f780831..44e0e52b585c 100644
--- a/include/net/udp.h
+++ b/include/net/udp.h
@@ -167,7 +167,7 @@ typedef struct sock *(*udp_lookup_t)(struct sk_buff *skb, __be16 sport,
 				     __be16 dport);
 
 struct sk_buff *udp_gro_receive(struct list_head *head, struct sk_buff *skb,
-				struct udphdr *uh, udp_lookup_t lookup);
+				struct udphdr *uh, struct sock *sk);
 int udp_gro_complete(struct sk_buff *skb, int nhoff, udp_lookup_t lookup);
 
 struct sk_buff *__udp_gso_segment(struct sk_buff *gso_skb,
diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
index a3908e55ed89..e0b835890507 100644
--- a/net/ipv4/udp_offload.c
+++ b/net/ipv4/udp_offload.c
@@ -401,35 +401,24 @@ static struct sk_buff *udp_gro_receive_segment(struct list_head *head,
 	return NULL;
 }
 
-INDIRECT_CALLABLE_DECLARE(struct sock *udp6_lib_lookup_skb(struct sk_buff *skb,
-						   __be16 sport, __be16 dport));
 struct sk_buff *udp_gro_receive(struct list_head *head, struct sk_buff *skb,
-				struct udphdr *uh, udp_lookup_t lookup)
+				struct udphdr *uh, struct sock *sk)
 {
 	struct sk_buff *pp = NULL;
 	struct sk_buff *p;
 	struct udphdr *uh2;
 	unsigned int off = skb_gro_offset(skb);
 	int flush = 1;
-	struct sock *sk;
-
-	rcu_read_lock();
-	sk = INDIRECT_CALL_INET(lookup, udp6_lib_lookup_skb,
-				udp4_lib_lookup_skb, skb, uh->source, uh->dest);
-	if (!sk)
-		goto out_unlock;
 
-	if (udp_sk(sk)->gro_enabled) {
+	if (!sk || !udp_sk(sk)->gro_receive) {
 		pp = call_gro_receive(udp_gro_receive_segment, head, skb);
-		rcu_read_unlock();
 		return pp;
 	}
 
 	if (NAPI_GRO_CB(skb)->encap_mark ||
 	    (skb->ip_summed != CHECKSUM_PARTIAL &&
 	     NAPI_GRO_CB(skb)->csum_cnt == 0 &&
-	     !NAPI_GRO_CB(skb)->csum_valid) ||
-	    !udp_sk(sk)->gro_receive)
+	     !NAPI_GRO_CB(skb)->csum_valid))
 		goto out_unlock;
 
 	/* mark that this skb passed once through the tunnel gro layer */
@@ -468,8 +457,10 @@ INDIRECT_CALLABLE_SCOPE
 struct sk_buff *udp4_gro_receive(struct list_head *head, struct sk_buff *skb)
 {
 	struct udphdr *uh = udp_gro_udphdr(skb);
+	struct sk_buff *pp;
+	struct sock *sk;
 
-	if (unlikely(!uh) || !static_branch_unlikely(&udp_encap_needed_key))
+	if (unlikely(!uh))
 		goto flush;
 
 	/* Don't bother verifying checksum if we're going to flush anyway. */
@@ -484,7 +475,11 @@ struct sk_buff *udp4_gro_receive(struct list_head *head, struct sk_buff *skb)
 					     inet_gro_compute_pseudo);
 skip:
 	NAPI_GRO_CB(skb)->is_ipv6 = 0;
-	return udp_gro_receive(head, skb, uh, udp4_lib_lookup_skb);
+	rcu_read_lock();
+	sk = static_branch_unlikely(&udp_encap_needed_key) ? udp4_lib_lookup_skb(skb, uh->source, uh->dest) : NULL;
+	pp = udp_gro_receive(head, skb, uh, sk);
+	rcu_read_unlock();
+	return pp;
 
 flush:
 	NAPI_GRO_CB(skb)->flush = 1;
@@ -517,9 +512,7 @@ int udp_gro_complete(struct sk_buff *skb, int nhoff,
 	rcu_read_lock();
 	sk = INDIRECT_CALL_INET(lookup, udp6_lib_lookup_skb,
 				udp4_lib_lookup_skb, skb, uh->source, uh->dest);
-	if (sk && udp_sk(sk)->gro_enabled) {
-		err = udp_gro_complete_segment(skb);
-	} else if (sk && udp_sk(sk)->gro_complete) {
+	if (sk && udp_sk(sk)->gro_complete) {
 		skb_shinfo(skb)->gso_type = uh->check ? SKB_GSO_UDP_TUNNEL_CSUM
 					: SKB_GSO_UDP_TUNNEL;
 
@@ -529,6 +522,8 @@ int udp_gro_complete(struct sk_buff *skb, int nhoff,
 		skb->encapsulation = 1;
 		err = udp_sk(sk)->gro_complete(sk, skb,
 				nhoff + sizeof(struct udphdr));
+	} else {
+		err = udp_gro_complete_segment(skb);
 	}
 	rcu_read_unlock();
 
diff --git a/net/ipv6/udp_offload.c b/net/ipv6/udp_offload.c
index 64b8f05d6735..47a3b1348371 100644
--- a/net/ipv6/udp_offload.c
+++ b/net/ipv6/udp_offload.c
@@ -115,8 +115,10 @@ INDIRECT_CALLABLE_SCOPE
 struct sk_buff *udp6_gro_receive(struct list_head *head, struct sk_buff *skb)
 {
 	struct udphdr *uh = udp_gro_udphdr(skb);
+	struct sk_buff *pp;
+	struct sock *sk;
 
-	if (unlikely(!uh) || !static_branch_unlikely(&udpv6_encap_needed_key))
+	if (unlikely(!uh))
 		goto flush;
 
 	/* Don't bother verifying checksum if we're going to flush anyway. */
@@ -132,7 +134,11 @@ struct sk_buff *udp6_gro_receive(struct list_head *head, struct sk_buff *skb)
 
 skip:
 	NAPI_GRO_CB(skb)->is_ipv6 = 1;
-	return udp_gro_receive(head, skb, uh, udp6_lib_lookup_skb);
+	rcu_read_lock();
+	sk = static_branch_unlikely(&udp_encap_needed_key) ? udp6_lib_lookup_skb(skb, uh->source, uh->dest) : NULL;
+	pp = udp_gro_receive(head, skb, uh, sk);
+	rcu_read_unlock();
+	return pp;
 
 flush:
 	NAPI_GRO_CB(skb)->flush = 1;
-- 
2.17.1

[PATCH RFC v3 2/5] net: Add NETIF_F_GRO_LIST feature

[Steffen Klassert]

This adds a new NETIF_F_GRO_LIST feature flag. I will be used
to configure listfyed GRO what will be implemented with some
followup paches.

Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 include/linux/netdev_features.h | 2 ++
 net/core/ethtool.c              | 1 +
 2 files changed, 3 insertions(+)

diff --git a/include/linux/netdev_features.h b/include/linux/netdev_features.h
index 4b19c544c59a..1b6baa1b6fe9 100644
--- a/include/linux/netdev_features.h
+++ b/include/linux/netdev_features.h
@@ -80,6 +80,7 @@ enum {
 
 	NETIF_F_GRO_HW_BIT,		/* Hardware Generic receive offload */
 	NETIF_F_HW_TLS_RECORD_BIT,	/* Offload TLS record */
+	NETIF_F_GRO_LIST_BIT,		/* Listifyed GRO */
 
 	/*
 	 * Add your fresh new feature above and remember to update
@@ -150,6 +151,7 @@ enum {
 #define NETIF_F_GSO_UDP_L4	__NETIF_F(GSO_UDP_L4)
 #define NETIF_F_HW_TLS_TX	__NETIF_F(HW_TLS_TX)
 #define NETIF_F_HW_TLS_RX	__NETIF_F(HW_TLS_RX)
+#define NETIF_F_GRO_LIST	__NETIF_F(GRO_LIST)
 
 /* Finds the next feature with the highest number of the range of start till 0.
  */
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 6288e69e94fc..ee8d2b58c2d7 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -111,6 +111,7 @@ static const char netdev_features_strings[NETDEV_FEATURE_COUNT][ETH_GSTRING_LEN]
 	[NETIF_F_HW_TLS_RECORD_BIT] =	"tls-hw-record",
 	[NETIF_F_HW_TLS_TX_BIT] =	 "tls-hw-tx-offload",
 	[NETIF_F_HW_TLS_RX_BIT] =	 "tls-hw-rx-offload",
+	[NETIF_F_GRO_LIST_BIT] =         "rx-gro-list",
 };
 
 static const char
-- 
2.17.1

[PATCH RFC v3 3/5] net: Add a netdev software feature set that defaults to off.

[Steffen Klassert]

The previous patch added the NETIF_F_GRO_LIST feature.
This is a software feature that should default to off.
Current software features default to on, so add a new
feature set that defaults to off.

Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 include/linux/netdev_features.h | 3 +++
 net/core/dev.c                  | 2 +-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/include/linux/netdev_features.h b/include/linux/netdev_features.h
index 1b6baa1b6fe9..e8b3c943d835 100644
--- a/include/linux/netdev_features.h
+++ b/include/linux/netdev_features.h
@@ -228,6 +228,9 @@ static inline int find_next_netdev_feature(u64 feature, unsigned long start)
 /* changeable features with no special hardware requirements */
 #define NETIF_F_SOFT_FEATURES	(NETIF_F_GSO | NETIF_F_GRO)
 
+/* Changeable features with no special hardware requirements that defaults to off. */
+#define NETIF_F_SOFT_FEATURES_OFF	NETIF_F_GRO_LIST
+
 #define NETIF_F_VLAN_FEATURES	(NETIF_F_HW_VLAN_CTAG_FILTER | \
 				 NETIF_F_HW_VLAN_CTAG_RX | \
 				 NETIF_F_HW_VLAN_CTAG_TX | \
diff --git a/net/core/dev.c b/net/core/dev.c
index b1afafee3e2a..cc0bbec0f1d7 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -8730,7 +8730,7 @@ int register_netdevice(struct net_device *dev)
 	/* Transfer changeable features to wanted_features and enable
 	 * software offloads (GSO and GRO).
 	 */
-	dev->hw_features |= NETIF_F_SOFT_FEATURES;
+	dev->hw_features |= (NETIF_F_SOFT_FEATURES | NETIF_F_SOFT_FEATURES_OFF);
 	dev->features |= NETIF_F_SOFT_FEATURES;
 
 	if (dev->netdev_ops->ndo_udp_tunnel_add) {
-- 
2.17.1

[PATCH RFC v3 4/5] net: Support GRO/GSO fraglist chaining.

[Steffen Klassert]

This patch adds the core functions to chain/unchain
GSO skbs at the frag_list pointer. This also adds
a new GSO type SKB_GSO_FRAGLIST and a is_flist
flag to napi_gro_cb which indicates that this
flow will be GROed by fraglist chaining.

Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 include/linux/netdevice.h |   4 +-
 include/linux/skbuff.h    |   4 ++
 net/core/dev.c            |   2 +-
 net/core/skbuff.c         | 106 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 114 insertions(+), 2 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index d7d5626002e9..e97ca3e51466 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2306,7 +2306,8 @@ struct napi_gro_cb {
 	/* Number of gro_receive callbacks this packet already went through */
 	u8 recursion_counter:4;
 
-	/* 1 bit hole */
+	/* GRO is done by frag_list pointer chaining. */
+	u8	is_flist:1;
 
 	/* used to support CHECKSUM_COMPLETE for tunneling protocols */
 	__wsum	csum;
@@ -2656,6 +2657,7 @@ struct net_device *dev_get_by_napi_id(unsigned int napi_id);
 int netdev_get_name(struct net *net, char *name, int ifindex);
 int dev_restart(struct net_device *dev);
 int skb_gro_receive(struct sk_buff *p, struct sk_buff *skb);
+int skb_gro_receive_list(struct sk_buff *p, struct sk_buff *skb);
 
 static inline unsigned int skb_gro_offset(const struct sk_buff *skb)
 {
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 028e684fa974..3d5fd0a0eea7 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -595,6 +595,8 @@ enum {
 	SKB_GSO_UDP = 1 << 16,
 
 	SKB_GSO_UDP_L4 = 1 << 17,
+
+	SKB_GSO_FRAGLIST = 1 << 18,
 };
 
 #if BITS_PER_LONG > 32
@@ -3509,6 +3511,8 @@ void skb_scrub_packet(struct sk_buff *skb, bool xnet);
 bool skb_gso_validate_network_len(const struct sk_buff *skb, unsigned int mtu);
 bool skb_gso_validate_mac_len(const struct sk_buff *skb, unsigned int len);
 struct sk_buff *skb_segment(struct sk_buff *skb, netdev_features_t features);
+struct sk_buff *skb_segment_list(struct sk_buff *skb, netdev_features_t features,
+				 unsigned int offset);
 struct sk_buff *skb_vlan_untag(struct sk_buff *skb);
 int skb_ensure_writable(struct sk_buff *skb, int write_len);
 int __skb_vlan_pop(struct sk_buff *skb, u16 *vlan_tci);
diff --git a/net/core/dev.c b/net/core/dev.c
index cc0bbec0f1d7..f2a66198154d 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3109,7 +3109,7 @@ struct sk_buff *__skb_gso_segment(struct sk_buff *skb,
 
 	segs = skb_mac_gso_segment(skb, features);
 
-	if (unlikely(skb_needs_check(skb, tx_path) && !IS_ERR(segs)))
+	if (segs != skb && unlikely(skb_needs_check(skb, tx_path) && !IS_ERR(segs)))
 		skb_warn_bad_offload(skb);
 
 	return segs;
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 2b40b5a9425b..3ff56677a6fb 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -3638,6 +3638,112 @@ static inline skb_frag_t skb_head_frag_to_page_desc(struct sk_buff *frag_skb)
 	return head_frag;
 }
 
+struct sk_buff *skb_segment_list(struct sk_buff *skb,
+				 netdev_features_t features,
+				 unsigned int offset)
+{
+	struct sk_buff *list_skb = skb_shinfo(skb)->frag_list;
+	unsigned int tnl_hlen = skb_tnl_header_len(skb);
+	unsigned int delta_truesize = 0;
+	unsigned int delta_len = 0;
+	struct sk_buff *tail = NULL;
+	struct sk_buff *nskb;
+
+	skb_push(skb, -skb_network_offset(skb) + offset);
+
+	skb_shinfo(skb)->frag_list = NULL;
+
+	do {
+		nskb = list_skb;
+		list_skb = list_skb->next;
+
+		if (!tail)
+			skb->next = nskb;
+		else
+			tail->next = nskb;
+
+		tail = nskb;
+
+		delta_len += nskb->len;
+		delta_truesize += nskb->truesize;
+
+		skb_push(nskb, -skb_network_offset(nskb) + offset);
+
+		if (!secpath_exists(nskb))
+			__skb_ext_copy(nskb, skb);
+
+		memcpy(nskb->cb, skb->cb, sizeof(skb->cb));
+
+		nskb->ip_summed = skb->ip_summed;
+		nskb->csum_valid = skb->csum_valid;
+		nskb->tstamp = skb->tstamp;
+		nskb->dev = skb->dev;
+		nskb->queue_mapping = skb->queue_mapping;
+
+		nskb->mac_len = skb->mac_len;
+		nskb->mac_header = skb->mac_header;
+		nskb->transport_header = skb->transport_header;
+		nskb->network_header = skb->network_header;
+		skb_dst_copy(nskb, skb);
+
+		skb_headers_offset_update(nskb, skb_headroom(nskb) - skb_headroom(skb));
+		skb_copy_from_linear_data_offset(skb, -tnl_hlen,
+						 nskb->data - tnl_hlen,
+						 offset + tnl_hlen);
+
+		if (skb_needs_linearize(nskb, features) &&
+		    __skb_linearize(nskb))
+			goto err_linearize;
+
+	} while (list_skb);
+
+	skb->truesize = skb->truesize - delta_truesize;
+	skb->data_len = skb->data_len - delta_len;
+	skb->len = skb->len - delta_len;
+
+	skb_gso_reset(skb);
+
+	skb->prev = tail;
+
+	if (skb_needs_linearize(skb, features) &&
+	    __skb_linearize(skb))
+		goto err_linearize;
+
+	skb_get(skb);
+
+	return skb;
+
+err_linearize:
+	kfree_skb_list(skb->next);
+	skb->next = NULL;
+	return ERR_PTR(-ENOMEM);
+}
+EXPORT_SYMBOL_GPL(skb_segment_list);
+
+int skb_gro_receive_list(struct sk_buff *p, struct sk_buff *skb)
+{
+	if (unlikely(p->len + skb->len >= 65536))
+		return -E2BIG;
+
+	if (NAPI_GRO_CB(p)->last == p)
+		skb_shinfo(p)->frag_list = skb;
+	else
+		NAPI_GRO_CB(p)->last->next = skb;
+
+	skb_pull(skb, skb_gro_offset(skb));
+
+	NAPI_GRO_CB(p)->last = skb;
+	NAPI_GRO_CB(p)->count++;
+	p->data_len += skb->len;
+	p->truesize += skb->truesize;
+	p->len += skb->len;
+
+	NAPI_GRO_CB(skb)->same_flow = 1;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(skb_gro_receive_list);
+
 /**
  *	skb_segment - Perform protocol segmentation on skb.
  *	@head_skb: buffer to segment
-- 
2.17.1

[PATCH RFC v3 5/5] udp: Support UDP fraglist GRO/GSO.

[Steffen Klassert]

This patch extends UDP GRO to support fraglist GRO/GSO
by using the previously introduced infrastructure.
All UDP packets that are not targeted to a GRO capable
UDP sockets are going to fraglist GRO now (local input
and forward).

Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/ipv4/udp_offload.c | 61 +++++++++++++++++++++++++++++++++++++-----
 net/ipv6/udp_offload.c |  9 +++++++
 2 files changed, 64 insertions(+), 6 deletions(-)

diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
index e0b835890507..37daafb06d4c 100644
--- a/net/ipv4/udp_offload.c
+++ b/net/ipv4/udp_offload.c
@@ -184,6 +184,20 @@ struct sk_buff *skb_udp_tunnel_segment(struct sk_buff *skb,
 }
 EXPORT_SYMBOL(skb_udp_tunnel_segment);
 
+static struct sk_buff *__udp_gso_segment_list(struct sk_buff *skb,
+					      netdev_features_t features)
+{
+	unsigned int mss = skb_shinfo(skb)->gso_size;
+
+	skb = skb_segment_list(skb, features, skb_mac_header_len(skb));
+	if (IS_ERR(skb))
+		return skb;
+
+	udp_hdr(skb)->len = htons(sizeof(struct udphdr) + mss);
+
+	return skb;
+}
+
 struct sk_buff *__udp_gso_segment(struct sk_buff *gso_skb,
 				  netdev_features_t features)
 {
@@ -196,6 +210,9 @@ struct sk_buff *__udp_gso_segment(struct sk_buff *gso_skb,
 	__sum16 check;
 	__be16 newlen;
 
+	if (skb_shinfo(gso_skb)->gso_type & SKB_GSO_FRAGLIST)
+		return __udp_gso_segment_list(gso_skb, features);
+
 	mss = skb_shinfo(gso_skb)->gso_size;
 	if (gso_skb->len <= sizeof(*uh) + mss)
 		return ERR_PTR(-EINVAL);
@@ -354,6 +371,7 @@ static struct sk_buff *udp_gro_receive_segment(struct list_head *head,
 	struct udphdr *uh2;
 	struct sk_buff *p;
 	unsigned int ulen;
+	int ret = 0;
 
 	/* requires non zero csum, for symmetry with GSO */
 	if (!uh->check) {
@@ -369,7 +387,6 @@ static struct sk_buff *udp_gro_receive_segment(struct list_head *head,
 	}
 	/* pull encapsulating udp header */
 	skb_gro_pull(skb, sizeof(struct udphdr));
-	skb_gro_postpull_rcsum(skb, uh, sizeof(struct udphdr));
 
 	list_for_each_entry(p, head, list) {
 		if (!NAPI_GRO_CB(p)->same_flow)
@@ -383,14 +400,35 @@ static struct sk_buff *udp_gro_receive_segment(struct list_head *head,
 			continue;
 		}
 
+		if (NAPI_GRO_CB(skb)->is_flist != NAPI_GRO_CB(p)->is_flist) {
+			NAPI_GRO_CB(skb)->flush = 1;
+			return p;
+		}
+
 		/* Terminate the flow on len mismatch or if it grow "too much".
 		 * Under small packet flood GRO count could elsewhere grow a lot
 		 * leading to excessive truesize values.
 		 * On len mismatch merge the first packet shorter than gso_size,
 		 * otherwise complete the GRO packet.
 		 */
-		if (ulen > ntohs(uh2->len) || skb_gro_receive(p, skb) ||
-		    ulen != ntohs(uh2->len) ||
+		if (ulen > ntohs(uh2->len)) {
+			pp = p;
+		} else {
+			if (NAPI_GRO_CB(skb)->is_flist) {
+				if (!pskb_may_pull(skb, skb_gro_offset(skb))) {
+					NAPI_GRO_CB(skb)->flush = 1;
+					return NULL;
+				}
+				ret = skb_gro_receive_list(p, skb);
+			} else {
+				skb_gro_postpull_rcsum(skb, uh,
+						       sizeof(struct udphdr));
+
+				ret = skb_gro_receive(p, skb);
+			}
+		}
+
+		if (ret || ulen != ntohs(uh2->len) ||
 		    NAPI_GRO_CB(p)->count >= UDP_GRO_CNT_MAX)
 			pp = p;
 
@@ -411,6 +449,9 @@ struct sk_buff *udp_gro_receive(struct list_head *head, struct sk_buff *skb,
 	int flush = 1;
 
 	if (!sk || !udp_sk(sk)->gro_receive) {
+		if (skb->dev->features & NETIF_F_GRO_LIST)
+			NAPI_GRO_CB(skb)->is_flist = sk ? !udp_sk(sk)->gro_enabled: 1;
+
 		pp = call_gro_receive(udp_gro_receive_segment, head, skb);
 		return pp;
 	}
@@ -419,7 +460,7 @@ struct sk_buff *udp_gro_receive(struct list_head *head, struct sk_buff *skb,
 	    (skb->ip_summed != CHECKSUM_PARTIAL &&
 	     NAPI_GRO_CB(skb)->csum_cnt == 0 &&
 	     !NAPI_GRO_CB(skb)->csum_valid))
-		goto out_unlock;
+		goto out;
 
 	/* mark that this skb passed once through the tunnel gro layer */
 	NAPI_GRO_CB(skb)->encap_mark = 1;
@@ -446,8 +487,7 @@ struct sk_buff *udp_gro_receive(struct list_head *head, struct sk_buff *skb,
 	skb_gro_postpull_rcsum(skb, uh, sizeof(struct udphdr));
 	pp = call_gro_receive_sk(udp_sk(sk)->gro_receive, sk, head, skb);
 
-out_unlock:
-	rcu_read_unlock();
+out:
 	skb_gro_flush_final(skb, pp, flush);
 	return pp;
 }
@@ -539,6 +579,15 @@ INDIRECT_CALLABLE_SCOPE int udp4_gro_complete(struct sk_buff *skb, int nhoff)
 	const struct iphdr *iph = ip_hdr(skb);
 	struct udphdr *uh = (struct udphdr *)(skb->data + nhoff);
 
+	if (NAPI_GRO_CB(skb)->is_flist) {
+		uh->len = htons(skb->len - nhoff);
+
+		skb_shinfo(skb)->gso_type |= (SKB_GSO_FRAGLIST|SKB_GSO_UDP_L4);
+		skb_shinfo(skb)->gso_segs = NAPI_GRO_CB(skb)->count;
+
+		return 0;
+	}
+
 	if (uh->check)
 		uh->check = ~udp_v4_check(skb->len - nhoff, iph->saddr,
 					  iph->daddr, 0);
diff --git a/net/ipv6/udp_offload.c b/net/ipv6/udp_offload.c
index 47a3b1348371..e503b5cf9023 100644
--- a/net/ipv6/udp_offload.c
+++ b/net/ipv6/udp_offload.c
@@ -150,6 +150,15 @@ INDIRECT_CALLABLE_SCOPE int udp6_gro_complete(struct sk_buff *skb, int nhoff)
 	const struct ipv6hdr *ipv6h = ipv6_hdr(skb);
 	struct udphdr *uh = (struct udphdr *)(skb->data + nhoff);
 
+	if (NAPI_GRO_CB(skb)->is_flist) {
+		uh->len = htons(skb->len - nhoff);
+
+		skb_shinfo(skb)->gso_type |= (SKB_GSO_FRAGLIST|SKB_GSO_UDP_L4);
+		skb_shinfo(skb)->gso_segs = NAPI_GRO_CB(skb)->count;
+
+		return 0;
+	}
+
 	if (uh->check)
 		uh->check = ~udp_v6_check(skb->len - nhoff, &ipv6h->saddr,
 					  &ipv6h->daddr, 0);
-- 
2.17.1

Re: [PATCH RFC v3 2/5] net: Add NETIF_F_GRO_LIST feature

[Willem de Bruijn]

On Wed, Sep 18, 2019 at 3:25 AM Steffen Klassert
<steffen.klassert@secunet.com> wrote:
>
> This adds a new NETIF_F_GRO_LIST feature flag. I will be used
> to configure listfyed GRO what will be implemented with some
> followup paches.

This should probably simultaneously introduce SKB_GSO_FRAGLIST as well
as a BUILD_BUG_ON in net_gso_ok.

Please also in the commit describe the constraints of skbs that have
this type. If I'm not mistaken, an skb with either gso_size linear
data or one gso_sized frag, followed by a frag_list of the same. With
the exception of the last frag_list member, whose mss may be less than
gso_size. This will help when reasoning about all the types of skbs we
may see at segmentation, as we recently had to do [1]

Minor nit: I think it's listified, not listifyed.

[1] https://lore.kernel.org/netdev/CA+FuTSfVsgNDi7c=GUU8nMg2hWxF2SjCNLXetHeVPdnxAW5K-w@mail.gmail.com/

> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
> ---



>  include/linux/netdev_features.h | 2 ++
>  net/core/ethtool.c              | 1 +
>  2 files changed, 3 insertions(+)
>
> diff --git a/include/linux/netdev_features.h b/include/linux/netdev_features.h
> index 4b19c544c59a..1b6baa1b6fe9 100644
> --- a/include/linux/netdev_features.h
> +++ b/include/linux/netdev_features.h
> @@ -80,6 +80,7 @@ enum {
>
>         NETIF_F_GRO_HW_BIT,             /* Hardware Generic receive offload */
>         NETIF_F_HW_TLS_RECORD_BIT,      /* Offload TLS record */
> +       NETIF_F_GRO_LIST_BIT,           /* Listifyed GRO */
>
>         /*
>          * Add your fresh new feature above and remember to update
> @@ -150,6 +151,7 @@ enum {
>  #define NETIF_F_GSO_UDP_L4     __NETIF_F(GSO_UDP_L4)
>  #define NETIF_F_HW_TLS_TX      __NETIF_F(HW_TLS_TX)
>  #define NETIF_F_HW_TLS_RX      __NETIF_F(HW_TLS_RX)
> +#define NETIF_F_GRO_LIST       __NETIF_F(GRO_LIST)
>
>  /* Finds the next feature with the highest number of the range of start till 0.
>   */
> diff --git a/net/core/ethtool.c b/net/core/ethtool.c
> index 6288e69e94fc..ee8d2b58c2d7 100644
> --- a/net/core/ethtool.c
> +++ b/net/core/ethtool.c
> @@ -111,6 +111,7 @@ static const char netdev_features_strings[NETDEV_FEATURE_COUNT][ETH_GSTRING_LEN]
>         [NETIF_F_HW_TLS_RECORD_BIT] =   "tls-hw-record",
>         [NETIF_F_HW_TLS_TX_BIT] =        "tls-hw-tx-offload",
>         [NETIF_F_HW_TLS_RX_BIT] =        "tls-hw-rx-offload",
> +       [NETIF_F_GRO_LIST_BIT] =         "rx-gro-list",
>  };
>
>  static const char
> --
> 2.17.1
>

Re: [PATCH RFC v3 5/5] udp: Support UDP fraglist GRO/GSO.

[Willem de Bruijn]

On Wed, Sep 18, 2019 at 3:25 AM Steffen Klassert
<steffen.klassert@secunet.com> wrote:
>
> This patch extends UDP GRO to support fraglist GRO/GSO
> by using the previously introduced infrastructure.
> All UDP packets that are not targeted to a GRO capable
> UDP sockets are going to fraglist GRO now (local input
> and forward).
>
> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>


> @@ -419,7 +460,7 @@ struct sk_buff *udp_gro_receive(struct list_head *head, struct sk_buff *skb,
>             (skb->ip_summed != CHECKSUM_PARTIAL &&
>              NAPI_GRO_CB(skb)->csum_cnt == 0 &&
>              !NAPI_GRO_CB(skb)->csum_valid))
> -               goto out_unlock;
> +               goto out;
>
>         /* mark that this skb passed once through the tunnel gro layer */
>         NAPI_GRO_CB(skb)->encap_mark = 1;
> @@ -446,8 +487,7 @@ struct sk_buff *udp_gro_receive(struct list_head *head, struct sk_buff *skb,
>         skb_gro_postpull_rcsum(skb, uh, sizeof(struct udphdr));
>         pp = call_gro_receive_sk(udp_sk(sk)->gro_receive, sk, head, skb);
>
> -out_unlock:
> -       rcu_read_unlock();
> +out:
>         skb_gro_flush_final(skb, pp, flush);
>         return pp;
>  }

This probably belongs in patch 1?

Re: [PATCH RFC v3 0/5] Support fraglist GRO/GSO

[Willem de Bruijn]

On Wed, Sep 18, 2019 at 3:25 AM Steffen Klassert
<steffen.klassert@secunet.com> wrote:
>
> This patchset adds support to do GRO/GSO by chaining packets
> of the same flow at the SKB frag_list pointer. This avoids
> the overhead to merge payloads into one big packet, and
> on the other end, if GSO is needed it avoids the overhead
> of splitting the big packet back to the native form.
>
> Patch 1 Enables UDP GRO by default.
>
> Patch 2 adds a netdev feature flag to enable listifyed GRO,
> this implements one of the configuration options discussed
> at netconf 2019.
>
> Patch 3 adds a netdev software feature set that defaults to off
> and assigns the new listifyed GRO feature flag to it.
>
> Patch 4 adds the core infrastructure to do fraglist GRO/GSO.
>
> Patch 5 enables UDP to use fraglist GRO/GSO if configured and no
> GRO supported socket is found.

Very nice feature, Steffen. Aside from questions around performance,
my only question is really how this relates to GSO_BY_FRAGS.

More specifically, whether we can remove that in favor of using your
new skb_segment_list. That would actually be a big first step in
simplifying skb_segment back to something manageable.

Re: [PATCH RFC v3 0/5] Support fraglist GRO/GSO

[Marcelo Ricardo Leitner]

On Wed, Sep 18, 2019 at 12:17:08PM -0400, Willem de Bruijn wrote:
> On Wed, Sep 18, 2019 at 3:25 AM Steffen Klassert
> <steffen.klassert@secunet.com> wrote:
> >
> > This patchset adds support to do GRO/GSO by chaining packets
> > of the same flow at the SKB frag_list pointer. This avoids
> > the overhead to merge payloads into one big packet, and
> > on the other end, if GSO is needed it avoids the overhead
> > of splitting the big packet back to the native form.
> >
> > Patch 1 Enables UDP GRO by default.
> >
> > Patch 2 adds a netdev feature flag to enable listifyed GRO,
> > this implements one of the configuration options discussed
> > at netconf 2019.
> >
> > Patch 3 adds a netdev software feature set that defaults to off
> > and assigns the new listifyed GRO feature flag to it.
> >
> > Patch 4 adds the core infrastructure to do fraglist GRO/GSO.
> >
> > Patch 5 enables UDP to use fraglist GRO/GSO if configured and no
> > GRO supported socket is found.
> 
> Very nice feature, Steffen. Aside from questions around performance,
> my only question is really how this relates to GSO_BY_FRAGS.

They do the exact same thing AFAICT: they GSO according to a
pre-formatted list of fragments/packets, and not to a specific size
(such as MSS).

> 
> More specifically, whether we can remove that in favor of using your
> new skb_segment_list. That would actually be a big first step in
> simplifying skb_segment back to something manageable.

The main issue (that I know) on obsoleting GSO_BY_FRAGS is that
dealing with frags instead of frag_list was considered easier to be
offloaded, if ever attempted.  So this would be a step back on that
aspect.  Other than this, it should be doable.

Re: [PATCH RFC v3 0/5] Support fraglist GRO/GSO

[Subash Abhinov Kasiviswanathan]

On 2019-09-18 10:58, Marcelo Ricardo Leitner wrote:
> On Wed, Sep 18, 2019 at 12:17:08PM -0400, Willem de Bruijn wrote:
>> On Wed, Sep 18, 2019 at 3:25 AM Steffen Klassert
>> <steffen.klassert@secunet.com> wrote:
>> >
>> > This patchset adds support to do GRO/GSO by chaining packets
>> > of the same flow at the SKB frag_list pointer. This avoids
>> > the overhead to merge payloads into one big packet, and
>> > on the other end, if GSO is needed it avoids the overhead
>> > of splitting the big packet back to the native form.
>> >
>> > Patch 1 Enables UDP GRO by default.
>> >
>> > Patch 2 adds a netdev feature flag to enable listifyed GRO,
>> > this implements one of the configuration options discussed
>> > at netconf 2019.
>> >
>> > Patch 3 adds a netdev software feature set that defaults to off
>> > and assigns the new listifyed GRO feature flag to it.
>> >
>> > Patch 4 adds the core infrastructure to do fraglist GRO/GSO.
>> >
>> > Patch 5 enables UDP to use fraglist GRO/GSO if configured and no
>> > GRO supported socket is found.
>> 
>> Very nice feature, Steffen. Aside from questions around performance,
>> my only question is really how this relates to GSO_BY_FRAGS.
> 
> They do the exact same thing AFAICT: they GSO according to a
> pre-formatted list of fragments/packets, and not to a specific size
> (such as MSS).
> 
>> 
>> More specifically, whether we can remove that in favor of using your
>> new skb_segment_list. That would actually be a big first step in
>> simplifying skb_segment back to something manageable.
> 
> The main issue (that I know) on obsoleting GSO_BY_FRAGS is that
> dealing with frags instead of frag_list was considered easier to be
> offloaded, if ever attempted.  So this would be a step back on that
> aspect.  Other than this, it should be doable.

Is there an existing userspace interface for GSO_BY_FRAGS for UDP?
Per my understanding, the current UDP_GSO CMSG option only allows
for a specific GSO_SIZE segmentation.

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

Re: [PATCH RFC v3 2/5] net: Add NETIF_F_GRO_LIST feature

[Subash Abhinov Kasiviswanathan]

On 2019-09-18 10:10, Willem de Bruijn wrote:
> On Wed, Sep 18, 2019 at 3:25 AM Steffen Klassert
> <steffen.klassert@secunet.com> wrote:
>> 
>> This adds a new NETIF_F_GRO_LIST feature flag. I will be used
>> to configure listfyed GRO what will be implemented with some
>> followup paches.
> 
> This should probably simultaneously introduce SKB_GSO_FRAGLIST as well
> as a BUILD_BUG_ON in net_gso_ok.
> 
> Please also in the commit describe the constraints of skbs that have
> this type. If I'm not mistaken, an skb with either gso_size linear
> data or one gso_sized frag, followed by a frag_list of the same. With
> the exception of the last frag_list member, whose mss may be less than
> gso_size. This will help when reasoning about all the types of skbs we
> may see at segmentation, as we recently had to do [1]
> 

Would it be preferrable to allow any size skbs for the listification.
Since the original skbs are being restored, single gso_size shoudln't
be a constraint here.

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

Re: [PATCH RFC v3 2/5] net: Add NETIF_F_GRO_LIST feature

[Steffen Klassert]

On Wed, Sep 18, 2019 at 12:10:31PM -0400, Willem de Bruijn wrote:
> On Wed, Sep 18, 2019 at 3:25 AM Steffen Klassert
> <steffen.klassert@secunet.com> wrote:
> >
> > This adds a new NETIF_F_GRO_LIST feature flag. I will be used
> > to configure listfyed GRO what will be implemented with some
> > followup paches.
> 
> This should probably simultaneously introduce SKB_GSO_FRAGLIST as well
> as a BUILD_BUG_ON in net_gso_ok.

Yes, good point. I'll also rename NETIF_F_GRO_LIST to NETIF_F_GRO_FRAGLIST
and add NETIF_F_GSO_FRAGLIST what is currently missing.

> 
> Please also in the commit describe the constraints of skbs that have
> this type. If I'm not mistaken, an skb with either gso_size linear
> data or one gso_sized frag, followed by a frag_list of the same. With
> the exception of the last frag_list member, whose mss may be less than
> gso_size. This will help when reasoning about all the types of skbs we
> may see at segmentation, as we recently had to do [1]

We don't use skb_segment(), so I think we don't have this constraint.

> 
> Minor nit: I think it's listified, not listifyed.

I think neither of both words really exist :)
I'll rename it to fraglist GRO.

Re: [PATCH RFC v3 2/5] net: Add NETIF_F_GRO_LIST feature

[Steffen Klassert]

On Wed, Sep 18, 2019 at 08:04:18PM -0600, Subash Abhinov Kasiviswanathan wrote:
> On 2019-09-18 10:10, Willem de Bruijn wrote:
> > On Wed, Sep 18, 2019 at 3:25 AM Steffen Klassert
> > <steffen.klassert@secunet.com> wrote:
> > > 
> > > This adds a new NETIF_F_GRO_LIST feature flag. I will be used
> > > to configure listfyed GRO what will be implemented with some
> > > followup paches.
> > 
> > This should probably simultaneously introduce SKB_GSO_FRAGLIST as well
> > as a BUILD_BUG_ON in net_gso_ok.
> > 
> > Please also in the commit describe the constraints of skbs that have
> > this type. If I'm not mistaken, an skb with either gso_size linear
> > data or one gso_sized frag, followed by a frag_list of the same. With
> > the exception of the last frag_list member, whose mss may be less than
> > gso_size. This will help when reasoning about all the types of skbs we
> > may see at segmentation, as we recently had to do [1]
> > 
> 
> Would it be preferrable to allow any size skbs for the listification.

We currently require a single gso_size because we adjust uh->len
on the head skb to the full size to do correct memory accounting
on the local input path. That is going to be restored with the
gso_size on segmentation.

> Since the original skbs are being restored, single gso_size shoudln't
> be a constraint here.

It might be possible to allow any sized skbs with some extra work, though.

Re: [PATCH RFC v3 5/5] udp: Support UDP fraglist GRO/GSO.

[Steffen Klassert]

On Wed, Sep 18, 2019 at 12:13:26PM -0400, Willem de Bruijn wrote:
> On Wed, Sep 18, 2019 at 3:25 AM Steffen Klassert
> <steffen.klassert@secunet.com> wrote:
> >
> > This patch extends UDP GRO to support fraglist GRO/GSO
> > by using the previously introduced infrastructure.
> > All UDP packets that are not targeted to a GRO capable
> > UDP sockets are going to fraglist GRO now (local input
> > and forward).
> >
> > Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
> 
> 
> > @@ -419,7 +460,7 @@ struct sk_buff *udp_gro_receive(struct list_head *head, struct sk_buff *skb,
> >             (skb->ip_summed != CHECKSUM_PARTIAL &&
> >              NAPI_GRO_CB(skb)->csum_cnt == 0 &&
> >              !NAPI_GRO_CB(skb)->csum_valid))
> > -               goto out_unlock;
> > +               goto out;
> >
> >         /* mark that this skb passed once through the tunnel gro layer */
> >         NAPI_GRO_CB(skb)->encap_mark = 1;
> > @@ -446,8 +487,7 @@ struct sk_buff *udp_gro_receive(struct list_head *head, struct sk_buff *skb,
> >         skb_gro_postpull_rcsum(skb, uh, sizeof(struct udphdr));
> >         pp = call_gro_receive_sk(udp_sk(sk)->gro_receive, sk, head, skb);
> >
> > -out_unlock:
> > -       rcu_read_unlock();
> > +out:
> >         skb_gro_flush_final(skb, pp, flush);
> >         return pp;
> >  }
> 
> This probably belongs in patch 1?

Yes, apparently :)

Re: [PATCH RFC v3 0/5] Support fraglist GRO/GSO

[Steffen Klassert]

On Wed, Sep 18, 2019 at 12:17:08PM -0400, Willem de Bruijn wrote:
> On Wed, Sep 18, 2019 at 3:25 AM Steffen Klassert
> <steffen.klassert@secunet.com> wrote:
> >
> > This patchset adds support to do GRO/GSO by chaining packets
> > of the same flow at the SKB frag_list pointer. This avoids
> > the overhead to merge payloads into one big packet, and
> > on the other end, if GSO is needed it avoids the overhead
> > of splitting the big packet back to the native form.
> >
> > Patch 1 Enables UDP GRO by default.
> >
> > Patch 2 adds a netdev feature flag to enable listifyed GRO,
> > this implements one of the configuration options discussed
> > at netconf 2019.
> >
> > Patch 3 adds a netdev software feature set that defaults to off
> > and assigns the new listifyed GRO feature flag to it.
> >
> > Patch 4 adds the core infrastructure to do fraglist GRO/GSO.
> >
> > Patch 5 enables UDP to use fraglist GRO/GSO if configured and no
> > GRO supported socket is found.
> 
> Very nice feature, Steffen.

Thanks!

> Aside from questions around performance,
> my only question is really how this relates to GSO_BY_FRAGS.
> 
> More specifically, whether we can remove that in favor of using your
> new skb_segment_list. That would actually be a big first step in
> simplifying skb_segment back to something manageable.

As Marcelo pointed out, this should be doable.

Thanks for all your review. I'll incorporate your comments and do
RFC v4, so that we hopefully can start the mainlining process as
soon as net-next opens again.

Re: [PATCH RFC v3 0/5] Support fraglist GRO/GSO

[Steffen Klassert]

On Wed, Sep 18, 2019 at 01:58:17PM -0300, Marcelo Ricardo Leitner wrote:
> On Wed, Sep 18, 2019 at 12:17:08PM -0400, Willem de Bruijn wrote:
> > 
> > More specifically, whether we can remove that in favor of using your
> > new skb_segment_list. That would actually be a big first step in
> > simplifying skb_segment back to something manageable.
> 
> The main issue (that I know) on obsoleting GSO_BY_FRAGS is that
> dealing with frags instead of frag_list was considered easier to be
> offloaded, if ever attempted.  So this would be a step back on that
> aspect.  Other than this, it should be doable.

I wonder why offloading the frag_list should be harder.
I looked at the i40e driver, it just iterates over the page
fragments found in skb_shinfo(skb)->frags in i40e_tx_map().

If the packet data of all the fraglist GRO skbs are backed by a
page fragment then we could just do the same by iterating with
skb_walk_frags(). I'm not a driver expert and might be misstaken,
but it looks like that could be done with existing hardware that
supports segmentation offload.

Re: [PATCH RFC v3 0/5] Support fraglist GRO/GSO

[David Miller]

From: Steffen Klassert <steffen.klassert@secunet.com>
Date: Thu, 19 Sep 2019 13:01:25 +0200

> If the packet data of all the fraglist GRO skbs are backed by a
> page fragment then we could just do the same by iterating with
> skb_walk_frags(). I'm not a driver expert and might be misstaken,
> but it looks like that could be done with existing hardware that
> supports segmentation offload.

Having to add frag list as well as page frag iterating in a driver is
quite a bit of logic, and added complexity.

If the frag list SKBs are indeed backed by a page, you could just as
easily coalesce everything into the page frag array of the first SKB.

Re: [PATCH RFC v3 0/5] Support fraglist GRO/GSO

[Steffen Klassert]

On Thu, Sep 19, 2019 at 01:18:16PM +0200, David Miller wrote:
> From: Steffen Klassert <steffen.klassert@secunet.com>
> Date: Thu, 19 Sep 2019 13:01:25 +0200
> 
> > If the packet data of all the fraglist GRO skbs are backed by a
> > page fragment then we could just do the same by iterating with
> > skb_walk_frags(). I'm not a driver expert and might be misstaken,
> > but it looks like that could be done with existing hardware that
> > supports segmentation offload.
> 
> Having to add frag list as well as page frag iterating in a driver is
> quite a bit of logic, and added complexity.
> 
> If the frag list SKBs are indeed backed by a page, you could just as
> easily coalesce everything into the page frag array of the first SKB.

That is true indeed. Fraglist GRO is more optimized to the case
where we still need the skb arround the packet data. I.e. if it 
can't be offloaded.

Re: [PATCH RFC v3 0/5] Support fraglist GRO/GSO

[Or Gerlitz]

On Wed, Sep 18, 2019 at 2:48 PM Steffen Klassert
<steffen.klassert@secunet.com> wrote:
> This patchset adds support to do GRO/GSO by chaining packets
> of the same flow at the SKB frag_list pointer. This avoids
> the overhead to merge payloads into one big packet, and
> on the other end, if GSO is needed it avoids the overhead
> of splitting the big packet back to the native form.
>
> Patch 1 Enables UDP GRO by default.
>
> Patch 2 adds a netdev feature flag to enable listifyed GRO,
> this implements one of the configuration options discussed
> at netconf 2019.
[..]

The slide say that linked packets travel together though the stack.

This sounds somehow similar to the approach suggested by Ed
for skb lists. I wonder what we can say on cases where each of the
approaches would function better.

Or.

Re: [PATCH RFC v3 0/5] Support fraglist GRO/GSO

[Willem de Bruijn]

On Wed, Sep 18, 2019 at 12:58 PM Marcelo Ricardo Leitner
<marcelo.leitner@gmail.com> wrote:
>
> On Wed, Sep 18, 2019 at 12:17:08PM -0400, Willem de Bruijn wrote:
> > On Wed, Sep 18, 2019 at 3:25 AM Steffen Klassert
> > <steffen.klassert@secunet.com> wrote:
> > >
> > > This patchset adds support to do GRO/GSO by chaining packets
> > > of the same flow at the SKB frag_list pointer. This avoids
> > > the overhead to merge payloads into one big packet, and
> > > on the other end, if GSO is needed it avoids the overhead
> > > of splitting the big packet back to the native form.
> > >
> > > Patch 1 Enables UDP GRO by default.
> > >
> > > Patch 2 adds a netdev feature flag to enable listifyed GRO,
> > > this implements one of the configuration options discussed
> > > at netconf 2019.
> > >
> > > Patch 3 adds a netdev software feature set that defaults to off
> > > and assigns the new listifyed GRO feature flag to it.
> > >
> > > Patch 4 adds the core infrastructure to do fraglist GRO/GSO.
> > >
> > > Patch 5 enables UDP to use fraglist GRO/GSO if configured and no
> > > GRO supported socket is found.
> >
> > Very nice feature, Steffen. Aside from questions around performance,
> > my only question is really how this relates to GSO_BY_FRAGS.
>
> They do the exact same thing AFAICT: they GSO according to a
> pre-formatted list of fragments/packets, and not to a specific size
> (such as MSS).
>
> >
> > More specifically, whether we can remove that in favor of using your
> > new skb_segment_list. That would actually be a big first step in
> > simplifying skb_segment back to something manageable.
>
> The main issue (that I know) on obsoleting GSO_BY_FRAGS is that
> dealing with frags instead of frag_list was considered easier to be
> offloaded, if ever attempted.  So this would be a step back on that
> aspect.  Other than this, it should be doable.

But GSO_BY_FRAGS also uses frag_list, not frags?

And list_skb->len for mss.

Re: [PATCH RFC v3 0/5] Support fraglist GRO/GSO

[Marcelo Ricardo Leitner]

On Thu, Sep 19, 2019 at 08:55:22AM -0400, Willem de Bruijn wrote:
> On Wed, Sep 18, 2019 at 12:58 PM Marcelo Ricardo Leitner
> <marcelo.leitner@gmail.com> wrote:
> >
> > On Wed, Sep 18, 2019 at 12:17:08PM -0400, Willem de Bruijn wrote:
> > > On Wed, Sep 18, 2019 at 3:25 AM Steffen Klassert
> > > <steffen.klassert@secunet.com> wrote:
> > > >
> > > > This patchset adds support to do GRO/GSO by chaining packets
> > > > of the same flow at the SKB frag_list pointer. This avoids
> > > > the overhead to merge payloads into one big packet, and
> > > > on the other end, if GSO is needed it avoids the overhead
> > > > of splitting the big packet back to the native form.
> > > >
> > > > Patch 1 Enables UDP GRO by default.
> > > >
> > > > Patch 2 adds a netdev feature flag to enable listifyed GRO,
> > > > this implements one of the configuration options discussed
> > > > at netconf 2019.
> > > >
> > > > Patch 3 adds a netdev software feature set that defaults to off
> > > > and assigns the new listifyed GRO feature flag to it.
> > > >
> > > > Patch 4 adds the core infrastructure to do fraglist GRO/GSO.
> > > >
> > > > Patch 5 enables UDP to use fraglist GRO/GSO if configured and no
> > > > GRO supported socket is found.
> > >
> > > Very nice feature, Steffen. Aside from questions around performance,
> > > my only question is really how this relates to GSO_BY_FRAGS.
> >
> > They do the exact same thing AFAICT: they GSO according to a
> > pre-formatted list of fragments/packets, and not to a specific size
> > (such as MSS).
> >
> > >
> > > More specifically, whether we can remove that in favor of using your
> > > new skb_segment_list. That would actually be a big first step in
> > > simplifying skb_segment back to something manageable.
> >
> > The main issue (that I know) on obsoleting GSO_BY_FRAGS is that
> > dealing with frags instead of frag_list was considered easier to be
> > offloaded, if ever attempted.  So this would be a step back on that
> > aspect.  Other than this, it should be doable.
> 
> But GSO_BY_FRAGS also uses frag_list, not frags?

/me is scratching his head.
My bad. I thought it was already using frags. Thanks.

> 
> And list_skb->len for mss.

Which stands more for 'current frag size', yes.
(list_skb, not head_skb)

Re: [PATCH RFC v3 0/5] Support fraglist GRO/GSO

[Marcelo Ricardo Leitner]

On Thu, Sep 19, 2019 at 11:41:06AM +0200, Steffen Klassert wrote:
> On Wed, Sep 18, 2019 at 12:17:08PM -0400, Willem de Bruijn wrote:
> > On Wed, Sep 18, 2019 at 3:25 AM Steffen Klassert
> > <steffen.klassert@secunet.com> wrote:
> > >
> > > This patchset adds support to do GRO/GSO by chaining packets
> > > of the same flow at the SKB frag_list pointer. This avoids
> > > the overhead to merge payloads into one big packet, and
> > > on the other end, if GSO is needed it avoids the overhead
> > > of splitting the big packet back to the native form.
> > >
> > > Patch 1 Enables UDP GRO by default.
> > >
> > > Patch 2 adds a netdev feature flag to enable listifyed GRO,
> > > this implements one of the configuration options discussed
> > > at netconf 2019.
> > >
> > > Patch 3 adds a netdev software feature set that defaults to off
> > > and assigns the new listifyed GRO feature flag to it.
> > >
> > > Patch 4 adds the core infrastructure to do fraglist GRO/GSO.
> > >
> > > Patch 5 enables UDP to use fraglist GRO/GSO if configured and no
> > > GRO supported socket is found.
> > 
> > Very nice feature, Steffen.
> 
> Thanks!
> 
> > Aside from questions around performance,
> > my only question is really how this relates to GSO_BY_FRAGS.
> > 
> > More specifically, whether we can remove that in favor of using your
> > new skb_segment_list. That would actually be a big first step in
> > simplifying skb_segment back to something manageable.
> 
> As Marcelo pointed out, this should be doable.

And easier than I had thought :)
But please let me know if you need anything on it. This approach is
much cleaner.

Re: [PATCH RFC v3 0/5] Support fraglist GRO/GSO

[Willem de Bruijn]

On Thu, Sep 19, 2019 at 9:07 AM Marcelo Ricardo Leitner
<marcelo.leitner@gmail.com> wrote:
>
> On Thu, Sep 19, 2019 at 08:55:22AM -0400, Willem de Bruijn wrote:
> > On Wed, Sep 18, 2019 at 12:58 PM Marcelo Ricardo Leitner
> > <marcelo.leitner@gmail.com> wrote:
> > >
> > > On Wed, Sep 18, 2019 at 12:17:08PM -0400, Willem de Bruijn wrote:
> > > > On Wed, Sep 18, 2019 at 3:25 AM Steffen Klassert
> > > > <steffen.klassert@secunet.com> wrote:
> > > > >
> > > > > This patchset adds support to do GRO/GSO by chaining packets
> > > > > of the same flow at the SKB frag_list pointer. This avoids
> > > > > the overhead to merge payloads into one big packet, and
> > > > > on the other end, if GSO is needed it avoids the overhead
> > > > > of splitting the big packet back to the native form.
> > > > >
> > > > > Patch 1 Enables UDP GRO by default.
> > > > >
> > > > > Patch 2 adds a netdev feature flag to enable listifyed GRO,
> > > > > this implements one of the configuration options discussed
> > > > > at netconf 2019.
> > > > >
> > > > > Patch 3 adds a netdev software feature set that defaults to off
> > > > > and assigns the new listifyed GRO feature flag to it.
> > > > >
> > > > > Patch 4 adds the core infrastructure to do fraglist GRO/GSO.
> > > > >
> > > > > Patch 5 enables UDP to use fraglist GRO/GSO if configured and no
> > > > > GRO supported socket is found.
> > > >
> > > > Very nice feature, Steffen. Aside from questions around performance,
> > > > my only question is really how this relates to GSO_BY_FRAGS.
> > >
> > > They do the exact same thing AFAICT: they GSO according to a
> > > pre-formatted list of fragments/packets, and not to a specific size
> > > (such as MSS).
> > >
> > > >
> > > > More specifically, whether we can remove that in favor of using your
> > > > new skb_segment_list. That would actually be a big first step in
> > > > simplifying skb_segment back to something manageable.
> > >
> > > The main issue (that I know) on obsoleting GSO_BY_FRAGS is that
> > > dealing with frags instead of frag_list was considered easier to be
> > > offloaded, if ever attempted.  So this would be a step back on that
> > > aspect.  Other than this, it should be doable.
> >
> > But GSO_BY_FRAGS also uses frag_list, not frags?
>
> /me is scratching his head.
> My bad. I thought it was already using frags. Thanks.
>
> >
> > And list_skb->len for mss.
>
> Which stands more for 'current frag size', yes.
> (list_skb, not head_skb)

Great. I thought I missed something :)

Frags might be cheaper from an allocation point. If at some point
going down that road.

But in the meantime, it looks like we can handle these too with
skb_segment_list, then (not necessarily in the initial patch set).

Re: [PATCH RFC v3 0/5] Support fraglist GRO/GSO

[Paolo Abeni]

On Thu, 2019-09-19 at 15:37 +0300, Or Gerlitz wrote:
> On Wed, Sep 18, 2019 at 2:48 PM Steffen Klassert
> <steffen.klassert@secunet.com> wrote:
> > This patchset adds support to do GRO/GSO by chaining packets
> > of the same flow at the SKB frag_list pointer. This avoids
> > the overhead to merge payloads into one big packet, and
> > on the other end, if GSO is needed it avoids the overhead
> > of splitting the big packet back to the native form.
> > 
> > Patch 1 Enables UDP GRO by default.
> > 
> > Patch 2 adds a netdev feature flag to enable listifyed GRO,
> > this implements one of the configuration options discussed
> > at netconf 2019.
> [..]
> 
> The slide say that linked packets travel together though the stack.
> 
> This sounds somehow similar to the approach suggested by Ed
> for skb lists. I wonder what we can say on cases where each of the
> approaches would function better.

The 'listification' by Ed Cree can potentially aggregate across
multiple flows, so it can trigger when UDP GRO can not.

On the other side, fraglist performance impact is more limited, because
we still have to walk the list on each step. UDP GRO will improve
performances considerably more, if it can be triggered - e.g. all pkts
belong to the same flow.

Cheers,

Paolo