[PATCH net-next v4 0/3] net: A lightweight zero-copy notification

[PATCH net-next v4 0/3] net: A lightweight zero-copy notification

[zijianzhang]

From: Zijian Zhang <zijianzhang@bytedance.com>

Original title is "net: socket sendmsg MSG_ZEROCOPY_UARG". 

Original notification mechanism needs poll + recvmmsg which is not
easy for applcations to accommodate. And, it also incurs unignorable
overhead including extra system calls and usage of socket optmem.

While making maximum reuse of the existing MSG_ZEROCOPY related code,
this patch set introduces a new zerocopy socket notification mechanism.
Users of sendmsg pass a control message as a placeholder for the incoming
notifications. Upon returning, kernel embeds notifications directly into
user arguments passed in. By doing so, we can significantly reduce the
complexity and overhead for managing notifications. In an ideal pattern,
the user will keep calling sendmsg with SCM_ZC_NOTIFICATION msg_control,
and the notification will be delivered as soon as possible.

Users need to pass in a user space address pointing to an array of struct
zc_info_elem, and the cmsg_len should be the memory size of the array
instead of the size of the pointer itself.

As Willem commented,

> The main design issue with this series is this indirection, rather
> than passing the array of notifications as cmsg.

> This trick circumvents having to deal with compat issues and having to
> figure out copy_to_user in ____sys_sendmsg (as msg_control is an
> in-kernel copy).

> This is quite hacky, from an API design PoV.

> As is passing a pointer, but expecting msg_controllen to hold the
> length not of the pointer, but of the pointed to user buffer.

> I had also hoped for more significant savings. Especially with the
> higher syscall overhead due to meltdown and spectre mitigations vs
> when MSG_ZEROCOPY was introduced and I last tried this optimization.

Changelog:
  v1 -> v2:
    - Reuse errormsg queue in the new notification mechanism,
      users can actually use these two mechanisms in hybrid way
      if they want to do so.
    - Update case SCM_ZC_NOTIFICATION in __sock_cmsg_send
      1. Regardless of 32-bit, 64-bit program, we will always handle
      u64 type user address.
      2. The size of data to copy_to_user is precisely calculated
      in case of kernel stack leak.
    - fix (kbuild-bot)
      1. Add SCM_ZC_NOTIFICATION to arch-specific header files.
      2. header file types.h in include/uapi/linux/socket.h

  v2 -> v3:
    - 1. Users can now pass in the address of the zc_info_elem directly
      with appropriate cmsg_len instead of the ugly user interface. Plus,
      the handler is now compatible with MSG_CMSG_COMPAT and 32-bit
      pointer.
    - 2. Suggested by Willem, another strategy of getting zc info is
      briefly taking the lock of sk_error_queue and move to a private
      list, like net_rx_action. I thought sk_error_queue is protected by
      sock_lock, so that it's impossible for the handling of zc info and
      users recvmsg from the sk_error_queue at the same time.
      However, sk_error_queue is protected by its own lock. I am afraid
      that during the time it is handling the private list, users may
      fail to get other error messages in the queue via recvmsg. Thus,
      I don't implement the splice logic in this version. Any comments?

  v3 -> v4:
    - 1. Change SOCK_ZC_INFO_MAX to 64 to avoid large stack frame size.
    - 2. Fix minor typos.
    - 3. Change cfg_zerocopy from int to enum in msg_zerocopy.c

* Performance

I extend the selftests/msg_zerocopy.c to accommodate the new mechanism,
test result is as follows,

cfg_notification_limit = 1, in this case the original method approximately
aligns with the semantics of new one. In this case, the new flag has
around 13% cpu savings in TCP and 18% cpu savings in UDP.

+---------------------+---------+---------+---------+---------+
| Test Type / Protocol| TCP v4  | TCP v6  | UDP v4  | UDP v6  |
+---------------------+---------+---------+---------+---------+
| ZCopy (MB)          | 5147    | 4885    | 7489    | 7854    |
+---------------------+---------+---------+---------+---------+
| New ZCopy (MB)      | 5859    | 5505    | 9053    | 9236    |
+---------------------+---------+---------+---------+---------+
| New ZCopy / ZCopy   | 113.83% | 112.69% | 120.88% | 117.59% |
+---------------------+---------+---------+---------+---------+


cfg_notification_limit = 32, the new mechanism performs 8% better in TCP.
For UDP, no obvious performance gain is observed and sometimes may lead
to degradation. Thus, if users don't need to retrieve the notification
ASAP in UDP, the original mechanism is preferred.

+---------------------+---------+---------+---------+---------+
| Test Type / Protocol| TCP v4  | TCP v6  | UDP v4  | UDP v6  |
+---------------------+---------+---------+---------+---------+
| ZCopy (MB)          | 6272    | 6138    | 12138   | 10055   |
+---------------------+---------+---------+---------+---------+
| New ZCopy (MB)      | 6774    | 6620    | 11504   | 10355   |
+---------------------+---------+---------+---------+---------+
| New ZCopy / ZCopy   | 108.00% | 107.85% | 94.78%  | 102.98% |
+---------------------+---------+---------+---------+---------+

Zijian Zhang (3):
  selftests: fix OOM problem in msg_zerocopy selftest
  sock: add MSG_ZEROCOPY notification mechanism based on msg_control
  selftests: add MSG_ZEROCOPY msg_control notification test

 arch/alpha/include/uapi/asm/socket.h        |   2 +
 arch/mips/include/uapi/asm/socket.h         |   2 +
 arch/parisc/include/uapi/asm/socket.h       |   2 +
 arch/sparc/include/uapi/asm/socket.h        |   2 +
 include/uapi/asm-generic/socket.h           |   2 +
 include/uapi/linux/socket.h                 |  10 ++
 net/core/sock.c                             |  68 ++++++++++++
 tools/testing/selftests/net/msg_zerocopy.c  | 114 ++++++++++++++++++--
 tools/testing/selftests/net/msg_zerocopy.sh |   1 +
 9 files changed, 196 insertions(+), 7 deletions(-)

-- 
2.20.1

[PATCH net-next v4 1/3] selftests: fix OOM problem in msg_zerocopy selftest

[zijianzhang]

From: Zijian Zhang <zijianzhang@bytedance.com>

In selftests/net/msg_zerocopy.c, it has a while loop keeps calling sendmsg
on a socket with MSG_ZEROCOPY flag, and it will recv the notifications
until the socket is not writable. Typically, it will start the receiving
process after around 30+ sendmsgs. However, because of the
commit dfa2f0483360 ("tcp: get rid of sysctl_tcp_adv_win_scale")
the sender is always writable and does not get any chance to run recv
notifications. The selftest always exits with OUT_OF_MEMORY because the
memory used by opt_skb exceeds the core.sysctl_optmem_max.

According to our experiments, this problem can be mitigated by open the
DEBUG_LOCKDEP configuration for the kernel. But it will makes the
notifications disordered even in good commits before
commit dfa2f0483360 ("tcp: get rid of sysctl_tcp_adv_win_scale").

We introduce "cfg_notification_limit" to force sender to receive
notifications after some number of sendmsgs. And, notifications may not
come in order, because of the reason we present above. We have order
checking code managed by cfg_verbose.

Signed-off-by: Zijian Zhang <zijianzhang@bytedance.com>
Signed-off-by: Xiaochun Lu <xiaochun.lu@bytedance.com>
---
 tools/testing/selftests/net/msg_zerocopy.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/net/msg_zerocopy.c b/tools/testing/selftests/net/msg_zerocopy.c
index bdc03a2097e8..ba6c257f689c 100644
--- a/tools/testing/selftests/net/msg_zerocopy.c
+++ b/tools/testing/selftests/net/msg_zerocopy.c
@@ -85,6 +85,7 @@ static bool cfg_rx;
 static int  cfg_runtime_ms	= 4200;
 static int  cfg_verbose;
 static int  cfg_waittime_ms	= 500;
+static int  cfg_notification_limit = 32;
 static bool cfg_zerocopy;
 
 static socklen_t cfg_alen;
@@ -95,6 +96,8 @@ static char payload[IP_MAXPACKET];
 static long packets, bytes, completions, expected_completions;
 static int  zerocopied = -1;
 static uint32_t next_completion;
+/* The number of sendmsgs which have not received notified yet */
+static uint32_t sendmsg_counter;
 
 static unsigned long gettimeofday_ms(void)
 {
@@ -208,6 +211,7 @@ static bool do_sendmsg(int fd, struct msghdr *msg, bool do_zerocopy, int domain)
 		error(1, errno, "send");
 	if (cfg_verbose && ret != len)
 		fprintf(stderr, "send: ret=%u != %u\n", ret, len);
+	sendmsg_counter++;
 
 	if (len) {
 		packets++;
@@ -435,7 +439,7 @@ static bool do_recv_completion(int fd, int domain)
 	/* Detect notification gaps. These should not happen often, if at all.
 	 * Gaps can occur due to drops, reordering and retransmissions.
 	 */
-	if (lo != next_completion)
+	if (cfg_verbose && lo != next_completion)
 		fprintf(stderr, "gap: %u..%u does not append to %u\n",
 			lo, hi, next_completion);
 	next_completion = hi + 1;
@@ -460,6 +464,7 @@ static bool do_recv_completion(int fd, int domain)
 static void do_recv_completions(int fd, int domain)
 {
 	while (do_recv_completion(fd, domain)) {}
+	sendmsg_counter = 0;
 }
 
 /* Wait for all remaining completions on the errqueue */
@@ -549,6 +554,9 @@ static void do_tx(int domain, int type, int protocol)
 		else
 			do_sendmsg(fd, &msg, cfg_zerocopy, domain);
 
+		if (cfg_zerocopy && sendmsg_counter >= cfg_notification_limit)
+			do_recv_completions(fd, domain);
+
 		while (!do_poll(fd, POLLOUT)) {
 			if (cfg_zerocopy)
 				do_recv_completions(fd, domain);
@@ -708,7 +716,7 @@ static void parse_opts(int argc, char **argv)
 
 	cfg_payload_len = max_payload_len;
 
-	while ((c = getopt(argc, argv, "46c:C:D:i:mp:rs:S:t:vz")) != -1) {
+	while ((c = getopt(argc, argv, "46c:C:D:i:l:mp:rs:S:t:vz")) != -1) {
 		switch (c) {
 		case '4':
 			if (cfg_family != PF_UNSPEC)
@@ -736,6 +744,9 @@ static void parse_opts(int argc, char **argv)
 			if (cfg_ifindex == 0)
 				error(1, errno, "invalid iface: %s", optarg);
 			break;
+		case 'l':
+			cfg_notification_limit = strtoul(optarg, NULL, 0);
+			break;
 		case 'm':
 			cfg_cork_mixed = true;
 			break;
-- 
2.20.1

[PATCH net-next v4 2/3] sock: add MSG_ZEROCOPY notification mechanism based on msg_control

[zijianzhang]

From: Zijian Zhang <zijianzhang@bytedance.com>

The MSG_ZEROCOPY flag enables copy avoidance for socket send calls.
However, zerocopy is not a free lunch. Apart from the management of user
pages, the combination of poll + recvmsg to receive notifications incurs
unignorable overhead in the applications. The overhead of such sometimes
might be more than the CPU savings from zerocopy. We try to solve this
problem with a new notification mechanism based on msgcontrol.
This new mechanism aims to reduce the overhead associated with receiving
notifications by embedding them directly into user arguments passed with
each sendmsg control message. By doing so, we can significantly reduce
the complexity and overhead for managing notifications. In an ideal
pattern, the user will keep calling sendmsg with SCM_ZC_NOTIFICATION
msg_control, and the notification will be delivered as soon as possible.

Signed-off-by: Zijian Zhang <zijianzhang@bytedance.com>
Signed-off-by: Xiaochun Lu <xiaochun.lu@bytedance.com>
---
 arch/alpha/include/uapi/asm/socket.h  |  2 +
 arch/mips/include/uapi/asm/socket.h   |  2 +
 arch/parisc/include/uapi/asm/socket.h |  2 +
 arch/sparc/include/uapi/asm/socket.h  |  2 +
 include/uapi/asm-generic/socket.h     |  2 +
 include/uapi/linux/socket.h           | 10 ++++
 net/core/sock.c                       | 68 +++++++++++++++++++++++++++
 7 files changed, 88 insertions(+)

diff --git a/arch/alpha/include/uapi/asm/socket.h b/arch/alpha/include/uapi/asm/socket.h
index e94f621903fe..7761a4e0ea2c 100644
--- a/arch/alpha/include/uapi/asm/socket.h
+++ b/arch/alpha/include/uapi/asm/socket.h
@@ -140,6 +140,8 @@
 #define SO_PASSPIDFD		76
 #define SO_PEERPIDFD		77
 
+#define SCM_ZC_NOTIFICATION 78
+
 #if !defined(__KERNEL__)
 
 #if __BITS_PER_LONG == 64
diff --git a/arch/mips/include/uapi/asm/socket.h b/arch/mips/include/uapi/asm/socket.h
index 60ebaed28a4c..89edc51380f0 100644
--- a/arch/mips/include/uapi/asm/socket.h
+++ b/arch/mips/include/uapi/asm/socket.h
@@ -151,6 +151,8 @@
 #define SO_PASSPIDFD		76
 #define SO_PEERPIDFD		77
 
+#define SCM_ZC_NOTIFICATION 78
+
 #if !defined(__KERNEL__)
 
 #if __BITS_PER_LONG == 64
diff --git a/arch/parisc/include/uapi/asm/socket.h b/arch/parisc/include/uapi/asm/socket.h
index be264c2b1a11..2911b43e6a9d 100644
--- a/arch/parisc/include/uapi/asm/socket.h
+++ b/arch/parisc/include/uapi/asm/socket.h
@@ -132,6 +132,8 @@
 #define SO_PASSPIDFD		0x404A
 #define SO_PEERPIDFD		0x404B
 
+#define SCM_ZC_NOTIFICATION 0x404C
+
 #if !defined(__KERNEL__)
 
 #if __BITS_PER_LONG == 64
diff --git a/arch/sparc/include/uapi/asm/socket.h b/arch/sparc/include/uapi/asm/socket.h
index 682da3714686..dc045e87cc8e 100644
--- a/arch/sparc/include/uapi/asm/socket.h
+++ b/arch/sparc/include/uapi/asm/socket.h
@@ -133,6 +133,8 @@
 #define SO_PASSPIDFD             0x0055
 #define SO_PEERPIDFD             0x0056
 
+#define SCM_ZC_NOTIFICATION 0x0057
+
 #if !defined(__KERNEL__)
 
 
diff --git a/include/uapi/asm-generic/socket.h b/include/uapi/asm-generic/socket.h
index 8ce8a39a1e5f..7474c8a244bc 100644
--- a/include/uapi/asm-generic/socket.h
+++ b/include/uapi/asm-generic/socket.h
@@ -135,6 +135,8 @@
 #define SO_PASSPIDFD		76
 #define SO_PEERPIDFD		77
 
+#define SCM_ZC_NOTIFICATION 78
+
 #if !defined(__KERNEL__)
 
 #if __BITS_PER_LONG == 64 || (defined(__x86_64__) && defined(__ILP32__))
diff --git a/include/uapi/linux/socket.h b/include/uapi/linux/socket.h
index d3fcd3b5ec53..15cec8819f34 100644
--- a/include/uapi/linux/socket.h
+++ b/include/uapi/linux/socket.h
@@ -2,6 +2,8 @@
 #ifndef _UAPI_LINUX_SOCKET_H
 #define _UAPI_LINUX_SOCKET_H
 
+#include <linux/types.h>
+
 /*
  * Desired design of maximum size and alignment (see RFC2553)
  */
@@ -35,4 +37,12 @@ struct __kernel_sockaddr_storage {
 #define SOCK_TXREHASH_DISABLED	0
 #define SOCK_TXREHASH_ENABLED	1
 
+#define SOCK_ZC_INFO_MAX 128
+
+struct zc_info_elem {
+	__u32 lo;
+	__u32 hi;
+	__u8 zerocopy;
+};
+
 #endif /* _UAPI_LINUX_SOCKET_H */
diff --git a/net/core/sock.c b/net/core/sock.c
index 521e6373d4f7..21239469d75c 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -2847,6 +2847,74 @@ int __sock_cmsg_send(struct sock *sk, struct cmsghdr *cmsg,
 	case SCM_RIGHTS:
 	case SCM_CREDENTIALS:
 		break;
+	case SCM_ZC_NOTIFICATION: {
+		int ret, i = 0;
+		int cmsg_data_len, zc_info_elem_num;
+		void __user	*usr_addr;
+		struct zc_info_elem zc_info_kern[SOCK_ZC_INFO_MAX];
+		unsigned long flags;
+		struct sk_buff_head *q, local_q;
+		struct sk_buff *skb, *tmp;
+		struct sock_exterr_skb *serr;
+
+		if (!sock_flag(sk, SOCK_ZEROCOPY) || sk->sk_family == PF_RDS)
+			return -EINVAL;
+
+		cmsg_data_len = cmsg->cmsg_len - sizeof(struct cmsghdr);
+		if (cmsg_data_len % sizeof(struct zc_info_elem))
+			return -EINVAL;
+
+		zc_info_elem_num = cmsg_data_len / sizeof(struct zc_info_elem);
+		if (!zc_info_elem_num || zc_info_elem_num > SOCK_ZC_INFO_MAX)
+			return -EINVAL;
+
+		if (in_compat_syscall())
+			usr_addr = compat_ptr(*(compat_uptr_t *)CMSG_DATA(cmsg));
+		else
+			usr_addr = (void __user *)*(void **)CMSG_DATA(cmsg);
+		if (!access_ok(usr_addr, cmsg_data_len))
+			return -EFAULT;
+
+		q = &sk->sk_error_queue;
+		skb_queue_head_init(&local_q);
+		spin_lock_irqsave(&q->lock, flags);
+		skb = skb_peek(q);
+		while (skb && i < zc_info_elem_num) {
+			struct sk_buff *skb_next = skb_peek_next(skb, q);
+
+			serr = SKB_EXT_ERR(skb);
+			if (serr->ee.ee_errno == 0 &&
+			    serr->ee.ee_origin == SO_EE_ORIGIN_ZEROCOPY) {
+				zc_info_kern[i].hi = serr->ee.ee_data;
+				zc_info_kern[i].lo = serr->ee.ee_info;
+				zc_info_kern[i].zerocopy = !(serr->ee.ee_code
+								& SO_EE_CODE_ZEROCOPY_COPIED);
+				__skb_unlink(skb, q);
+				__skb_queue_tail(&local_q, skb);
+				i++;
+			}
+			skb = skb_next;
+		}
+		spin_unlock_irqrestore(&q->lock, flags);
+
+		ret = copy_to_user(usr_addr,
+				   zc_info_kern,
+					i * sizeof(struct zc_info_elem));
+
+		if (unlikely(ret)) {
+			spin_lock_irqsave(&q->lock, flags);
+			skb_queue_reverse_walk_safe(&local_q, skb, tmp) {
+				__skb_unlink(skb, &local_q);
+				__skb_queue_head(q, skb);
+			}
+			spin_unlock_irqrestore(&q->lock, flags);
+			return -EFAULT;
+		}
+
+		while ((skb = __skb_dequeue(&local_q)))
+			consume_skb(skb);
+		break;
+	}
 	default:
 		return -EINVAL;
 	}
-- 
2.20.1

Re: [PATCH net-next v4 2/3] sock: add MSG_ZEROCOPY notification mechanism based on msg_control

[Simon Horman]

On Tue, May 28, 2024 at 09:21:02PM +0000, zijianzhang@bytedance.com wrote:
> From: Zijian Zhang <zijianzhang@bytedance.com>
> 
> The MSG_ZEROCOPY flag enables copy avoidance for socket send calls.
> However, zerocopy is not a free lunch. Apart from the management of user
> pages, the combination of poll + recvmsg to receive notifications incurs
> unignorable overhead in the applications. The overhead of such sometimes
> might be more than the CPU savings from zerocopy. We try to solve this
> problem with a new notification mechanism based on msgcontrol.
> This new mechanism aims to reduce the overhead associated with receiving
> notifications by embedding them directly into user arguments passed with
> each sendmsg control message. By doing so, we can significantly reduce
> the complexity and overhead for managing notifications. In an ideal
> pattern, the user will keep calling sendmsg with SCM_ZC_NOTIFICATION
> msg_control, and the notification will be delivered as soon as possible.
> 
> Signed-off-by: Zijian Zhang <zijianzhang@bytedance.com>
> Signed-off-by: Xiaochun Lu <xiaochun.lu@bytedance.com>

...

> diff --git a/net/core/sock.c b/net/core/sock.c
> index 521e6373d4f7..21239469d75c 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -2847,6 +2847,74 @@ int __sock_cmsg_send(struct sock *sk, struct cmsghdr *cmsg,
>  	case SCM_RIGHTS:
>  	case SCM_CREDENTIALS:
>  		break;
> +	case SCM_ZC_NOTIFICATION: {
> +		int ret, i = 0;
> +		int cmsg_data_len, zc_info_elem_num;
> +		void __user	*usr_addr;
> +		struct zc_info_elem zc_info_kern[SOCK_ZC_INFO_MAX];
> +		unsigned long flags;
> +		struct sk_buff_head *q, local_q;
> +		struct sk_buff *skb, *tmp;
> +		struct sock_exterr_skb *serr;

Hi Zijian Zhang, Xiaochun Lu, all,

When compiling on ARM (32bit) with multi_v7_defconfig using clang-18
I see the following warning:

.../sock.c:2808:5: warning: stack frame size (1664) exceeds limit (1024) in '__sock_cmsg_send' [-Wframe-larger-than]
 2808 | int __sock_cmsg_send(struct sock *sk, struct cmsghdr *cmsg,

I expect this is mostly explained by the addition of zc_info_kern above.

> +
> +		if (!sock_flag(sk, SOCK_ZEROCOPY) || sk->sk_family == PF_RDS)
> +			return -EINVAL;
> +
> +		cmsg_data_len = cmsg->cmsg_len - sizeof(struct cmsghdr);
> +		if (cmsg_data_len % sizeof(struct zc_info_elem))
> +			return -EINVAL;
> +
> +		zc_info_elem_num = cmsg_data_len / sizeof(struct zc_info_elem);
> +		if (!zc_info_elem_num || zc_info_elem_num > SOCK_ZC_INFO_MAX)
> +			return -EINVAL;
> +
> +		if (in_compat_syscall())
> +			usr_addr = compat_ptr(*(compat_uptr_t *)CMSG_DATA(cmsg));
> +		else
> +			usr_addr = (void __user *)*(void **)CMSG_DATA(cmsg);
> +		if (!access_ok(usr_addr, cmsg_data_len))
> +			return -EFAULT;
> +
> +		q = &sk->sk_error_queue;
> +		skb_queue_head_init(&local_q);
> +		spin_lock_irqsave(&q->lock, flags);
> +		skb = skb_peek(q);
> +		while (skb && i < zc_info_elem_num) {
> +			struct sk_buff *skb_next = skb_peek_next(skb, q);
> +
> +			serr = SKB_EXT_ERR(skb);
> +			if (serr->ee.ee_errno == 0 &&
> +			    serr->ee.ee_origin == SO_EE_ORIGIN_ZEROCOPY) {
> +				zc_info_kern[i].hi = serr->ee.ee_data;
> +				zc_info_kern[i].lo = serr->ee.ee_info;
> +				zc_info_kern[i].zerocopy = !(serr->ee.ee_code
> +								& SO_EE_CODE_ZEROCOPY_COPIED);
> +				__skb_unlink(skb, q);
> +				__skb_queue_tail(&local_q, skb);
> +				i++;
> +			}
> +			skb = skb_next;
> +		}
> +		spin_unlock_irqrestore(&q->lock, flags);
> +
> +		ret = copy_to_user(usr_addr,
> +				   zc_info_kern,
> +					i * sizeof(struct zc_info_elem));
> +
> +		if (unlikely(ret)) {
> +			spin_lock_irqsave(&q->lock, flags);
> +			skb_queue_reverse_walk_safe(&local_q, skb, tmp) {
> +				__skb_unlink(skb, &local_q);
> +				__skb_queue_head(q, skb);
> +			}
> +			spin_unlock_irqrestore(&q->lock, flags);
> +			return -EFAULT;
> +		}
> +
> +		while ((skb = __skb_dequeue(&local_q)))
> +			consume_skb(skb);
> +		break;
> +	}
>  	default:
>  		return -EINVAL;
>  	}
> -- 
> 2.20.1
> 
> 

Re: [External] Re: [PATCH net-next v4 2/3] sock: add MSG_ZEROCOPY notification mechanism based on msg_control

[Zijian Zhang]

On 6/1/24 3:37 AM, Simon Horman wrote:
> On Tue, May 28, 2024 at 09:21:02PM +0000, zijianzhang@bytedance.com wrote:
>> From: Zijian Zhang <zijianzhang@bytedance.com>
>>
>> The MSG_ZEROCOPY flag enables copy avoidance for socket send calls.
>> However, zerocopy is not a free lunch. Apart from the management of user
>> pages, the combination of poll + recvmsg to receive notifications incurs
>> unignorable overhead in the applications. The overhead of such sometimes
>> might be more than the CPU savings from zerocopy. We try to solve this
>> problem with a new notification mechanism based on msgcontrol.
>> This new mechanism aims to reduce the overhead associated with receiving
>> notifications by embedding them directly into user arguments passed with
>> each sendmsg control message. By doing so, we can significantly reduce
>> the complexity and overhead for managing notifications. In an ideal
>> pattern, the user will keep calling sendmsg with SCM_ZC_NOTIFICATION
>> msg_control, and the notification will be delivered as soon as possible.
>>
>> Signed-off-by: Zijian Zhang <zijianzhang@bytedance.com>
>> Signed-off-by: Xiaochun Lu <xiaochun.lu@bytedance.com>
> 
> ...
> 
>> diff --git a/net/core/sock.c b/net/core/sock.c
>> index 521e6373d4f7..21239469d75c 100644
>> --- a/net/core/sock.c
>> +++ b/net/core/sock.c
>> @@ -2847,6 +2847,74 @@ int __sock_cmsg_send(struct sock *sk, struct cmsghdr *cmsg,
>>   	case SCM_RIGHTS:
>>   	case SCM_CREDENTIALS:
>>   		break;
>> +	case SCM_ZC_NOTIFICATION: {
>> +		int ret, i = 0;
>> +		int cmsg_data_len, zc_info_elem_num;
>> +		void __user	*usr_addr;
>> +		struct zc_info_elem zc_info_kern[SOCK_ZC_INFO_MAX];
>> +		unsigned long flags;
>> +		struct sk_buff_head *q, local_q;
>> +		struct sk_buff *skb, *tmp;
>> +		struct sock_exterr_skb *serr;
> 
> Hi Zijian Zhang, Xiaochun Lu, all,
> 
> When compiling on ARM (32bit) with multi_v7_defconfig using clang-18
> I see the following warning:
> 
> .../sock.c:2808:5: warning: stack frame size (1664) exceeds limit (1024) in '__sock_cmsg_send' [-Wframe-larger-than]
>   2808 | int __sock_cmsg_send(struct sock *sk, struct cmsghdr *cmsg,
> 
> I expect this is mostly explained by the addition of zc_info_kern above.
> 

Nice catch, thanks for the info!

>> +
>> +		if (!sock_flag(sk, SOCK_ZEROCOPY) || sk->sk_family == PF_RDS)
>> +			return -EINVAL;
>> +
>> +		cmsg_data_len = cmsg->cmsg_len - sizeof(struct cmsghdr);
>> +		if (cmsg_data_len % sizeof(struct zc_info_elem))
>> +			return -EINVAL;
>> +
>> +		zc_info_elem_num = cmsg_data_len / sizeof(struct zc_info_elem);
>> +		if (!zc_info_elem_num || zc_info_elem_num > SOCK_ZC_INFO_MAX)
>> +			return -EINVAL;
>> +
>> +		if (in_compat_syscall())
>> +			usr_addr = compat_ptr(*(compat_uptr_t *)CMSG_DATA(cmsg));
>> +		else
>> +			usr_addr = (void __user *)*(void **)CMSG_DATA(cmsg);
>> +		if (!access_ok(usr_addr, cmsg_data_len))
>> +			return -EFAULT;
>> +
>> +		q = &sk->sk_error_queue;
>> +		skb_queue_head_init(&local_q);
>> +		spin_lock_irqsave(&q->lock, flags);
>> +		skb = skb_peek(q);
>> +		while (skb && i < zc_info_elem_num) {
>> +			struct sk_buff *skb_next = skb_peek_next(skb, q);
>> +
>> +			serr = SKB_EXT_ERR(skb);
>> +			if (serr->ee.ee_errno == 0 &&
>> +			    serr->ee.ee_origin == SO_EE_ORIGIN_ZEROCOPY) {
>> +				zc_info_kern[i].hi = serr->ee.ee_data;
>> +				zc_info_kern[i].lo = serr->ee.ee_info;
>> +				zc_info_kern[i].zerocopy = !(serr->ee.ee_code
>> +								& SO_EE_CODE_ZEROCOPY_COPIED);
>> +				__skb_unlink(skb, q);
>> +				__skb_queue_tail(&local_q, skb);
>> +				i++;
>> +			}
>> +			skb = skb_next;
>> +		}
>> +		spin_unlock_irqrestore(&q->lock, flags);
>> +
>> +		ret = copy_to_user(usr_addr,
>> +				   zc_info_kern,
>> +					i * sizeof(struct zc_info_elem));
>> +
>> +		if (unlikely(ret)) {
>> +			spin_lock_irqsave(&q->lock, flags);
>> +			skb_queue_reverse_walk_safe(&local_q, skb, tmp) {
>> +				__skb_unlink(skb, &local_q);
>> +				__skb_queue_head(q, skb);
>> +			}
>> +			spin_unlock_irqrestore(&q->lock, flags);
>> +			return -EFAULT;
>> +		}
>> +
>> +		while ((skb = __skb_dequeue(&local_q)))
>> +			consume_skb(skb);
>> +		break;
>> +	}
>>   	default:
>>   		return -EINVAL;
>>   	}
>> -- 
>> 2.20.1
>>
>>

Re: [External] Re: [PATCH net-next v4 2/3] sock: add MSG_ZEROCOPY notification mechanism based on msg_control

[Zijian Zhang]

On 6/2/24 3:29 PM, Willem de Bruijn wrote:
> On Fri, May 31, 2024 at 7:20 PM Zijian Zhang <zijianzhang@bytedance.com> wrote:
>>
>>
>>
>> On 5/28/24 2:21 PM, zijianzhang@bytedance.com wrote:
>>> From: Zijian Zhang <zijianzhang@bytedance.com>
>>>
>>> The MSG_ZEROCOPY flag enables copy avoidance for socket send calls.
>>> However, zerocopy is not a free lunch. Apart from the management of user
>>> pages, the combination of poll + recvmsg to receive notifications incurs
>>> unignorable overhead in the applications. The overhead of such sometimes
>>> might be more than the CPU savings from zerocopy. We try to solve this
>>> problem with a new notification mechanism based on msgcontrol.
>>> This new mechanism aims to reduce the overhead associated with receiving
>>> notifications by embedding them directly into user arguments passed with
>>> each sendmsg control message. By doing so, we can significantly reduce
>>> the complexity and overhead for managing notifications. In an ideal
>>> pattern, the user will keep calling sendmsg with SCM_ZC_NOTIFICATION
>>> msg_control, and the notification will be delivered as soon as possible.
>>>
>>> Signed-off-by: Zijian Zhang <zijianzhang@bytedance.com>
>>> Signed-off-by: Xiaochun Lu <xiaochun.lu@bytedance.com>
>>> ---
>>>    arch/alpha/include/uapi/asm/socket.h  |  2 +
>>>    arch/mips/include/uapi/asm/socket.h   |  2 +
>>>    arch/parisc/include/uapi/asm/socket.h |  2 +
>>>    arch/sparc/include/uapi/asm/socket.h  |  2 +
>>>    include/uapi/asm-generic/socket.h     |  2 +
>>>    include/uapi/linux/socket.h           | 10 ++++
>>>    net/core/sock.c                       | 68 +++++++++++++++++++++++++++
>>>    7 files changed, 88 insertions(+)
>>>
>>> ...
>>> diff --git a/include/uapi/linux/socket.h b/include/uapi/linux/socket.h
>>> index d3fcd3b5ec53..15cec8819f34 100644
>>> --- a/include/uapi/linux/socket.h
>>> +++ b/include/uapi/linux/socket.h
>>> @@ -2,6 +2,8 @@
>>>    #ifndef _UAPI_LINUX_SOCKET_H
>>>    #define _UAPI_LINUX_SOCKET_H
>>>
>>> +#include <linux/types.h>
>>> +
>>>    /*
>>>     * Desired design of maximum size and alignment (see RFC2553)
>>>     */
>>> @@ -35,4 +37,12 @@ struct __kernel_sockaddr_storage {
>>>    #define SOCK_TXREHASH_DISABLED      0
>>>    #define SOCK_TXREHASH_ENABLED       1
>>>
>>> +#define SOCK_ZC_INFO_MAX 128
>>> +
>>> +struct zc_info_elem {
>>> +     __u32 lo;
>>> +     __u32 hi;
>>> +     __u8 zerocopy;
>>> +};
>>> +
>>>    #endif /* _UAPI_LINUX_SOCKET_H */
>>> diff --git a/net/core/sock.c b/net/core/sock.c
>>> index 521e6373d4f7..21239469d75c 100644
>>> --- a/net/core/sock.c
>>> +++ b/net/core/sock.c
>>> @@ -2847,6 +2847,74 @@ int __sock_cmsg_send(struct sock *sk, struct cmsghdr *cmsg,
>>>        case SCM_RIGHTS:
>>>        case SCM_CREDENTIALS:
>>>                break;
>>> +     case SCM_ZC_NOTIFICATION: {
>>> +             int ret, i = 0;
>>> +             int cmsg_data_len, zc_info_elem_num;
>>> +             void __user     *usr_addr;
>>> +             struct zc_info_elem zc_info_kern[SOCK_ZC_INFO_MAX];
>>> +             unsigned long flags;
>>> +             struct sk_buff_head *q, local_q;
>>> +             struct sk_buff *skb, *tmp;
>>> +             struct sock_exterr_skb *serr;
>>> +
>>> +             if (!sock_flag(sk, SOCK_ZEROCOPY) || sk->sk_family == PF_RDS)
>>> +                     return -EINVAL;
>>> +
>>> +             cmsg_data_len = cmsg->cmsg_len - sizeof(struct cmsghdr);
>>> +             if (cmsg_data_len % sizeof(struct zc_info_elem))
>>> +                     return -EINVAL;
>>> +
>>> +             zc_info_elem_num = cmsg_data_len / sizeof(struct zc_info_elem);
>>> +             if (!zc_info_elem_num || zc_info_elem_num > SOCK_ZC_INFO_MAX)
>>> +                     return -EINVAL;
>>> +
>>> +             if (in_compat_syscall())
>>> +                     usr_addr = compat_ptr(*(compat_uptr_t *)CMSG_DATA(cmsg));
>>> +             else
>>> +                     usr_addr = (void __user *)*(void **)CMSG_DATA(cmsg);
>>
>> First of all, thanks for your efforts and time to review this series of
>> patchsets!
> 
> Please try to keep this conversation on the netdev list. What I
> respond below would be good to have in public discourse. Among
> others for others to come disagree with me and tell you that they
> prefer your patchset just the way it is ;-)
> 
>> I believe compat issue has been resolved in this if code block? I know
>> that the current design is quite hacky, and want to discuss next steps
>> with you,
>>
>> 1. Is it possible to change ____sys_sendmsg? So that we can copy
>> msg_controldata back to the user space.
> 
> I do think that this is the clean way to support passing metadata
> up to userspace.
> 
> The current approach looks like a hack to me. It works, but arguably
> is not how you implement a serious user API (ABI).
> 
> A more complete solution can potentially also be reused by other
> features that want to piggy-back information from the kernel back
> up to userspace with sendmsg. Timestamps is an example.
> 
> I did implement the full method once.
> 
> Initially, don't spend time on modifying ___sys_sendmsg *and*
> on supporting the compat version. With the right structs (that are
> not ambiguous between 32 and 64 bit) it may not even be needed.
> 
> But I would be more supportive of this full interface.
> 
> It also ties into the performance benefits observed. If they were
> shockingly good, I would still argue in favor of a cleaner API, to be
> clear. But a minor improvement is even less reason to consider a
> hacky API.
> 
> All of this is just one person's opinion, of course.
> 
>> 2. Is it possible to support this feature in recvmsg instead of
>> sendmsg? In the case of selftest where one thread keeps sending
>> and another keeps recving, this feature is useless. But in other
>> cases where one socket will send and recv, this might help?
> 
> That's a lot easier. And halves the cost of having to calll both
> recvmsg + recvmsg MSG_ERRQUEUE.
> 
>> For sockets that send many times but recv very few times, a hybrid
>> mode of notification using msg_control and errmsg_queue can be used.
>>
>> 3. Or, do you have any idea?
>>

I did not remember the reason why we directly pass in the user address
and have ctl_len to be the array size. Can I pass in a struct like
{
   __user void *user_addr;
   __u32 array_size;
}
and have ctl_len to be the sizeof the struct, just like my first patch
set? Of course, I will consider the compat issue for user_addr this time.

>> If the above solutions are hard to be accepted, I can totally
>> understand, and I will submit the fix to msg_zerocopy selftest only.

Re: [External] Re: [PATCH net-next v4 2/3] sock: add MSG_ZEROCOPY notification mechanism based on msg_control

[Willem de Bruijn]

Zijian Zhang wrote:
> On 6/2/24 3:29 PM, Willem de Bruijn wrote:
> > On Fri, May 31, 2024 at 7:20 PM Zijian Zhang <zijianzhang@bytedance.com> wrote:
> >>
> >>
> >>
> >> On 5/28/24 2:21 PM, zijianzhang@bytedance.com wrote:
> >>> From: Zijian Zhang <zijianzhang@bytedance.com>
> >>>
> >>> The MSG_ZEROCOPY flag enables copy avoidance for socket send calls.
> >>> However, zerocopy is not a free lunch. Apart from the management of user
> >>> pages, the combination of poll + recvmsg to receive notifications incurs
> >>> unignorable overhead in the applications. The overhead of such sometimes
> >>> might be more than the CPU savings from zerocopy. We try to solve this
> >>> problem with a new notification mechanism based on msgcontrol.
> >>> This new mechanism aims to reduce the overhead associated with receiving
> >>> notifications by embedding them directly into user arguments passed with
> >>> each sendmsg control message. By doing so, we can significantly reduce
> >>> the complexity and overhead for managing notifications. In an ideal
> >>> pattern, the user will keep calling sendmsg with SCM_ZC_NOTIFICATION
> >>> msg_control, and the notification will be delivered as soon as possible.
> >>>
> >>> Signed-off-by: Zijian Zhang <zijianzhang@bytedance.com>
> >>> Signed-off-by: Xiaochun Lu <xiaochun.lu@bytedance.com>
> >>> ---
> >>>    arch/alpha/include/uapi/asm/socket.h  |  2 +
> >>>    arch/mips/include/uapi/asm/socket.h   |  2 +
> >>>    arch/parisc/include/uapi/asm/socket.h |  2 +
> >>>    arch/sparc/include/uapi/asm/socket.h  |  2 +
> >>>    include/uapi/asm-generic/socket.h     |  2 +
> >>>    include/uapi/linux/socket.h           | 10 ++++
> >>>    net/core/sock.c                       | 68 +++++++++++++++++++++++++++
> >>>    7 files changed, 88 insertions(+)
> >>>
> >>> ...
> >>> diff --git a/include/uapi/linux/socket.h b/include/uapi/linux/socket.h
> >>> index d3fcd3b5ec53..15cec8819f34 100644
> >>> --- a/include/uapi/linux/socket.h
> >>> +++ b/include/uapi/linux/socket.h
> >>> @@ -2,6 +2,8 @@
> >>>    #ifndef _UAPI_LINUX_SOCKET_H
> >>>    #define _UAPI_LINUX_SOCKET_H
> >>>
> >>> +#include <linux/types.h>
> >>> +
> >>>    /*
> >>>     * Desired design of maximum size and alignment (see RFC2553)
> >>>     */
> >>> @@ -35,4 +37,12 @@ struct __kernel_sockaddr_storage {
> >>>    #define SOCK_TXREHASH_DISABLED      0
> >>>    #define SOCK_TXREHASH_ENABLED       1
> >>>
> >>> +#define SOCK_ZC_INFO_MAX 128
> >>> +
> >>> +struct zc_info_elem {
> >>> +     __u32 lo;
> >>> +     __u32 hi;
> >>> +     __u8 zerocopy;
> >>> +};
> >>> +
> >>>    #endif /* _UAPI_LINUX_SOCKET_H */
> >>> diff --git a/net/core/sock.c b/net/core/sock.c
> >>> index 521e6373d4f7..21239469d75c 100644
> >>> --- a/net/core/sock.c
> >>> +++ b/net/core/sock.c
> >>> @@ -2847,6 +2847,74 @@ int __sock_cmsg_send(struct sock *sk, struct cmsghdr *cmsg,
> >>>        case SCM_RIGHTS:
> >>>        case SCM_CREDENTIALS:
> >>>                break;
> >>> +     case SCM_ZC_NOTIFICATION: {
> >>> +             int ret, i = 0;
> >>> +             int cmsg_data_len, zc_info_elem_num;
> >>> +             void __user     *usr_addr;
> >>> +             struct zc_info_elem zc_info_kern[SOCK_ZC_INFO_MAX];
> >>> +             unsigned long flags;
> >>> +             struct sk_buff_head *q, local_q;
> >>> +             struct sk_buff *skb, *tmp;
> >>> +             struct sock_exterr_skb *serr;
> >>> +
> >>> +             if (!sock_flag(sk, SOCK_ZEROCOPY) || sk->sk_family == PF_RDS)
> >>> +                     return -EINVAL;
> >>> +
> >>> +             cmsg_data_len = cmsg->cmsg_len - sizeof(struct cmsghdr);
> >>> +             if (cmsg_data_len % sizeof(struct zc_info_elem))
> >>> +                     return -EINVAL;
> >>> +
> >>> +             zc_info_elem_num = cmsg_data_len / sizeof(struct zc_info_elem);
> >>> +             if (!zc_info_elem_num || zc_info_elem_num > SOCK_ZC_INFO_MAX)
> >>> +                     return -EINVAL;
> >>> +
> >>> +             if (in_compat_syscall())
> >>> +                     usr_addr = compat_ptr(*(compat_uptr_t *)CMSG_DATA(cmsg));
> >>> +             else
> >>> +                     usr_addr = (void __user *)*(void **)CMSG_DATA(cmsg);
> >>
> >> First of all, thanks for your efforts and time to review this series of
> >> patchsets!
> > 
> > Please try to keep this conversation on the netdev list. What I
> > respond below would be good to have in public discourse. Among
> > others for others to come disagree with me and tell you that they
> > prefer your patchset just the way it is ;-)
> > 
> >> I believe compat issue has been resolved in this if code block? I know
> >> that the current design is quite hacky, and want to discuss next steps
> >> with you,
> >>
> >> 1. Is it possible to change ____sys_sendmsg? So that we can copy
> >> msg_controldata back to the user space.
> > 
> > I do think that this is the clean way to support passing metadata
> > up to userspace.
> > 
> > The current approach looks like a hack to me. It works, but arguably
> > is not how you implement a serious user API (ABI).
> > 
> > A more complete solution can potentially also be reused by other
> > features that want to piggy-back information from the kernel back
> > up to userspace with sendmsg. Timestamps is an example.
> > 
> > I did implement the full method once.
> > 
> > Initially, don't spend time on modifying ___sys_sendmsg *and*
> > on supporting the compat version. With the right structs (that are
> > not ambiguous between 32 and 64 bit) it may not even be needed.
> > 
> > But I would be more supportive of this full interface.
> > 
> > It also ties into the performance benefits observed. If they were
> > shockingly good, I would still argue in favor of a cleaner API, to be
> > clear. But a minor improvement is even less reason to consider a
> > hacky API.
> > 
> > All of this is just one person's opinion, of course.
> > 
> >> 2. Is it possible to support this feature in recvmsg instead of
> >> sendmsg? In the case of selftest where one thread keeps sending
> >> and another keeps recving, this feature is useless. But in other
> >> cases where one socket will send and recv, this might help?
> > 
> > That's a lot easier. And halves the cost of having to calll both
> > recvmsg + recvmsg MSG_ERRQUEUE.
> > 
> >> For sockets that send many times but recv very few times, a hybrid
> >> mode of notification using msg_control and errmsg_queue can be used.
> >>
> >> 3. Or, do you have any idea?
> >>
> 
> I did not remember the reason why we directly pass in the user address
> and have ctl_len to be the array size. Can I pass in a struct like
> {
>    __user void *user_addr;
>    __u32 array_size;
> }
> and have ctl_len to be the sizeof the struct, just like my first patch
> set? Of course, I will consider the compat issue for user_addr this time.

A clean CMSG API uses put_cmsg to write the metadata into msg_control.

This will take care of any length, no need for additional explicit
length params.

See also how cmsg is used for kernel to user metadata on recv.

But, since ____sys_sendmsg actually creates a kernel copy of
msg_control and passes that to the callees, put_cmsg will write into
this kernel buffer, so on return ____sys_sendmsg will have to
copy_to_user to the original buffer.

So this definitely

- is more work, requiring changes to ____sys_sendmsg
- may slow down the ____sys_sendmsg hot path with extra branches

So I see the appeal of just passing a user pointer and punt on the
whole issue.

But that is rather crude, bypassing an established mechanism.

Which may be reusable for other kernel to user metadata associated
with the transmit path. As said, the tx timestamping example.

I remain that the biggest hurdle is that the demonstrated impact from
this new API is small. ABI changes can (almost) not be undone. So does
the new maintenance warrant that cost?

[PATCH net-next v4 3/3] selftests: add MSG_ZEROCOPY msg_control notification test

[zijianzhang]

From: Zijian Zhang <zijianzhang@bytedance.com>

We update selftests/net/msg_zerocopy.c to accommodate the new mechanism.

Test result from selftests/net/msg_zerocopy.c,
cfg_notification_limit = 1, in this case MSG_ZEROCOPY approximately
aligns with the semantics of MSG_ZEROCOPY_UARG. In this case, the new
flag has around 13% cpu savings in TCP and 18% cpu savings in UDP.
+---------------------+---------+---------+---------+---------+
| Test Type / Protocol| TCP v4  | TCP v6  | UDP v4  | UDP v6  |
+---------------------+---------+---------+---------+---------+
| ZCopy (MB)          | 5147    | 4885    | 7489    | 7854    |
+---------------------+---------+---------+---------+---------+
| New ZCopy (MB)      | 5859    | 5505    | 9053    | 9236    |
+---------------------+---------+---------+---------+---------+
| New ZCopy / ZCopy   | 113.83% | 112.69% | 120.88% | 117.59% |
+---------------------+---------+---------+---------+---------+

cfg_notification_limit = 32, it means less poll + recvmsg overhead,
the new mechanism performs 8% better in TCP. For UDP, no obvious
performance gain is observed and sometimes may lead to degradation.
Thus, if users don't need to retrieve the notification ASAP in UDP,
the original mechanism is preferred.
+---------------------+---------+---------+---------+---------+
| Test Type / Protocol| TCP v4  | TCP v6  | UDP v4  | UDP v6  |
+---------------------+---------+---------+---------+---------+
| ZCopy (MB)          | 6272    | 6138    | 12138   | 10055   |
+---------------------+---------+---------+---------+---------+
| New ZCopy (MB)      | 6774    | 6620    | 11504   | 10355   |
+---------------------+---------+---------+---------+---------+
| New ZCopy / ZCopy   | 108.00% | 107.85% | 94.78%  | 102.98% |
+---------------------+---------+---------+---------+---------+

Signed-off-by: Zijian Zhang <zijianzhang@bytedance.com>
Signed-off-by: Xiaochun Lu <xiaochun.lu@bytedance.com>
---
 tools/testing/selftests/net/msg_zerocopy.c  | 103 ++++++++++++++++++--
 tools/testing/selftests/net/msg_zerocopy.sh |   1 +
 2 files changed, 97 insertions(+), 7 deletions(-)

diff --git a/tools/testing/selftests/net/msg_zerocopy.c b/tools/testing/selftests/net/msg_zerocopy.c
index ba6c257f689c..48750a7a162c 100644
--- a/tools/testing/selftests/net/msg_zerocopy.c
+++ b/tools/testing/selftests/net/msg_zerocopy.c
@@ -1,3 +1,4 @@
+// SPDX-License-Identifier: GPL-2.0
 /* Evaluate MSG_ZEROCOPY
  *
  * Send traffic between two processes over one of the supported
@@ -66,6 +67,10 @@
 #define SO_ZEROCOPY	60
 #endif
 
+#ifndef SCM_ZC_NOTIFICATION
+#define SCM_ZC_NOTIFICATION	78
+#endif
+
 #ifndef SO_EE_CODE_ZEROCOPY_COPIED
 #define SO_EE_CODE_ZEROCOPY_COPIED	1
 #endif
@@ -74,6 +79,13 @@
 #define MSG_ZEROCOPY	0x4000000
 #endif
 
+#define ZC_MSGERR_NOTIFICATION 1
+#define ZC_MSGCTL_NOTIFICATION 2
+
+#define SOCK_ZC_INFO_NUM 8
+
+#define INVALID_ZEROCOPY_VAL 2
+
 static int  cfg_cork;
 static bool cfg_cork_mixed;
 static int  cfg_cpu		= -1;		/* default: pin to last cpu */
@@ -86,13 +98,16 @@ static int  cfg_runtime_ms	= 4200;
 static int  cfg_verbose;
 static int  cfg_waittime_ms	= 500;
 static int  cfg_notification_limit = 32;
-static bool cfg_zerocopy;
+static int  cfg_zerocopy;           /* Either 1 or 2, mode for SO_ZEROCOPY */
 
 static socklen_t cfg_alen;
 static struct sockaddr_storage cfg_dst_addr;
 static struct sockaddr_storage cfg_src_addr;
 
 static char payload[IP_MAXPACKET];
+static struct zc_info_elem zc_info[SOCK_ZC_INFO_NUM];
+static char zc_ckbuf[CMSG_SPACE(sizeof(void *))];
+static struct zc_info_elem *zc_info_ptr = zc_info;
 static long packets, bytes, completions, expected_completions;
 static int  zerocopied = -1;
 static uint32_t next_completion;
@@ -170,6 +185,26 @@ static int do_accept(int fd)
 	return fd;
 }
 
+static void add_zcopy_info(struct msghdr *msg)
+{
+	int i;
+	struct cmsghdr *cm;
+
+	if (!msg->msg_control)
+		error(1, errno, "NULL user arg");
+	cm = (void *)msg->msg_control;
+	/* Although only the address of the array will be written into the
+	 * zc_ckbuf, we assign cmsg_len to CMSG_LEN(sizeof(zc_info)) to specify
+	 * the length of the array.
+	 */
+	cm->cmsg_len = CMSG_LEN(sizeof(zc_info));
+	cm->cmsg_level = SOL_SOCKET;
+	cm->cmsg_type = SCM_ZC_NOTIFICATION;
+	memcpy(CMSG_DATA(cm), &zc_info_ptr, sizeof(zc_info_ptr));
+	for (i = 0; i < SOCK_ZC_INFO_NUM; i++)
+		zc_info[i].zerocopy = INVALID_ZEROCOPY_VAL;
+}
+
 static void add_zcopy_cookie(struct msghdr *msg, uint32_t cookie)
 {
 	struct cmsghdr *cm;
@@ -183,7 +218,7 @@ static void add_zcopy_cookie(struct msghdr *msg, uint32_t cookie)
 	memcpy(CMSG_DATA(cm), &cookie, sizeof(cookie));
 }
 
-static bool do_sendmsg(int fd, struct msghdr *msg, bool do_zerocopy, int domain)
+static bool do_sendmsg(int fd, struct msghdr *msg, int do_zerocopy, int domain)
 {
 	int ret, len, i, flags;
 	static uint32_t cookie;
@@ -201,6 +236,15 @@ static bool do_sendmsg(int fd, struct msghdr *msg, bool do_zerocopy, int domain)
 			msg->msg_controllen = CMSG_SPACE(sizeof(cookie));
 			msg->msg_control = (struct cmsghdr *)ckbuf;
 			add_zcopy_cookie(msg, ++cookie);
+		} else if (do_zerocopy == ZC_MSGCTL_NOTIFICATION) {
+			memset(&msg->msg_control, 0, sizeof(msg->msg_control));
+			/* Although only the address of the array will be written into the
+			 * zc_ckbuf, msg_controllen must be larger or equal than any cmsg_len
+			 * in it. Otherwise, we will get -EINVAL.
+			 */
+			msg->msg_controllen = CMSG_SPACE(sizeof(zc_info));
+			msg->msg_control = (struct cmsghdr *)zc_ckbuf;
+			add_zcopy_info(msg);
 		}
 	}
 
@@ -219,7 +263,7 @@ static bool do_sendmsg(int fd, struct msghdr *msg, bool do_zerocopy, int domain)
 		if (do_zerocopy && ret)
 			expected_completions++;
 	}
-	if (do_zerocopy && domain == PF_RDS) {
+	if (msg->msg_control) {
 		msg->msg_control = NULL;
 		msg->msg_controllen = 0;
 	}
@@ -393,6 +437,42 @@ static bool do_recvmsg_completion(int fd)
 	return ret;
 }
 
+static void do_recv_completions2(void)
+{
+	int i;
+	__u32 hi, lo, range;
+	__u8 zerocopy;
+
+	for (i = 0; zc_info[i].zerocopy != INVALID_ZEROCOPY_VAL; i++) {
+		struct zc_info_elem elem = zc_info[i];
+
+		hi = elem.hi;
+		lo = elem.lo;
+		zerocopy = elem.zerocopy;
+		range = hi - lo + 1;
+
+		if (cfg_verbose && lo != next_completion)
+			fprintf(stderr, "gap: %u..%u does not append to %u\n",
+				lo, hi, next_completion);
+		next_completion = hi + 1;
+
+		if (zerocopied == -1)
+			zerocopied = zerocopy;
+		else if (zerocopied != zerocopy) {
+			fprintf(stderr, "serr: inconsistent\n");
+			zerocopied = zerocopy;
+		}
+
+		completions += range;
+
+		if (cfg_verbose >= 2)
+			fprintf(stderr, "completed: %u (h=%u l=%u)\n",
+				range, hi, lo);
+	}
+
+	sendmsg_counter -= i;
+}
+
 static bool do_recv_completion(int fd, int domain)
 {
 	struct sock_extended_err *serr;
@@ -554,11 +634,15 @@ static void do_tx(int domain, int type, int protocol)
 		else
 			do_sendmsg(fd, &msg, cfg_zerocopy, domain);
 
-		if (cfg_zerocopy && sendmsg_counter >= cfg_notification_limit)
+		if (cfg_zerocopy == ZC_MSGERR_NOTIFICATION &&
+			sendmsg_counter >= cfg_notification_limit)
 			do_recv_completions(fd, domain);
 
+		if (cfg_zerocopy == ZC_MSGCTL_NOTIFICATION)
+			do_recv_completions2();
+
 		while (!do_poll(fd, POLLOUT)) {
-			if (cfg_zerocopy)
+			if (cfg_zerocopy == ZC_MSGERR_NOTIFICATION)
 				do_recv_completions(fd, domain);
 		}
 
@@ -716,7 +800,7 @@ static void parse_opts(int argc, char **argv)
 
 	cfg_payload_len = max_payload_len;
 
-	while ((c = getopt(argc, argv, "46c:C:D:i:l:mp:rs:S:t:vz")) != -1) {
+	while ((c = getopt(argc, argv, "46c:C:D:i:l:mnp:rs:S:t:vz")) != -1) {
 		switch (c) {
 		case '4':
 			if (cfg_family != PF_UNSPEC)
@@ -750,6 +834,9 @@ static void parse_opts(int argc, char **argv)
 		case 'm':
 			cfg_cork_mixed = true;
 			break;
+		case 'n':
+			cfg_zerocopy = ZC_MSGCTL_NOTIFICATION;
+			break;
 		case 'p':
 			cfg_port = strtoul(optarg, NULL, 0);
 			break;
@@ -769,7 +856,7 @@ static void parse_opts(int argc, char **argv)
 			cfg_verbose++;
 			break;
 		case 'z':
-			cfg_zerocopy = true;
+			cfg_zerocopy = ZC_MSGERR_NOTIFICATION;
 			break;
 		}
 	}
@@ -780,6 +867,8 @@ static void parse_opts(int argc, char **argv)
 			error(1, 0, "-D <server addr> required for PF_RDS\n");
 		if (!cfg_rx && !saddr)
 			error(1, 0, "-S <client addr> required for PF_RDS\n");
+		if (cfg_zerocopy == ZC_MSGCTL_NOTIFICATION)
+			error(1, 0, "PF_RDS does not support ZC_MSGCTL_NOTIFICATION");
 	}
 	setup_sockaddr(cfg_family, daddr, &cfg_dst_addr);
 	setup_sockaddr(cfg_family, saddr, &cfg_src_addr);
diff --git a/tools/testing/selftests/net/msg_zerocopy.sh b/tools/testing/selftests/net/msg_zerocopy.sh
index 89c22f5320e0..022a6936d86f 100755
--- a/tools/testing/selftests/net/msg_zerocopy.sh
+++ b/tools/testing/selftests/net/msg_zerocopy.sh
@@ -118,4 +118,5 @@ do_test() {
 
 do_test "${EXTRA_ARGS}"
 do_test "-z ${EXTRA_ARGS}"
+do_test "-n ${EXTRA_ARGS}"
 echo ok
-- 
2.20.1

Re: [PATCH net-next v4 0/3] net: A lightweight zero-copy notification

[Willem de Bruijn]

zijianzhang@ wrote:
> From: Zijian Zhang <zijianzhang@bytedance.com>
> 
> Original title is "net: socket sendmsg MSG_ZEROCOPY_UARG". 
> 
> Original notification mechanism needs poll + recvmmsg which is not
> easy for applcations to accommodate. And, it also incurs unignorable
> overhead including extra system calls and usage of socket optmem.
> 
> While making maximum reuse of the existing MSG_ZEROCOPY related code,
> this patch set introduces a new zerocopy socket notification mechanism.
> Users of sendmsg pass a control message as a placeholder for the incoming
> notifications. Upon returning, kernel embeds notifications directly into
> user arguments passed in. By doing so, we can significantly reduce the
> complexity and overhead for managing notifications. In an ideal pattern,
> the user will keep calling sendmsg with SCM_ZC_NOTIFICATION msg_control,
> and the notification will be delivered as soon as possible.
> 
> Users need to pass in a user space address pointing to an array of struct
> zc_info_elem, and the cmsg_len should be the memory size of the array
> instead of the size of the pointer itself.
> 
> As Willem commented,
> 
> > The main design issue with this series is this indirection, rather
> > than passing the array of notifications as cmsg.
> 
> > This trick circumvents having to deal with compat issues and having to
> > figure out copy_to_user in ____sys_sendmsg (as msg_control is an
> > in-kernel copy).
> 
> > This is quite hacky, from an API design PoV.
> 
> > As is passing a pointer, but expecting msg_controllen to hold the
> > length not of the pointer, but of the pointed to user buffer.
> 
> > I had also hoped for more significant savings. Especially with the
> > higher syscall overhead due to meltdown and spectre mitigations vs
> > when MSG_ZEROCOPY was introduced and I last tried this optimization.

Thanks for quoting this.

This revision does not address either of these concerns, right?

> 
> Changelog:
>   v1 -> v2:
>     - Reuse errormsg queue in the new notification mechanism,
>       users can actually use these two mechanisms in hybrid way
>       if they want to do so.
>     - Update case SCM_ZC_NOTIFICATION in __sock_cmsg_send
>       1. Regardless of 32-bit, 64-bit program, we will always handle
>       u64 type user address.
>       2. The size of data to copy_to_user is precisely calculated
>       in case of kernel stack leak.
>     - fix (kbuild-bot)
>       1. Add SCM_ZC_NOTIFICATION to arch-specific header files.
>       2. header file types.h in include/uapi/linux/socket.h
> 
>   v2 -> v3:
>     - 1. Users can now pass in the address of the zc_info_elem directly
>       with appropriate cmsg_len instead of the ugly user interface. Plus,
>       the handler is now compatible with MSG_CMSG_COMPAT and 32-bit
>       pointer.
>     - 2. Suggested by Willem, another strategy of getting zc info is
>       briefly taking the lock of sk_error_queue and move to a private
>       list, like net_rx_action. I thought sk_error_queue is protected by
>       sock_lock, so that it's impossible for the handling of zc info and
>       users recvmsg from the sk_error_queue at the same time.
>       However, sk_error_queue is protected by its own lock. I am afraid
>       that during the time it is handling the private list, users may
>       fail to get other error messages in the queue via recvmsg. Thus,
>       I don't implement the splice logic in this version. Any comments?
> 
>   v3 -> v4:
>     - 1. Change SOCK_ZC_INFO_MAX to 64 to avoid large stack frame size.
>     - 2. Fix minor typos.
>     - 3. Change cfg_zerocopy from int to enum in msg_zerocopy.c
> 
> * Performance
> 
> I extend the selftests/msg_zerocopy.c to accommodate the new mechanism,
> test result is as follows,
> 
> cfg_notification_limit = 1, in this case the original method approximately
> aligns with the semantics of new one. In this case, the new flag has
> around 13% cpu savings in TCP and 18% cpu savings in UDP.
> 
> +---------------------+---------+---------+---------+---------+
> | Test Type / Protocol| TCP v4  | TCP v6  | UDP v4  | UDP v6  |
> +---------------------+---------+---------+---------+---------+
> | ZCopy (MB)          | 5147    | 4885    | 7489    | 7854    |
> +---------------------+---------+---------+---------+---------+
> | New ZCopy (MB)      | 5859    | 5505    | 9053    | 9236    |
> +---------------------+---------+---------+---------+---------+
> | New ZCopy / ZCopy   | 113.83% | 112.69% | 120.88% | 117.59% |
> +---------------------+---------+---------+---------+---------+
> 
> 
> cfg_notification_limit = 32, the new mechanism performs 8% better in TCP.
> For UDP, no obvious performance gain is observed and sometimes may lead
> to degradation. Thus, if users don't need to retrieve the notification
> ASAP in UDP, the original mechanism is preferred.
> 
> +---------------------+---------+---------+---------+---------+
> | Test Type / Protocol| TCP v4  | TCP v6  | UDP v4  | UDP v6  |
> +---------------------+---------+---------+---------+---------+
> | ZCopy (MB)          | 6272    | 6138    | 12138   | 10055   |
> +---------------------+---------+---------+---------+---------+
> | New ZCopy (MB)      | 6774    | 6620    | 11504   | 10355   |
> +---------------------+---------+---------+---------+---------+
> | New ZCopy / ZCopy   | 108.00% | 107.85% | 94.78%  | 102.98% |
> +---------------------+---------+---------+---------+---------+
> 
> Zijian Zhang (3):
>   selftests: fix OOM problem in msg_zerocopy selftest
>   sock: add MSG_ZEROCOPY notification mechanism based on msg_control
>   selftests: add MSG_ZEROCOPY msg_control notification test
> 
>  arch/alpha/include/uapi/asm/socket.h        |   2 +
>  arch/mips/include/uapi/asm/socket.h         |   2 +
>  arch/parisc/include/uapi/asm/socket.h       |   2 +
>  arch/sparc/include/uapi/asm/socket.h        |   2 +
>  include/uapi/asm-generic/socket.h           |   2 +
>  include/uapi/linux/socket.h                 |  10 ++
>  net/core/sock.c                             |  68 ++++++++++++
>  tools/testing/selftests/net/msg_zerocopy.c  | 114 ++++++++++++++++++--
>  tools/testing/selftests/net/msg_zerocopy.sh |   1 +
>  9 files changed, 196 insertions(+), 7 deletions(-)
> 
> -- 
> 2.20.1
> 

Re: [External] Re: [PATCH net-next v4 0/3] net: A lightweight zero-copy notification

[Zijian Zhang]

On 5/29/24 6:59 AM, Willem de Bruijn wrote:
> zijianzhang@ wrote:
>> From: Zijian Zhang <zijianzhang@bytedance.com>
>>
>> Original title is "net: socket sendmsg MSG_ZEROCOPY_UARG".
>>
>> Original notification mechanism needs poll + recvmmsg which is not
>> easy for applcations to accommodate. And, it also incurs unignorable
>> overhead including extra system calls and usage of socket optmem.
>>
>> While making maximum reuse of the existing MSG_ZEROCOPY related code,
>> this patch set introduces a new zerocopy socket notification mechanism.
>> Users of sendmsg pass a control message as a placeholder for the incoming
>> notifications. Upon returning, kernel embeds notifications directly into
>> user arguments passed in. By doing so, we can significantly reduce the
>> complexity and overhead for managing notifications. In an ideal pattern,
>> the user will keep calling sendmsg with SCM_ZC_NOTIFICATION msg_control,
>> and the notification will be delivered as soon as possible.
>>
>> Users need to pass in a user space address pointing to an array of struct
>> zc_info_elem, and the cmsg_len should be the memory size of the array
>> instead of the size of the pointer itself.
>>
>> As Willem commented,
>>
>>> The main design issue with this series is this indirection, rather
>>> than passing the array of notifications as cmsg.
>>
>>> This trick circumvents having to deal with compat issues and having to
>>> figure out copy_to_user in ____sys_sendmsg (as msg_control is an
>>> in-kernel copy).
>>
>>> This is quite hacky, from an API design PoV.
>>
>>> As is passing a pointer, but expecting msg_controllen to hold the
>>> length not of the pointer, but of the pointed to user buffer.
>>
>>> I had also hoped for more significant savings. Especially with the
>>> higher syscall overhead due to meltdown and spectre mitigations vs
>>> when MSG_ZEROCOPY was introduced and I last tried this optimization.
> 
> Thanks for quoting this.
> 
> This revision does not address either of these concerns, right?
> 

Right, this revision just fixed some code according to your comments.

>>
>> Changelog:
>>    v1 -> v2:
>>      - Reuse errormsg queue in the new notification mechanism,
>>        users can actually use these two mechanisms in hybrid way
>>        if they want to do so.
>>      - Update case SCM_ZC_NOTIFICATION in __sock_cmsg_send
>>        1. Regardless of 32-bit, 64-bit program, we will always handle
>>        u64 type user address.
>>        2. The size of data to copy_to_user is precisely calculated
>>        in case of kernel stack leak.
>>      - fix (kbuild-bot)
>>        1. Add SCM_ZC_NOTIFICATION to arch-specific header files.
>>        2. header file types.h in include/uapi/linux/socket.h
>>
>>    v2 -> v3:
>>      - 1. Users can now pass in the address of the zc_info_elem directly
>>        with appropriate cmsg_len instead of the ugly user interface. Plus,
>>        the handler is now compatible with MSG_CMSG_COMPAT and 32-bit
>>        pointer.
>>      - 2. Suggested by Willem, another strategy of getting zc info is
>>        briefly taking the lock of sk_error_queue and move to a private
>>        list, like net_rx_action. I thought sk_error_queue is protected by
>>        sock_lock, so that it's impossible for the handling of zc info and
>>        users recvmsg from the sk_error_queue at the same time.
>>        However, sk_error_queue is protected by its own lock. I am afraid
>>        that during the time it is handling the private list, users may
>>        fail to get other error messages in the queue via recvmsg. Thus,
>>        I don't implement the splice logic in this version. Any comments?
>>
>>    v3 -> v4:
>>      - 1. Change SOCK_ZC_INFO_MAX to 64 to avoid large stack frame size.
>>      - 2. Fix minor typos.
>>      - 3. Change cfg_zerocopy from int to enum in msg_zerocopy.c
>>
>> * Performance
>>
>> I extend the selftests/msg_zerocopy.c to accommodate the new mechanism,
>> test result is as follows,
>>
>> cfg_notification_limit = 1, in this case the original method approximately
>> aligns with the semantics of new one. In this case, the new flag has
>> around 13% cpu savings in TCP and 18% cpu savings in UDP.
>>
>> +---------------------+---------+---------+---------+---------+
>> | Test Type / Protocol| TCP v4  | TCP v6  | UDP v4  | UDP v6  |
>> +---------------------+---------+---------+---------+---------+
>> | ZCopy (MB)          | 5147    | 4885    | 7489    | 7854    |
>> +---------------------+---------+---------+---------+---------+
>> | New ZCopy (MB)      | 5859    | 5505    | 9053    | 9236    |
>> +---------------------+---------+---------+---------+---------+
>> | New ZCopy / ZCopy   | 113.83% | 112.69% | 120.88% | 117.59% |
>> +---------------------+---------+---------+---------+---------+
>>
>>
>> cfg_notification_limit = 32, the new mechanism performs 8% better in TCP.
>> For UDP, no obvious performance gain is observed and sometimes may lead
>> to degradation. Thus, if users don't need to retrieve the notification
>> ASAP in UDP, the original mechanism is preferred.
>>
>> +---------------------+---------+---------+---------+---------+
>> | Test Type / Protocol| TCP v4  | TCP v6  | UDP v4  | UDP v6  |
>> +---------------------+---------+---------+---------+---------+
>> | ZCopy (MB)          | 6272    | 6138    | 12138   | 10055   |
>> +---------------------+---------+---------+---------+---------+
>> | New ZCopy (MB)      | 6774    | 6620    | 11504   | 10355   |
>> +---------------------+---------+---------+---------+---------+
>> | New ZCopy / ZCopy   | 108.00% | 107.85% | 94.78%  | 102.98% |
>> +---------------------+---------+---------+---------+---------+
>>
>> Zijian Zhang (3):
>>    selftests: fix OOM problem in msg_zerocopy selftest
>>    sock: add MSG_ZEROCOPY notification mechanism based on msg_control
>>    selftests: add MSG_ZEROCOPY msg_control notification test
>>
>>   arch/alpha/include/uapi/asm/socket.h        |   2 +
>>   arch/mips/include/uapi/asm/socket.h         |   2 +
>>   arch/parisc/include/uapi/asm/socket.h       |   2 +
>>   arch/sparc/include/uapi/asm/socket.h        |   2 +
>>   include/uapi/asm-generic/socket.h           |   2 +
>>   include/uapi/linux/socket.h                 |  10 ++
>>   net/core/sock.c                             |  68 ++++++++++++
>>   tools/testing/selftests/net/msg_zerocopy.c  | 114 ++++++++++++++++++--
>>   tools/testing/selftests/net/msg_zerocopy.sh |   1 +
>>   9 files changed, 196 insertions(+), 7 deletions(-)
>>
>> -- 
>> 2.20.1
>>
> 
> 

[PATCH net-next v4 0/3] net: A lightweight zero-copy notification

[zijianzhang]

From: Zijian Zhang <zijianzhang@bytedance.com>

Original title is "net: socket sendmsg MSG_ZEROCOPY_UARG". 

Original notification mechanism needs poll + recvmmsg which is not
easy for applcations to accommodate. And, it also incurs unignorable
overhead including extra system calls and usage of socket optmem.

While making maximum reuse of the existing MSG_ZEROCOPY related code,
this patch set introduces a new zerocopy socket notification mechanism.
Users of sendmsg pass a control message as a placeholder for the incoming
notifications. Upon returning, kernel embeds notifications directly into
user arguments passed in. By doing so, we can significantly reduce the
complexity and overhead for managing notifications. In an ideal pattern,
the user will keep calling sendmsg with SCM_ZC_NOTIFICATION msg_control,
and the notification will be delivered as soon as possible.

Users need to pass in a user space address pointing to an array of struct
zc_info_elem, and the cmsg_len should be the memory size of the array
instead of the size of the pointer itself.

As Willem commented,

> The main design issue with this series is this indirection, rather
> than passing the array of notifications as cmsg.

> This trick circumvents having to deal with compat issues and having to
> figure out copy_to_user in ____sys_sendmsg (as msg_control is an
> in-kernel copy).

> This is quite hacky, from an API design PoV.

> As is passing a pointer, but expecting msg_controllen to hold the
> length not of the pointer, but of the pointed to user buffer.

> I had also hoped for more significant savings. Especially with the
> higher syscall overhead due to meltdown and spectre mitigations vs
> when MSG_ZEROCOPY was introduced and I last tried this optimization.

Changelog:
  v1 -> v2:
    - Reuse errormsg queue in the new notification mechanism,
      users can actually use these two mechanisms in hybrid way
      if they want to do so.
    - Update case SCM_ZC_NOTIFICATION in __sock_cmsg_send
      1. Regardless of 32-bit, 64-bit program, we will always handle
      u64 type user address.
      2. The size of data to copy_to_user is precisely calculated
      in case of kernel stack leak.
    - fix (kbuild-bot)
      1. Add SCM_ZC_NOTIFICATION to arch-specific header files.
      2. header file types.h in include/uapi/linux/socket.h

  v2 -> v3:
    - 1. Users can now pass in the address of the zc_info_elem directly
      with appropriate cmsg_len instead of the ugly user interface. Plus,
      the handler is now compatible with MSG_CMSG_COMPAT and 32-bit
      pointer.
    - 2. Suggested by Willem, another strategy of getting zc info is
      briefly taking the lock of sk_error_queue and move to a private
      list, like net_rx_action. I thought sk_error_queue is protected by
      sock_lock, so that it's impossible for the handling of zc info and
      users recvmsg from the sk_error_queue at the same time.
      However, sk_error_queue is protected by its own lock. I am afraid
      that during the time it is handling the private list, users may
      fail to get other error messages in the queue via recvmsg. Thus,
      I don't implement the splice logic in this version. Any comments?

  v3 -> v4:
    - 1. Change SOCK_ZC_INFO_MAX to 64 to avoid large stack frame size.
    - 2. Fix minor typos.
    - 3. Change cfg_zerocopy from int to enum in msg_zerocopy.c

* Performance

I extend the selftests/msg_zerocopy.c to accommodate the new mechanism,
test result is as follows,

cfg_notification_limit = 1, in this case the original method approximately
aligns with the semantics of new one. In this case, the new flag has
around 13% cpu savings in TCP and 18% cpu savings in UDP.

+---------------------+---------+---------+---------+---------+
| Test Type / Protocol| TCP v4  | TCP v6  | UDP v4  | UDP v6  |
+---------------------+---------+---------+---------+---------+
| ZCopy (MB)          | 5147    | 4885    | 7489    | 7854    |
+---------------------+---------+---------+---------+---------+
| New ZCopy (MB)      | 5859    | 5505    | 9053    | 9236    |
+---------------------+---------+---------+---------+---------+
| New ZCopy / ZCopy   | 113.83% | 112.69% | 120.88% | 117.59% |
+---------------------+---------+---------+---------+---------+


cfg_notification_limit = 32, the new mechanism performs 8% better in TCP.
For UDP, no obvious performance gain is observed and sometimes may lead
to degradation. Thus, if users don't need to retrieve the notification
ASAP in UDP, the original mechanism is preferred.

+---------------------+---------+---------+---------+---------+
| Test Type / Protocol| TCP v4  | TCP v6  | UDP v4  | UDP v6  |
+---------------------+---------+---------+---------+---------+
| ZCopy (MB)          | 6272    | 6138    | 12138   | 10055   |
+---------------------+---------+---------+---------+---------+
| New ZCopy (MB)      | 6774    | 6620    | 11504   | 10355   |
+---------------------+---------+---------+---------+---------+
| New ZCopy / ZCopy   | 108.00% | 107.85% | 94.78%  | 102.98% |
+---------------------+---------+---------+---------+---------+

Zijian Zhang (3):
  selftests: fix OOM problem in msg_zerocopy selftest
  sock: add MSG_ZEROCOPY notification mechanism based on msg_control
  selftests: add MSG_ZEROCOPY msg_control notification test

 arch/alpha/include/uapi/asm/socket.h        |   2 +
 arch/mips/include/uapi/asm/socket.h         |   2 +
 arch/parisc/include/uapi/asm/socket.h       |   2 +
 arch/sparc/include/uapi/asm/socket.h        |   2 +
 include/uapi/asm-generic/socket.h           |   2 +
 include/uapi/linux/socket.h                 |  10 ++
 net/core/sock.c                             |  65 +++++++++++
 tools/testing/selftests/net/msg_zerocopy.c  | 116 ++++++++++++++++++--
 tools/testing/selftests/net/msg_zerocopy.sh |   1 +
 9 files changed, 195 insertions(+), 7 deletions(-)

-- 
2.20.1

Re: [PATCH net-next v4 0/3] net: A lightweight zero-copy notification

[Jakub Kicinski]

On Mon, 13 May 2024 21:17:52 +0000 zijianzhang@bytedance.com wrote:
> Original title is "net: socket sendmsg MSG_ZEROCOPY_UARG". 
> 
> Original notification mechanism needs poll + recvmmsg which is not
> easy for applcations to accommodate. And, it also incurs unignorable
> overhead including extra system calls and usage of socket optmem.

Hi, as described in:
https://www.kernel.org/doc/html/next/process/maintainer-netdev.html
we close the net-next tree for the duration of the merge window.
We will not be applying any -next patches for the next 2 weeks.

Feel free to continue posting new versions once you get feedback,
but please switch to posting as RFC.
-- 
pw-bot: defer