chore(release): promote rc-2026.4.4 - #77
Conversation
- Enhanced relay diagnostic logs for clarity during inbound and outbound traffic handling. - Refined UDP relay logging paths to include detailed source, target, and payload information.
- connection: bound PQC `min_initial_size` by the current path MTU so coalesced Initial+Handshake packets never exceed what the path has proven; restart MTUD from the established MTU instead of jumping to the PQC-preferred floor. - masque: add out-of-band `TunnelControlFrame::PmtuUpdate` emitted by the relay-server on egress `EMSGSIZE` and enforced by the relay-client `MasqueRelaySocket` as a per-target send cap that drives Quinn's DPLPMTUD downward. - masque/relay_server: enable `IP_DONTFRAG` / `IP_PMTUDISC_DO` on the bound socket so oversized egress fails fast instead of silently fragmenting and getting dropped by intermediate NATs. - nat_traversal_api, p2p_endpoint: clamp the inner endpoint's transport config to a 1200-byte initial/min MTU on both accept- and dial-through-relay paths, with DPLPMTUD enabled to probe upward. - p2p_endpoint, config/nat_timeouts: treat `send_ack_timeout` as a best-effort observation window rather than a hard failure — data is already queued to QUIC; later connection state or application-level timeouts handle retries. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ervedExternal
Add `NatTraversalEvent::PeerObservedExternal { peer_addr, observed_external }`
and the corresponding `P2pEvent::PeerObservedExternal`, fired on every distinct
`(peer, observed-address)` pair from the OBSERVED_ADDRESS path — both the
regular discovery loop and the bootstrap reflexive-candidate path. The signal
is emitted regardless of whether saorsa-transport's pinning quorum
(`MIN_OBSERVERS_FOR_QUORUM = 2`) has been reached.
Upper layers can now do per-address attribution of cold-dialability proof:
the peer's frame is their own statement that they reached us at a specific
external. Saorsa-core consumes this in its passive reachability classifier
(separate PR) to credit only the externals each inbound peer actually
reported observing — closing a per-family-leak in the previous design where
two source-disjoint inbounds promoted every same-family pinned external,
including stale NAT rebindings and unrelated externals.
Additive on the public enums; `Node::convert_event` adds the new variant to
its already-ignored arm. Other matchers in saorsa-transport already used
catch-all arms.
Tests: cargo fmt clean, cargo clippy --all-targets --all-features -- -D
warnings clean, 1506 lib tests pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous send path used `write_all` which could block indefinitely
on a stuck stream, and collapsed every failure into the opaque
`EndpointError::Connection(String)`. Upper layers had no way to tell
"open failed" from "stopped by peer" from "stalled mid-write" and so
could not pick a sensible retry strategy.
This introduces:
- `SendFailureStage` enum identifying where a send broke (open,
open progress timeout, write, write progress timeout, finish,
stopped, acknowledgement).
- `EndpointError::SendFailed { stage, bytes_written, reason }` so
callers can branch on stage and know how many bytes the QUIC stream
accepted before the failure.
- `write_stream_with_progress_timeout` helper that writes in 64 KiB
chunks with a per-write progress timeout, returning the byte count
for diagnostics and resetting the stream on failure.
- A wrapping `timeout` around `open_uni` to surface stuck stream
opens the same way.
Constants `STREAM_WRITE_CHUNK_SIZE`, `STREAM_WRITE_PROGRESS_TIMEOUT`
and `STREAM_RESET_ABORT_CODE` are introduced rather than inlined.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…nnections - Introduced distinct timeouts for direct connection stages: progress and handshake. - Enhanced `HappyEyeballs` logging to reflect both timeouts. - Modified `race_connect` for explicit timeout handling at each stage and surface errors appropriately. - Updated `StrategyConfig` to include configurable `direct_handshake_timeout`. - Adjusted defaults and augmented tests to validate new timeout logic.
…cket Add Connecting::first_peer_response() that resolves once the first QUIC packet authenticated by the peer is received, distinct from full handshake completion. P2pEndpoint's Happy Eyeballs direct race now uses it instead of handshake_data() so direct_connect_timeout bounds basic peer reachability while direct_handshake_timeout continues to govern full handshake completion. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The post-`finish()` `stopped()` wait was a 1s+payload best-effort window that tied each send to a remote acknowledgement that may never come on busy or torn-down connections. Per-step progress timeouts (open_uni and write) plus close_reason short-circuiting already cover real failure modes; the extra wait only added latency without delivery guarantees. BREAKING CHANGE: removes the TimeoutConfig::send_ack_timeout field and the SendFailureStage::Stopped/Acknowledgement variants that existed only to drive that path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
fix(p2p,relay): tunnel timeout, MTU, and connect timeout fixes
is_message_too_large referenced libc::EMSGSIZE unconditionally, but the libc crate is declared as [target.'cfg(unix)'.dependencies] and so is unavailable on Windows targets. The Windows build broke with E0433: "cannot find module or crate `libc` in this scope". Match the platform pattern set_dont_fragment already follows in this module: keep the libc-backed implementation behind cfg(unix), and add a non-Unix fallback that returns false. The PMTU control-frame loop simply does not fire on those targets — same outcome as the existing no-op set_dont_fragment fallback for non-Linux/BSD targets. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
|
||
| /// Default packet count for the initial congestion window — matches BBRv1 | ||
| /// in this crate (200 × BASE_DATAGRAM_SIZE). | ||
| const K_INITIAL_WINDOW_PACKETS: u64 = 200; | ||
| /// Default packet count for the initial congestion window. | ||
| /// | ||
| /// Keep this aligned with RFC 9002/quiche-style defaults; callers that want a | ||
| /// more aggressive startup can still override `Bbr2Config::initial_window`. | ||
| const K_INITIAL_WINDOW_PACKETS: u64 = 10; |
There was a problem hiding this comment.
Initial window reduced 20× — bulk throughput impact
K_INITIAL_WINDOW_PACKETS drops from 200 to 10. The old value (200 × ~1 450 B ≈ 290 KB) was far above the RFC 9002 §7.2 limit of 10 packets and the change is correct for compliance. At the same time this is effectively a 20× reduction in the burst the sender is allowed before the first ACK arrives. On high-bandwidth / high-RTT paths (e.g. the relay-tunnelled cross-continent scenarios described in connection_strategy.rs) this will substantially extend the slow-start ramp-up. Worth confirming this was measured against the ant-rc testnet numbers mentioned elsewhere in the codebase before shipping.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/congestion/bbr2/adapter.rs
Line: 30-35
Comment:
**Initial window reduced 20× — bulk throughput impact**
`K_INITIAL_WINDOW_PACKETS` drops from 200 to 10. The old value (200 × ~1 450 B ≈ 290 KB) was far above the RFC 9002 §7.2 limit of 10 packets and the change is correct for compliance. At the same time this is effectively a 20× reduction in the burst the sender is allowed before the first ACK arrives. On high-bandwidth / high-RTT paths (e.g. the relay-tunnelled cross-continent scenarios described in `connection_strategy.rs`) this will substantially extend the slow-start ramp-up. Worth confirming this was measured against the ant-rc testnet numbers mentioned elsewhere in the codebase before shipping.
How can I resolve this? If you propose a fix, please make it concise.| }) | ||
| } | ||
|
|
||
| /// Wait for the first authenticated QUIC packet from the peer. | ||
| /// | ||
| /// This completes before the full handshake and can be used to distinguish basic peer | ||
| /// reachability from handshake completion. | ||
| pub async fn first_peer_response(&mut self) -> Result<(), ConnectionError> { | ||
| if !self.has_peer_response()? { | ||
| if let Some(x) = self.first_peer_response.take() { | ||
| let _ = x.await; | ||
| } | ||
| } | ||
|
|
||
| let conn = self.conn.as_ref().ok_or_else(|| { | ||
| tracing::error!("Connection state missing while waiting for first peer response"); | ||
| ConnectionError::LocallyClosed | ||
| })?; | ||
| let inner = conn.state.lock("first_peer_response"); | ||
| if inner.peer_response_received { | ||
| Ok(()) | ||
| } else { | ||
| Err(inner.error.clone().unwrap_or_else(|| { | ||
| error!("Spurious peer response notification with no error"); | ||
| ConnectionError::TransportError(crate::transport_error::Error::INTERNAL_ERROR( | ||
| "Spurious peer response notification".to_string(), | ||
| )) | ||
| })) | ||
| } | ||
| } | ||
|
|
||
| fn has_peer_response(&self) -> Result<bool, ConnectionError> { | ||
| let conn = self.conn.as_ref().ok_or_else(|| { | ||
| tracing::error!("Connection state missing while checking first peer response"); | ||
| ConnectionError::LocallyClosed | ||
| })?; | ||
| Ok(conn | ||
| .state | ||
| .lock("first_peer_response") | ||
| .peer_response_received) | ||
| } |
There was a problem hiding this comment.
first_peer_response receiver dropped silently if called after connection close
If Connecting::first_peer_response() is called after the connection has already terminated (so self.conn is None), has_peer_response() returns Err(LocallyClosed). This is propagated by ? — correct. However, if self.conn is still Some but peer_response_received is false AND on_first_peer_response has already been taken (e.g., a second call), the function silently skips the oneshot wait and falls through to the state lock. At that point peer_response_received is still false and error may not yet be set, so the function hits the unwrap_or_else branch and logs a spurious error!. Consider guarding against multiple calls more explicitly or documenting that the method is single-use.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/high_level/connection.rs
Line: 190-230
Comment:
**`first_peer_response` receiver dropped silently if called after connection close**
If `Connecting::first_peer_response()` is called after the connection has already terminated (so `self.conn` is `None`), `has_peer_response()` returns `Err(LocallyClosed)`. This is propagated by `?` — correct. However, if `self.conn` is still `Some` but `peer_response_received` is false AND `on_first_peer_response` has already been taken (e.g., a second call), the function silently skips the oneshot wait and falls through to the state lock. At that point `peer_response_received` is still false and `error` may not yet be set, so the function hits the `unwrap_or_else` branch and logs a spurious `error!`. Consider guarding against multiple calls more explicitly or documenting that the method is single-use.
How can I resolve this? If you propose a fix, please make it concise.
Benchmark ResultsPerformance Comparison
SummaryConfiguration
|
Promotes
rc-2026.4.4to release version(s): 0.34.1.-rc.*from[package].versionCargo.lockOnce merged, the release tag will be pushed to fire the publish workflow.
Greptile Summary
This release PR promotes
rc-2026.4.4to version0.34.1, stripping RC suffixes and regeneratingCargo.lock. Despite the chore label, the diff contains a substantial set of substantive changes across the transport, congestion, NAT traversal, and relay layers — all of which appear to be feature/fix work bundled into the RC that is now being released.unacked_packetsswitched from a monotonic scalar to aBTreeSetfor correct out-of-order ACK tracking; lost MTU probes now callon_packet_abandonedto avoid spurious congestion reactions; congestion window check changed from>=to>with overflow safety viasaturating_add.tunnel_control.rsmodule introduces an out-of-band control-frame protocol; relay-server now setsIP_PMTUDISC_DO/IP_DONTFRAGon egress sockets and emitsPmtuUpdateframes onEMSGSIZE; relay-client'sMasqueRelaySocket::try_sendsilently drops oversized packets to drive Quinn DPLPMTUD downward.AttemptHandlesRAII wrapper ensures in-flight tasks are aborted when the outer future is dropped; connection timeout split intodirect_connect_timeoutanddirect_handshake_timeout; newfirst_peer_responseoneshot enables two-phase timeout; socket-address normalization applied throughout to eliminate IPv4 vs. IPv4-mapped-IPv6 lookup mismatches.Confidence Score: 3/5
Hold on the release tag until the datagram-loop EMSGSIZE gap in relay_server.rs is resolved; all other changes are sound.
The relay PMTU feedback mechanism is partially wired: stream-based relay sessions benefit from the new PmtuUpdate control frames, but datagram-loop relay sessions now have DF=1 set on their egress socket without any corresponding feedback path, meaning MTU-exceeded datagrams will be silently dropped for the life of those sessions. Every other change — BBR2 BTreeSet tracking, AttemptHandles RAII, socket normalization, first_peer_response two-phase timeout — looks correct and well-tested.
src/masque/relay_server.rs (datagram-loop EMSGSIZE handling); src/p2p_endpoint.rs (Ok(0) write branch)
Important Files Changed
Sequence Diagram
sequenceDiagram participant Quinn as Quinn (relay-client) participant RS as MasqueRelaySocket participant RServer as MasqueRelayServer participant Target as Remote Target Note over Quinn,Target: Stream-based relay with PMTU feedback (new) Quinn->>RS: try_send(Transmit { destination: Target }) RS->>RS: check target_mtu[Target]? alt segment > MTU cap RS-->>Quinn: Ok(()) drop silently, DPLPMTUD sees loss else within cap RS->>RServer: UncompressedDatagram frame (stream) RServer->>Target: UDP send_to(payload) alt EMSGSIZE RServer->>RServer: encode PmtuUpdate{target, mtu=1200} RServer-->>RS: WriterItem::Control(PmtuUpdate) RS->>RS: target_mtu.insert(Target, 1200) else OK Target-->>RServer: UDP reply RServer-->>RS: UncompressedDatagram (Direction 1) RS-->>Quinn: recv datagram end end Note over Quinn,Target: Happy Eyeballs with split timeouts (new) participant P2P as P2pEndpoint participant Peer as Remote Peer P2P->>Peer: QUIC connect(addr) P2P->>P2P: timeout(connect_timeout=1s, first_peer_response()) Peer-->>P2P: first authenticated QUIC packet P2P->>P2P: peer_response_received = true P2P->>P2P: timeout(handshake_timeout=4s, connecting.await) Peer-->>P2P: handshake complete P2P->>P2P: ConnectionEstablishedComments Outside Diff (2)
src/masque/relay_server.rs, line 1116-1151 (link)The stream-forwarding loop (
run_stream_forwarding_loop) now catchesEMSGSIZEfromsend_toand emits aPmtuUpdatecontrol frame back to the relay-client. The datagram-loop inhandle_session_innerreceived the sameset_dont_fragmenttreatment on the egress socket, so it will also start seeingEMSGSIZEfailures — but the only response here is awarn!log and a silent packet drop. Becausectrl_txis not threaded into this path, noPmtuUpdateframe is sent, and the relay-client'starget_mtumap is never populated for sessions that go through this code path. Packets will keep hitting the MTU cap silently for the lifetime of the session.Prompt To Fix With AI
src/p2p_endpoint.rs, line 637-720 (link)Ok(0)fromSendStream::write()treated as a hard fatal errorwrite_stream_with_progress_timeoutreturnsErr(SendFailed { stage: Write, ... })whenever Quinn'swrite()yieldsOk(0). Quinn'sSendStream::write()is documented to returnOk(0)when the stream has been stopped (i.e. the peer already reset it withSTOP_SENDING). In that case the correct response is to surface the stop error — butwrite()can also transiently returnOk(0)on an empty-capacity flow-control window before the async machinery has a chance to park. Treating this as a fatal error causes the stream to be reset with code 0 and the send to fail even though the connection may be healthy. MatchingOk(0)separately and checkingsend_stream.stopped()for aSTOP_SENDINGcode, or just retrying the write, would be safer.Prompt To Fix With AI
Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "chore(release): promote rc-2026.4.4 to 0..." | Re-trigger Greptile