diff --git a/noq-proto/src/connection/mod.rs b/noq-proto/src/connection/mod.rs index 898bd58a7f..e2b279d6d6 100644 --- a/noq-proto/src/connection/mod.rs +++ b/noq-proto/src/connection/mod.rs @@ -2491,6 +2491,26 @@ impl Connection { self.close_path_inner(now, path_id, PathAbandonReason::TimedOut) { warn!(?err, "failed closing path"); + // The path could not be abandoned because it is the last + // remaining open path. Re-arm the idle timer so the close is + // retried once another path becomes available. Without this the + // timed-out path would be stuck forever: it can never be + // abandoned locally, and its keep-alive timer would keep the + // connection active indefinitely. + if matches!(err, ClosePathError::LastOpenPath) + && let Some(timeout) = + self.path_data(path_id).idle_timeout + { + let dt = cmp::max( + timeout, + 3 * self.pto(SpaceKind::Data, path_id), + ); + self.timers.set( + Timer::PerPath(path_id, PathTimer::PathIdle), + now + dt, + self.qlog.with_time(now), + ); + } } } diff --git a/noq-proto/src/tests/proptests.rs b/noq-proto/src/tests/proptests.rs index a0f32e3c1e..d2ec1e1f5f 100644 --- a/noq-proto/src/tests/proptests.rs +++ b/noq-proto/src/tests/proptests.rs @@ -14,10 +14,13 @@ use test_strategy::proptest; use tracing::error; use crate::{ - ClientConfig, Connection, ConnectionClose, ConnectionError, Event, PathStatus, Side, - TransportConfig, TransportErrorCode, - tests::random_interaction::{TestOp, run_random_interaction}, - tests::util::{ManyToManyRouting, Pair, Routing, client_config, server_config, subscribe}, + ClientConfig, Connection, ConnectionClose, ConnectionError, Event, MtuDiscoveryConfig, + PathStatus, Side, TransportConfig, TransportErrorCode, + tests::random_interaction::Establishment, + tests::{ + random_interaction::{TestOp, run_random_interaction}, + util::{ManyToManyRouting, Pair, Routing, client_config, server_config, subscribe}, + }, }; // These TransportConfig constants are designed to match iroh for now. @@ -70,6 +73,9 @@ const SERVER_ADDRS: [SocketAddr; 3] = [ struct PairSetup { seed: Seed, extensions: Extensions, + mtud_enabled: bool, + client_enable_gso: bool, + server_enable_gso: bool, routing_setup: RoutingSetup, } @@ -141,10 +147,15 @@ impl PairSetup { transport.max_remote_nat_traversal_addresses(MAX_QNT_ADDRS); } + // enable/disable MTUD + transport.mtu_discovery_config(self.mtud_enabled.then_some(MtuDiscoveryConfig::default())); + // Initialize the server config let mut server_cfg = server_config(); - server_cfg.transport = Arc::new(transport.clone()); + let mut server_transport = transport.clone(); + server_transport.enable_segmentation_offload(self.server_enable_gso); + server_cfg.transport = Arc::new(server_transport); pair.server .endpoint .set_server_config(Some(Arc::new(server_cfg))); @@ -152,6 +163,7 @@ impl PairSetup { // Initialize the client config let mut client_cfg = client_config(); + transport.enable_segmentation_offload(self.client_enable_gso); client_cfg.transport = Arc::new(transport); // Add routing, if enabled @@ -195,18 +207,22 @@ impl Seed { #[proptest(cases = 256)] fn random_interaction( setup: PairSetup, + establishment: Establishment, #[strategy(vec(any::(), 0..100))] interactions: Vec, ) { let (mut pair, client_config) = setup.run("random_interaction"); - let (client_ch, server_ch) = run_random_interaction(&mut pair, interactions, client_config); + let (client_ch, server_ch) = + run_random_interaction(&mut pair, interactions, client_config, establishment); prop_assert!(!pair.drive_bounded(1000), "connection never became idle"); prop_assert!(allowed_error(poll_to_close( pair.client_conn_mut(client_ch) ))); - prop_assert!(allowed_error(poll_to_close( - pair.server_conn_mut(server_ch) - ))); + if let Some(server_ch) = server_ch { + prop_assert!(allowed_error(poll_to_close( + pair.server_conn_mut(server_ch) + ))); + } } fn routing_table() -> impl Strategy { @@ -301,6 +317,9 @@ fn regression_unset_packet_acked() { 33, 89, 203, 28, 107, 123, 117, 6, 54, 215, 244, 47, 1, ]), extensions: Extensions::MultipathOnly, + mtud_enabled: true, + client_enable_gso: true, + server_enable_gso: true, routing_setup: RoutingSetup::Complex(old_routing_table()), }; let interactions = vec![ @@ -322,15 +341,18 @@ fn regression_unset_packet_acked() { let _guard = subscribe(); let (mut pair, client_config) = setup.run(prefix); - let (client_ch, server_ch) = run_random_interaction(&mut pair, interactions, client_config); + let (client_ch, server_ch) = + run_random_interaction(&mut pair, interactions, client_config, Establishment::Full); assert!(!pair.drive_bounded(1000), "connection never became idle"); assert!(allowed_error(poll_to_close( pair.client_conn_mut(client_ch) ))); - assert!(allowed_error(poll_to_close( - pair.server_conn_mut(server_ch) - ))); + if let Some(server_ch) = server_ch { + assert!(allowed_error(poll_to_close( + pair.server_conn_mut(server_ch) + ))); + } } #[test] @@ -342,6 +364,9 @@ fn regression_invalid_key() { 130, 117, 84, 250, 190, 50, 237, 14, 167, 60, 5, 140, 149, ]), extensions: Extensions::MultipathOnly, + mtud_enabled: true, + client_enable_gso: true, + server_enable_gso: true, routing_setup: RoutingSetup::Complex(old_routing_table()), }; let interactions = vec![ @@ -361,15 +386,18 @@ fn regression_invalid_key() { let _guard = subscribe(); let (mut pair, client_config) = setup.run(prefix); - let (client_ch, server_ch) = run_random_interaction(&mut pair, interactions, client_config); + let (client_ch, server_ch) = + run_random_interaction(&mut pair, interactions, client_config, Establishment::Full); assert!(!pair.drive_bounded(1000), "connection never became idle"); assert!(allowed_error(poll_to_close( pair.client_conn_mut(client_ch) ))); - assert!(allowed_error(poll_to_close( - pair.server_conn_mut(server_ch) - ))); + if let Some(server_ch) = server_ch { + assert!(allowed_error(poll_to_close( + pair.server_conn_mut(server_ch) + ))); + } } /// Regression test for the "invalid key" panic in `noq-proto::Endpoint::handle_event`. @@ -389,6 +417,9 @@ fn regression_invalid_key2() { let setup = PairSetup { seed: Seed::Zeroes, extensions: Extensions::MultipathOnly, + mtud_enabled: true, + client_enable_gso: true, + server_enable_gso: true, routing_setup: RoutingSetup::SimpleSymmetric, }; let interactions = vec![ @@ -412,15 +443,18 @@ fn regression_invalid_key2() { let _guard = subscribe(); let (mut pair, client_config) = setup.run(prefix); - let (client_ch, server_ch) = run_random_interaction(&mut pair, interactions, client_config); + let (client_ch, server_ch) = + run_random_interaction(&mut pair, interactions, client_config, Establishment::Full); assert!(!pair.drive_bounded(1000), "connection never became idle"); assert!(allowed_error(poll_to_close( pair.client_conn_mut(client_ch) ))); - assert!(allowed_error(poll_to_close( - pair.server_conn_mut(server_ch) - ))); + if let Some(server_ch) = server_ch { + assert!(allowed_error(poll_to_close( + pair.server_conn_mut(server_ch) + ))); + } } #[test] @@ -432,6 +466,9 @@ fn regression_key_update_error() { 187, 208, 54, 158, 239, 190, 82, 198, 62, 91, 51, 53, 226, ]), extensions: Extensions::MultipathOnly, + mtud_enabled: true, + client_enable_gso: true, + server_enable_gso: true, routing_setup: RoutingSetup::Complex(old_routing_table()), }; let interactions = vec![ @@ -446,15 +483,18 @@ fn regression_key_update_error() { let _guard = subscribe(); let (mut pair, client_config) = setup.run(prefix); - let (client_ch, server_ch) = run_random_interaction(&mut pair, interactions, client_config); + let (client_ch, server_ch) = + run_random_interaction(&mut pair, interactions, client_config, Establishment::Full); assert!(!pair.drive_bounded(1000), "connection never became idle"); assert!(allowed_error(poll_to_close( pair.client_conn_mut(client_ch) ))); - assert!(allowed_error(poll_to_close( - pair.server_conn_mut(server_ch) - ))); + if let Some(server_ch) = server_ch { + assert!(allowed_error(poll_to_close( + pair.server_conn_mut(server_ch) + ))); + } } #[test] @@ -463,6 +503,9 @@ fn regression_never_idle() { let setup = PairSetup { seed: Seed::Zeroes, extensions: Extensions::MultipathOnly, + mtud_enabled: true, + client_enable_gso: true, + server_enable_gso: true, routing_setup: RoutingSetup::Complex(old_routing_table()), }; let interactions = vec![ @@ -485,15 +528,18 @@ fn regression_never_idle() { let _guard = subscribe(); let (mut pair, client_config) = setup.run(prefix); - let (client_ch, server_ch) = run_random_interaction(&mut pair, interactions, client_config); + let (client_ch, server_ch) = + run_random_interaction(&mut pair, interactions, client_config, Establishment::Full); assert!(!pair.drive_bounded(1000), "connection never became idle"); assert!(allowed_error(poll_to_close( pair.client_conn_mut(client_ch) ))); - assert!(allowed_error(poll_to_close( - pair.server_conn_mut(server_ch) - ))); + if let Some(server_ch) = server_ch { + assert!(allowed_error(poll_to_close( + pair.server_conn_mut(server_ch) + ))); + } } #[test] @@ -502,6 +548,9 @@ fn regression_never_idle2() { let setup = PairSetup { seed: Seed::Zeroes, extensions: Extensions::MultipathOnly, + mtud_enabled: true, + client_enable_gso: true, + server_enable_gso: true, routing_setup: RoutingSetup::Complex(old_routing_table()), }; let interactions = vec![ @@ -526,16 +575,19 @@ fn regression_never_idle2() { let _guard = subscribe(); let (mut pair, client_config) = setup.run(prefix); - let (client_ch, server_ch) = run_random_interaction(&mut pair, interactions, client_config); + let (client_ch, server_ch) = + run_random_interaction(&mut pair, interactions, client_config, Establishment::Full); // We needed to increase the bounds. It eventually times out. assert!(!pair.drive_bounded(1000), "connection never became idle"); assert!(allowed_error(poll_to_close( pair.client_conn_mut(client_ch) ))); - assert!(allowed_error(poll_to_close( - pair.server_conn_mut(server_ch) - ))); + if let Some(server_ch) = server_ch { + assert!(allowed_error(poll_to_close( + pair.server_conn_mut(server_ch) + ))); + } } #[test] @@ -544,6 +596,9 @@ fn regression_packet_number_space_missing() { let setup = PairSetup { seed: Seed::Zeroes, extensions: Extensions::MultipathOnly, + mtud_enabled: true, + client_enable_gso: true, + server_enable_gso: true, routing_setup: RoutingSetup::SimpleSymmetric, }; let interactions = vec![ @@ -568,15 +623,18 @@ fn regression_packet_number_space_missing() { let _guard = subscribe(); let (mut pair, client_config) = setup.run(prefix); - let (client_ch, server_ch) = run_random_interaction(&mut pair, interactions, client_config); + let (client_ch, server_ch) = + run_random_interaction(&mut pair, interactions, client_config, Establishment::Full); assert!(!pair.drive_bounded(1000), "connection never became idle"); assert!(allowed_error(poll_to_close( pair.client_conn_mut(client_ch) ))); - assert!(allowed_error(poll_to_close( - pair.server_conn_mut(server_ch) - ))); + if let Some(server_ch) = server_ch { + assert!(allowed_error(poll_to_close( + pair.server_conn_mut(server_ch) + ))); + } } #[test] @@ -585,6 +643,9 @@ fn regression_peer_failed_to_respond_with_path_abandon() { let setup = PairSetup { seed: Seed::Zeroes, extensions: Extensions::MultipathOnly, + mtud_enabled: true, + client_enable_gso: true, + server_enable_gso: true, routing_setup: RoutingSetup::Complex(old_routing_table()), }; let interactions = vec![ @@ -602,15 +663,18 @@ fn regression_peer_failed_to_respond_with_path_abandon() { let _guard = subscribe(); let (mut pair, client_config) = setup.run(prefix); - let (client_ch, server_ch) = run_random_interaction(&mut pair, interactions, client_config); + let (client_ch, server_ch) = + run_random_interaction(&mut pair, interactions, client_config, Establishment::Full); assert!(!pair.drive_bounded(1000), "connection never became idle"); assert!(allowed_error(poll_to_close( pair.client_conn_mut(client_ch) ))); - assert!(allowed_error(poll_to_close( - pair.server_conn_mut(server_ch) - ))); + if let Some(server_ch) = server_ch { + assert!(allowed_error(poll_to_close( + pair.server_conn_mut(server_ch) + ))); + } } #[test] @@ -619,6 +683,9 @@ fn regression_peer_failed_to_respond_with_path_abandon2() { let setup = PairSetup { seed: Seed::Zeroes, extensions: Extensions::MultipathOnly, + mtud_enabled: true, + client_enable_gso: true, + server_enable_gso: true, routing_setup: RoutingSetup::SimpleSymmetric, }; let interactions = vec![ @@ -646,15 +713,18 @@ fn regression_peer_failed_to_respond_with_path_abandon2() { let _guard = subscribe(); let (mut pair, client_config) = setup.run(prefix); - let (client_ch, server_ch) = run_random_interaction(&mut pair, interactions, client_config); + let (client_ch, server_ch) = + run_random_interaction(&mut pair, interactions, client_config, Establishment::Full); assert!(!pair.drive_bounded(1000), "connection never became idle"); assert!(allowed_error(poll_to_close( pair.client_conn_mut(client_ch) ))); - assert!(allowed_error(poll_to_close( - pair.server_conn_mut(server_ch) - ))); + if let Some(server_ch) = server_ch { + assert!(allowed_error(poll_to_close( + pair.server_conn_mut(server_ch) + ))); + } } /// This test sets up two addresses for the server side: @@ -694,6 +764,9 @@ fn regression_path_validation() { let setup = PairSetup { seed: Seed::Zeroes, extensions: Extensions::MultipathOnly, + mtud_enabled: true, + client_enable_gso: true, + server_enable_gso: true, routing_setup: RoutingSetup::Complex(ManyToManyRouting::from_routes( vec![("[::ffff:1.1.1.0]:44433".parse().unwrap(), 0)], vec![ @@ -725,15 +798,18 @@ fn regression_path_validation() { let _guard = subscribe(); let (mut pair, client_config) = setup.run(prefix); - let (client_ch, server_ch) = run_random_interaction(&mut pair, interactions, client_config); + let (client_ch, server_ch) = + run_random_interaction(&mut pair, interactions, client_config, Establishment::Full); assert!(!pair.drive_bounded(1000), "connection never became idle"); assert!(allowed_error(poll_to_close( pair.client_conn_mut(client_ch) ))); - assert!(allowed_error(poll_to_close( - pair.server_conn_mut(server_ch) - ))); + if let Some(server_ch) = server_ch { + assert!(allowed_error(poll_to_close( + pair.server_conn_mut(server_ch) + ))); + } } /// This regression test used to fail with the client never becoming idle. @@ -758,6 +834,9 @@ fn regression_never_idle3() { let setup = PairSetup { seed: Seed::Zeroes, extensions: Extensions::MultipathOnly, + mtud_enabled: true, + client_enable_gso: true, + server_enable_gso: true, routing_setup: RoutingSetup::SimpleSymmetric, }; let interactions = vec![ @@ -782,15 +861,18 @@ fn regression_never_idle3() { let _guard = subscribe(); let (mut pair, client_config) = setup.run(prefix); - let (client_ch, server_ch) = run_random_interaction(&mut pair, interactions, client_config); + let (client_ch, server_ch) = + run_random_interaction(&mut pair, interactions, client_config, Establishment::Full); assert!(!pair.drive_bounded(1000), "connection never became idle"); assert!(allowed_error(poll_to_close( pair.client_conn_mut(client_ch) ))); - assert!(allowed_error(poll_to_close( - pair.server_conn_mut(server_ch) - ))); + if let Some(server_ch) = server_ch { + assert!(allowed_error(poll_to_close( + pair.server_conn_mut(server_ch) + ))); + } } #[test] @@ -799,6 +881,9 @@ fn regression_frame_encoding_error() { let setup = PairSetup { seed: Seed::Zeroes, extensions: Extensions::MultipathOnly, + mtud_enabled: true, + client_enable_gso: true, + server_enable_gso: true, routing_setup: RoutingSetup::SimpleSymmetric, }; let interactions = vec![ @@ -821,15 +906,18 @@ fn regression_frame_encoding_error() { let _guard = subscribe(); let (mut pair, client_config) = setup.run(prefix); - let (client_ch, server_ch) = run_random_interaction(&mut pair, interactions, client_config); + let (client_ch, server_ch) = + run_random_interaction(&mut pair, interactions, client_config, Establishment::Full); assert!(!pair.drive_bounded(1000), "connection never became idle"); assert!(allowed_error(poll_to_close( pair.client_conn_mut(client_ch) ))); - assert!(allowed_error(poll_to_close( - pair.server_conn_mut(server_ch) - ))); + if let Some(server_ch) = server_ch { + assert!(allowed_error(poll_to_close( + pair.server_conn_mut(server_ch) + ))); + } } #[test] @@ -838,6 +926,9 @@ fn regression_there_should_be_at_least_one_path() { let setup = PairSetup { seed: Seed::Zeroes, extensions: Extensions::MultipathOnly, + mtud_enabled: true, + client_enable_gso: true, + server_enable_gso: true, routing_setup: RoutingSetup::SimpleSymmetric, }; let interactions = vec![ @@ -853,15 +944,18 @@ fn regression_there_should_be_at_least_one_path() { let _guard = subscribe(); let (mut pair, client_config) = setup.run(prefix); - let (client_ch, server_ch) = run_random_interaction(&mut pair, interactions, client_config); + let (client_ch, server_ch) = + run_random_interaction(&mut pair, interactions, client_config, Establishment::Full); assert!(!pair.drive_bounded(1000), "connection never became idle"); assert!(allowed_error(poll_to_close( pair.client_conn_mut(client_ch) ))); - assert!(allowed_error(poll_to_close( - pair.server_conn_mut(server_ch) - ))); + if let Some(server_ch) = server_ch { + assert!(allowed_error(poll_to_close( + pair.server_conn_mut(server_ch) + ))); + } } /// This test will loop forever, unless the loss detection timer is allowed to back off @@ -889,6 +983,9 @@ fn regression_conn_never_idle5() { let setup = PairSetup { seed: Seed::Zeroes, extensions: Extensions::MultipathOnly, + mtud_enabled: true, + client_enable_gso: true, + server_enable_gso: true, routing_setup: RoutingSetup::SimpleSymmetric, }; let interactions = vec![ @@ -905,15 +1002,18 @@ fn regression_conn_never_idle5() { let _guard = subscribe(); let (mut pair, client_config) = setup.run(prefix); - let (client_ch, server_ch) = run_random_interaction(&mut pair, interactions, client_config); + let (client_ch, server_ch) = + run_random_interaction(&mut pair, interactions, client_config, Establishment::Full); assert!(!pair.drive_bounded(1000), "connection never became idle"); assert!(allowed_error(poll_to_close( pair.client_conn_mut(client_ch) ))); - assert!(allowed_error(poll_to_close( - pair.server_conn_mut(server_ch) - ))); + if let Some(server_ch) = server_ch { + assert!(allowed_error(poll_to_close( + pair.server_conn_mut(server_ch) + ))); + } } /// Yet another regression with PATH_ABANDON "not being answered" by our peer. @@ -944,6 +1044,9 @@ fn regression_peer_ignored_path_abandon() { let setup = PairSetup { seed: Seed::Zeroes, extensions: Extensions::MultipathOnly, + mtud_enabled: true, + client_enable_gso: true, + server_enable_gso: true, routing_setup: RoutingSetup::SimpleSymmetric, }; let interactions = vec![ @@ -973,15 +1076,18 @@ fn regression_peer_ignored_path_abandon() { let _guard = subscribe(); let (mut pair, client_config) = setup.run(prefix); - let (client_ch, server_ch) = run_random_interaction(&mut pair, interactions, client_config); + let (client_ch, server_ch) = + run_random_interaction(&mut pair, interactions, client_config, Establishment::Full); assert!(!pair.drive_bounded(1000), "connection never became idle"); assert!(allowed_error(poll_to_close( pair.client_conn_mut(client_ch) ))); - assert!(allowed_error(poll_to_close( - pair.server_conn_mut(server_ch) - ))); + if let Some(server_ch) = server_ch { + assert!(allowed_error(poll_to_close( + pair.server_conn_mut(server_ch) + ))); + } } /// A regression test that used to put noq into a state of sending PATH_CHALLENGE @@ -1013,6 +1119,9 @@ fn regression_never_idle4() { let setup = PairSetup { seed: Seed::Zeroes, extensions: Extensions::MultipathOnly, + mtud_enabled: true, + client_enable_gso: true, + server_enable_gso: true, routing_setup: RoutingSetup::Complex(ManyToManyRouting::from_routes( vec![ ("[::ffff:1.1.1.0]:44433".parse().unwrap(), 0), @@ -1062,15 +1171,18 @@ fn regression_never_idle4() { let _guard = subscribe(); let (mut pair, client_config) = setup.run(prefix); - let (client_ch, server_ch) = run_random_interaction(&mut pair, interactions, client_config); + let (client_ch, server_ch) = + run_random_interaction(&mut pair, interactions, client_config, Establishment::Full); assert!(!pair.drive_bounded(1000), "connection never became idle"); assert!(allowed_error(poll_to_close( pair.client_conn_mut(client_ch) ))); - assert!(allowed_error(poll_to_close( - pair.server_conn_mut(server_ch) - ))); + if let Some(server_ch) = server_ch { + assert!(allowed_error(poll_to_close( + pair.server_conn_mut(server_ch) + ))); + } } /// This test reproduced an infinite loop in loss detection. @@ -1103,6 +1215,9 @@ fn regression_infinite_loop() { let setup = PairSetup { seed: Seed::Zeroes, extensions: Extensions::MultipathOnly, + mtud_enabled: true, + client_enable_gso: true, + server_enable_gso: true, routing_setup: RoutingSetup::SimpleSymmetric, }; let interactions = vec![ @@ -1124,7 +1239,8 @@ fn regression_infinite_loop() { let _guard = subscribe(); let (mut pair, client_config) = setup.run(prefix); - let (client_ch, server_ch) = run_random_interaction(&mut pair, interactions, client_config); + let (client_ch, server_ch) = + run_random_interaction(&mut pair, interactions, client_config, Establishment::Full); // This bug originally occurred at exactly 4540 iterations. // At 4539 it still finishes (but fails the assertion). @@ -1133,9 +1249,11 @@ fn regression_infinite_loop() { assert!(allowed_error(poll_to_close( pair.client_conn_mut(client_ch) ))); - assert!(allowed_error(poll_to_close( - pair.server_conn_mut(server_ch) - ))); + if let Some(server_ch) = server_ch { + assert!(allowed_error(poll_to_close( + pair.server_conn_mut(server_ch) + ))); + } } /// This test reproduced a situation in which a QNT-enabled connection sends path challenges indefinitely. @@ -1162,6 +1280,9 @@ fn regression_qnt_revalidating_path_forever() { let setup = PairSetup { seed: Seed::Zeroes, extensions: Extensions::QntAndMultipath, + mtud_enabled: true, + client_enable_gso: true, + server_enable_gso: true, routing_setup: RoutingSetup::SimpleSymmetric, }; let interactions = vec![ @@ -1185,15 +1306,162 @@ fn regression_qnt_revalidating_path_forever() { let _guard = subscribe(); let (mut pair, client_config) = setup.run(prefix); - let (client_ch, server_ch) = run_random_interaction(&mut pair, interactions, client_config); + let (client_ch, server_ch) = + run_random_interaction(&mut pair, interactions, client_config, Establishment::Full); assert!(!pair.drive_bounded(1000), "connection never became idle"); assert!(allowed_error(poll_to_close( pair.client_conn_mut(client_ch) ))); + if let Some(server_ch) = server_ch { + assert!(allowed_error(poll_to_close( + pair.server_conn_mut(server_ch) + ))); + } +} + +#[test] +fn regression_1() { + let prefix = "regression_1"; + let setup = PairSetup { + seed: Seed::Zeroes, + extensions: Extensions::MultipathOnly, + mtud_enabled: true, + client_enable_gso: true, + server_enable_gso: true, + routing_setup: RoutingSetup::SimpleSymmetric, + }; + let establishment = Establishment::Full; + let interactions = vec![ + TestOp::PassiveMigration { + side: Side::Server, + addr_idx: 0, + }, + TestOp::SendDatagram { + side: Side::Client, + size: 0, + drop: false, + }, + TestOp::DriveBothToIdle, + TestOp::OpenPath { + side: Side::Client, + status: PathStatus::Available, + addr_idx: 0, + }, + ]; + + let _guard = subscribe(); + let (mut pair, client_config) = setup.run(prefix); + let (client_ch, server_ch) = + run_random_interaction(&mut pair, interactions, client_config, establishment); + + assert!(!pair.drive_bounded(1000), "connection never became idle"); assert!(allowed_error(poll_to_close( - pair.server_conn_mut(server_ch) + pair.client_conn_mut(client_ch) ))); + if let Some(server_ch) = server_ch { + assert!(allowed_error(poll_to_close( + pair.server_conn_mut(server_ch) + ))); + } +} + +#[test] +fn regression_2() { + let prefix = "regression_2"; + let setup = PairSetup { + seed: Seed::Zeroes, + extensions: Extensions::MultipathOnly, + mtud_enabled: true, + client_enable_gso: true, + server_enable_gso: true, + routing_setup: RoutingSetup::SimpleSymmetric, + }; + let interactions = vec![ + TestOp::SendDatagram { + side: Side::Client, + size: 0, + drop: false, + }, + TestOp::PassiveMigration { + side: Side::Server, + addr_idx: 0, + }, + TestOp::DriveBothToIdle, + TestOp::FinishConnect, + TestOp::DriveBothToIdle, + TestOp::OpenPath { + side: Side::Client, + status: PathStatus::Available, + addr_idx: 0, + }, + ]; + + let _guard = subscribe(); + let (mut pair, client_config) = setup.run(prefix); + let (client_ch, server_ch) = + run_random_interaction(&mut pair, interactions, client_config, Establishment::Full); + + assert!(!pair.drive_bounded(1000), "connection never became idle"); + assert!(allowed_error(poll_to_close( + pair.client_conn_mut(client_ch) + ))); + if let Some(server_ch) = server_ch { + assert!(allowed_error(poll_to_close( + pair.server_conn_mut(server_ch) + ))); + } +} + +#[test] +fn regression_3() { + let prefix = "regression_3"; + let setup = PairSetup { + seed: Seed::Zeroes, + extensions: Extensions::MultipathOnly, + mtud_enabled: true, + client_enable_gso: true, + server_enable_gso: true, + routing_setup: RoutingSetup::SimpleSymmetric, + }; + let establishment = Establishment::BeforeHandshake; + let interactions = vec![ + TestOp::DriveBothToIdle, + TestOp::PassiveMigration { + side: Side::Server, + addr_idx: 0, + }, + TestOp::OpenPath { + side: Side::Client, + status: PathStatus::Available, + addr_idx: 2, + }, + TestOp::PassiveMigration { + side: Side::Server, + addr_idx: 2, + }, + TestOp::DriveBothToIdle, + TestOp::OpenPath { + side: Side::Client, + status: PathStatus::Available, + addr_idx: 0, + }, + ]; + + let _guard = subscribe(); + let (mut pair, client_config) = setup.run(prefix); + let (client_ch, server_ch) = + run_random_interaction(&mut pair, interactions, client_config, establishment); + + assert!(!pair.drive_bounded(1000), "connection never became idle"); + assert!(allowed_error(poll_to_close( + pair.client_conn_mut(client_ch) + ))); + if let Some(server_ch) = server_ch { + assert!(allowed_error(poll_to_close( + pair.server_conn_mut(server_ch) + ))); + } } /// This reproduced a never-idle infinite loop where both the client and server would @@ -1215,8 +1483,12 @@ fn regression_migration_probing_loop() { let setup = PairSetup { seed: Seed::Zeroes, extensions: Extensions::QntAndMultipath, + mtud_enabled: true, + client_enable_gso: true, + server_enable_gso: true, routing_setup: RoutingSetup::SimpleSymmetric, }; + let establishment = Establishment::Full; let interactions = vec![ TestOp::OpenPath { side: Side::Client, @@ -1241,15 +1513,18 @@ fn regression_migration_probing_loop() { let _guard = subscribe(); let (mut pair, client_config) = setup.run(prefix); - let (client_ch, server_ch) = run_random_interaction(&mut pair, interactions, client_config); + let (client_ch, server_ch) = + run_random_interaction(&mut pair, interactions, client_config, establishment); assert!(!pair.drive_bounded(1000), "connection never became idle"); assert!(allowed_error(poll_to_close( pair.client_conn_mut(client_ch) ))); - assert!(allowed_error(poll_to_close( - pair.server_conn_mut(server_ch) - ))); + if let Some(server_ch) = server_ch { + assert!(allowed_error(poll_to_close( + pair.server_conn_mut(server_ch) + ))); + } } /// Test for a case where we kept sending so many PATH_CHALLENGEs that we'd run out of `drive_bounded` budget. @@ -1282,6 +1557,9 @@ fn regression_challenge_resend_loop() { let setup = PairSetup { seed: Seed::Zeroes, extensions: Extensions::MultipathOnly, + mtud_enabled: true, + client_enable_gso: true, + server_enable_gso: true, routing_setup: RoutingSetup::Complex(ManyToManyRouting::from_routes( vec![("[::ffff:1.1.1.0]:44433".parse().unwrap(), 0)], vec![ @@ -1291,6 +1569,7 @@ fn regression_challenge_resend_loop() { ], )), }; + let establishment = Establishment::Full; let interactions = vec![ TestOp::OpenPath { side: Side::Client, @@ -1334,13 +1613,16 @@ fn regression_challenge_resend_loop() { let _guard = subscribe(); let (mut pair, client_config) = setup.run(prefix); - let (client_ch, server_ch) = run_random_interaction(&mut pair, interactions, client_config); + let (client_ch, server_ch) = + run_random_interaction(&mut pair, interactions, client_config, establishment); assert!(!pair.drive_bounded(1000), "connection never became idle"); assert!(allowed_error(poll_to_close( pair.client_conn_mut(client_ch) ))); - assert!(allowed_error(poll_to_close( - pair.server_conn_mut(server_ch) - ))); + if let Some(server_ch) = server_ch { + assert!(allowed_error(poll_to_close( + pair.server_conn_mut(server_ch) + ))); + } } diff --git a/noq-proto/src/tests/random_interaction.rs b/noq-proto/src/tests/random_interaction.rs index b9b5fd53de..573ad0809e 100644 --- a/noq-proto/src/tests/random_interaction.rs +++ b/noq-proto/src/tests/random_interaction.rs @@ -1,4 +1,7 @@ -use std::net::{Ipv4Addr, Ipv6Addr, SocketAddr, SocketAddrV4}; +use std::{ + fmt::Debug, + net::{Ipv4Addr, Ipv6Addr, SocketAddr, SocketAddrV4}, +}; use bytes::Bytes; use test_strategy::Arbitrary; @@ -12,6 +15,10 @@ use super::util::{Pair, Routing, TestEndpoint}; #[derive(Debug, Clone, Copy, Arbitrary)] pub(super) enum TestOp { + /// Finish the started connection attempt by accepting it on the server side. + FinishConnect, + /// Drive both sides until the connection is idle. + DriveBothToIdle, /// Drive the endpoint on the given `side`, processing all pending I/O. Drive { side: Side }, /// Advance the simulated time forward, unless both endpoints are idle. @@ -59,6 +66,15 @@ pub(super) enum TestOp { }, /// Perform a stream-level operation on the connection belonging to `side`. StreamOp { side: Side, stream_op: StreamOp }, + /// Send a datagram. + SendDatagram { + side: Side, + #[strategy(0..2000usize)] + size: usize, + drop: bool, + }, + /// Read all datagrams on given `side`. + ReadDatagrams { side: Side }, /// Close the connection belonging to `side`. CloseConn { side: Side, @@ -95,39 +111,94 @@ pub(super) enum StreamOp { Stop(#[strategy(0..3usize)] usize, u32), } +/// The type of connection establishment to generate. +#[derive(Debug, Clone, Copy, Arbitrary)] +pub(super) enum Establishment { + /// Fully establish the connection before test operations. + Full, + /// Start running test operations before the handshake is finished. + BeforeHandshake, +} + pub(super) struct State { send_streams: Vec, recv_streams: Vec, - handle: ConnectionHandle, + handle: Option, side: Side, } +/// Possible reasons a [`TestOp`] might not apply cleanly. +#[derive(Debug)] +pub(super) enum Error { + /// Running the operation has no effect. + /// + /// E.g. the operation requires some stream to be active or some path to be opened, + /// which isn't opened at this point in time, and thus the operation is skipped. + NoEffect, + /// Running the operation returned an error. + /// + /// E.g. we attempted to run multipath operations before we negotiated the multipath extension. + ApiError(Box), +} + +impl Error { + fn api(api_error: impl Debug + 'static) -> Self { + Self::ApiError(Box::new(api_error)) + } +} + impl TestOp { - fn run(self, pair: &mut Pair, client: &mut State, server: &mut State) -> Option<()> { + fn run(self, pair: &mut Pair, client: &mut State, server: &mut State) -> Result<(), Error> { let now = pair.time; match self { + Self::FinishConnect => { + let accept = pair + .server + .accepted + .take() + .ok_or(Error::NoEffect)? + .map_err(Error::api)?; + server.handle = Some(accept); + } + Self::DriveBothToIdle => { + if pair.drive_bounded(100) { + error!("DriveBothToIdle exceeded 100 steps"); + } + } Self::Drive { side: Side::Client } => pair.drive_client(), Self::Drive { side: Side::Server } => pair.drive_server(), Self::AdvanceTime => { + let before = pair.time; // If we advance during idle, we just immediately hit the idle timeout if !pair.client.is_idle() || !pair.server.is_idle() { pair.advance_time(); } + if before == pair.time { + return Err(Error::NoEffect); + } } Self::DropInbound { side: Side::Client } => { - debug!(len = pair.client.inbound.len(), "dropping inbound"); + let len = pair.client.inbound.len(); + debug!(len, "dropping inbound"); pair.client.inbound.clear(); + if len == 0 { + return Err(Error::NoEffect); + } } Self::DropInbound { side: Side::Server } => { - debug!(len = pair.server.inbound.len(), "dropping inbound"); + let len = pair.server.inbound.len(); + debug!(len, "dropping inbound"); pair.server.inbound.clear(); + if len == 0 { + return Err(Error::NoEffect); + } } Self::ReorderInbound { side: Side::Client } => { - let item = pair.client.inbound.pop_front()?; + let item = pair.client.inbound.pop_front().ok_or(Error::NoEffect)?; pair.client.inbound.push_back(item); } Self::ReorderInbound { side: Side::Server } => { - let item = pair.server.inbound.pop_front()?; + let item = pair.server.inbound.pop_front().ok_or(Error::NoEffect)?; pair.server.inbound.push_back(item); } Self::ForceKeyUpdate { side: Side::Client } => client.conn(pair)?.force_key_update(), @@ -168,8 +239,8 @@ impl TestOp { }, Routing::SimpleFirewall(_) => unimplemented!(), Routing::ManyToMany(ref routes) => match side { - Side::Client => routes.server_addr(addr_idx)?, - Side::Server => routes.client_addr(addr_idx)?, + Side::Client => routes.server_addr(addr_idx).ok_or(Error::NoEffect)?, + Side::Server => routes.client_addr(addr_idx).ok_or(Error::NoEffect)?, }, }; let state = match side { @@ -182,8 +253,7 @@ impl TestOp { local_ip: None, }; conn.open_path(network_path, status, now) - .inspect_err(|err| error!(?err, "OpenPath failed")) - .ok(); + .map_err(Error::api)?; } Self::ClosePath { side, @@ -197,8 +267,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()) - .inspect_err(|err| error!(?err, "ClosePath failed")) - .ok(); + .map_err(Error::api)?; } Self::PathSetStatus { side, @@ -211,16 +280,35 @@ impl TestOp { }; let conn = state.conn(pair)?; let path_id = get_path_id(conn, path_idx)?; - conn.set_path_status(path_id, status) - .inspect_err(|err| error!(?err, "PathSetStatus failed")) - .ok(); + conn.set_path_status(path_id, status).map_err(Error::api)?; } Self::StreamOp { side, stream_op } => { let state = match side { Side::Client => client, Side::Server => server, }; - stream_op.run(pair, state); + stream_op.run(pair, state)?; + } + Self::SendDatagram { side, size, drop } => { + let state = match side { + Side::Client => client, + Side::Server => server, + }; + let data = vec![42u8; size]; + state + .conn(pair)? + .datagrams() + .send(data.into(), drop) + .map_err(Error::api)?; + } + Self::ReadDatagrams { side } => { + let state = match side { + Side::Client => client, + Side::Server => server, + }; + while let Some(data) = state.conn(pair)?.datagrams().recv() { + trace!(len = data.len(), "ReadDatagrams read a datagram"); + } } Self::CloseConn { side, error_code } => { let state = match side { @@ -238,8 +326,8 @@ impl TestOp { }, Routing::SimpleFirewall(_) => unimplemented!(), Routing::ManyToMany(ref routes) => match side { - Side::Client => routes.client_addr(addr_idx)?, - Side::Server => routes.server_addr(addr_idx)?, + Side::Client => routes.client_addr(addr_idx).ok_or(Error::NoEffect)?, + Side::Server => routes.server_addr(addr_idx).ok_or(Error::NoEffect)?, }, }; let state = match side { @@ -248,8 +336,7 @@ impl TestOp { }; let conn = state.conn(pair)?; conn.add_nat_traversal_address(address) - .inspect_err(|err| error!(?err, "AddHpAddr failed")) - .ok(); + .map_err(Error::api)?; } Self::InitiateHpRound { side } => { let state = match side { @@ -257,60 +344,67 @@ impl TestOp { Side::Server => server, }; let conn = state.conn(pair)?; - let addrs = conn - .initiate_nat_traversal_round(now) - .inspect_err(|err| error!(?err, "InitiateHpRound failed")) - .ok()?; + let addrs = conn.initiate_nat_traversal_round(now).map_err(Error::api)?; trace!(?addrs, "initiating NAT Traversal"); } } - Some(()) + Ok(()) } } impl StreamOp { - fn run(self, pair: &mut Pair, state: &mut State) -> Option<()> { + fn run(self, pair: &mut Pair, state: &mut State) -> Result<(), Error> { let conn = state.conn(pair)?; // We generally ignore application-level errors. It's legal to call these APIs, so we do. We don't expect them to work all the time. match self { Self::Open(kind) => state.send_streams.extend(conn.streams().open(kind)), Self::Send { stream, num_bytes } => { - let stream_id = state.send_streams.get(stream)?; + let stream_id = state.send_streams.get(stream).ok_or(Error::NoEffect)?; let data = vec![0; num_bytes]; - let bytes = conn.send_stream(*stream_id).write(&data).ok()?; + let bytes = conn + .send_stream(*stream_id) + .write(&data) + .map_err(Error::api)?; trace!(attempted_write = %num_bytes, actually_written = %bytes, "random interaction: Wrote stream bytes"); } Self::Finish(stream) => { - let stream_id = state.send_streams.get(stream)?; - conn.send_stream(*stream_id).finish().ok(); + let stream_id = state.send_streams.get(stream).ok_or(Error::NoEffect)?; + conn.send_stream(*stream_id).finish().map_err(Error::api)?; } Self::Reset(stream, code) => { - let stream_id = state.send_streams.get(stream)?; - conn.send_stream(*stream_id).reset(code.into()).ok(); + let stream_id = state.send_streams.get(stream).ok_or(Error::NoEffect)?; + conn.send_stream(*stream_id) + .reset(code.into()) + .map_err(Error::api)?; } Self::Accept(kind) => state.recv_streams.extend(conn.streams().accept(kind)), Self::Receive(stream, ordered) => { - let stream_id = state.recv_streams.get(stream)?; + let stream_id = state.recv_streams.get(stream).ok_or(Error::NoEffect)?; let mut recv_stream = conn.recv_stream(*stream_id); - let mut chunks = recv_stream.read(ordered).ok()?; - let chunk = chunks.next(usize::MAX).ok()??; + let mut chunks = recv_stream.read(ordered).map_err(Error::api)?; + let chunk = chunks + .next(usize::MAX) + .map_err(Error::api)? + .ok_or(Error::NoEffect)?; trace!(chunk_len = %chunk.bytes.len(), offset = %chunk.offset, "read from stream"); } Self::Stop(stream, code) => { - let stream_id = state.recv_streams.get(stream)?; - conn.recv_stream(*stream_id).stop(code.into()).ok(); + let stream_id = state.recv_streams.get(stream).ok_or(Error::NoEffect)?; + conn.recv_stream(*stream_id) + .stop(code.into()) + .map_err(Error::api)?; } }; - Some(()) + Ok(()) } } impl State { - fn new(side: Side, handle: ConnectionHandle) -> Self { + fn new(side: Side) -> Self { Self { send_streams: Vec::new(), recv_streams: Vec::new(), - handle, + handle: None, side, } } @@ -322,16 +416,20 @@ impl State { } } - fn conn<'a>(&self, pair: &'a mut Pair) -> Option<&'a mut Connection> { - self.endpoint(pair).connections.get_mut(&self.handle) + fn conn<'a>(&self, pair: &'a mut Pair) -> Result<&'a mut Connection, Error> { + self.endpoint(pair) + .connections + .get_mut(&self.handle.ok_or(Error::NoEffect)?) + .ok_or(Error::NoEffect) } } -fn get_path_id(conn: &mut Connection, idx: usize) -> Option { +fn get_path_id(conn: &mut Connection, idx: usize) -> Result { let paths = conn.paths(); paths .get(idx.clamp(0, paths.len().saturating_sub(1))) .copied() + .ok_or(Error::NoEffect) } fn inc_last_addr_octet(addr: SocketAddr) -> SocketAddr { @@ -355,16 +453,39 @@ pub(super) fn run_random_interaction( pair: &mut Pair, interactions: Vec, client_config: ClientConfig, -) -> (ConnectionHandle, ConnectionHandle) { - let (client_ch, server_ch) = pair.connect_with(client_config); - pair.drive(); // finish establishing the connection; - info!("INTERACTION SETUP FINISHED"); - let mut client = State::new(Side::Client, client_ch); - let mut server = State::new(Side::Server, server_ch); + establishment: Establishment, +) -> (ConnectionHandle, Option) { + let mut client = State::new(Side::Client); + let mut server = State::new(Side::Server); + + let client_handle = pair.begin_connect(client_config); + client.handle = Some(client_handle); + + if matches!(establishment, Establishment::Full) { + pair.drive(); + TestOp::FinishConnect + .run(pair, &mut client, &mut server) + .expect("server experienced error connecting"); + pair.drive(); + } + + info!(?establishment, "INTERACTION SETUP COMPLETE"); for interaction in interactions { info!(?interaction, "INTERACTION STEP"); - interaction.run(pair, &mut client, &mut server); + match interaction.run(pair, &mut client, &mut server) { + Ok(()) => {} + Err(Error::NoEffect) => { + info!( + ?interaction, + "interaction step skipped due to invalid state" + ); + } + Err(Error::ApiError(err)) => { + error!(?interaction, ?err, "interaction step produced an API error"); + } + } } - (client.handle, server.handle) + + (client_handle, server.handle) }