Re: [PATCH net] tcp: do not change rcv_ssthresh in tcp_measure_rcv_mss()
Paolo Abeni <[email protected]>
| Newsgroups | org.kernel.vger.netdev,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
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 <[email protected]>
>
> 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