From 98efdff3f249e69dee299311b4b3b82fb0adeee7 Mon Sep 17 00:00:00 2001 From: Floris Bruynooghe Date: Fri, 20 Feb 2026 19:10:15 +0100 Subject: [PATCH 01/44] it's the weekend, this is useful in my head so let's check in totally broken code. --- quinn-proto/src/connection/mod.rs | 314 +++++++++++++++++---------- quinn-proto/src/connection/spaces.rs | 85 ++++++-- 2 files changed, 265 insertions(+), 134 deletions(-) diff --git a/quinn-proto/src/connection/mod.rs b/quinn-proto/src/connection/mod.rs index fbc3f411a..b1c1d1dc0 100644 --- a/quinn-proto/src/connection/mod.rs +++ b/quinn-proto/src/connection/mod.rs @@ -312,57 +312,6 @@ pub struct Connection { qlog: QlogSink, } -/// Return value for [`Connection::poll_transmit_path`]. -#[derive(Debug)] -enum PollPathStatus { - /// Nothing to send on the path, nothing was written into the [`TransmitBuf`]. - NothingToSend { - /// If true there was data to send but congestion control did not allow so. - congestion_blocked: bool, - }, - /// The transmit is ready to be sent. - Send(Transmit), -} - -/// Return value for [`Connection::poll_transmit_path_space`]. -#[derive(Debug)] -enum PollPathSpaceStatus { - /// Nothing to send in the space, nothing was written into the [`TransmitBuf`]. - NothingToSend { - /// If true there was data to send but congestion control did not allow so. - congestion_blocked: bool, - }, - /// One or more packets have been written into the [`TransmitBuf`]. - WrotePacket { - /// The highest packet number. - last_packet_number: u64, - /// Whether to pad an already started datagram in the next packet. - /// - /// When packets in Initial, 0-RTT or Handshake packet do not fill the entire - /// datagram they may decide to coalesce with the next packet from a higher - /// encryption level on the same path. But the earlier packet may require specific - /// size requirements for the datagram they are sent in. - /// - /// If a space did not complete the datagram, they use this to request the correct - /// padding in the final packet of the datagram so that the final datagram will have - /// the correct size. - /// - /// If a space did fill an entire datagram, it leaves this to the default of - /// [`PadDatagram::No`]. - pad_datagram: PadDatagram, - }, - /// Send the contents of the transmit immediately. - /// - /// Packets were written and the GSO batch must end now, regardless from whether higher - /// spaces still have frames to write. This is used when the last datagram written would - /// require too much padding to continue a GSO batch, which would waste space on the - /// wire. - Send { - /// The highest packet number written into the transmit. - last_packet_number: u64, - }, -} - impl Connection { pub(crate) fn new( endpoint_config: Arc, @@ -1027,33 +976,21 @@ impl Connection { && self.peer_supports_ack_frequency(); } - // TODO(flub): path scheduling logic might be buggy if there are only un-validated - // paths and PATH_STATUS_BACKUP paths. - - // Path scheduling logic is currently as such: - // - // - For any un-validated paths we only send frames that *must* be sent on that - // path. E.g. PATH_CHALLENGE, PATH_RESPONSE. - // - // - If there are any validated paths with CIDs and PathStatus::Available: - // - Frames that can be sent on any path, e.g. STREAM, DATAGRAM, are only sent on - // these available paths. - // - All other paths only send frames that *must* be sent on those paths, - // e.g. PATH_CHALLENGE, PATH_RESPONSE, tail-loss probes, keep alive PING. - // - // - If there are no validated paths with CIDs and PathStatus::Available all frames - // are sent on the earlierst possible path. - // - // For all this we use the *path_exclusive_only* boolean: If set to true, only - // frames that must be sent on the path will be built into the packet. - - // Is there any open, validated and status available path with dst CIDs? If so we'll - // want to set path_exclusive_only for any other paths. - let have_available_path = self.paths.iter().any(|(id, path)| { - path.data.validated - && path.data.local_status() == PathStatus::Available - && self.remote_cids.contains_key(id) - }); + let scheduling_info: BTreeMap = self + .paths + .iter() + .map(|(path_id, path)| { + ( + *path_id, + PathSchedulingInfo { + has_cids: self.remote_cids.contains_key(&path_id), + validated: path.data.validated, + abandoned: self.abandoned_paths.contains(&path_id), + status: path.data.local_status(), + }, + ) + }) + .collect(); // TODO: how to avoid the allocation? Cannot use a for loop because of // borrowing. Maybe SmallVec or similar. @@ -1076,7 +1013,7 @@ impl Connection { buf, path_id, max_datagrams, - have_available_path, + scheduling_info, connection_close_pending, ) { PollPathStatus::Send(transmit) => { @@ -1191,7 +1128,7 @@ impl Connection { buf: &mut Vec, path_id: PathId, max_datagrams: NonZeroUsize, - have_available_path: bool, + scheduling_info: BTreeMap, connection_close_pending: bool, ) -> PollPathStatus { // Check if there is at least one active CID to use for sending @@ -1236,7 +1173,7 @@ impl Connection { path_id, space_id, remote_cid, - have_available_path, + scheduling_info, connection_close_pending, pad_datagram, ) { @@ -1302,8 +1239,7 @@ impl Connection { path_id: PathId, space_id: SpaceId, remote_cid: ConnectionId, - // If any other packet space has a usable path with PathStatus::Available. - have_available_path: bool, + scheduling_info: BTreeMap, // If we need to send a CONNECTION_CLOSE frame. connection_close_pending: bool, // Whether the current datagram needs to be padded to a certain size. @@ -1340,22 +1276,99 @@ impl Connection { let can_send = self.space_can_send(space_id, path_id, max_packet_size, connection_close_pending); - // Whether we would like to send any frames on this packet space. See the packet - // scheduling described in poll_transmit. - let space_should_send = { - let path_exclusive_only = space_id == SpaceId::Data - && have_available_path - && self.path_data(path_id).local_status() == PathStatus::Backup; - let path_should_send = if path_exclusive_only { - can_send.path_exclusive - } else { - !can_send.is_empty() - }; - let needs_loss_probe = self.spaces[space_id].for_path(path_id).loss_probes > 0; - path_should_send || needs_loss_probe + let will_space_send = || { + let this_path = scheduling_info.get(&path_id).unwrap(); + if !this_path.has_cids { + // Without CIDs we can not send anything. + return false; + } + if this_path.abandoned { + // We only ever set a path to abandoned once we no longer wish to send + // anything on it. If we ever want to send PATH_ABANDON on the abandoned + // path, we will not set the path as abandoned until we have sent that + // frame. This is how the spec describes that situation. So we never + // send on an abandoned path. + return false; + } + if can_send.validation { + // If we need to send stuff to validate the path, we're definitely + // sending. + return true; + } + + if !this_path.validated { + // If we don't have a validated path, we only send things when we also + // have to send a PATH_CHALLENGE or PATH_RESPONSE. It is totally allowed + // to start sending anything and everything on an unvalidated path + // already, as long as you don't exceed any anti-amplification + // limit. But we also open a lot of paths which we don't expect to work + // out for nat traversal and doing so would result in a lot of lost data + // that needs to be resent. And it is nicer to keep the + // anti-amplification budget for actual path validation. + // + // Even if there is only a single path, this postpones sending anything + // else on the path until validation succeeds. + if can_send.close { + // However, if we have to send CONNECTION_CLOSE then we may have to + // send it on this path if there is nowhere better to send it. + // + // - a non-abandoned, validated, status-available path is better + // - a non-abandoned, validated, status-backup path is 2nd best + // - a non-abandoned, non-validated path is same-same + // - an abandoned path is always worse + // + // If an earlier path was better it would have already send this + // CONNECTION_CLOSE, unless it was congestion-blocked. In that case + // we still prefer to send on that earlier path and would rather + // postpone this to the next poll_transmit call. We also can not be + // better than ourselves, so there is no need to filter by PathId. + let have_better_path = scheduling_info + .values() + .any(|info| info.has_cids && !info.abandoned && info.validated); + return !have_better_path; + } + return false; + } + + // + // Now the current path has a CID, is not abandoned and is validated. + // + + if can_send.space_id_only { + // If there is anything that can only be sent on this path, we will be + // sending it. + return true; + } + + // + // Now we only have things to send, which can conceivably be sent in any space. + // + + match this_path.status { + PathStatus::Available => { + // If there is anything to send, it will be sent. + return !can_send.is_empty(); + } + PathStatus::Backup => { + // If there is a better path to send on, we should prefer to send on + // that. If an earlier path was better it would already have sent, + // unless it was congestion blocked. In that case we prefer to send + // nothing and wait for a future call to poll_transmit so we can + // keep sending on that better path. Also no need to filter out this + // path, because we can not be better than ourselves. + let have_better_path = scheduling_info.values().any(|info| { + info.has_cids + && !info.abandoned + && info.validated + && info.status == PathStatus::Available + }); + return !have_better_path; + } + } }; + let space_will_send = will_space_send(); - if !space_should_send { + if !space_will_send { // Nothing more to send. Previous iterations of this loop may have built // packets already. return match last_packet_number { @@ -1476,7 +1489,7 @@ impl Connection { path_id, remote_cid, transmit, - can_send.other, + can_send.is_ack_eliciting(), self, ) else { // Confidentiality limit is exceeded and the connection has been killed. We @@ -1491,7 +1504,9 @@ impl Connection { }; last_packet_number = Some(builder.packet_number); - if space_id == SpaceId::Initial && (self.side.is_client() || can_send.other) { + if space_id == SpaceId::Initial + && (self.side.is_client() || can_send.is_ack_eliciting()) + { // https://www.rfc-editor.org/rfc/rfc9000.html#section-14.1 pad_datagram |= PadDatagram::ToMinMtu; } @@ -1598,7 +1613,7 @@ impl Connection { debug_assert!( !(builder.sent_frames().is_ack_only(&self.streams) && !can_send.acks - && can_send.other + && (can_send.other || can_send.validation || can_send.space_id_only) && builder.buf.segment_size() == self.path_data(path_id).current_mtu() as usize && self.datagrams.outgoing.is_empty()), @@ -2025,7 +2040,9 @@ impl Connection { }) } - /// Indicate what types of frames are ready to send for the given space + /// Indicate what types of frames are ready to send for the given space. + /// + /// Only for on-path data. /// /// *packet_size* is the number of bytes available to build the next packet. /// *connection_close_pending* indicates whether a CONNECTION_CLOSE frame needs to be @@ -6470,7 +6487,7 @@ impl Connection { } } - /// Whether we have 1-RTT data to send + /// Whether we have on-path 1-RTT data to send. /// /// This checks for frames that can only be sent in the data space (1-RTT): /// - Pending PATH_CHALLENGE frames on the active and previous path if just migrated. @@ -6481,25 +6498,25 @@ impl Connection { /// See also [`PacketSpace::can_send`] which keeps track of all other frame types that /// may need to be sent. fn can_send_1rtt(&self, path_id: PathId, max_size: usize) -> SendableFrames { - let path_exclusive = self.paths.get(&path_id).is_some_and(|path| { - path.data.send_new_challenge - || path - .prev - .as_ref() - .is_some_and(|(_, path)| path.send_new_challenge) - || !path.data.path_responses.is_empty() + let validation = self.paths.get(&path_id).is_some_and(|path| { + path.data.send_new_challenge || !path.data.path_responses.is_empty() }); + + // Stream control frames are checked in PacketSpace::can_send, only check data here. let other = self.streams.can_send_stream_data() || self .datagrams .outgoing .front() .is_some_and(|x| x.size(true) <= max_size); + + // All `false` fields are set in PacketSpace::can_send. SendableFrames { acks: false, - other, close: false, - path_exclusive, + validation, + space_id_only: false, + other, } } @@ -6839,6 +6856,81 @@ impl fmt::Debug for Connection { } } +/// Return value for [`Connection::poll_transmit_path`]. +#[derive(Debug)] +enum PollPathStatus { + /// Nothing to send on the path, nothing was written into the [`TransmitBuf`]. + NothingToSend { + /// If true there was data to send but congestion control did not allow so. + congestion_blocked: bool, + }, + /// The transmit is ready to be sent. + Send(Transmit), +} + +/// Return value for [`Connection::poll_transmit_path_space`]. +#[derive(Debug)] +enum PollPathSpaceStatus { + /// Nothing to send in the space, nothing was written into the [`TransmitBuf`]. + NothingToSend { + /// If true there was data to send but congestion control did not allow so. + congestion_blocked: bool, + }, + /// One or more packets have been written into the [`TransmitBuf`]. + WrotePacket { + /// The highest packet number. + last_packet_number: u64, + /// Whether to pad an already started datagram in the next packet. + /// + /// When packets in Initial, 0-RTT or Handshake packet do not fill the entire + /// datagram they may decide to coalesce with the next packet from a higher + /// encryption level on the same path. But the earlier packet may require specific + /// size requirements for the datagram they are sent in. + /// + /// If a space did not complete the datagram, they use this to request the correct + /// padding in the final packet of the datagram so that the final datagram will have + /// the correct size. + /// + /// If a space did fill an entire datagram, it leaves this to the default of + /// [`PadDatagram::No`]. + pad_datagram: PadDatagram, + }, + /// Send the contents of the transmit immediately. + /// + /// Packets were written and the GSO batch must end now, regardless from whether higher + /// spaces still have frames to write. This is used when the last datagram written would + /// require too much padding to continue a GSO batch, which would waste space on the + /// wire. + Send { + /// The highest packet number written into the transmit. + last_packet_number: u64, + }, +} + +struct SchedulingInfo { + paths: BTreeSet, + current: PathId, +} + +impl SchedulingInfo {} + +/// TODO(flub): this really is duplicate info, but it makes it easy to reason about what +/// info is used for scheduling right now. +#[derive(Debug)] +struct PathSchedulingInfo { + /// Whether the path has an active CID. + has_cids: bool, + /// Whether the path is validated. + validated: bool, + /// Whether the path is abandoned. + /// + /// Note that a path that is abandoned but still has CIDs can still send a packet. After + /// sending that packet the CIDs have to be considered retired as well. + abandoned: bool, + /// The status of the path. + status: PathStatus, +} + #[derive(Debug, Copy, Clone, PartialEq, Eq)] enum PathBlocked { No, diff --git a/quinn-proto/src/connection/spaces.rs b/quinn-proto/src/connection/spaces.rs index 5acc8c4a5..346402f82 100644 --- a/quinn-proto/src/connection/spaces.rs +++ b/quinn-proto/src/connection/spaces.rs @@ -11,7 +11,7 @@ use rustc_hash::{FxHashMap, FxHashSet}; use sorted_index_buffer::SortedIndexBuffer; use tracing::trace; -use super::PathId; +use super::{PathData, PathId}; use crate::{ Dir, Duration, FourTuple, Instant, StreamId, TransportError, TransportErrorCode, VarInt, connection::StreamsState, @@ -151,16 +151,17 @@ impl PacketSpace { .number_spaces .values() .any(|pns| pns.pending_acks.can_send()); - let path_exclusive = self + let space_id_only = self .number_spaces .get(&path_id) .is_some_and(|s| s.ping_pending || s.immediate_ack_pending); - let other = !self.pending.is_empty(streams) || path_exclusive; + let other = !self.pending.is_empty(streams) || space_id_only; SendableFrames { acks, - other, close: false, - path_exclusive, + validation: false, // see Connection::can_send_1rtt + space_id_only, + other, } } } @@ -811,21 +812,26 @@ impl Dedup { } } -/// Indicates which data is available for sending +/// Indicates which data is available for sending. +/// +/// This applies to a particular space ID that was queried. #[derive(Clone, Copy, PartialEq, Eq, Debug)] pub(super) struct SendableFrames { - /// Whether there ACK frames to send, these are not ack-eliciting + /// Whether there are ACK frames to send, these are not ack-eliciting. pub(super) acks: bool, - /// Whether there are any other frames to send, these are ack-eliciting - pub(super) other: bool, - /// Whether there is a CONNECTION_CLOSE to send, this is not ack-eliciting + /// Whether there is a CONNECTION_CLOSE to send, this is not ack-eliciting. pub(super) close: bool, - /// Whether there are frames to send, which can only be sent on the path queried + /// Whether there are any frames to send to validate a path, these are ack-eliciting. + /// + /// These are frames such as PATH_CHALLENGE and PATH_RESPONSE. + pub(super) validation: bool, + /// Whether there are any frames that must be sent on this specific space ID. /// - /// These are ack-eliciting, and a subset of [`SendableFrames::other`]. This is useful - /// for QUIC-MULTIPATH, which may desire not to send any frames on a backup path, which - /// can also be sent on an active path. - pub(super) path_exclusive: bool, + /// These are ack-eliciting. Some frames are scheduled per path, e.g. PING or + /// IMMEDIATE_ACK. + pub(super) space_id_only: bool, + /// Whether there are any other frames to send, these are ack-eliciting. + pub(super) other: bool, } impl SendableFrames { @@ -833,24 +839,57 @@ impl SendableFrames { pub(super) fn empty() -> Self { Self { acks: false, - other: false, close: false, - path_exclusive: false, + validation: false, + space_id_only: false, + other: false, } } - /// Whether no data is sendable + /// Whether an ack-eliciting packet will be sent. + pub(super) fn is_ack_eliciting(&self) -> bool { + let Self { + acks, + close, + validation, + space_id_only, + other, + } = *self; + if close { + // No ack-eliciting frames are included with a CONNECTION_CLOSE, only acks. + return false; + } + validation || space_id_only || other + } + + /// Whether no data is sendable. pub(super) fn is_empty(&self) -> bool { - !self.acks && !self.other && !self.close && !self.path_exclusive + let Self { + acks, + close, + validation, + space_id_only, + other, + } = *self; + !acks && !close && !validation && !space_id_only && !other } } impl ::std::ops::BitOrAssign for SendableFrames { fn bitor_assign(&mut self, rhs: Self) { - self.acks |= rhs.acks; - self.other |= rhs.other; - self.close |= rhs.close; - self.path_exclusive |= rhs.path_exclusive; + let Self { + acks, + close, + validation, + space_id_only, + other, + } = rhs; + + self.acks |= acks; + self.close |= close; + self.validation |= validation; + self.space_id_only |= space_id_only; + self.other |= other; } } From f5691196af361093b104a9448eee25fcc376bd71 Mon Sep 17 00:00:00 2001 From: Floris Bruynooghe Date: Mon, 23 Feb 2026 15:29:29 +0100 Subject: [PATCH 02/44] more --- quinn-proto/src/connection/mod.rs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/quinn-proto/src/connection/mod.rs b/quinn-proto/src/connection/mod.rs index b1c1d1dc0..23521c3da 100644 --- a/quinn-proto/src/connection/mod.rs +++ b/quinn-proto/src/connection/mod.rs @@ -1597,12 +1597,13 @@ impl Connection { }; } - // If this boolean is true we only want to send frames which can not be sent on - // any other path. See the path scheduling notes in Self::poll_transmit. - let path_exclusive_only = - have_available_path && self.path_data(path_id).local_status() == PathStatus::Backup; - - self.populate_packet(now, space_id, path_id, path_exclusive_only, &mut builder); + self.populate_packet( + now, + space_id, + path_id, + scheduling_info.get(path_id).unwrap(), + &mut builder, + ); // ACK-only packets should only be sent when explicitly allowed. If we write them due to // any other reason, there is a bug which leads to one component announcing write @@ -5593,7 +5594,7 @@ impl Connection { now: Instant, space_id: SpaceId, path_id: PathId, - path_exclusive_only: bool, + scheduling_info: &PathSchedulingInfo, builder: &mut PacketBuilder<'a, 'b>, ) { let pn = builder.packet_number; From 295de00a80fc2367fd20b93bad5a933cc5c6ae23 Mon Sep 17 00:00:00 2001 From: Floris Bruynooghe Date: Tue, 24 Feb 2026 12:19:01 +0100 Subject: [PATCH 03/44] wip --- quinn-proto/src/connection/mod.rs | 38 +++++++++++++++++++++++-------- 1 file changed, 29 insertions(+), 9 deletions(-) diff --git a/quinn-proto/src/connection/mod.rs b/quinn-proto/src/connection/mod.rs index 627908de4..a8e5d789c 100644 --- a/quinn-proto/src/connection/mod.rs +++ b/quinn-proto/src/connection/mod.rs @@ -1636,13 +1636,7 @@ impl Connection { }; } - self.populate_packet( - now, - space_id, - path_id, - scheduling_info.get(path_id).unwrap(), - &mut builder, - ); + self.populate_packet(now, space_id, path_id, scheduling_info, &mut builder); // ACK-only packets should only be sent when explicitly allowed. If we write them due to // any other reason, there is a bug which leads to one component announcing write @@ -5599,9 +5593,11 @@ impl Connection { now: Instant, space_id: SpaceId, path_id: PathId, - scheduling_info: &PathSchedulingInfo, + scheduling_info: BTreeMap, builder: &mut PacketBuilder<'a, 'b>, ) { + let this_path = scheduling_info.get(&path_id).unwrap(); + debug_assert!(this_path.has_cids, "must have CIDs to populate packet"); let pn = builder.packet_number; let is_multipath_negotiated = self.is_multipath_negotiated(); let space_has_keys = self.crypto_state.has_keys(space_id.encryption_level()); @@ -6940,12 +6936,36 @@ struct PathSchedulingInfo { /// Whether the path is abandoned. /// /// Note that a path that is abandoned but still has CIDs can still send a packet. After - /// sending that packet the CIDs have to be considered retired as well. + /// sending that packet the CIDs have to be considered retired as well and + /// [`Self::has_cids`] should turn `false`. abandoned: bool, /// The status of the path. status: PathStatus, } +fn should_send_data(all: &BTreeMap, current: PathId) -> bool { + // To send SpaceKind::Data we want a path: + // - with CIDs + // - validated + // - not abandoned + // - status-available unless there is no such path + let this = all.get(¤t).unwrap(); + if !this.has_cids || !this.validated || this.abandoned { + return false; + } + if this.status == PathStatus::Available { + return true; + } + !data_space_available(all) +} + +/// Whether there is any path that can send SpaceKind::Data with PathStatus::Available +fn data_space_available(all: &BTreeMap) -> bool { + all.values() + .filter(|info| info.has_cids && info.validated && !info.abandoned) + .any(|info| info.status == PathStatus::Available) +} + #[derive(Debug, Copy, Clone, PartialEq, Eq)] enum PathBlocked { No, From 876fbc8cd914a8d32f67e3b9685b7e85e76e4e95 Mon Sep 17 00:00:00 2001 From: Floris Bruynooghe Date: Tue, 24 Feb 2026 13:26:57 +0100 Subject: [PATCH 04/44] wip --- quinn-proto/src/connection/mod.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/quinn-proto/src/connection/mod.rs b/quinn-proto/src/connection/mod.rs index a8e5d789c..6b0f7ebe4 100644 --- a/quinn-proto/src/connection/mod.rs +++ b/quinn-proto/src/connection/mod.rs @@ -5598,7 +5598,6 @@ impl Connection { ) { let this_path = scheduling_info.get(&path_id).unwrap(); debug_assert!(this_path.has_cids, "must have CIDs to populate packet"); - let pn = builder.packet_number; let is_multipath_negotiated = self.is_multipath_negotiated(); let space_has_keys = self.crypto_state.has_keys(space_id.encryption_level()); let is_0rtt = space_id == SpaceId::Data && !space_has_keys; @@ -5612,7 +5611,7 @@ impl Connection { // HANDSHAKE_DONE if !is_0rtt - && !path_exclusive_only + && should_send_data(&scheduling_info, path_id) && mem::replace(&mut space.pending.handshake_done, false) { builder.write_frame(frame::HandshakeDone, stats); @@ -5620,7 +5619,7 @@ impl Connection { // REACH_OUT if let Some((round, addresses)) = space.pending.reach_out.as_mut() - && !path_exclusive_only + && should_send_data(&scheduling_info, path_id) { while let Some(local_addr) = addresses.iter().next().copied() { let local_addr = addresses.take(&local_addr).expect("found from iter"); @@ -5638,7 +5637,7 @@ impl Connection { } // OBSERVED_ADDR - if !path_exclusive_only + if should_send_data(&scheduling_info, path_id) && space_id == SpaceId::Data && self .config @@ -5659,6 +5658,7 @@ impl Connection { } // PING + // TODO(flub): continue here if mem::replace(&mut space.for_path(path_id).ping_pending, false) { builder.write_frame(frame::Ping, stats); } @@ -5721,7 +5721,7 @@ impl Connection { builder.write_frame(frame, stats); self.ack_frequency - .ack_frequency_sent(path_id, pn, max_ack_delay); + .ack_frequency_sent(path_id, builder.packet_number, max_ack_delay); } // PATH_CHALLENGE @@ -6919,8 +6919,11 @@ enum PollPathSpaceStatus { } struct SchedulingInfo { - paths: BTreeSet, + paths: BTreeMap, current: PathId, + // is_multipath_negotiated + // is_0rtt + // space_has_keys } impl SchedulingInfo {} From 2a152770eb48e5efe1388353285e6f7d93c89585 Mon Sep 17 00:00:00 2001 From: Floris Bruynooghe Date: Fri, 27 Feb 2026 10:49:20 +0100 Subject: [PATCH 05/44] wip --- quinn-proto/src/connection/mod.rs | 47 +++++++++++++++++++++---------- 1 file changed, 32 insertions(+), 15 deletions(-) diff --git a/quinn-proto/src/connection/mod.rs b/quinn-proto/src/connection/mod.rs index 6b0f7ebe4..ae07c1a22 100644 --- a/quinn-proto/src/connection/mod.rs +++ b/quinn-proto/src/connection/mod.rs @@ -5658,7 +5658,6 @@ impl Connection { } // PING - // TODO(flub): continue here if mem::replace(&mut space.for_path(path_id).ping_pending, false) { builder.write_frame(frame::Ping, stats); } @@ -5674,9 +5673,7 @@ impl Connection { } // ACK - // TODO(flub): Should this send acks for this path anyway? - - if !path_exclusive_only { + if should_send_data(&scheduling_info, path_id) { for path_id in space .number_spaces .iter_mut() @@ -5696,10 +5693,29 @@ impl Connection { space_has_keys, ); } + } else if this_path.abandoned { + // If we are abandoning this very path, also include ACKs for this path, + // together with the PATH_ABANDON that will be sent. Normally we don't do this + // though and send the PATH_ABANDON on a different path. + if !space.for_path(path_id).pending_acks.ranges().is_empty() { + Self::populate_acks( + now, + self.receiving_ecn, + path_id, + space_id, + space, + is_multipath_negotiated, + builder, + stats, + space_has_keys, + ) + } } // ACK_FREQUENCY - if !path_exclusive_only && mem::replace(&mut space.pending.ack_frequency, false) { + if should_send_data(&scheduling_info, path_id) + && mem::replace(&mut space.pending.ack_frequency, false) + { let sequence_number = self.ack_frequency.next_sequence_number(); // Safe to unwrap because this is always provided when ACK frequency is enabled @@ -5811,9 +5827,9 @@ impl Connection { } // CRYPTO - while !path_exclusive_only + while !is_0rtt + && (space_id < SpaceId::Data || should_send_data(&scheduling_info, path_id)) && builder.frame_space_remaining() > frame::Crypto::SIZE_BOUND - && !is_0rtt { let mut frame = match space.pending.crypto.pop_front() { Some(x) => x, @@ -5848,8 +5864,8 @@ impl Connection { // TODO(flub): maybe this is much higher priority? // PATH_ABANDON - while !path_exclusive_only - && space_id == SpaceId::Data + while space_id == SpaceId::Data + && should_send_data(&scheduling_info, path_id) && frame::PathAbandon::SIZE_BOUND <= builder.frame_space_remaining() { let Some((abandoned_path_id, error_code)) = space.pending.path_abandon.pop_first() @@ -5864,8 +5880,8 @@ impl Connection { } // PATH_STATUS_AVAILABLE & PATH_STATUS_BACKUP - while !path_exclusive_only - && space_id == SpaceId::Data + while space_id == SpaceId::Data + && should_send_data(&scheduling_info, path_id) && frame::PathStatusAvailable::SIZE_BOUND <= builder.frame_space_remaining() { let Some(path_id) = space.pending.path_status.pop_first() else { @@ -5897,7 +5913,7 @@ impl Connection { // MAX_PATH_ID if space_id == SpaceId::Data - && !path_exclusive_only + && should_send_data(&scheduling_info, path_id) && space.pending.max_path_id && frame::MaxPathId::SIZE_BOUND <= builder.frame_space_remaining() { @@ -5908,7 +5924,7 @@ impl Connection { // PATHS_BLOCKED if space_id == SpaceId::Data - && !path_exclusive_only + && should_send_data(&scheduling_info, path_id) && space.pending.paths_blocked && frame::PathsBlocked::SIZE_BOUND <= builder.frame_space_remaining() { @@ -5919,7 +5935,7 @@ impl Connection { // PATH_CIDS_BLOCKED while space_id == SpaceId::Data - && !path_exclusive_only + && should_send_data(&scheduling_info, path_id) && frame::PathCidsBlocked::SIZE_BOUND <= builder.frame_space_remaining() { let Some(path_id) = space.pending.path_cids_blocked.pop_first() else { @@ -5933,8 +5949,9 @@ impl Connection { builder.write_frame(frame, stats); } + // TODO(flub): continue here // RESET_STREAM, STOP_SENDING, MAX_DATA, MAX_STREAM_DATA, MAX_STREAMS - if space_id == SpaceId::Data && !path_exclusive_only { + if space_id == SpaceId::Data && should_send_data(&scheduling_info, path_id) { self.streams .write_control_frames(builder, &mut space.pending, stats); } From 6cc0671f72a32494e527ee347846e7d40b6f4065 Mon Sep 17 00:00:00 2001 From: Floris Bruynooghe Date: Fri, 27 Feb 2026 16:20:43 +0100 Subject: [PATCH 06/44] try more stuff --- quinn-proto/src/connection/mod.rs | 201 +++++++++------------------ quinn-proto/src/connection/spaces.rs | 4 +- 2 files changed, 67 insertions(+), 138 deletions(-) diff --git a/quinn-proto/src/connection/mod.rs b/quinn-proto/src/connection/mod.rs index ae07c1a22..134049991 100644 --- a/quinn-proto/src/connection/mod.rs +++ b/quinn-proto/src/connection/mod.rs @@ -1052,7 +1052,7 @@ impl Connection { buf, path_id, max_datagrams, - scheduling_info, + &scheduling_info, connection_close_pending, ) { PollPathStatus::Send(transmit) => { @@ -1167,7 +1167,7 @@ impl Connection { buf: &mut Vec, path_id: PathId, max_datagrams: NonZeroUsize, - scheduling_info: BTreeMap, + scheduling_info: &BTreeMap, connection_close_pending: bool, ) -> PollPathStatus { // Check if there is at least one active CID to use for sending @@ -1278,7 +1278,7 @@ impl Connection { path_id: PathId, space_id: SpaceId, remote_cid: ConnectionId, - scheduling_info: BTreeMap, + scheduling_info: &BTreeMap, // If we need to send a CONNECTION_CLOSE frame. connection_close_pending: bool, // Whether the current datagram needs to be padded to a certain size. @@ -1319,93 +1319,66 @@ impl Connection { let this_path = scheduling_info.get(&path_id).unwrap(); if !this_path.has_cids { // Without CIDs we can not send anything. + trace!(?path_id, "no destination CIDs available"); return false; } if this_path.abandoned { - // We only ever set a path to abandoned once we no longer wish to send - // anything on it. If we ever want to send PATH_ABANDON on the abandoned - // path, we will not set the path as abandoned until we have sent that - // frame. This is how the spec describes that situation. So we never - // send on an abandoned path. + // Don't currently send on an abandoned path. We could in the future use + // this to decide to send PATH_ABANDON on the abandoned path. + trace!(?path_id, "path abandoned"); return false; } - if can_send.validation { + // TODO: these two probably need to be merged? + if can_send.validation || can_send.space_id_only { // If we need to send stuff to validate the path, we're definitely // sending. return true; } - if !this_path.validated { - // If we don't have a validated path, we only send things when we also - // have to send a PATH_CHALLENGE or PATH_RESPONSE. It is totally allowed - // to start sending anything and everything on an unvalidated path - // already, as long as you don't exceed any anti-amplification - // limit. But we also open a lot of paths which we don't expect to work - // out for nat traversal and doing so would result in a lot of lost data - // that needs to be resent. And it is nicer to keep the - // anti-amplification budget for actual path validation. - // - // Even if there is only a single path, this postpones sending anything - // else on the path until validation succeeds. - if can_send.close { - // However, if we have to send CONNECTION_CLOSE then we may have to - // send it on this path if there is nowhere better to send it. - // - // - a non-abandoned, validated, status-available path is better - // - a non-abandoned, validated, status-backup path is 2nd best - // - a non-abandoned, non-validated path is same-same - // - an abandoned path is always worse - // - // If an earlier path was better it would have already send this - // CONNECTION_CLOSE, unless it was congestion-blocked. In that case - // we still prefer to send on that earlier path and would rather - // postpone this to the next poll_transmit call. We also can not be - // better than ourselves, so there is no need to filter by PathId. - let have_better_path = scheduling_info - .values() - .any(|info| info.has_cids && !info.abandoned && info.validated); - return !have_better_path; - } - return false; - } - - // - // Now the current path has a CID, is not abandoned and is validated. - // - - if can_send.space_id_only { - // If there is anything that can only be sent on this path, we will be - // sending it. - return true; - } - - // - // Now we only have things to send, which can conceivably be sent in any space. - // + // From here on we only want to send on this path, if there is no better + // path available. It does not matter if the better path was already checked + // in this poll_transmit call however, because we would never have made it + // past there. We could go out of our way to debug_assert that. - match this_path.status { - PathStatus::Available => { - // If there is anything to send, it will be sent. - return !can_send.is_empty(); - } - PathStatus::Backup => { - // If there is a better path to send on, we should prefer to send on - // that. If an earlier path was better it would already have sent, - // unless it was congestion blocked. In that case we prefer to send - // nothing and wait for a future call to poll_transmit so we can - // keep sending on that better path. Also no need to filter out this - // path, because we can not be better than ourselves. - let have_better_path = scheduling_info.values().any(|info| { - info.has_cids - && !info.abandoned - && info.validated - && info.status == PathStatus::Available - }); - return !have_better_path; + if !this_path.validated { + let have_better_path = scheduling_info.values().any(|info| { + // We do not want to send if there is a better path to send on. + let PathSchedulingInfo { + has_cids, + validated, + abandoned, + status: _, + } = *info; + has_cids && !abandoned && validated + }); + return !have_better_path; + } else { + // This path is validated, a better path depends on the status. + match this_path.status { + PathStatus::Available => { + // Can't get better than this. + return true; + } + PathStatus::Backup => { + let have_better_path = scheduling_info.values().any(|info| { + let PathSchedulingInfo { + has_cids, + validated, + abandoned, + status, + } = *info; + has_cids + && !abandoned + && validated + && status == PathStatus::Available + }); + return !have_better_path; + } } } }; let space_will_send = will_space_send(); + tracing::warn!(?can_send, ?space_will_send, "can_send"); if !space_will_send { // Nothing more to send. Previous iterations of this loop may have built @@ -5593,7 +5566,7 @@ impl Connection { now: Instant, space_id: SpaceId, path_id: PathId, - scheduling_info: BTreeMap, + scheduling_info: &BTreeMap, builder: &mut PacketBuilder<'a, 'b>, ) { let this_path = scheduling_info.get(&path_id).unwrap(); @@ -5949,7 +5922,6 @@ impl Connection { builder.write_frame(frame, stats); } - // TODO(flub): continue here // RESET_STREAM, STOP_SENDING, MAX_DATA, MAX_STREAM_DATA, MAX_STREAMS if space_id == SpaceId::Data && should_send_data(&scheduling_info, path_id) { self.streams @@ -5965,7 +5937,9 @@ impl Connection { .expect("some local CID state must exist"); let new_cid_size_bound = frame::NewConnectionId::size_bound(is_multipath_negotiated, cid_len); - while !path_exclusive_only && builder.frame_space_remaining() > new_cid_size_bound { + while should_send_data(&scheduling_info, path_id) + && builder.frame_space_remaining() > new_cid_size_bound + { let issued = match space.pending.new_cids.pop() { Some(x) => x, None => break, @@ -5995,7 +5969,9 @@ impl Connection { // RETIRE_CONNECTION_ID let retire_cid_bound = frame::RetireConnectionId::size_bound(is_multipath_negotiated); - while !path_exclusive_only && builder.frame_space_remaining() > retire_cid_bound { + while should_send_data(&scheduling_info, path_id) + && builder.frame_space_remaining() > retire_cid_bound + { let (path_id, sequence) = match space.pending.retire_cids.pop() { Some((PathId::ZERO, seq)) if !is_multipath_negotiated => (None, seq), Some((path_id, seq)) => (Some(path_id), seq), @@ -6007,7 +5983,7 @@ impl Connection { // DATAGRAM let mut sent_datagrams = false; - while !path_exclusive_only + while should_send_data(&scheduling_info, path_id) && builder.frame_space_remaining() > Datagram::SIZE_BOUND && space_id == SpaceId::Data { @@ -6026,7 +6002,7 @@ impl Connection { let path = &mut self.paths.get_mut(&path_id).expect("known path").data; // NEW_TOKEN - if !path_exclusive_only { + if should_send_data(&scheduling_info, path_id) { while let Some(network_path) = space.pending.new_tokens.pop() { debug_assert_eq!(space_id, SpaceId::Data); let ConnectionSide::Server { server_config } = &self.side else { @@ -6063,14 +6039,14 @@ impl Connection { } // STREAM - if !path_exclusive_only && space_id == SpaceId::Data { + if should_send_data(&scheduling_info, path_id) && space_id == SpaceId::Data { self.streams .write_stream_frames(builder, self.config.send_fairness, stats); } // ADD_ADDRESS while space_id == SpaceId::Data - && !path_exclusive_only + && should_send_data(&scheduling_info, path_id) && frame::AddAddress::SIZE_BOUND <= builder.frame_space_remaining() { if let Some(added_address) = space.pending.add_address.pop_last() { @@ -6082,7 +6058,7 @@ impl Connection { // REMOVE_ADDRESS while space_id == SpaceId::Data - && !path_exclusive_only + && should_send_data(&scheduling_info, path_id) && frame::RemoveAddress::SIZE_BOUND <= builder.frame_space_remaining() { if let Some(removed_address) = space.pending.remove_address.pop_last() { @@ -6884,57 +6860,6 @@ enum PollPathSpaceStatus { }, } -/// Return value for [`Connection::poll_transmit_path`]. -#[derive(Debug)] -enum PollPathStatus { - /// Nothing to send on the path, nothing was written into the [`TransmitBuf`]. - NothingToSend { - /// If true there was data to send but congestion control did not allow so. - congestion_blocked: bool, - }, - /// The transmit is ready to be sent. - Send(Transmit), -} - -/// Return value for [`Connection::poll_transmit_path_space`]. -#[derive(Debug)] -enum PollPathSpaceStatus { - /// Nothing to send in the space, nothing was written into the [`TransmitBuf`]. - NothingToSend { - /// If true there was data to send but congestion control did not allow so. - congestion_blocked: bool, - }, - /// One or more packets have been written into the [`TransmitBuf`]. - WrotePacket { - /// The highest packet number. - last_packet_number: u64, - /// Whether to pad an already started datagram in the next packet. - /// - /// When packets in Initial, 0-RTT or Handshake packet do not fill the entire - /// datagram they may decide to coalesce with the next packet from a higher - /// encryption level on the same path. But the earlier packet may require specific - /// size requirements for the datagram they are sent in. - /// - /// If a space did not complete the datagram, they use this to request the correct - /// padding in the final packet of the datagram so that the final datagram will have - /// the correct size. - /// - /// If a space did fill an entire datagram, it leaves this to the default of - /// [`PadDatagram::No`]. - pad_datagram: PadDatagram, - }, - /// Send the contents of the transmit immediately. - /// - /// Packets were written and the GSO batch must end now, regardless from whether higher - /// spaces still have frames to write. This is used when the last datagram written would - /// require too much padding to continue a GSO batch, which would waste space on the - /// wire. - Send { - /// The highest packet number written into the transmit. - last_packet_number: u64, - }, -} - struct SchedulingInfo { paths: BTreeMap, current: PathId, @@ -6947,7 +6872,7 @@ impl SchedulingInfo {} /// TODO(flub): this really is duplicate info, but it makes it easy to reason about what /// info is used for scheduling right now. -#[derive(Debug)] +#[derive(Debug, Copy, Clone)] struct PathSchedulingInfo { /// Whether the path has an active CID. has_cids: bool, @@ -6963,6 +6888,10 @@ struct PathSchedulingInfo { status: PathStatus, } +/// Whether we should send data on the `current` path. +/// +/// Here "data" means any frames which may be sent on any path. It does not necessarily +/// match to the [`SpaceKind::Data`] spaces, e.g. frames sent in 0-RTT also count. fn should_send_data(all: &BTreeMap, current: PathId) -> bool { // To send SpaceKind::Data we want a path: // - with CIDs diff --git a/quinn-proto/src/connection/spaces.rs b/quinn-proto/src/connection/spaces.rs index be30c143f..e78d67d0f 100644 --- a/quinn-proto/src/connection/spaces.rs +++ b/quinn-proto/src/connection/spaces.rs @@ -11,7 +11,7 @@ use rustc_hash::{FxHashMap, FxHashSet}; use sorted_index_buffer::SortedIndexBuffer; use tracing::trace; -use super::{PathData, PathId}; +use super::PathId; use crate::{ Dir, Duration, FourTuple, Instant, StreamId, TransportError, TransportErrorCode, VarInt, connection::StreamsState, @@ -896,7 +896,7 @@ impl SendableFrames { /// Whether an ack-eliciting packet will be sent. pub(super) fn is_ack_eliciting(&self) -> bool { let Self { - acks, + acks: _, close, validation, space_id_only, From b86ddad911263bf6e6f4eff2b13eab09fab23abf Mon Sep 17 00:00:00 2001 From: Floris Bruynooghe Date: Fri, 27 Feb 2026 16:33:49 +0100 Subject: [PATCH 07/44] moar --- quinn-proto/src/connection/mod.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/quinn-proto/src/connection/mod.rs b/quinn-proto/src/connection/mod.rs index 134049991..5e1132080 100644 --- a/quinn-proto/src/connection/mod.rs +++ b/quinn-proto/src/connection/mod.rs @@ -1799,10 +1799,11 @@ impl Connection { loop { let can_send = self.space_can_send( space_id, - PathId::ZERO, + PathId::ZERO, // TODO: why only PathId 0????? max_packet_size, connection_close_pending, ); + tracing::warn!(?can_send, ?space_id, "has_pending_packet"); if !can_send.is_empty() { return true; } From 8a0b779d20f54b47dcb376aecfaef75f2f4471d1 Mon Sep 17 00:00:00 2001 From: Floris Bruynooghe Date: Mon, 2 Mar 2026 11:38:18 +0100 Subject: [PATCH 08/44] more wip --- quinn-proto/src/connection/mod.rs | 39 +++++++++++++++++++++---------- 1 file changed, 27 insertions(+), 12 deletions(-) diff --git a/quinn-proto/src/connection/mod.rs b/quinn-proto/src/connection/mod.rs index 5e1132080..baccc131a 100644 --- a/quinn-proto/src/connection/mod.rs +++ b/quinn-proto/src/connection/mod.rs @@ -6900,20 +6900,35 @@ fn should_send_data(all: &BTreeMap, current: PathId) // - not abandoned // - status-available unless there is no such path let this = all.get(¤t).unwrap(); - if !this.has_cids || !this.validated || this.abandoned { + if !this.has_cids || this.abandoned { return false; } - if this.status == PathStatus::Available { - return true; - } - !data_space_available(all) -} - -/// Whether there is any path that can send SpaceKind::Data with PathStatus::Available -fn data_space_available(all: &BTreeMap) -> bool { - all.values() - .filter(|info| info.has_cids && info.validated && !info.abandoned) - .any(|info| info.status == PathStatus::Available) + let have_better_path = if !this.validated { + all.values().any(|info| { + let PathSchedulingInfo { + has_cids, + validated, + abandoned, + status: _, + } = *info; + has_cids && !abandoned && validated + }) + } else { + // path is validated + match this.status { + PathStatus::Available => return true, + PathStatus::Backup => all.values().any(|info| { + let PathSchedulingInfo { + has_cids, + validated, + abandoned, + status, + } = *info; + has_cids && !abandoned && validated && status == PathStatus::Available + }), + } + }; + !have_better_path } #[derive(Debug, Copy, Clone, PartialEq, Eq)] From ff11b1c69b15c27f90963169076e9d868dbd2b9d Mon Sep 17 00:00:00 2001 From: Floris Bruynooghe Date: Mon, 2 Mar 2026 17:11:15 +0100 Subject: [PATCH 09/44] can't send anything when there's nothing to send This stopped us from going to the next space to coalesce the Initial with the Handshake packet on the server-side. And probably lots of other stuff --- noq-proto/src/connection/mod.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/noq-proto/src/connection/mod.rs b/noq-proto/src/connection/mod.rs index 509890c21..482e8c9d4 100644 --- a/noq-proto/src/connection/mod.rs +++ b/noq-proto/src/connection/mod.rs @@ -1308,6 +1308,9 @@ impl Connection { self.space_can_send(space_id, path_id, max_packet_size, connection_close_pending); let will_space_send = || { + if can_send.is_empty() { + return false; + } let this_path = scheduling_info.get(&path_id).unwrap(); if !this_path.has_cids { // Without CIDs we can not send anything. From 651a1eba6fe8dbdaff2e5533957a7a5046efee15 Mon Sep 17 00:00:00 2001 From: Floris Bruynooghe Date: Mon, 2 Mar 2026 17:38:45 +0100 Subject: [PATCH 10/44] wip --- noq-proto/src/connection/mod.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/noq-proto/src/connection/mod.rs b/noq-proto/src/connection/mod.rs index 482e8c9d4..35e4bde79 100644 --- a/noq-proto/src/connection/mod.rs +++ b/noq-proto/src/connection/mod.rs @@ -1373,7 +1373,7 @@ impl Connection { } }; let space_will_send = will_space_send(); - tracing::warn!(?can_send, ?space_will_send, "can_send"); + tracing::warn!(?space_id, ?path_id, ?can_send, ?space_will_send, "can_send"); if !space_will_send { // Nothing more to send. Previous iterations of this loop may have built @@ -1798,7 +1798,6 @@ impl Connection { max_packet_size, connection_close_pending, ); - tracing::warn!(?can_send, ?space_id, "has_pending_packet"); if !can_send.is_empty() { return true; } From cec40b266637af7778ae5095e77591f986d6980e Mon Sep 17 00:00:00 2001 From: Floris Bruynooghe Date: Mon, 2 Mar 2026 17:56:17 +0100 Subject: [PATCH 11/44] and check for loss probes that need to be sent --- noq-proto/src/connection/mod.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/noq-proto/src/connection/mod.rs b/noq-proto/src/connection/mod.rs index 35e4bde79..be48154e7 100644 --- a/noq-proto/src/connection/mod.rs +++ b/noq-proto/src/connection/mod.rs @@ -1372,7 +1372,8 @@ impl Connection { } } }; - let space_will_send = will_space_send(); + let needs_loss_probe = self.spaces[space_id].for_path(path_id).loss_probes > 0; + let space_will_send = will_space_send() || needs_loss_probe; tracing::warn!(?space_id, ?path_id, ?can_send, ?space_will_send, "can_send"); if !space_will_send { @@ -1445,6 +1446,7 @@ impl Connection { // We need something to send for a tail-loss probe. let request_immediate_ack = space_id == SpaceId::Data && self.peer_supports_ack_frequency(); + // TODO(flub): this is really scheduling logic hiding here. self.spaces[space_id].maybe_queue_probe( path_id, request_immediate_ack, From 8b605f0949462f13b61b794ef9ebe140d079c6c0 Mon Sep 17 00:00:00 2001 From: Floris Bruynooghe Date: Tue, 3 Mar 2026 17:58:16 +0100 Subject: [PATCH 12/44] some doc updates --- noq-proto/src/connection/mod.rs | 1 - noq-proto/src/connection/spaces.rs | 4 +++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/noq-proto/src/connection/mod.rs b/noq-proto/src/connection/mod.rs index be48154e7..9754e61d3 100644 --- a/noq-proto/src/connection/mod.rs +++ b/noq-proto/src/connection/mod.rs @@ -1374,7 +1374,6 @@ impl Connection { }; let needs_loss_probe = self.spaces[space_id].for_path(path_id).loss_probes > 0; let space_will_send = will_space_send() || needs_loss_probe; - tracing::warn!(?space_id, ?path_id, ?can_send, ?space_will_send, "can_send"); if !space_will_send { // Nothing more to send. Previous iterations of this loop may have built diff --git a/noq-proto/src/connection/spaces.rs b/noq-proto/src/connection/spaces.rs index 854d8653d..c0c4334f5 100644 --- a/noq-proto/src/connection/spaces.rs +++ b/noq-proto/src/connection/spaces.rs @@ -143,9 +143,11 @@ impl PacketSpace { /// Whether there is anything to send in this space /// - /// For the data space [`Connection::can_send_1rtt`] also needs to be consulted. + /// For the data space [`Connection::can_send_1rtt`] also needs to be consulted. Prefer + /// to use [`Connection::space_can_send`] which handles this. /// /// [`Connection::can_send_1rtt`]: super::Connection::can_send_1rtt + /// [`Connection::space_can_send`]: super::Connection::space_can_send pub(super) fn can_send(&self, path_id: PathId, streams: &StreamsState) -> SendableFrames { let acks = self .number_spaces From cb9e52aa685a7606f179a832f5f143274b59fc73 Mon Sep 17 00:00:00 2001 From: Floris Bruynooghe Date: Tue, 3 Mar 2026 18:22:39 +0100 Subject: [PATCH 13/44] remove SendableFrames::validation, it was redundant Turns out this was exactly the same case as SendableFrames::space_id_only. --- noq-proto/src/connection/mod.rs | 10 ++++------ noq-proto/src/connection/spaces.rs | 20 +++++--------------- 2 files changed, 9 insertions(+), 21 deletions(-) diff --git a/noq-proto/src/connection/mod.rs b/noq-proto/src/connection/mod.rs index 9754e61d3..a02e7551d 100644 --- a/noq-proto/src/connection/mod.rs +++ b/noq-proto/src/connection/mod.rs @@ -1323,8 +1323,7 @@ impl Connection { trace!(?path_id, "path abandoned"); return false; } - // TODO: these two probably need to be merged? - if can_send.validation || can_send.space_id_only { + if can_send.space_id_only { // If we need to send stuff to validate the path, we're definitely // sending. return true; @@ -1616,7 +1615,7 @@ impl Connection { debug_assert!( !(builder.sent_frames().is_ack_only(&self.streams) && !can_send.acks - && (can_send.other || can_send.validation || can_send.space_id_only) + && (can_send.other || can_send.space_id_only) && builder.buf.segment_size() == self.path_data(path_id).current_mtu() as usize && self.datagrams.outgoing.is_empty()), @@ -6457,7 +6456,7 @@ impl Connection { /// See also [`PacketSpace::can_send`] which keeps track of all other frame types that /// may need to be sent. fn can_send_1rtt(&self, path_id: PathId, max_size: usize) -> SendableFrames { - let validation = self.paths.get(&path_id).is_some_and(|path| { + let space_id_only = self.paths.get(&path_id).is_some_and(|path| { path.data.pending_on_path_challenge || !path.data.path_responses.is_empty() }); @@ -6473,8 +6472,7 @@ impl Connection { SendableFrames { acks: false, close: false, - validation, - space_id_only: false, + space_id_only, other, } } diff --git a/noq-proto/src/connection/spaces.rs b/noq-proto/src/connection/spaces.rs index c0c4334f5..5cdfda5fa 100644 --- a/noq-proto/src/connection/spaces.rs +++ b/noq-proto/src/connection/spaces.rs @@ -161,7 +161,6 @@ impl PacketSpace { SendableFrames { acks, close: false, - validation: false, // see Connection::can_send_1rtt space_id_only, other, } @@ -863,21 +862,17 @@ impl Dedup { /// Indicates which data is available for sending. /// -/// This applies to a particular space ID that was queried. +/// This applies to a particular space ID that was queried and all refers to on-path data. #[derive(Clone, Copy, PartialEq, Eq, Debug)] pub(super) struct SendableFrames { /// Whether there are ACK frames to send, these are not ack-eliciting. pub(super) acks: bool, /// Whether there is a CONNECTION_CLOSE to send, this is not ack-eliciting. pub(super) close: bool, - /// Whether there are any frames to send to validate a path, these are ack-eliciting. - /// - /// These are frames such as PATH_CHALLENGE and PATH_RESPONSE. - pub(super) validation: bool, /// Whether there are any frames that must be sent on this specific space ID. /// - /// These are ack-eliciting. Some frames are scheduled per path, e.g. PING or - /// IMMEDIATE_ACK. + /// These are ack-eliciting. Some frames are scheduled per path, e.g. PING, + /// IMMEDIATE_ACK, PATH_CHALLENGE or PATH_RESPONSE. pub(super) space_id_only: bool, /// Whether there are any other frames to send, these are ack-eliciting. pub(super) other: bool, @@ -889,7 +884,6 @@ impl SendableFrames { Self { acks: false, close: false, - validation: false, space_id_only: false, other: false, } @@ -900,7 +894,6 @@ impl SendableFrames { let Self { acks: _, close, - validation, space_id_only, other, } = *self; @@ -908,7 +901,7 @@ impl SendableFrames { // No ack-eliciting frames are included with a CONNECTION_CLOSE, only acks. return false; } - validation || space_id_only || other + space_id_only || other } /// Whether no data is sendable. @@ -916,11 +909,10 @@ impl SendableFrames { let Self { acks, close, - validation, space_id_only, other, } = *self; - !acks && !close && !validation && !space_id_only && !other + !acks && !close && !space_id_only && !other } } @@ -929,14 +921,12 @@ impl ::std::ops::BitOrAssign for SendableFrames { let Self { acks, close, - validation, space_id_only, other, } = rhs; self.acks |= acks; self.close |= close; - self.validation |= validation; self.space_id_only |= space_id_only; self.other |= other; } From 42bc677e795a51934fb71496d41b67f4158b8fbc Mon Sep 17 00:00:00 2001 From: Floris Bruynooghe Date: Wed, 4 Mar 2026 15:50:46 +0100 Subject: [PATCH 14/44] Add a test that checks we send on an available path --- noq-proto/src/tests/multipath.rs | 40 ++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/noq-proto/src/tests/multipath.rs b/noq-proto/src/tests/multipath.rs index 172c1a584..2239f3d75 100644 --- a/noq-proto/src/tests/multipath.rs +++ b/noq-proto/src/tests/multipath.rs @@ -986,3 +986,43 @@ fn path_open_deadline_is_set_on_send() -> TestResult { Ok(()) } + +#[test] +fn path_scheduling_path_status() -> TestResult { + let _guard = subscribe(); + let mut pair = multipath_pair(); + + info!("Setting Path 0 to PathStatus::Backup"); + let prev_status = pair.set_path_status(Client, PathId::ZERO, PathStatus::Backup)?; + assert_eq!(prev_status, PathStatus::Available); + + // Send the frame to the server + pair.drive(); + + assert_eq!( + pair.remote_path_status(Server, PathId::ZERO), + Some(PathStatus::Backup) + ); + + info!("Opening Path 1 with PathStatus::Available"); + let server_addr = pair.addrs_to_server(); + let path_1 = pair.open_path(Client, server_addr, PathStatus::Available)?; + pair.drive(); + + let stats_path0_t0 = pair.conn_mut(Client).path_stats(PathId::ZERO).unwrap(); + let stats_path1_t0 = pair.conn_mut(Client).path_stats(path_1).unwrap(); + + info!("Sending STREAM frame"); + let s = pair.streams(Client).open(Dir::Uni).unwrap(); + pair.send_stream(Client, s).write(b"hello").unwrap(); + pair.drive(); + + let stats_path0_t1 = pair.conn_mut(Client).path_stats(PathId::ZERO).unwrap(); + let stats_path1_t1 = pair.conn_mut(Client).path_stats(path_1).unwrap(); + + info!("assert"); + assert!((stats_path0_t1.udp_tx.datagrams - stats_path0_t0.udp_tx.datagrams) == 0); + assert!((stats_path1_t1.udp_tx.datagrams - stats_path1_t0.udp_tx.datagrams) > 0); + + Ok(()) +} From e9832764982048515e9d70cde799c2ff72068814 Mon Sep 17 00:00:00 2001 From: Floris Bruynooghe Date: Wed, 4 Mar 2026 15:51:23 +0100 Subject: [PATCH 15/44] simplify the packet scheduling logic a little --- noq-proto/src/connection/mod.rs | 55 ++++++++++--------------------- noq-proto/src/connection/paths.rs | 2 +- 2 files changed, 19 insertions(+), 38 deletions(-) diff --git a/noq-proto/src/connection/mod.rs b/noq-proto/src/connection/mod.rs index a02e7551d..b11ab29a2 100644 --- a/noq-proto/src/connection/mod.rs +++ b/noq-proto/src/connection/mod.rs @@ -1314,7 +1314,7 @@ impl Connection { let this_path = scheduling_info.get(&path_id).unwrap(); if !this_path.has_cids { // Without CIDs we can not send anything. - trace!(?path_id, "no destination CIDs available"); + trace!(?path_id, "no CIDs available"); return false; } if this_path.abandoned { @@ -1333,43 +1333,24 @@ impl Connection { // path available. It does not matter if the better path was already checked // in this poll_transmit call however, because we would never have made it // past there. We could go out of our way to debug_assert that. - - if !this_path.validated { - let have_better_path = scheduling_info.values().any(|info| { - // We do not want to send if there is a better path to send on. - let PathSchedulingInfo { - has_cids, - validated, - abandoned, - status: _, - } = *info; - has_cids && !abandoned && validated - }); - return !have_better_path; - } else { - // This path is validated, a better path depends on the status. - match this_path.status { - PathStatus::Available => { - // Can't get better than this. - return true; - } - PathStatus::Backup => { - let have_better_path = scheduling_info.values().any(|info| { - let PathSchedulingInfo { - has_cids, - validated, - abandoned, - status, - } = *info; - has_cids - && !abandoned - && validated - && status == PathStatus::Available - }); - return !have_better_path; + let have_better_path = scheduling_info.values().any(|info| { + let PathSchedulingInfo { + has_cids, + validated, + abandoned, + status, + } = *info; + has_cids + && !abandoned + && if !this_path.validated { + // Any validated path is better than an non-validated path. + validated + } else { + // A status-available path is better than status-backup path. + validated && status < this_path.status } - } - } + }); + !have_better_path }; let needs_loss_probe = self.spaces[space_id].for_path(path_id).loss_probes > 0; let space_will_send = will_space_send() || needs_loss_probe; diff --git a/noq-proto/src/connection/paths.rs b/noq-proto/src/connection/paths.rs index 81e06626c..e804dba8d 100644 --- a/noq-proto/src/connection/paths.rs +++ b/noq-proto/src/connection/paths.rs @@ -950,7 +950,7 @@ impl PathStatusState { /// See section "3.3 Path Status Management": /// #[cfg_attr(test, derive(test_strategy::Arbitrary))] -#[derive(Debug, Copy, Clone, Default, PartialEq, Eq)] +#[derive(Debug, Copy, Clone, Default, PartialEq, Eq, PartialOrd, Ord)] pub enum PathStatus { /// Paths marked with as available will be used when scheduling packets /// From 580e0445f5cd1b35963c50c49df51322c9977498 Mon Sep 17 00:00:00 2001 From: Floris Bruynooghe Date: Fri, 6 Mar 2026 17:28:51 +0100 Subject: [PATCH 16/44] Simplify, immediate close during handshake is broken though --- noq-proto/src/connection/mod.rs | 286 +++++++++++++------------------- 1 file changed, 115 insertions(+), 171 deletions(-) diff --git a/noq-proto/src/connection/mod.rs b/noq-proto/src/connection/mod.rs index b11ab29a2..c7dfc90e1 100644 --- a/noq-proto/src/connection/mod.rs +++ b/noq-proto/src/connection/mod.rs @@ -1007,21 +1007,63 @@ impl Connection { && self.peer_supports_ack_frequency(); } - let scheduling_info: BTreeMap = self - .paths - .iter() - .map(|(path_id, path)| { - ( - *path_id, - PathSchedulingInfo { - has_cids: self.remote_cids.contains_key(&path_id), - validated: path.data.validated, - abandoned: self.abandoned_paths.contains(&path_id), - status: path.data.local_status(), - }, - ) - }) - .collect(); + // Build up some packet scheduling information about all paths. + let scheduling_info: BTreeMap = { + let have_validated_status_available_space = self.paths.iter().any(|(path_id, path)| { + self.remote_cids.contains_key(path_id) + && !self.abandoned_paths.contains(path_id) + && path.data.validated + && path.data.local_status() == PathStatus::Available + }); + let is_handshaking = self.is_handshaking(); + tracing::warn!(?is_handshaking); + self.paths + .iter() + .map(|(path_id, path)| { + let has_cids = self.remote_cids.contains_key(&path_id); + let validated = path.data.validated; + let abandoned = self.abandoned_paths.contains(&path_id); + let status = path.data.local_status(); + + // This is the core packet scheduling, whether this space ID may send + // SpaceKind::Data frames. + let may_send_data = has_cids + && !abandoned + && if is_handshaking { + // There is only one path during the handshake. We want to + // already send 0-RTT and 0.5-RTT (permitting anti-amplification + // limit) data. + true + } else if !validated { + // TODO(flub): When we have a network change we might end up + // having to abandon all paths and re-open new ones to the + // same remotes. This leaves us without any validated + // path. Perhaps we should have a way to figure out if the + // path is to a previously-validated remote address and allow + // sending data to such remotes immediately. + false + } else { + match status { + PathStatus::Available => { + // Best possible space to send data on. + true + } + PathStatus::Backup => { + // If there is an status-available path we prefer that. + !have_validated_status_available_space + } + } + }; + ( + *path_id, + PathSchedulingInfo { + abandoned, + may_send_data, + }, + ) + }) + .collect() + }; // TODO: how to avoid the allocation? Cannot use a for loop because of // borrowing. Maybe SmallVec or similar. @@ -1031,7 +1073,7 @@ impl Connection { // nothing to send or because we were congestion blocked. let mut congestion_blocked = false; - for &path_id in &path_ids { + for (&path_id, info) in scheduling_info.iter() { if !connection_close_pending && let Some(transmit) = self.poll_transmit_off_path(now, buf, path_id) { @@ -1044,7 +1086,7 @@ impl Connection { buf, path_id, max_datagrams, - &scheduling_info, + &info, connection_close_pending, ) { PollPathStatus::Send(transmit) => { @@ -1159,7 +1201,7 @@ impl Connection { buf: &mut Vec, path_id: PathId, max_datagrams: NonZeroUsize, - scheduling_info: &BTreeMap, + scheduling_info: &PathSchedulingInfo, connection_close_pending: bool, ) -> PollPathStatus { // Check if there is at least one active CID to use for sending @@ -1270,7 +1312,7 @@ impl Connection { path_id: PathId, space_id: SpaceId, remote_cid: ConnectionId, - scheduling_info: &BTreeMap, + scheduling_info: &PathSchedulingInfo, // If we need to send a CONNECTION_CLOSE frame. connection_close_pending: bool, // Whether the current datagram needs to be padded to a certain size. @@ -1296,7 +1338,7 @@ impl Connection { // space. The TransmitBuf will contain a started datagram with space if // coalescing, or completely filled datagram if not coalescing. loop { - // Determine if anything can be sent in this packet number space (SpaceId + PathId). + // Determine if anything can be sent in this packet number space. let max_packet_size = if transmit.datagram_remaining_mut() > 0 { // A datagram is started already, we are coalescing another packet into it. transmit.datagram_remaining_mut() @@ -1306,54 +1348,25 @@ impl Connection { }; let can_send = self.space_can_send(space_id, path_id, max_packet_size, connection_close_pending); - - let will_space_send = || { - if can_send.is_empty() { - return false; - } - let this_path = scheduling_info.get(&path_id).unwrap(); - if !this_path.has_cids { - // Without CIDs we can not send anything. - trace!(?path_id, "no CIDs available"); - return false; - } - if this_path.abandoned { - // Don't currently send on an abandoned path. We could in the future use - // this to decide to send PATH_ABANDON on the abandoned path. - trace!(?path_id, "path abandoned"); - return false; - } - if can_send.space_id_only { - // If we need to send stuff to validate the path, we're definitely - // sending. - return true; - } - - // From here on we only want to send on this path, if there is no better - // path available. It does not matter if the better path was already checked - // in this poll_transmit call however, because we would never have made it - // past there. We could go out of our way to debug_assert that. - let have_better_path = scheduling_info.values().any(|info| { - let PathSchedulingInfo { - has_cids, - validated, - abandoned, - status, - } = *info; - has_cids - && !abandoned - && if !this_path.validated { - // Any validated path is better than an non-validated path. - validated - } else { - // A status-available path is better than status-backup path. - validated && status < this_path.status - } - }); - !have_better_path - }; let needs_loss_probe = self.spaces[space_id].for_path(path_id).loss_probes > 0; - let space_will_send = will_space_send() || needs_loss_probe; + let space_will_send = { + if scheduling_info.abandoned { + // Currently we don't send on an abandoned path, PATH_ABANDON is always + // sent on a different path. + false + } else if needs_loss_probe { + // We always send if a loss probe if the path is not abandoned. + true + } else if can_send.space_id_only { + // We always send space-specific frames if not abandoned. + true + } else { + // Anything else we only send if we're the best path for SpaceKind::Data + // frames. + !can_send.is_empty() && scheduling_info.may_send_data + } + }; + tracing::warn!(?can_send, ?scheduling_info, ?space_will_send, "checking"); if !space_will_send { // Nothing more to send. Previous iterations of this loop may have built @@ -5533,11 +5546,9 @@ impl Connection { now: Instant, space_id: SpaceId, path_id: PathId, - scheduling_info: &BTreeMap, + scheduling_info: &PathSchedulingInfo, builder: &mut PacketBuilder<'a, 'b>, ) { - let this_path = scheduling_info.get(&path_id).unwrap(); - debug_assert!(this_path.has_cids, "must have CIDs to populate packet"); let is_multipath_negotiated = self.is_multipath_negotiated(); let space_has_keys = self.crypto_state.has_keys(space_id.encryption_level()); let is_0rtt = space_id == SpaceId::Data && !space_has_keys; @@ -5551,7 +5562,7 @@ impl Connection { // HANDSHAKE_DONE if !is_0rtt - && should_send_data(&scheduling_info, path_id) + && scheduling_info.may_send_data && mem::replace(&mut space.pending.handshake_done, false) { builder.write_frame(frame::HandshakeDone, stats); @@ -5559,7 +5570,7 @@ impl Connection { // REACH_OUT if let Some((round, addresses)) = space.pending.reach_out.as_mut() - && should_send_data(&scheduling_info, path_id) + && scheduling_info.may_send_data { while let Some(local_addr) = addresses.iter().next().copied() { let local_addr = addresses.take(&local_addr).expect("found from iter"); @@ -5577,7 +5588,7 @@ impl Connection { } // OBSERVED_ADDR - if should_send_data(&scheduling_info, path_id) + if scheduling_info.may_send_data && space_id == SpaceId::Data && self .config @@ -5613,7 +5624,7 @@ impl Connection { } // ACK - if should_send_data(&scheduling_info, path_id) { + if scheduling_info.may_send_data { for path_id in space .number_spaces .iter_mut() @@ -5633,29 +5644,10 @@ impl Connection { space_has_keys, ); } - } else if this_path.abandoned { - // If we are abandoning this very path, also include ACKs for this path, - // together with the PATH_ABANDON that will be sent. Normally we don't do this - // though and send the PATH_ABANDON on a different path. - if !space.for_path(path_id).pending_acks.ranges().is_empty() { - Self::populate_acks( - now, - self.receiving_ecn, - path_id, - space_id, - space, - is_multipath_negotiated, - builder, - stats, - space_has_keys, - ) - } } // ACK_FREQUENCY - if should_send_data(&scheduling_info, path_id) - && mem::replace(&mut space.pending.ack_frequency, false) - { + if scheduling_info.may_send_data && mem::replace(&mut space.pending.ack_frequency, false) { let sequence_number = self.ack_frequency.next_sequence_number(); // Safe to unwrap because this is always provided when ACK frequency is enabled @@ -5777,7 +5769,7 @@ impl Connection { // CRYPTO while !is_0rtt - && (space_id < SpaceId::Data || should_send_data(&scheduling_info, path_id)) + && (space_id < SpaceId::Data || scheduling_info.may_send_data) && builder.frame_space_remaining() > frame::Crypto::SIZE_BOUND { let mut frame = match space.pending.crypto.pop_front() { @@ -5814,7 +5806,7 @@ impl Connection { // TODO(flub): maybe this is much higher priority? // PATH_ABANDON while space_id == SpaceId::Data - && should_send_data(&scheduling_info, path_id) + && scheduling_info.may_send_data && frame::PathAbandon::SIZE_BOUND <= builder.frame_space_remaining() { let Some((abandoned_path_id, error_code)) = space.pending.path_abandon.pop_first() @@ -5830,7 +5822,7 @@ impl Connection { // PATH_STATUS_AVAILABLE & PATH_STATUS_BACKUP while space_id == SpaceId::Data - && should_send_data(&scheduling_info, path_id) + && scheduling_info.may_send_data && frame::PathStatusAvailable::SIZE_BOUND <= builder.frame_space_remaining() { let Some(path_id) = space.pending.path_status.pop_first() else { @@ -5862,7 +5854,7 @@ impl Connection { // MAX_PATH_ID if space_id == SpaceId::Data - && should_send_data(&scheduling_info, path_id) + && scheduling_info.may_send_data && space.pending.max_path_id && frame::MaxPathId::SIZE_BOUND <= builder.frame_space_remaining() { @@ -5873,7 +5865,7 @@ impl Connection { // PATHS_BLOCKED if space_id == SpaceId::Data - && should_send_data(&scheduling_info, path_id) + && scheduling_info.may_send_data && space.pending.paths_blocked && frame::PathsBlocked::SIZE_BOUND <= builder.frame_space_remaining() { @@ -5884,7 +5876,7 @@ impl Connection { // PATH_CIDS_BLOCKED while space_id == SpaceId::Data - && should_send_data(&scheduling_info, path_id) + && scheduling_info.may_send_data && frame::PathCidsBlocked::SIZE_BOUND <= builder.frame_space_remaining() { let Some(path_id) = space.pending.path_cids_blocked.pop_first() else { @@ -5899,7 +5891,7 @@ impl Connection { } // RESET_STREAM, STOP_SENDING, MAX_DATA, MAX_STREAM_DATA, MAX_STREAMS - if space_id == SpaceId::Data && should_send_data(&scheduling_info, path_id) { + if space_id == SpaceId::Data && scheduling_info.may_send_data { self.streams .write_control_frames(builder, &mut space.pending, stats); } @@ -5913,8 +5905,7 @@ impl Connection { .expect("some local CID state must exist"); let new_cid_size_bound = frame::NewConnectionId::size_bound(is_multipath_negotiated, cid_len); - while should_send_data(&scheduling_info, path_id) - && builder.frame_space_remaining() > new_cid_size_bound + while scheduling_info.may_send_data && builder.frame_space_remaining() > new_cid_size_bound { let issued = match space.pending.new_cids.pop() { Some(x) => x, @@ -5945,9 +5936,7 @@ impl Connection { // RETIRE_CONNECTION_ID let retire_cid_bound = frame::RetireConnectionId::size_bound(is_multipath_negotiated); - while should_send_data(&scheduling_info, path_id) - && builder.frame_space_remaining() > retire_cid_bound - { + while scheduling_info.may_send_data && builder.frame_space_remaining() > retire_cid_bound { let (path_id, sequence) = match space.pending.retire_cids.pop() { Some((PathId::ZERO, seq)) if !is_multipath_negotiated => (None, seq), Some((path_id, seq)) => (Some(path_id), seq), @@ -5959,7 +5948,7 @@ impl Connection { // DATAGRAM let mut sent_datagrams = false; - while should_send_data(&scheduling_info, path_id) + while scheduling_info.may_send_data && builder.frame_space_remaining() > Datagram::SIZE_BOUND && space_id == SpaceId::Data { @@ -5978,7 +5967,7 @@ impl Connection { let path = &mut self.paths.get_mut(&path_id).expect("known path").data; // NEW_TOKEN - if should_send_data(&scheduling_info, path_id) { + if scheduling_info.may_send_data { while let Some(network_path) = space.pending.new_tokens.pop() { debug_assert_eq!(space_id, SpaceId::Data); let ConnectionSide::Server { server_config } = &self.side else { @@ -6015,14 +6004,14 @@ impl Connection { } // STREAM - if should_send_data(&scheduling_info, path_id) && space_id == SpaceId::Data { + if scheduling_info.may_send_data && space_id == SpaceId::Data { self.streams .write_stream_frames(builder, self.config.send_fairness, stats); } // ADD_ADDRESS while space_id == SpaceId::Data - && should_send_data(&scheduling_info, path_id) + && scheduling_info.may_send_data && frame::AddAddress::SIZE_BOUND <= builder.frame_space_remaining() { if let Some(added_address) = space.pending.add_address.pop_last() { @@ -6034,7 +6023,7 @@ impl Connection { // REMOVE_ADDRESS while space_id == SpaceId::Data - && should_send_data(&scheduling_info, path_id) + && scheduling_info.may_send_data && frame::RemoveAddress::SIZE_BOUND <= builder.frame_space_remaining() { if let Some(removed_address) = space.pending.remove_address.pop_last() { @@ -6849,74 +6838,29 @@ enum PollPathSpaceStatus { }, } -struct SchedulingInfo { - paths: BTreeMap, - current: PathId, - // is_multipath_negotiated - // is_0rtt - // space_has_keys -} - -impl SchedulingInfo {} - -/// TODO(flub): this really is duplicate info, but it makes it easy to reason about what -/// info is used for scheduling right now. +/// Information used to decide what frames to schedule into which packets. +/// +/// Primarily used by [`Connection::poll_transmit_on_path`] and the functions that help +/// building packets for it: [`Connection::poll_transmit_path_space`] and +/// [`Connection::populate_packet`]. #[derive(Debug, Copy, Clone)] struct PathSchedulingInfo { - /// Whether the path has an active CID. - has_cids: bool, - /// Whether the path is validated. - validated: bool, /// Whether the path is abandoned. /// /// Note that a path that is abandoned but still has CIDs can still send a packet. After /// sending that packet the CIDs have to be considered retired as well and /// [`Self::has_cids`] should turn `false`. abandoned: bool, - /// The status of the path. - status: PathStatus, -} - -/// Whether we should send data on the `current` path. -/// -/// Here "data" means any frames which may be sent on any path. It does not necessarily -/// match to the [`SpaceKind::Data`] spaces, e.g. frames sent in 0-RTT also count. -fn should_send_data(all: &BTreeMap, current: PathId) -> bool { - // To send SpaceKind::Data we want a path: - // - with CIDs - // - validated - // - not abandoned - // - status-available unless there is no such path - let this = all.get(¤t).unwrap(); - if !this.has_cids || this.abandoned { - return false; - } - let have_better_path = if !this.validated { - all.values().any(|info| { - let PathSchedulingInfo { - has_cids, - validated, - abandoned, - status: _, - } = *info; - has_cids && !abandoned && validated - }) - } else { - // path is validated - match this.status { - PathStatus::Available => return true, - PathStatus::Backup => all.values().any(|info| { - let PathSchedulingInfo { - has_cids, - validated, - abandoned, - status, - } = *info; - has_cids && !abandoned && validated && status == PathStatus::Available - }), - } - }; - !have_better_path + /// Whether the path may send [`SpaceKind::Data`] frames. + /// + /// Some paths should only send frames from [`SendableFrames::space_id_only`]. All other + /// frames are essentially frames that can be sent on any [`SpaceKind::Data`] space. For + /// those we want to respect packet scheduling rules however. + /// + /// Roughly speaking data frames are only sent on spaces that have CIDs, are not + /// abandoned and have no *better* spaces. However see to comments where this is + /// populated for the exact packet scheduling implementation. + may_send_data: bool, } #[derive(Debug, Copy, Clone, PartialEq, Eq)] From 9295ade369533d4a53eef161b860326a4e1fa7db Mon Sep 17 00:00:00 2001 From: Floris Bruynooghe Date: Mon, 9 Mar 2026 12:33:28 +0100 Subject: [PATCH 17/44] wip --- noq-proto/src/connection/mod.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/noq-proto/src/connection/mod.rs b/noq-proto/src/connection/mod.rs index c7dfc90e1..25ced1aec 100644 --- a/noq-proto/src/connection/mod.rs +++ b/noq-proto/src/connection/mod.rs @@ -981,8 +981,8 @@ impl Connection { return None; } StateType::Draining | StateType::Closed => { - // self.close is only reset once the associated packet had been - // encoded successfully + // self.connection_close_pending is only reset once the associated packet + // had been encoded successfully if !self.connection_close_pending { self.app_limited = true; return None; @@ -6861,6 +6861,10 @@ struct PathSchedulingInfo { /// abandoned and have no *better* spaces. However see to comments where this is /// populated for the exact packet scheduling implementation. may_send_data: bool, + /// Whether the path may send a CONNECTION_CLOSE frame. + /// + /// Is this a good idea? + may_send_close: bool, } #[derive(Debug, Copy, Clone, PartialEq, Eq)] From b67d78083f447c69f2dc68dcc918955984ec5b2f Mon Sep 17 00:00:00 2001 From: Floris Bruynooghe Date: Mon, 9 Mar 2026 16:59:16 +0100 Subject: [PATCH 18/44] Guess we do need a new field for this Slightly sad to have to make an explicit exception for this, at least currently. --- noq-proto/src/connection/mod.rs | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/noq-proto/src/connection/mod.rs b/noq-proto/src/connection/mod.rs index 25ced1aec..6d776b820 100644 --- a/noq-proto/src/connection/mod.rs +++ b/noq-proto/src/connection/mod.rs @@ -1054,11 +1054,25 @@ impl Connection { } } }; + // CONNECTION_CLOSE is allowed to be sent on an non-validated + // path. Particularly during the handshake we want to send it before the + // path is validated. Later if there is no validated path available we + // will also accept sending it on an un-validated path. + let may_send_close = has_cids + && !abandoned + && if !validated && have_validated_status_available_space { + // We have a better space to send on. + false + } else { + // No other validated space, this is as good as it gets. + true + }; ( *path_id, PathSchedulingInfo { abandoned, may_send_data, + may_send_close, }, ) }) @@ -1354,6 +1368,9 @@ impl Connection { // Currently we don't send on an abandoned path, PATH_ABANDON is always // sent on a different path. false + } else if can_send.close && scheduling_info.may_send_close { + // This is the best path to send a CONNECTION_CLOSE on. + true } else if needs_loss_probe { // We always send if a loss probe if the path is not abandoned. true @@ -6860,10 +6877,17 @@ struct PathSchedulingInfo { /// Roughly speaking data frames are only sent on spaces that have CIDs, are not /// abandoned and have no *better* spaces. However see to comments where this is /// populated for the exact packet scheduling implementation. + /// + /// This is essentially marks this paths as the best validated space ID, though other + /// paths could be qually good. Except during the handshake in which case it does not + /// need to be validated. Note that once in the closed or draining states this will + /// never be true. may_send_data: bool, /// Whether the path may send a CONNECTION_CLOSE frame. /// - /// Is this a good idea? + /// This is essentially marks this path as the best validated space ID with a fallback + /// to unvalidated spaces if there are no validated spaces. Though other paths could be + /// equally good. may_send_close: bool, } From c3eed892cf6ab907aeb98ecea15d44d7548c6654 Mon Sep 17 00:00:00 2001 From: Floris Bruynooghe Date: Mon, 9 Mar 2026 17:03:08 +0100 Subject: [PATCH 19/44] clippy --- noq-proto/src/connection/mod.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/noq-proto/src/connection/mod.rs b/noq-proto/src/connection/mod.rs index 6d776b820..ce3cfcbd2 100644 --- a/noq-proto/src/connection/mod.rs +++ b/noq-proto/src/connection/mod.rs @@ -1020,9 +1020,9 @@ impl Connection { self.paths .iter() .map(|(path_id, path)| { - let has_cids = self.remote_cids.contains_key(&path_id); + let has_cids = self.remote_cids.contains_key(path_id); let validated = path.data.validated; - let abandoned = self.abandoned_paths.contains(&path_id); + let abandoned = self.abandoned_paths.contains(path_id); let status = path.data.local_status(); // This is the core packet scheduling, whether this space ID may send @@ -1100,7 +1100,7 @@ impl Connection { buf, path_id, max_datagrams, - &info, + info, connection_close_pending, ) { PollPathStatus::Send(transmit) => { From 453fa89b88ef3d18fb15da390fd464e916bc5d1f Mon Sep 17 00:00:00 2001 From: Floris Bruynooghe Date: Mon, 9 Mar 2026 17:19:29 +0100 Subject: [PATCH 20/44] may_send_data has been made smarter, don't need this --- noq-proto/src/connection/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/noq-proto/src/connection/mod.rs b/noq-proto/src/connection/mod.rs index ce3cfcbd2..7163a6a50 100644 --- a/noq-proto/src/connection/mod.rs +++ b/noq-proto/src/connection/mod.rs @@ -5786,7 +5786,7 @@ impl Connection { // CRYPTO while !is_0rtt - && (space_id < SpaceId::Data || scheduling_info.may_send_data) + && scheduling_info.may_send_data && builder.frame_space_remaining() > frame::Crypto::SIZE_BOUND { let mut frame = match space.pending.crypto.pop_front() { From 55d3c7911552a958ccdb46d4acada0568297a11b Mon Sep 17 00:00:00 2001 From: Floris Bruynooghe Date: Tue, 10 Mar 2026 11:57:22 +0100 Subject: [PATCH 21/44] wip --- noq-proto/src/connection/mod.rs | 2 -- noq-proto/src/tests/random_interaction.rs | 6 +++--- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/noq-proto/src/connection/mod.rs b/noq-proto/src/connection/mod.rs index 7163a6a50..2cf221da2 100644 --- a/noq-proto/src/connection/mod.rs +++ b/noq-proto/src/connection/mod.rs @@ -1016,7 +1016,6 @@ impl Connection { && path.data.local_status() == PathStatus::Available }); let is_handshaking = self.is_handshaking(); - tracing::warn!(?is_handshaking); self.paths .iter() .map(|(path_id, path)| { @@ -1383,7 +1382,6 @@ impl Connection { !can_send.is_empty() && scheduling_info.may_send_data } }; - tracing::warn!(?can_send, ?scheduling_info, ?space_will_send, "checking"); if !space_will_send { // Nothing more to send. Previous iterations of this loop may have built diff --git a/noq-proto/src/tests/random_interaction.rs b/noq-proto/src/tests/random_interaction.rs index 4e9a27d5c..897d78778 100644 --- a/noq-proto/src/tests/random_interaction.rs +++ b/noq-proto/src/tests/random_interaction.rs @@ -5,7 +5,7 @@ use std::{ use bytes::Bytes; use test_strategy::Arbitrary; -use tracing::{debug, trace}; +use tracing::{debug, info, trace}; use crate::{ Connection, ConnectionHandle, Dir, FourTuple, PathId, PathStatus, Side, StreamId, @@ -330,12 +330,12 @@ pub(super) fn run_random_interaction( client_cfg.transport = Arc::new(transport_config); let (client_ch, server_ch) = pair.connect_with(client_cfg); pair.drive(); // finish establishing the connection; - debug!("INTERACTION SETUP FINISHED"); + info!("INTERACTION SETUP FINISHED"); let mut client = State::new(Side::Client, client_ch); let mut server = State::new(Side::Server, server_ch); for interaction in interactions { - debug!(?interaction, "INTERACTION STEP"); + info!(?interaction, "INTERACTION STEP"); interaction.run(pair, &mut client, &mut server); } (client.handle, server.handle) From 50a3eca1fbb8b2015f20c4251e34705801433aa2 Mon Sep 17 00:00:00 2001 From: Floris Bruynooghe Date: Wed, 11 Mar 2026 13:09:27 +0100 Subject: [PATCH 22/44] tweak logging --- noq-proto/src/connection/mod.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/noq-proto/src/connection/mod.rs b/noq-proto/src/connection/mod.rs index 2cf221da2..3116ae2e1 100644 --- a/noq-proto/src/connection/mod.rs +++ b/noq-proto/src/connection/mod.rs @@ -660,6 +660,8 @@ impl Connection { } } + trace!(%path_id, ?reason, "closing path"); + // Send PATH_ABANDON self.spaces[SpaceId::Data] .pending @@ -1849,7 +1851,13 @@ impl Connection { if can_send.other && !need_loss_probe && !can_send.close { let path = self.path_data(path_id); if path.in_flight.bytes + bytes_to_send >= path.congestion.window() { - trace!(?space_id, %path_id, "blocked by congestion control"); + trace!( + ?space_id, + %path_id, + in_flight=%path.in_flight.bytes, + congestion_window=%path.congestion.window(), + "blocked by congestion control", + ); return PathBlocked::Congestion; } } @@ -5753,7 +5761,6 @@ impl Connection { && let Some(token) = path.path_responses.pop_on_path(path.network_path) { let response = frame::PathResponse(token); - trace!(frame = %response); builder.write_frame(response, stats); builder.require_padding(); From 236b97728f25e3aacd1ca7ac3d50a164b0f28d89 Mon Sep 17 00:00:00 2001 From: Floris Bruynooghe Date: Wed, 11 Mar 2026 13:09:43 +0100 Subject: [PATCH 23/44] proptests should maybe log errors? --- noq-proto/src/tests/random_interaction.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/noq-proto/src/tests/random_interaction.rs b/noq-proto/src/tests/random_interaction.rs index 897d78778..0cbee73ac 100644 --- a/noq-proto/src/tests/random_interaction.rs +++ b/noq-proto/src/tests/random_interaction.rs @@ -5,7 +5,7 @@ use std::{ use bytes::Bytes; use test_strategy::Arbitrary; -use tracing::{debug, info, trace}; +use tracing::{debug, error, info, trace}; use crate::{ Connection, ConnectionHandle, Dir, FourTuple, PathId, PathStatus, Side, StreamId, @@ -181,7 +181,9 @@ impl TestOp { }; let conn = state.conn(pair)?; let path_id = get_path_id(conn, path_idx)?; - conn.close_path(now, path_id, error_code.into()).ok(); + conn.close_path(now, path_id, error_code.into()) + .map_err(|err| error!(?err, "failed to close path")) + .ok(); } Self::PathSetStatus { side, From 23241b1cbef620696deb6eed31e88c2875ade235 Mon Sep 17 00:00:00 2001 From: Floris Bruynooghe Date: Wed, 11 Mar 2026 13:10:17 +0100 Subject: [PATCH 24/44] drive to idle for a 1000 iterations, 100 is a bit small --- noq-proto/src/tests/proptest.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/noq-proto/src/tests/proptest.rs b/noq-proto/src/tests/proptest.rs index 2ac4f7a71..b7b8295a8 100644 --- a/noq-proto/src/tests/proptest.rs +++ b/noq-proto/src/tests/proptest.rs @@ -361,7 +361,7 @@ fn regression_never_idle() { let (client_ch, server_ch) = run_random_interaction(&mut pair, interactions, multipath_transport_config(prefix)); - assert!(!pair.drive_bounded(100), "connection never became idle"); + assert!(!pair.drive_bounded(1000), "connection never became idle"); assert!(allowed_error(poll_to_close( pair.client_conn_mut(client_ch) ))); @@ -440,7 +440,7 @@ fn regression_packet_number_space_missing() { let (client_ch, server_ch) = run_random_interaction(&mut pair, interactions, multipath_transport_config(prefix)); - assert!(!pair.drive_bounded(100), "connection never became idle"); + assert!(!pair.drive_bounded(1000), "connection never became idle"); assert!(allowed_error(poll_to_close( pair.client_conn_mut(client_ch) ))); @@ -472,7 +472,7 @@ fn regression_peer_failed_to_respond_with_path_abandon() { let (client_ch, server_ch) = run_random_interaction(&mut pair, interactions, multipath_transport_config(prefix)); - assert!(!pair.drive_bounded(100), "connection never became idle"); + assert!(!pair.drive_bounded(1000), "connection never became idle"); assert!(allowed_error(poll_to_close( pair.client_conn_mut(client_ch) ))); @@ -867,7 +867,7 @@ fn regression_never_idle4() { let (client_ch, server_ch) = run_random_interaction(&mut pair, interactions, multipath_transport_config(prefix)); - assert!(!pair.drive_bounded(100), "connection never became idle"); + assert!(!pair.drive_bounded(1000), "connection never became idle"); assert!(allowed_error(poll_to_close( pair.client_conn_mut(client_ch) ))); From 540afe0910af81463bea1dce451c719ab46e5cf2 Mon Sep 17 00:00:00 2001 From: Floris Bruynooghe Date: Wed, 11 Mar 2026 13:15:28 +0100 Subject: [PATCH 25/44] turns out we don't need this and i find it a bit confusing --- noq-proto/src/connection/paths.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/noq-proto/src/connection/paths.rs b/noq-proto/src/connection/paths.rs index e804dba8d..81e06626c 100644 --- a/noq-proto/src/connection/paths.rs +++ b/noq-proto/src/connection/paths.rs @@ -950,7 +950,7 @@ impl PathStatusState { /// See section "3.3 Path Status Management": /// #[cfg_attr(test, derive(test_strategy::Arbitrary))] -#[derive(Debug, Copy, Clone, Default, PartialEq, Eq, PartialOrd, Ord)] +#[derive(Debug, Copy, Clone, Default, PartialEq, Eq)] pub enum PathStatus { /// Paths marked with as available will be used when scheduling packets /// From 68221360a206d7749e88c9d491a55ac126ea25c2 Mon Sep 17 00:00:00 2001 From: Floris Bruynooghe Date: Wed, 11 Mar 2026 13:34:55 +0100 Subject: [PATCH 26/44] remove stuff split off into #494 --- noq-proto/src/connection/mod.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/noq-proto/src/connection/mod.rs b/noq-proto/src/connection/mod.rs index 3116ae2e1..62da029bf 100644 --- a/noq-proto/src/connection/mod.rs +++ b/noq-proto/src/connection/mod.rs @@ -660,8 +660,6 @@ impl Connection { } } - trace!(%path_id, ?reason, "closing path"); - // Send PATH_ABANDON self.spaces[SpaceId::Data] .pending @@ -983,8 +981,8 @@ impl Connection { return None; } StateType::Draining | StateType::Closed => { - // self.connection_close_pending is only reset once the associated packet - // had been encoded successfully + // self.close is only reset once the associated packet had been + // encoded successfully if !self.connection_close_pending { self.app_limited = true; return None; From 2fdae31c42c4594c1447347cf11020a531453fff Mon Sep 17 00:00:00 2001 From: Floris Bruynooghe Date: Wed, 11 Mar 2026 13:50:47 +0100 Subject: [PATCH 27/44] Remove changes split off into 495 --- noq-proto/src/tests/random_interaction.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/noq-proto/src/tests/random_interaction.rs b/noq-proto/src/tests/random_interaction.rs index 0cbee73ac..4e9a27d5c 100644 --- a/noq-proto/src/tests/random_interaction.rs +++ b/noq-proto/src/tests/random_interaction.rs @@ -5,7 +5,7 @@ use std::{ use bytes::Bytes; use test_strategy::Arbitrary; -use tracing::{debug, error, info, trace}; +use tracing::{debug, trace}; use crate::{ Connection, ConnectionHandle, Dir, FourTuple, PathId, PathStatus, Side, StreamId, @@ -181,9 +181,7 @@ impl TestOp { }; let conn = state.conn(pair)?; let path_id = get_path_id(conn, path_idx)?; - conn.close_path(now, path_id, error_code.into()) - .map_err(|err| error!(?err, "failed to close path")) - .ok(); + conn.close_path(now, path_id, error_code.into()).ok(); } Self::PathSetStatus { side, @@ -332,12 +330,12 @@ pub(super) fn run_random_interaction( client_cfg.transport = Arc::new(transport_config); let (client_ch, server_ch) = pair.connect_with(client_cfg); pair.drive(); // finish establishing the connection; - info!("INTERACTION SETUP FINISHED"); + debug!("INTERACTION SETUP FINISHED"); let mut client = State::new(Side::Client, client_ch); let mut server = State::new(Side::Server, server_ch); for interaction in interactions { - info!(?interaction, "INTERACTION STEP"); + debug!(?interaction, "INTERACTION STEP"); interaction.run(pair, &mut client, &mut server); } (client.handle, server.handle) From 1fdcc6acdc21ffb219148c7e72244890cc3e338f Mon Sep 17 00:00:00 2001 From: Floris Bruynooghe Date: Wed, 11 Mar 2026 15:11:39 +0100 Subject: [PATCH 28/44] typo --- noq-proto/src/connection/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/noq-proto/src/connection/mod.rs b/noq-proto/src/connection/mod.rs index 62da029bf..4dc3b59e1 100644 --- a/noq-proto/src/connection/mod.rs +++ b/noq-proto/src/connection/mod.rs @@ -1048,7 +1048,7 @@ impl Connection { true } PathStatus::Backup => { - // If there is an status-available path we prefer that. + // If there is a status-available path we prefer that. !have_validated_status_available_space } } From e4983c5d40dc9650aa5960825b692242eed25cb3 Mon Sep 17 00:00:00 2001 From: Floris Bruynooghe Date: Wed, 11 Mar 2026 15:12:02 +0100 Subject: [PATCH 29/44] style --- noq-proto/src/connection/mod.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/noq-proto/src/connection/mod.rs b/noq-proto/src/connection/mod.rs index 4dc3b59e1..c858c4fb2 100644 --- a/noq-proto/src/connection/mod.rs +++ b/noq-proto/src/connection/mod.rs @@ -1053,6 +1053,7 @@ impl Connection { } } }; + // CONNECTION_CLOSE is allowed to be sent on an non-validated // path. Particularly during the handshake we want to send it before the // path is validated. Later if there is no validated path available we From 1ddef5c415ca51680a4c9d349ba3375ee4eb8426 Mon Sep 17 00:00:00 2001 From: Floris Bruynooghe Date: Wed, 11 Mar 2026 15:12:41 +0100 Subject: [PATCH 30/44] ww --- noq-proto/src/connection/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/noq-proto/src/connection/mod.rs b/noq-proto/src/connection/mod.rs index c858c4fb2..c380559a0 100644 --- a/noq-proto/src/connection/mod.rs +++ b/noq-proto/src/connection/mod.rs @@ -1054,7 +1054,7 @@ impl Connection { } }; - // CONNECTION_CLOSE is allowed to be sent on an non-validated + // CONNECTION_CLOSE is allowed to be sent on a non-validated // path. Particularly during the handshake we want to send it before the // path is validated. Later if there is no validated path available we // will also accept sending it on an un-validated path. From 405df62fa30af4f8925350f315f40780a3c05d07 Mon Sep 17 00:00:00 2001 From: Floris Bruynooghe Date: Wed, 11 Mar 2026 15:15:46 +0100 Subject: [PATCH 31/44] remove this old thing --- noq-proto/src/connection/mod.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/noq-proto/src/connection/mod.rs b/noq-proto/src/connection/mod.rs index c380559a0..b0a0ab436 100644 --- a/noq-proto/src/connection/mod.rs +++ b/noq-proto/src/connection/mod.rs @@ -1079,10 +1079,6 @@ impl Connection { .collect() }; - // TODO: how to avoid the allocation? Cannot use a for loop because of - // borrowing. Maybe SmallVec or similar. - let path_ids: Vec<_> = self.paths.keys().copied().collect(); - // If we end up not sending anything, we need to know if that was because there was // nothing to send or because we were congestion blocked. let mut congestion_blocked = false; @@ -1130,8 +1126,8 @@ impl Connection { if self.state.is_established() { // Try MTU probing now - for path_id in path_ids { - if let Some(transmit) = self.poll_transmit_mtu_probe(now, buf, path_id) { + for path_id in scheduling_info.keys() { + if let Some(transmit) = self.poll_transmit_mtu_probe(now, buf, *path_id) { return Some(transmit); } } From 1c7048f1570707dbd43c4c50ad4b9ebb30c387ac Mon Sep 17 00:00:00 2001 From: Floris Bruynooghe Date: Wed, 11 Mar 2026 17:33:46 +0100 Subject: [PATCH 32/44] tweak --- noq-proto/src/connection/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/noq-proto/src/connection/mod.rs b/noq-proto/src/connection/mod.rs index b0a0ab436..0c59fcdf3 100644 --- a/noq-proto/src/connection/mod.rs +++ b/noq-proto/src/connection/mod.rs @@ -1368,7 +1368,7 @@ impl Connection { // This is the best path to send a CONNECTION_CLOSE on. true } else if needs_loss_probe { - // We always send if a loss probe if the path is not abandoned. + // We always send a loss probe if the path is not abandoned. true } else if can_send.space_id_only { // We always send space-specific frames if not abandoned. From fbb9575e58739b2defc0144be9775acb5cca2e96 Mon Sep 17 00:00:00 2001 From: Floris Bruynooghe Date: Wed, 11 Mar 2026 17:34:36 +0100 Subject: [PATCH 33/44] needs_loss_probe is a variable above, it's not that bad --- noq-proto/src/connection/mod.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/noq-proto/src/connection/mod.rs b/noq-proto/src/connection/mod.rs index 0c59fcdf3..633524cbd 100644 --- a/noq-proto/src/connection/mod.rs +++ b/noq-proto/src/connection/mod.rs @@ -1450,7 +1450,6 @@ impl Connection { // We need something to send for a tail-loss probe. let request_immediate_ack = space_id == SpaceId::Data && self.peer_supports_ack_frequency(); - // TODO(flub): this is really scheduling logic hiding here. self.spaces[space_id].maybe_queue_probe( path_id, request_immediate_ack, From 05b5f6e4502b96b38bcd738f46da6a8e2865c61b Mon Sep 17 00:00:00 2001 From: Floris Bruynooghe Date: Wed, 11 Mar 2026 17:35:36 +0100 Subject: [PATCH 34/44] collapse these two cases. it's a hard call though --- noq-proto/src/connection/mod.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/noq-proto/src/connection/mod.rs b/noq-proto/src/connection/mod.rs index 633524cbd..e8c42232d 100644 --- a/noq-proto/src/connection/mod.rs +++ b/noq-proto/src/connection/mod.rs @@ -1367,11 +1367,9 @@ impl Connection { } else if can_send.close && scheduling_info.may_send_close { // This is the best path to send a CONNECTION_CLOSE on. true - } else if needs_loss_probe { - // We always send a loss probe if the path is not abandoned. - true - } else if can_send.space_id_only { - // We always send space-specific frames if not abandoned. + } else if needs_loss_probe || can_send.space_id_only { + // We always send a loss probe or space-specific frames if the path is + // not abandoned. true } else { // Anything else we only send if we're the best path for SpaceKind::Data From 016413b653072cf5bc1f8d9b119947bf4221cd05 Mon Sep 17 00:00:00 2001 From: Floris Bruynooghe Date: Wed, 11 Mar 2026 17:36:58 +0100 Subject: [PATCH 35/44] well doh, the doc comment says why --- noq-proto/src/connection/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/noq-proto/src/connection/mod.rs b/noq-proto/src/connection/mod.rs index e8c42232d..842078205 100644 --- a/noq-proto/src/connection/mod.rs +++ b/noq-proto/src/connection/mod.rs @@ -1797,7 +1797,7 @@ impl Connection { loop { let can_send = self.space_can_send( space_id, - PathId::ZERO, // TODO: why only PathId 0????? + PathId::ZERO, max_packet_size, connection_close_pending, ); From 184e46547a041ac595c8d2f63f3521e9ba451849 Mon Sep 17 00:00:00 2001 From: Floris Bruynooghe Date: Wed, 11 Mar 2026 17:46:07 +0100 Subject: [PATCH 36/44] wordsmithing, because 90% of packet scheduling is about that --- noq-proto/src/connection/mod.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/noq-proto/src/connection/mod.rs b/noq-proto/src/connection/mod.rs index 842078205..16728618b 100644 --- a/noq-proto/src/connection/mod.rs +++ b/noq-proto/src/connection/mod.rs @@ -6875,16 +6875,19 @@ struct PathSchedulingInfo { /// abandoned and have no *better* spaces. However see to comments where this is /// populated for the exact packet scheduling implementation. /// - /// This is essentially marks this paths as the best validated space ID, though other - /// paths could be qually good. Except during the handshake in which case it does not - /// need to be validated. Note that once in the closed or draining states this will - /// never be true. + /// This is essentially marks this paths as the best validated space ID. Except during + /// the handshake in which case it does not need to be validated. Several paths could be + /// equally good and all have this set to `true`, in that case packet scheduling can + /// choose which path to use. Currently it chooses the lowest path that is not + /// congestion blocked. + /// + /// Note that once in the closed or draining states this will never be true. may_send_data: bool, /// Whether the path may send a CONNECTION_CLOSE frame. /// /// This is essentially marks this path as the best validated space ID with a fallback - /// to unvalidated spaces if there are no validated spaces. Though other paths could be - /// equally good. + /// to unvalidated spaces if there are no validated spaces. Like for + /// [`Self::may_send_data`] other paths could be equally good. may_send_close: bool, } From 78d1dcd7ea8387163623fbe443abc36b2d86e2d3 Mon Sep 17 00:00:00 2001 From: Floris Bruynooghe Date: Wed, 11 Mar 2026 17:50:36 +0100 Subject: [PATCH 37/44] put this back, split off into another pr --- noq-proto/src/connection/mod.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/noq-proto/src/connection/mod.rs b/noq-proto/src/connection/mod.rs index 16728618b..53f367c8e 100644 --- a/noq-proto/src/connection/mod.rs +++ b/noq-proto/src/connection/mod.rs @@ -5753,6 +5753,7 @@ impl Connection { && let Some(token) = path.path_responses.pop_on_path(path.network_path) { let response = frame::PathResponse(token); + trace!(frame = %response); builder.write_frame(response, stats); builder.require_padding(); From 2da4a66cac937c8e559921bc0e3b4eabdac73ea0 Mon Sep 17 00:00:00 2001 From: Floris Bruynooghe Date: Thu, 12 Mar 2026 10:10:39 +0100 Subject: [PATCH 38/44] fix doc comment --- noq-proto/src/connection/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/noq-proto/src/connection/mod.rs b/noq-proto/src/connection/mod.rs index c66005cca..9802a3394 100644 --- a/noq-proto/src/connection/mod.rs +++ b/noq-proto/src/connection/mod.rs @@ -6882,8 +6882,8 @@ struct PathSchedulingInfo { /// Whether the path is abandoned. /// /// Note that a path that is abandoned but still has CIDs can still send a packet. After - /// sending that packet the CIDs have to be considered retired as well and - /// [`Self::has_cids`] should turn `false`. + /// sending that packet the CIDs issued by the remote have to be considered retired as + /// well. abandoned: bool, /// Whether the path may send [`SpaceKind::Data`] frames. /// From 4320b36fe4aea23a50fc92c713486b143c4fe2b4 Mon Sep 17 00:00:00 2001 From: Floris Bruynooghe Date: Mon, 16 Mar 2026 12:08:24 +0100 Subject: [PATCH 39/44] Typos from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Philipp Krüger --- noq-proto/src/connection/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/noq-proto/src/connection/mod.rs b/noq-proto/src/connection/mod.rs index 9802a3394..ebac6c300 100644 --- a/noq-proto/src/connection/mod.rs +++ b/noq-proto/src/connection/mod.rs @@ -6895,7 +6895,7 @@ struct PathSchedulingInfo { /// abandoned and have no *better* spaces. However see to comments where this is /// populated for the exact packet scheduling implementation. /// - /// This is essentially marks this paths as the best validated space ID. Except during + /// This essentially marks this paths as the best validated space ID. Except during /// the handshake in which case it does not need to be validated. Several paths could be /// equally good and all have this set to `true`, in that case packet scheduling can /// choose which path to use. Currently it chooses the lowest path that is not @@ -6905,7 +6905,7 @@ struct PathSchedulingInfo { may_send_data: bool, /// Whether the path may send a CONNECTION_CLOSE frame. /// - /// This is essentially marks this path as the best validated space ID with a fallback + /// This essentially marks this path as the best validated space ID with a fallback /// to unvalidated spaces if there are no validated spaces. Like for /// [`Self::may_send_data`] other paths could be equally good. may_send_close: bool, From 4c73e3d475086cb7399282d3a9edf95d27e2e934 Mon Sep 17 00:00:00 2001 From: Floris Bruynooghe Date: Mon, 16 Mar 2026 15:10:29 +0100 Subject: [PATCH 40/44] Do not allocate and move scheduling info to separate function Two bits of PR review: - Move the scheduling to a separate function. - Avoid an allocation, but this has some tradeoffs. - I now have two loops that look for the PathId by doing `next_path_id = self.path.keys().find(|i| **i > path_id).copied();`. It might be possible to fold the MTU discovery in the main poll loop, MTU packets would get a slightly higher priority but probably not really harmful overall. - I now need to do the computation for `have_validated_status_available_space many more times. I'm not sure how much the compiler manages to remove all of that. Is it smart enough to figure out that `have_validate_status_available_space` won't change between the calls and does it move it out? Does it make the iteration as fast as the previous version? On the other hand, we now have some situations where we don't have to compute the scheduling information, and no longer need to compute it for all paths if we don't send on the last path. What do you think, which version is better (though I also adopted @matheus23's feedback about splitting it off to a function, but that doesn't affect this really. It does make the diff a little bit more though)? As an aside, in working out of how scheduling should work it was really helpful to have to extremely explicit as a bunch of data that's computed up-front. But it's fair that now we know this is how it should work that we can implement it in the most optimal way. --- noq-proto/src/connection/mod.rs | 169 +++++++++++++++++--------------- 1 file changed, 92 insertions(+), 77 deletions(-) diff --git a/noq-proto/src/connection/mod.rs b/noq-proto/src/connection/mod.rs index ebac6c300..312587deb 100644 --- a/noq-proto/src/connection/mod.rs +++ b/noq-proto/src/connection/mod.rs @@ -1014,96 +1014,25 @@ impl Connection { && self.peer_supports_ack_frequency(); } - // Build up some packet scheduling information about all paths. - let scheduling_info: BTreeMap = { - let have_validated_status_available_space = self.paths.iter().any(|(path_id, path)| { - self.remote_cids.contains_key(path_id) - && !self.abandoned_paths.contains(path_id) - && path.data.validated - && path.data.local_status() == PathStatus::Available - }); - let is_handshaking = self.is_handshaking(); - self.paths - .iter() - .map(|(path_id, path)| { - let has_cids = self.remote_cids.contains_key(path_id); - let validated = path.data.validated; - let abandoned = self.abandoned_paths.contains(path_id); - let status = path.data.local_status(); - - // This is the core packet scheduling, whether this space ID may send - // SpaceKind::Data frames. - let may_send_data = has_cids - && !abandoned - && if is_handshaking { - // There is only one path during the handshake. We want to - // already send 0-RTT and 0.5-RTT (permitting anti-amplification - // limit) data. - true - } else if !validated { - // TODO(flub): When we have a network change we might end up - // having to abandon all paths and re-open new ones to the - // same remotes. This leaves us without any validated - // path. Perhaps we should have a way to figure out if the - // path is to a previously-validated remote address and allow - // sending data to such remotes immediately. - false - } else { - match status { - PathStatus::Available => { - // Best possible space to send data on. - true - } - PathStatus::Backup => { - // If there is a status-available path we prefer that. - !have_validated_status_available_space - } - } - }; - - // CONNECTION_CLOSE is allowed to be sent on a non-validated - // path. Particularly during the handshake we want to send it before the - // path is validated. Later if there is no validated path available we - // will also accept sending it on an un-validated path. - let may_send_close = has_cids - && !abandoned - && if !validated && have_validated_status_available_space { - // We have a better space to send on. - false - } else { - // No other validated space, this is as good as it gets. - true - }; - ( - *path_id, - PathSchedulingInfo { - abandoned, - may_send_data, - may_send_close, - }, - ) - }) - .collect() - }; - // If we end up not sending anything, we need to know if that was because there was // nothing to send or because we were congestion blocked. let mut congestion_blocked = false; - for (&path_id, info) in scheduling_info.iter() { + let mut next_path_id = self.paths.first_entry().map(|e| *e.key()); + while let Some(path_id) = next_path_id { if !connection_close_pending && let Some(transmit) = self.poll_transmit_off_path(now, buf, path_id) { return Some(transmit); } - // Poll for on-path transmits. + let info = self.scheduling_info(path_id); match self.poll_transmit_on_path( now, buf, path_id, max_datagrams, - info, + &info, connection_close_pending, ) { PollPathStatus::Send(transmit) => { @@ -1121,6 +1050,8 @@ impl Connection { ); } } + + next_path_id = self.paths.keys().find(|i| **i > path_id).copied(); } // We didn't produce any application data packet @@ -1133,16 +1064,100 @@ impl Connection { if self.state.is_established() { // Try MTU probing now - for path_id in scheduling_info.keys() { - if let Some(transmit) = self.poll_transmit_mtu_probe(now, buf, *path_id) { + let mut next_path_id = self.paths.first_entry().map(|e| *e.key()); + while let Some(path_id) = next_path_id { + if let Some(transmit) = self.poll_transmit_mtu_probe(now, buf, path_id) { return Some(transmit); } + next_path_id = self.paths.keys().find(|i| **i > path_id).copied(); } } None } + /// Computes the packet scheduling information for this path. + /// + /// While this information is only returned for a single path, it is important to know + /// that this information remains static for the entire span of a single + /// [`Connection::poll_transmit`] call. In other words, the return value is purely + /// functional and only depends on the [`PathId`] **during a single** `poll_transmit` + /// call. It can be computed up-front for all paths but we don't do that because it + /// involves an allocation. + /// + /// See the inline comments for how the packet scheduling works. + /// + /// # Panics + /// + /// This will panic if called for a path for which we do not have any [`PathData`], like + /// so many other functions we have. But this is the only one to document this in its + /// doc comment. Maybe that should change. Eventually we'll refactor things for this + /// panic to go away. + fn scheduling_info(&self, path_id: PathId) -> PathSchedulingInfo { + let have_validated_status_available_space = self.paths.iter().any(|(path_id, path)| { + self.remote_cids.contains_key(path_id) + && !self.abandoned_paths.contains(path_id) + && path.data.validated + && path.data.local_status() == PathStatus::Available + }); + let is_handshaking = self.is_handshaking(); + let has_cids = self.remote_cids.contains_key(&path_id); + let abandoned = self.abandoned_paths.contains(&path_id); + let path_data = self.path_data(path_id); + let validated = path_data.validated; + let status = path_data.local_status(); + + // This is the core packet scheduling, whether this space ID may send + // SpaceKind::Data frames. + let may_send_data = has_cids + && !abandoned + && if is_handshaking { + // There is only one path during the handshake. We want to + // already send 0-RTT and 0.5-RTT (permitting anti-amplification + // limit) data. + true + } else if !validated { + // TODO(flub): When we have a network change we might end up + // having to abandon all paths and re-open new ones to the + // same remotes. This leaves us without any validated + // path. Perhaps we should have a way to figure out if the + // path is to a previously-validated remote address and allow + // sending data to such remotes immediately. + false + } else { + match status { + PathStatus::Available => { + // Best possible space to send data on. + true + } + PathStatus::Backup => { + // If there is a status-available path we prefer that. + !have_validated_status_available_space + } + } + }; + + // CONNECTION_CLOSE is allowed to be sent on a non-validated + // path. Particularly during the handshake we want to send it before the + // path is validated. Later if there is no validated path available we + // will also accept sending it on an un-validated path. + let may_send_close = has_cids + && !abandoned + && if !validated && have_validated_status_available_space { + // We have a better space to send on. + false + } else { + // No other validated space, this is as good as it gets. + true + }; + + PathSchedulingInfo { + abandoned, + may_send_data, + may_send_close, + } + } + fn build_transmit(&mut self, path_id: PathId, transmit: TransmitBuf<'_>) -> Transmit { debug_assert!( !transmit.is_empty(), From f07f4438c9b9cc392d362ec78e78316d19bb3782 Mon Sep 17 00:00:00 2001 From: Floris Bruynooghe Date: Mon, 16 Mar 2026 15:22:39 +0100 Subject: [PATCH 41/44] Small attempt at making this clearer --- noq-proto/src/connection/mod.rs | 8 ++++---- noq-proto/src/connection/spaces.rs | 27 +++++++++++++++------------ 2 files changed, 19 insertions(+), 16 deletions(-) diff --git a/noq-proto/src/connection/mod.rs b/noq-proto/src/connection/mod.rs index af6d15ab8..c250a20bd 100644 --- a/noq-proto/src/connection/mod.rs +++ b/noq-proto/src/connection/mod.rs @@ -1389,7 +1389,7 @@ impl Connection { } else if can_send.close && scheduling_info.may_send_close { // This is the best path to send a CONNECTION_CLOSE on. true - } else if needs_loss_probe || can_send.space_id_only { + } else if needs_loss_probe || can_send.space_only { // We always send a loss probe or space-specific frames if the path is // not abandoned. true @@ -1640,7 +1640,7 @@ impl Connection { debug_assert!( !(builder.sent_frames().is_ack_only(&self.streams) && !can_send.acks - && (can_send.other || can_send.space_id_only) + && (can_send.other || can_send.space_only) && builder.buf.segment_size() == self.path_data(path_id).current_mtu() as usize && self.datagrams.outgoing.is_empty()), @@ -6464,7 +6464,7 @@ impl Connection { /// See also [`PacketSpace::can_send`] which keeps track of all other frame types that /// may need to be sent. fn can_send_1rtt(&self, path_id: PathId, max_size: usize) -> SendableFrames { - let space_id_only = self.paths.get(&path_id).is_some_and(|path| { + let space_only = self.paths.get(&path_id).is_some_and(|path| { path.data.pending_on_path_challenge || !path.data.path_responses.is_empty() }); @@ -6480,7 +6480,7 @@ impl Connection { SendableFrames { acks: false, close: false, - space_id_only, + space_only, other, } } diff --git a/noq-proto/src/connection/spaces.rs b/noq-proto/src/connection/spaces.rs index 65ab75a73..38394a3ad 100644 --- a/noq-proto/src/connection/spaces.rs +++ b/noq-proto/src/connection/spaces.rs @@ -153,15 +153,15 @@ impl PacketSpace { .number_spaces .values() .any(|pns| pns.pending_acks.can_send()); - let space_id_only = self + let space_only = self .number_spaces .get(&path_id) .is_some_and(|s| s.ping_pending || s.immediate_ack_pending); - let other = !self.pending.is_empty(streams) || space_id_only; + let other = !self.pending.is_empty(streams) || space_only; SendableFrames { acks, close: false, - space_id_only, + space_only, other, } } @@ -916,11 +916,14 @@ pub(super) struct SendableFrames { pub(super) acks: bool, /// Whether there is a CONNECTION_CLOSE to send, this is not ack-eliciting. pub(super) close: bool, - /// Whether there are any frames that must be sent on this specific space ID. + /// Whether there are any frames that must be sent on this specific space. + /// + /// A space here in the sense of a QUIC Multipath packet number space: `Initial`, + /// `Handshake` and all `Data(PathId)` spaces. /// /// These are ack-eliciting. Some frames are scheduled per path, e.g. PING, /// IMMEDIATE_ACK, PATH_CHALLENGE or PATH_RESPONSE. - pub(super) space_id_only: bool, + pub(super) space_only: bool, /// Whether there are any other frames to send, these are ack-eliciting. pub(super) other: bool, } @@ -931,7 +934,7 @@ impl SendableFrames { Self { acks: false, close: false, - space_id_only: false, + space_only: false, other: false, } } @@ -941,14 +944,14 @@ impl SendableFrames { let Self { acks: _, close, - space_id_only, + space_only, other, } = *self; if close { // No ack-eliciting frames are included with a CONNECTION_CLOSE, only acks. return false; } - space_id_only || other + space_only || other } /// Whether no data is sendable. @@ -956,10 +959,10 @@ impl SendableFrames { let Self { acks, close, - space_id_only, + space_only, other, } = *self; - !acks && !close && !space_id_only && !other + !acks && !close && !space_only && !other } } @@ -968,13 +971,13 @@ impl ::std::ops::BitOrAssign for SendableFrames { let Self { acks, close, - space_id_only, + space_only, other, } = rhs; self.acks |= acks; self.close |= close; - self.space_id_only |= space_id_only; + self.space_only |= space_only; self.other |= other; } } From 16c9164f68c4d1a5f7e9c879bc5ccc4316c4435a Mon Sep 17 00:00:00 2001 From: Floris Bruynooghe Date: Mon, 16 Mar 2026 15:27:29 +0100 Subject: [PATCH 42/44] fix docs --- noq-proto/src/connection/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/noq-proto/src/connection/mod.rs b/noq-proto/src/connection/mod.rs index c250a20bd..6a1237a39 100644 --- a/noq-proto/src/connection/mod.rs +++ b/noq-proto/src/connection/mod.rs @@ -6893,7 +6893,7 @@ struct PathSchedulingInfo { abandoned: bool, /// Whether the path may send [`SpaceKind::Data`] frames. /// - /// Some paths should only send frames from [`SendableFrames::space_id_only`]. All other + /// Some paths should only send frames from [`SendableFrames::space_only`]. All other /// frames are essentially frames that can be sent on any [`SpaceKind::Data`] space. For /// those we want to respect packet scheduling rules however. /// From 298b7beb57ae2a5a545bc63b288e9c5fd9991cf5 Mon Sep 17 00:00:00 2001 From: Floris Bruynooghe Date: Thu, 19 Mar 2026 11:29:06 +0100 Subject: [PATCH 43/44] Rename field based on more feedback --- noq-proto/src/connection/mod.rs | 8 ++++---- noq-proto/src/connection/spaces.rs | 22 +++++++++++----------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/noq-proto/src/connection/mod.rs b/noq-proto/src/connection/mod.rs index 6a1237a39..8b8263088 100644 --- a/noq-proto/src/connection/mod.rs +++ b/noq-proto/src/connection/mod.rs @@ -1389,7 +1389,7 @@ impl Connection { } else if can_send.close && scheduling_info.may_send_close { // This is the best path to send a CONNECTION_CLOSE on. true - } else if needs_loss_probe || can_send.space_only { + } else if needs_loss_probe || can_send.space_specific { // We always send a loss probe or space-specific frames if the path is // not abandoned. true @@ -1640,7 +1640,7 @@ impl Connection { debug_assert!( !(builder.sent_frames().is_ack_only(&self.streams) && !can_send.acks - && (can_send.other || can_send.space_only) + && (can_send.other || can_send.space_specific) && builder.buf.segment_size() == self.path_data(path_id).current_mtu() as usize && self.datagrams.outgoing.is_empty()), @@ -6464,7 +6464,7 @@ impl Connection { /// See also [`PacketSpace::can_send`] which keeps track of all other frame types that /// may need to be sent. fn can_send_1rtt(&self, path_id: PathId, max_size: usize) -> SendableFrames { - let space_only = self.paths.get(&path_id).is_some_and(|path| { + let space_specific = self.paths.get(&path_id).is_some_and(|path| { path.data.pending_on_path_challenge || !path.data.path_responses.is_empty() }); @@ -6480,7 +6480,7 @@ impl Connection { SendableFrames { acks: false, close: false, - space_only, + space_specific, other, } } diff --git a/noq-proto/src/connection/spaces.rs b/noq-proto/src/connection/spaces.rs index 38394a3ad..ab3bdd35c 100644 --- a/noq-proto/src/connection/spaces.rs +++ b/noq-proto/src/connection/spaces.rs @@ -153,15 +153,15 @@ impl PacketSpace { .number_spaces .values() .any(|pns| pns.pending_acks.can_send()); - let space_only = self + let space_specific = self .number_spaces .get(&path_id) .is_some_and(|s| s.ping_pending || s.immediate_ack_pending); - let other = !self.pending.is_empty(streams) || space_only; + let other = !self.pending.is_empty(streams) || space_specific; SendableFrames { acks, close: false, - space_only, + space_specific, other, } } @@ -923,7 +923,7 @@ pub(super) struct SendableFrames { /// /// These are ack-eliciting. Some frames are scheduled per path, e.g. PING, /// IMMEDIATE_ACK, PATH_CHALLENGE or PATH_RESPONSE. - pub(super) space_only: bool, + pub(super) space_specific: bool, /// Whether there are any other frames to send, these are ack-eliciting. pub(super) other: bool, } @@ -934,7 +934,7 @@ impl SendableFrames { Self { acks: false, close: false, - space_only: false, + space_specific: false, other: false, } } @@ -944,14 +944,14 @@ impl SendableFrames { let Self { acks: _, close, - space_only, + space_specific, other, } = *self; if close { // No ack-eliciting frames are included with a CONNECTION_CLOSE, only acks. return false; } - space_only || other + space_specific || other } /// Whether no data is sendable. @@ -959,10 +959,10 @@ impl SendableFrames { let Self { acks, close, - space_only, + space_specific, other, } = *self; - !acks && !close && !space_only && !other + !acks && !close && !space_specific && !other } } @@ -971,13 +971,13 @@ impl ::std::ops::BitOrAssign for SendableFrames { let Self { acks, close, - space_only, + space_specific, other, } = rhs; self.acks |= acks; self.close |= close; - self.space_only |= space_only; + self.space_specific |= space_specific; self.other |= other; } } From 6bbe83c1b69ff44094745b5c60d8406d9db41e87 Mon Sep 17 00:00:00 2001 From: Floris Bruynooghe Date: Thu, 19 Mar 2026 11:39:58 +0100 Subject: [PATCH 44/44] fix docs --- noq-proto/src/connection/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/noq-proto/src/connection/mod.rs b/noq-proto/src/connection/mod.rs index 8b8263088..c08a14fed 100644 --- a/noq-proto/src/connection/mod.rs +++ b/noq-proto/src/connection/mod.rs @@ -6893,7 +6893,7 @@ struct PathSchedulingInfo { abandoned: bool, /// Whether the path may send [`SpaceKind::Data`] frames. /// - /// Some paths should only send frames from [`SendableFrames::space_only`]. All other + /// Some paths should only send frames from [`SendableFrames::space_specific`]. All other /// frames are essentially frames that can be sent on any [`SpaceKind::Data`] space. For /// those we want to respect packet scheduling rules however. ///