Skip to content

quinn-proto: add TransportConfig::max_rtt to bound RTT samples#2723

Open
mjansson wants to merge 1 commit into
quinn-rs:mainfrom
mjansson:quinn-proto-max-rtt
Open

quinn-proto: add TransportConfig::max_rtt to bound RTT samples#2723
mjansson wants to merge 1 commit into
quinn-rs:mainfrom
mjansson:quinn-proto-max-rtt

Conversation

@mjansson

@mjansson mjansson commented Jul 9, 2026

Copy link
Copy Markdown

Motivation

Every RTT sample feeds straight into the smoothed RTT estimator with no upper bound. When the host is momentarily unable to process incoming packets in a timely fashion — e.g. an async runtime starved of CPU, or packets sitting unread in the socket receive buffer — an ack can be processed long after it arrived, producing a spuriously large RTT sample. That single sample poisons the smoothed estimate, which inflates the PTO, the time-based loss-detection threshold and the idle timeout, and can leave a connection unable to recover.

We observed exactly this under runtime starvation and now configure a 2s bound on both client and server connections.

Summary

  • Add a TransportConfig::max_rtt option that clamps each raw RTT sample before it enters the estimator (RttEstimator gains a max bound).
  • Defaults to 2 seconds, comfortably above any legitimate internet RTT, so existing behavior is unchanged in practice.
  • Covered by unit tests (rtt_estimator_clamps_samples_above_max, rtt_estimator_keeps_samples_below_max).

Every RTT sample feeds directly into the smoothed RTT estimator with no
upper bound. When the host is momentarily unable to process incoming
packets in a timely fashion -- for example an async runtime starved of
CPU, or packets left sitting unread in the socket receive buffer -- an
ack can be handled long after it arrived, yielding a spuriously large
RTT sample. That single sample poisons the smoothed estimate, which in
turn inflates the PTO, the time-based loss detection threshold and the
idle timeout, and can leave a connection unable to recover.

Add a TransportConfig::max_rtt option that clamps each raw sample before
it enters the estimator. It defaults to 2 seconds, comfortably above any
legitimate internet RTT, so existing behaviour is unchanged in practice.
The clamping is covered by unit tests.

Signed-off-by: Mattias Jansson <mattias.jansson@epicgames.com>
@mjansson

mjansson commented Jul 9, 2026

Copy link
Copy Markdown
Author

Linter failure is preexisting and not related to this change as far as I can tell

@Ralith

Ralith commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

Can you elaborate on "unable to recover"? It's not obvious to me why the spurious estimate wouldn't get averaged right back down with following traffic. Even if the connection is lost immediately after such an event, surely even an inflated PTO should still resolve in a reasonable amount of time? There should be a hard upper bound at which the connection becomes lost before the delayed ACK is processed anyway.

@mjansson

Copy link
Copy Markdown
Author

I'm digging up my old notes here, we had this issue ~4 months ago but due to being in crunch to get Lore released we never go around to putting up the upstream PR.

The problem we saw is when the spurious sample is not followed by clean samples. No returning ACKs → no new RTT samples → nothing that average the estimate back down. The estimator is pinned at the poisoned value precisely during the window where recovery has to happen.

The timers that are supposed to bound and repair the connection are all keyed off that poisoned estimate, including the idle timeout. In connection/mod.rs:2014 (reset_idle_timeout)

let dt = cmp::max(timeout, 3 * self.pto(space));

The effective idle timeout is max(negotiated_timeout, 3*PTO). And PTO is smoothed + max(4*rttvar, granularity). So poisoning rttvar inflates PTO, which inflates the idle-timeout floor, so the hard upper bound expected to fire moves with the poisoned sample.

Loss detection is coupled the same way with loss_delay = conservative_rtt * time_threshold (mod.rs:1703, where conservative() = max(smoothed, latest)) so genuine losses aren't declared lost for far longer, and PTO probes back off geometrically on top of the inflated base (pto_base * 2^backoff, mod.rs:1857).

Concretely, baseline RTT ~50 ms + one 20 s sample:

  • smoothed = (7*50 ms + 20 s)/8 = 20350/8 ≈ **2.54 s**
  • rttvar = (3*~20 ms + |50 ms − 20 s|)/4 = (60 + 19950)/4 ≈ **5.0 s**
  • PTO ≈ smoothed + 4*rttvar = 2.54 + 20.0 ≈ **22.5 s**
  • idle floor = 3*PTO ≈ **68 s**
  • first loss interval (uses conservative = max(smoothed, latest) → latest = 20 s): loss_delay ≈ 20 * 9/8 ≈ **22.5 s**

And since receiving any packet resets the idle timer to now + 3*PTO, a peer that keeps retransmitting can keep a barely-progressing connection pinned open well past the configured timeout.

@Ralith

Ralith commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

the hard upper bound expected to fire moves with the poisoned sample.

The hard upper bound I was referring to was the duration of delay after which the connection times out before the delayed ACK can be processed at all.

a peer that keeps retransmitting can keep a barely-progressing connection pinned open well past the configured timeout.

I don't follow. If the peer is still communicating, why shouldn't the connection remain open?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants