udp: implement GRO support

RFC PATCH In review Paolo Abeni 3 participants 19 messages Oct 31, 2018 – Nov 2, 2018

Subsystems (5)

Networking Network drivers Kselftest IPv4/IPv6 +1 more subsystems

lore

Share & tools

lore

Paolo Abeni

author maintainer

2018-10-31 02:19 UTC #0 lore raw

v3 cover udp: implement GRO support

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.

This revision of the series try to address the feedback provided by Willem, Steffen and Subash fixing several bugs all along

rfc v2 - rfc v3:
- cope better with exceptional conditions
- test cases cleanup

rfc v1 - rfc v2:
- use a new option to enable UDP GRO
- use static keys to protect the UDP GRO socket lookup
- cope with UDP GRO misdirection
- add self-tests

Paolo Abeni (10):
udp: implement complete book-keeping for encap_needed
udp: implement GRO for plain UDP sockets.
udp: add support for UDP_GRO cmsg
ip: factor out protocol delivery helper
ipv6: factor out protocol delivery helper
udp: cope with UDP GRO packet misdirection
selftests: add GRO support to udp bench rx program
selftests: conditionally enable XDP support in udpgso_bench_rx
selftests: add some benchmark for UDP GRO
selftests: add functionals test for UDP GRO

 include/linux/udp.h                           |  25 ++-
 include/net/udp.h                             |  51 ++++-
 include/net/udp_tunnel.h                      |   6 +
 include/uapi/linux/udp.h                      |   1 +
 net/ipv4/ip_input.c                           |  73 ++++---
 net/ipv4/udp.c                                |  54 ++++-
 net/ipv4/udp_offload.c                        | 109 ++++++++--
 net/ipv6/ip6_input.c                          |  28 +--
 net/ipv6/udp.c                                |  44 +++-
 net/ipv6/udp_offload.c                        |   6 +-
 tools/testing/selftests/net/Makefile          |  70 +++++++
 tools/testing/selftests/net/udpgro.sh         | 147 +++++++++++++
 tools/testing/selftests/net/udpgro_bench.sh   |  94 +++++++++
 tools/testing/selftests/net/udpgso_bench.sh   |   2 +-
 tools/testing/selftests/net/udpgso_bench_rx.c | 193 ++++++++++++++++--
 tools/testing/selftests/net/udpgso_bench_tx.c |  22 +-
 tools/testing/selftests/net/xdp_dummy.c       |  13 ++
 17 files changed, 816 insertions(+), 122 deletions(-)
 create mode 100755 tools/testing/selftests/net/udpgro.sh
 create mode 100755 tools/testing/selftests/net/udpgro_bench.sh
 create mode 100644 tools/testing/selftests/net/xdp_dummy.c

Signature · 1 line

2.17.2

Paolo Abeni

author maintainer

2018-10-31 02:19 UTC #1 lore raw

RFC PATCH v3 1/10 udp: implement complete book-keeping for encap_needed

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.

rfc v2 - rfc v3:
- use udp_tunnel_encap_enable() in setsockopt()

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

---
 include/linux/udp.h      |  7 ++++++-
 include/net/udp_tunnel.h |  6 ++++++
 net/ipv4/udp.c           | 17 +++++++++++------
 net/ipv6/udp.c           | 14 +++++++++-----
 4 files changed, 32 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 +11 -6

index ca3ed931f2a9..c51721fb293a 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);

@@ -2398,11 +2399,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);
 	}
 }

@@ -2447,7 +2452,7 @@ int udp_lib_setsockopt(struct sock *sk, int level, int optname,
 			/* FALLTHROUGH */
 		case UDP_ENCAP_L2TPINUDP:
 			up->encap_type = val;
-			udp_encap_enable();
+			udp_tunnel_encap_enable(sk->sk_socket);
 			break;
 		default:
 			err = -ENOPROTOOPT;

net/ipv6/udp.c +9 -5

index d2d97d07ef27..fc0ce6c59ebb 100644
@@ -1458,11 +1458,15 @@ void udpv6_destroy_sock(struct sock *sk)
 	udp_v6_flush_pending_frames(sk);
 	release_sock(sk);
 
-	if (static_branch_unlikely(&udpv6_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(&udpv6_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(&udpv6_encap_needed_key);
 	}
 
 	inet6_destroy_sock(sk);

Signature · 1 line

2.17.2

Paolo Abeni

author maintainer

2018-10-31 02:19 UTC #4 lore raw

RFC PATCH v3 2/10 udp: implement GRO for plain UDP sockets.

In reply to 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.

rfc v2 -> rfc v3:
- fixed typos in macro name and comments
- really enforce UDP_GRO_CNT_MAX, instead of UDP_GRO_CNT_MAX + 1
- acquire socket lock in UDP_GRO setsockopt

rfc v1 -> rfc 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]>

--
Note: I opted for acquiring the socket lock only for the newly introduced setsockopt instead for every value, despite the previous conversation on this topic, to avoid introducing somewhat larger and unrelated changes.

---
 include/linux/udp.h      |   3 +-
 include/uapi/linux/udp.h |   1 +
 net/ipv4/udp.c           |   8 +++
 net/ipv4/udp_offload.c   | 109 +++++++++++++++++++++++++++++++--------
 net/ipv6/udp_offload.c   |   6 +--
 5 files changed, 99 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 +8 -0

index c51721fb293a..4d4f4d044c28 100644
@@ -2474,6 +2474,14 @@ int udp_lib_setsockopt(struct sock *sk, int level, int optname,
 		up->gso_size = val;
 		break;
 
+	case UDP_GRO:
+		lock_sock(sk);
+		if (valbool)
+			udp_tunnel_encap_enable(sk->sk_socket);
+		up->gro_enabled = valbool;
+		release_sock(sk);
+		break;
+
 	/*
 	 * 	UDP-Lite's partial checksum coverage (RFC 3828).
 	 */

net/ipv4/udp_offload.c +87 ~~-22~~

158 lines not on this page · raw message

net/ipv6/udp_offload.c +1 -5

index 1b8e161ac527..828b2457f97b 100644
@@ -147,13 +147,9 @@ static 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 (uh->check) {
-		skb_shinfo(skb)->gso_type |= SKB_GSO_UDP_TUNNEL_CSUM;
+	if (uh->check)
 		uh->check = ~udp_v6_check(skb->len - nhoff, &ipv6h->saddr,
 					  &ipv6h->daddr, 0);
-	} else {
-		skb_shinfo(skb)->gso_type |= SKB_GSO_UDP_TUNNEL;
-	}
 
 	return udp_gro_complete(skb, nhoff, udp6_lib_lookup_skb);
 }

Signature · 1 line

2.17.2

Paolo Abeni

author maintainer

2018-10-31 02:19 UTC #5 lore raw

RFC PATCH v3 3/10 udp: add support for UDP_GRO cmsg

In reply to 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]>

---
Note: I avoided setting a bit in cmsg_flag for UDP_GRO, as that attempt produced some uglyfication, expecially on the ipv6 side with no measurable performances benefits.

---
 include/linux/udp.h | 11 +++++++++++
 net/ipv4/udp.c      |  4 ++++
 net/ipv6/udp.c      |  3 +++
 3 files changed, 18 insertions(+)

include/linux/udp.h +11 -0

index f613b329852e..e23d5024f42f 100644
@@ -121,6 +121,17 @@ static inline bool udp_get_no_check6_rx(struct sock *sk)
 	return udp_sk(sk)->no_check6_rx;
 }
 
+static inline void udp_cmsg_recv(struct msghdr *msg, struct sock *sk,
+				 struct sk_buff *skb)
+{
+	int gso_size;
+
+	if (skb_shinfo(skb)->gso_type & SKB_GSO_UDP_L4) {
+		gso_size = skb_shinfo(skb)->gso_size;
+		put_cmsg(msg, SOL_UDP, UDP_GRO, sizeof(gso_size), &gso_size);
+	}
+}
+
 #define udp_portaddr_for_each_entry(__sk, list) \
 	hlist_for_each_entry(__sk, list, __sk_common.skc_portaddr_node)

net/ipv4/udp.c +4 -0

index 4d4f4d044c28..b345f71b1cbb 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);
+
 	if (inet->cmsg_flags)
 		ip_cmsg_recv_offset(msg, sk, skb, sizeof(struct udphdr), off);

net/ipv6/udp.c +3 -0

index fc0ce6c59ebb..8e76e719305c 100644
@@ -421,6 +421,9 @@ int udpv6_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
 		*addr_len = sizeof(*sin6);
 	}
 
+	if (udp_sk(sk)->gro_enabled)
+		udp_cmsg_recv(msg, sk, skb);
+
 	if (np->rxopt.all)
 		ip6_datagram_recv_common_ctl(sk, msg, skb);

Signature · 1 line

2.17.2

Paolo Abeni

author maintainer

2018-10-31 02:19 UTC #6 lore raw

RFC PATCH v3 4/10 ip: factor out protocol delivery helper

In reply to Paolo Abeni

So that we can re-use it at the UDP lavel in a later patch

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

---
 net/ipv4/ip_input.c | 73 ++++++++++++++++++++++-----------------------
 1 file changed, 36 insertions(+), 37 deletions(-)

net/ipv4/ip_input.c +36 ~~-37~~

index 35a786c0aaa0..72250b4e466d 100644
@@ -188,51 +188,50 @@ bool ip_call_ra_chain(struct sk_buff *skb)
 	return false;
 }
 
-static int ip_local_deliver_finish(struct net *net, struct sock *sk, struct sk_buff *skb)
+void ip_protocol_deliver_rcu(struct net *net, struct sk_buff *skb, int protocol)
 {
-	__skb_pull(skb, skb_network_header_len(skb));
-
-	rcu_read_lock();
-	{
-		int protocol = ip_hdr(skb)->protocol;
-		const struct net_protocol *ipprot;
-		int raw;
+	const struct net_protocol *ipprot;
+	int raw, ret;
 
-	resubmit:
-		raw = raw_local_deliver(skb, protocol);
+resubmit:
+	raw = raw_local_deliver(skb, protocol);
 
-		ipprot = rcu_dereference(inet_protos[protocol]);
-		if (ipprot) {
-			int ret;
-
-			if (!ipprot->no_policy) {
-				if (!xfrm4_policy_check(NULL, XFRM_POLICY_IN, skb)) {
-					kfree_skb(skb);
-					goto out;
-				}
-				nf_reset(skb);
+	ipprot = rcu_dereference(inet_protos[protocol]);
+	if (ipprot) {
+		if (!ipprot->no_policy) {
+			if (!xfrm4_policy_check(NULL, XFRM_POLICY_IN, skb)) {
+				kfree_skb(skb);
+				return;
 			}
-			ret = ipprot->handler(skb);
-			if (ret < 0) {
-				protocol = -ret;
-				goto resubmit;
+			nf_reset(skb);
+		}
+		ret = ipprot->handler(skb);
+		if (ret < 0) {
+			protocol = -ret;
+			goto resubmit;
+		}
+		__IP_INC_STATS(net, IPSTATS_MIB_INDELIVERS);
+	} else {
+		if (!raw) {
+			if (xfrm4_policy_check(NULL, XFRM_POLICY_IN, skb)) {
+				__IP_INC_STATS(net, IPSTATS_MIB_INUNKNOWNPROTOS);
+				icmp_send(skb, ICMP_DEST_UNREACH,
+					  ICMP_PROT_UNREACH, 0);
 			}
-			__IP_INC_STATS(net, IPSTATS_MIB_INDELIVERS);
+			kfree_skb(skb);
 		} else {
-			if (!raw) {
-				if (xfrm4_policy_check(NULL, XFRM_POLICY_IN, skb)) {
-					__IP_INC_STATS(net, IPSTATS_MIB_INUNKNOWNPROTOS);
-					icmp_send(skb, ICMP_DEST_UNREACH,
-						  ICMP_PROT_UNREACH, 0);
-				}
-				kfree_skb(skb);
-			} else {
-				__IP_INC_STATS(net, IPSTATS_MIB_INDELIVERS);
-				consume_skb(skb);
-			}
+			__IP_INC_STATS(net, IPSTATS_MIB_INDELIVERS);
+			consume_skb(skb);
 		}
 	}
- out:
+}
+
+static int ip_local_deliver_finish(struct net *net, struct sock *sk, struct sk_buff *skb)
+{
+	__skb_pull(skb, skb_network_header_len(skb));
+
+	rcu_read_lock();
+	ip_protocol_deliver_rcu(net, skb, ip_hdr(skb)->protocol);
 	rcu_read_unlock();
 
 	return 0;

Signature · 1 line

2.17.2

Paolo Abeni

author maintainer

2018-10-31 02:19 UTC #9 lore raw

RFC PATCH v3 5/10 ipv6: factor out protocol delivery helper

In reply to Paolo Abeni

So that we can re-use it at the UDP lavel in the next patch

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

---
 net/ipv6/ip6_input.c | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

net/ipv6/ip6_input.c +16 ~~-12~~

index 96577e742afd..3065226bdc57 100644
@@ -319,28 +319,26 @@ void ipv6_list_rcv(struct list_head *head, struct packet_type *pt,
 /*
  *	Deliver the packet to the host
  */
-
-
-static int ip6_input_finish(struct net *net, struct sock *sk, struct sk_buff *skb)
+void ip6_protocol_deliver_rcu(struct net *net, struct sk_buff *skb, int nexthdr,
+			      bool have_final)
 {
 	const struct inet6_protocol *ipprot;
 	struct inet6_dev *idev;
 	unsigned int nhoff;
-	int nexthdr;
 	bool raw;
-	bool have_final = false;
 
 	/*
 	 *	Parse extension headers
 	 */
 
-	rcu_read_lock();
 resubmit:
 	idev = ip6_dst_idev(skb_dst(skb));
-	if (!pskb_pull(skb, skb_transport_offset(skb)))
-		goto discard;
 	nhoff = IP6CB(skb)->nhoff;
-	nexthdr = skb_network_header(skb)[nhoff];
+	if (!have_final) {
+		if (!pskb_pull(skb, skb_transport_offset(skb)))
+			goto discard;
+		nexthdr = skb_network_header(skb)[nhoff];
+	}
 
 resubmit_final:
 	raw = raw6_local_deliver(skb, nexthdr);

@@ -411,13 +409,19 @@ static int ip6_input_finish(struct net *net, struct sock *sk, struct sk_buff *sk
 			consume_skb(skb);
 		}
 	}
-	rcu_read_unlock();
-	return 0;
+	return;
 
 discard:
 	__IP6_INC_STATS(net, idev, IPSTATS_MIB_INDISCARDS);
-	rcu_read_unlock();
 	kfree_skb(skb);
+}
+
+static int ip6_input_finish(struct net *net, struct sock *sk, struct sk_buff *skb)
+{
+	rcu_read_lock();
+	ip6_protocol_deliver_rcu(net, skb, 0, false);
+	rcu_read_unlock();
+
 	return 0;
 }

Signature · 1 line

2.17.2

Paolo Abeni

author maintainer

2018-10-31 02:19 UTC #10 lore raw

RFC PATCH v3 6/10 udp: cope with UDP GRO packet misdirection

In reply to 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.

Segmentation is performed by a common helper, which takes care of updating socket and protocol stats is case of failure.

rfc v2 -> rfc v3
- moved udp_rcv_segment() into net/udp.h, account errors to socket and ns, always return NULL or segs list

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

---
 include/linux/udp.h |  6 ++++++
 include/net/udp.h   | 51 ++++++++++++++++++++++++++++++++++++++-------
 net/ipv4/udp.c      | 25 +++++++++++++++++++++-
 net/ipv6/udp.c      | 27 +++++++++++++++++++++++-
 4 files changed, 99 insertions(+), 10 deletions(-)

include/linux/udp.h +6 -0

index e23d5024f42f..0a9c54e76305 100644
@@ -132,6 +132,12 @@ static inline void udp_cmsg_recv(struct msghdr *msg, struct sock *sk,
 	}
 }
 
+static inline bool udp_unexpected_gso(struct sock *sk, struct sk_buff *skb)
+{
+	return !udp_sk(sk)->gro_enabled && skb_is_gso(skb) &&
+	       skb_shinfo(skb)->gso_type & SKB_GSO_UDP_L4;
+}
+
 #define udp_portaddr_for_each_entry(__sk, list) \
 	hlist_for_each_entry(__sk, list, __sk_common.skc_portaddr_node)

include/net/udp.h +43 -8

64 lines not on this page · raw message

net/ipv4/udp.c +24 -1

index b345f71b1cbb..b45033f63673 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) {
+		next = skb->next;
+		__skb_pull(skb, skb_transport_offset(skb));
+		ret = udp_queue_rcv_one_skb(sk, skb);
+		if (ret > 0)
+			ip_protocol_deliver_rcu(dev_net(skb->dev), skb, -ret);
+	}
+	return 0;
+}
+
 /* For TCP sockets, sk_rx_dst is protected by socket lock
  * For UDP, we use xchg() to guard against concurrent changes.
  */

net/ipv6/udp.c +26 -1

index 8e76e719305c..137c421bef82 100644
@@ -558,7 +558,7 @@ void udpv6_encap_enable(void)
 }
 EXPORT_SYMBOL(udpv6_encap_enable);
 
-static int udpv6_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
+static int udpv6_queue_rcv_one_skb(struct sock *sk, struct sk_buff *skb)
 {
 	struct udp_sock *up = udp_sk(sk);
 	int is_udplite = IS_UDPLITE(sk);

@@ -641,6 +641,31 @@ static int udpv6_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
 	return -1;
 }
 
+void ip6_protocol_deliver_rcu(struct net *net, struct sk_buff *skb, int nexthdr,
+			      bool have_final);
+
+static int udpv6_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 udpv6_queue_rcv_one_skb(sk, skb);
+
+	__skb_push(skb, -skb_mac_offset(skb));
+	segs = udp_rcv_segment(sk, skb);
+	for (skb = segs; skb; skb = next) {
+		next = skb->next;
+		__skb_pull(skb, skb_transport_offset(skb));
+
+		ret = udpv6_queue_rcv_one_skb(sk, skb);
+		if (ret > 0)
+			ip6_protocol_deliver_rcu(dev_net(skb->dev), skb, ret,
+						 true);
+	}
+	return 0;
+}
+
 static bool __udp_v6_is_mcast_sock(struct net *net, struct sock *sk,
 				   __be16 loc_port, const struct in6_addr *loc_addr,
 				   __be16 rmt_port, const struct in6_addr *rmt_addr,

Signature · 1 line

2.17.2

Paolo Abeni

author maintainer

2018-10-31 02:19 UTC #14 lore raw

RFC PATCH v3 7/10 selftests: add GRO support to udp bench rx program

In reply to Paolo Abeni

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

rfc v2 -> rfc v3:
- use ETH_MAX_MTU

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 +30 -7

index 727cf67a3f75..8f48d7fb32cf 100644
@@ -31,9 +31,15 @@
 #include <sys/wait.h>
 #include <unistd.h>
 
+#ifndef UDP_GRO
+#define UDP_GRO		104
+#endif
+
 static int  cfg_port		= 8000;
 static bool cfg_tcp;
 static bool cfg_verify;
+static bool cfg_read_all;
+static bool cfg_gro_segment;
 
 static bool interrupted;
 static unsigned long packets, bytes;

@@ -63,6 +69,8 @@ static void do_poll(int fd)
 
 	do {
 		ret = poll(&pfd, 1, 10);
+		if (interrupted)
+			break;
 		if (ret == -1)
 			error(1, errno, "poll");
 		if (ret == 0)

@@ -70,7 +78,7 @@ static void do_poll(int fd)
 		if (pfd.revents != POLLIN)
 			error(1, errno, "poll: 0x%x expected 0x%x\n",
 					pfd.revents, POLLIN);
-	} while (!ret && !interrupted);
+	} while (!ret);
 }
 
 static int do_socket(bool do_tcp)

