Repository navigation
net_ib_cast: port net_ib error-path fixes (AICOMRCCL-2750) - #14015
Merged
nikitaxgusev merged 5 commits intoOct 28, 2026
Merged
nikitaxgusev merged 5 commits into
nikitaxgusev merged 5 commits into
Conversation
✅ All Policy Checks Passed
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
Static review found no blocking issues in the targeted backports or regression-test integration; runtime validation was not performed.
No open issues 🎉
What changed in this PR
Backports four existing net_ib error-path fixes into net_ib_cast, keeping the renamed transport aligned where upstream fixes require manual porting.
Changes:
- Corrects error-completion device indexing, adds the IGet request name, and fixes the receiver request-ID boundary.
- Updates GIN allgather to poll completions while sends wait for CTS.
- Adds host-only regression helpers and coverage to
rccl-UnitTestsMPI.
| File | Description |
|---|---|
projects/rccl/test/transport/NetIbMPI/GinProxyErrorPathTests.cpp |
Adds regression checks for all four fixes. |
projects/rccl/test/CMakeLists.txt |
Includes the new test in the MPI test target. |
projects/rccl/src/transport/net_ib_cast/p2p.cc |
Adds the IGet name and lookup helper. |
projects/rccl/src/transport/net_ib_cast/p2p_resiliency.cc |
Fixes the receiver boundary check and adds a regression helper. |
projects/rccl/src/transport/net_ib_cast/net_ib_cast_inspect.h |
Declares the host-only regression helpers. |
projects/rccl/src/transport/net_ib_cast/gin.cc |
Fixes allgather progress and error logging; adds regression helpers. |
Review effort: Balanced
![amd-argus[bot]](https://avatars.githubusercontent.com/in/4740020?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
ARGUS
Findings
5 more finding(s), not posted inline
projects/rccl/src/transport/net_ib_cast/p2p_resiliency.cc:1279(Low) ncclIbCastTestRecvWrIdBound passes a stack ncclIbNetCommBase, so a reverted bound reads recvReqs[256] past the object (UB)projects/rccl/src/transport/net_ib_cast/gin.cc:898(Low) RmaProxy seam uses an InfiniBand link layer, so the remDevs[0] half of the fix (gin.cc:768) is never executedprojects/rccl/src/transport/net_ib_cast/net_ib_cast_inspect.h:141(Nit) Comment says NULL if the IGet slot is missing, but a missing slot is an out-of-bounds readprojects/rccl/src/transport/net_ib_cast/gin.cc:977(Nit) ncclIbCastTestGinAllGatherPollsRecv swaps the live global netIbCast op table, unguarded, in release librcclprojects/rccl/src/transport/net_ib_cast/gin.cc:988(Nit) Five per-op save/restore pairs of netIbCast could be one struct copy (ncclNet_t savedNet = netIbCast)
Bottom line: Worth settling before approval: NetIbCast.CastGinErrorPath is not selected by any CI category or test_runner config, so the new regression checks never run. The four ported fixes match net_ib line for line.
Comment @argus review to re-run, or @argus re-review after pushing.
i-kosarev
approved these changes
Oct 22, 2026
Contributor
Author
|
@argus re-review |
abouteiller
approved these changes
Oct 23, 2026
nikitaxgusev
force-pushed
the
users/nikitaxgusev/aicomrccl-2750-net-ib-cast-error-paths
branch
from
October 26, 2026 10:17
7ebb5bf to
fda5bd1
Compare
Contributor
pr-rocm-systems-14015-oci-rccl-pr-smokeBuilds
FailuresNo failing tests in this run. 15 tests skipped. |
Contributor
pr-rocm-systems-14015-ruby64-rccl-pr-buildBuilds
FailuresNo failing tests in this run. 183 tests skipped. pr-rocm-systems-14015-ruby64-rccl-pr-smokeBuilds
Failures
15 tests skipped. |
nikitaxgusev
force-pushed
the
users/nikitaxgusev/aicomrccl-2750-net-ib-cast-error-paths
branch
from
October 26, 2026 11:53
fda5bd1 to
7ad5b68
Compare
Contributor
pr-rocm-systems-14015-oci-rccl-pr-buildBuilds
FailuresNo failing tests in this run. 183 tests skipped. |
Contributor
pr-rocm-systems-14015-ruby64-rccl-pr-buildBuilds
FailuresNo failing tests in this run. 183 tests skipped. |
nikitaxgusev
force-pushed
the
users/nikitaxgusev/aicomrccl-2750-net-ib-cast-error-paths
branch
from
October 27, 2026 08:12
7ad5b68 to
233307d
Compare
net_ib_cast is a renamed copy of net_ib, so the NCCL 2.32.3 sync never merged these upstream fixes. Bring over the completion-index warning, the IGet request name, the receiver wr_id bound, and the GIN allgather poll so a failed IB-cast GIN operation returns an error instead of segfaulting or hanging.
Call the ported net_ib error paths with no NIC. An error completion after a success returns ncclRemoteError instead of indexing devBases by the completion slot (gin.cc IbCastRmaIbProxyTest), IGet is named in IbCastReqTypeStr, wr_id == NET_IB_MAX_REQUESTS is rejected (p2p_resiliency.cc), and GIN allgather polls the receive while isend is still waiting for CTS.
- p2p.cc: add the 7th "Failed" IbCastReqTypeStr entry so the array matches net_ib 1:1 (net_ib has 7; cast had 6). - gin.cc: save/restore the whole netIbCast ops table in one struct copy instead of five field pairs in the host-only allgather seam. - net_ib_cast_inspect.h: correct the IGet-name comment (a short array reads out of bounds, it does not return NULL). - mi455_ainic_roce_func.json: add a num_ranks:1 host-only entry so NetIbCast.CastGinErrorPath is selected and actually runs. JIRA ID : AICOMRCCL-2750
nikitaxgusev
force-pushed
the
users/nikitaxgusev/aicomrccl-2750-net-ib-cast-error-paths
branch
from
October 27, 2026 08:14
233307d to
9f0891d
Compare
The error-path port extended IbCastReqTypeStr in
net_ib_cast/p2p.cc from 5 to 7 entries ("IGet", "Failed", matching
net_ib 1:1). The host-test drift guard pins the old line, so
"Configure host tests" fails with "p2p.cc no longer contains". Update
the pinned lines in test/host/CMakeLists.txt and the matching copy in
fakes/net_ib_cast_p2p_fakes.cc so all three agree.
JIRA ID : AICOMRCCL-2750
Contributor
pr-rocm-systems-14015-oci-rccl-pr-smokeBuilds
FailuresNo failing tests in this run. 15 tests skipped. |
Contributor
pr-rocm-systems-14015-ruby64-rccl-pr-smokeBuilds
FailuresNo failing tests in this run. 15 tests skipped. |
Fix 1 (AICOMRCCL-2750) makes IbCastRmaIbProxyTest index devBases[] by the request's own device instead of the completion's slot, so an error in a later completion slot of a one-device comm is now reported as a remote error instead of crashing. Update the net-ib-cast-gin host microtest that pinned the former crash (AICOMRCCL-3095) to assert the clean behavior, matching net_ib. JIRA ID : AICOMRCCL-2750
Contributor
pr-rocm-systems-14015-oci-rccl-pr-smokeBuilds
FailuresNo failing tests in this run. 15 tests skipped. |
Contributor
pr-rocm-systems-14015-ruby64-rccl-pr-smokeBuilds
FailuresNo failing tests in this run. 15 tests skipped. |
nikitaxgusev
deleted the
users/nikitaxgusev/aicomrccl-2750-net-ib-cast-error-paths
branch
October 28, 2026 09:21
ywang103-amd
pushed a commit
that referenced
this pull request
Oct 28, 2026
## Motivation `net_ib_cast` is a renamed copy of `net_ib` (`ncclIb*` to `IbCast*`), so NCCL syncs never merge upstream `net_ib` fixes into it; they only arrive through explicit backports. A few error-path fixes that already live in RCCL `net_ib` are still missing in `net_ib_cast`. They only matter when an IB-cast GIN proxy operation or connection fails (AINIC, or `NCCL_NET=IB-CAST` / `ROCM-IB`). Normal operation, correctness, and performance are unaffected. ## Technical Details Ports four upstream `net_ib` error-path fixes into `net_ib_cast`, matching RCCL `net_ib` on `develop` one to one (only `ncclIb*` to `IbCast*` renames, no behavioral divergence): | # | Upstream NCCL fix | `net_ib` on develop | `net_ib_cast` change | |---|---|---|---| | 1 | `b4a53400` gin/proxy latent bugs | `gin.cc` `ncclRmaIbProxyTest` uses `devBase->gidInfo.link_layer`, `remDevs[0]` | `gin.cc` `IbCastRmaIbProxyTest`: `devBases[i]` to `devBase`, `remDevs[i]` to `remDevs[0]` (error log used the completion index as a device index; a non-first error CQE segfaulted) | | 2 | `b4a53400` | `p2p.cc` `ncclIbReqTypeStr` contains `"IGet"` | `p2p.cc` `IbCastReqTypeStr`: add `"IGet"` (IGet failure read past the array) | | 3 | `8b967feb` (NCCL 2.30.7) | `p2p_resiliency.cc` `wr_id < NET_IB_MAX_REQUESTS` | `p2p_resiliency.cc`: `wr_id <= NET_IB_MAX_REQUESTS` to `wr_id <` (off-by-one that would index `recvReqs[]` out of bounds) | | 4 | `799ca467` gin: poll completions while waiting for CTS | `gin.cc` `ncclGinIbAllGather` single post-and-poll loop | `gin.cc` `IbCastGinIbAllGather`: same single post-and-poll loop (a lost CTS no longer hangs GIN context creation / window registration) | Not ported on purpose: `net_ib`'s `ginSeq`/batch and `printIbWcStatusHint` are `net_ib` architecture, outside these fixes. Performance commits (`b736405d`, `ccdb45e8`) and the cosmetic `qp_num` warning are tracked separately. ## Issue Tracking JIRA ID : AICOMRCCL-2750 ## Test Plan Added `NetIbCast.CastGinErrorPath` in `test/transport/NetIbMPI/GinProxyErrorPathTests.cpp`, a whitebox host-only test (no NIC, no MPI traffic) that drives the four ported paths through test-only seams: the IGet name lookup, an error completion that is not first in the poll batch (with `devBases[1]` left NULL), `wr_id == NET_IB_MAX_REQUESTS`, and the GIN allgather polling the receive while `isend` still waits for CTS. Builds into `rccl-UnitTestsMPI`. A standalone logic harness (`g++` with ASan/UBSan) reproduces the exact control flow of all four functions in both fixed and buggy form to confirm the test discriminates the fix. ## Test Result Standalone harness, fixed vs buggy, trimmed: ``` == Fix 1: completion-index warning == ok: rmaProxyTest(fixed) == ncclRemoteError (buggy: NULL devBases[1] -> segfault) == Fix 2: IGet name == ok: ReqTypeStr[GIN_IGET] == "IGet" (buggy: index 5 past 5-entry array) == Fix 3: receiver wr_id bound == ok: wr_id==NET_IB_MAX_REQUESTS -> ncclInternalError (buggy: recvReqs[256] OOB) == Fix 4: allgather polls recv before send == ok: recvPolledBeforeSend == 1 (buggy: 0, lost CTS hangs) ALL CHECKS PASSED (0 failures) ``` `NetIbCast.CastGinErrorPath` builds into `rccl-UnitTestsMPI`; a full on-node run can follow on an MI350X node. ## Submission Checklist - [ ] Look over the contributing guidelines at https://github.com/ROCm/rocm-systems/blob/develop/CONTRIBUTING.md.
Contributor
TheRock Submodule Bump ActivityNewest first
|
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.
Motivation
net_ib_castis a renamed copy ofnet_ib(ncclIb*toIbCast*), so NCCL syncs never merge upstreamnet_ibfixes into it; they only arrive through explicit backports. A few error-path fixes that already live in RCCLnet_ibare still missing innet_ib_cast. They only matter when an IB-cast GIN proxy operation or connection fails (AINIC, orNCCL_NET=IB-CAST/ROCM-IB). Normal operation, correctness, and performance are unaffected.Technical Details
Ports four upstream
net_iberror-path fixes intonet_ib_cast, matching RCCLnet_ibondevelopone to one (onlyncclIb*toIbCast*renames, no behavioral divergence):net_ibon developnet_ib_castchangeb4a53400gin/proxy latent bugsgin.ccncclRmaIbProxyTestusesdevBase->gidInfo.link_layer,remDevs[0]gin.ccIbCastRmaIbProxyTest:devBases[i]todevBase,remDevs[i]toremDevs[0](error log used the completion index as a device index; a non-first error CQE segfaulted)b4a53400p2p.ccncclIbReqTypeStrcontains"IGet"p2p.ccIbCastReqTypeStr: add"IGet"(IGet failure read past the array)8b967feb(NCCL 2.30.7)p2p_resiliency.ccwr_id < NET_IB_MAX_REQUESTSp2p_resiliency.cc:wr_id <= NET_IB_MAX_REQUESTStowr_id <(off-by-one that would indexrecvReqs[]out of bounds)799ca467gin: poll completions while waiting for CTSgin.ccncclGinIbAllGathersingle post-and-poll loopgin.ccIbCastGinIbAllGather: same single post-and-poll loop (a lost CTS no longer hangs GIN context creation / window registration)Not ported on purpose:
net_ib'sginSeq/batch andprintIbWcStatusHintarenet_ibarchitecture, outside these fixes. Performance commits (b736405d,ccdb45e8) and the cosmeticqp_numwarning are tracked separately.Issue Tracking
JIRA ID : AICOMRCCL-2750
Test Plan
Added
NetIbCast.CastGinErrorPathintest/transport/NetIbMPI/GinProxyErrorPathTests.cpp, a whitebox host-only test (no NIC, no MPI traffic) that drives the four ported paths through test-only seams: the IGet name lookup, an error completion that is not first in the poll batch (withdevBases[1]left NULL),wr_id == NET_IB_MAX_REQUESTS, and the GIN allgather polling the receive whileisendstill waits for CTS. Builds intorccl-UnitTestsMPI.A standalone logic harness (
g++with ASan/UBSan) reproduces the exact control flow of all four functions in both fixed and buggy form to confirm the test discriminates the fix.Test Result
Standalone harness, fixed vs buggy, trimmed:
NetIbCast.CastGinErrorPathbuilds intorccl-UnitTestsMPI; a full on-node run can follow on an MI350X node.Submission Checklist