udp: implement GRO support

The *encap_needed static keys are enabled by UDP tunnels and several UDP encapsulations type, but they are never turned off. This can cause unneeded overall performance degradation for systems where such features are used transiently.

This patch introduces complete book-keeping for such keys, decreasing the usage at socket destruction time, if needed, and avoiding that the same socket could increase the key usage multiple times.

Signed-off-by Paolo Abeni <[email protected]>

---
 include/linux/udp.h      |  7 ++++++-
 include/net/udp_tunnel.h |  6 ++++++
 net/ipv4/udp.c           | 18 ++++++++++++------
 net/ipv6/udp.c           | 14 +++++++++-----
 4 files changed, 33 insertions(+), 12 deletions(-)

include/linux/udp.h +6 -1

index 320d49d85484..a4dafff407fb 100644
@@ -49,7 +49,12 @@ struct udp_sock {
        unsigned int     corkflag;      /* Cork is required */
        __u8             encap_type;    /* Is this an Encapsulation socket? */
        unsigned char    no_check6_tx:1,/* Send zero UDP6 checksums on TX? */
-                        no_check6_rx:1;/* Allow zero UDP6 checksums on RX? */
+                        no_check6_rx:1,/* Allow zero UDP6 checksums on RX? */
+                        encap_enabled:1; /* This socket enabled encap
+                                          * processing; UDP tunnels and
+                                          * different encapsulation layer set
+                                          * this
+                                          */
        /*
         * Following member retains the information to create a UDP header
         * when the socket is uncorked.

include/net/udp_tunnel.h +6 -0

index fe680ab6b15a..3fbe56430e3b 100644
@@ -165,6 +165,12 @@ static inline int udp_tunnel_handle_offloads(struct sk_buff *skb, bool udp_csum)
 
 static inline void udp_tunnel_encap_enable(struct socket *sock)
 {
+       struct udp_sock *up = udp_sk(sock->sk);
+
+       if (up->encap_enabled)
+               return;
+
+       up->encap_enabled = 1;
 #if IS_ENABLED(CONFIG_IPV6)
        if (sock->sk->sk_family == PF_INET6)
                ipv6_stub->udpv6_encap_enable();

net/ipv4/udp.c +9 -5

index cf8252d05a01..9fcb5374e166 100644
@@ -2382,11 +2382,15 @@ void udp_destroy_sock(struct sock *sk)
        bool slow = lock_sock_fast(sk);
        udp_flush_pending_frames(sk);
        unlock_sock_fast(sk, slow);
-       if (static_branch_unlikely(&udp_encap_needed_key) && up->encap_type) {
-               void (*encap_destroy)(struct sock *sk);
-               encap_destroy = READ_ONCE(up->encap_destroy);
-               if (encap_destroy)
-                       encap_destroy(sk);
+       if (static_branch_unlikely(&udp_encap_needed_key)) {
+               if (up->encap_type) {
+                       void (*encap_destroy)(struct sock *sk);
+                       encap_destroy = READ_ONCE(up->encap_destroy);
+                       if (encap_destroy)
+                               encap_destroy(sk);
+               }
+               if (up->encap_enabled)
+                       static_branch_disable(&udp_encap_needed_key);
        }
 }
 

(quoted diff) +0 -0

@@ -2431,7 +2435,9 @@ int udp_lib_setsockopt(struct sock *sk, int level, int optname,

net/ipv4/udp.c int udp_lib_setsockopt(struct sock *sk, int level, int optname, · line 2440

Quoted 7 lines

                        /* FALLTHROUGH */
                case UDP_ENCAP_L2TPINUDP:
                        up->encap_type = val;

net/ipv4/udp.c +3 -1

-                       udp_encap_enable();
+                       if (!up->encap_enabled)
+                               udp_encap_enable();
+                       up->encap_enabled = 1;

nit: no need for the branch: udp_encap_enable already has a branch and is static inline.

Perhaps it makes sense to convert that to take the udp_sock and handle the state change within, to avoid having to open code at multiple locations.

This is the RX counterpart of commit bec1f6f69736 ("udp: generate gso with UDP_SEGMENT"). When UDP_GRO is enabled, such socket is also eligible for GRO in the rx path: UDP segments directed to such socket are assembled into a larger GSO_UDP_L4 packet.

The core UDP GRO support is enabled with setsockopt(UDP_GRO).

Initial benchmark numbers:

Before:
udp rx: 1079 MB/s 769065 calls/s

After:
udp rx: 1466 MB/s 24877 calls/s

This change introduces a side effect in respect to UDP tunnels:
after a UDP tunnel creation, now the kernel performs a lookup per ingress UDP packet, while before such lookup happened only if the ingress packet carried a valid internal header csum.

v1 -> v2:
- use a new option to enable UDP GRO
- use static keys to protect the UDP GRO socket lookup

Signed-off-by Paolo Abeni <[email protected]>

---
 include/linux/udp.h      |   3 +-
 include/uapi/linux/udp.h |   1 +
 net/ipv4/udp.c           |   7 +++
 net/ipv4/udp_offload.c   | 109 +++++++++++++++++++++++++++++++--------
 net/ipv6/udp_offload.c   |   6 +--
 5 files changed, 98 insertions(+), 28 deletions(-)

include/linux/udp.h +2 -1

index a4dafff407fb..f613b329852e 100644
@@ -50,11 +50,12 @@ struct udp_sock {
        __u8             encap_type;    /* Is this an Encapsulation socket? */
        unsigned char    no_check6_tx:1,/* Send zero UDP6 checksums on TX? */
                         no_check6_rx:1,/* Allow zero UDP6 checksums on RX? */
-                        encap_enabled:1; /* This socket enabled encap
+                        encap_enabled:1, /* This socket enabled encap
                                           * processing; UDP tunnels and
                                           * different encapsulation layer set
                                           * this
                                           */
+                        gro_enabled:1; /* Can accept GRO packets */
 
        /*
         * Following member retains the information to create a UDP header

* when the socket is uncorked.

include/uapi/linux/udp.h +1 -0

index 09502de447f5..30baccb6c9c4 100644
@@ -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     /* This socket can receive UDP GRO packets */
 
 /* UDP encapsulation types */
 #define UDP_ENCAP_ESPINUDP_NON_IKE     1 /* draft-ietf-ipsec-nat-t-ike-00/01 */

net/ipv4/udp.c +1 -0

index 9fcb5374e166..3c277378814f 100644
@@ -115,6 +115,7 @@
 #include "udp_impl.h"
 #include <net/sock_reuseport.h>
 #include <net/addrconf.h>
+#include <net/udp_tunnel.h>
 
 struct udp_table udp_table __read_mostly;
 EXPORT_SYMBOL(udp_table);

Quoted 82 lines · Paolo Abeni

This is the RX counterpart of commit bec1f6f69736 ("udp: generate gso with UDP_SEGMENT"). When UDP_GRO is enabled, such socket is also eligible for GRO in the rx path: UDP segments directed to such socket are assembled into a larger GSO_UDP_L4 packet.

The core UDP GRO support is enabled with setsockopt(UDP_GRO).

Initial benchmark numbers:

Before:
udp rx: 1079 MB/s 769065 calls/s

After:
udp rx: 1466 MB/s 24877 calls/s

This change introduces a side effect in respect to UDP tunnels:
after a UDP tunnel creation, now the kernel performs a lookup per ingress UDP packet, while before such lookup happened only if the ingress packet carried a valid internal header csum.

v1 -> v2:
- use a new option to enable UDP GRO
- use static keys to protect the UDP GRO socket lookup

Signed-off-by Paolo Abeni <[email protected]>

---
 include/linux/udp.h      |   3 +-
 include/uapi/linux/udp.h |   1 +
 net/ipv4/udp.c           |   7 +++
 net/ipv4/udp_offload.c   | 109 +++++++++++++++++++++++++++++++--------
 net/ipv6/udp_offload.c   |   6 +--
 5 files changed, 98 insertions(+), 28 deletions(-)

include/linux/udp.h +2 -1

index a4dafff407fb..f613b329852e 100644
@@ -50,11 +50,12 @@ struct udp_sock {
        __u8             encap_type;    /* Is this an Encapsulation socket? */
        unsigned char    no_check6_tx:1,/* Send zero UDP6 checksums on TX? */
                         no_check6_rx:1,/* Allow zero UDP6 checksums on RX? */
-                        encap_enabled:1; /* This socket enabled encap
+                        encap_enabled:1, /* This socket enabled encap
                                           * processing; UDP tunnels and
                                           * different encapsulation layer set
                                           * this
                                           */
+                        gro_enabled:1; /* Can accept GRO packets */
 
        /*
         * Following member retains the information to create a UDP header

* when the socket is uncorked.

include/uapi/linux/udp.h +1 -0

index 09502de447f5..30baccb6c9c4 100644
@@ -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     /* This socket can receive UDP GRO packets */
 
 /* UDP encapsulation types */
 #define UDP_ENCAP_ESPINUDP_NON_IKE     1 /* draft-ietf-ipsec-nat-t-ike-00/01 */

net/ipv4/udp.c +1 -0

index 9fcb5374e166..3c277378814f 100644
@@ -115,6 +115,7 @@
 #include "udp_impl.h"
 #include <net/sock_reuseport.h>
 #include <net/addrconf.h>
+#include <net/udp_tunnel.h>
 
 struct udp_table udp_table __read_mostly;
 EXPORT_SYMBOL(udp_table);

@@ -2459,6 +2460,12 @@ int udp_lib_setsockopt(struct sock *sk, int level, int optname,
                up->gso_size = val;
                break;
 

net/ipv4/udp.c int udp_lib_setsockopt(struct sock *sk, int level, int optname, · line 2466

Quoted 4 lines

net/ipv4/udp.c +4 -0

+       case UDP_GRO:
+               if (valbool)
+                       udp_tunnel_encap_enable(sk->sk_socket);
+               up->gro_enabled = valbool;

The socket lock is not held here, so multiple updates to up->gro_enabled and the up->encap_enabled and the static branch can race. Syzkaller is adept at generating those.

Quoted 4 lines

net/ipv4/udp.c +4 -0

+#define UDO_GRO_CNT_MAX 64
+static struct sk_buff *udp_gro_receive_segment(struct list_head *head,
+                                              struct sk_buff *skb)
+{

net/ipv4/udp.c int udp_lib_setsockopt(struct sock *sk, int level, int optname,

Quoted 6 lines

net/ipv4/udp.c +6 -0

+       struct udphdr *uh = udp_hdr(skb);
+       struct sk_buff *pp = NULL;
+       struct udphdr *uh2;
+       struct sk_buff *p;
+
+       /* requires non zero csum, for simmetry with GSO */

symmetry

Quoted 83 lines · Willem de Bruijn

Quoted 82 lines · Paolo Abeni

This is the RX counterpart of commit bec1f6f69736 ("udp: generate gso with UDP_SEGMENT"). When UDP_GRO is enabled, such socket is also eligible for GRO in the rx path: UDP segments directed to such socket are assembled into a larger GSO_UDP_L4 packet.

The core UDP GRO support is enabled with setsockopt(UDP_GRO).

Initial benchmark numbers:

Before:
udp rx: 1079 MB/s 769065 calls/s

After:
udp rx: 1466 MB/s 24877 calls/s

This change introduces a side effect in respect to UDP tunnels:
after a UDP tunnel creation, now the kernel performs a lookup per ingress UDP packet, while before such lookup happened only if the ingress packet carried a valid internal header csum.

v1 -> v2:
- use a new option to enable UDP GRO
- use static keys to protect the UDP GRO socket lookup

Signed-off-by Paolo Abeni <[email protected]>

---
 include/linux/udp.h      |   3 +-
 include/uapi/linux/udp.h |   1 +
 net/ipv4/udp.c           |   7 +++
 net/ipv4/udp_offload.c   | 109 +++++++++++++++++++++++++++++++--------
 net/ipv6/udp_offload.c   |   6 +--
 5 files changed, 98 insertions(+), 28 deletions(-)

include/linux/udp.h +2 -1

index a4dafff407fb..f613b329852e 100644
@@ -50,11 +50,12 @@ struct udp_sock {
        __u8             encap_type;    /* Is this an Encapsulation socket? */
        unsigned char    no_check6_tx:1,/* Send zero UDP6 checksums on TX? */
                         no_check6_rx:1,/* Allow zero UDP6 checksums on RX? */
-                        encap_enabled:1; /* This socket enabled encap
+                        encap_enabled:1, /* This socket enabled encap
                                           * processing; UDP tunnels and
                                           * different encapsulation layer set
                                           * this
                                           */
+                        gro_enabled:1; /* Can accept GRO packets */
 
        /*
         * Following member retains the information to create a UDP header

* when the socket is uncorked.

include/uapi/linux/udp.h +1 -0

index 09502de447f5..30baccb6c9c4 100644
@@ -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     /* This socket can receive UDP GRO packets */
 
 /* UDP encapsulation types */
 #define UDP_ENCAP_ESPINUDP_NON_IKE     1 /* draft-ietf-ipsec-nat-t-ike-00/01 */

net/ipv4/udp.c +1 -0

index 9fcb5374e166..3c277378814f 100644
@@ -115,6 +115,7 @@
 #include "udp_impl.h"
 #include <net/sock_reuseport.h>
 #include <net/addrconf.h>
+#include <net/udp_tunnel.h>
 
 struct udp_table udp_table __read_mostly;
 EXPORT_SYMBOL(udp_table);

@@ -2459,6 +2460,12 @@ int udp_lib_setsockopt(struct sock *sk, int level, int optname,
                up->gso_size = val;
                break;
 

Quoted 8 lines

net/ipv4/udp.c int udp_lib_setsockopt(struct sock *sk, int level, int optname, · line 2466

Quoted 4 lines

net/ipv4/udp.c +4 -0

+       case UDP_GRO:
+               if (valbool)
+                       udp_tunnel_encap_enable(sk->sk_socket);
+               up->gro_enabled = valbool;

The socket lock is not held here, so multiple updates to up->gro_enabled and the up->encap_enabled and the static branch can race. Syzkaller is adept at generating those.

Good catch. I was fooled by the current existing code. I think there are potentially similar issues for UDP_ENCAP, UDPLITE_SEND_CSCOV, ...

Since the rx path don't take it anymore and we don't risk starving, I think we should could/always acquire the socket lock on setsockopt, wdyt?

+#define UDO_GRO_CNT_MAX 64

net/ipv4/udp_offload.c +13 -0

+
+	/* 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)
+			continue;
+
+		uh2 = udp_hdr(p);
+
+		/* Match ports only, as csum is always non zero */
+		if ((*(u32 *)&uh->source != *(u32 *)&uh2->source)) {

net/ipv4/udp_offload.c +10 -0

+			pp = p;
+		else if (uh->len != uh2->len)
+			pp = p;
+
+		return pp;
+	}
+
+	/* mismatch, but we never need to flush */
+	return NULL;
+}

Quoted 2 lines · Paolo Abeni

+#define UDO_GRO_CNT_MAX 64

Maybe better UDP_GRO_CNT_MAX?

Btw. do we really need this explicit limit? We should not get more than 64 packets during one napi poll cycle.

Quoted 12 lines

net/ipv4/udp_offload.c +11 -0

+static struct sk_buff *udp_gro_receive_segment(struct list_head *head,
+					       struct sk_buff *skb)
+{
+	struct udphdr *uh = udp_hdr(skb);
+	struct sk_buff *pp = NULL;
+	struct udphdr *uh2;
+	struct sk_buff *p;
+
+	/* requires non zero csum, for simmetry with GSO */
+	if (!uh->check) {
+		NAPI_GRO_CB(skb)->flush = 1;

Review

Quoted 2 lines

(quoted diff) +2 -0

+		return NULL;
+	}

Why is the requirement of checksums different than in udp_gro_receive? It's not that I care much about UDP packets without a checksum, but you would not need to implement your own loop if the requirement could be the same as in udp_gro_receive.

Quoted 17 lines

net/ipv4/udp_offload.c +17 -0

+
+	/* 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)
+			continue;
+
+		uh2 = udp_hdr(p);
+
+		/* Match ports only, as csum is always non zero */
+		if ((*(u32 *)&uh->source != *(u32 *)&uh2->source)) {
+			NAPI_GRO_CB(p)->same_flow = 0;
+			continue;
+		}
+

net/ipv4/udp_offload.c static struct sk_buff *udp4_ufo_fragment(struct sk_buff *skb, · line 382

Quoted 6 lines

net/ipv4/udp_offload.c +6 -0

+		/* 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 execessive truesize values
+		 */
+		if (!skb_gro_receive(p, skb) &&
+		    NAPI_GRO_CB(p)->count > UDO_GRO_CNT_MAX)

This allows to merge UDO_GRO_CNT_MAX + 1 packets.

Quoted 12 lines

Quoted 12 lines

net/ipv4/udp_offload.c +11 -0

+static struct sk_buff *udp_gro_receive_segment(struct list_head *head,
+                                          struct sk_buff *skb)
+{
+   struct udphdr *uh = udp_hdr(skb);
+   struct sk_buff *pp = NULL;
+   struct udphdr *uh2;
+   struct sk_buff *p;
+
+   /* requires non zero csum, for simmetry with GSO */
+   if (!uh->check) {
+           NAPI_GRO_CB(skb)->flush = 1;

Quoted 8 lines

Review

Quoted 2 lines

(quoted diff) +2 -0

+           return NULL;
+   }

Why is the requirement of checksums different than in udp_gro_receive? It's not that I care much about UDP packets without a checksum, but you would not need to implement your own loop if the requirement could be the same as in udp_gro_receive.

uhm....
AFAIU, we need to generated aggregated packets that UDP GSO is able to process/segment. I was unable to get a nocsum packet segment (possibly PEBKAC) so I enforced that condition on the rx path.

@Willem: did I see ghost here? is UDP_SEGMENT fine with no checksum segment?

When UDP GRO is enabled, the UDP_GRO cmsg will carry the ingress datagram size. User-space can use such info to compute the original packets layout.

Signed-off-by Paolo Abeni <[email protected]>

---
CHECK: should we use a separate setsockopt to explicitly enable gso_size cmsg reception? So that user space can enable UDP_GRO and fetch cmsg without forcefully receiving GRO related info.

net/ipv4/udp.c +0 -0

        if (inet->cmsg_flags)
                ip_cmsg_recv_offset(msg, sk, skb, sizeof(struct udphdr), off);
 

Quoted 10 lines · Paolo Abeni

When UDP GRO is enabled, the UDP_GRO cmsg will carry the ingress datagram size. User-space can use such info to compute the original packets layout.

Signed-off-by Paolo Abeni <[email protected]>

---
CHECK: should we use a separate setsockopt to explicitly enable gso_size cmsg reception? So that user space can enable UDP_GRO and fetch cmsg without forcefully receiving GRO related info.

A user can avoid the message by not passing control data. Though in most practical cases it seems unsafe to do so, anyway, as the path MTU can be lower than the expected device MTU.

net/ipv4/udp.c int udp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len, int noblock, · line 1720

Quoted 12 lines

net/ipv4/udp.c +4 -0

index 3c277378814f..2331ac9de954 100644
@@ -1714,6 +1714,10 @@ int udp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len, int noblock,
                memset(sin->sin_zero, 0, sizeof(sin->sin_zero));
                *addr_len = sizeof(*sin);
        }
+
+       if (udp_sk(sk)->gro_enabled)
+               udp_cmsg_recv(msg, sk, skb);
+

Perhaps we can avoid adding a branch by setting a bit in inet->cmsg_flags for gso_size to let the below branch handle the cmsg processing.

I'd still set that as part of the UDP_GRO setsockopt. Though if you insist it could be a value 2 instead of 1, effectively allowing the above mentioned opt-out.

In some scenarios, the GRO engine can assemble an UDP GRO packet that ultimately lands on a non GRO-enabled socket.
This patch tries to address the issue explicitly checking for the UDP socket features before enqueuing the packet, and eventually segmenting the unexpected GRO packet, as needed.

We must also cope with re-insertion requests: after segmentation the UDP code calls the helper introduced by the previous patches, as needed.

Signed-off-by Paolo Abeni <[email protected]>

---

include/linux/udp.h +21 -0

+static inline struct sk_buff *udp_rcv_segment(struct sock *sk,
+                                             struct sk_buff *skb)
+{
+       struct sk_buff *segs;
+
+       /* the GSO CB lays after the UDP one, no need to save and restore any
+        * CB fragment, just initialize it
+        */
+       segs = __skb_gso_segment(skb, NETIF_F_SG, false);
+       if (unlikely(IS_ERR(segs)))
+               kfree_skb(skb);
+       else if (segs)
+               consume_skb(skb);
+       return segs;
+}
+
+
+void ip_protocol_deliver_rcu(struct net *net, struct sk_buff *skb, int proto);
+
+static int udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
+{

Quoted 37 lines · Paolo Abeni

In some scenarios, the GRO engine can assemble an UDP GRO packet that ultimately lands on a non GRO-enabled socket.
This patch tries to address the issue explicitly checking for the UDP socket features before enqueuing the packet, and eventually segmenting the unexpected GRO packet, as needed.

We must also cope with re-insertion requests: after segmentation the UDP code calls the helper introduced by the previous patches, as needed.

Signed-off-by Paolo Abeni <[email protected]>

---

include/linux/udp.h +25 -0

+static inline struct sk_buff *udp_rcv_segment(struct sock *sk,
+                                             struct sk_buff *skb)
+{
+       struct sk_buff *segs;
+
+       /* the GSO CB lays after the UDP one, no need to save and restore any
+        * CB fragment, just initialize it
+        */
+       segs = __skb_gso_segment(skb, NETIF_F_SG, false);
+       if (unlikely(IS_ERR(segs)))
+               kfree_skb(skb);
+       else if (segs)
+               consume_skb(skb);
+       return segs;
+}
+
+
+void ip_protocol_deliver_rcu(struct net *net, struct sk_buff *skb, int proto);
+
+static int udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
+{
+       struct sk_buff *next, *segs;
+       int ret;
+
+       if (likely(!udp_unexpected_gso(sk, skb)))

net/ipv4/udp.c static int udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb) · line 2028

Quoted 6 lines

net/ipv4/udp.c +6 -0

+               return udp_queue_rcv_one_skb(sk, skb);
+
+       BUILD_BUG_ON(sizeof(struct udp_skb_cb) > SKB_SGO_CB_OFFSET);
+       __skb_push(skb, -skb_mac_offset(skb));
+       segs = udp_rcv_segment(sk, skb);
+       for (skb = segs; skb; skb = next) {

need to check IS_ERR(segs) again?

Quoted 38 lines · Willem de Bruijn

Quoted 37 lines · Paolo Abeni

In some scenarios, the GRO engine can assemble an UDP GRO packet that ultimately lands on a non GRO-enabled socket.
This patch tries to address the issue explicitly checking for the UDP socket features before enqueuing the packet, and eventually segmenting the unexpected GRO packet, as needed.

We must also cope with re-insertion requests: after segmentation the UDP code calls the helper introduced by the previous patches, as needed.

Signed-off-by Paolo Abeni <[email protected]>

---

include/linux/udp.h +25 -0

+static inline struct sk_buff *udp_rcv_segment(struct sock *sk,
+                                             struct sk_buff *skb)
+{
+       struct sk_buff *segs;
+
+       /* the GSO CB lays after the UDP one, no need to save and restore any
+        * CB fragment, just initialize it
+        */
+       segs = __skb_gso_segment(skb, NETIF_F_SG, false);
+       if (unlikely(IS_ERR(segs)))
+               kfree_skb(skb);
+       else if (segs)
+               consume_skb(skb);
+       return segs;
+}
+
+
+void ip_protocol_deliver_rcu(struct net *net, struct sk_buff *skb, int proto);
+
+static int udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
+{
+       struct sk_buff *next, *segs;
+       int ret;
+
+       if (likely(!udp_unexpected_gso(sk, skb)))

Quoted 8 lines

net/ipv4/udp.c static int udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb) · line 2028

Quoted 6 lines

net/ipv4/udp.c +6 -0

+               return udp_queue_rcv_one_skb(sk, skb);
+
+       BUILD_BUG_ON(sizeof(struct udp_skb_cb) > SKB_SGO_CB_OFFSET);
+       __skb_push(skb, -skb_mac_offset(skb));
+       segs = udp_rcv_segment(sk, skb);
+       for (skb = segs; skb; skb = next) {

need to check IS_ERR(segs) again?

whooops ... yes, I think so, thanks for catching it.

Since the error code is always discarded, perhpas udp_rcv_segment() can simply return 0 when IS_ERR(segs) is true, so we can save a conditional here. This is currently a slower/exceptional path, but if we will enable UDP GRO for forwaded packets, it will be hit often.

include/linux/udp.h +8 -0

+
+static inline struct sk_buff *udp_rcv_segment(struct sock *sk,
+					      struct sk_buff *skb)
+{
+	struct sk_buff *segs;
+
+	/* the GSO CB lays after the UDP one, no need to save and restore any
+	 * CB fragment, just initialize it

#define udp_portaddr_for_each_entry(__sk, list) \

include/linux/udp.h +0 -0

 	hlist_for_each_entry(__sk, list, __sk_common.skc_portaddr_node)

net/ipv4/udp.c +8 -1

index 2331ac9de954..0d55145ce9f5 100644
@@ -1909,7 +1909,7 @@ EXPORT_SYMBOL(udp_encap_enable);
  * Note that in the success and error cases, the skb is assumed to
  * have either been requeued or freed.
  */
-static int udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
+static int udp_queue_rcv_one_skb(struct sock *sk, struct sk_buff *skb)
 {
 	struct udp_sock *up = udp_sk(sk);
 	int is_udplite = IS_UDPLITE(sk);

@@ -2012,6 +2012,29 @@ static int udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
 	return -1;
 }
 
+void ip_protocol_deliver_rcu(struct net *net, struct sk_buff *skb, int proto);
+
+static int udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
+{
+	struct sk_buff *next, *segs;
+	int ret;
+

Quoted 12 lines · Paolo Abeni

include/linux/udp.h +12 -0

+
+static inline struct sk_buff *udp_rcv_segment(struct sock *sk,
+					      struct sk_buff *skb)
+{
+	struct sk_buff *segs;
+
+	/* the GSO CB lays after the UDP one, no need to save and restore any
+	 * CB fragment, just initialize it
+	 */
+	segs = __skb_gso_segment(skb, NETIF_F_SG, false);
+	if (unlikely(IS_ERR(segs)))
+		kfree_skb(skb);

include/linux/udp.h static inline void udp_cmsg_recv(struct msghdr *msg, struct sock *sk, · line 157

Quoted 6 lines

include/linux/udp.h +6 -0

+	else if (segs)
+		consume_skb(skb);
+	return segs;
+}
+
+

One empty line too much.

Quoted 34 lines

#define udp_portaddr_for_each_entry(__sk, list) \

include/linux/udp.h +0 -0

 	hlist_for_each_entry(__sk, list, __sk_common.skc_portaddr_node)

net/ipv4/udp.c +15 -1

index 2331ac9de954..0d55145ce9f5 100644
@@ -1909,7 +1909,7 @@ EXPORT_SYMBOL(udp_encap_enable);
  * Note that in the success and error cases, the skb is assumed to
  * have either been requeued or freed.
  */
-static int udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
+static int udp_queue_rcv_one_skb(struct sock *sk, struct sk_buff *skb)
 {
 	struct udp_sock *up = udp_sk(sk);
 	int is_udplite = IS_UDPLITE(sk);

@@ -2012,6 +2012,29 @@ static int udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
 	return -1;
 }
 
+void ip_protocol_deliver_rcu(struct net *net, struct sk_buff *skb, int proto);
+
+static int udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
+{
+	struct sk_buff *next, *segs;
+	int ret;
+
+	if (likely(!udp_unexpected_gso(sk, skb)))
+		return udp_queue_rcv_one_skb(sk, skb);
+
+	BUILD_BUG_ON(sizeof(struct udp_skb_cb) > SKB_SGO_CB_OFFSET);
+	__skb_push(skb, -skb_mac_offset(skb));
+	segs = udp_rcv_segment(sk, skb);
+	for (skb = segs; skb; skb = next) {

net/ipv4/udp.c static int udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb) · line 2031

Quoted 3 lines

net/ipv4/udp.c +3 -0

+		next = skb->next;
+		__skb_pull(skb, skb_transport_offset(skb));
+		ret = udp_queue_rcv_one_skb(sk, skb);

udp_queue_rcv_one_skb() starts with doing a xfrm4_policy_check(). Maybe we can do this on the GSO packet instead of the segments. So far this code is just for handling a corner case, but this might change.

Quoted 13 lines

Quoted 13 lines

net/ipv4/udp.c +13 -0

+
+static int udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
+{
+	struct sk_buff *next, *segs;
+	int ret;
+
+	if (likely(!udp_unexpected_gso(sk, skb)))
+		return udp_queue_rcv_one_skb(sk, skb);
+
+	BUILD_BUG_ON(sizeof(struct udp_skb_cb) > SKB_SGO_CB_OFFSET);
+	__skb_push(skb, -skb_mac_offset(skb));
+	segs = udp_rcv_segment(sk, skb);
+	for (skb = segs; skb; skb = next) {

Quoted 8 lines

net/ipv4/udp.c static int udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb) · line 2031

Quoted 3 lines

net/ipv4/udp.c +3 -0

+		next = skb->next;
+		__skb_pull(skb, skb_transport_offset(skb));
+		ret = udp_queue_rcv_one_skb(sk, skb);

udp_queue_rcv_one_skb() starts with doing a xfrm4_policy_check(). Maybe we can do this on the GSO packet instead of the segments. So far this code is just for handling a corner case, but this might change.

I thought about keeping the policy check here, but then I preferred what looked the safest option. Perhaps we can improve with a follow-up?

In some scenarios, the GRO engine can assemble an UDP GRO packet that ultimately lands on a non GRO-enabled socket.
This patch tries to address the issue explicitly checking for the UDP socket features before enqueuing the packet, and eventually segmenting the unexpected GRO packet, as needed.

We must also cope with re-insertion requests: after segmentation the UDP code calls the helper introduced by the previous patches, as needed.

Signed-off-by Paolo Abeni <[email protected]>

---
+static inline bool udp_unexpected_gso(struct sock *sk, struct sk_buff *skb)

include/linux/udp.h +11 -0

+{
+	return !udp_sk(sk)->gro_enabled && skb_is_gso(skb) &&
+	       skb_shinfo(skb)->gso_type & SKB_GSO_UDP_L4;
+}
+
+static inline struct sk_buff *udp_rcv_segment(struct sock *sk,
+					      struct sk_buff *skb)
+{
+	struct sk_buff *segs;
+
+	/* the GSO CB lays after the UDP one, no need to save and restore

any

include/linux/udp.h +1 -0

+	 * CB fragment, just initialize it

+void ip_protocol_deliver_rcu(struct net *net, struct sk_buff *skb, int proto);

net/ipv4/udp.c +7 -0

+
+static int udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
+{
+	struct sk_buff *next, *segs;
+	int ret;
+
+	if (likely(!udp_unexpected_gso(sk, skb)))

Quoted 27 lines · Paolo Abeni

In some scenarios, the GRO engine can assemble an UDP GRO packet that ultimately lands on a non GRO-enabled socket.
This patch tries to address the issue explicitly checking for the UDP socket features before enqueuing the packet, and eventually segmenting the unexpected GRO packet, as needed.

We must also cope with re-insertion requests: after segmentation the UDP code calls the helper introduced by the previous patches, as needed.

Signed-off-by Paolo Abeni <[email protected]>

---
+static inline bool udp_unexpected_gso(struct sock *sk, struct sk_buff *skb)

include/linux/udp.h +11 -0

+{
+	return !udp_sk(sk)->gro_enabled && skb_is_gso(skb) &&
+	       skb_shinfo(skb)->gso_type & SKB_GSO_UDP_L4;
+}
+
+static inline struct sk_buff *udp_rcv_segment(struct sock *sk,
+					      struct sk_buff *skb)
+{
+	struct sk_buff *segs;
+
+	/* the GSO CB lays after the UDP one, no need to save and restore

any

include/linux/udp.h +1 -0

+	 * CB fragment, just initialize it

include/linux/udp.h static inline void udp_cmsg_recv(struct msghdr *msg, struct sock *sk, · line 157

Quoted 10 lines

include/linux/udp.h +10 -0

+	 */
+	segs = __skb_gso_segment(skb, NETIF_F_SG, false);
+	if (unlikely(IS_ERR(segs)))
+		kfree_skb(skb);
+	else if (segs)
+		consume_skb(skb);
+	return segs;
+}
+
+

Hi Paolo

Do we need to check for IS_ERR_OR_NULL(segs)

Quoted 13 lines

+void ip_protocol_deliver_rcu(struct net *net, struct sk_buff *skb, int proto);

net/ipv4/udp.c +10 -0

+
+static int udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
+{
+	struct sk_buff *next, *segs;
+	int ret;
+
+	if (likely(!udp_unexpected_gso(sk, skb)))
+		return udp_queue_rcv_one_skb(sk, skb);
+static int udpv6_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
+{

net/ipv6/udp.c static int udpv6_queue_rcv_skb(struct sock *sk, struct sk_buff *skb) · line 654

Quoted 6 lines

net/ipv6/udp.c +6 -0

+	struct sk_buff *next, *segs;
+	int ret;
+
+	if (likely(!udp_unexpected_gso(sk, skb)))
+		return udpv6_queue_rcv_one_skb(sk, skb);
+

Is the "likely" required here?

HW can coalesce all incoming streams of UDP and may not know the socket state.
In that case, a socket not having UDP GRO option might see a penalty here.

Quoted 1 line

Is the "likely" required here?

Not required, but currently helpful IMHO, as we should hit the above only on unlikey and really unwonted configuration.

Note that only SKB_GSO_UDP_L4 GSO packets will not match the above likely condition.

Quoted 4 lines

HW can coalesce all incoming streams of UDP and may not know the socket state.
In that case, a socket not having UDP GRO option might see a penalty here.

Really? Is there any HW creating SKB_GSO_UDP_L4 packets on RX? if the HW is doing that, without this patch, I think it's breaking existing applications (which may expext that the read UDP frame length implicitly describe the application level message length).

And fix a couple of buglets (port option processing, clean termination on SIGINT). This is preparatory work for GRO tests.

Signed-off-by Paolo Abeni <[email protected]>

---
 tools/testing/selftests/net/udpgso_bench_rx.c | 37 +++++++++++++++----
 1 file changed, 30 insertions(+), 7 deletions(-)

Quoted 10 lines · Paolo Abeni

And fix a couple of buglets (port option processing, clean termination on SIGINT). This is preparatory work for GRO tests.

Signed-off-by Paolo Abeni <[email protected]>

---
 tools/testing/selftests/net/udpgso_bench_rx.c | 37 +++++++++++++++----
 1 file changed, 30 insertions(+), 7 deletions(-)

tools/testing/selftests/net/udpgso_bench_rx.c static void do_verify_udp(const char *data, int len) · line 180

Quoted 7 lines

tools/testing/selftests/net/udpgso_bench_rx.c +1 -1

@@ -167,10 +177,10 @@ static void do_verify_udp(const char *data, int len)
 /* Flush all outstanding datagrams. Verify first few bytes of each. */
 static void do_flush_udp(int fd)
 {
-       static char rbuf[ETH_DATA_LEN];
+       static char rbuf[65535];

we can use ETH_MAX_MTU.

XDP support will be used by a later patch to test the GRO path in a net namespace, leveraging the veth XDP implementation. To avoid breaking existing setup, XDP support is conditionally enabled and build only if llc is locally available.

Signed-off-by Paolo Abeni <[email protected]>

---

tools/testing/selftests/net/Makefile +43 -0

index 256d82d5fa87..176459b7c4d6 100644
@@ -16,8 +16,77 @@ TEST_GEN_PROGS = reuseport_bpf reuseport_bpf_cpu reuseport_bpf_numa
 TEST_GEN_PROGS += reuseport_dualstack reuseaddr_conflict tls
 
 KSFT_KHDR_INSTALL := 1
+
+# Allows pointing LLC/CLANG to a LLVM backend with bpf support, redefine on cmdline:
+#  make samples/bpf/ LLC=~/git/llvm/build/bin/llc CLANG=~/git/llvm/build/bin/clang
+LLC ?= llc
+CLANG ?= clang
+LLVM_OBJCOPY ?= llvm-objcopy
+BTF_PAHOLE ?= pahole
+HAS_LLC := $(shell which $(LLC) 2>/dev/null)
+
+# conditional enable testes requiring llc
+ifneq (, $(HAS_LLC))
+TEST_GEN_FILES += xdp_dummy.o
+endif
+
 include ../lib.mk
 
+ifneq (, $(HAS_LLC))
+
+# Detect that we're cross compiling and use the cross compiler
+ifdef CROSS_COMPILE
+CLANG_ARCH_ARGS = -target $(ARCH)
+endif
+
+PROBE := $(shell $(LLC) -march=bpf -mcpu=probe -filetype=null /dev/null 2>&1)
+
+# Let newer LLVM versions transparently probe the kernel for availability
+# of full BPF instruction set.
+ifeq ($(PROBE),)
+  CPU ?= probe
+else
+  CPU ?= generic
+endif
+
+SRC_PATH := $(abspath ../../../..)
+LIB_PATH := $(SRC_PATH)/tools/lib
+XDP_CFLAGS := -D SUPPORT_XDP=1 -I$(LIB_PATH)
+LIBBPF = $(LIB_PATH)/bpf/libbpf.a
+BTF_LLC_PROBE := $(shell $(LLC) -march=bpf -mattr=help 2>&1 | grep dwarfris)
+BTF_PAHOLE_PROBE := $(shell $(BTF_PAHOLE) --help 2>&1 | grep BTF)
+BTF_OBJCOPY_PROBE := $(shell $(LLVM_OBJCOPY) --help 2>&1 | grep -i 'usage.*llvm')
+CLANG_SYS_INCLUDES := $(shell $(CLANG) -v -E - </dev/null 2>&1 \
+        | sed -n '/<...> search starts here:/,/End of search list./{ s| \(/.*\)|-idirafter \1|p }')
+CLANG_FLAGS = -I. -I$(SRC_PATH)/include -I../bpf/ \
+             $(CLANG_SYS_INCLUDES) -Wno-compare-distinct-pointer-types
+

Quoted 66 lines · Paolo Abeni

XDP support will be used by a later patch to test the GRO path in a net namespace, leveraging the veth XDP implementation. To avoid breaking existing setup, XDP support is conditionally enabled and build only if llc is locally available.

Signed-off-by Paolo Abeni <[email protected]>

---

tools/testing/selftests/net/Makefile +48 -0

index 256d82d5fa87..176459b7c4d6 100644
@@ -16,8 +16,77 @@ TEST_GEN_PROGS = reuseport_bpf reuseport_bpf_cpu reuseport_bpf_numa
 TEST_GEN_PROGS += reuseport_dualstack reuseaddr_conflict tls
 
 KSFT_KHDR_INSTALL := 1
+
+# Allows pointing LLC/CLANG to a LLVM backend with bpf support, redefine on cmdline:
+#  make samples/bpf/ LLC=~/git/llvm/build/bin/llc CLANG=~/git/llvm/build/bin/clang
+LLC ?= llc
+CLANG ?= clang
+LLVM_OBJCOPY ?= llvm-objcopy
+BTF_PAHOLE ?= pahole
+HAS_LLC := $(shell which $(LLC) 2>/dev/null)
+
+# conditional enable testes requiring llc
+ifneq (, $(HAS_LLC))
+TEST_GEN_FILES += xdp_dummy.o
+endif
+
 include ../lib.mk
 
+ifneq (, $(HAS_LLC))
+
+# Detect that we're cross compiling and use the cross compiler
+ifdef CROSS_COMPILE
+CLANG_ARCH_ARGS = -target $(ARCH)
+endif
+
+PROBE := $(shell $(LLC) -march=bpf -mcpu=probe -filetype=null /dev/null 2>&1)
+
+# Let newer LLVM versions transparently probe the kernel for availability
+# of full BPF instruction set.
+ifeq ($(PROBE),)
+  CPU ?= probe
+else
+  CPU ?= generic
+endif
+
+SRC_PATH := $(abspath ../../../..)
+LIB_PATH := $(SRC_PATH)/tools/lib
+XDP_CFLAGS := -D SUPPORT_XDP=1 -I$(LIB_PATH)
+LIBBPF = $(LIB_PATH)/bpf/libbpf.a
+BTF_LLC_PROBE := $(shell $(LLC) -march=bpf -mattr=help 2>&1 | grep dwarfris)
+BTF_PAHOLE_PROBE := $(shell $(BTF_PAHOLE) --help 2>&1 | grep BTF)
+BTF_OBJCOPY_PROBE := $(shell $(LLVM_OBJCOPY) --help 2>&1 | grep -i 'usage.*llvm')
+CLANG_SYS_INCLUDES := $(shell $(CLANG) -v -E - </dev/null 2>&1 \
+        | sed -n '/<...> search starts here:/,/End of search list./{ s| \(/.*\)|-idirafter \1|p }')
+CLANG_FLAGS = -I. -I$(SRC_PATH)/include -I../bpf/ \
+             $(CLANG_SYS_INCLUDES) -Wno-compare-distinct-pointer-types
+
+ifneq ($(and $(BTF_LLC_PROBE),$(BTF_PAHOLE_PROBE),$(BTF_OBJCOPY_PROBE)),)
+       CLANG_CFLAGS += -g
+       LLC_FLAGS += -mattr=dwarfris
+       DWARF2BTF = y
+endif

tools/testing/selftests/net/Makefile TEST_GEN_PROGS = reuseport_bpf reuseport_bpf_cpu reuseport_bpf_numa · line 73

Quoted 5 lines

tools/testing/selftests/net/Makefile +5 -0

+
+$(LIBBPF): FORCE
+# Fix up variables inherited from Kbuild that tools/ build system won't like
+       $(MAKE) -C $(dir $@) RM='rm -rf' LDFLAGS= srctree=$(SRC_PATH) O= $(nodir $@)
+

This is a lot of XDP specific code. Not for this patchset, per se, but would be nice if we can reuse the logic in selftests/bpf for all this.

Quoted 6 lines

tools/testing/selftests/net/Makefile +1 -1

--- a/tools/testing/selftests/net/udpgso_bench_rx.c

@@ -227,6 +234,13 @@ static void parse_opts(int argc, char **argv)
                        cfg_verify = true;
                        cfg_read_all = true;
                        break;
+#ifdef SUPPORT_XDP

tools/testing/selftests/net/Makefile static void parse_opts(int argc, char **argv) · line 243

Quoted 6 lines

tools/testing/selftests/net/Makefile +6 -0

+               case 'x':
+                       cfg_xdp_iface = if_nametoindex(optarg);
+                       if (!cfg_xdp_iface)
+                               error(1, errno, "unknown interface %s", optarg);
+                       break;
+#endif

nit: needs to be added to getopt string in this patch.

Extends the existing udp programs to allow checking for proper GRO aggregation/GSO size, and run the tests via a shell script, using a veth pair with XDP program attached to trigger the GRO code path.

Signed-off-by Paolo Abeni <[email protected]>

---
 tools/testing/selftests/net/Makefile          |   2 +-
 tools/testing/selftests/net/udpgro.sh         | 144 ++++++++++++++++++
 tools/testing/selftests/net/udpgro_bench.sh   |   8 +-
 tools/testing/selftests/net/udpgso_bench.sh   |   2 +-
 tools/testing/selftests/net/udpgso_bench_rx.c | 125 +++++++++++++--
 tools/testing/selftests/net/udpgso_bench_tx.c |  22 ++-
 6 files changed, 281 insertions(+), 22 deletions(-)
 create mode 100755 tools/testing/selftests/net/udpgro.sh

Quoted 16 lines · Paolo Abeni

Extends the existing udp programs to allow checking for proper GRO aggregation/GSO size, and run the tests via a shell script, using a veth pair with XDP program attached to trigger the GRO code path.

Signed-off-by Paolo Abeni <[email protected]>

---
 tools/testing/selftests/net/Makefile          |   2 +-
 tools/testing/selftests/net/udpgro.sh         | 144 ++++++++++++++++++
 tools/testing/selftests/net/udpgro_bench.sh   |   8 +-
 tools/testing/selftests/net/udpgso_bench.sh   |   2 +-
 tools/testing/selftests/net/udpgso_bench_rx.c | 125 +++++++++++++--
 tools/testing/selftests/net/udpgso_bench_tx.c |  22 ++-
 6 files changed, 281 insertions(+), 22 deletions(-)
 create mode 100755 tools/testing/selftests/net/udpgro.sh

tools/testing/selftests/net/udpgro.sh line 122

Quoted 3 lines

tools/testing/selftests/net/udpgro.sh +2 -0

+       run_test "no GRO chk cmsg" "${ipv4_args} -M 10 -s 1400" "-4 -n 10 -l 1400 -S -1"
+       run_test "no GRO chk cmsg" "${ipv6_args} -M 10 -s 1400" "-n 10 -l 1400 -S -1"

why expected segment size -1 in these two?

tools/testing/selftests/net/udpgso_bench_tx.c static int send_udp_segment(int fd, char *data) · line 247

Quoted 7 lines

tools/testing/selftests/net/udpgso_bench_tx.c +1 -1

 static void usage(const char *filepath)
 {
-       error(1, 0, "Usage: %s [-46cmStuz] [-C cpu] [-D dst ip] [-l secs] [-p port] [-s sendsize]",
+       error(1, 0, "Usage: %s [-46cmtuz] [-C cpu] [-D dst ip] [-l secs] [-m messagenr] [-p port] [-s sendsize] [-S gsosize]",
                    filepath);

missing -M

This series implements GRO support for UDP sockets, as the RX counterpart of commit bec1f6f69736 ("udp: generate gso with UDP_SEGMENT").
The core functionality is implemented by the second patch, introducing a new sockopt to enable UDP_GRO, while patch 3 implements support for passing the segment size to the user space via a new cmsg.
UDP GRO performs a socket lookup for each ingress packets and aggregate datagram directed to UDP GRO enabled sockets with constant l4 tuple.

UDP GRO packets can land on non GRO-enabled sockets, e.g. due to iptables NAT rules, and that could potentially confuse existing applications.

The solution adopted here is to de-segment the GRO packet before enqueuing as needed. Since we must cope with packet reinsertion after de-segmentation, the relevant code is factored-out in ipv4 and ipv6 specific helpers and exposed to UDP usage.

While the current code can probably be improved, this safeguard ,implemented in the patches 4-7, allows future enachements to enable UDP GSO offload on more virtual devices eventually even on forwarded packets.

The last 4 for patches implement some performance and functional self-tests, re-using the existing udpgso infrastructure. The problematic scenario described above is explicitly tested.

Quoted 11 lines · Paolo Abeni

This series implements GRO support for UDP sockets, as the RX counterpart of commit bec1f6f69736 ("udp: generate gso with UDP_SEGMENT").
The core functionality is implemented by the second patch, introducing a new sockopt to enable UDP_GRO, while patch 3 implements support for passing the segment size to the user space via a new cmsg.
UDP GRO performs a socket lookup for each ingress packets and aggregate datagram directed to UDP GRO enabled sockets with constant l4 tuple.

UDP GRO packets can land on non GRO-enabled sockets, e.g. due to iptables NAT rules, and that could potentially confuse existing applications.

Good catch.

Quoted 9 lines

The solution adopted here is to de-segment the GRO packet before enqueuing as needed. Since we must cope with packet reinsertion after de-segmentation, the relevant code is factored-out in ipv4 and ipv6 specific helpers and exposed to UDP usage.

While the current code can probably be improved, this safeguard ,implemented in the patches 4-7, allows future enachements to enable UDP GSO offload on more virtual devices eventually even on forwarded packets.

Quoted 3 lines

The last 4 for patches implement some performance and functional self-tests, re-using the existing udpgso infrastructure. The problematic scenario described above is explicitly tested.

This looks awesome! Impressive testing, too.

A few comments in the individual patches, mostly minor.

This series implements GRO support for UDP sockets, as the RX counterpart of commit bec1f6f69736 ("udp: generate gso with UDP_SEGMENT").
The core functionality is implemented by the second patch, introducing a new sockopt to enable UDP_GRO, while patch 3 implements support for passing the segment size to the user space via a new cmsg.
UDP GRO performs a socket lookup for each ingress packets and aggregate datagram directed to UDP GRO enabled sockets with constant l4 tuple.

UDP GRO packets can land on non GRO-enabled sockets, e.g. due to iptables NAT rules, and that could potentially confuse existing applications.

The solution adopted here is to de-segment the GRO packet before enqueuing as needed. Since we must cope with packet reinsertion after de-segmentation, the relevant code is factored-out in ipv4 and ipv6 specific helpers and exposed to UDP usage.

While the current code can probably be improved, this safeguard ,implemented in the patches 4-7, allows future enachements to enable UDP GSO offload on more virtual devices eventually even on forwarded packets.

Quoted 11 lines · Paolo Abeni

This series implements GRO support for UDP sockets, as the RX counterpart of commit bec1f6f69736 ("udp: generate gso with UDP_SEGMENT").
The core functionality is implemented by the second patch, introducing a new sockopt to enable UDP_GRO, while patch 3 implements support for passing the segment size to the user space via a new cmsg.
UDP GRO performs a socket lookup for each ingress packets and aggregate datagram directed to UDP GRO enabled sockets with constant l4 tuple.

UDP GRO packets can land on non GRO-enabled sockets, e.g. due to iptables NAT rules, and that could potentially confuse existing applications.

Quoted 8 lines

The solution adopted here is to de-segment the GRO packet before enqueuing as needed. Since we must cope with packet reinsertion after de-segmentation, the relevant code is factored-out in ipv4 and ipv6 specific helpers and exposed to UDP usage.

While the current code can probably be improved, this safeguard ,implemented in the patches 4-7, allows future enachements to enable UDP GSO offload on more virtual devices eventually even on forwarded packets.

I was curious what I get with this when doing packet forwarding. So I added forwarding support with the patch below (on top of your patchset). While the packet processing could work the way I do it in this patch, I'm not sure about the way I enable UDP GRO on forwarding. Maybe there it a better way than just do UDP GRO if forwarding is enabled on the receiving device.

Some quick benchmark numbers with UDP packet forwarding (1460 byte packets) through two gateways:

net-next: 16.4 Gbps

net-next + UDP GRO: 20.3 Gbps

Quoted 6 lines · Steffen Klassert

Some quick benchmark numbers with UDP packet forwarding (1460 byte packets) through two gateways:

net-next: 16.4 Gbps

net-next + UDP GRO: 20.3 Gbps

uhmmm... what do you think about this speed-up ?