Skip to content

fix(async): stitch AR packets across the DMA buffer boundary#45

Merged
mrmidi merged 2 commits into
mrmidi:mainfrom
mhellevang:pr/async-transport-fixes
Jul 6, 2026
Merged

fix(async): stitch AR packets across the DMA buffer boundary#45
mrmidi merged 2 commits into
mrmidi:mainfrom
mhellevang:pr/async-transport-fixes

Conversation

@mhellevang

Copy link
Copy Markdown
Contributor

Problem

The asynchronous receive request ring committed the whole 4160-byte DMA buffer per interrupt without stitching packets that straddle the buffer boundary. An inbound request spanning two AR buffers was silently destroyed, and its tail in the next buffer was misparsed as pseudo-packets — which the driver then answered with address-error responses to whatever node id the garbage decoded to. The AR response ring already stitched correctly; the asymmetry between the two rings was the defect.

Under sustained inbound traffic this is near-deterministic once ~4160 bytes have accumulated, and it corrupts the async stream for any device that pushes enough request traffic (observed wedging an SBP-2 target mid-transfer).

Fix

Introduce a shared ARStreamProcessor used by both AR rings:

  • commit only the bytes actually parsed (not the whole buffer),
  • stitch a fragment across the buffer boundary by re-presenting the head buffer once its successor has data,
  • drop an unparseable/garbage head loudly rather than replaying stale bytes — matching Linux firewire-ohci, which aborts an unparseable AR context instead of continuing.

ParseNext now returns a typed failure (NeedMoreBytes / UnknownTCode / ZeroGarbage / OversizedPayload) instead of silently swallowing a bad head, and oversized data_length is rejected (Linux MAX_ASYNC_PAYLOAD parity) so it can't masquerade as a fragment.

Also in this change:

  • set the AT-response split-timeout deadline in the descriptor timeStamp (cross-checked against Linux compute_split_timeout_timestamp, incl. the 16-bit wrap) so OHCI stops retiring otherwise-valid responses as evt_timeout;
  • minor BufferRing / bus-reset hardening.

Testing

  • New ARStreamProcessorTests (416 lines) cover fragment/stitch, cross-pass straddle, and garbage-head drop.
  • Full host suite green (1203 tests).
  • Verified on hardware end-to-end: a full high-resolution transfer that previously wedged at the buffer boundary now completes with 0 unclaimed / 0 dropped / 0 unparseable across ~42k stitched boundary crossings.

References

Cross-validated against Linux drivers/firewire/ohci.c (AR context abort, AR_WRAPAROUND_PAGES) and core-transaction.c (split-timeout timestamp).

The AR request ring routed and committed the whole 4160-byte buffer without
stitching packets that straddle the boundary. An inbound request spanning two
AR buffers was silently destroyed and its tail misparsed as pseudo-packets,
which the driver then answered with address-error responses. The response ring
already stitched correctly; the asymmetry was the defect.

Introduce a shared ARStreamProcessor used by both AR rings: commit only the
bytes actually parsed, stitch a fragment across the buffer boundary, and drop a
garbage head loudly (matching the Linux firewire-ohci behaviour of aborting an
unparseable context rather than replaying stale bytes). ParseNext now returns a
typed failure (NeedMoreBytes / UnknownTCode / ZeroGarbage / OversizedPayload)
instead of silently swallowing a bad head.

Also sets the AT-response split-timeout deadline in the descriptor timeStamp so
OHCI stops retiring valid responses as evt_timeout, plus BufferRing / bus-reset
hardening. Adds ARStreamProcessorTests.
@mrmidi mrmidi merged commit c19f8eb into mrmidi:main Jul 6, 2026
2 checks passed
@mhellevang mhellevang deleted the pr/async-transport-fixes branch July 6, 2026 17:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants