fix(proto): match picoquic's PTO cap logic

Replace flat 2s PTO cap with picoquic's two-tier approach
(timing.c:42-88):

1. Bind PTO to idle_timeout/16 so ~16 retransmits fit before the
   idle timer fires (only when idle_timeout > 15s)
2. Hard cap at 2s, or 1.5 * smoothed_rtt for satellite paths
   (RTT > 610ms)
3. Use max(last_ack_eliciting, now) as PTO base to prevent
   scheduling in the past

Update proptests to explicitly close connections before asserting
idle, since with the PTO cap active paths with reachable peers
correctly stay alive instead of timing out.
This commit is contained in:
dignifiedquire
2026-03-23 11:12:16 +01:00
parent 7d757368e3
commit a18eddeda9
2 changed files with 50 additions and 5 deletions
+37 -4
View File
@@ -3382,13 +3382,33 @@ impl Connection {
}
// Include max_ack_delay and backoff for ApplicationData.
duration += self.ack_frequency.max_ack_delay_for_pto() * backoff;
// Cap PTO post-handshake to prevent long retransmit gaps after connectivity loss.
duration = duration.min(MAX_PTO_INTERVAL);
// Cap PTO post-handshake, matching picoquic's
// picoquic_current_retransmit_timer() (timing.c:42-88):
//
// 1. Bind to idle_timeout / 16 so ~16 retransmits fit before
// the idle timer fires (timing.c:59-63).
if let Some(idle) = self.idle_timeout {
if idle > MIN_IDLE_FOR_PTO_CAP {
duration = duration.min(idle / IDLE_TIMEOUT_PTO_DIVISOR);
}
}
// 2. Hard cap at 2s, or 1.5 * smoothed_rtt for satellite paths
// with RTT > 610ms (timing.c:77-85).
let hard_cap = if path.rtt.get() > SATELLITE_RTT_THRESHOLD {
(path.rtt.get() * 3) / 2
} else {
MAX_PTO_INTERVAL
};
duration = duration.min(hard_cap);
}
let Some(last_ack_eliciting) = pns.time_of_last_ack_eliciting_packet else {
continue;
};
let pto = last_ack_eliciting + duration;
// Use the later of (last_ack_eliciting + duration) and (now + duration)
// to prevent the PTO from firing in the past when the cap makes duration
// small relative to how long ago the last packet was sent.
let pto = last_ack_eliciting.max(now) + duration;
if result.is_none_or(|(earliest_pto, _)| pto < earliest_pto) {
if path.anti_amplification_blocked(1) {
// Nothing would be able to be sent.
@@ -7233,9 +7253,22 @@ fn get_max_ack_delay(params: &TransportParameters) -> Duration {
// Prevents overflow and improves behavior in extreme circumstances
const MAX_BACKOFF_EXPONENT: u32 = 16;
// Cap on PTO after backoff, matching picoquic's PICOQUIC_LARGE_RETRANSMIT_TIMER.
// Hard cap on PTO after backoff, matching picoquic's PICOQUIC_LARGE_RETRANSMIT_TIMER.
const MAX_PTO_INTERVAL: Duration = Duration::from_secs(2);
// PTO is capped at idle_timeout / IDLE_TIMEOUT_PTO_DIVISOR so ~16 retransmits
// fit before the idle timer fires. Matches picoquic (idle_timeout >> 4).
const IDLE_TIMEOUT_PTO_DIVISOR: u32 = 16;
// RTT threshold above which the PTO cap uses 1.5 * smoothed_rtt instead of
// MAX_PTO_INTERVAL. Matches picoquic's PICOQUIC_TARGET_SATELLITE_RTT (610ms).
const SATELLITE_RTT_THRESHOLD: Duration = Duration::from_millis(610);
// Idle timeout must exceed this for the idle-bound PTO cap to apply.
// With short idle timeouts, the cap would make PTO too aggressive.
// Matches picoquic's guard (idle_timeout > 15).
const MIN_IDLE_FOR_PTO_CAP: Duration = Duration::from_secs(15);
/// Minimal remaining size to allow packet coalescing, excluding cryptographic tag
///
/// This must be at least as large as the header for a well-formed empty packet to be coalesced,
+13 -1
View File
@@ -11,9 +11,11 @@ use proptest::{
use test_strategy::proptest;
use tracing::error;
use bytes::Bytes;
use crate::{
Connection, ConnectionClose, ConnectionError, Event, PathStatus, Side, TransportConfig,
TransportErrorCode,
TransportErrorCode, VarInt,
tests::{
Pair, RoutingTable,
random_interaction::{TestOp, run_random_interaction},
@@ -114,6 +116,11 @@ fn random_interaction_with_multipath_simple_routing(
let (client_ch, server_ch) =
run_random_interaction(&mut pair, interactions, multipath_transport_config(prefix));
// Close the connection explicitly — with PTO caps, active paths with
// reachable peers never idle on their own (correct behavior).
let now = pair.time;
pair.client_conn_mut(client_ch)
.close(now, VarInt(0), Bytes::new());
prop_assert!(!pair.drive_bounded(1000), "connection never became idle");
prop_assert!(allowed_error(poll_to_close(
pair.client_conn_mut(client_ch)
@@ -167,6 +174,11 @@ fn random_interaction_with_multipath_complex_routing(
let (client_ch, server_ch) =
run_random_interaction(&mut pair, interactions, multipath_transport_config(prefix));
// Close the connection explicitly — with PTO caps, active paths with
// reachable peers never idle on their own (correct behavior).
let now = pair.time;
pair.client_conn_mut(client_ch)
.close(now, VarInt(0), Bytes::new());
prop_assert!(!pair.drive_bounded(1000), "connection never became idle");
prop_assert!(allowed_error(poll_to_close(
pair.client_conn_mut(client_ch)