Netdev List
 help / color / mirror / Atom feed
* [PATCH net] tcp: do not change rcv_ssthresh in tcp_measure_rcv_mss()
@ 2026-07-25  3:08 Nathan Gao
  2026-08-01  0:46 ` Jakub Kicinski
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Nathan Gao @ 2026-07-25  3:08 UTC (permalink / raw)
  To: Eric Dumazet, Neal Cardwell
  Cc: Kuniyuki Iwashima, David S . Miller, Jakub Kicinski, Paolo Abeni,
	Simon Horman, netdev, linux-kernel, Nathan Gao

Commit f5da7c45188e ("tcp: adjust rcvq_space after updating scaling
ratio") replaced the direct window_clamp update in tcp_measure_rcv_mss()
with a call to tcp_set_window_clamp(), a helper that implements the
TCP_WINDOW_CLAMP setsockopt. As a side effect, the helper also shrinks
rcv_ssthresh via __tcp_adjust_rcv_ssthresh().

As a result, each scaling_ratio decrease detected by
tcp_measure_rcv_mss() also cuts rcv_ssthresh. Elsewhere in TCP,
rcv_ssthresh is usually cut under memory pressure and grows via
tcp_grow_window().

Flows whose segment sizes vary keep scaling_ratio oscillating, which
leads to an unstable rcv_ssthresh: a dip of rcv_ssthresh only recovers
via tcp_grow_window(), keeping the advertised window at a relatively
low level even after the ratio itself has recovered, and can even stall
the sender.

Observed on a customer's proxy gateway after upgrading from kernel 6.1
to 6.12: in the worst case, rcv_ssthresh was cut in half by a
scaling_ratio dip. P99 latency jumped from <10ms on 6.1 to ~100ms on
6.12, and almost returned to the 6.1 level with this patch applied.

Restore the plain WRITE_ONCE() update of window_clamp, as introduced
in commit a2cbb1603943 ("tcp: Update window clamping condition"), and
keep the rcvq_space.space adjustment. Now rcv_ssthresh is decoupled from
scaling_ratio changes in tcp_measure_rcv_mss().

Fixes: f5da7c45188e ("tcp: adjust rcvq_space after updating scaling ratio")
Signed-off-by: Nathan Gao <zcgao@amazon.com>
---
 net/ipv4/tcp_input.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index daff93d513428..5b6378b94701e 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -252,7 +252,7 @@ static void tcp_measure_rcv_mss(struct sock *sk, const struct sk_buff *skb)
 				struct tcp_sock *tp = tcp_sk(sk);
 
 				val = tcp_win_from_space(sk, sk->sk_rcvbuf);
-				tcp_set_window_clamp(sk, val);
+				WRITE_ONCE(tp->window_clamp, val);
 
 				if (tp->window_clamp < tp->rcvq_space.space)
 					tp->rcvq_space.space = tp->window_clamp;

base-commit: 78f75d632f74b8de0f081a128588f7c37d0d1164
-- 
2.47.3


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH net] tcp: do not change rcv_ssthresh in tcp_measure_rcv_mss()
  2026-07-25  3:08 [PATCH net] tcp: do not change rcv_ssthresh in tcp_measure_rcv_mss() Nathan Gao
@ 2026-08-01  0:46 ` Jakub Kicinski
  2026-08-03  8:09   ` Paolo Abeni
  2026-08-03 21:30 ` patchwork-bot+netdevbpf
  2026-08-03 22:33 ` Nathan Gao
  2 siblings, 1 reply; 7+ messages in thread
From: Jakub Kicinski @ 2026-08-01  0:46 UTC (permalink / raw)
  To: Nathan Gao, Kuniyuki Iwashima, Paolo Abeni
  Cc: Eric Dumazet, Neal Cardwell, David S . Miller, Simon Horman,
	netdev, linux-kernel

On Fri, 24 Jul 2026 20:08:06 -0700 Nathan Gao wrote:
> Commit f5da7c45188e ("tcp: adjust rcvq_space after updating scaling
> ratio") replaced the direct window_clamp update in tcp_measure_rcv_mss()
> with a call to tcp_set_window_clamp(), a helper that implements the
> TCP_WINDOW_CLAMP setsockopt. As a side effect, the helper also shrinks
> rcv_ssthresh via __tcp_adjust_rcv_ssthresh().
> 
> As a result, each scaling_ratio decrease detected by
> tcp_measure_rcv_mss() also cuts rcv_ssthresh. Elsewhere in TCP,
> rcv_ssthresh is usually cut under memory pressure and grows via
> tcp_grow_window().
> 
> Flows whose segment sizes vary keep scaling_ratio oscillating, which
> leads to an unstable rcv_ssthresh: a dip of rcv_ssthresh only recovers
> via tcp_grow_window(), keeping the advertised window at a relatively
> low level even after the ratio itself has recovered, and can even stall
> the sender.
> 
> Observed on a customer's proxy gateway after upgrading from kernel 6.1
> to 6.12: in the worst case, rcv_ssthresh was cut in half by a
> scaling_ratio dip. P99 latency jumped from <10ms on 6.1 to ~100ms on
> 6.12, and almost returned to the 6.1 level with this patch applied.
> 
> Restore the plain WRITE_ONCE() update of window_clamp, as introduced
> in commit a2cbb1603943 ("tcp: Update window clamping condition"), and
> keep the rcvq_space.space adjustment. Now rcv_ssthresh is decoupled from
> scaling_ratio changes in tcp_measure_rcv_mss().
> 
> Fixes: f5da7c45188e ("tcp: adjust rcvq_space after updating scaling ratio")
> Signed-off-by: Nathan Gao <zcgao@amazon.com>

Not sure, I mean regression is a regression, but also the previous
behavior seems to have just been lucky rather than correct in principle?

Looks like Eric and Neal are AFK, Kuniyuki, Paolo, any opinion on this
patch?

> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index daff93d513428..5b6378b94701e 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -252,7 +252,7 @@ static void tcp_measure_rcv_mss(struct sock *sk, const struct sk_buff *skb)
>  				struct tcp_sock *tp = tcp_sk(sk);
>  
>  				val = tcp_win_from_space(sk, sk->sk_rcvbuf);
> -				tcp_set_window_clamp(sk, val);
> +				WRITE_ONCE(tp->window_clamp, val);
>  
>  				if (tp->window_clamp < tp->rcvq_space.space)
>  					tp->rcvq_space.space = tp->window_clamp;
> 
> base-commit: 78f75d632f74b8de0f081a128588f7c37d0d1164


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net] tcp: do not change rcv_ssthresh in tcp_measure_rcv_mss()
  2026-08-01  0:46 ` Jakub Kicinski
@ 2026-08-03  8:09   ` Paolo Abeni
  2026-08-03  8:18     ` Paolo Abeni
  2026-08-03 20:50     ` Kuniyuki Iwashima
  0 siblings, 2 replies; 7+ messages in thread
From: Paolo Abeni @ 2026-08-03  8:09 UTC (permalink / raw)
  To: Jakub Kicinski, Nathan Gao, Kuniyuki Iwashima
  Cc: Eric Dumazet, Neal Cardwell, David S . Miller, Simon Horman,
	netdev, linux-kernel

On 8/1/26 2:46 AM, Jakub Kicinski wrote:
> On Fri, 24 Jul 2026 20:08:06 -0700 Nathan Gao wrote:
>> Commit f5da7c45188e ("tcp: adjust rcvq_space after updating scaling
>> ratio") replaced the direct window_clamp update in tcp_measure_rcv_mss()
>> with a call to tcp_set_window_clamp(), a helper that implements the
>> TCP_WINDOW_CLAMP setsockopt. As a side effect, the helper also shrinks
>> rcv_ssthresh via __tcp_adjust_rcv_ssthresh().
>>
>> As a result, each scaling_ratio decrease detected by
>> tcp_measure_rcv_mss() also cuts rcv_ssthresh. Elsewhere in TCP,
>> rcv_ssthresh is usually cut under memory pressure and grows via
>> tcp_grow_window().
>>
>> Flows whose segment sizes vary keep scaling_ratio oscillating, which
>> leads to an unstable rcv_ssthresh: a dip of rcv_ssthresh only recovers
>> via tcp_grow_window(), keeping the advertised window at a relatively
>> low level even after the ratio itself has recovered, and can even stall
>> the sender.
>>
>> Observed on a customer's proxy gateway after upgrading from kernel 6.1
>> to 6.12: in the worst case, rcv_ssthresh was cut in half by a
>> scaling_ratio dip. P99 latency jumped from <10ms on 6.1 to ~100ms on
>> 6.12, and almost returned to the 6.1 level with this patch applied.
>>
>> Restore the plain WRITE_ONCE() update of window_clamp, as introduced
>> in commit a2cbb1603943 ("tcp: Update window clamping condition"), and
>> keep the rcvq_space.space adjustment. Now rcv_ssthresh is decoupled from
>> scaling_ratio changes in tcp_measure_rcv_mss().
>>
>> Fixes: f5da7c45188e ("tcp: adjust rcvq_space after updating scaling ratio")
>> Signed-off-by: Nathan Gao <zcgao@amazon.com>
> 
> Not sure, I mean regression is a regression, but also the previous
> behavior seems to have just been lucky rather than correct in principle?
> 
> Looks like Eric and Neal are AFK, Kuniyuki, Paolo, any opinion on this
> patch?
A quick grep confirm that except for f5da7c45188e, only the control path
calls tcp_set_window_clamp(), which IMHO supports this patch rationale.
My understanding is also that this patch should not re-introduce the
issue addressed by the blamed commit.

It would be great to have a pktdrill tests for at least one of the 2
relevant scenarios (the one described here and the one relevant for
f5da7c45188e). My totally uneducated impression is that writing a packet
drill for the case described here should be slightly less difficult than
the other option, as there is no MTU dependency.

TL;DR: I *think* this patch make sense, pktdrill would be helpful but
not a blocker.

/P


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net] tcp: do not change rcv_ssthresh in tcp_measure_rcv_mss()
  2026-08-03  8:09   ` Paolo Abeni
