netdev - Re: [PATCH v1 13/15] io_uring/zcrx: add copy fallback

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: <e13c8d8b-eb66-4dd3-bfd4-8303393c592c@davidwei.uk>
Date: Thu, 10 Oct 2024 23:22:53 -0700
From: David Wei <dw@...idwei.uk>
To: Pavel Begunkov <asml.silence@...il.com>,
 Stanislav Fomichev <stfomichev@...il.com>
Cc: io-uring@...r.kernel.org, netdev@...r.kernel.org,
 Jens Axboe <axboe@...nel.dk>, Jakub Kicinski <kuba@...nel.org>,
 Paolo Abeni <pabeni@...hat.com>, "David S. Miller" <davem@...emloft.net>,
 Eric Dumazet <edumazet@...gle.com>, Jesper Dangaard Brouer
 <hawk@...nel.org>, David Ahern <dsahern@...nel.org>,
 Mina Almasry <almasrymina@...gle.com>, David Wei <dw@...idwei.uk>
Subject: Re: [PATCH v1 13/15] io_uring/zcrx: add copy fallback

On 2024-10-09 16:05, Pavel Begunkov wrote:
> On 10/9/24 17:30, Stanislav Fomichev wrote:
>> On 10/08, David Wei wrote:
>>> On 2024-10-08 08:58, Stanislav Fomichev wrote:
>>>> On 10/07, David Wei wrote:
>>>>> From: Pavel Begunkov <asml.silence@...il.com>
>>>>>
>>>>> There are scenarios in which the zerocopy path might get a normal
>>>>> in-kernel buffer, it could be a mis-steered packet or simply the linear
>>>>> part of an skb. Another use case is to allow the driver to allocate
>>>>> kernel pages when it's out of zc buffers, which makes it more resilient
>>>>> to spikes in load and allow the user to choose the balance between the
>>>>> amount of memory provided and performance.
>>>>
>>>> Tangential: should there be some clear way for the users to discover that
>>>> (some counter of some entry on cq about copy fallback)?
>>>>
>>>> Or the expectation is that somebody will run bpftrace to diagnose
>>>> (supposedly) poor ZC performance when it falls back to copy?
>>>
>>> Yeah there definitely needs to be a way to notify the user that copy
>>> fallback happened. Right now I'm relying on bpftrace hooking into
>>> io_zcrx_copy_chunk(). Doing it per cqe (which is emitted per frag) is
>>> too much. I can think of two other options:
>>>
>>> 1. Send a final cqe at the end of a number of frag cqes with a count of
>>>     the number of copies.
>>> 2. Register a secondary area just for handling copies.
>>>
>>> Other suggestions are also very welcome.
>>
>> SG, thanks. Up to you and Pavel on the mechanism and whether to follow
>> up separately. Maybe even move this fallback (this patch) into that separate
>> series as well? Will be easier to review/accept the rest.
> 
> I think it's fine to leave it? It shouldn't be particularly
> interesting to the net folks to review, and without it any skb
> with the linear part would break it, but perhaps it's not such
> a concern for bnxt.
> 

My preference is to leave it. Actually from real workloads, fully
linearized skbs are not uncommon due to the minimum size for HDS to kick
in for bnxt. Taking this out would imo make this patchset functionally
broken. Since we're all in agreement here, let's defer the improvements
as a follow up.