netdev - Re: [PATCH net-next] net/smc: Use kvzalloc for allocating smc_link

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] [day] [month] [year] [list]

Message-ID: <8f56ed63-79b4-2a75-4e95-f4f6a0ad8430@linux.ibm.com>
Date:   Thu, 27 Jan 2022 16:28:38 +0100
From:   Karsten Graul <kgraul@...ux.ibm.com>
To:     Tony Lu <tonylu@...ux.alibaba.com>
Cc:     kuba@...nel.org, davem@...emloft.net, netdev@...r.kernel.org,
        linux-s390@...r.kernel.org
Subject: Re: [PATCH net-next] net/smc: Use kvzalloc for allocating
 smc_link_group

On 24/01/2022 10:46, Tony Lu wrote:
> On Fri, Jan 21, 2022 at 12:06:56PM +0100, Karsten Graul wrote:
>> On 21/01/2022 04:24, Tony Lu wrote:
>>> On Thu, Jan 20, 2022 at 03:50:26PM +0100, Karsten Graul wrote:
>>>> On 20/01/2022 15:09, Tony Lu wrote:
>> I am still not fully convinced of this change. It does not harm and the overhead of
>> a vmalloc() is acceptable because a link group is not created so often. But since
>> kvzmalloc() will first try to use normal kmalloc() and if that fails switch to the
>> (more expensive) vmalloc() this will not _save_ any contiguous memory.
>> And for the subsequent required allocations of at least one RMB we need another 16KB.
> I agree with you. kvzmalloc doesn't save contiguous memory for the most
> time, only when high order contiguous memory is used out, or malloc link
> group when another link group just freed its buffer. This race window is
> too small to reach it in real world.

Okay I see we are in sync with that, and we should drop your kvzalloc() patch.
It generates noise and doesn't solve a real problem.

I appreciate your work on this topic, but when I see the numbers then the whole lgr 
WITH all links inside it would occupy less than one 4K page of memory (~3808 bytes).
The vast majority of memory in this struct is needed by the

struct smc_rtoken rtokens[255][3]; /*  3472 12240 */

array. This is where continuous space could be saved, but that needs some effort
to provide an equivalent fast way to store and lookup the RMBs.

Moving out the links from the lgr will not help here.

A link group holds up to 255 connections, so even with your 10000 connection test
we need no more than 40 instances of lgr...I am not sure if it is worth the time that
you need to spend for this particular change (lgr). The other topics you listed also
sound interesting!