diff --git a/crates/ironrdp-server/src/builder.rs b/crates/ironrdp-server/src/builder.rs index e54e53a1fc..c61fc3c9d8 100644 --- a/crates/ironrdp-server/src/builder.rs +++ b/crates/ironrdp-server/src/builder.rs @@ -60,6 +60,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, @@ -173,6 +174,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, @@ -207,6 +209,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, @@ -418,6 +421,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. /// @@ -493,6 +508,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 f33f99bae3..f85c94d32b 100644 --- a/crates/ironrdp-server/src/server.rs +++ b/crates/ironrdp-server/src/server.rs @@ -743,6 +743,18 @@ 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": that + /// value repeats often (a quiet link reads the same low figure for + /// several consecutive windows), so diffing it is not a valid freshness + /// signal. Incremented with `Release` after the bandwidth value is + /// stored, so an `Acquire` load of it makes that value visible. + /// 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 @@ -1375,6 +1387,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() { @@ -1441,6 +1454,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, @@ -1783,6 +1798,22 @@ 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`]: load this with + /// `Ordering::Acquire` 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. The server + /// increments this with `Ordering::Release` after storing the value, so + /// the bandwidth read is at least as new as the generation observed. It + /// is not an exact pair: If the next window closes between the two reads, + /// the value can already belong to that later window. 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 @@ -3872,17 +3903,21 @@ impl RdpServer { } AutoDetectOutcome::Bandwidth(Some(bandwidth_kbps)) => { self.autodetect_bandwidth.store(bandwidth_kbps, Ordering::Relaxed); - debug!( - bandwidth_kbps, - seq = pdu.response.sequence_number(), - "Bandwidth measured" - ); + self.autodetect_bandwidth_generation.fetch_add(1, Ordering::Release); + // Logging the whole response, 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 the response's time_delta_ms/byte_count + // alongside it to show why. + debug!(bandwidth_kbps, response = ?pdu.response, "Bandwidth measured"); } AutoDetectOutcome::Bandwidth(None) => { // The manager just cleared its own figure rather than keep // 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::Release); trace!( seq = pdu.response.sequence_number(), "Bandwidth measurement completed without a usable figure" diff --git a/crates/ironrdp-testsuite-core/tests/server/autodetect.rs b/crates/ironrdp-testsuite-core/tests/server/autodetect.rs index 7a94d2e22e..8b1e6a6391 100644 --- a/crates/ironrdp-testsuite-core/tests/server/autodetect.rs +++ b/crates/ironrdp-testsuite-core/tests/server/autodetect.rs @@ -543,6 +543,58 @@ fn with_autodetect_bandwidth_handle_round_trips_the_same_arc() { assert_eq!(server.autodetect_bandwidth_handle().load(Ordering::Relaxed), 42); } +#[test] +fn autodetect_bandwidth_generation_handle_defaults_to_zero() { + use core::net::{Ipv4Addr, SocketAddr}; + use core::sync::atomic::Ordering; + + use ironrdp_server::RdpServer; + + let server = RdpServer::builder() + .with_addr(SocketAddr::from((Ipv4Addr::LOCALHOST, 0))) + .with_no_security() + .with_no_input() + .with_no_display() + .build(); + + assert_eq!( + server.autodetect_bandwidth_generation_handle().load(Ordering::Acquire), + 0 + ); +} + +#[test] +fn with_autodetect_bandwidth_generation_handle_round_trips_the_same_arc() { + use core::net::{Ipv4Addr, SocketAddr}; + use core::sync::atomic::{AtomicU32, Ordering}; + use std::sync::Arc; + + use ironrdp_server::RdpServer; + + let handle = Arc::new(AtomicU32::new(7)); + let server = RdpServer::builder() + .with_addr(SocketAddr::from((Ipv4Addr::LOCALHOST, 0))) + .with_no_security() + .with_no_input() + .with_no_display() + .with_autodetect_bandwidth_generation_handle(Arc::clone(&handle)) + .build(); + + assert!(Arc::ptr_eq(&handle, &server.autodetect_bandwidth_generation_handle())); + // Unlike the bandwidth value, an injected generation counter is not reset at + // construction: an embedder sharing one counter across servers keeps counting. + assert_eq!( + server.autodetect_bandwidth_generation_handle().load(Ordering::Acquire), + 7 + ); + // The Arc is shared: advancing the original is visible through the server's handle. + handle.fetch_add(1, Ordering::Release); + assert_eq!( + server.autodetect_bandwidth_generation_handle().load(Ordering::Acquire), + 8 + ); +} + #[test] fn stale_probe_expiry() { let mut mgr = AutoDetectManager::new();