Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 41 additions & 44 deletions quinn-proto/src/connection/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -675,12 +675,12 @@ impl Connection {
self.endpoint_events
.push_back(EndpointEventInner::RetireResetToken(path_id));

let pto = self.pto_max_path(SpaceId::Data, false);

let end = self.calculate_end_timer(now, 3, SpaceId::Data);
let extended_end = self.calculate_end_timer(now, 6, SpaceId::Data);
Comment on lines +678 to +679
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let end = self.calculate_end_timer(now, 3, SpaceId::Data);
let extended_end = self.calculate_end_timer(now, 6, SpaceId::Data);
let last_allowed_receive = self.calculate_end_timer(now, 3, SpaceId::Data);
let path_discard = self.calculate_end_timer(now, 6, SpaceId::Data);

let path = self.paths.get_mut(&path_id).expect("checked above");

// We record the time after which receiving data on this path generates a transport error.
path.data.last_allowed_receive = Some(now + 3 * pto);
path.data.last_allowed_receive = Some(end);
self.abandoned_paths.insert(path_id);

self.set_max_path_id(now, self.local_max_path_id.saturating_add(1u8));
Expand All @@ -691,7 +691,7 @@ impl Connection {
// PATH_ABANDON or not.
self.timers.set(
Timer::PerPath(path_id, PathTimer::DiscardPath),
now + 6 * pto,
extended_end,
self.qlog.with_time(now),
);
Ok(())
Expand Down Expand Up @@ -2597,9 +2597,10 @@ impl Connection {
};

// QUIC-MULTIPATH § 2.5 Key Phase Update Process: use largest PTO off all paths.
let end = self.calculate_end_timer(start, 3, space);
self.timers.set(
Timer::Conn(ConnTimer::KeyDiscard),
start + self.pto_max_path(space, false) * 3,
end,
self.qlog.with_time(now),
);
}
Expand Down Expand Up @@ -3077,31 +3078,6 @@ impl Connection {
}
}

/// The maximum probe timeout across all paths
///
/// If `is_closing` is set to `true` it will filter out paths that have not yet been used.
///
/// See [`Connection::pto`]
fn pto_max_path(&self, space: SpaceId, is_closing: bool) -> Duration {
match space {
SpaceId::Initial | SpaceId::Handshake => self.pto(space, PathId::ZERO),
SpaceId::Data => self
.paths
.iter()
.filter_map(|(path_id, state)| {
if is_closing && state.data.total_sent == 0 && state.data.total_recvd == 0 {
// If we are closing and haven't sent anything yet, do not include
None
} else {
let pto = self.pto(space, *path_id);
Some(pto)
}
})
.max()
.expect("there should be one at least path"),
}
}

/// Probe Timeout
///
/// The PTO is logically the time in which you'd expect to receive an acknowledgement
Expand Down Expand Up @@ -3209,12 +3185,10 @@ impl Connection {
self.timers
.stop(Timer::Conn(ConnTimer::Idle), self.qlog.with_time(now));
} else {
let dt = cmp::max(timeout, 3 * self.pto_max_path(space, false));
self.timers.set(
Timer::Conn(ConnTimer::Idle),
now + dt,
self.qlog.with_time(now),
);
// TODO: include max(timeout)
let end = self.calculate_end_timer(now, 3, space);
self.timers
.set(Timer::Conn(ConnTimer::Idle), end, self.qlog.with_time(now));
}
}

Expand All @@ -3226,10 +3200,12 @@ impl Connection {
self.qlog.with_time(now),
);
} else {
let dt = cmp::max(timeout, 3 * self.pto(space, path_id));
let pto = self.pto(space, path_id);
let end = self.path_data(path_id).timer_offset(now, 3 * pto);
let dt = cmp::max(now + timeout, end);
self.timers.set(
Timer::PerPath(path_id, PathTimer::PathIdle),
now + dt,
dt,
self.qlog.with_time(now),
);
}
Expand Down Expand Up @@ -5710,15 +5686,36 @@ impl Connection {
self.timers.reset();
}

fn calculate_end_timer(&self, now: Instant, factor: u32, space: SpaceId) -> Instant {
match space {
SpaceId::Initial | SpaceId::Handshake => {
let duration = factor * self.pto(space, PathId::ZERO);
self.path_data(PathId::ZERO).timer_offset(now, duration)
}
SpaceId::Data => self
.paths
.iter()
.filter_map(|(path_id, state)| {
if state.data.total_sent == 0 && state.data.total_recvd == 0 {
// If we are closing and haven't sent anything yet, do not include
None
} else {
let duration = factor * self.pto(self.highest_space, *path_id);
Some(self.path_data(*path_id).timer_offset(now, duration))
}
})
.max()
.unwrap_or(now), // TODO:: reevaluate
}
}

fn set_close_timer(&mut self, now: Instant) {
// QUIC-MULTIPATH § 2.6 Connection Closure: draining for 3*PTO with PTO the max of
// the PTO for all paths.
let pto_max = self.pto_max_path(self.highest_space, true);
self.timers.set(
Timer::Conn(ConnTimer::Close),
now + 3 * pto_max,
self.qlog.with_time(now),
);

let end = self.calculate_end_timer(now, 3, self.highest_space);
self.timers
.set(Timer::Conn(ConnTimer::Close), end, self.qlog.with_time(now));
}

/// Handle transport parameters received from the peer
Expand Down
1 change: 1 addition & 0 deletions quinn-proto/src/connection/packet_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,7 @@ impl<'a, 'b> PacketBuilder<'a, 'b> {
};

conn.paths.get_mut(&path_id).unwrap().data.sent(
now,
exact_number,
packet,
conn.spaces[space_id].for_path(path_id),
Expand Down
22 changes: 21 additions & 1 deletion quinn-proto/src/connection/paths.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,8 @@ pub(super) struct PathData {
pub(super) validated: bool,
/// Total size of all UDP datagrams sent on this path
pub(super) total_sent: u64,
/// When did we last send data.
pub(super) last_sent: Option<Instant>,
/// Total size of all UDP datagrams received on this path
pub(super) total_recvd: u64,
/// The state of the MTU discovery process
Expand Down Expand Up @@ -250,6 +252,7 @@ impl PathData {
path_responses: PathResponses::default(),
validated: false,
total_sent: 0,
last_sent: None,
total_recvd: 0,
mtud: config
.mtu_discovery_config
Expand Down Expand Up @@ -305,6 +308,7 @@ impl PathData {
path_responses: PathResponses::default(),
validated: false,
total_sent: 0,
last_sent: None,
total_recvd: 0,
mtud: prev.mtud.clone(),
first_packet_after_rtt_sample: prev.first_packet_after_rtt_sample,
Expand Down Expand Up @@ -341,7 +345,14 @@ impl PathData {
}

/// Account for transmission of `packet` with number `pn` in `space`
pub(super) fn sent(&mut self, pn: u64, packet: SentPacket, space: &mut PacketNumberSpace) {
pub(super) fn sent(
&mut self,
now: Instant,
pn: u64,
packet: SentPacket,
space: &mut PacketNumberSpace,
) {
self.last_sent.replace(now);
self.in_flight.insert(&packet);
if self.first_packet.is_none() {
self.first_packet = Some(pn);
Expand All @@ -351,6 +362,15 @@ impl PathData {
}
}

pub(crate) fn timer_offset(&self, now: Instant, duration: Duration) -> Instant {
let start = self.last_sent.unwrap_or(now);
let end = start + duration;
if end > start {
Copy link
Member

@Frando Frando Dec 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be if end > now I think?

return now;
}
end
}

/// Remove `packet` with number `pn` from this path's congestion control counters, or return
/// `false` if `pn` was sent before this path was established.
pub(super) fn remove_in_flight(&mut self, packet: &SentPacket) -> bool {
Expand Down
Loading