netdev - Re: [PATCH net] net: allow small head cache usage with large MAX_SKB

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: <389ee8e5-8c25-414c-ae19-7dfeebecf1d3@redhat.com>
Date: Tue, 18 Feb 2025 15:50:53 +0100
From: Paolo Abeni <pabeni@...hat.com>
To: Eric Dumazet <edumazet@...gle.com>
Cc: netdev@...r.kernel.org, "David S. Miller" <davem@...emloft.net>,
 Jakub Kicinski <kuba@...nel.org>, Simon Horman <horms@...nel.org>,
 Sabrina Dubroca <sd@...asysnail.net>
Subject: Re: [PATCH net] net: allow small head cache usage with large
 MAX_SKB_FRAGS values

On 2/18/25 2:43 PM, Eric Dumazet wrote:
> On Mon, Feb 17, 2025 at 3:48 PM Paolo Abeni <pabeni@...hat.com> wrote:
>>
>> On 2/12/25 9:47 PM, Eric Dumazet wrote:
>>> diff --git a/include/net/tcp.h b/include/net/tcp.h
>>> index 5b2b04835688f65daa25ca208e29775326520e1e..a14ab14c14f1bd6275ab2d1d93bf230b6be14f49
>>> 100644
>>> --- a/include/net/tcp.h
>>> +++ b/include/net/tcp.h
>>> @@ -56,7 +56,11 @@ DECLARE_PER_CPU(u32, tcp_tw_isn);
>>>
>>>  void tcp_time_wait(struct sock *sk, int state, int timeo);
>>>
>>> -#define MAX_TCP_HEADER L1_CACHE_ALIGN(128 + MAX_HEADER)
>>> +#define MAX_TCP_HEADER L1_CACHE_ALIGN(64 + MAX_HEADER)
>>
>> I'm sorry for the latency following-up here, I really want to avoid
>> another fiasco.
>>
>> If I read correctly, you see the warning on top of my patch because you
>> have the above chunk in your local tree, am I correct?
> 
> Not at all, simply using upstream trees, perhaps a different .config
> than yours ?

Could you please share the conf, if you have that still handy?

> I think I suggested to change MAX_TCP_HEADER like this because max TCP
> header is 60 bytes.
> 
> Add to this MAX_HEADER, and round to a cache line, this comes to :
> 
> #define MAX_TCP_HEADER L1_CACHE_ALIGN(64 + MAX_HEADER)
> 
> This standalone change certainly can be done much later in net-next

Great, I'll opt for that option then.

>> If so, would you be ok to split the change in a 'net' patch doing the
>> minimal fix (basically the initially posted patch) and following-up on
>> net-next to adjust MAX_TCP_HEADER and SKB_SMALL_HEAD_SIZE as you suggest?
>>
>> I have a vague fear some encap scenario may suffer from the reduced TCP
>> headroom, I would refrain from pushing such change on stable, if possible.
> 
> Then MAX_HEADER might be too small ?

Indeed, and a "too large" MAX_TCP_HEADER would be hiding the problem.
Putting the change on net-next should help catching such a case.

Thanks,

Paolo