@ 2026-08-03  8:18     ` Paolo Abeni
  2026-08-03 20:50     ` Kuniyuki Iwashima
  1 sibling, 0 replies; 7+ messages in thread
From: Paolo Abeni @ 2026-08-03  8:18 UTC (permalink / raw)
  To: Jakub Kicinski, Nathan Gao, Kuniyuki Iwashima
  Cc: Eric Dumazet, Neal Cardwell, David S . Miller, Simon Horman,
	netdev, linux-kernel

On 8/3/26 10:09 AM, Paolo Abeni wrote:
> On 8/1/26 2:46 AM, Jakub Kicinski wrote:
>> On Fri, 24 Jul 2026 20:08:06 -0700 Nathan Gao wrote:
>>> Commit f5da7c45188e ("tcp: adjust rcvq_space after updating scaling
>>> ratio") replaced the direct window_clamp update in tcp_measure_rcv_mss()
>>> with a call to tcp_set_window_clamp(), a helper that implements the
>>> TCP_WINDOW_CLAMP setsockopt. As a side effect, the helper also shrinks
>>> rcv_ssthresh via __tcp_adjust_rcv_ssthresh().
>>>
>>> As a result, each scaling_ratio decrease detected by
>>> tcp_measure_rcv_mss() also cuts rcv_ssthresh. Elsewhere in TCP,
>>> rcv_ssthresh is usually cut under memory pressure and grows via
>>> tcp_grow_window().
>>>
>>> Flows whose segment sizes vary keep scaling_ratio oscillating, which
>>> leads to an unstable rcv_ssthresh: a dip of rcv_ssthresh only recovers
>>> via tcp_grow_window(), keeping the advertised window at a relatively
>>> low level even after the ratio itself has recovered, and can even stall
>>> the sender.
>>>
>>> Observed on a customer's proxy gateway after upgrading from kernel 6.1
>>> to 6.12: in the worst case, rcv_ssthresh was cut in half by a
>>> scaling_ratio dip. P99 latency jumped from <10ms on 6.1 to ~100ms on
>>> 6.12, and almost returned to the 6.1 level with this patch applied.
>>>
>>> Restore the plain WRITE_ONCE() update of window_clamp, as introduced
>>> in commit a2cbb1603943 ("tcp: Update window clamping condition"), and
>>> keep the rcvq_space.space adjustment. Now rcv_ssthresh is decoupled from
>>> scaling_ratio changes in tcp_measure_rcv_mss().
>>>
>>> Fixes: f5da7c45188e ("tcp: adjust rcvq_space after updating scaling ratio")
>>> Signed-off-by: Nathan Gao <zcgao@amazon.com>
>>
>> Not sure, I mean regression is a regression, but also the previous
>> behavior seems to have just been lucky rather than correct in principle?
>>
>> Looks like Eric and Neal are AFK, Kuniyuki, Paolo, any opinion on this
>> patch?
> A quick grep confirm that except for f5da7c45188e, only the control path
> calls tcp_set_window_clamp(), which IMHO supports this patch rationale.
> My understanding is also that this patch should not re-introduce the
> issue addressed by the blamed commit.
> 
> It would be great to have a pktdrill tests for at least one of the 2
> relevant scenarios (the one described here and the one relevant for
> f5da7c45188e). My totally uneducated impression is that writing a packet
> drill for the case described here should be slightly less difficult than
> the other option, as there is no MTU dependency.
> 
> TL;DR: I *think* this patch make sense, pktdrill would be helpful but
> not a blocker.
Uhm... Above I did not take in account how far we are in the current
release cycle. The issue has been unnoticed for a considerable amount of
time, and the chances the fix would introduce some other regressions are
not 0, so I think this patch would deserve at least another positive
review to be merged now.

Thanks,

Paolo


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net] tcp: do not change rcv_ssthresh in tcp_measure_rcv_mss()
  2026-08-03  8:09   ` Paolo Abeni
  2026-08-03  8:18     ` Paolo Abeni
