Repository navigation
Frame the TCP stream into whole USB/IP messages; run each on a per-endpoint lane; set TCP_NODELAY - #35
Open
Ytuf wants to merge 2 commits into
Open
Frame the TCP stream into whole USB/IP messages; run each on a per-endpoint lane; set TCP_NODELAY#35Ytuf wants to merge 2 commits into
Ytuf wants to merge 2 commits into
Conversation
…dpoint lane; set TCP_NODELAY The receive handler passed every TCP chunk to the request processor as if it were exactly one message. Two commands the client sent back-to-back arrived in one chunk, so the second was never decoded and its URB was never answered until the client timed out and unlinked it. hw_server posts a bulk IN a few microseconds after its bulk OUT, so every JTAG scan lost its read and later reads returned the accumulated FTDI FIFO, which Vivado decoded as a chain of bogus IDCODEs. USBIPMessageFramer buffers bytes and yields complete messages by phase: op_common (8 + body) before import, usbip_header_basic (48 + OUT payload + ISO descriptors) after. ConnectionLanes runs each message on a serial queue per device/endpoint/direction so writes to one pipe keep their order while a blocking read elsewhere holds nothing else up; unlinks get their own lane so they can cancel a transfer that is still blocking. The concurrent-request check now warns instead of silently dropping, and the listener sets TCP_NODELAY: with Nagle on, a reply waited behind the previous unacknowledged one for the client's delayed ACK, 40-100 ms per URB. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
swiftlint --strict (multiline_parameters), which CI runs, rejected the wrapped signature. No behaviour change. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SCK1o7poPFec8DnJd2GA3s
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Who did this
Full disclosure up front: the account submitting this PR (Ytuf) has not read the code. Ytuf was setting up USB/IP passthrough from a Mac to a Linux box for a Xilinx board, it did not work, and Claude Fable 5.1 (Anthropic's model, running in Claude Code) did the diagnosis, wrote the fix, built it, and wrote this PR. Ytuf ran the install commands with sudo and confirmed the board came up in Vivado afterwards. The commit is authored as Claude Fable 5.1 for that reason. Treat the PR as coming from the model, with Ytuf as the delivery method; review it on its merits.
The failure
Vivado 2019.1
hw_serveron Linux, Digilent Nexys A7-50T (FT2232H) attached to a Mac through usbipd-mac built frommainat 1c2ab45.hw_serverenumerated the cable, then the JTAG scan returned eitherNo devices detected on targetor a chain of bogus devices:The same board through usbipd-win worked. openFPGALoader through usbipd-mac worked. Not latency (tested with a keepalive that took the link to 4.6 ms), not the FTDI, not AppleUSBFTDI holding interface 1.
usbmon on the client showed the shape of it.
hw_serversubmits a bulk OUT and then the bulk IN for its reply 17 µs later. The OUT completed in 6 ms. The IN never completed and was unlinked by the client after 5 s. Every scan command did the same, and the replies stacked up in the FTDI FIFO until one read finally got through and returned 416 bytes of everything at once, which Vivado decoded as the chain above.A 100-line script using raw usbfs URBs, with no hw_server involved, reproduced it on demand:
A6 is the tell: the stuck requests complete the moment the next message arrives on the socket. Whichever of two back-to-back messages lands second in a TCP read is not processed until more bytes arrive.
The cause
ServerCoordinator'sonDataReceivedhanded every chunk fromNWConnection.receive(minimumIncompleteLength: 1, maximumLength: 65536)straight toRequestProcessor.processRequest, which decodes exactly one message from the front of the data. There was no framing: a second message in the same chunk was dropped, and a message split across chunks was corrupt. Any driver that keeps a read posted while it writes (FTDI MPSSE, CDC-ACM, most vendor protocols) loses requests at random depending on how TCP delivers them.Two smaller things sat next to it:
NWParameters.tcpleaves Nagle on, so a small reply waited behind the previous unacknowledged one until the client's delayed ACK (40–100 ms per transfer in the traces,A8/A9in the script); and the per-client "concurrent request limit" check silently dropped requests over the limit instead of processing them.The fix
USBIPMessageFramerbuffers bytes per connection and yields whole messages. Length comes from the header for the connection's phase: op_common plus its body (OP_REQ_DEVLIST8,OP_REQ_IMPORT40) before import;usbip_header_basic(48) plus the OUT transfer buffer plus ISO descriptors after. In the handshake phase it releases one message at a time, because the reply may switch framing; the pipeline re-frames what follows once that reply is out.ConnectionLanesgives each device/endpoint/direction a serial queue, so writes to one pipe keep their order (the processor blocks its thread until IOKit completes, so a shared concurrent queue could reorder them) while a blocking read on another pipe holds nothing else up. UNLINK gets its own lane so it can cancel a transfer that is still blocking. This keeps the per-endpoint guaranteeEndpointQueuesalready documents, one layer up.ConnectionReceivePipelineties the two together; the handler inServerCoordinatoris otherwise unchanged. The concurrency check now warns instead of dropping.TCPServersetsNWProtocolTCP.Options.noDelay = true. The Linux usbip tools setTCP_NODELAYon both ends for the same reason.Testing
Tests/USBIPDCoreTests/Protocol/USBIPMessageFramerTests.swift: coalesced messages, a message split across three chunks, IN with a largetransfer_buffer_lengthcarrying no payload, ISO descriptor accounting, the handshake-to-URB phase switch, unknown commands, lane keys, and a pipeline test that two OUTs on one lane complete in order while an IN on another lane is blocked. The machine this was written on has only the Command Line Tools, so XCTest could not run there. The same cases were compiled into a standalone harness against the framer source and all pass; CI should be the first XCTest run of that file.swift build -c releaseon macOS 27.0 / Swift 6.4, installed over the Homebrew binary, daemon restarted.open_hw_targetfrom the Linux host, twice:xc7a50t_0, IDCODE0x0362C093.Not addressed here
Found on the way, not in this PR: after a client detaches, the daemon keeps its
USBInterfaceOpenhandle on interface 0, so the nextbindsees its ownAppleUSBHostInterfaceUserClientas a foreign owner and withholds the interface until the daemon is restarted. And all transfers go throughinterfaceRefs[0](// using interface 0 as default for now), so endpoints on interface 1 of a composite device never work. Both reproducible on the FT2232H; happy to file issues.🤖 Generated with Claude Code