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: <CANn89iLjPLbzBprZp3KFcbzsBYWefLgB3witokh5fvk3P2SFsA@mail.gmail.com>
Date: Fri, 24 Oct 2025 04:19:07 -0700
From: Eric Dumazet <edumazet@...gle.com>
To: 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>, 
	Matthieu Baerts <matttbe@...nel.org>, 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 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.

> 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/ ?