@ 2026-08-03 20:50     ` Kuniyuki Iwashima
  1 sibling, 0 replies; 7+ messages in thread
From: Kuniyuki Iwashima @ 2026-08-03 20:50 UTC (permalink / raw)
  To: Paolo Abeni
  Cc: Jakub Kicinski, Nathan Gao, Eric Dumazet, Neal Cardwell,
	David S . Miller, Simon Horman, netdev, linux-kernel

On Mon, Aug 3, 2026 at 1:09 AM Paolo Abeni <pabeni@redhat.com> wrote:
>
> On 8/1/26 2:46 AM, Jakub Kicinski wrote:
> > On Fri, 24 Jul 2026 20:08:06 -0700 Nathan Gao wrote:
> >> Commit f5da7c45188e ("tcp: adjust rcvq_space after updating scaling
> >> ratio") replaced the direct window_clamp update in tcp_measure_rcv_mss()
> >> with a call to tcp_set_window_clamp(), a helper that implements the
> >> TCP_WINDOW_CLAMP setsockopt. As a side effect, the helper also shrinks
> >> rcv_ssthresh via __tcp_adjust_rcv_ssthresh().
> >>
> >> As a result, each scaling_ratio decrease detected by
> >> tcp_measure_rcv_mss() also cuts rcv_ssthresh. Elsewhere in TCP,
> >> rcv_ssthresh is usually cut under memory pressure and grows via
> >> tcp_grow_window().
> >>
> >> Flows whose segment sizes vary keep scaling_ratio oscillating, which
> >> leads to an unstable rcv_ssthresh: a dip of rcv_ssthresh only recovers
> >> via tcp_grow_window(), keeping the advertised window at a relatively
> >> low level even after the ratio itself has recovered, and can even stall
> >> the sender.
> >>
> >> Observed on a customer's proxy gateway after upgrading from kernel 6.1
> >> to 6.12: in the worst case, rcv_ssthresh was cut in half by a
> >> scaling_ratio dip. P99 latency jumped from <10ms on 6.1 to ~100ms on
> >> 6.12, and almost returned to the 6.1 level with this patch applied.
> >>
> >> Restore the plain WRITE_ONCE() update of window_clamp, as introduced
> >> in commit a2cbb1603943 ("tcp: Update window clamping condition"), and
> >> keep the rcvq_space.space adjustment. Now rcv_ssthresh is decoupled from
> >> scaling_ratio changes in tcp_measure_rcv_mss().
> >>
> >> Fixes: f5da7c45188e ("tcp: adjust rcvq_space after updating scaling ratio")
> >> Signed-off-by: Nathan Gao <zcgao@amazon.com>
> >
> > Not sure, I mean regression is a regression, but also the previous
> > behavior seems to have just been lucky rather than correct in principle?
> >
> > Looks like Eric and Neal are AFK, Kuniyuki, Paolo, any opinion on this
> > patch?
> A quick grep confirm that except for f5da7c45188e, only the control path
> calls tcp_set_window_clamp(), which IMHO supports this patch rationale.
> My understanding is also that this patch should not re-introduce the
> issue addressed by the blamed commit.

+1, and

>
> It would be great to have a pktdrill tests for at least one of the 2

+1.  I also told Nathan offlist that it would be nicer to write a
packetdrill test.


> relevant scenarios (the one described here and the one relevant for
> f5da7c45188e). My totally uneducated impression is that writing a packet
> drill for the case described here should be slightly less difficult than
> the other option, as there is no MTU dependency.
>
> TL;DR: I *think* this patch make sense, pktdrill would be helpful but
> not a blocker.
>
> /P
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net] tcp: do not change rcv_ssthresh in tcp_measure_rcv_mss()
  2026-07-25  3:08 [PATCH net] tcp: do not change rcv_ssthresh in tcp_measure_rcv_mss() Nathan Gao
  2026-08-01  0:46 ` Jakub Kicinski
@ 2026-08-03 21:30 ` patchwork-bot+netdevbpf
  2026-08-03 22:33 ` Nathan Gao
  2 siblings, 0 replies; 7+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-08-03 21:30 UTC (permalink / raw)
  To: Nathan Gao
  Cc: edumazet, ncardwell, kuniyu, davem, kuba, pabeni, horms, netdev,
	linux-kernel

Hello:

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

On Fri, 24 Jul 2026 20:08:06 -0700 you wrote:
> Commit f5da7c45188e ("tcp: adjust rcvq_space after updating scaling
> ratio") replaced the direct window_clamp update in tcp_measure_rcv_mss()
> with a call to tcp_set_window_clamp(), a helper that implements the
> TCP_WINDOW_CLAMP setsockopt. As a side effect, the helper also shrinks
> rcv_ssthresh via __tcp_adjust_rcv_ssthresh().
> 
> As a result, each scaling_ratio decrease detected by
> tcp_measure_rcv_mss() also cuts rcv_ssthresh. Elsewhere in TCP,
> rcv_ssthresh is usually cut under memory pressure and grows via
> tcp_grow_window().
> 
> [...]

Here is the summary with links:
  - [net] tcp: do not change rcv_ssthresh in tcp_measure_rcv_mss()
    https://git.kernel.org/netdev/net/c/0e125ecfe20c

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



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net] tcp: do not change rcv_ssthresh in tcp_measure_rcv_mss()
  2026-07-25  3:08 [PATCH net] tcp: do not change rcv_ssthresh in tcp_measure_rcv_mss() Nathan Gao
  2026-08-01  0:46 ` Jakub Kicinski
  2026-08-03 21:30 ` patchwork-bot+netdevbpf
@ 2026-08-03 22:33 ` Nathan Gao
  2 siblings, 0 replies; 7+ messages in thread
From: Nathan Gao @ 2026-08-03 22:33 UTC (permalink / raw)
  To: zcgao
  Cc: davem, edumazet, horms, kuba, kuniyu, linux-kernel, ncardwell,
	netdev, pabeni

Thanks for the review! I will try to come up with a packetdrill test
and send it to net-next.

Nathan

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-08-03 22:33 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-25  3:08 [PATCH net] tcp: do not change rcv_ssthresh in tcp_measure_rcv_mss() Nathan Gao
2026-08-01  0:46 ` Jakub Kicinski
2026-08-03  8:09   ` Paolo Abeni
2026-08-03  8:18     ` Paolo Abeni
2026-08-03 20:50     ` Kuniyuki Iwashima
2026-08-03 21:30 ` patchwork-bot+netdevbpf
2026-08-03 22:33 ` Nathan Gao

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox