[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID:
<SA1PR15MB5187A4D321D44A7EB4CBD195A3BCA@SA1PR15MB5187.namprd15.prod.outlook.com>
Date: Tue, 28 Nov 2023 10:56:24 +0000
From: Neil Spring <ntspring@...a.com>
To: Paolo Abeni <pabeni@...hat.com>, Eric Dumazet <edumazet@...gle.com>,
Neal
Cardwell <ncardwell@...gle.com>, Wei Wang <weiwan@...gle.com>
CC: "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"David S. Miller"
<davem@...emloft.net>,
David Ahern <dsahern@...nel.org>, Jakub Kicinski
<kuba@...nel.org>,
David Gibson <david@...son.dropbear.id.au>
Subject: Re: [PATCH net] tcp: fix mid stream window clamp.
>
>
> ________________________________________
> From: Paolo Abeni <pabeni@...hat.com>
> Sent: Tuesday, November 28, 2023 12:59 AM
> To: Neil Spring; Eric Dumazet; Neal Cardwell; Wei Wang
> Cc: netdev@...r.kernel.org; David S. Miller; David Ahern; Jakub Kicinski; David Gibson
> Subject: Re: [PATCH net] tcp: fix mid stream window clamp.
>
> !-------------------------------------------------------------------|
> This Message Is From an External Sender
>
> |-------------------------------------------------------------------!
>
> On Mon, 2023-11-27 at 22:59 +0000, Neil Spring wrote:
> >
> > Would the following address the concern?
> >
> > tp->rcv_ssthresh = min(max(tp->rcv_ssthresh, tp->rcv_wnd), tp-
> > >window_clamp);
> >
> > (that is, rcv_sshthresh must be no greater than window_clamp, but
> > otherwise it can keep the larger of its current value or the last
> > advertised window.)
> >
> > I believe this addresses both problem cases (transient tiny clamp;
> > closed window when clamping) and passes (slightly less picky)
> > packetdrill tests.
>
> Note that the above is basically the patch I submitted (it yields the
> same values).
Yes!
>
> Yes, it addresses the issue.
>
> But it does not address Eric's concerns reported in this thread.
>
> It's unclear to me if the more involved approach proposed here:
>
> https://lore.kernel.org/netdev/ebb26a4a8a80292423c8cfc965c7b16e2aa4e201.camel@redhat.com/
>
> would be ok?
My understanding of the documentation of TCP_WINDOW_CLAMP is: "Bound the size of the advertised window to this value. The kernel imposes a minimum size of SOCK_MIN_RCVBUF/2." So in my opinion, increasing this value above the application-requested clamp to at least the unused reserved buffer space adds complexity and is not correct. I suspect the result is also not predictable, since it depends on whether the application has consumed the buffer.
The clamp and allocated buffer space weren't tied like this before - I could set SO_RCVBUF large and TCP_WINDOW_CLAMP small, perhaps slowing the network transfer to match a slow receiving application, and expect the clamp to do as I asked - so I don't quite see what's new here.
I guess Eric has a different perspective and I should let you figure out what's best for the interaction with reserved memory.
Thanks again for fixing this.
-neil
>
> Thanks!
>
> Paolo
>
>
Powered by blists - more mailing lists