Merge branch 'docs/audit-corrections'

crates/irtt-client/AGENTS.md

@@ -26,30 +26,30 @@ configuration (`try_enable_tx_timestamping` restores it explicitly) and
 timing falls back to userspace `sent_at` as it always has. TX capability
 failure is never a public error and is never logged from the library.
 
-### Correlation invariant: kernel ID == wire sequence
+### Best-effort kernel-ID correlation
 
-The client uses the kernel's automatic `OPT_ID` directly as the probe's wire
+The client uses the kernel's automatic `OPT_ID` as the probe's wire-sequence
-sequence number — no separate correlation map. This is safe because:
+correlation ID — no separate correlation map. On the normal successful-send
+path, both counters start at zero after Open and advance together. This is
+not a permanent identity: a kernel ID can theoretically be consumed while a
+send is being prepared and then survive a subsequent send failure, leaving
+kernel IDs one or more steps ahead of `next_wire_seq`. Do not restore an
+assumption that failed or nonblocking sends cannot cause that desynchrony.
 
-- **A failed/nonblocking send never consumes an ID.** Linux increments the
+The effect is confined to the optional timing enhancement. A kernel timestamp
-  per-socket ID counter (`sk->sk_tskey`) speculatively while building a
+whose ID cannot be matched to a pending or retained timed-out probe is
-  datagram and rolls the increment back (`atomic_dec`) on every error path
+discarded, as is any implausible timestamp; `sent_at` remains the ordinary
-  out of `__ip_append_data`/`__ip6_append_data`
+userspace fallback. **After every failed probe submission, disable kernel-ID
-  (`net/ipv4/ip_output.c`, `net/ipv6/ip6_output.c`), guarded by a local
+correlation for the rest of that session.** A gap means an otherwise plausible
-  `hold_tskey` flag. A `WouldBlock` `try_send` therefore can never leave a
+later ID can name the wrong probe, so subsequent error-queue timestamps must
-  gap in, or otherwise advance, the ID the kernel will assign to the next
+be discarded rather than attached by ID. This is safe because:
-  *successful* submission. (`include/net/sock.h`'s `_sock_tx_timestamp`
+
-  performs the same increment for the non-corked path.) Client-side, the
+- **Normal successful sends track together.** `wire_seq` starts at 0 on every
-  existing send transaction only calls `commit_probe_sent` — which is the
+  successful Open (`SessionMachine::commit_open` builds a fresh
-  only thing that advances `next_wire_seq` — after a successful socket send;
+  `ActiveSession` with `next_wire_seq: 0`) and advances by exactly 1 only via
-  a `WouldBlock` result never reaches it.
+  `commit_probe_sent`, after a confirmed successful socket send. TX
-- **The two counters start together and advance together.** `wire_seq`
+  timestamping is enabled after that same successful Open and before any probe
-  starts at 0 on every successful Open (`SessionMachine::commit_open` builds
+  is sent, so the normal zero points line up.
-  a fresh `ActiveSession` with `next_wire_seq: 0`) and advances by exactly 1
-  only via `commit_probe_sent`, which only runs after a confirmed successful
-  send — the same condition under which the kernel's counter advances by 1.
-  TX timestamping is enabled only after that same successful Open and before
-  any probe is sent, so both counters' zero points line up exactly.
 - **One socket, one session, no reopen.** `SessionMachine`'s state machine
   only reaches `Open` from `Connected`, and `commit_open`/`commit_local_close`
   never route back to `Connected`. A `Client`/`AsyncClient` cannot reopen a

crates/irtt-client/src/async_client.rs

@@ -608,8 +608,14 @@ impl AsyncClient {
                 // No sent_at measurement, no machine commit, no schedule
                 // commit: the transaction is fully discarded and retried
                 // with a fresh send_anchor on the next loop iteration.
-                Err(error) if error.kind() == io::ErrorKind::WouldBlock => continue,
+                Err(error) if error.kind() == io::ErrorKind::WouldBlock => {
-                Err(error) => return Poll::Ready(Err(ClientError::Socket(error))),
+                    self.machine.invalidate_kernel_tx_correlation();
+                    continue;
+                }
+                Err(error) => {
+                    self.machine.invalidate_kernel_tx_correlation();
+                    return Poll::Ready(Err(ClientError::Socket(error)));
+                }
             };
 
             #[cfg(not(test))]

crates/irtt-client/src/client.rs

@@ -163,12 +163,13 @@ impl Client {
     ///
     /// On success, returns the negotiated open outcome and transitions the
     /// client into either an open probe session or completed no-test state.
-    /// Open attempts use [`ClientConfig::open_timeouts`]. Malformed, unrelated,
+    /// Open attempts use [`ClientConfig::open_timeouts`]. Malformed or
-    /// or unauthenticated datagrams are ignored until the current attempt's
+    /// unrelated datagrams are ignored until the current attempt's absolute
-    /// absolute deadline, so one attempt may consume several datagrams without
+    /// deadline, so one attempt may consume several datagrams without
     /// retransmitting. Silence or ignored traffic eventually produces
-    /// [`ClientError::OpenTimeout`], while authenticated incompatibility remains
+    /// [`ClientError::OpenTimeout`]. A structurally recognizable malformed or
-    /// terminal.
+    /// incompatible Open reply from the connected peer is terminal; when HMAC
+    /// is configured, that recognition additionally requires authentication.
     ///
     /// When a trusted reply allocates a token but later negotiation or socket
     /// preparation fails, the client sends a best-effort cleanup close and
@@ -523,11 +524,18 @@ impl Client {
             .unwrap_or_else(Instant::now);
         #[cfg(test)]
         if fail_send {
+            runtime.invalidate_kernel_tx_correlation();
             return Err(ClientError::Socket(io::Error::other(
                 "injected probe send failure",
             )));
         }
-        let bytes = socket.send(&prepared.bytes)?;
+        let bytes = match socket.send(&prepared.bytes) {
+            Ok(bytes) => bytes,
+            Err(error) => {
+                runtime.invalidate_kernel_tx_correlation();
+                return Err(ClientError::Socket(error));
+            }
+        };
         #[cfg(not(test))]
         let send_finished_at = Instant::now();
         #[cfg(test)]

crates/irtt-client/src/probe.rs

@@ -39,7 +39,8 @@ pub(crate) struct PendingProbe {
     /// eligible only for upstream one-way delay after local plausibility
     /// validation against `tx_not_before_wall` (see `compute_one_way`'s
     /// `preferred_send_wall`). `sent_at` remains the userspace fallback for
-    /// upstream one-way delay when this is absent or implausible.
+    /// upstream one-way delay when this is absent, implausible, or kernel-ID
+    /// correlation was invalidated by a failed probe submission.
     pub kernel_tx_timestamp: Option<SystemTime>,
 }
 

crates/irtt-client/src/receive/linux.rs

@@ -12,14 +12,17 @@
 //! After a successful Open, the client adapter best-effort upgrades the
 //! socket from [`RX_TIMESTAMPING_FLAGS`] to [`error_queue::TX_TIMESTAMPING_FLAGS`]
 //! via [`try_enable_tx_timestamping`]. When that succeeds, the automatic
-//! `SOF_TIMESTAMPING_OPT_ID` counter the kernel assigns to each successfully
+//! `SOF_TIMESTAMPING_OPT_ID` counter normally tracks the probe wire sequence
-//! submitted datagram is used directly as the probe's wire sequence number
+//! after successful sends, so it is used as a best-effort correlation ID.
-//! (see the client crate's `AGENTS.md` for the correlation invariant this
+//! A kernel ID can theoretically be consumed by a send that later fails,
-//! relies on). [`drain_tx_timestamps`] performs a small bounded, nonblocking
+//! desynchronizing that correlation. [`drain_tx_timestamps`] discards
-//! read of `MSG_ERRQUEUE` so the adapter can opportunistically collect those
+//! unmatched or implausible records; the userspace `sent_at` timestamp remains
-//! timestamps without ever waiting for one. The observed kernel TX wall time
+//! the fallback. After a probe send failure, correlation is disabled for that
-//! is retained as metadata only; it is not consumed by any measurement in
+//! session so later IDs cannot be misattributed. It performs a small bounded,
-//! this change.
+//! nonblocking read of
+//! `MSG_ERRQUEUE` so the adapter can opportunistically collect timestamps
+//! without ever waiting for one. The observed kernel TX wall time is retained
+//! as metadata only; it is not consumed by any measurement in this change.
 
 use std::{
     io::{self, IoSliceMut},

crates/irtt-client/src/receive/linux/error_queue.rs

@@ -624,22 +624,14 @@ mod tests {
         ));
     }
 
-    // NOTE: a live test proving that a failed nonblocking `sendmsg` does not
+    // NOTE: There is no live test for whether a failed nonblocking `sendmsg`
-    // consume an automatic OPT_ID is not included here. Forcing a reliable,
+    // consumes an automatic OPT_ID. Forcing a reliable, non-flaky `WouldBlock`
-    // non-flaky `WouldBlock` on a connected loopback UDP socket (e.g. via
+    // on a connected loopback UDP socket (for example through socket-buffer
-    // socket-buffer saturation) has no deterministic OS-level mechanism
+    // saturation) has no deterministic OS-level mechanism in this environment,
-    // available in this environment, and the repository's testing policy
+    // and the repository's testing policy forbids sleep/saturation-based
-    // explicitly forbids faking coverage with sleep/saturation-based
+    // flakiness. The correlation is intentionally best effort: a kernel ID can
-    // flakiness. That safety is instead established from Linux kernel
+    // theoretically be consumed by a send that later fails. The deterministic
-    // source: `_sock_tx_timestamp` (include/net/sock.h) and
+    // adapter test covers only the local half: an injected `WouldBlock` does
-    // `__ip_append_data`/`__ip6_append_data` (net/ipv4/ip_output.c,
+    // not advance the local wire-sequence counter. Any later unmatched kernel
-    // net/ipv6/ip6_output.c) increment `sk->sk_tskey` speculatively before
+    // timestamp is discarded and the userspace `sent_at` remains the fallback.
-    // a datagram is fully built and explicitly `atomic_dec` it back out on
-    // every error path (tracked via a local `hold_tskey` flag), so a failed
-    // send can never leave a gap in — or otherwise advance — the ID space
-    // the kernel hands out to successfully submitted datagrams. See the
-    // client crate's `AGENTS.md` for the full citation. The client-side
-    // half of this invariant (a `WouldBlock` `try_send` never advances the
-    // local wire-sequence counter used as the correlation ID) is covered by
-    // a deterministic test in the adapter module that injects `WouldBlock`.
 }

