| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <66816f57ea8e9_e2572942@willemb.c.googlers.com.notmuch>
Date: Sun, 30 Jun 2024 10:44:39 -0400
From: Willem de Bruijn <willemdebruijn.kernel@...il.com>
To: zijianzhang@...edance.com,
netdev@...r.kernel.org
Cc: edumazet@...gle.com,
willemdebruijn.kernel@...il.com,
cong.wang@...edance.com,
xiaochun.lu@...edance.com,
Zijian Zhang <zijianzhang@...edance.com>
Subject: Re: [PATCH net-next v6 3/4] sock: add MSG_ZEROCOPY notification
mechanism based on msg_control
zijianzhang@ wrote:
> From: Zijian Zhang <zijianzhang@...edance.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@...edance.com>
> Signed-off-by: Xiaochun Lu <xiaochun.lu@...edance.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/linux/socket.h | 2 +-
> include/uapi/asm-generic/socket.h | 2 ++
> include/uapi/linux/socket.h | 10 +++++++
> net/core/sock.c | 42 +++++++++++++++++++++++++++
> 8 files changed, 63 insertions(+), 1 deletion(-)
>
> 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/linux/socket.h b/include/linux/socket.h
> index 35adc30c9db6..f2f013166525 100644
> --- a/include/linux/socket.h
> +++ b/include/linux/socket.h
> @@ -170,7 +170,7 @@ static inline struct cmsghdr * cmsg_nxthdr (struct msghdr *__msg, struct cmsghdr
>
> static inline bool cmsg_copy_to_user(struct cmsghdr *__cmsg)
> {
> - return 0;
> + return __cmsg->cmsg_type == SCM_ZC_NOTIFICATION;
> }
>
> static inline size_t msg_data_left(struct msghdr *msg)
> 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..26bee6291c6c 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 16
> +
> +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 4a766a91ff5c..1b2ce72e1338 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -2863,6 +2863,48 @@ int __sock_cmsg_send(struct sock *sk, struct cmsghdr *cmsg,
> case SCM_RIGHTS:
> case SCM_CREDENTIALS:
> break;
> + case SCM_ZC_NOTIFICATION: {
> + struct zc_info_elem *zc_info_kern = CMSG_DATA(cmsg);
> + int cmsg_data_len, zc_info_elem_num;
> + struct sock_exterr_skb *serr;
> + struct sk_buff_head *q;
> + unsigned long flags;
> + struct sk_buff *skb;
> + int i = 0;
> +
> + 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;
> +
> + q = &sk->sk_error_queue;
> + 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);
> + consume_skb(skb);
> + }
> + skb = skb_next;
> + i++;
> + }
How will userspace know the number of entries written? Since the
cmsg_len is not updated, is the expectation that the CMSG_DATA is zero
initialized by the user and the list will be zero-element terminated?
> + spin_unlock_irqrestore(&q->lock, flags);
> + break;
> + }
> default:
> return -EINVAL;
> }
> --
> 2.20.1
>
Powered by blists - more mailing lists