feat(server): measure bandwidth on the UDP tunnel when EGFX uses it by glamberson · Pull Request #2031 · Devolutions/IronRDP

Greg Lamberson (glamberson) · 2026-09-28T05:31:46Z

Summary

Once EGFX moves to the UDP transport, no large write crosses TCP any more, so the bracketed bandwidth measurement never runs again and the reported figure stays at its pre-migration value for the rest of the session.
Large EGFX batches on the tunnel are now bracketed the same way, with Bandwidth Measure Start and Stop (0x0014 / 0x0429) in RDP_TUNNEL_SUBHEADERs (MS-RDPBCGR 1.3.9, 2.2.14.1.2). Each sub-header is the request itself: Its SubHeaderLength and SubHeaderType are the request's headerLength and headerTypeId (MS-RDPEMT 2.2.1.1.1), so the request's first two bytes are not repeated in SubHeaderData. Each goes in a Tunnel Data PDU with no data of its own, so the client's count (only data after the tunnel header, 3.2.5.14) is exactly the graphics between them.
Bandwidth Measure Results the client returns in tunnel sub-headers are handled like the ones on the message channel; they are decoded from the whole sub-header, which is the response structure; the shared handling moved into one method.

Validation

cargo xtask check fmt/lints/tests/typos/locks all pass. Tests in ironrdp-testsuite-core cover the sub-header encoding of Start and results arriving in tunnel sub-headers, through a private __test feature on ironrdp-server, as well as the sub-header wire layout against the spec.

Notes

Stacked on #1954 (UDP multitransport wiring), #2021 (bracketed measurement) and #2030 (tunnel sub-headers); this PR's own changes are the commits from feat(server): measure bandwidth on the UDP tunnel when EGFX uses it onward. The #2021 commits in this branch are adapted to #1954's EGFX routing, where the bracket moves into the TCP write path.

github-actions · 2026-09-28T05:41:04Z

This pull request may overlap with #1954.

Substantially the same scope as #1954: RdpServerBuilder::with_udp_transport, server multitransport bootstrapping with accept_finalize_with_multitransport, and migrating EGFX onto the UDP tunnel via Soft-Sync. It also carries the same TunnelMessage sub-header transport work as #2030 and the bracketed bandwidth measurement as #2021, both also open candidates.

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).

AKolenda · 2026-09-28T06:16:49Z

For context, #2009 is the client side of this. It answers the RTT and bandwidth requests Windows sends in tunnel sub-headers, and it reads a sub-header as the auto-detect structure itself, the same way your latest commit does.

I ran your Start (06 00 07 00 14 00) through the #2009 responder and decoded its Results the way record_tunnel_sub_headers does, and they line up, so an IronRDP client and server should agree on the tunnel.

The two branches do conflict in ironrdp-rdpeudp-tokio (transport.rs, tunnel.rs, framed.rs), because #2009 adds its own way to push encoded PDUs through the write pump. TunnelMessage from #2030 is the cleaner way to do that, so if #2030 lands first I'll rebase #2009 onto it and drop the Outgoing enum.

Fixes three high-severity issues: Soft-Sync now requires both a negotiated SOFT_SYNC_TCP_TO_UDP flag and a successful Initiate Multitransport Response before migrating any channel, the shared UDP transport handle exposes a lock-free sender independent of its receive-side mutex, and the finalize handler no longer blocks the RDP handshake on the UDP accept, spawning it instead and picking it up opportunistically from client_loop's own select loop once it resolves. Fixes a medium-severity bug in ironrdp-dvc's Soft-Sync response handling: a declined channel stayed routed for outgoing data because the outgoing tunnel map was never filtered by the response, only the incoming one. Addresses four low-severity findings: corrects a false single-connection premise in the UDP accept doc comment, documents the AddrInUse tradeoff under session preemption, combines a duplicated drdynvc guard into one failure path, and confirms two findings already resolved by rebasing onto PR Devolutions#1953's own review-response commit.

Fixes a high-severity bug: after the sideband UDP tunnel closes, the shared transport handle now gets cleared so dispatch_egfx_messages actually falls back to TCP instead of silently dropping every subsequent EGFX batch onto a dead connection. Documents an accepted timing limitation: a late Initiate Multitransport Response arriving after finalization completes cannot retroactively enable Soft-Sync migration, since nothing on the message channel recognizes it post-handoff. This degrades to TCP-only for the session rather than causing any correctness issue. Inherits the S_OK/SOFTSYNC test fix from PR Devolutions#1953 by rebasing onto its review-response commit, reconciling the resulting connection.rs conflict between that PR's bool-returning rename and this branch's own earlier &mut self change for response tracking. Addresses three low-severity findings: removes an unused accessor, substitutes an equivalent enum match with the existing tls_acceptor() helper, and reuses get_svc_processor() instead of inlining its body.

The UDP handshake is started with spawn_local, so with_udp_transport makes run, run_connection and run_connection_with panic unless they are driven inside a tokio LocalSet. State that on with_udp_transport and on each entry point, and correct the code comment that said run already documented it.

A client that answers the Initiate Multitransport Request with E_ABORT has given up on the sideband transport (MS-RDPBCGR 2.2.15.2), yet the server kept its UDP accept running and its socket bound until the 15 s timeout, then logged a handshake timeout. Seen with a Windows client. Abort the pending accept once finalization reports the failure; with no response at all it keeps running, since the client may still connect.

Windows clients answer the Initiate Multitransport Request with E_ABORT about 2.7 s after connecting, after finalization has completed, so the earlier check never saw it and the accept still ran to its 15 s timeout. Now built on Devolutions#1964, which decodes that late response on the message channel: keep the pending accept's abort handle and stop the accept when the response reports failure, and report a cancelled accept as a decline rather than a panic.

…eached A socket bound to the unspecified address replies from whichever local address the routing table picks. On a host with several IPv6 addresses that was not the one mstsc sent to, so mstsc dropped the replies and gave up on the handshake with E_ABORT. When the configured UDP address is unspecified, bind to the connection's local address instead: run() records it, and embedders driving run_connection_with pass it through the new RdpServer::set_connection_local_addr.

mstsc finishes its UDP bootstrap after the TCP finalization, so its successful Initiate Multitransport Response arrives on the message channel once the client loop is running. Migration was decided once at finalization and never revisited, so the sideband transport came up and EGFX stayed on TCP for the whole session. Keep the decision on the connection instead: finalization sets it as before, and a later success enables it when Soft-Sync was negotiated.

An embedder that takes a frame handle from its GfxServerFactory has the channel registered as GfxDvcBridge, which the migration lookup did not recognise, so the Soft-Sync Request was never sent and EGFX stayed on TCP with the sideband transport up. Look the channel up as either type. Log the Soft-Sync Request, the client's response with the tunnels and channels it accepted, and the switch of EGFX onto UDP, which were silent.

The server now routes EGFX over the tunnel from the Soft-Sync Request on, since the request carries SOFT_SYNC_TCP_FLUSHED and MS-RDPEDYC 3.3.5.3.1 requires the server to keep using the named tunnel immediately after sending it. The Soft-Sync Response only narrows what the server reads (2.2.5.2), so it no longer changes the outgoing tunnel. Tunnel data that reaches the server before the Response is held and processed once it arrives (3.3.5.3.2) instead of being dropped, and DRDYNVC replies for a tunneled channel go over the tunnel whichever path produced them. The UDP accept is spawned with tokio::spawn, so the server no longer needs a LocalSet. UdpTransportHandle::send returns nothing, and the tunnel loop reads the transport handle once. Adds an end-to-end test that brings up the tunnel with a real client and checks all three routing rules; ironrdp-testsuite-extra now enables ironrdp-server's egfx feature for it.

The server's run future grew past clippy::large_futures' 16 KiB limit on Windows (16,440 bytes) with the DRDYNVC tunnel routing, failing the workspace lint there. Box it in the example rather than keep a future that size in main's stack frame.

Soft-Sync only moves channels onto a tunnel (MS-RDPEDYC 2.2.5.1 has no TCP tunnel type), its request promises no more of their data over TCP, and the tunnel lasts as long as the connection (MS-RDPEMT 1.3.3). Sending EGFX over TCP after the tunnel closed therefore reached a client with nowhere to read it: mstsc froze and reset the connection about 19 s later. With EGFX on the tunnel, the connection now ends when the tunnel does. With nothing moved, the session stays on TCP as before.

The doc on dispatch_egfx_messages called a repeat request a no-op under request_reliable_udp's idempotency guard, but the guard returns an error that the caller only traces. Both docs now say the request is made once per connection, that the state never returns to idle, and that the request declares the TCP path flushed for the channel (SOFT_SYNC_TCP_FLUSHED, MS-RDPEDYC 2.2.5.1) so its data goes over the tunnel from then on (3.3.5.3.1), whatever the client's response lists.

The continuous measurement opened a fixed window of RTT ticks, and the client counts only the traffic that happens to pass between Start and Stop, so a window over a quiet desktop timed idle time and reported a fast link as a few hundred kbps. Bracket one EGFX write of at least 10 KiB with Start and Stop instead, at most once a second, so the client times a burst the link carried. Sessions without EGFX keep the tick window until the first bracketed measurement. A zero time delta now counts as one millisecond: the client's timer has millisecond resolution and a burst on a fast link completes within one. A result with no bytes counted still clears the stored figure.

The first bracketed measurement no longer turns the tick window off for good: each bracketed Start resets the tick count, so the window takes over again once eight ticks pass without one. A bracketed measurement, or one waiting for the client's results, is now dropped by expire_stale_probes once it is older than the RTT probe age, so a Stop that is never sent or a client that never answers no longer stops measurement for the rest of the session. A tick window that is still open is not timed, since it sends its own Stop. Also folds the Start and Stop writes into let chains, passes the measured fields to measured_bandwidth_kbps, and allocates sequence numbers through one helper.

The RDPEMT codec already handled sub-headers, but UdpTransport exposed only a PDU's data, so the Continuous Auto-Detection messages MS-RDPBCGR 1.3.9 carries there could be neither sent nor received. TunnelMessage carries both; send_message and recv_message use it, and send/recv keep working on data alone.

The sub-header examples carried a whole Bandwidth Measure Start, header bytes included. An auto-detect sub-header is the auto-detect structure itself (MS-RDPEMT 2.2.1.1.1): its SubHeaderLength and SubHeaderType are the structure's headerLength and headerTypeId, so on the wire those examples would repeat the header. The transport treats the data as opaque and the tests passed either way; the examples now carry only what follows the header.

…ment Nothing builds a default TunnelMessage, and a default one would be an empty Tunnel Data PDU. The FramedRead comment said the tunnel consumes the sub-headers before this point; recv drops them, and a caller that needs them uses recv_message.

The send_message precheck restates the HeaderLength bound that ironrdp-rdpemt's TunnelData encoder owns. The boundary test now also runs the largest accepted message and the smallest refused one through the real encoder, so the two cannot drift apart unnoticed, and the comment on the check cites MS-RDPEMT 2.2.1.1.

Once EGFX moves to the UDP transport no large write crosses TCP, so the bracketed measurement never runs again and the bandwidth figure stays at its pre-migration value. Bracket large EGFX writes on the tunnel instead, with Start and Stop in auto-detect sub-headers, and take the client's results from the sub-headers it returns.

AUTODETECT_HEADER_SIZE was added between autodetect_sub_header's doc comment and the function, so the constant carried both doc comments and the function had none. Each doc comment is back above its own item. No code change.

ironrdp-server builds with `[lib] test = false`, so its inline tests never ran. The three covering tunnel auto-detect move to ironrdp-testsuite-core, which reaches the internals they need through a new private `__test` feature, as ironrdp-rdpsnd and ironrdp-session do. Also points the tick-path comment at the functions that now bracket writes.

A measurement only goes out on the tunnel once EGFX is on it, and the tunnel closing then ends the connection (Devolutions#1954), so the cancel could no longer run. Removes it with AutoDetectManager::cancel_bandwidth_measure and its tests.

github-actions

The PR wires Continuous Auto-Detection bandwidth measurement onto the UDP tunnel (bracketed Start/Stop as RDP_TUNNEL_SUBHEADERs, results decoded from tunnel sub-headers), migrates EGFX to the tunnel via Soft-Sync with early-payload holdback, and adds server-side UDP multitransport bootstrapping. Independent inspection confirmed the protocol mechanics are spec-conformant and well-tested (sub-header wire layout, encoder-bound precheck, Soft-Sync gating, tunnel-loss and decline-abort handling). Remaining issues are maintainability/architectural: a duplicated bandwidth formula that orphans a divergent public pdu-crate method, unconditional RDPEMT/RDPEUDP dependencies on ironrdp-server for a runtime-opt-in feature, and a redundant Option<u16> in the TCP bracket path. Because the diff is the entire stacked branch with no per-commit boundary, hunk attribution to this PR is uncertain, so line ranges are withheld and one scope question stands.

[skeptical + code-compressor] Server re-implements the pdu crate's bandwidth formula with divergent zero-timeDelta semantics — medium 🟠 — crates/ironrdp-server/src/autodetect.rs
measured_bandwidth_kbps (autodetect.rs:485) duplicates AutoDetectResponse::computed_bandwidth_kbps (ironrdp-pdu/src/rdp/autodetect.rs:613) but returns a clamped figure for zero timeDelta where the pdu method returns None; the pdu method remains public and now has no non-test callers. One wire value thus has two in-repo computations with different edge-case results and nothing prevents drift. Fold the new clamp semantics into the pdu method (noting the API-visible change) or delete the orphaned method instead.
[skeptical] RDPEMT/RDPEUDP stack added as unconditional ironrdp-server dependencies — medium 🟠 — crates/ironrdp-server/Cargo.toml
ironrdp-rdpemt, ironrdp-rdpeudp and ironrdp-rdpeudp-tokio are non-optional public dependencies, so every consumer of the server skeleton compiles the whole UDP multitransport stack even though the feature is runtime opt-in (udp_bind_addr defaults to None; nothing changes without with_udp_transport). The crate's own convention gates optional integrations behind features (egfx, usb) with explicit build-cost rationale, and the code already tolerates cfg-style plumbing (dispatch_egfx_messages is cfg'd on egfx), so a 'udp' feature appears feasible.
[skeptical] Full stacked-branch diff prevents attributing public API additions to this PR — low 🟡 ❓ — crates/ironrdp-server/src/builder.rs
The PR body states only the commits from 'feat(server): measure bandwidth on the UDP tunnel when EGFX uses it' onward are this PR's own, but the diff covers the whole stacked branch (#1954, #2021, #2030 plus this PR) with no per-commit boundary. Cross-cutting additions - Builder::with_udp_transport, RdpServer::set_connection_local_addr, AcceptorResult::multitransport_response_success, UdpTransportSender/TunnelMessage/SubHeadersTooLarge - therefore cannot be attributed to this PR, so their necessity and scope cannot be concluded from the provided evidence.
[code-compressor] Option<u16> bracket tracking only re-stores an in-scope parameter; a bool is flatter — low 🟡 — crates/ironrdp-server/src/server.rs
In write_egfx_over_tcp, bracket_channel_id = Some(message_channel_id) then re-unwraps it for the Stop, but the stored value always equals the method's own message_channel_id parameter. The sibling write_egfx_over_udp models the same bracket with a plain bool; using bracketed: bool (or a linear Start/data/Stop block with early return) removes the carried state with no behavior change since begin_bandwidth_measure is still called exactly once per batch. Pure clarity win.

Greg Lamberson (glamberson) deployed to llm-providers September 28, 2026 05:32 — with GitHub Actions Active

Greg Lamberson (glamberson) deployed to llm-providers September 28, 2026 06:03 — with GitHub Actions Active

Greg Lamberson (glamberson) mentioned this pull request Sep 28, 2026

fix(client): answer auto-detect requests on the UDP tunnel #2009

Open

github-actions Bot added risk/medium Behavioral change that does not substantially alter a core public API and removed risk/high Substantial core public API impact, or fail-closed triage; needs maintainer-level scrutiny labels Sep 28, 2026

Greg Lamberson (glamberson) deployed to llm-providers September 28, 2026 06:30 — with GitHub Actions Active

github-actions Bot added breaking-change Includes a breaking change, and requires special scrutiny at the boundaries needs-review A human reviewer is the current next actor labels Sep 28, 2026

Greg Lamberson (glamberson) force-pushed the feat/server-tunnel-bandwidth branch from 20f4ae7 to 6a57c9b Compare September 28, 2026 11:04

Greg Lamberson (glamberson) deployed to llm-providers September 28, 2026 11:05 — with GitHub Actions Active

github-actions Bot mentioned this pull request Sep 28, 2026

feat(server): wire UDP multitransport into ironrdp-server #1954

Merged

Greg Lamberson (glamberson) mentioned this pull request Sep 28, 2026

feat(rdpeudp): carry tunnel sub-headers through the UDP transport #2030

Open

github-actions Bot mentioned this pull request Sep 28, 2026

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

Open

Greg Lamberson (glamberson) force-pushed the feat/server-tunnel-bandwidth branch from 6a57c9b to 29d1ad7 Compare September 28, 2026 19:26

Greg Lamberson (glamberson) deployed to llm-providers September 28, 2026 19:26 — with GitHub Actions Active

Greg Lamberson (glamberson) added 12 commits September 29, 2026 08:18

fix(server): pass no UDP transport in the clipboard dispatch test

4552d9c

Greg Lamberson (glamberson) force-pushed the feat/server-tunnel-bandwidth branch from 6245e1b to b5dd80f Compare September 29, 2026 13:32

Greg Lamberson (glamberson) deployed to llm-providers September 29, 2026 13:33 — with GitHub Actions Active

Greg Lamberson (glamberson) added 12 commits September 29, 2026 18:19

fix(server): send tunnel auto-detect as the sub-header itself

2c6d861

fix(server): put the sub-header docs back on their items

2274e13

AUTODETECT_HEADER_SIZE was added between autodetect_sub_header's doc comment and the function, so the constant carried both doc comments and the function had none. Each doc comment is back above its own item. No code change.

Greg Lamberson (glamberson) force-pushed the feat/server-tunnel-bandwidth branch from b5dd80f to d04020e Compare September 29, 2026 23:28

Greg Lamberson (glamberson) deployed to llm-providers September 29, 2026 23:29 — with GitHub Actions Active

github-actions Bot reviewed Sep 30, 2026

View reviewed changes

github-actions Bot added the ai-reviewed/1 One automated review completed label Sep 30, 2026

Navigation Menu

feat(server): measure bandwidth on the UDP tunnel when EGFX uses it - #2031

feat(server): measure bandwidth on the UDP tunnel when EGFX uses it#2031
Greg Lamberson (glamberson) wants to merge 25 commits into
Devolutions:masterfrom
lamco-admin:feat/server-tunnel-bandwidth

Greg Lamberson (glamberson) commented Sep 28, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Sep 28, 2026 •

edited

Loading

Uh oh!

AKolenda commented Sep 28, 2026

Uh oh!

github-actions Bot left a comment

Uh oh!

This branch was successfully deployed

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

Conversation

Greg Lamberson (glamberson) commented Sep 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Validation

Notes

Uh oh!

github-actions Bot commented Sep 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AKolenda commented Sep 28, 2026

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

This branch was successfully deployed

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

Greg Lamberson (glamberson) commented Sep 28, 2026 •

edited

Loading

github-actions Bot commented Sep 28, 2026 •

edited

Loading