netdev - Re: [RFC Patch v1 1/3] net: ena: implement local page cache (LPC) system

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: <pj41zla6r3ld6m.fsf@u570694869fb251.ant.amazon.com>
Date:   Tue, 16 Mar 2021 10:23:29 +0200
From:   Shay Agroskin <shayagr@...zon.com>
To:     Saeed Mahameed <saeed@...nel.org>
CC:     Andrew Lunn <andrew@...n.ch>,
        Eric Dumazet <eric.dumazet@...il.com>,
        Jesper Dangaard Brouer <jbrouer@...hat.com>,
        Matteo Croce <mcroce@...rosoft.com>,
        David Miller <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>, <netdev@...r.kernel.org>,
        "Woodhouse, David" <dwmw@...zon.com>,
        "Machulsky, Zorik" <zorik@...zon.com>,
        "Matushevsky, Alexander" <matua@...zon.com>,
        Saeed Bshara <saeedb@...zon.com>,
        "Wilson, Matt" <msw@...zon.com>,
        "Liguori, Anthony" <aliguori@...zon.com>,
        "Bshara, Nafea" <nafea@...zon.com>,
        "Tzalik, Guy" <gtzalik@...zon.com>,
        "Belgazal, Netanel" <netanel@...zon.com>,
        "Saidi, Ali" <alisaidi@...zon.com>,
        "Herrenschmidt, Benjamin" <benh@...zon.com>,
        "Kiyanovski, Arthur" <akiyano@...zon.com>,
        "Jubran, Samih" <sameehj@...zon.com>,
        "Dagan, Noam" <ndagan@...zon.com>
Subject: Re: [RFC Patch v1 1/3] net: ena: implement local page cache (LPC)
 system


Saeed Mahameed <saeed@...nel.org> writes:

> On Wed, 2021-03-10 at 03:04 +0100, Andrew Lunn wrote:
>> On Tue, Mar 09, 2021 at 06:57:06PM +0100, Eric Dumazet wrote:
>> > 
>> > 
>> > On 3/9/21 6:10 PM, Shay Agroskin wrote:
>> > > The page cache holds pages we allocated in the past during 
>> > > napi
>> > > cycle,
>> > > and tracks their availability status using page ref count.
>> > > 
>> > > The cache can hold up to 2048 pages. Upon allocating a 
>> > > page, we
>
> 2048 per core ? IMHO this is too much ! ideally you want twice 
> the napi
> budget.
>
> you are trying to mitigate against TCP/L4 delays/congestion but 
> this is
> very prone to DNS attacks, if your memory allocators are under 
> stress,
> you shouldn't be hogging own pages and worsen the situation.

First of all, thank you for taking a look in this patchset.

We are trying to mitigate a simultaneous access to a shared 
resource, the buddy allocator.
When using local caches, I reduce the number of accesses to this 
shared resource by about 90%, thus
avoiding this contention.

I might not understand you correctly, but this patch doesn't try 
to mitigate network peaks. I agree that we're hogging quite a lot 
of system's resources, I'll run some tests with smaller cache size 
(e.g. 2x napi badget) and see if it mitigates the problem we have

>
>> > > check
>> > > whether the next entry in the cache contains an unused 
>> > > page, and
>> > > if so
>> > > fetch it. If the next page is already used by another 
>> > > entity or
>> > > if it
>> > > belongs to a different NUMA core than the napi routine, we
>> > > allocate a
>> > > page in the regular way (page from a different NUMA core is
>> > > replaced by
>> > > the newly allocated page).
>> > > 
>> > > This system can help us reduce the contention between 
>> > > different
>> > > cores
>> > > when allocating page since every cache is unique to a 
>> > > queue.
>> > 
>> > For reference, many drivers already use a similar strategy.
>> 
>> Hi Eric
>> 
>> So rather than yet another implementation, should we push for a
>> generic implementation which any driver can use?
>> 
>
> We already have it:
> https://www.kernel.org/doc/html/latest/networking/page_pool.html

Yup the original page pool implementation didn't suit our needs 
since we never got to free the pages using its specialized 
function for non-XDP traffic.

>
> also please checkout this fresh page pool extension, SKB buffer
> recycling RFC, might be useful for the use cases ena are 
> interested in
>
> https://patchwork.kernel.org/project/netdevbpf/patch/20210311194256.53706-4-mcroce@linux.microsoft.com/

Gone over the code and ran some tests with this patchset. On first 
look it seems like it does allow us to mitigate the problem this 
patchset solves. I'll run
more tests with it and report on this thread my conclusions.

Thanks a lot for pointing it out (:

Shay