Skip to content

Fix connection byte offsets and Ethernet chunk accounting - #936

Draft
Changho Hwang (chhwang) wants to merge 2 commits into
mainfrom
fix/connection-byte-offset-and-ethernet-chunks-20261023
Draft

Changho Hwang (chhwang) wants to merge 2 commits into
mainfrom
fix/connection-byte-offset-and-ethernet-chunks-20261023

Conversation

@chhwang

@chhwang Changho Hwang (chhwang) commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Fix CudaIpcConnection::updateAndSync applying its byte offset twice; the copy and log now use the already offset destination pointer.
  • Fix EthernetConnection::write counting the initial frame header as payload progress, which skipped bytes on chunked writes.
  • Integrate the regression checks into existing CommunicatorTest.BasicWrite and PortChannelOneToOneTest.PingPongEthernet. No standalone tests were added, and performance tests and production staging-buffer defaults are unchanged.

Regression coverage

  • BasicWrite exercises nonzero byte offsets only on existing CudaIpc connections; IB connections skip that per-edge phase while all ranks keep the same barriers. Per-sender slots and a guard at the former 9 * dstOffset target are checked. A uniform capacity guard skips only this added phase if its largest guard address will not fit the fixture buffer.
  • PingPongEthernet retains its original ping-pong exercise, then uses the existing bootstrap and memory-pair helpers with a 64-byte staging buffer to verify a 128-byte chunked write followed by a 16-byte frame. It follows the existing two-participant fixture; additional world ranks take the fixture's existing nonparticipant path.

Validation

  • Bounded CPU source-extract checks with fake GPU/socket boundaries passed under ASan/UBSan for CudaIpc offset 8 and Ethernet payloads of 48, 49, 128, and 16 bytes across four frames. These are not full-library or real-transport tests.
  • clang-format --dry-run --Werror and git diff --check passed.
  • The CUDA/MPI integration tests were not compiled or run: CMake, nvcc, mpicxx, and mpirun are unavailable. No GPU, MPI, socket, or real transport execution was performed.
  • No benchmark was run. The fixes add no per-chunk operations, locks, allocations, or synchronization; large transfers may require extra sends to carry bytes the old path omitted. No measured no-regression claim is made.

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

This branch has not been deployed

No deployments
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.

1 participant