@@ -102,6 +110,8 @@ static int do_socket(bool do_tcp)
 			error(1, errno, "listen");
 
 		do_poll(accept_fd);
+		if (interrupted)
+			exit(0);
 
 		fd = accept(accept_fd, NULL, NULL);
 		if (fd == -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[ETH_MAX_MTU];
 	int ret, len, budget = 256;
 
-	len = cfg_verify ? sizeof(rbuf) : 0;
+	len = cfg_read_all ? sizeof(rbuf) : 0;
 	while (budget--) {
 		/* MSG_TRUNC will make return value full datagram length */
 		ret = recv(fd, rbuf, len, MSG_TRUNC | MSG_DONTWAIT);

@@ -178,7 +188,7 @@ static void do_flush_udp(int fd)
 			return;
 		if (ret == -1)
 			error(1, errno, "recv");
-		if (len) {
+		if (len && cfg_verify) {
 			if (ret == 0)
 				error(1, errno, "recv: 0 byte datagram\n");

@@ -192,23 +202,30 @@ static void do_flush_udp(int fd)
 
 static void usage(const char *filepath)
 {
-	error(1, 0, "Usage: %s [-tv] [-p port]", filepath);
+	error(1, 0, "Usage: %s [-Grtv] [-p port]", filepath);
 }
 
 static void parse_opts(int argc, char **argv)
 {
 	int c;
 
-	while ((c = getopt(argc, argv, "ptv")) != -1) {
+	while ((c = getopt(argc, argv, "Gp:rtv")) != -1) {
 		switch (c) {
+		case 'G':
+			cfg_gro_segment = true;
+			break;
 		case 'p':
-			cfg_port = htons(strtoul(optarg, NULL, 0));
+			cfg_port = strtoul(optarg, NULL, 0);
+			break;
+		case 'r':
+			cfg_read_all = true;
 			break;
 		case 't':
 			cfg_tcp = true;
 			break;
 		case 'v':
 			cfg_verify = true;
+			cfg_read_all = true;
 			break;
 		}
 	}

@@ -227,6 +244,12 @@ static void do_recv(void)
 
 	fd = do_socket(cfg_tcp);
 
+	if (cfg_gro_segment && !cfg_tcp) {
+		int val = 1;
+		if (setsockopt(fd, IPPROTO_UDP, UDP_GRO, &val, sizeof(val)))
+			error(1, errno, "setsockopt UDP_GRO");
+	}
+
 	treport = gettimeofday_ms() + 1000;
 	do {
 		do_poll(fd);

Signature · 1 line

2.17.2

Paolo Abeni

author maintainer

2018-10-31 02:19 UTC #15 lore raw

RFC PATCH v3 8/10 selftests: conditionally enable XDP support in udpgso_bench_rx

In reply to 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.

rfc v2 -> rfc v3:
- move 'x' option handling here

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

---
 tools/testing/selftests/net/Makefile          | 69 +++++++++++++++++++
 tools/testing/selftests/net/udpgso_bench_rx.c | 41 ++++++++++-
 tools/testing/selftests/net/xdp_dummy.c       | 13 ++++
 3 files changed, 121 insertions(+), 2 deletions(-)
 create mode 100644 tools/testing/selftests/net/xdp_dummy.c

tools/testing/selftests/net/Makefile +69 -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
+
+$(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 $@)
+
+$(OUTPUT)/udpgso_bench_rx: $(OUTPUT)/udpgso_bench_rx.c $(LIBBPF)
+	$(CC) -o $@ $(XDP_CFLAGS) $(CFLAGS) $(LOADLIBES) $(LDLIBS) $^ -lelf
+
+FORCE:
+
+# bpf program[s] generation
+$(OUTPUT)/%.o: %.c
+	$(CLANG) $(CLANG_FLAGS) \
+		 -O2 -target bpf -emit-llvm -c $< -o - |      \
+	$(LLC) -march=bpf -mcpu=$(CPU) $(LLC_FLAGS) -filetype=obj -o $@
+ifeq ($(DWARF2BTF),y)
+	$(BTF_PAHOLE) -J $@
+endif
+
+endif
+
 $(OUTPUT)/reuseport_bpf_numa: LDFLAGS += -lnuma
 $(OUTPUT)/tcp_mmap: LDFLAGS += -lpthread
 $(OUTPUT)/tcp_inq: LDFLAGS += -lpthread

tools/testing/selftests/net/udpgso_bench_rx.c +39 -2

89 lines not on this page · raw message

tools/testing/selftests/net/xdp_dummy.c new file +13 -0

new file mode 100644
index 000000000000..1a64cf5099ed
@@ -0,0 +1,13 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#define KBUILD_MODNAME "xdp_dummy"
+#include <uapi/linux/bpf.h>
+#include "bpf_helpers.h"
+
+SEC("xdp_dummy")
+int xdp_dummy_prog(struct xdp_md *ctx)
+{
+	return XDP_PASS;
+}
+
+char _license[] SEC("license") = "GPL";

Signature · 1 line

2.17.2

Paolo Abeni

author maintainer

2018-10-31 02:19 UTC #17 lore raw

RFC PATCH v3 9/10 selftests: add some benchmark for UDP GRO

In reply to Paolo Abeni

Run on top of veth pair, using a dummy XDP program to enable the GRO.

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

---
 tools/testing/selftests/net/Makefile        |  1 +
 tools/testing/selftests/net/udpgro_bench.sh | 92 +++++++++++++++++++++
 2 files changed, 93 insertions(+)
 create mode 100755 tools/testing/selftests/net/udpgro_bench.sh

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

index 176459b7c4d6..ac999354af54 100644
@@ -7,6 +7,7 @@ CFLAGS += -I../../../../usr/include/
 TEST_PROGS := run_netsocktests run_afpackettests test_bpf.sh netdevice.sh rtnetlink.sh
 TEST_PROGS += fib_tests.sh fib-onlink-tests.sh pmtu.sh udpgso.sh ip_defrag.sh
 TEST_PROGS += udpgso_bench.sh fib_rule_tests.sh msg_zerocopy.sh psock_snd.sh
+TEST_PROGS += udpgro_bench.sh
 TEST_PROGS_EXTENDED := in_netns.sh
 TEST_GEN_FILES =  socket
 TEST_GEN_FILES += psock_fanout psock_tpacket msg_zerocopy

tools/testing/selftests/net/udpgro_bench.sh new file +92 -0

92 lines not on this page · raw message

Signature · 1 line

2.17.2

Paolo Abeni

author maintainer

2018-10-31 02:19 UTC #18 lore raw

RFC PATCH v3 10/10 selftests: add functionals test for UDP GRO

In reply to 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.

rfc v2 -> rfc v3:
- add missing test program options documentation
- fix sporatic test failures (receiver faster than sender)

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

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

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

index ac999354af54..a8a0d256aafb 100644
@@ -7,7 +7,7 @@ CFLAGS += -I../../../../usr/include/
 TEST_PROGS := run_netsocktests run_afpackettests test_bpf.sh netdevice.sh rtnetlink.sh
 TEST_PROGS += fib_tests.sh fib-onlink-tests.sh pmtu.sh udpgso.sh ip_defrag.sh
 TEST_PROGS += udpgso_bench.sh fib_rule_tests.sh msg_zerocopy.sh psock_snd.sh
-TEST_PROGS += udpgro_bench.sh
+TEST_PROGS += udpgro_bench.sh udpgro.sh
 TEST_PROGS_EXTENDED := in_netns.sh
 TEST_GEN_FILES =  socket
 TEST_GEN_FILES += psock_fanout psock_tpacket msg_zerocopy

tools/testing/selftests/net/udpgro.sh new file +147 -0

147 lines not on this page · raw message

tools/testing/selftests/net/udpgro_bench.sh +5 -3

index 03d37e5e7424..77a1fb0ae0bc 100755
@@ -18,7 +18,9 @@ run_one() {
 	# use 'rx' as separator between sender args and receiver args
 	local -r all="$@"
 	local -r tx_args=${all%rx*}
-	local -r rx_args=${all#*rx}
+	local rx_args=${all#*rx}
+
+	[[ "${tx_args}" == *"-4"* ]] && rx_args="${rx_args} -4"
 
 	ip netns add "${PEER_NS}"
 	ip -netns "${PEER_NS}" link set lo up

@@ -50,10 +52,10 @@ run_udp() {
 	local -r args=$@
 
 	echo "udp gso - over veth touching data"
-	run_in_netns ${args} -S rx
+	run_in_netns ${args} -S 0 rx
 
 	echo "udp gso and gro - over veth touching data"
-	run_in_netns ${args} -S rx -G
+	run_in_netns ${args} -S 0 rx -G
 }
 
 run_tcp() {

tools/testing/selftests/net/udpgso_bench.sh +1 -1

index 99e537ab5ad9..0f0628613f81 100755
@@ -34,7 +34,7 @@ run_udp() {
 	run_in_netns ${args}
 
 	echo "udp gso"
-	run_in_netns ${args} -S
+	run_in_netns ${args} -S 0
 }
 
 run_tcp() {

tools/testing/selftests/net/udpgso_bench_rx.c +111 ~~-12~~

214 lines not on this page · raw message

tools/testing/selftests/net/udpgso_bench_tx.c +16 -6

index e821564053cf..2b24de666750 100644
@@ -52,6 +52,8 @@ static bool	cfg_segment;
 static bool	cfg_sendmmsg;
 static bool	cfg_tcp;
 static bool	cfg_zerocopy;
+static int	cfg_msg_nr;
+static uint16_t	cfg_gso_size;
 
 static socklen_t cfg_alen;
 static struct sockaddr_storage cfg_dst_addr;

@@ -205,14 +207,14 @@ static void send_udp_segment_cmsg(struct cmsghdr *cm)
 
 	cm->cmsg_level = SOL_UDP;
 	cm->cmsg_type = UDP_SEGMENT;
-	cm->cmsg_len = CMSG_LEN(sizeof(cfg_mss));
+	cm->cmsg_len = CMSG_LEN(sizeof(cfg_gso_size));
 	valp = (void *)CMSG_DATA(cm);
-	*valp = cfg_mss;
+	*valp = cfg_gso_size;
 }
 
 static int send_udp_segment(int fd, char *data)
 {
-	char control[CMSG_SPACE(sizeof(cfg_mss))] = {0};
+	char control[CMSG_SPACE(sizeof(cfg_gso_size))] = {0};
 	struct msghdr msg = {0};
 	struct iovec iov = {0};
 	int ret;

@@ -241,7 +243,7 @@ static int send_udp_segment(int fd, char *data)
 
 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);
 }

@@ -250,7 +252,7 @@ static void parse_opts(int argc, char **argv)
 	int max_len, hdrlen;
 	int c;
 
-	while ((c = getopt(argc, argv, "46cC:D:l:mp:s:Stuz")) != -1) {
+	while ((c = getopt(argc, argv, "46cC:D:l:mM:p:s:S:tuz")) != -1) {
 		switch (c) {
 		case '4':
 			if (cfg_family != PF_UNSPEC)

@@ -279,6 +281,9 @@ static void parse_opts(int argc, char **argv)
 		case 'm':
 			cfg_sendmmsg = true;
 			break;
+		case 'M':
+			cfg_msg_nr = strtoul(optarg, NULL, 10);
+			break;
 		case 'p':
 			cfg_port = strtoul(optarg, NULL, 0);
 			break;

@@ -286,6 +291,7 @@ static void parse_opts(int argc, char **argv)
 			cfg_payload_len = strtoul(optarg, NULL, 0);
 			break;
 		case 'S':
+			cfg_gso_size = strtoul(optarg, NULL, 0);
 			cfg_segment = true;
 			break;
 		case 't':

@@ -317,6 +323,8 @@ static void parse_opts(int argc, char **argv)
 
 	cfg_mss = ETH_DATA_LEN - hdrlen;
 	max_len = ETH_MAX_MTU - hdrlen;
+	if (!cfg_gso_size)
+		cfg_gso_size = cfg_mss;
 
 	if (cfg_payload_len > max_len)
 		error(1, 0, "payload length %u exceeds max %u",

@@ -392,10 +400,12 @@ int main(int argc, char **argv)
 		else
 			num_sends += send_udp(fd, buf[i]);
 		num_msgs++;
-
 		if (cfg_zerocopy && ((num_msgs & 0xF) == 0))
 			flush_zerocopy(fd);
 
+		if (cfg_msg_nr && num_msgs >= cfg_msg_nr)
+			break;
+
 		tnow = gettimeofday_ms();
 		if (tnow > treport) {
 			fprintf(stderr,

Signature · 1 line

2.17.2

Paolo Abeni

author maintainer

2018-10-31 18:51 UTC #11 lore raw

Re: [RFC PATCH v3 06/10] udp: cope with UDP GRO packet misdirection

In reply to Paolo Abeni

Quoted 22 lines · Paolo Abeni

include/net/udp.h +7 -8

@@ -406,17 +406,24 @@ static inline int copy_linear_skb(struct sk_buff *skb, int len, int off,
 } while(0)
 
 #if IS_ENABLED(CONFIG_IPV6)
-#define __UDPX_INC_STATS(sk, field)					\
-do {									\
-	if ((sk)->sk_family == AF_INET)					\
-		__UDP_INC_STATS(sock_net(sk), field, 0);		\
-	else								\
-		__UDP6_INC_STATS(sock_net(sk), field, 0);		\
-} while (0)
+#define __UDPX_MIB(sk, ipv4)						\
+({									\
+	ipv4 ? (IS_UDPLITE(sk) ? sock_net(sk)->mib.udplite_statistics :	\
+				 sock_net(sk)->mib.udp_statistics) :	\
+		(IS_UDPLITE(sk) ? sock_net(sk)->mib.udplite_stats_in6 :	\
+				 sock_net(sk)->mib.udp_stats_in6);	\
+})
 #else
-#define __UDPX_INC_STATS(sk, field) __UDP_INC_STATS(sock_net(sk), field, 0)

include/net/udp.h static inline int copy_linear_skb(struct sk_buff *skb, int len, int off, · line 426

Quoted 10 lines

include/net/udp.h +8 -0

+#define __UDPX_MIB(sk, ipv4)						\
+({									\
+	IS_UDPLITE(sk) ? sock_net(sk)->mib.udplite_statistics :		\
+			 sock_net(sk)->mib.udp_statistics;		\
+})
 #endif
 
+#define __UDPX_INC_STATS(sk, field) \
+	__SNMP_INC_STATS(__UDPX_MIB(sk, (sk)->sk_family == AF_INET, field)
+

This is broken (complains only if CONFIG_AF_RXRPC is set), will fix in next iteration (thanks kbuildbot).

But I'd prefer to keep the above helper: it can be used in a follow-up patch to cleanup a bit udp6_recvmsg().

include/net/udp.h DECLARE_STATIC_KEY_FALSE(udpv6_encap_needed_key); · line 463

Quoted 11 lines

include/net/udp.h +4 -0

 #ifdef CONFIG_PROC_FS
 struct udp_seq_afinfo {
 	sa_family_t			family;

@@ -450,4 +457,32 @@ DECLARE_STATIC_KEY_FALSE(udpv6_encap_needed_key);
 void udpv6_encap_enable(void);
 #endif
 
+static inline struct sk_buff *udp_rcv_segment(struct sock *sk,
+					      struct sk_buff *skb)
+{
+	bool ipv4 = skb->protocol == htons(ETH_P_IP);

And this cause a compile warning when # CONFIG_IPV6 is not set, I will fix in the next iteration (again thanks kbuildbot)

Cheers,

Paolo

Subash Abhinov Kasiviswanathan

2018-11-01 15:37 UTC #7 lore raw

Re: [RFC PATCH v3 04/10] ip: factor out protocol delivery helper

In reply to Paolo Abeni

Quoted 2 lines · Paolo Abeni

So that we can re-use it at the UDP lavel in a later patch

Hi Paolo

Minor queries -
Should it be "level" instead of "lavel"? Similar comment for the ipv6 patch as well.

Quoted 10 lines

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

---
 net/ipv4/ip_input.c | 73 ++++++++++++++++++++++-----------------------
 1 file changed, 36 insertions(+), 37 deletions(-)

net/ipv4/ip_input.c +0 -0

index 35a786c0aaa0..72250b4e466d 100644

net/ipv4/ip_input.c bool ip_call_ra_chain(struct sk_buff *skb) · line 193

Quoted 7 lines

net/ipv4/ip_input.c +1 -1

@@ -188,51 +188,50 @@ bool ip_call_ra_chain(struct sk_buff *skb)
 	return false;
 }
 
-static int ip_local_deliver_finish(struct net *net, struct sock *sk,
 struct sk_buff *skb)
+void ip_protocol_deliver_rcu(struct net *net, struct sk_buff *skb,
 int protocol)

Would it be better if this function was declared in include/net/ip.h & include/net/ipv6.h rather than in net/ipv4/udp.c & net/ipv6/udp.c as in the patch "udp: cope with UDP GRO packet misdirection"

include/net/ip.h +2 -0

index 72593e1..3d7fdb4 100644
@@ -717,4 +717,6 @@ static inline void ip_cmsg_recv(struct msghdr *msg, struct sk_buff *skb)
 int rtm_getroute_parse_ip_proto(struct nlattr *attr, u8 *ip_proto,
                                struct netlink_ext_ack *extack);
 
+void ip_protocol_deliver_rcu(struct net *net, struct sk_buff *skb, int proto);
+
 #endif /* _IP_H */

include/net/ipv6.h +0 -0

index 8296505..4d4d2cfe 100644
@@ -1101,4 +1101,8 @@ int ipv6_sock_mc_join_ssm(struct sock *sk, int ifindex,
                          const struct in6_addr *addr, unsigned int

mode);
int ipv6_sock_mc_drop(struct sock *sk, int ifindex,

                      const struct in6_addr *addr);

+
+void ip6_protocol_deliver_rcu(struct net *net, struct sk_buff *skb, int nexthdr,
+ bool have_final);
+
#endif /* _NET_IPV6_H */

Signature · 2 lines

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

Willem de Bruijn

maintainer

2018-11-02 06:05 UTC #2 lore raw

Re: [RFC PATCH v3 01/10] udp: implement complete book-keeping for encap_needed

In reply to Paolo Abeni

Quoted 18 lines · Paolo Abeni

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.
rfc v2 - rfc v3:
- use udp_tunnel_encap_enable() in setsockopt()
Signed-off-by Paolo Abeni <[email protected]>

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

Quoted 5 lines

net/ipv4/udp.c +1 -1

@@ -2447,7 +2452,7 @@ int udp_lib_setsockopt(struct sock *sk, int level, int optname,
                        /* FALLTHROUGH */
                case UDP_ENCAP_L2TPINUDP:
                        up->encap_type = val;
-                       udp_encap_enable();
+                       udp_tunnel_encap_enable(sk->sk_socket);

this now also needs lock_sock?

Willem de Bruijn

maintainer Tested-by on v3 8/10

2018-11-02 06:05 UTC #16 lore raw

Re: [RFC PATCH v3 08/10] selftests: conditionally enable XDP support in udpgso_bench_rx

In reply to Paolo Abeni

Quoted 86 lines · Paolo Abeni

rfc v2 -> rfc v3:
- move 'x' option handling here

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

---
 tools/testing/selftests/net/Makefile          | 69 +++++++++++++++++++
 tools/testing/selftests/net/udpgso_bench_rx.c | 41 ++++++++++-
 tools/testing/selftests/net/xdp_dummy.c       | 13 ++++
 3 files changed, 121 insertions(+), 2 deletions(-)
 create mode 100644 tools/testing/selftests/net/xdp_dummy.c

tools/testing/selftests/net/Makefile +59 -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
+
+$(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 $@)
+
+$(OUTPUT)/udpgso_bench_rx: $(OUTPUT)/udpgso_bench_rx.c $(LIBBPF)
+       $(CC) -o $@ $(XDP_CFLAGS) $(CFLAGS) $(LOADLIBES) $(LDLIBS) $^ -lelf
+
+FORCE:
+
+# bpf program[s] generation

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

Quoted 10 lines

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

+$(OUTPUT)/%.o: %.c
+       $(CLANG) $(CLANG_FLAGS) \
+                -O2 -target bpf -emit-llvm -c $< -o - |      \
+       $(LLC) -march=bpf -mcpu=$(CPU) $(LLC_FLAGS) -filetype=obj -o $@
+ifeq ($(DWARF2BTF),y)
+       $(BTF_PAHOLE) -J $@
+endif
+
+endif
+

To get around having to add all this Makefile boilerplate, perhaps don't integrate the xdp loader with udpgso_bench_rx, but add a standalone trivial loader to /tools/testing/selftests/bpf. Akin to samples/xdp1_user.c, but even simpler because without map. Then also move xdp_dummy.c there.

That does add a cross-directory dependency, but that is not significantly different from the SUPPORT_XDP conditional dependency today.

On which note, the test initially silently failed for me because the binary compiled, but without that option. It is probably better to fail the test hard and with a clear error if XDP is not supported. With that aside,

Tested-by Willem de Bruijn <[email protected]>

Willem de Bruijn

maintainer

2018-11-02 06:06 UTC #12 lore raw

Re: [RFC PATCH v3 06/10] udp: cope with UDP GRO packet misdirection

In reply to Paolo Abeni

Quoted 48 lines · Paolo Abeni

Quoted 22 lines · Paolo Abeni

include/net/udp.h +7 -8

@@ -406,17 +406,24 @@ static inline int copy_linear_skb(struct sk_buff *skb, int len, int off,
 } while(0)
 
 #if IS_ENABLED(CONFIG_IPV6)
-#define __UDPX_INC_STATS(sk, field)                                  \
-do {                                                                 \
-     if ((sk)->sk_family == AF_INET)                                 \
-             __UDP_INC_STATS(sock_net(sk), field, 0);                \
-     else                                                            \
-             __UDP6_INC_STATS(sock_net(sk), field, 0);               \
-} while (0)
+#define __UDPX_MIB(sk, ipv4)                                         \
+({                                                                   \
+     ipv4 ? (IS_UDPLITE(sk) ? sock_net(sk)->mib.udplite_statistics : \
+                              sock_net(sk)->mib.udp_statistics) :    \
+             (IS_UDPLITE(sk) ? sock_net(sk)->mib.udplite_stats_in6 : \
+                              sock_net(sk)->mib.udp_stats_in6);      \
+})
 #else
-#define __UDPX_INC_STATS(sk, field) __UDP_INC_STATS(sock_net(sk), field, 0)

include/net/udp.h static inline int copy_linear_skb(struct sk_buff *skb, int len, int off, · line 426

Quoted 10 lines

include/net/udp.h +8 -0

+#define __UDPX_MIB(sk, ipv4)                                         \
+({                                                                   \
+     IS_UDPLITE(sk) ? sock_net(sk)->mib.udplite_statistics :         \
+                      sock_net(sk)->mib.udp_statistics;              \
+})
 #endif
 
+#define __UDPX_INC_STATS(sk, field) \
+     __SNMP_INC_STATS(__UDPX_MIB(sk, (sk)->sk_family == AF_INET, field)
+

This is broken (complains only if CONFIG_AF_RXRPC is set), will fix in next iteration (thanks kbuildbot).

But I'd prefer to keep the above helper: it can be used in a follow-up patch to cleanup a bit udp6_recvmsg().

Quoted 7 lines

include/net/udp.h +0 -0

 #ifdef CONFIG_PROC_FS
 struct udp_seq_afinfo {
      sa_family_t                     family;

@@ -450,4 +457,32 @@ DECLARE_STATIC_KEY_FALSE(udpv6_encap_needed_key);
 void udpv6_encap_enable(void);
 #endif

Quoted 7 lines

include/net/udp.h DECLARE_STATIC_KEY_FALSE(udpv6_encap_needed_key); · line 463
Quoted 4 lines
include/net/udp.h +4 -0
+static inline struct sk_buff *udp_rcv_segment(struct sock *sk,
+                                           struct sk_buff *skb)
+{
+     bool ipv4 = skb->protocol == htons(ETH_P_IP);
And this cause a compile warning when # CONFIG_IPV6 is not set, I will fix in the next iteration (again thanks kbuildbot)

Can also just pass it as argument. This skb->protocol should work correctly with tunneled packets, but it wasn't as immediately obvious to me.

Also

+ if (unlikely(!segs))
+ goto drop;

this should not happen. But if it could and the caller treats it the same as error (both now return NULL), then skb needs to be freed.

Paolo Abeni

author maintainer

2018-11-02 22:37 UTC #8 lore raw

Re: [RFC PATCH v3 04/10] ip: factor out protocol delivery helper

In reply to Subash Abhinov Kasiviswanathan

Quoted 9 lines · Subash Abhinov Kasiviswanathan

Quoted 2 lines · Paolo Abeni
So that we can re-use it at the UDP lavel in a later patch
Hi Paolo
Minor queries -
Should it be "level" instead of "lavel"? Similar comment for the ipv6 patch as well.

Indeed. Will fix in the next iteration.

Quoted 44 lines

net/ipv4/ip_input.c bool ip_call_ra_chain(struct sk_buff *skb) · line 193

Quoted 12 lines

net/ipv4/ip_input.c +1 -1

index 35a786c0aaa0..72250b4e466d 100644
@@ -188,51 +188,50 @@ bool ip_call_ra_chain(struct sk_buff *skb)
 	return false;
 }
 
-static int ip_local_deliver_finish(struct net *net, struct sock *sk,
 struct sk_buff *skb)
+void ip_protocol_deliver_rcu(struct net *net, struct sk_buff *skb,
 int protocol)

include/net/ip.h +1 -0

index 72593e1..3d7fdb4 100644
@@ -717,4 +717,6 @@ static inline void ip_cmsg_recv(struct msghdr *msg, 
 struct sk_buff *skb)
  int rtm_getroute_parse_ip_proto(struct nlattr *attr, u8 *ip_proto,
                                 struct netlink_ext_ack *extack);
 
+void ip_protocol_deliver_rcu(struct net *net, struct sk_buff *skb, int

proto);

include/net/ip.h +1 -0

+
  #endif /* _IP_H */

include/net/ipv6.h +2 -0

index 8296505..4d4d2cfe 100644
@@ -1101,4 +1101,8 @@ int ipv6_sock_mc_join_ssm(struct sock *sk, int 
 ifindex,
                           const struct in6_addr *addr, unsigned int 
 mode);
  int ipv6_sock_mc_drop(struct sock *sk, int ifindex,
                       const struct in6_addr *addr);
+
+void ip6_protocol_deliver_rcu(struct net *net, struct sk_buff *skb, int

nexthdr,

include/net/ipv6.h

Quoted 3 lines

include/net/ipv6.h +2 -0

+                              bool have_final);
+
  #endif /* _NET_IPV6_H */

Agreed, I will do in the next iteration.

Thanks,

Paolo

Paolo Abeni

author maintainer

2018-11-02 22:38 UTC #3 lore raw

Re: [RFC PATCH v3 01/10] udp: implement complete book-keeping for encap_needed

In reply to Willem de Bruijn

Hi,

Quoted 16 lines · Willem de Bruijn

Quoted 15 lines · Paolo Abeni
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.
rfc v2 - rfc v3:
- use udp_tunnel_encap_enable() in setsockopt()

Quoted 9 lines

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

Quoted 7 lines

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

net/ipv4/udp.c +1 -1

@@ -2447,7 +2452,7 @@ int udp_lib_setsockopt(struct sock *sk, int level, int optname,
                        /* FALLTHROUGH */
                case UDP_ENCAP_L2TPINUDP:
                        up->encap_type = val;
-                       udp_encap_enable();
+                       udp_tunnel_encap_enable(sk->sk_socket);

this now also needs lock_sock?

Yep, you are right. I'll add it in the next iteration.

Thanks,

Paolo

Paolo Abeni

author maintainer

2018-11-02 22:52 UTC #13 lore raw

Re: [RFC PATCH v3 06/10] udp: cope with UDP GRO packet misdirection

In reply to Willem de Bruijn

Quoted 13 lines · Willem de Bruijn

Quoted 11 lines · Paolo Abeni
(quoted diff) +0 -0
@@ -450,4 +457,32 @@ DECLARE_STATIC_KEY_FALSE(udpv6_encap_needed_key);
 >  void udpv6_encap_enable(void);
include/net/udp.h DECLARE_STATIC_KEY_FALSE(udpv6_encap_needed_key); · line 463
Quoted 6 lines
#endif
include/net/udp.h +4 -0
+static inline struct sk_buff *udp_rcv_segment(struct sock *sk,
+                                           struct sk_buff *skb)
+{
+     bool ipv4 = skb->protocol == htons(ETH_P_IP);
And this cause a compile warning when # CONFIG_IPV6 is not set, I will fix in the next iteration (again thanks kbuildbot)

Quoted 1 line

Can also just pass it as argument.

Agreed.

Quoted 10 lines

This skb->protocol should work correctly
with tunneled packets, but it wasn't as immediately obvious to me.
Also
include/net/udp.h +2 -0
+       if (unlikely(!segs))
+               goto drop;
this should not happen. But if it could and the caller treats it the same as error (both now return NULL), then skb needs to be freed.

Right you are. Will do in the next iteration.

Thanks,

Paolo