docs: implement release-readiness audit findings #127

Merged
Lochnair merged 8 commits from docs/release-readiness-audit-fixes into main 2026-08-20 08:33:08 +00:00
5 changed files with 87 additions and 17 deletions

No files matched your search

+22 -7
View File
@@ -26,12 +26,10 @@ The installed dispatcher is named `irtt-rs` rather than `irtt` to avoid conflict
The build requires Rust 1.88 or newer. The build requires Rust 1.88 or newer.
From a local checkout: Install the latest release from crates.io:
```sh ```sh
git clone https://github.com/Lochnair/irtt-rs.git cargo install irtt-rs --locked
cd irtt-rs
cargo install --path crates/irtt-app
``` ```
This installs: This installs:
@@ -45,7 +43,15 @@ Each applet can also be built in isolation with `--no-default-features` plus
the applet's own feature. For example, a lightweight client-only install: the applet's own feature. For example, a lightweight client-only install:
```sh ```sh
cargo install --path crates/irtt-app --no-default-features --features client cargo install irtt-rs --locked --no-default-features --features client
```
For development, or to install directly from a checkout:
```sh
git clone https://github.com/Lochnair/irtt-rs.git
cd irtt-rs
cargo install --path crates/irtt-app --locked
``` ```
## Quick start ## Quick start
@@ -133,7 +139,7 @@ Multi-target sessions use staggered pacing by default. To send each target's pro
irtt-client host-a:2112 host-b:2112 --pacing burst irtt-client host-a:2112 host-b:2112 --pacing burst
``` ```
Target labels are included in the default multi-target output. Target labels are included in the default multi-target output. Final per-target summaries are printed in the order targets were supplied on the command line, not the order in which they finish or their labels sort alphabetically.
## Output formats ## Output formats
@@ -179,6 +185,8 @@ Useful measurement fields include:
Adjusted RTT can be negative when server processing exceeds the measured raw RTT. One-way delay estimates can be negative because of clock skew between the client and server. Adjusted RTT can be negative when server processing exceeds the measured raw RTT. One-way delay estimates can be negative because of clock skew between the client and server.
On Linux, `sd_us` and `rd_us` (and the underlying one-way delay samples) prefer a kernel-captured timestamp over userspace timing for this client's own send/receive endpoint where the platform and socket support it, reducing the effect of scheduling and queuing delay on these measurements. The peer's reported endpoint gets the same preference when it is the `irtt-server` applet running on Linux; against another IRTT-compatible server, whatever wall-clock value it reports is used as-is. This is a best-effort fallback: unsupported or implausible kernel timestamps fall back to userspace timing. `raw_rtt_us` always uses userspace timing and is unaffected; `adjusted_rtt_us`/`effective_rtt_us` can still shift slightly, since server processing time is derived from the same server receive endpoint.
Default table output prints a final summary after completed finite runs and interrupted continuous runs when the run policy permits it. CSV, TSV, and JSON Lines output do not print this summary. Default table output prints a final summary after completed finite runs and interrupted continuous runs when the run policy permits it. CSV, TSV, and JSON Lines output do not print this summary.
A peer closing a finite CLI or TUI run is accepted as a terminal outcome. In A peer closing a finite CLI or TUI run is accepted as a terminal outcome. In
@@ -294,7 +302,14 @@ irtt-server --help
`irtt-client` exposes the client session and event layer independently of CLI formatting and statistics. `irtt-client` exposes the client session and event layer independently of CLI formatting and statistics.
Add it from a local checkout: Add it from crates.io:
```toml
[dependencies]
irtt-client = { version = "0.5", features = ["tokio"] }
```
For development against an unpublished change, use a path dependency instead:
```toml ```toml
[dependencies] [dependencies]
+29 -3
View File
@@ -12,6 +12,7 @@
use std::time::Duration; use std::time::Duration;
#[cfg(feature = "client")]
use irtt_client::SignedDuration; use irtt_client::SignedDuration;
/// Rendering used wherever a scalar is unavailable. /// Rendering used wherever a scalar is unavailable.
@@ -61,6 +62,7 @@ pub(crate) fn format_optional_ns_i128(value: Option<i128>) -> String {
} }
/// Format an optional nanosecond scalar, rendering `None` as [`ABSENT`]. /// Format an optional nanosecond scalar, rendering `None` as [`ABSENT`].
#[cfg(feature = "client")]
pub(crate) fn format_optional_ns_f64(value: Option<f64>) -> String { pub(crate) fn format_optional_ns_f64(value: Option<f64>) -> String {
value.map(format_ns_f64).unwrap_or_else(absent) value.map(format_ns_f64).unwrap_or_else(absent)
} }
@@ -71,6 +73,7 @@ pub(crate) fn format_duration(value: Duration) -> String {
} }
/// Format a signed duration as a nanosecond scalar, preserving its sign. /// Format a signed duration as a nanosecond scalar, preserving its sign.
#[cfg(feature = "client")]
pub(crate) fn format_signed_duration(value: SignedDuration) -> String { pub(crate) fn format_signed_duration(value: SignedDuration) -> String {
format_ns_i128(value.as_nanos()) format_ns_i128(value.as_nanos())
} }
@@ -88,6 +91,7 @@ pub(crate) fn format_percent(value: f64) -> String {
/// Format `value` as a percentage of `total`, capped at 100%. /// Format `value` as a percentage of `total`, capped at 100%.
/// ///
/// A zero total has no percentage to report and renders as [`ABSENT`]. /// A zero total has no percentage to report and renders as [`ABSENT`].
#[cfg(feature = "tui")]
pub(crate) fn format_percent_ratio(value: u64, total: u64) -> String { pub(crate) fn format_percent_ratio(value: u64, total: u64) -> String {
if total == 0 { if total == 0 {
ABSENT.to_owned() ABSENT.to_owned()
@@ -97,11 +101,13 @@ pub(crate) fn format_percent_ratio(value: u64, total: u64) -> String {
} }
/// Format a scalar count. /// Format a scalar count.
#[cfg(feature = "tui")]
pub(crate) fn format_count(value: u64) -> String { pub(crate) fn format_count(value: u64) -> String {
value.to_string() value.to_string()
} }
/// Format an optional scalar count, rendering `None` as [`ABSENT`]. /// Format an optional scalar count, rendering `None` as [`ABSENT`].
#[cfg(feature = "tui")]
pub(crate) fn format_optional_count(value: Option<u64>) -> String { pub(crate) fn format_optional_count(value: Option<u64>) -> String {
value.map(format_count).unwrap_or_else(absent) value.map(format_count).unwrap_or_else(absent)
} }
@@ -152,6 +158,11 @@ mod tests {
fn durations_render_as_nanosecond_scalars() { fn durations_render_as_nanosecond_scalars() {
assert_eq!(format_duration(Duration::from_millis(25)), "25.0ms"); assert_eq!(format_duration(Duration::from_millis(25)), "25.0ms");
assert_eq!(format_duration(Duration::from_secs(1)), "1.000s"); assert_eq!(format_duration(Duration::from_secs(1)), "1.000s");
}
#[test]
#[cfg(feature = "client")]
fn signed_durations_render_as_nanosecond_scalars() {
assert_eq!( assert_eq!(
format_signed_duration(SignedDuration::from_nanos(-1_500)), format_signed_duration(SignedDuration::from_nanos(-1_500)),
"-1.5µs" "-1.5µs"
@@ -161,18 +172,33 @@ mod tests {
#[test] #[test]
fn absent_and_non_finite_scalars_render_as_a_placeholder() { fn absent_and_non_finite_scalars_render_as_a_placeholder() {
assert_eq!(format_optional_ns_i128(None), ABSENT); assert_eq!(format_optional_ns_i128(None), ABSENT);
assert_eq!(format_optional_ns_f64(None), ABSENT);
assert_eq!(format_optional_count(None), ABSENT);
assert_eq!(format_ns_f64(f64::NAN), ABSENT); assert_eq!(format_ns_f64(f64::NAN), ABSENT);
assert_eq!(format_ns_f64(f64::INFINITY), ABSENT); assert_eq!(format_ns_f64(f64::INFINITY), ABSENT);
assert_eq!(format_percent(f64::NAN), ABSENT); assert_eq!(format_percent(f64::NAN), ABSENT);
}
#[test]
#[cfg(feature = "client")]
fn absent_optional_ns_f64_renders_as_a_placeholder() {
assert_eq!(format_optional_ns_f64(None), ABSENT);
}
#[test]
#[cfg(feature = "tui")]
fn absent_counts_and_ratios_render_as_a_placeholder() {
assert_eq!(format_optional_count(None), ABSENT);
assert_eq!(format_percent_ratio(1, 0), ABSENT); assert_eq!(format_percent_ratio(1, 0), ABSENT);
} }
#[test] #[test]
fn percentages_and_counts_have_one_spelling() { fn percentages_have_one_spelling() {
assert_eq!(format_percent(0.0), "0.00%"); assert_eq!(format_percent(0.0), "0.00%");
assert_eq!(format_percent(12.345), "12.35%"); assert_eq!(format_percent(12.345), "12.35%");
}
#[test]
#[cfg(feature = "tui")]
fn percent_ratios_and_counts_have_one_spelling() {
assert_eq!(format_percent_ratio(1, 4), "25.00%"); assert_eq!(format_percent_ratio(1, 4), "25.00%");
assert_eq!(format_percent_ratio(5, 4), "100.00%"); assert_eq!(format_percent_ratio(5, 4), "100.00%");
assert_eq!(format_count(42), "42"); assert_eq!(format_count(42), "42");
+16 -1
View File
@@ -351,7 +351,22 @@ pub struct OneWayDelaySample {
/// Client-to-server delay. /// Client-to-server delay.
chatgpt-codex-connector[bot] commented 2026-08-20 08:09:07 +00:00 (Migrated from github.com)
Review

P2 Badge Limit the unaffected claim to raw RTT

When the peer is the Linux irtt-server and negotiates wall-clock receive-and-send timestamps, the preferred kernel receive timestamp also becomes recv_wall; compute_server_processing subtracts it from send_wall, and that processing interval changes RttSample::adjusted and therefore effective. Only raw RTT remains an unaffected userspace-monotonic measurement, so qualify this sentence—and the matching README claim—to avoid misleading users interpreting adjusted/effective RTT results.

Useful? React with 👍 / 👎.

**<sub><sub>![P2 Badge](https://img.shields.io/badge/P2-yellow?style=flat)</sub></sub> Limit the unaffected claim to raw RTT** When the peer is the Linux `irtt-server` and negotiates wall-clock receive-and-send timestamps, the preferred kernel receive timestamp also becomes `recv_wall`; `compute_server_processing` subtracts it from `send_wall`, and that processing interval changes `RttSample::adjusted` and therefore `effective`. Only `raw` RTT remains an unaffected userspace-monotonic measurement, so qualify this sentence—and the matching README claim—to avoid misleading users interpreting adjusted/effective RTT results. Useful? React with 👍 / 👎.
Lochnair commented 2026-08-20 08:12:09 +00:00 (Migrated from github.com)
Review

Fixed: qualified this to raw RTT only. adjusted_rtt_us/effective_rtt_us are derived from server-reported processing time, which shares the same server receive endpoint this field documents as kernel-preferred, so I updated both the client_to_server rustdoc and the matching README sentence (commit c9269ca) to say raw RTT is unaffected while adjusted/effective can still shift.

🤖 Addressed by Claude Code

Fixed: qualified this to raw RTT only. `adjusted_rtt_us`/`effective_rtt_us` are derived from server-reported processing time, which shares the same server receive endpoint this field documents as kernel-preferred, so I updated both the `client_to_server` rustdoc and the matching README sentence (commit c9269ca) to say raw RTT is unaffected while adjusted/effective can still shift. _🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
chatgpt-codex-connector[bot] commented 2026-08-20 08:17:39 +00:00 (Migrated from github.com)
Review

P2 Badge Scope the server kernel timestamp claim to this server

When irtt-client is used with an arbitrary IRTT-compatible peer, it only receives a wall-clock value from the wire and cannot know whether that peer obtained it from the kernel, userspace, or another clock source. This rustdoc therefore overpromises that the server endpoint prefers a plausible kernel timestamp; that behavior is specific to the Linux Tokio runtime in this repository. Qualify this claim—and the matching broad README statement—so consumers do not misinterpret measurements from other servers.

Useful? React with 👍 / 👎.

**<sub><sub>![P2 Badge](https://img.shields.io/badge/P2-yellow?style=flat)</sub></sub> Scope the server kernel timestamp claim to this server** When `irtt-client` is used with an arbitrary IRTT-compatible peer, it only receives a wall-clock value from the wire and cannot know whether that peer obtained it from the kernel, userspace, or another clock source. This rustdoc therefore overpromises that the server endpoint prefers a plausible kernel timestamp; that behavior is specific to the Linux Tokio runtime in this repository. Qualify this claim—and the matching broad README statement—so consumers do not misinterpret measurements from other servers. Useful? React with 👍 / 👎.
Lochnair commented 2026-08-20 08:18:57 +00:00 (Migrated from github.com)
Review

Fixed: scoped the kernel-preference claim to this crate's own irtt-server on Linux. Against any other IRTT-compatible peer, the client has no way to know what clock source produced the reported wall-clock endpoint, so it's now stated as used as-is in that case, in both this rustdoc and the matching README sentence (commit cab1fd1).

🤖 Addressed by Claude Code

Fixed: scoped the kernel-preference claim to this crate's own `irtt-server` on Linux. Against any other IRTT-compatible peer, the client has no way to know what clock source produced the reported wall-clock endpoint, so it's now stated as used as-is in that case, in both this rustdoc and the matching README sentence (commit cab1fd1). _🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
chatgpt-codex-connector[bot] commented 2026-08-20 08:23:06 +00:00 (Migrated from github.com)
Review

P2 Badge Limit the kernel timestamp claim to the Tokio server

When an application pairs this client with the public irtt_server::ServerCore::handle_datagram API on Linux, the peer is still this crate's irtt-server, but no kernel receive timestamp is used: crates/irtt-server/src/core.rs:161-165 states that this path samples every instant itself, while only the crate-private Tokio runtime entry point supplies transport metadata. Qualify this claim, and the matching README sentence, to the Tokio Server/server applet rather than the whole server crate. Fresh evidence is the inspected public ServerCore path, which always calls process(..., None) at core.rs:173-178.

Useful? React with 👍 / 👎.

**<sub><sub>![P2 Badge](https://img.shields.io/badge/P2-yellow?style=flat)</sub></sub> Limit the kernel timestamp claim to the Tokio server** When an application pairs this client with the public `irtt_server::ServerCore::handle_datagram` API on Linux, the peer is still this crate's `irtt-server`, but no kernel receive timestamp is used: `crates/irtt-server/src/core.rs:161-165` states that this path samples every instant itself, while only the crate-private Tokio runtime entry point supplies transport metadata. Qualify this claim, and the matching README sentence, to the Tokio `Server`/server applet rather than the whole server crate. Fresh evidence is the inspected public `ServerCore` path, which always calls `process(..., None)` at `core.rs:173-178`. Useful? React with 👍 / 👎.
Lochnair commented 2026-08-20 08:24:52 +00:00 (Migrated from github.com)
Review

Fixed: scoped this to the Tokio server runtime specifically. ServerCore::handle_datagram is a public API that always passes kernel_rx_timestamp = None; only the crate-private entry point the Tokio runtime (the irtt-server applet) uses actually supplies one. Both the rustdoc and README now say so explicitly (commit 1754b77).

🤖 Addressed by Claude Code

Fixed: scoped this to the Tokio server runtime specifically. `ServerCore::handle_datagram` is a public API that always passes `kernel_rx_timestamp = None`; only the crate-private entry point the Tokio runtime (the `irtt-server` applet) uses actually supplies one. Both the rustdoc and README now say so explicitly (commit 1754b77). _🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
/// ///
/// This is computed from the client send wall time and the server receive /// This is computed from the client send wall time and the server receive
/// wall time when both are available. /// wall time when both are available. The client send endpoint is the
/// correlated Linux kernel TX timestamp for this send when one is
/// available and plausible for this send, and the userspace post-send wall
/// time otherwise. The server receive endpoint is whatever wall time the
/// peer reported; against this crate's own Tokio server runtime on Linux
/// (the `irtt-server` applet), that is its kernel receive timestamp when
/// one is available and plausible, and its userspace receive wall time
/// otherwise. Against another IRTT-compatible peer, or an application
/// driving `irtt_server::ServerCore::handle_datagram` directly, the
/// client has no way to know which clock source produced it — the public
/// `ServerCore` API never supplies a kernel timestamp of its own accord.
/// `RttSample::raw` is unaffected and remains a userspace
/// monotonic measurement, but `adjusted` and `effective` can still shift:
/// when the session negotiated wall-clock timing rather than monotonic,
/// server processing time is derived from this same server receive
/// endpoint.
pub client_to_server: Option<SignedDuration>, pub client_to_server: Option<SignedDuration>,
/// Server-to-client delay. /// Server-to-client delay.
/// ///
+15 -1
View File
@@ -236,7 +236,21 @@ impl ManagedClientHandle {
ManagedStopReceipt::new(Arc::clone(&self.stop)) ManagedStopReceipt::new(Arc::clone(&self.stop))
} }
/// Submit one complete desired target set without awaiting queue capacity. /// Submit the complete desired target set, replacing any previous one.
///
/// `targets` is not a delta: any live target whose id is missing from
/// `targets` is stopped and removed. A target that is still present with an
/// unchanged configuration continues undisturbed — it is not restarted and
/// does not get a new generation. A target present with a *changed*
/// configuration, or whose current generation already reached a terminal
/// state (e.g. failed, completed, or was closed by the peer), is treated as
/// a fresh start: it is retired and a new generation is created for it, even
/// if the configuration is byte-for-byte identical to the one that just
/// finished.
///
/// This call enqueues the update without waiting for queue capacity; await
/// the returned [`ManagedCommandReceipt`] to observe whether — and how — it
/// was applied.
pub fn update_targets( pub fn update_targets(
&self, &self,
targets: Vec<ManagedTargetConfig>, targets: Vec<ManagedTargetConfig>,
+5 -5
View File
@@ -32,11 +32,11 @@
//! instant, is served normally with no timestamp. Nothing about which datagrams //! instant, is served normally with no timestamp. Nothing about which datagrams
//! reach [`ServerCore`](crate::ServerCore) depends on it. //! reach [`ServerCore`](crate::ServerCore) depends on it.
//! //!
//! **Nothing consumes it yet.** It is captured here so a follow-up change can //! [`ServerCore`](crate::ServerCore) prefers it over its own receive sample
//! decide how a kernel-observed arrival time enters the protocol core; today //! when reporting a request's receive instant, falling back to that sample
//! the core still samples its own receive instant and no protocol timing is //! when no plausible kernel reading is available; see
//! affected. It is a software wall-clock reading only — not a NIC hardware //! `preferred_receive_wall_ns`. It is a software wall-clock reading only —
//! timestamp, not monotonic, and not a transmit timestamp. //! not a NIC hardware timestamp, not monotonic, and not a transmit timestamp.
use std::{io, net::SocketAddr, time::SystemTime}; use std::{io, net::SocketAddr, time::SystemTime};