Re: [PATCH net] tcp: do not change rcv_ssthresh in tcp_measure_rcv_mss()

[Paolo Abeni <pabeni@redhat.com>]

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