Skip to content

fix(uring): resolve a dial only once its final handshake flight is staged - #4042

Closed
kixelated wants to merge 2 commits into
mainfrom
quest/m1/uring-handshake-flush
Closed

kixelated wants to merge 2 commits into
mainfrom
quest/m1/uring-handshake-flush

Conversation

@kixelated

Copy link
Copy Markdown
Collaborator

Draft, blocked: this needs Connection::has_pending_handshake_data() in moq-noq-proto. That method is not in 1.3.0, so CI will fail to compile until the fork ships it and the pin moves.

Implements quest/m1/uring-handshake-flush.md.

Problem

An io_uring dial resolved on noq's Event::Connected. That event fires when the client's handshake completes locally, before its final flight has been sent. When congestion control or pacing holds back part of that flight, a worker stopped as soon as the dial resolves never sends the rest. The server then times out while the client holds a connection that looks established.

Reproduced on Linux 7.0 (loopback): a client with a ~20 KB mTLS certificate dialed 20 times, dropping its worker as each dial resolved. The server completed 0/20 handshakes before this change and 20/20 after it. With a small single-datagram final flight, it completes 20/20 either way. The window only opens when the flight is split: a multi-datagram client chain, a slow link where the pacer's burst is one MTU, or a flight bigger than the initial congestion window. On the relay, that means mTLS cluster dials.

Fix

  • On Connected the driver only records that the handshake completed. After each flush it hands the connection over once noq has no Initial or Handshake crypto left to send, which means the whole flight has been staged on the socket. Worker teardown already submits staged sends (tests/teardown.rs). Accepted connections go through the same path, but their flight is always empty by then.
  • establish fails with Error::Io if the worker has been dropped, so the dial errors instead of pending forever.
  • No timeout, no retry, and worker shutdown does not drain sends.

Why noq needs a change

None of noq's public signals says when the last handshake flight has left:

  • poll_transmit returning None means either "nothing queued" or "blocked by cwnd or pacing".
  • stats().frame_tx.crypto counts frames, not the flight.
  • Checking the headers of our own datagrams fails when the flight is congestion-blocked, because 1-RTT ACK-only packets still go out.
  • HandshakeConfirmed is exact but adds a full RTT to every dial.

So the fork needs this accessor (validated locally by patching a copy of moq-noq-proto 1.3.0 with it):

/// Whether handshake data is queued that has not been transmitted yet
pub fn has_pending_handshake_data(&self) -> bool {
    [SpaceKind::Initial, SpaceKind::Handshake].into_iter().any(|space| {
        self.crypto_state.has_keys(space.encryption_level())
            && !self.spaces[space].pending.crypto.is_empty()
    })
}

A noq-proto unit test can pin it: set max_outgoing_bytes_per_second(Some(12_000)) on the client, and with no time passing the ClientHello uses up the pacer's only burst. After the server's flight, the client is connected and has_pending_handshake_data() is true. It turns false after pair.drive().

Tests

New rs/moq-uring/tests/handshake.rs, kernel-gated like the other moq-uring tests:

  • a_stopped_dial_sends_all_of_a_split_final_flight: 20 one-shot dials with a 20 KB client chain, under Loss and Delay congestion control. Fails without the fix (server accepts 0).
  • a_worker_stopped_mid_dial_fails_it: drops the worker mid-handshake, and the dial returns Error::Io. Fails without the fix (it stays pending).
  • a_stopped_dial_finishes_the_server_handshake: normal dials still resolve promptly, and every one completes on the server.

just check was run with the local noq patch. Only the temporary [patch] line in Cargo.toml failed formatting, and it is not committed. cargo clippy -p moq-uring --all-targets --all-features -D warnings and the full cargo test -p moq-uring pass.

Impact

  • Public API: none in moq. The docs for Endpoint::connect and client::connect now say when the dial resolves.
  • Wire: none.
  • Latency: none for a flight that fits in one burst. A split flight resolves when its last datagram is staged, not after an extra RTT.

Decisions taken unattended

  • Chose to add a noq accessor over waiting for HandshakeConfirmed: it costs no extra RTT. Header parsing was rejected because it is unsound under congestion blocking.
  • Did not open the moq-dev/noq PR: this agent could not build that repository here. Next steps: land the accessor in moq-dev/noq, release moq-noq-proto, bump the pin, then mark this PR ready and delete the quest in it.

(Written by Claude Opus 5.5)

🤖 Generated with Claude Code

kixelated and others added 2 commits September 24, 2026 10:31
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…aged

noq reports the handshake complete before the client's last flight has
left: congestion control and pacing can hold part of it back, so a worker
stopped the moment the dial resolved stranded the server. The driver now
hands the connection over only once noq has no handshake data left to
send, and a dial whose worker stopped fails instead of pending forever.

Needs Connection::has_pending_handshake_data from moq-noq-proto, which
is not released yet.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated

Copy link
Copy Markdown
Collaborator Author

Closing: the maintainer abandoned this quest. If a worker stops, its connection dies anyway, and the peer times out either way (on the handshake instead of idle). That doesn't justify a new noq method and noq's first tagged release. #4064 deletes the quest.

(written by Opus 5.5)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant