Skip to content

feat(usb)!: support SuperSpeed packet sizes - #1883

Merged
Benoît Cortier (CBenoit) merged 5 commits into
Devolutions:masterfrom
uchouT:usb-superspeed
Sep 7, 2026
Merged

Benoît Cortier (CBenoit) merged 5 commits into
Devolutions:masterfrom
uchouT:usb-superspeed

Conversation

@uchouT

@uchouT uchouT (uchouT) commented Sep 2, 2026 •

Copy link
Copy Markdown
Contributor

ironrdp-usb could not describe a SuperSpeed device: is_valid_for_usb2 rejected every SuperSpeed endpoint, and endpoint_zero_max_packet_size required a bus speed to read bMaxPacketSize0.

That speed parameter added nothing. USB 3.2 Section 9.6.1 admits only 9 there, as a base-2 exponent, while USB 2.0 admits only 8, 16, 32 and 64, stated literally, so the two encodings are disjoint and the value identifies itself. Requiring a speed only let a caller contradict the descriptor, which is exactly what a transport that cannot express SuperSpeed ends up doing.

Endpoint zero now decodes from the value alone, and the endpoint predicate carries the USB 3.2 Section 9.6.6 limits, so is_valid_for_usb2 and usb2_max_payload lose their USB 2.0 names.

Marc-André Lureau (@elmarco) has helped test with a USB 3 device from FreeRDP.

`endpoint_zero_max_packet_size` took the negotiated speed and used it to
pick between the two encodings USB defines for `bMaxPacketSize0`: the
literal byte count of USB 2.0, and the base-2 exponent SuperSpeed uses
because 512 does not fit in the field.

Signed-off-by: uchouT <i@uchout.moe>
`is_valid_for_usb2` and `usb2_max_payload` were scoped to USB 2.0 and
answered `false` and `None` for SuperSpeed, deferring its endpoint
semantics. A consumer holding a SuperSpeed configuration descriptor
therefore had no way to validate an endpoint or size its payload.

Signed-off-by: uchouT <i@uchout.moe>
@uchouT
uchouT (uchouT) marked this pull request as ready for review September 4, 2026 02:37
Copilot AI balanced review requested due to automatic review settings September 4, 2026 02:37

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions github-actions Bot added breaking-change Includes a breaking change, and requires special scrutiny at the boundaries kind/protocol Affects RDP or related protocol behavior needs-review A human reviewer is the current next actor risk/high Substantial core public API impact, or fail-closed triage; needs maintainer-level scrutiny size/M Size: up to 449 counted lines and 10 files; exceeds S in either measure labels Sep 4, 2026

@CBenoit Benoît Cortier (CBenoit) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for looking into this! I don’t see any issue with this code, I’ll just see what the reviewer pipeline says about the small details 🙂

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

It drops required USB 2.0 speed validation and gives incomplete SuperSpeed payload semantics.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 7/7 changed files
  • Comments generated: 4
  • Review effort level: Balanced

Comment thread crates/ironrdp-usb/src/descriptor/device.rs
Comment thread crates/ironrdp-usb/src/endpoint.rs Outdated
Comment thread crates/ironrdp-usb/src/control.rs
Comment thread crates/ironrdp-usb/src/endpoint.rs
Signed-off-by: uchouT <i@uchout.moe>
Signed-off-by: uchouT <i@uchout.moe>
@github-actions github-actions Bot added risk/unknown Risk could not be determined automatically; needs maintainer-level scrutiny and removed risk/high Substantial core public API impact, or fail-closed triage; needs maintainer-level scrutiny labels Sep 5, 2026

@CBenoit Benoît Cortier (CBenoit) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@CBenoit
Benoît Cortier (CBenoit) merged commit 8fd0b17 into Devolutions:master Sep 7, 2026
41 checks passed
@uchouT
uchouT (uchouT) deleted the usb-superspeed branch September 8, 2026 05:06
Anton Mostovoy (antonmos) added a commit to antonmos/IronRDP that referenced this pull request Sep 8, 2026
…nnection-negotiation

* upstream/master: (124 commits)
  feat(server): let an authenticated connection preempt an existing session (Devolutions#1476)
  ci(pr-automation): recover reviewer stages (Devolutions#1922)
  ci(pr-automation): report review check diagnostics (Devolutions#1926)
  ci(pr-automation): recover agent runtime (Devolutions#1921)
  ci(pr-automation): report reviewer recovery (Devolutions#1920)
  docs(pr-automation): define review recovery intent (Devolutions#1915)
  feat(web): expose enable_server_pointer from the WASM module (Devolutions#1914)
  fix(web): restore default remote cursor rendering (Devolutions#1910)
  feat(usb)!: support SuperSpeed packet sizes (Devolutions#1883)
  ci(pr-automation): explain reviewer ineligibility (Devolutions#1906)
  ci(pr-automation): explain skipped reviews (Devolutions#1907)
  ci(pr-automation): request JSON mode (Devolutions#1908)
  ci(pr-automation): repair empty agent responses (Devolutions#1905)
  ci(pr-automation): diagnose invalid repairs (Devolutions#1904)
  ci(pr-automation): simplify review findings (Devolutions#1901)
  fix(server): bound accept_finalize so a wedged client cannot hold the server (Devolutions#1890)
  ci(pr-automation): improve reviewer diagnostics (Devolutions#1898)
  ci(pr-automation): redispatch explicit retries (Devolutions#1897)
  test: focus PR automation workflow coverage (Devolutions#1896)
  feat(agent): add clipboard image support (Devolutions#1877)
  ...

# Conflicts:
#	crates/ironrdp-server/src/server.rs

This branch was previously deployed

1 inactive deployment
llm-providers — deee4fef Deployed Sep 5, 2026 by uchouT via Classify pull request #5098
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking-change Includes a breaking change, and requires special scrutiny at the boundaries kind/protocol Affects RDP or related protocol behavior needs-review A human reviewer is the current next actor risk/unknown Risk could not be determined automatically; needs maintainer-level scrutiny size/M Size: up to 449 counted lines and 10 files; exceeds S in either measure

Development

Successfully merging this pull request may close these issues.

3 participants