netdev - Re: [PATCH net-next 0/3] net: socket sendmsg 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: <ae625989c2b1a21b9f2550ff1d835210d2cf2ca9.camel@redhat.com>
Date: Wed, 10 Apr 2024 10:46:06 +0200
From: Paolo Abeni <pabeni@...hat.com>
To: zijianzhang@...edance.com, netdev@...r.kernel.org
Cc: edumazet@...gle.com, willemdebruijn.kernel@...il.com,
 davem@...emloft.net,  kuba@...nel.org, cong.wang@...edance.com,
 xiaochun.lu@...edance.com
Subject: Re: [PATCH net-next 0/3] net: socket sendmsg MSG_ZEROCOPY_UARG

On Tue, 2024-04-09 at 20:52 +0000, zijianzhang@...edance.com wrote:
> From: Zijian Zhang <zijianzhang@...edance.com>
> 
> 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 optmem.
> 
> While making maximum reuse of the existing MSG_ZEROCOPY related code,
> this patch set introduces zerocopy socket send flag MSG_ZEROCOPY_UARG.
> It provides a new notification method. 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 MSG_ZEROCOPY_UARG flag, and the notification will be
> delivered as soon as possible.
> 
> MSG_ZEROCOPY_UARG does not need to queue skb into errqueue. Thus,
> skbuffs allocated from optmem are not a must. In theory, a new struct
> carrying the zcopy information should be defined along with its memory
> management code. However, existing zcopy generic code assumes the
> information is skbuff. Given the very limited performance gain or maybe
> no gain of this method, and the need to change a lot of existing code,
> we still use skbuffs allocated from optmem to carry zcopy information.
> 
> * Performance
> 
> I extend the selftests/msg_zerocopy.c to accommodate the new flag, test
> result is as follows, the new flag performs 7% better in TCP and 4%
> better in UDP.
> 
> cfg_notification_limit = 8
> +---------------------+---------+---------+---------+---------+
> > Test Type / Protocol| TCP v4  | TCP v6  | UDP v4  | UDP v6  |
> +---------------------+---------+---------+---------+---------+
> > Copy                | 5328    | 5159    | 8581    | 8457    |
> +---------------------+---------+---------+---------+---------+
> > ZCopy               | 5877    | 5568    | 10314   | 10091   |
> +---------------------+---------+---------+---------+---------+
> > New ZCopy           | 6254    | 5901    | 10674   | 10293   |
> +---------------------+---------+---------+---------+---------+
> > ZCopy / Copy        | 110.30% | 107.93% | 120.20% | 119.32% |
> +---------------------+---------+---------+---------+---------+
> > New ZCopy / Copy    | 117.38% | 114.38% | 124.39% | 121.71% |
> +---------------------+---------+---------+---------+---------+

Minor nit for a future revision: the relevant part here is the 'New
ZCopy/ ZCopy' direct comparison, which is missing - even if inferable
from the above.  You should provide such data, and you could drop the
'ZCopy / Copy' 'New ZCopy / Copy' and possibly even 'Copy' lines.

Thanks,

Paolo