fix(server): measure bandwidth across a single large graphics write

[Greg Lamberson (glamberson)]

Summary

• Continuous bandwidth measurement opened a fixed window of RTT ticks (Start, four ticks, Stop). The client counts the ordinary traffic between Start and Stop (MS-RDPBCGR 3.2.5.14), so a window over a quiet desktop timed idle time: on a 2 ms LAN the server reported a few hundred kbps, and mstsc told the user the network was slow.
• When an EGFX write of at least 10 KiB goes out, the server now sends Bandwidth Measure Start immediately before it and Stop immediately after, on the same stream, so the client times a burst the link actually carried. At most one such measurement per second, and never while one is outstanding. gnome-remote-desktop measures the same way.
• The tick window takes over whenever no bracketed measurement has started for eight RTT ticks, so a session without EGFX, or one whose only large frame was the initial render, keeps measuring.
• A bandwidth measurement the client never answers, or a bracket whose Stop is never sent, is abandoned after the same 30 s as an unanswered RTT probe (expire_stale_probes), instead of stopping bandwidth measurement for the rest of the session. This also covers the tick window, which could already get stuck that way.
• A Bandwidth Measure Results with a zero time delta now counts as one millisecond instead of discarding the figure: The client's timer has millisecond resolution, and a burst on a fast link routinely completes within one. A result with no bytes counted still clears the stored figure.
• New public API: AutoDetectManager::begin_bandwidth_measure and AutoDetectManager::end_bandwidth_measure. Additive; build_bandwidth_measure is unchanged.

Validation

cargo xtask check fmt/lints/tests/typos/locks all pass. Nine new tests in ironrdp-testsuite-core cover the bracket, the size threshold, the pacing, the tick window staying closed while brackets recur and resuming when they stop, the expiry of an unended bracket and of an unanswered measurement, a slow tick window still reaching its Stop, and the zero-delta rule. The existing test that treated a zero time delta as unusable now checks a zero byte count instead.

[github-actions]

This pull request may overlap with #2031.

Both bracket large EGFX server writes with Bandwidth Measure Start/Stop for auto-detect per MS-RDPBCGR: this PR brackets message-channel writes, while #2031 brackets EGFX batches carried on the UDP tunnel with tunnel sub-headers. The shared scope is the server's bandwidth measurement around EGFX traffic, which a human should assess for interaction.

This notice is advisory only. Automated review continues as usual, and how these pull requests relate is for maintainers and authors to decide.

Note

LLM-assisted content (no human feedback).

[github-actions]

The PR brackets large EGFX writes with Bandwidth Measure Start/Stop so the client times a real burst, retires the tick-window measurement after the first bracket, floors zero time deltas at one millisecond, and rejects zero byte counts. Independent inspection confirms the bracketing is protocol-conforming, the zero-delta floor only under-reports, and the in-tree Egfx caller pairs begin/end and aborts on write errors. Published: a one-way latch retires the tick window after a single large write, leaving one-large-write-then-quiet sessions without any bandwidth re-measurement; the new public bracket API can wedge the state machine with no timeout or cancel; and three no-behavior-change compression nits in the new code.

[github-actions]

PR 2021 replaces the fixed tick-window bandwidth measurement (which times idle desktop stretches and reports implausibly low figures) with a Start/data/Stop bracket around EGFX writes of at least 10 KiB, paced to one per second, with the tick window kept as a fallback after eight bracket-free ticks, and adds 30 s expiry for abandoned bandwidth transactions. Independent inspection confirms the mechanism is correct: the bracketed continuous Start/Stop go out back-to-back with matching sequence numbers on the message channel, brackets never overlap and hold off the tick window, the tick counter reset keeps the fallback closed only while brackets recur, expiry covers both a Stop that is never sent and an unanswered measurement without cutting short an open window, and sequence allocation is centralized. The wire-level integration in server.rs matches the existing auto-detect path. Remaining concerns are the unverified zero-timeDelta semantic inversion (which also deviates from the spec-ma…

1. [protocol + skeptical] Zero timeDelta clamped to 1 ms without client-behavior evidence, deviating from the spec-mandated calculation — medium 🟠 ❓ — crates/ironrdp-server/src/autodetect.rs
   measured_bandwidth_kbps inverts the previously tested semantic (zero timeDelta made the result unusable and cleared the stored figure) into clamp-to-1-ms-and-report. The timer-resolution argument is physically plausible and the old rule would discard most bracketed measurements on fast links, but nothing in the repository shows clients never use timeDelta 0 as a no-measurement sentinel, and the PR cites no evidence that gnome-remote-desktop clamps zero deltas. A client that does mean 'invalid' receives a fabricated byte_count*8 kbps figure (capped at u32::MAX, ~4.3 Tbps) advertised in RDP_NETCHAR_RESULTS. The handling also intentionally deviates from the MS-RDPBCGR-mandated (byteCount*8)/timeDelta calculation at both degenerate inputs (zero bytes yields None rather than 0 kbps; zero delta floors to 1 ms where the formula is undefined); the deviation is defensible and partly documented, but the divergence from the MUST clause should be documented against the spec. Missing client-behavior context prevents a firm conclusion.
2. [skeptical] measured_bandwidth_kbps duplicates the PDU crate's computed_bandwidth_kbps, which now has no production callers — low 🟡 — crates/ironrdp-server/src/autodetect.rs
   After this change the server computes bandwidth from the raw Bandwidth Measure Results fields with a private helper, and a search confirms AutoDetectResponse::computed_bandwidth_kbps (ironrdp-pdu/src/rdp/autodetect.rs:613) is referenced only by its own unit tests; the server was its sole production caller. The same wire formula now lives in two crates with deliberately divergent zero handling, so any future correction (rounding, overflow, units) must be found and mirrored by hand, and the two implementations will silently disagree on what a client's result means. The stated goal of avoiding the impossible branch did not require duplication: the special case could wrap a call to the PDU helper or the helper could take the clamped delta.