From 50a902197103c82ce72d38251b9fff2034da8ad1 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Tue, 13 Oct 2026 13:54:59 -0700 Subject: [PATCH 1/3] quest: claim handshake-ack Co-Authored-By: Claude Opus 5.5 From 7fd206c87fec564933184d934d8489d30d540369 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Tue, 13 Oct 2026 14:14:17 -0700 Subject: [PATCH 2/3] test(quic): a lost handshake ACK rides the next packet in its space moq-quic already bundles pending ACK ranges into packets carrying other frames. Pin that for the handshake, so the switch off noq keeps the fix moq-dev/noq#38 ports for the impaired cluster drill. Co-Authored-By: Claude Opus 5.5 --- quest/m1/test-flakes-2/README.md | 1 - quest/m1/test-flakes-2/handshake-ack.md | 77 ------------------------- rs/moq-quic/src/tests/mod.rs | 50 ++++++++++++++++ 3 files changed, 50 insertions(+), 78 deletions(-) delete mode 100644 quest/m1/test-flakes-2/handshake-ack.md diff --git a/quest/m1/test-flakes-2/README.md b/quest/m1/test-flakes-2/README.md index 504a9699393..380853a87b8 100644 --- a/quest/m1/test-flakes-2/README.md +++ b/quest/m1/test-flakes-2/README.md @@ -36,7 +36,6 @@ Public API: none. Wire: none. - [TS resume late frames](/quest/m1/test-flakes-2/ts-resume-late-frames.md) - the same-instance TS resume test keeps its late frames, fixed in production or the fixture as the evidence says - [WebTransport close release](/quest/m1/test-flakes-2/interop-close-release.md) - a compatible published adapter contains the acknowledged-close fix - [Interop close code](/quest/m1/test-flakes-2/interop-close-code.md) - adopt that release so the browser reliably sees `unauthorized` for a refused session -- [Handshake ACK](/quest/m1/test-flakes-2/handshake-ack.md) - the impaired cluster drill's connect survives a lost handshake ACK - [Auth refresh mocks time](/quest/m1/test-flakes-2/auth-refresh-mock-time.md) - moq-relay's auth refresh and fixed-lease tests measure a grant's life on one mocked clock - [uring teardown close](/quest/m1/test-flakes-2/uring-teardown-close.md) - moq-uring's published-close teardown test holds under load, fixed at its cause - [Auth serve signals](/quest/m1/test-flakes-2/auth-serve-signals.md) - moq-cli's auth serve signal tests hold under load diff --git a/quest/m1/test-flakes-2/handshake-ack.md b/quest/m1/test-flakes-2/handshake-ack.md deleted file mode 100644 index 9716e3afd2c..00000000000 --- a/quest/m1/test-flakes-2/handshake-ack.md +++ /dev/null @@ -1,77 +0,0 @@ -# [S] A lost handshake ACK does not stall an impaired connect - -## Goal - -The impaired cluster drill connects its publisher and subscriber however the -shaper drops their handshake ACKs. `MOQ_SHAPER_SEED=13876846873975217772` -`bursts_cross_a_cluster::impaired` (`rs/moq-relay/tests/drills.rs`), which -fails on `main` every run with `publisher connect timed out`, passes. -`bursts_cross_a_transport_peer::impaired` failed the same way in the merge -queue on 2026-10-15, under the same shaper. Treat it as the same bug. Open a -separate investigation only if it still fails after this fix lands. - -## Plan - -Reproduced 2026-10-13 on `0e3ca5e6b`, with noq trace logs. It fails 3 of 3 -runs at that seed. 200 random seeds all passed, so this is a rare tail, and -the merge queue hit it twice that day. - -The path makes every handshake flight two full datagrams that cannot cross -together. The hybrid X25519MLKEM768 key share makes the ClientHello 1491 -bytes and the ServerHello 1178, so each side's first flight is 2 x 1200 -bytes. `bursty()`'s bucket admits 1500 bytes of burst plus a 50 ms queue at -100 kbit/s, 2125 bytes in all. The second datagram of every back-to-back -flight and every two-packet PTO probe overflows, every time. A connect -through it only progresses when an ACK tells the sender which half arrived, -so the sender can retransmit just the other half. - -The failing seed: the client receives the server's first Initial (the -ServerHello head) and ACKs it three times, at +0.2s, +1.2s, and +4.2s. The -shaper's 5% loss drops all three. Between them the client sends Initial -probes that reach the server, but noq writes ACK frames only when -`PendingAcks::can_send()` holds. That is false once an ACK was sent and no -new packet arrived, so the probes carry no ACK. The server keeps resending -head and tail together, the tail always overflows, and the client never -gets Handshake keys. The fourth ACK lands at +9.2s. The server then fires -its PTO six times in one instant and bursts 12 Handshake probes, and the -drill's 10s connect budget runs out first. - -This is not a budget problem. Patching noq to repeat pending Initial and -Handshake ACK ranges in any packet it sends in that space makes the seed -pass, with the whole drill done in 6-7s (3 of 3 runs). 200 random seeds -still pass with it. A longer `TIMEOUT` would only move the tail, since -nothing repeats a lost ACK until the peer's next backed-off PTO. - -`moq-quic` already has the fix: quinn bundles pending ACK ranges into -packets that carry other frames (`can_send_with_other_frames`, paced by -`next_bundled_ack_time`). noq predates it. - -Decision (maintainer, 2026-10-13; ✅ marks the choice): - -- ✅ Port it to the frozen moq-dev/noq: repeat pending Initial and Handshake - ACK ranges in any packet sent in that space, as `moq-quic`'s - `can_send_with_other_frames` does, then bump the `moq-noq-proto` pin. - [#32](https://github.com/moq-dev/noq/pull/32) and - [#34](https://github.com/moq-dev/noq/pull/34) made the same exception for - this drill's last handshake flake. -- Wait for [the switch](/quest/m1/quic/fork/switch.md) to `moq-quic`, which - has not started. - -Land a mock-time regression test in noq with the fix: the peer's first ACK -of a two-datagram Initial flight is lost, and the next packet in that space -carries the ACK. Check that `moq-quic` passes the -same test, so the switch keeps the behavior. - -Also look at the PTO cascade. When the ACK reset `pto_count`, the Handshake -PTO deadline was already in the past, so the timer fired back to back until -the backoff passed `now`. Each firing queued two probes. Fix it only if -`moq-quic` does the same. - -Not covered: the `subscription aborted: internal error` failure also seen in -the merge queue. No seed was recorded for it, so it may be a different cause. - -Public API: none in this repo. Wire: none. - -## Related - -- [Switch](/quest/m1/quic/fork/switch.md) - moves the runtimes onto `moq-quic`, which already bundles ACKs diff --git a/rs/moq-quic/src/tests/mod.rs b/rs/moq-quic/src/tests/mod.rs index 42b2571d558..fedc9af91d7 100644 --- a/rs/moq-quic/src/tests/mod.rs +++ b/rs/moq-quic/src/tests/mod.rs @@ -3295,6 +3295,56 @@ fn lost_finished_after_initial_backoff() { assert!(elapsed < Duration::from_secs(1), "{elapsed:?}"); } +/// A lost ACK of a multi-datagram flight's head is repeated in the next packet the receiver +/// sends in that space, so the sender learns the head arrived without waiting on another ACK. +#[test] +fn lost_handshake_ack_repeats_in_next_packet() { + let _guard = subscribe(); + let (cert, key) = big_cert_and_key(); + let mut pair = Pair::new( + Default::default(), + server_config_with_cert(cert.clone(), key), + ); + pair.latency = Duration::from_millis(50); + + pair.begin_connect(client_config_with_certs(vec![cert])); + pair.drive_client(); + pair.time = pair.server.next_wakeup().unwrap(); + pair.server.drive(pair.time, pair.client.addr); + // Only the head of the server's flight arrives. The rest, whether sent with it or paced + // out behind it, is lost. + let mut lost = pair.server.outbound.len() - 1; + pair.server.outbound.truncate(1); + pair.drive_server(); + while pair.server.next_wakeup() < pair.client.next_wakeup() { + pair.time = pair.server.next_wakeup().unwrap(); + pair.server.drive(pair.time, pair.client.addr); + lost += pair.server.outbound.len(); + pair.server.outbound.clear(); + } + assert!(lost > 0); + let server_ch = pair.server.assert_accept(); + + // The client ACKs the head, and that ACK is lost. + pair.time = pair.client.next_wakeup().unwrap(); + pair.drive_client(); + assert!(!pair.server.inbound.is_empty()); + pair.server.inbound.clear(); + let acks = pair.server_conn_mut(server_ch).stats().frame_rx.acks; + + // The client's next packet is its PTO probe, which carries the ACK again. + while pair.server.inbound.is_empty() { + pair.time = pair.client.next_wakeup().unwrap(); + pair.drive_client(); + } + pair.time = pair.server.inbound.front().unwrap().0; + pair.drive_server(); + assert_eq!( + pair.server_conn_mut(server_ch).stats().frame_rx.acks, + acks + 1 + ); +} + /// A short idle timeout does not cut the handshake to two Initial flights. /// /// Before an RTT sample the PTO is 999ms, so a 2s idle timeout is armed at 2997ms, while From 32e159b6d256f4fd90ccea46f192d37bd9ab1562 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Thu, 15 Oct 2026 14:22:41 -0700 Subject: [PATCH 3/3] deps: bump moq-noq to 2.0.5 for the lost handshake ACK fix 2.0.5 carries moq-dev/noq#38: Initial and Handshake packets repeat the pending ACK ranges, so a lost handshake ACK is resent in the next packet. Co-Authored-By: Claude Opus 5.5 --- Cargo.lock | 16 ++++++++-------- Cargo.toml | 4 ++-- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3bf788a746d..939d551eab0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4666,9 +4666,9 @@ version = "0.0.0" [[package]] name = "moq-noq" -version = "2.0.4" +version = "2.0.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6bbf0af241543ba9abfbee6c436eeae5367efde3037c9c4982aec2567d74b92b" +checksum = "20d9d1334f556a15e60f6d406020fc3a40409335ae8a2ad472f6bf074a01acfa" dependencies = [ "bytes", "cfg_aliases", @@ -4688,9 +4688,9 @@ dependencies = [ [[package]] name = "moq-noq-proto" -version = "2.0.4" +version = "2.0.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "102495e6fc86b36d47e75947bd136f0e60d0d360cf15fc3b9712b60cb3c979a2" +checksum = "50ac444c6c1ed18d45536cd939be7f0fd664996e79c0519119b13d6ae9d3d5e2" dependencies = [ "aes-gcm", "aws-lc-rs", @@ -4719,9 +4719,9 @@ dependencies = [ [[package]] name = "moq-noq-udp" -version = "2.0.4" +version = "2.0.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "589717ed483c3b74a5fdb628139191b9a1e1b92877847a0ac9f3ff44df7bbd4e" +checksum = "8e978cded333ca8a6d50b96a7a4388ccc8d98db51d89df7dbee598e9b849ee79" dependencies = [ "cfg_aliases", "libc", @@ -10200,9 +10200,9 @@ dependencies = [ [[package]] name = "web-transport-moq" -version = "2.0.4" +version = "2.0.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "60d31032c966e67089b5369f9aecf925ef7d0f8a155530da7dc355bc930b097d" +checksum = "5dc946e718371f88a79d849c352b6bdd95e0a36139e9b76818c1a101b45ed97a" dependencies = [ "bytes", "futures", diff --git a/Cargo.toml b/Cargo.toml index 6070495c7e9..e8b71ab3b03 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -159,7 +159,7 @@ moq-mux = { version = "0.10.11", path = "rs/moq-mux" } moq-net = { version = "0.3.10", path = "rs/moq-net" } # The MoQ fork of noq (moq-dev/noq). iroh keeps upstream noq, so a build with the # iroh feature carries both stacks. -moq-noq-proto = { version = "2.0.4", default-features = false } +moq-noq-proto = { version = "2.0.5", default-features = false } # NVENC bindings, forked from ViliamVadocz/nvidia-video-codec-sdk to dlopen the # driver at runtime. Compiles on any platform (macOS included) but only actually # used by moq-video on Linux. @@ -243,7 +243,7 @@ web-transport-iroh = "0.8.1" # default-features off so the QUIC crypto provider is chosen by moq-tokio's # aws-lc-rs / ring features. # The WebTransport adapter released with moq-noq, from the same repository. -web-transport-moq = { version = "2.0.4", default-features = false } +web-transport-moq = { version = "2.0.5", default-features = false } web-transport-proto = "0.6" web-transport-trait = "0.5" web-transport-wasm = "0.8"