netdev - Re: [External] 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]

Date: Tue, 9 Apr 2024 17:11:23 -0700
From: Zijian Zhang <zijianzhang@...edance.com>
To: Eric Dumazet <edumazet@...gle.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: [External] Re: [PATCH net-next 1/3] sock: add MSG_ZEROCOPY_UARG



On 4/9/24 2:23 PM, Eric Dumazet wrote:
> 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()...

Thanks for the info, I will reuse error queue in the next patch set and
this line of code will be deleted.