netdev - Re: [PATCH net] tcp: fix mid stream window clamp.

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] [day] [month] [year] [list]

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