[PATCH v2 net] tcp: fix mid stream window clamp.

[PATCH v2 net] tcp: fix mid stream window clamp.

[Paolo Abeni]

After the blamed commit below, if the user-space application performs
window clamping when tp->rcv_wnd is 0, the TCP socket will never be
able to announce a non 0 receive window, even after completely emptying
the receive buffer and re-setting the window clamp to higher values.

Refactor tcp_set_window_clamp() to address the issue: when the user
decreases the current clamp value, set rcv_ssthresh according to the
same logic used at buffer initialization, but ensuring reserved mem
provisioning.

To avoid code duplication factor-out the relevant bits from
tcp_adjust_rcv_ssthresh() in a new helper and reuse it in the above
scenario.

When increasing the clamp value, give the rcv_ssthresh a chance to grow
according to previously implemented heuristic.

Fixes: 3aa7857fe1d7 ("tcp: enable mid stream window clamp")
Reported-by: David Gibson <david@gibson.dropbear.id.au>
Reported-by: Stefano Brivio <sbrivio@redhat.com>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
v1 -> v2:
 - respect SO_RESERVE_MEM provisioning (Eric)
---
 include/net/tcp.h |  9 +++++++--
 net/ipv4/tcp.c    | 22 +++++++++++++++++++---
 2 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index d2f0736b76b8..144ba48bb07b 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -1514,17 +1514,22 @@ static inline int tcp_full_space(const struct sock *sk)
 	return tcp_win_from_space(sk, READ_ONCE(sk->sk_rcvbuf));
 }
 
-static inline void tcp_adjust_rcv_ssthresh(struct sock *sk)
+static inline void __tcp_adjust_rcv_ssthresh(struct sock *sk, u32 new_ssthresh)
 {
 	int unused_mem = sk_unused_reserved_mem(sk);
 	struct tcp_sock *tp = tcp_sk(sk);
 
-	tp->rcv_ssthresh = min(tp->rcv_ssthresh, 4U * tp->advmss);
+	tp->rcv_ssthresh = min(tp->rcv_ssthresh, new_ssthresh);
 	if (unused_mem)
 		tp->rcv_ssthresh = max_t(u32, tp->rcv_ssthresh,
 					 tcp_win_from_space(sk, unused_mem));
 }
 
+static inline void tcp_adjust_rcv_ssthresh(struct sock *sk)
+{
+	__tcp_adjust_rcv_ssthresh(sk, 4U * tcp_sk(sk)->advmss);
+}
+
 void tcp_cleanup_rbuf(struct sock *sk, int copied);
 void __tcp_cleanup_rbuf(struct sock *sk, int copied);
 
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 53bcc17c91e4..c9f078224569 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -3368,9 +3368,25 @@ int tcp_set_window_clamp(struct sock *sk, int val)
 			return -EINVAL;
 		tp->window_clamp = 0;
 	} else {
-		tp->window_clamp = val < SOCK_MIN_RCVBUF / 2 ?
-			SOCK_MIN_RCVBUF / 2 : val;
-		tp->rcv_ssthresh = min(tp->rcv_wnd, tp->window_clamp);
+		u32 new_rcv_ssthresh, old_window_clamp = tp->window_clamp;
+		u32 new_window_clamp = val < SOCK_MIN_RCVBUF / 2 ?
+						SOCK_MIN_RCVBUF / 2 : val;
+
+		if (new_window_clamp == old_window_clamp)
+			return 0;
+
+		tp->window_clamp = new_window_clamp;
+		if (new_window_clamp < old_window_clamp) {
+			/* need to apply the reserved mem provisioning only
+			 * when shrinking the window clamp
+			 */
+			__tcp_adjust_rcv_ssthresh(sk, tp->window_clamp);
+
+		} else {
+			new_rcv_ssthresh = min(tp->rcv_wnd, tp->window_clamp);
+			tp->rcv_ssthresh = max(new_rcv_ssthresh,
+					       tp->rcv_ssthresh);
+		}
 	}
 	return 0;
 }
-- 
2.42.0

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

[Eric Dumazet]

On Mon, Dec 4, 2023 at 5:08 PM Paolo Abeni <pabeni@redhat.com> wrote:
>
> After the blamed commit below, if the user-space application performs
> window clamping when tp->rcv_wnd is 0, the TCP socket will never be
> able to announce a non 0 receive window, even after completely emptying
> the receive buffer and re-setting the window clamp to higher values.
>
> Refactor tcp_set_window_clamp() to address the issue: when the user
> decreases the current clamp value, set rcv_ssthresh according to the
> same logic used at buffer initialization, but ensuring reserved mem
> provisioning.
>
> To avoid code duplication factor-out the relevant bits from
> tcp_adjust_rcv_ssthresh() in a new helper and reuse it in the above
> scenario.
>
> When increasing the clamp value, give the rcv_ssthresh a chance to grow
> according to previously implemented heuristic.
>
> Fixes: 3aa7857fe1d7 ("tcp: enable mid stream window clamp")
> Reported-by: David Gibson <david@gibson.dropbear.id.au>
> Reported-by: Stefano Brivio <sbrivio@redhat.com>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>

Reviewed-by: Eric Dumazet <edumazet@google.com>

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

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Mon,  4 Dec 2023 17:08:05 +0100 you wrote:
> After the blamed commit below, if the user-space application performs
> window clamping when tp->rcv_wnd is 0, the TCP socket will never be
> able to announce a non 0 receive window, even after completely emptying
> the receive buffer and re-setting the window clamp to higher values.
> 
> Refactor tcp_set_window_clamp() to address the issue: when the user
> decreases the current clamp value, set rcv_ssthresh according to the
> same logic used at buffer initialization, but ensuring reserved mem
> provisioning.
> 
> [...]

Here is the summary with links:
  - [v2,net] tcp: fix mid stream window clamp.
    https://git.kernel.org/netdev/net/c/58d3aade20cd

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html