diff --git a/noq-proto/src/connection/mod.rs b/noq-proto/src/connection/mod.rs index 881e9536d..5c5701c9d 100644 --- a/noq-proto/src/connection/mod.rs +++ b/noq-proto/src/connection/mod.rs @@ -590,7 +590,7 @@ impl Connection { return Err(PathError::RemoteCidsExhausted); } - let path = self.ensure_path(path_id, network_path, now, None); + let path = self.create_path(path_id, network_path, now, None); path.status.local_update(initial_status); Ok(path_id) @@ -717,9 +717,7 @@ impl Connection { // These timers deal with sending and receiving PATH_CHALLENGE and // PATH_RESPONSE, but now that the path is abandoned, we no longer care about // these frames or their timing - PathTimer::PathValidationFailed - | PathTimer::PathChallengeLost - | PathTimer::AbandonFromValidation => false, + PathTimer::PathValidationFailed | PathTimer::PathChallengeLost => false, // These timers deal with the lifetime of the path. Now that the path is abandoned, // these are not relevant. PathTimer::PathKeepAlive | PathTimer::PathIdle => false, @@ -929,7 +927,7 @@ impl Connection { /// Creates the [`PathData`] for a new [`PathId`]. /// /// Called for incoming packets as well as when opening a new path locally. - fn ensure_path( + fn create_path( &mut self, path_id: PathId, network_path: FourTuple, @@ -978,6 +976,18 @@ impl Connection { .address_discovery_role .should_report(&self.peer_params.address_discovery_role); + // if !data.validated { + // // Hard cap on how long to try validation for. + // // TODO(flub): 3 * PTO is way too short, it makes opening paths very susceptible + // // to packet loss. I prefer using the path idle timeout probably. + // let pto = self.ack_frequency.max_ack_delay_for_pto() + data.rtt.pto_base(); + // self.timers.set( + // Timer::PerPath(path_id, PathTimer::AbandonFromValidation), + // now + 3 * pto, + // self.qlog.with_time(now), + // ); + // } + let path = vacant_entry.insert(PathState { data, prev: None }); let mut pn_space = spaces::PacketNumberSpace::new(now, SpaceId::Data, &mut self.rng); @@ -2527,24 +2537,11 @@ impl Connection { trace!(?path.data.lost_challenge_count, "path challenge deemed lost"); path.data.pending_challenge = true; path.data.lost_challenge_count += 1; - } - PathTimer::AbandonFromValidation => { - let Some(path) = self.paths.get_mut(&path_id) else { - continue; - }; - path.data.reset_on_path_challenges(); - self.timers.stop( + self.timers.set( Timer::PerPath(path_id, PathTimer::PathChallengeLost), + now + path.data.on_path_challenge_pto(), self.qlog.with_time(now), ); - debug!("new path validation failed"); - if let Err(err) = self.close_path_inner( - now, - path_id, - PathAbandonReason::ValidationFailed, - ) { - warn!(?err, "failed closing path"); - } } PathTimer::Pacing => {} PathTimer::MaxAckDelay => { @@ -4349,7 +4346,7 @@ impl Connection { if self.side().is_server() && !self.abandoned_paths.contains(&path_id) { // Only the client is allowed to open paths - self.ensure_path(path_id, network_path, now, pn); + self.create_path(path_id, network_path, now, pn); } if self.paths.contains_key(&path_id) { self.on_packet_authenticated( @@ -5622,10 +5619,6 @@ impl Connection { Timer::PerPath(path_id, PathTimer::PathValidationFailed), qlog.clone(), ); - self.timers.stop( - Timer::PerPath(path_id, PathTimer::AbandonFromValidation), - qlog.clone(), - ); let next_challenge = path .data .earliest_on_path_expiring_challenge() @@ -6179,23 +6172,14 @@ impl Connection { let challenge = frame::PathChallenge(token); builder.write_frame(challenge, stats); builder.require_padding(); - let pto = self.ack_frequency.max_ack_delay_for_pto() + path.rtt.pto_base(); - let pns = space.for_path(path_id); - match pns.open_status { - OpenStatus::Sent | OpenStatus::Informed => {} - OpenStatus::Pending => { - pns.open_status = OpenStatus::Sent; - self.timers.set( - Timer::PerPath(path_id, PathTimer::AbandonFromValidation), - now + 3 * pto, - self.qlog.with_time(now), - ); - } - } + // On-path challenges need a PATH_RESPONSE and not only an ACK which can be + // received on any path. So set a timer manually instead of relying on the usual + // LossDetection/PTO timer. This timer interval keeps exponentially increasing + // with missing responses like the normal PTO interval. self.timers.set( Timer::PerPath(path_id, PathTimer::PathChallengeLost), - now + path.on_path_challenge_expiry(), + now + path.on_path_challenge_pto(), self.qlog.with_time(now), ); diff --git a/noq-proto/src/connection/paths.rs b/noq-proto/src/connection/paths.rs index e7a6ed8c6..cca56bcea 100644 --- a/noq-proto/src/connection/paths.rs +++ b/noq-proto/src/connection/paths.rs @@ -477,14 +477,22 @@ impl PathData { if self.unconfirmed_challenges.is_empty() { return None; } - let duration = self.on_path_challenge_expiry(); + let duration = self.on_path_challenge_pto(); self.unconfirmed_challenges .values() .map(|info| info.sent_instant + duration) .min() } - pub(super) fn on_path_challenge_expiry(&self) -> Duration { + /// The duration after which a PTO expires for an on-path challenge, if sent now. + /// + /// Since challenges need an on-path response rather than just an ACK that can be sent + /// on any path they need a different timer from the + /// [`PathTimer::LossDetection`]. Functionally this behaves as the probe timeout + /// however. + /// + /// [`PathTimer::LossDetection`]: super::timer::PathTimer::LossDetection + pub(super) fn on_path_challenge_pto(&self) -> Duration { let backoff = 2u32.pow(self.lost_challenge_count.min(MAX_BACKOFF_EXPONENT)); let duration = self.rtt.pto_base() * backoff; duration.min(MAX_PTO_INTERVAL) @@ -1095,6 +1103,10 @@ pub enum PathAbandonReason { error_code: VarInt, }, /// We didn't receive a path response in time after opening this path. + /// + /// This event is no longer emitted, when validation fails a path is only abandoned once + /// there's a path timeout and the [`Self::TimedOut`] event would be emitted instead. + // TODO(flub): should this be deprecated? ValidationFailed, /// We didn't receive any data from the remote within the path's idle timeout. TimedOut, diff --git a/noq-proto/src/connection/qlog.rs b/noq-proto/src/connection/qlog.rs index ed18865e9..5925a9f7c 100644 --- a/noq-proto/src/connection/qlog.rs +++ b/noq-proto/src/connection/qlog.rs @@ -323,9 +323,6 @@ impl QlogSink { PathTimer::PathIdle => Some(TimerType::custom("path_idle")), PathTimer::PathValidationFailed => Some(QlogTimerType::PathValidation.into()), PathTimer::PathChallengeLost => Some(TimerType::custom("path_challenge_lost")), - PathTimer::AbandonFromValidation => { - Some(TimerType::custom("abandon_from_validation")) - } PathTimer::PathKeepAlive => Some(TimerType::custom("path_keep_alive")), PathTimer::Pacing => Some(TimerType::custom("pacing")), PathTimer::MaxAckDelay => Some(QlogTimerType::Ack.into()), diff --git a/noq-proto/src/connection/spaces.rs b/noq-proto/src/connection/spaces.rs index 0f9d70d72..ebaa14739 100644 --- a/noq-proto/src/connection/spaces.rs +++ b/noq-proto/src/connection/spaces.rs @@ -496,12 +496,9 @@ impl PacketNumberSpace { #[derive(Debug, Default, Clone, Copy, PartialEq, Eq)] pub(super) enum OpenStatus { - /// A first packet has not been sent using this [`PathId`]. + /// The application has not yet been informed of this path. #[default] Pending, - /// The first packet has been sent using this [`PathId`]. However, it is not yet deemed good - /// enough to be reported to the application. - Sent, /// The application has been informed of this path. Informed, } diff --git a/noq-proto/src/connection/timer.rs b/noq-proto/src/connection/timer.rs index 258140f28..1613b41e2 100644 --- a/noq-proto/src/connection/timer.rs +++ b/noq-proto/src/connection/timer.rs @@ -60,32 +60,22 @@ pub(crate) enum PathTimer { PathValidationFailed = 2, /// When to resend an on-path path challenge deemed lost PathChallengeLost = 3, - /// When to abandon a path due to failed validation. - /// - /// There are two situations in which we give up a path from validation. - /// 1. When opening a path we validate it according to RFC9000 ยง8.2 as required by the - /// multipath spec. This timer is armed to time-bound that validation. - /// 2. When validating an already opened multipath path for various reasons in which we - /// expect the path to either work or not work and want to respond correspondingly in - /// a timely manner. - AbandonFromValidation = 4, /// When to send a `PING` frame to keep the path alive - PathKeepAlive = 5, + PathKeepAlive = 4, /// When pacing will allow us to send a packet - Pacing = 6, + Pacing = 5, /// When to send an immediate ACK if there are unacked ack-eliciting packets of the peer - MaxAckDelay = 7, + MaxAckDelay = 6, /// When to clean up state for an abandoned path - PathDrained = 8, + PathDrained = 7, } impl PathTimer { - pub(super) const VALUES: [Self; 9] = [ + pub(super) const VALUES: [Self; 8] = [ Self::LossDetection, Self::PathIdle, Self::PathValidationFailed, Self::PathChallengeLost, - Self::AbandonFromValidation, Self::PathKeepAlive, Self::Pacing, Self::MaxAckDelay, diff --git a/noq-proto/src/tests/mod.rs b/noq-proto/src/tests/mod.rs index 34c62e26c..fb6e6dcf0 100644 --- a/noq-proto/src/tests/mod.rs +++ b/noq-proto/src/tests/mod.rs @@ -1506,72 +1506,6 @@ fn migration() { ); } -#[test] -fn path_challenge_retransmit() { - let _guard = subscribe(); - let mut pair = Pair::default(); - let (client_ch, server_ch) = pair.connect(); - pair.drive(); - - pair.client_conn_mut(client_ch).ping(); - pair.drive(); - - println!("-------- server wants path validation --------"); - pair.server_conn_mut(server_ch).trigger_path_validation(); - pair.drive_server(); // Send the path challenge - println!("-------- client loses messages --------"); - // Have the client lose the challenge - pair.client.inbound.clear(); - - pair.drive(); - - let client_tx = pair.client_conn_mut(client_ch).stats().frame_tx; - let server_tx = pair.server_conn_mut(server_ch).stats().frame_tx; - - assert_eq!( - server_tx.path_challenge, 2, - "expected server to send two path challenges" - ); - assert_eq!( - client_tx.path_response, 1, - "expected client to send one path response" - ); -} - -#[test] -fn path_response_retransmit() { - let _guard = subscribe(); - let mut pair = Pair::default(); - let (client_ch, server_ch) = pair.connect(); - pair.drive(); - - pair.client_conn_mut(client_ch).ping(); - pair.drive(); - - println!("-------- server wants path validation --------"); - pair.server_conn_mut(server_ch).trigger_path_validation(); - pair.drive_server(); // Send the path challenge - pair.drive_client(); // Send the path response - println!("-------- server loses messages --------"); - // Have the server lose the path response - pair.server.inbound.clear(); - - // The server should decide to re-send the path challenge - pair.drive(); - - let client_tx = pair.client_conn_mut(client_ch).stats().frame_tx; - let server_tx = pair.server_conn_mut(server_ch).stats().frame_tx; - - assert_eq!( - server_tx.path_challenge, 2, - "expected server to send two path challenges" - ); - assert_eq!( - client_tx.path_response, 2, - "expected client to send two path responses" - ); -} - /// Regression test: handle sent challenges that are waiting for a response while a passive /// migration occurs. /// diff --git a/noq-proto/src/tests/multipath.rs b/noq-proto/src/tests/multipath.rs index 4a45c37a6..e7d136188 100644 --- a/noq-proto/src/tests/multipath.rs +++ b/noq-proto/src/tests/multipath.rs @@ -412,72 +412,115 @@ fn open_path_key_update() -> TestResult { Ok(()) } -/// Client starts opening a path but the server fails to validate the path -/// -/// The client should receive an event closing the path. -#[test] -fn open_path_validation_fails_server_side() -> TestResult { - let _guard = subscribe(); - let mut pair = ConnPair::builder().enable_multipath().connect(); - - let different_addr = FourTuple { - remote: SocketAddr::new([9, 8, 7, 6].into(), 5), - local_ip: None, - }; - assert_ne!(different_addr.remote, Pair::SERVER_ADDR); - assert_ne!(different_addr.remote, Pair::CLIENT_ADDR); - let path_id = pair.open_path(Client, different_addr, PathStatus::Available)?; - - // block the server from receiving anything - while pair.blackhole_step(true, false) {} - assert_matches!( - pair.poll(Client), - Some(Event::Path(crate::PathEvent::Abandoned { id, reason: PathAbandonReason::ValidationFailed })) if id == path_id - ); - - assert!(pair.poll(Server).is_none()); - Ok(()) -} - -/// Client starts opening a path but the client fails to validate the path +// /// Client starts opening a path but the server fails to validate the path +// /// +// /// The client should receive an event closing the path. +// #[test] +// fn open_path_validation_fails_server_side() -> TestResult { +// let _guard = subscribe(); +// let mut pair = ConnPair::builder().enable_multipath().connect(); + +// let different_addr = FourTuple { +// remote: SocketAddr::new([9, 8, 7, 6].into(), 5), +// local_ip: None, +// }; +// assert_ne!(different_addr.remote, Pair::SERVER_ADDR); +// assert_ne!(different_addr.remote, Pair::CLIENT_ADDR); +// let path_id = pair.open_path(Client, different_addr, PathStatus::Available)?; + +// // block the server from receiving anything +// while pair.blackhole_step(true, false) {} +// assert_matches!( +// pair.poll(Client), +// Some(Event::Path(crate::PathEvent::Abandoned { id, reason: PathAbandonReason::ValidationFailed })) if id == path_id +// ); + +// assert!(pair.poll(Server).is_none()); +// Ok(()) +// } + +/// client starts opening a path but the client fails to validate the path /// /// The server should receive an event close the path #[test] fn open_path_validation_fails_client_side() -> TestResult { let _guard = subscribe(); - let mut pair = ConnPair::builder().enable_multipath().connect(); - - // make sure the new path cannot be validated using the existing path - let new_addr = SocketAddr::new([9, 8, 7, 6].into(), 5); - assert_ne!(new_addr, Pair::SERVER_ADDR); - assert_ne!(new_addr, Pair::CLIENT_ADDR); - pair.routes.as_basic_mut().client_addr = new_addr; + let client_addr_two = "[::1:2]:1".parse()?; + let server_addr_two = "[::2:2]:1".parse()?; + assert_ne!(client_addr_two, Pair::SERVER_ADDR); + assert_ne!(client_addr_two, Pair::CLIENT_ADDR); + assert_ne!(server_addr_two, Pair::SERVER_ADDR); + assert_ne!(server_addr_two, Pair::CLIENT_ADDR); + let mut builder = ConnPair::builder() + .enable_multipath() + .disable_mtud_discovery() + .with_routes( + // client_addr_two can send to server_addr_two, but the reverse is broken. + ManyToManyRouting::from_routes( + vec![(Pair::CLIENT_ADDR, 0), (client_addr_two, 0)], + vec![(Pair::SERVER_ADDR, 0), (server_addr_two, 1)], + ), + ); + builder + .server_transport_cfg + .default_path_max_idle_timeout(Some(Duration::from_secs(30))); + builder + .client_transport_cfg + .default_path_max_idle_timeout(Some(Duration::from_secs(30))); + let mut pair = builder.connect(); + // Open a path from ::1:2 to ::2:2. Client datagrams can be routed, server datagrams + // will come from ::2:2 from ::1:2 but only ::2:1 is allowed to send to ::1:2, so server + // datagrams will be dropped.. This might be crazy, please file complaints to + // ManyToManyRouting. let network_path = FourTuple { - remote: pair.routes.public_server_addr(), + remote: server_addr_two, local_ip: None, }; let path_id = pair.open_path(Client, network_path, PathStatus::Available)?; - // Make sure the client's path open makes it through to the server and is processed. - pair.drive_client(); - pair.drive_server(); + // // make sure the client's path open makes it through to the server and is processed. + // pair.drive_client(); + // pair.drive_server(); + + // info!("dropping client inbound queue"); + // pair.client.inbound.clear(); - info!("dropping client inbound queue"); - pair.client.inbound.clear(); + // // Sever the connection and run it to idle. + // // This makes sure that + // // - path validation can't succeed because path responses don't make it through and + // // - the server needs to decide to close the path on its own, because path abandons + // // don't make it through. + // while pair.blackhole_step(true, true) {} - // Sever the connection and run it to idle. - // This makes sure that - // - path validation can't succeed because path responses don't make it through and - // - the server needs to decide to close the path on its own, because path abandons - // don't make it through. - while pair.blackhole_step(true, true) {} + info!("drive to idle"); + pair.drive(); + + info!("manual keep-alive of PathId::ZERO"); + // Though some ACKs will have been flowing over this path already, keeping it alive. + pair.ping_path(Client, PathId::ZERO)?; + pair.ping_path(Client, PathId::ZERO)?; + + info!("drive past idle"); + // min_opt(pair.client.next_wakeup(), pair.server.next_wakeup()).map(|t| pair.time = t); + // pair.blackhole_step(true, true); + pair.drive(); + // The client gave up first and timed out. assert_matches!( - pair.poll(Server), - Some(Event::Path(PathEvent::Abandoned { id, reason: PathAbandonReason::ValidationFailed })) + pair.poll(Client), + Some(Event::Path(PathEvent::Abandoned { id, reason: PathAbandonReason::TimedOut })) if id == path_id ); + // Server would also fail to validate, but the client already abandoned. + assert_matches!( + pair.poll(Server), + Some(Event::Path(PathEvent::Abandoned { + id, + reason: PathAbandonReason::RemoteAbandoned { .. } + })) + if id == path_id + ); Ok(()) } @@ -1772,7 +1815,7 @@ fn test_simple_nat_traveral_opens_path() -> TestResult { let mut pair = ConnPair::builder() .enable_multipath() .enable_nat_traversal() - .with_routes(SimpleFirewallRouting::new().into()) + .with_routes(SimpleFirewallRouting::new()) .connect(); info!("adding addrs"); @@ -1876,7 +1919,7 @@ fn test_hard_nat_client_opens_path() -> TestResult { let mut pair = ConnPair::builder() .enable_multipath() .enable_nat_traversal() - .with_routes(routing.into()) + .with_routes(routing) .connect(); info!("adding addrs"); @@ -1926,7 +1969,7 @@ fn test_hard_nat_server_opens_path() -> TestResult { let mut pair = ConnPair::builder() .enable_multipath() .enable_nat_traversal() - .with_routes(routing.into()) + .with_routes(routing) .connect(); info!("adding addrs"); @@ -1972,7 +2015,7 @@ fn test_peer_may_probe() -> TestResult { .enable_multipath() .enable_nat_traversal() .disable_mtud_discovery() - .with_routes(SimpleFirewallRouting::new().into()); + .with_routes(SimpleFirewallRouting::new()); let server_cfg = ServerConfig { transport: Arc::new(builder.server_transport_cfg.clone()), migration: false, @@ -2009,113 +2052,78 @@ fn test_peer_may_probe() -> TestResult { } #[test] -fn on_path_challenge_lost_backoff() { +fn path_open_challenge_lost() -> TestResult { let _guard = subscribe(); + let mut pair = ConnPair::builder() + .enable_multipath() + .disable_mtud_discovery() + .connect(); - let mut pair = ConnPair::default(); - - // We use two helpers so we can "skip over" unrelated stopping points from `next_wakeup` - - /// Attempts to drive the client until it has sent the expected amount of path challenges. - /// - /// Tries for a maximum of 10 iterations. Panics if that fails. - fn drive_client_until_challenge_sent(pair: &mut ConnPair, expected_path_challenges_sent: u64) { - for _ in 0..10 { - pair.drive_client(); - if pair.stats(Client).frame_tx.path_challenge == expected_path_challenges_sent { - return; - } - pair.time = pair - .client - .next_wakeup() - .expect("couldn't drive client forward"); - info!("advancing to {:?} for client", pair.time - pair.epoch); - } - panic!( - "client never sent PATH_CHALLENGE #{}, actual: {}", - expected_path_challenges_sent, - pair.stats(Client).frame_tx.path_challenge - ); - } - - /// Attempts to drive the server until it has received the expected amount of path challenges. - /// - /// Tries for a maximum of 10 iterations. Panics if that fails. - fn drive_server_until_challenge_received( - pair: &mut ConnPair, - expected_path_challenges_received: u64, - ) { - for _ in 0..10 { - // attempt to drive the server until it receives the PATH_CHALLENGE and sends a PATH_RESPONSE - pair.time = pair - .server - .next_wakeup() - .expect("couldn't drive server forward"); - info!("advancing to {:?} for server", pair.time - pair.epoch); - pair.drive_server(); - if pair.stats(Server).frame_rx.path_challenge == expected_path_challenges_received - && pair.stats(Server).frame_tx.path_response == expected_path_challenges_received - { - return; - } - } - panic!( - "server never received PATH_CHALLENGE #{}, actual: {}", - expected_path_challenges_received, - pair.stats(Server).frame_rx.path_challenge - ); - } - - pair.conn_mut(Client).trigger_path_validation(); - - // Kickstart the process once to get a reference point for the last_challenge_sent - drive_client_until_challenge_sent(&mut pair, 1); - let mut last_challenge_send = pair.time; - let mut last_duration = Duration::ZERO; - - const MAX_DURATION: Duration = Duration::from_secs(2); // equivalent to MAX_PTO_INTERVAL - const MAX_ITERS: u64 = MAX_DURATION.as_millis().ilog2() as u64 + 2; - - for i in 1..=MAX_ITERS { - drive_server_until_challenge_received(&mut pair, i); - pair.client.inbound.clear(); // we drop the client-inbound PATH_RESPONSE - info!("dropped client inbound"); - - drive_client_until_challenge_sent(&mut pair, i + 1); - let time = pair.time.duration_since(last_challenge_send); - info!(?time, ?last_duration, "time since last PATH_CHALLENGE send"); - assert!( - time >= last_duration, - "duration between PATH_CHALLENGE sends must be monotonically increasing (backing off)" - ); - assert!( - time <= MAX_DURATION, - "duration between PATH_CHALLENGE sends must be bound to a maximum of 2s" - ); - if i == MAX_ITERS { - assert_eq!(time, MAX_DURATION); - } - last_duration = time; - last_challenge_send = pair.time; - } - - // Now we stop dropping the client inbound once and the backoff should return back to normal: + info!("opening path"); + let server_addr = pair.routes.public_server_addr(); + let path_id = pair.open_path( + Client, + FourTuple::from_remote(server_addr), + PathStatus::Available, + )?; + let first_challenge = pair.time; + pair.drive_client(); - drive_server_until_challenge_received(&mut pair, MAX_ITERS + 1); - pair.drive(); // Ensure the client fully processes the PATH_RESPONSE this time. - info!("client should have processed PATH_RESPONSE"); + // Drop the 1st PATH_CHALLENGE + let client_stats = pair.path_stats(Client, path_id).unwrap(); + assert_eq!(client_stats.frame_tx.path_challenge, 1); + assert!(pair.path_stats(Server, path_id).is_none()); + info!("dropping client's packet"); + pair.server.inbound.clear(); + + // Drop the 2nd PATH_CHALLENGE + pair.time = pair + .client + .next_wakeup() + .expect("couldn't drive client forward"); + info!("advancing to {:?} for client", pair.time - pair.epoch); + let second_challenge = pair.time; + pair.drive_client(); + let client_stats = pair.path_stats(Client, path_id).unwrap(); + assert_eq!(client_stats.frame_tx.path_challenge, 2); + assert!(pair.path_stats(Server, path_id).is_none()); + info!("dropping client's packet"); + pair.server.inbound.clear(); + + // Send the 3rd PATH_CHALLENGE + pair.time = pair + .client + .next_wakeup() + .expect("couldn't drive client forward"); + info!("advancing to {:?} for client", pair.time - pair.epoch); + let third_challenge = pair.time; + pair.drive_client(); + let client_stats = pair.path_stats(Client, path_id).unwrap(); + assert_eq!(client_stats.frame_tx.path_challenge, 3); + assert!(pair.path_stats(Server, path_id).is_none()); - // The next time challenges are sent, the backoff should be reset. + let first_delay = second_challenge - first_challenge; + let second_delay = third_challenge - second_challenge; + assert!( + first_delay < second_delay, + "delays must monotonically increase" + ); - pair.conn_mut(Client).trigger_path_validation(); - drive_client_until_challenge_sent(&mut pair, MAX_ITERS + 2); - last_challenge_send = pair.time; - drive_server_until_challenge_received(&mut pair, MAX_ITERS + 2); - pair.client.inbound.clear(); // we drop the client-inbound PATH_RESPONSE - info!("dropped client inbound"); - drive_client_until_challenge_sent(&mut pair, MAX_ITERS + 3); - let duration = pair.time.duration_since(last_challenge_send); - assert_eq!(duration, Duration::from_millis(1)); + // Open path, drive to completion + pair.drive_server(); + let server_stats = pair.path_stats(Server, path_id).unwrap(); + assert_eq!(server_stats.frame_rx.path_challenge, 1); + pair.advance_time(); + pair.drive(); + assert_matches!( + pair.poll(Client), + Some(Event::Path(crate::PathEvent::Established { id })) if id == path_id + ); + assert_matches!( + pair.poll(Server), + Some(Event::Path(crate::PathEvent::Established { id })) if id == path_id + ); + Ok(()) } #[test] diff --git a/noq-proto/src/tests/util.rs b/noq-proto/src/tests/util.rs index fb48a4edf..088819b16 100644 --- a/noq-proto/src/tests/util.rs +++ b/noq-proto/src/tests/util.rs @@ -569,8 +569,8 @@ impl ConnPairBuilder { } /// Sets the [`Routing`] to use. - pub(super) fn with_routes(mut self, routes: Routing) -> Self { - self.routes = Some(routes); + pub(super) fn with_routes(mut self, routes: impl Into) -> Self { + self.routes = Some(routes.into()); self }