netdev - Re: [PATCH net-next 1/3] sock: add MSG_ZEROCOPY

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CANn89iKjoEaSgHHKNvgWJ+Ro=rY_Z4ZzukTKe1Qn3y3Bt3X_-g@mail.gmail.com>
Date: Tue, 9 Apr 2024 23:23:10 +0200
From: Eric Dumazet <edumazet@...gle.com>
To: zijianzhang@...edance.com
Cc: netdev@...r.kernel.org, willemdebruijn.kernel@...il.com, 
	davem@...emloft.net, kuba@...nel.org, cong.wang@...edance.com, 
	xiaochun.lu@...edance.com
Subject: Re: [PATCH net-next 1/3] sock: add MSG_ZEROCOPY_UARG

On Tue, Apr 9, 2024 at 10:53 PM <zijianzhang@...edance.com> 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 option for TCP and UDP, MSG_ZEROCOPY_UARG.
> 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 MSG_ZEROCOPY_UARG
> flag, 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>
> ---
>  include/linux/skbuff.h                  |   7 +-
>  include/linux/socket.h                  |   1 +
>  include/linux/tcp.h                     |   3 +
>  include/linux/udp.h                     |   3 +
>  include/net/sock.h                      |  17 +++
>  include/net/udp.h                       |   1 +
>  include/uapi/asm-generic/socket.h       |   2 +
>  include/uapi/linux/socket.h             |  17 +++

...

> +
> +static inline void tx_message_zcopy_queue_init(struct tx_msg_zcopy_queue *q)
> +{
> +       spin_lock_init(&q->lock);
> +       INIT_LIST_HEAD(&q->head);
> +}
> +
>

> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index e767721b3a58..6254d0eef3af 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -462,6 +462,8 @@ void tcp_init_sock(struct sock *sk)
>
>         set_bit(SOCK_SUPPORT_ZC, &sk->sk_socket->flags);
>         sk_sockets_allocated_inc(sk);
> +
> +       tx_message_zcopy_queue_init(&tp->tx_zcopy_queue);
>  }


FYI,  tcp_init_sock() is not called for passive sockets.

syzbot would quite easily crash if zerovopy is used after accept()...