netdev - Re: [PATCH net-next 2/3] tcp: add newval parameter to tcp_rcvbuf

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: <44b10f91-1e19-48d0-9578-9b033b07fab7@kernel.org>
Date: Fri, 24 Oct 2025 16:02:51 +0200
From: Matthieu Baerts <matttbe@...nel.org>
To: Eric Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>
Cc: "David S . Miller" <davem@...emloft.net>, Jakub Kicinski
 <kuba@...nel.org>, Simon Horman <horms@...nel.org>,
 Neal Cardwell <ncardwell@...gle.com>, Willem de Bruijn <willemb@...gle.com>,
 Kuniyuki Iwashima <kuniyu@...gle.com>, Mat Martineau <martineau@...nel.org>,
 Geliang Tang <geliang@...nel.org>, netdev@...r.kernel.org,
 eric.dumazet@...il.com
Subject: Re: [PATCH net-next 2/3] tcp: add newval parameter to
 tcp_rcvbuf_grow()

Hi Eric,

On 24/10/2025 13:19, Eric Dumazet wrote:
> On Fri, Oct 24, 2025 at 3:09 AM Paolo Abeni <pabeni@...hat.com> wrote:
>>
>> Hi Eric,
>>
>> Many thanks for tracking this down!
>>
>> Recently we are observing mptcp selftests instabilities in
>> simult_flows.sh, Geliang bisected them to e118cdc34dd1 ("mptcp: rcvbuf
>> auto-tuning improvement") and the rcvbuf growing less. I *think* mptcp
>> selftests provide some value even for plain tcp :)
>>
>> On 10/24/25 9:50 AM, Eric Dumazet wrote:
>>> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
>>> index 94a5f6dcc5775e1265bb9f3c925fa80ae8c42924..2795acc96341765a3ec65657ec179cfd52ede483 100644
>>> --- a/net/mptcp/protocol.c
>>> +++ b/net/mptcp/protocol.c
>>> @@ -194,17 +194,19 @@ static bool mptcp_ooo_try_coalesce(struct mptcp_sock *msk, struct sk_buff *to,
>>>   * - mptcp does not maintain a msk-level window clamp
>>>   * - returns true when  the receive buffer is actually updated
>>>   */
>>> -static bool mptcp_rcvbuf_grow(struct sock *sk)
>>> +static bool mptcp_rcvbuf_grow(struct sock *sk, u32 newval)
>>>  {
>>>       struct mptcp_sock *msk = mptcp_sk(sk);
>>>       const struct net *net = sock_net(sk);
>>> -     int rcvwin, rcvbuf, cap;
>>> +     u32 rcvwin, rcvbuf, cap, oldval;
>>>
>>> +     oldval = msk->rcvq_space.copied;
>>> +     msk->rcvq_space.copied = newval;
>>
>> I *think* the above should be:
>>
>>         oldval = msk->rcvq_space.space;
>>         msk->rcvq_space.space = newval;
>>
> 
> You are right, thanks for catching this.
> 
> I developed / tested this series on a kernel where MPTCP changes were
> not there yet.
> 
> Only when rebasing to net-next I realized MPTCP had to be changed.

Thank you for the fix, and for having adapted MPTCP as well!

>> mptcp tracks the copied bytes incrementally - msk->rcvq_space.copied is
>> updated at each rcvmesg() iteration - and such difference IMHO makes
>> porting this kind of changes to mptcp a little more difficult.
>>
>> If you prefer, I can take care of the mptcp bits afterwards - I'll also
>> try to remove the mentioned difference and possibly move the algebra in
>> a common helper.
> 
> Do you want me to split this patch in two parts or is it okay if I
> send a V2 with
> the a/msk->rcvq_space.copied/msk->rcvq_space.space/ ?

If you send a v2, could it eventually target "net" instead please?

If the idea is to delay the fix to stable, it is always possible to ask
the stable team to backport it to stable in a few weeks / months, e.g.

  Cc: <stable@...r.kernel.org> # after -rc6

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.