From 7396856fb05a765249f8f39f653c3d2f62eb0626 Mon Sep 17 00:00:00 2001 From: Greg Lamberson Date: Sat, 12 Sep 2026 15:21:25 -0500 Subject: [PATCH 1/2] fix(server): log bandwidth measure's raw inputs, not just the figure bandwidth_kbps alone reads as noise on a damage-driven video source: a single 1.25s measurement window catches either near-idle traffic (a handful of cursor/autodetect PDUs, ~1.8KB) or a real EGFX frame landing in it (tens of KB), so the same unthrottled link reports anywhere from ~11kbps to ~600kbps depending on what the encoder happened to be doing in that specific window. Logging time_delta_ms/byte_count alongside the computed figure makes that bimodality visible instead of looking like a calculation bug -- the formula (byte_count*8/time_delta_ms) was already correct; the volatility is inherent to counting real traffic over a short window against a bursty source, not a defect. --- crates/ironrdp-server/src/server.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/crates/ironrdp-server/src/server.rs b/crates/ironrdp-server/src/server.rs index 5c9151b44..2cb37e3a8 100644 --- a/crates/ironrdp-server/src/server.rs +++ b/crates/ironrdp-server/src/server.rs @@ -3793,8 +3793,24 @@ impl RdpServer { } AutoDetectOutcome::Bandwidth(Some(bandwidth_kbps)) => { self.autodetect_bandwidth.store(bandwidth_kbps, Ordering::Relaxed); + // Logging the raw inputs, not just the computed figure: a + // damage-driven video source makes any single measurement + // window's byte count wildly bimodal (near-idle vs. a real + // frame landing in it), so bandwidth_kbps alone reads as + // noise without time_delta_ms/byte_count alongside it to + // show why. + let rdp::autodetect::AutoDetectResponse::BandwidthMeasureResults { + time_delta_ms, + byte_count, + .. + } = &pdu.response + else { + unreachable!("computed_bandwidth_kbps() only returns Some for this variant") + }; debug!( bandwidth_kbps, + time_delta_ms, + byte_count, seq = pdu.response.sequence_number(), "Bandwidth measured" ); From bcbbb9a39f40275ee8aa55e51ee610f90281fffe Mon Sep 17 00:00:00 2001 From: Greg Lamberson Date: Sat, 12 Sep 2026 18:22:30 -0500 Subject: [PATCH 2/2] feat(server): add bandwidth-measure generation counter The bandwidth figure alone repeats too often to tell a fresh measurement window apart from a stale one (a quiet link reads the same low figure for several consecutive windows). Expose a counter that increments on every completed Bandwidth Measure transaction, successful or not, so a consumer can gate its own filtering logic on "a new window just closed" instead of diffing the value itself. --- crates/ironrdp-server/src/builder.rs | 16 ++++++++++++++++ crates/ironrdp-server/src/server.rs | 27 +++++++++++++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/crates/ironrdp-server/src/builder.rs b/crates/ironrdp-server/src/builder.rs index 963f94d47..c13730681 100644 --- a/crates/ironrdp-server/src/builder.rs +++ b/crates/ironrdp-server/src/builder.rs @@ -56,6 +56,7 @@ pub struct BuilderDone { autodetect_rtt: Option>, autodetect_baseline_rtt: Option>, autodetect_bandwidth: Option>, + autodetect_bandwidth_generation: Option>, honor_client_desktop_size: Option, auto_reconnect_cookie: Option, connection_policy: ConnectionPolicy, @@ -168,6 +169,7 @@ impl RdpServerBuilder { autodetect_rtt: None, autodetect_baseline_rtt: None, autodetect_bandwidth: None, + autodetect_bandwidth_generation: None, honor_client_desktop_size: None, connection_policy: ConnectionPolicy::default(), auto_reconnect_cookie: None, @@ -201,6 +203,7 @@ impl RdpServerBuilder { autodetect_rtt: None, autodetect_baseline_rtt: None, autodetect_bandwidth: None, + autodetect_bandwidth_generation: None, honor_client_desktop_size: None, connection_policy: ConnectionPolicy::default(), auto_reconnect_cookie: None, @@ -406,6 +409,18 @@ impl RdpServerBuilder { self } + /// Inject a shared handle that increments every time a Bandwidth Measure + /// transaction completes, whether or not it produced a usable figure. + /// Pairs with [`Self::with_autodetect_bandwidth_handle`]: the bandwidth + /// figure alone repeats too often to tell a fresh measurement window + /// apart from a stale one. When not called, the server allocates its own + /// (still readable via + /// [`RdpServer::autodetect_bandwidth_generation_handle`]). + pub fn with_autodetect_bandwidth_generation_handle(mut self, handle: Arc) -> Self { + self.state.autodetect_bandwidth_generation = Some(handle); + self + } + /// Provision the Server Auto-Reconnect Cookie (MS-RDPBCGR 2.2.4.2 /// `ARC_SC_PRIVATE_PACKET`) handed to the client during logon. /// @@ -480,6 +495,7 @@ impl RdpServerBuilder { self.state.autodetect_rtt, self.state.autodetect_baseline_rtt, self.state.autodetect_bandwidth, + self.state.autodetect_bandwidth_generation, ); server.set_credential_validator(self.state.credential_validator); server.set_auto_reconnect_cookie(self.state.auto_reconnect_cookie); diff --git a/crates/ironrdp-server/src/server.rs b/crates/ironrdp-server/src/server.rs index 2cb37e3a8..413fef153 100644 --- a/crates/ironrdp-server/src/server.rs +++ b/crates/ironrdp-server/src/server.rs @@ -739,6 +739,16 @@ pub struct RdpServer { /// alone does not fix. autodetect_bandwidth: Arc, + /// Increments every time a Bandwidth Measure transaction completes + /// (whether or not it produced a usable figure — see + /// [`Self::autodetect_bandwidth`]'s doc comment on the None case). + /// [`Self::autodetect_bandwidth`] alone cannot tell an embedder "a new + /// window just closed" apart from "the value happens to repeat" — this + /// value repeats often (a quiet link reads the same low figure for + /// several consecutive windows), so diffing it is not a valid freshness + /// signal. Exposed via [`Self::autodetect_bandwidth_generation_handle`]. + autodetect_bandwidth_generation: Arc, + /// Optional Server Auto-Reconnect Cookie (MS-RDPBCGR 2.2.4.2 /// `ARC_SC_PRIVATE_PACKET`). When `Some`, the server validates a returning /// `ARC_CS_PRIVATE_PACKET`, replaces its random after every connection, and @@ -1363,6 +1373,7 @@ impl RdpServer { autodetect_rtt: Option>, autodetect_baseline_rtt: Option>, autodetect_bandwidth: Option>, + autodetect_bandwidth_generation: Option>, ) -> Self { let (ev_sender, ev_receiver) = ServerEvent::create_channel(); if let Some(cliprdr) = cliprdr_factory.as_mut() { @@ -1425,6 +1436,8 @@ impl RdpServer { handle.store(u32::MAX, Ordering::Relaxed); handle }, + autodetect_bandwidth_generation: autodetect_bandwidth_generation + .unwrap_or_else(|| Arc::new(AtomicU32::new(0))), auto_reconnect_cookie: None, previous_auto_reconnect_cookie: None, auto_reconnect_sent: false, @@ -1767,6 +1780,18 @@ impl RdpServer { Arc::clone(&self.autodetect_bandwidth) } + /// Returns a handle that increments every time a Bandwidth Measure + /// transaction completes, whether or not it produced a usable figure. + /// Pairs with [`Self::autodetect_bandwidth_handle`]: read this first to + /// detect a fresh measurement window (the bandwidth figure itself + /// repeats too often to be its own freshness signal), then read the + /// bandwidth handle for the value. Inject a shared instance at + /// construction with + /// [`RdpServerBuilder::with_autodetect_bandwidth_generation_handle`](crate::RdpServerBuilder::with_autodetect_bandwidth_generation_handle). + pub fn autodetect_bandwidth_generation_handle(&self) -> Arc { + Arc::clone(&self.autodetect_bandwidth_generation) + } + /// Returns the shared ECHO server handle for runtime probe requests and RTT measurements. pub fn echo_handle(&self) -> &EchoServerHandle { &self.echo_handle @@ -3793,6 +3818,7 @@ impl RdpServer { } AutoDetectOutcome::Bandwidth(Some(bandwidth_kbps)) => { self.autodetect_bandwidth.store(bandwidth_kbps, Ordering::Relaxed); + self.autodetect_bandwidth_generation.fetch_add(1, Ordering::Relaxed); // Logging the raw inputs, not just the computed figure: a // damage-driven video source makes any single measurement // window's byte count wildly bimodal (near-idle vs. a real @@ -3820,6 +3846,7 @@ impl RdpServer { // reporting a stale one (see `handle_response`'s doc comment); // mirror that here so the exposed handle does not disagree. self.autodetect_bandwidth.store(u32::MAX, Ordering::Relaxed); + self.autodetect_bandwidth_generation.fetch_add(1, Ordering::Relaxed); trace!( seq = pdu.response.sequence_number(), "Bandwidth measurement completed without a usable figure"