diff --git a/noq-proto/src/connection/mod.rs b/noq-proto/src/connection/mod.rs index f56f93a78..08b7bbbe5 100644 --- a/noq-proto/src/connection/mod.rs +++ b/noq-proto/src/connection/mod.rs @@ -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, diff --git a/noq-proto/src/tests/proptests.rs b/noq-proto/src/tests/proptests.rs index ce893c51c..db1e32438 100644 --- a/noq-proto/src/tests/proptests.rs +++ b/noq-proto/src/tests/proptests.rs @@ -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)