From 879f52ed160f3307b4602a260bcd608301960a1d Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Tue, 13 Oct 2026 13:54:59 -0700 Subject: [PATCH 1/2] quest: claim handshake-ack Co-Authored-By: Claude Opus 5.5 From 566d2f77491ed69952a2eba844f949d81bfa791c Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Tue, 13 Oct 2026 14:14:17 -0700 Subject: [PATCH 2/2] 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 | 74 ------------------------- rs/moq-quic/src/tests/mod.rs | 50 +++++++++++++++++ 3 files changed, 50 insertions(+), 75 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 5f8ee18eeb..e7f767c171 100644 --- a/quest/m1/test-flakes-2/README.md +++ b/quest/m1/test-flakes-2/README.md @@ -34,7 +34,6 @@ Public API: none. Wire: none. - [Interop close code](/quest/m1/test-flakes-2/interop-close-code.md) - the browser close-code test reliably sees `unauthorized` for a refused session - [QUIC-wins race](/quest/m1/test-flakes-2/quic-winning-race.md) - moq-tokio's QUIC-wins test sets up QUIC's win instead of racing the WebSocket head start -- [Handshake ACK](/quest/m1/test-flakes-2/handshake-ack.md) - the impaired cluster drill's connect survives a lost handshake ACK - [Auth refresh expiry](/quest/m1/test-flakes-2/auth-refresh-expiry.md) - the auth refresh outage test measures the grant's life on one clock ## Related 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 2a1d630a12..0000000000 --- a/quest/m1/test-flakes-2/handshake-ack.md +++ /dev/null @@ -1,74 +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. - -## 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 925ab83b94..f43a818642 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