crates/irtt-client/src/session/machine.rs

@@ -60,6 +60,7 @@ struct ActiveSession {
     pending: PendingMap,
     timed_out: TimedOutMap,
     completed: CompletedSet,
+    kernel_tx_correlation_valid: bool,
 }
 
 #[derive(Debug)]
@@ -423,15 +424,27 @@ impl SessionMachine {
         }
     }
 
+    /// Disable Linux kernel TX timestamp correlation after a probe send fails.
+    ///
+    /// A failed submission can consume a kernel `SOF_TIMESTAMPING_OPT_ID`
+    /// without advancing the wire sequence. Existing associated timestamps
+    /// remain valid observations; later error-queue timestamps cannot be
+    /// associated safely and are ignored for the rest of this session.
+    pub(crate) fn invalidate_kernel_tx_correlation(&mut self) {
+        if let MachineState::Open(session) = &mut self.state {
+            session.kernel_tx_correlation_valid = false;
+        }
+    }
+
     /// Record an observed Linux kernel TX timestamp for `wire_seq`, the
     /// automatic `SOF_TIMESTAMPING_OPT_ID` the kernel assigned when the
     /// datagram was submitted.
     ///
-    /// `wire_seq` doubles as that correlation ID: both counters start at
+    /// `wire_seq` normally matches that correlation ID after successful
-    /// zero for a session's first timestamped send and advance by exactly
+    /// sends. This is best effort rather than an invariant: a kernel ID can
-    /// one only on a confirmed successful submission, so they stay
+    /// theoretically be consumed by a send that later fails, leaving the
-    /// identical for the life of one open session (see the client crate's
+    /// counters desynchronized. An unmatched or implausible ID is discarded;
-    /// `AGENTS.md` for the full invariant).
+    /// the probe's userspace `sent_at` remains the fallback.
     ///
     /// Updates a still-pending or already-timed-out probe in place. Never
     /// resurrects a completed, evicted, or unknown probe, and never touches
@@ -443,6 +456,9 @@ impl SessionMachine {
         let MachineState::Open(session) = &mut self.state else {
             return;
         };
+        if !session.kernel_tx_correlation_valid {
+            return;
+        }
         if let Some(probe) = session.pending.get_mut(wire_seq) {
             probe.kernel_tx_timestamp.get_or_insert(timestamp);
             return;
@@ -636,6 +652,7 @@ impl SessionMachine {
             pending: PendingMap::new(self.config.max_pending_probes),
             timed_out: TimedOutMap::new(self.config.max_pending_probes),
             completed: CompletedSet::new(self.config.max_pending_probes),
+            kernel_tx_correlation_valid: true,
         }));
 
         let event = ClientEvent::SessionStarted {
@@ -1231,6 +1248,7 @@ mod tests {
             pending: PendingMap::new(max_pending_probes),
             timed_out: TimedOutMap::new(max_pending_probes),
             completed: CompletedSet::new(max_pending_probes),
+            kernel_tx_correlation_valid: true,
         }));
         machine
     }
@@ -2873,6 +2891,25 @@ mod tests {
         );
     }
 
+    #[test]
+    fn invalidated_kernel_tx_correlation_uses_userspace_send_time() {
+        let mono = Instant::now();
+        let mut machine = upstream_probe(send_wall_ms(1_000), mono);
+
+        // A failed send can consume a kernel ID without advancing wire_seq.
+        // A later timestamp whose ID happens to match a pending wire sequence
+        // must not be associated after that gap.
+        machine.invalidate_kernel_tx_correlation();
+        machine.record_kernel_tx_timestamp(0, send_wall_ms(1_005));
+
+        let events = upstream_reply(&mut machine, 1_020, send_wall_ms(1_040), mono);
+
+        assert_eq!(
+            reply_one_way(&events).unwrap().client_to_server,
+            Some(SignedDuration::from_nanos(20_000_000))
+        );
+    }
+
     #[test]
     fn upstream_one_way_delay_falls_back_without_kernel_tx() {
         let mono = Instant::now();
@@ -3195,6 +3232,7 @@ mod tests {
                 pending: PendingMap::new(MAX_PENDING_PROBES),
                 timed_out: TimedOutMap::new(MAX_PENDING_PROBES),
                 completed: CompletedSet::new(MAX_PENDING_PROBES),
+                kernel_tx_correlation_valid: true,
             }));
             (machine, params)
         }

crates/irtt-server/AGENTS.md

@@ -500,13 +500,14 @@ session policy lives.
   unmarked reply into a *dropped* one, because an unappliable marking stops the
   send. Verify a platform before extending that list.
 - A genuinely dual-stack `[::]` listener still emits **unmarked** IPv4-mapped
-  replies, and this is known. It is not fixed by choosing the option per reply,
+  replies on Linux. When reply DSCP/traffic-class marking for IPv4 peers
-  for the macOS reason above, and it predates wildcard source selection. Leave it
+  matters, configure the normal same-port `[::]:PORT` plus `0.0.0.0:PORT`
-  until it is worth its own slice, and do not "fix" it by weakening the rule that
+  listener pair instead; `ServerSet` makes the IPv6 half IPv6-only for that
-  an unappliable marking stops the send. The crate forbids unsafe code;
+  pair. Do not force `IPV6_V6ONLY` on every lone IPv6 listener, and do not
-  there is no raw `setsockopt` and no `sendmsg`/`cmsg` machinery. Keep the
+  weaken the rule that an unappliable marking stops the send. The crate forbids
-  target lists identical to the pinned `socket2` version's own gates — check
+  unsafe code; there is no raw `setsockopt` and no `sendmsg`/`cmsg` machinery.
-  its source rather than copying another crate's older matrix.
+  Keep the target lists identical to the pinned `socket2` version's own gates
+  — check its source rather than copying another crate's older matrix.
 
 ## Rate, lifetime and server-initiated close
 

crates/irtt-server/src/set.rs

@@ -91,6 +91,14 @@ impl ServerSet {
     /// An IPv4-mapped address such as `[::ffff:0.0.0.0]` is an IPv4 listener
     /// here, not an IPv6 one, matching how the runtime already reads it.
     ///
+    /// A lone explicit dual-stack `[::]:PORT` listener is not the recommended
+    /// configuration when reply DSCP/traffic-class marking for IPv4 peers
+    /// matters: Linux serves those peers as IPv4-mapped and cannot retain the
+    /// negotiated marking on their replies. Request the normal same-port
+    /// `[::]:PORT` and `0.0.0.0:PORT` pair instead. This method makes the IPv6
+    /// half IPv6-only for that pair only; a lone IPv6 listener retains its
+    /// normal socket behavior.
+    ///
     /// # Errors
     ///
     /// Returns [`ServerSetError::NoListeners`] if `addrs` is empty — a set

docs/irtt-server.md

@@ -63,6 +63,15 @@ different ports.
 With no `--bind` at all, see [Description](#description) above for the
 default wildcard pair and the address-family fallback it allows.
 
+When reply DSCP/traffic-class marking for IPv4 peers matters, do not configure
+one explicit dual-stack `[::]:PORT` listener by itself. On Linux, IPv4 peers
+served through that socket are IPv4-mapped and their replies cannot retain the
+negotiated marking. Configure the ordinary paired `[::]:PORT` and
+`0.0.0.0:PORT` listeners instead; the IPv6 listener is made IPv6-only for that
+same-port pair, so each family uses the correct socket option. This is a
+configuration workaround, not a reason to force `IPV6_V6ONLY` for every lone
+IPv6 listener.
+
 ### `--hmac KEY`
 
 HMAC key, taken as the UTF-8 bytes of this argument. With a key configured,