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: <CANn89iKgqF_9pn6FeyjKtq-oVS-TsYYhvyVRbOs3RzYqXY0DWQ@mail.gmail.com>
Date: Fri, 24 Oct 2025 07:47:31 -0700
From: Eric Dumazet <edumazet@...gle.com>
To: Matthieu Baerts <matttbe@...nel.org>
Cc: Paolo Abeni <pabeni@...hat.com>, "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()

On Fri, Oct 24, 2025 at 7:02 AM Matthieu Baerts <matttbe@...nel.org> wrote:
>
> 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
>

I usually stack multiple patches, and net-next allows for less merge conflicts.

See for instance
https://lore.kernel.org/netdev/20251024120707.3516550-1-edumazet@google.com/T/#u
which touches tcp_rcv_space_adjust(), and definitely net-next candidate.

Bug was added 5 months ago, and does not seem critical to me
(otherwise we would have caught it much much earlier) ?

Truth be told, I had first to fix TSO defer code, and thought the fix
was not good enough.