[i-kosarev]

Motivation

#9973 added --net_ib_nthreads but exercised it on two host-memory tests. This extends the matrix to 36 tests (registration, IB-CAST and fault-injection paths) and adds one test with no single-threaded equivalent: one worker breaks its own connection while the others keep transferring. Tests were picked where NET/IB shares state between independent plugin contexts — the per-device MR cache and protection domain, per-communicator fault and IB-CAST scheduler state, and the request pool and completion routing.

#9973 has since landed, so this targets develop directly.

Technical Details

Test-only. The harness (NetIbMPITestBase.hpp) gains worker-safe data-path
primitives returning ThreadResult instead of barriering and asserting fatally,
spreads workers over NICs with a routable GID rather than device 0, and
propagates the rank's HIP device into workers, as the current device is
thread-local.

The runner matrix grows from 352 to 445 executions and reaches CI through
ainic.json A17-A19 and a four-worker suite in each of the six per-cluster
functional configs. Each threaded suite extends a base declaring no tests, since
extends merges test lists: extending a suite that carries tests would silently
run those too, at the wrong worker count. Two settings are deliberate — a 10 s
RCCL_IB_QP_SCHED_UPDATE_INTERVAL, without which the RTT timer rewrites tokens
mid-assertion, and RCCL_IB_FAULT_INJECTION off in the threaded fault suites,
whose ops shims put a process-global mutex in every poll_cq and whose
device-keyed registry lets one worker's clear disarm another's faults.

Two runner bugs fixed on the way: six configs filtered on AlltoAllStress where
the test is AllToAllStress, and a gtest filter matching nothing exits zero, so
the 4-rank all-to-all stress test was quietly absent; and schema.json rejected
the num_gpus: 0 of CPU-only suites, which kept five cluster configs from
loading at all.

Issue Tracking

JIRA ID: AICOMNET-323

Test Plan

Enabled tests

Worker counts are 2, 4 and 16 unless stated otherwise. Tests marked (#9973) came
with the option itself; the rest are enabled here.

Independent connections, host memory and GPU buffers:

• SimpleSendRecv (rccl: add multithread support to NetIbMPI validation suite #9973), MultipleSequentialTransfers (rccl: add multithread support to NetIbMPI validation suite #9973) — one and then
  repeated transfers per worker on its own connection.
• SendRecvMultipleSizes — every worker sweeps the size ladder, so many sizes
  land in the shared per-device MR cache at once.
• LargeTransfer — concurrent 16 MB transfers hold several large registrations
  live.
• MemoryRegistrationStorm — register/deregister storms in forward, reverse and
  shuffled order race the cache's insert, lookup and eviction paths.
• MrCacheRefCount — all workers register the same address, contending on one
  entry's refcount rather than merely on the cache mutex.
• MixedSizeBarrage — 1 B to 64 MB per worker: alignment and chunking under
  bandwidth pressure.
• TagZeroReuse — every worker reuses tag 0, so a misrouted completion appears
  as a payload mismatch instead of passing.
• RequestSlotExhaustion — N x 32 requests in flight on one NIC, then slot
  recycling.
• RapidRecvPostDrain — post-and-drain batches while other communicators drain
  their own.
• FifoPressureSenderFast — N slow receivers against N tight senders; the
  backpressure assertion stays per connection.
• LongRunningEndurance — sustained concurrent traffic, RDMA leak check on the
  main thread around the whole run.
• GpuMemoryTransferStress — concurrent NCCL_PTR_CUDA registrations drive the
  peer-memory path into the same cache.

Fused devices (vNIC created once on the main thread, its index handed to workers):

• RegDeregCycling_VNic — each registration fans out across both members'
  protection domains and caches, doubling the contended surface.
• SequentialTransfers_VNic — long-lived registrations, one per worker.
• MixedSizes_VNic — uneven QP splits across the fused device.
• LargeTransfer_VNic — striped transfers; per-worker size scales down with the
  worker count, since each registration is pinned once per member.
• UnalignedSizeTransfer_VNic — the 128-byte striping boundary.
• FlushRepeated_VNic — concurrent GPU receives plus iflush.

Multi-QP outside IB-CAST, four workers: MultiQpSplitDataStress and
MultiQpNoSplitStress, multiplying QPs per device on the split and nDataQps
paths.

IB-CAST scheduler, with the RTT update suspended so token deltas are meaningful:

• CastStressMultiRoundTwoConns — the serial body builds many connections but
  drives them one at a time; here the schedulers advance simultaneously, each
  auditing its own ledger. Weaker in one respect: the serial body asserts the RTT
  timer fired, which cannot hold at the 10 s interval, so the threaded variant
  drops that assertion.
• CastSendRecvMultipleSizes — both sides of the WRR/split boundary.
• CastLargeTransfer — 16 MB split path, no tokens consumed.
• CastSendRecvZeroSize — a zero-byte send still takes the WRR path.
• CastSplitDataThresholdBoundary — at the threshold and one byte below.

Two guards against a global lock on the progress path, both new tests:

• ThreadedProgressDoesNotSerialize — the same transfer loop at one worker and
  at all of them; the parallel phase must stay under 60% of what full
  serialization would cost. A fourth phase holds a mutex across every transfer and
  the same measurement must flag it, so the gate cannot pass by having gone blind.
  It polls for completion without sleeping, unlike the functional tests: with the
  harness's 10 ms backoff a transfer costs one poll interval, and the phase
  timings then report the interval rather than the plugin.
• FaultInjectionShimsAbsentUnlessRequested — with RCCL_IB_FAULT_INJECTION
  off, arming an ops-level fault must be refused, which holds only while the
  libibverbs shims are absent. Also runs in a plain CAST suite, since such a leak
  costs the whole process and has nothing to do with threading. Deterministic
  because the timing gate's budget is deliberately loose enough for a loaded node
  to pass, which the shims also do, even though the gate separates them clearly
  (numbers under Test Result).

Fault injection, per-communicator hooks, two and four workers:

• FaultIsolationAcrossWorkers — new test. One worker breaks its own
  connection and must observe the failure while every other worker keeps
  transferring with a fatal count of zero. No serial test can assert this.
• FaultInjCastQpErrorIsFatal — each worker arms error injection on its own QPs
  and must see the failure itself.
• FaultInjCastDelayDataIntegrity — each worker slows its own QP 0 and still
  delivers intact data while the others load the device.
• FaultInjCastQpErrorClearRecovers — two connections per worker: break the
  first, clear, move data cleanly on the second.
• FailoverCqeErrorRecovered — N links fail at once, so the process-global
  recovery thread serves several communicators.
• FailoverLargeMessageDataIntegrity — 64 KB messages survive their own link
  failure byte for byte.
• FailoverMultiRequestInFlight — several messages posted first, then QP 0 driven
  to error under them.
• RecoverySuccessRestoresTraffic (two workers) — concurrent recoveries share one
  recovery thread, then traffic must resume everywhere. Two deliberate differences
  from the serial body: the QPs are broken while the connection is idle, because
  the serial in-flight break relies on an MPI handshake a worker cannot make, and
  only one message is sent after recovery. With ten, the ranks drift a message
  apart in about one run in six — the sender exhausting its FIFO-slot retries on
  message n+1 while the receiver waits for n, both devices reported healthy. That
  drift is recorded as a finding for the recovery path rather than smoothed over;
  the serial body hides it behind its per-phase handshakes.

Scheduler toggles, single-worker, at a 10 s update interval. These four require it
and had skipped in every shipped config, so this is their first execution:
CastWeightsDistributionOneRound, CastAlternatingWrrNonWrr,
CastEnableDisableSplitData, CastEnableDisableSched.

Procedure

Hardware: mia1, AINIC/Pensando with RoCE on rdma0..rdma3, gfx950, host build.

• Rebuild rccl-UnitTestsMPI in a debug configuration with MPI tests enabled.
• Run the worker-count matrix, the CAST and fault matrices and the long-interval
  scheduler suites, two ranks on one node and across two nodes.
• Run the complete NetIB transport configuration as a regression sweep.
• Verify the CLI contract: clamping, malformed values, conflicting duplicates.
• Measure branch coverage of src/transport/net_ib and net_ib_cast over the
  same test list at one worker and at four, and compare the union against the
  serial profile alone.

Test Result

mia1, gfx950, two ranks, on develop at 5d05d0483b:

• Worker-count, CAST and fault matrices: 93 PASS / 0 FAIL / 0 HANG / 1 SKIP,
  the same tally on one node and with the ranks on separate nodes; the skip is the
  gate declining to judge two workers. Long-interval suites: 4 PASS, the first
  execution of those four rather than a skip. AINIC CI on this branch: Group A
  20 PASS / 0 FAIL, A17-A19 included, two nodes.
• Full NetIB configuration: 435 PASS / 2 FAIL / 0 HANG / 7 SKIP / 1 ERROR of
  445. No non-passing entry belongs to this branch: two fail identically on
  develop alone, and NetIB_Conn_RapidConnectDisconnect tripped its leak check
  holding fewer resources than at start (CQs 14 to 13, MRs 22 to 15), a
  co-tenant moving a device-wide counter.
• CLI: --net_ib_nthreads=99 clamps and passes, abc is rejected before GTest,
  conflicting duplicates are rejected. The tests also compile with fault
  injection disabled, which no cluster configuration builds.

Scaling, GTest's own per-test duration in ms, median of three:

test	N=2	N=4	N=8	N=16
LongRunningEndurance (2000 transfers per worker)	21072	21217	22123	22314
MemoryRegistrationStorm (work divided by N)	6640	6486	6929	7874

The data path does not serialize: endurance holds transfers per worker fixed, so
eight times the workers is eight times the traffic for 6% more wall time.
Registration does, as intended: the storm divides work by N, so constant time
means a flat aggregate rate, which is the per-device MR cache mutex it is aimed
at.

ThreadedProgressDoesNotSerialize measures the same conclusion directly, at
about 7 us per transfer on one node and 10 us across two. Healthy against the
same build with RCCL_IB_FAULT_INJECTION set, whose ops shims take one
process-global mutex per poll_cq, three runs each:

workers	healthy	with the ops shims	budget	mutex held across each transfer
4	1.02, 1.02, 1.16	1.79, 1.74, 1.94	2.40	4.04
16	1.65, 2.11, 1.75	6.20, 5.42, 5.39	9.60	16.29

Cross-node, one rank per node: 1.05 at four workers and 1.27 at sixteen, the
single-node growth being the two ranks' threads sharing cores. The budget stays
at 0.6 x N so a loaded node cannot fail the suite, which is why the shim leak
keeps its deterministic probe rather than relying on these numbers.

Branch coverage over the 28 converted tests, serial against its union with the
four-worker profile: 2456 to 2497 of 31066 branches, with 41 taken only under
concurrent workers — both arms of the protection-domain refcount, MR cache
insert, miss and page-range hit, and completion processing in p2p.cc. A floor,
not a measure of what threading buys: coverage says nothing about interleavings.

Submission Checklist

• Look over the contributing guidelines at
  https://github.com/ROCm/rocm-systems/blob/develop/CONTRIBUTING.md.

[Copilot]

Pull request overview

Extends NetIbMPI multithread validation across registration, IB-CAST, fault-injection, GPU, and stress paths without modifying production code.

Changes:

• Adds worker-safe transfer, fault, scheduler, device-selection, and reporting helpers.
• Adds threaded variants across NetIbMPI tests and CI configurations.
• Corrects the AllToAllStress test filters and documents selection guidance.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
net_ib_transport.json	Expands threaded test matrices.
mi455_ainic_roce.json	Adds four-worker suites and fixes filter.
mi455_ainic_roce_func.json	Adds functional threaded suites.
mi355x_thor2_roce.json	Adds four-worker suites and fixes filter.
mi355x_thor2_roce_func.json	Adds functional threaded suites.
mi300x_mellanox_ib.json	Adds four-worker suites and fixes filter.
mi300_mellanox_ib_func.json	Adds functional threaded suites.
ainic.json	Adds A17–A19 CI coverage.
StressTests.cpp	Thread-enables registration and stress tests.
NicFusionTests.cpp	Thread-enables fused-NIC tests.
NetIbMPITestBase.hpp	Adds worker-safe harness primitives.
GeneralTests.cpp	Thread-enables general transfers.
FaultInjectTests.cpp	Adds concurrent fault and recovery tests.
CastTests.cpp	Adds concurrent scheduler validation.
multithread-validation.md	Documents threaded-test selection guidance.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[therock-pr-bot]

✅ All Policy Checks Passed

Check	Status	Details
📝 PR Description	✅ Pass	—
⛔ Forbidden Files	✅ Pass	—
🧪 Unit Test	✅ Pass	—
🚫 Draft PR	🔜 To Be Enabled	—
🚩 Feature Flag	🔜 To Be Enabled	—
📊 Code Coverage	🔜 To Be Enabled	—

🎉 All policy checks passed!

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

🙋 Wish to Override Policy?

[Copilot]

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.

Suppressed comments (3)

projects/rccl/test/transport/NetIbMPI/FaultInjectTests.cpp:3688

• The body start gate only synchronizes entry before allocation, registration, and warmup. There is no phase gate here, so the bystander loops can finish before worker 0 arms or triggers its fault; the fatal-count checks then pass without testing traffic alongside a broken connection. Synchronize workers after the victim has posted/armed the fault and before releasing bystanders to transfer.

            const bool victim = (threadIdx == 0);
            if (victim) {
                if (rank == 0) {

projects/rccl/tools/scripts/test_runner/configs/net_ib_transport.json:1069

• The threaded implementation scales each worker to 64 MB / nThreads (with a 4 MB floor), so no threaded execution performs a 64 MB transfer per worker. Describe this as 64 MB aggregate traffic to keep runner reports accurate.

          "description": "Concurrent 64 MB striped transfers over a fused device",

projects/rccl/test/transport/NetIbMPI/StressTests.cpp:719

• If either registration pass fails partway, or deregistration fails, this worker returns with earlier handles still live. The buffer scope guard then frees their backing allocations before connection teardown, leaving registered memory behind and obscuring the original failure with leaks. Add a scope guard that deregisters every non-null handle before bufCleanup runs.

                std::vector<void*> handles(kThreadedBufs, nullptr);
                auto registerAll = [&]() {
                    for (int i = 0; i < kThreadedBufs; i++) {
                        result = WorkerRegister(comm, bufs[i], kThreadedBufSz, NCCL_PTR_HOST,
                                                &handles[i]);
                        if (!result.ok) return false;

[Copilot]

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.

Suppressed comments (15)

projects/rccl/tools/scripts/test_runner/configs/net_ib_transport.json:1193

• Extending cast_wrr_no_split merges in its four tests, while the three _LongInterval entries below have different names and are added as duplicate filters. The suite consequently executes seven cases (three twice) rather than the three intended long-interval toggles. Use a testless no-split base or same-name overrides that do not retain unrelated tests.

      "extends": "cast_wrr_no_split",

projects/rccl/tools/scripts/test_runner/configs/net_ib_transport.json:1099

• Because inherited tests are merged, this no-split config also contains every independent test and MultiQpSplitDataStress. The latter is guaranteed to skip after this child changes NCCL_IB_SPLIT_DATA_ON_QPS to 0, contradicting the dedicated no-split suite and its no-skip matrix. Base this config on a testless common multi-QP configuration instead.

      "extends": "net_ib_multithread_multiqp",

projects/rccl/tools/scripts/test_runner/configs/net_ib_transport.json:1085

• extends merges parent and child test lists by name (lib/test_config.py:208-213,263-267), so this suite inherits all 19 independent tests and adds the one multi-QP test instead of containing only the advertised multi-QP case. Use a testless multithread base (or otherwise avoid inheriting the independent test list) so this suite does not add 19 unintended executions.

This issue also appears on line 1099 of the same file.

      "extends": "net_ib_multithread_independent",

projects/rccl/tools/scripts/test_runner/configs/net_ib_transport.json:1180

• Test lists merge by test name, so extending cast_wrr_split retains all seven parent tests and adds this differently named duplicate of CastEnableDisableSched. This suite therefore runs eight cases, not the single long-interval toggle case described here. Inherit from a testless CAST environment or override using the existing test name.

This issue also appears on line 1193 of the same file.

      "extends": "cast_wrr_split",

projects/rccl/tools/scripts/test_runner/configs/net_ib_transport.json:1216

• fault_inject_cast already defines five tests, and runner inheritance merges them by name. Since the seven new entries use different names, each threaded suite runs 12 cases; three filters (QpErrorIsFatal, DelayDataIntegrity, and QpErrorClearRecovers) run twice, and two unconverted parent cases run unexpectedly. Share only the parent's environment through a testless base.

      "extends": "fault_inject_cast",

projects/rccl/tools/scripts/test_runner/configs/net_ib_transport.json:1269

• This inheritance merges the complete expanded fault suite into the recovery configuration, so the recovery suite runs all inherited fault cases plus RecoverySuccessRestoresTraffic under recovery-enabled settings. It is not the one-test recovery suite described by the PR and further duplicates the fault matrix. Derive it from a testless fault environment instead.

      "extends": "fault_inject_multithread",

projects/rccl/tools/scripts/test_runner/configs/mi455_ainic_roce.json:268

• The runner merges tests from fault_inject_cast by name; it does not replace them. This config therefore runs the parent's five cases plus these two, and FaultInjCastQpErrorClearRecovers is executed twice because the entries have different names. Extend a testless fault environment so this cluster suite contains only its intended two tests.

      "extends": "fault_inject_cast",

projects/rccl/tools/scripts/test_runner/configs/mi455_ainic_roce_func.json:253

• The runner merges tests from fault_inject_cast by name; it does not replace them. This config therefore runs the parent's five cases plus these two, and FaultInjCastQpErrorClearRecovers is executed twice because the entries have different names. Extend a testless fault environment so this cluster suite contains only its intended two tests.

      "extends": "fault_inject_cast",

projects/rccl/tools/scripts/test_runner/configs/mi355x_thor2_roce.json:272

• The runner merges tests from fault_inject_cast by name; it does not replace them. This config therefore runs the parent's five cases plus these two, and FaultInjCastQpErrorClearRecovers is executed twice because the entries have different names. Extend a testless fault environment so this cluster suite contains only its intended two tests.

      "extends": "fault_inject_cast",

projects/rccl/tools/scripts/test_runner/configs/mi355x_thor2_roce_func.json:259

• The runner merges tests from fault_inject_cast by name; it does not replace them. This config therefore runs the parent's five cases plus these two, and FaultInjCastQpErrorClearRecovers is executed twice because the entries have different names. Extend a testless fault environment so this cluster suite contains only its intended two tests.

      "extends": "fault_inject_cast",

projects/rccl/tools/scripts/test_runner/configs/mi300x_mellanox_ib.json:509

• The runner merges tests from fault_inject_cast by name; it does not replace them. This config therefore runs the parent's five cases plus these two, and FaultInjCastQpErrorClearRecovers is executed twice because the entries have different names. Extend a testless fault environment so this cluster suite contains only its intended two tests.

      "extends": "fault_inject_cast",

projects/rccl/tools/scripts/test_runner/configs/mi300_mellanox_ib_func.json:253

• The runner merges tests from fault_inject_cast by name; it does not replace them. This config therefore runs the parent's five cases plus these two, and FaultInjCastQpErrorClearRecovers is executed twice because the entries have different names. Extend a testless fault environment so this cluster suite contains only its intended two tests.

      "extends": "fault_inject_cast",

projects/rccl/tools/scripts/test_runner/configs/net_ib_transport.json:1069

• This description says every concurrent transfer is 64 MB, but the threaded body uses max(4 MB, 64 MB / nThreads), so the configured 2/4/16-worker runs transfer 32/16/4 MB per worker. Describe the fixed 64 MB aggregate instead.

          "description": "Concurrent 64 MB striped transfers over a fused device",

projects/rccl/test/transport/NetIbMPI/StressTests.cpp:720

• If registration fails partway through this loop, earlier handles remain live while bufCleanup frees their backing buffers on return. The same leak occurs when a later deregistration fails. Add a scope guard that deregisters every non-null entry in handles; the normal path already nulls entries after successful deregistration.

                std::vector<void*> handles(kThreadedBufs, nullptr);
                auto registerAll = [&]() {
                    for (int i = 0; i < kThreadedBufs; i++) {
                        result = WorkerRegister(comm, bufs[i], kThreadedBufSz, NCCL_PTR_HOST,
                                                &handles[i]);
                        if (!result.ok) return false;

projects/rccl/test/transport/NetIbMPI/StressTests.cpp:82

• The || short-circuits, so if deregistering mh1 fails, mh2 is never deregistered and remains live while the shared buffer is freed. Attempt both cleanup operations before reporting either failure.

                if (DeregisterMemory(comm, mh1) != ncclSuccess
                    || DeregisterMemory(comm, mh2) != ncclSuccess) {

[Copilot]

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated no new comments.

Suppressed comments (2)

projects/rccl/test/transport/NetIbMPI/NetIbMPITestBase.hpp:1706

• Deferring only the allocation is not sufficient for the outstanding receives handled by the new fault tests. Their NetMHandleWorkerGuard remains active, so it calls deregMr when the worker returns even though WorkerDrainRecv reported that a work request still references this buffer; the QP is closed only later. Preserve the registration until the request is quiesced/QP is torn down as well, rather than invalidating the MR while the receive is live.

    void DeferWorkerBufferFree(void* buffer) {

projects/rccl/test/transport/NetIbMPI/FaultInjectTests.cpp:3785

• faultArmed is sticky, so it does not prove that any bystander transfer overlaps the live fault. The victim can arm, observe the failure, clear the fault, and set victimFinished before a bystander is scheduled; this wait then succeeds and all ten transfers run after the fault is gone, while the later victimFinished wait also passes. Keep the victim fault armed until the bystanders signal that they have entered or completed their transfer loop.

            if (!waitForFlag(faultArmed, kFlagPollIterations)) {

[Copilot]

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.

Suppressed comments (3)

projects/rccl/test/transport/NetIbMPI/StressTests.cpp:596

• The saturated 32-request phase cannot detect cross-communicator completion routing: every worker sends the same pattern for a slot, and the receiver checks neither the completion size nor payload. A request routed to the wrong worker can therefore satisfy all waits and pass. Use a worker-specific seed and validate each receive before testing slot reuse.

                        fillHostBufferWithPattern<uint8_t>(slot, sz, makeBytePattern(i));

projects/rccl/test/transport/NetIbMPI/StressTests.cpp:87

• This branch does not guarantee the cross-worker refcount contention it claims to test. After the body gate opens, one worker can register twice and deregister both handles before another worker runs, reducing this to the existing serial refs=2 case. Add a cancellable worker rendezvous after every worker owns both handles and before any deregistration.

This issue also appears on line 596 of the same file.

                ThreadResult result = WorkerRegister(comm, shared.get(), sharedSz,
                                                     NCCL_PTR_HOST, &mh1);
                if (!result.ok) return result;
                result = WorkerRegister(comm, shared.get(), sharedSz, NCCL_PTR_HOST, &mh2);

projects/rccl/test/transport/NetIbMPI/FaultInjectTests.cpp:3847

• faultArmed only releases the bystanders; it does not keep the fault armed until they run. The victim can execute the send and clear the fault before any bystander is scheduled, so process-global fault state that is cleared here could still let this isolation test pass. Keep the victim's fault active until all bystanders acknowledge completing their healthy transfers (with a bounded/cancellable rendezvous).

                faultArmed.store(true, std::memory_order_release);

                const WorkerFaultSendOutcome outcome =
                    WorkerCastFaultSend(pair.sendComm, buffer, size, 801, mhandle, 200);

[Copilot]

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.

Suppressed comments (2)

projects/rccl/tools/scripts/test_runner/configs/net_ib_transport.json:1008

• This description says the timing gate detects the libibverbs ops shims, but the test's own analysis states that their short mutex acquisitions leave the measured factor near 1.0 and are detected only by FaultInjectionShimsAbsentUnlessRequested. As written, the runner metadata promises coverage this test does not provide; describe only locks held across progress calls.

          "description": "Gate: the same busy-polled loop at one worker and at all of them, so a process-global lock on the progress path (the libibverbs ops shims being the known one) shows up as time that scales with the worker count. Skips below four workers",

projects/rccl/tools/scripts/test_runner/configs/ainic.json:187

• This description claims the gate catches the libibverbs ops shims, while the test and PR measurements explicitly say those short acquisitions are invisible to this timing ratio and require the separate shim probe. Update the CI metadata so it does not claim coverage the timing test cannot provide.

          "name": "A17_NetIB_MultiThread_ProgressDoesNotSerialize",
          "description": "Gate: concurrent workers must progress at the same time. Catches a process-global lock on the isend/irecv/test path, such as the libibverbs ops shims, which functional tests pass straight through.",
          "test_filter": "NetIbMPITest.ThreadedProgressDoesNotSerialize"

[Copilot]

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

projects/rccl/tools/scripts/test_runner/lib/test_executor.py:1058

• num_gpus: 0 is not inert when num_ranks is "auto": line 1032 resolves that combination to zero ranks, this guard accepts it on a single node, and the MPI path later emits -np 0 (and host:0 under SLURM). The runtime validation should reject any non-positive resolved rank count as well as negative GPU/node values, rather than allowing a configuration that launches no test.

        if num_nodes > 1 and num_gpus < 1:
            msg = (f"Invalid num_gpus for test '{test_name}': {num_gpus} GPUs/node cannot place "
                   f"ranks across {num_nodes} nodes")

[Copilot]

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.

Suppressed comments (2)

projects/rccl/test/transport/NetIbMPI/StressTests.cpp:99

• The || short-circuits, so if deregistering mh1 fails, mh2 is never deregistered. That leaves an avoidable live registration and can turn the original failure into a misleading MR leak. Invoke both deregistrations before combining their results.

                if (DeregisterMemory(comm, mh1) != ncclSuccess
                    || DeregisterMemory(comm, mh2) != ncclSuccess) {

projects/rccl/test/transport/NetIbMPI/NetIbMPITestBase.hpp:1435

• Posting all requests does not ensure they are still in flight when this error is injected. Once the sender has obtained each request, the NIC can complete these 4 KiB transfers immediately, and the receiver also starts polling independently; there is no synchronization or delay that holds completions until this line. The test can therefore pass after faulting an idle QP without exercising recovery of concurrent failed requests. Add worker-safe cross-rank coordination or a controlled fault/delay that keeps the requests pending, and verify they are pending before driving QP 0 to error.

        if (rank == 1) {
            result = WorkerCastDriveQpToError(pair.sendComm, 0);
            if (!result.ok) return result;

[Copilot]

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated no new comments.

Suppressed comments (3)

projects/rccl/tools/scripts/test_runner/lib/schema.json:199

• 0 is not actually inert for every single-node run as documented. A valid CPU-only config with num_gpus: 0, num_nodes: 1, and multiple ranks passes this schema/placement guard, but the executor later treats num_ranks as the required GPU count (test_executor.py:1161) and, under SLURM, emits each host with zero slots (test_executor.py:1299). Such a run is therefore skipped or cannot launch. Either restrict zero-GPU configurations to one rank, or make the GPU-capacity and host-slot logic explicitly handle CPU-only multi-rank runs.

          "description": "GPUs per node; controls --map-by ppr:{num_gpus}:node for multi-node runs. The string \"auto\" (also the default when omitted) resolves to the detected GPUs per node. Zero declares a test that needs no GPU, such as a CPU-only build smoke; the executor only uses this value for multi-node placement, so zero is inert for single-node runs.",
          "oneOf": [
            { "type": "integer", "minimum": 0 },

projects/rccl/test/transport/NetIbMPI/NetIbMPITestBase.hpp:1738

• This routability check is device-name-wide, not port-specific: CanRouteCrossNode returns true if any sysfs port for props.name has a routable GID, while the plugin creates a distinct physical device entry for each active port (src/transport/net_ib/init.cc:499-505). On a multi-port RoCE HCA with one routable port and one link-local-only port, both indices are admitted and Spread() can still assign a worker to the unusable port, causing the timeout this filter is meant to prevent. Resolve routability for the specific underlying port represented by vProps.devs[0] rather than accepting any port on the HCA.

            if (props.name && !CanRouteCrossNode(props.name)) continue;
            if (seen.insert(props.vProps.devs[0]).second) indices.push_back(i);

projects/rccl/test/transport/NetIbMPI/FaultInjectTests.cpp:2091

• This says the transfer is already posted, but WorkerTransferAcrossQpFailure first drives the receive/send QP to error and only then calls WorkerSendRecvPattern (NetIbMPITestBase.hpp:1588-1598). The helper's own documentation and the test plan explicitly describe an idle-connection break, so this comment currently misstates the recovery scenario being tested.

                // Both directions lose QP 0 with the transfer already posted: the
                // alive-message handshake needs both sides to notice and enter
                // recovery, and the payload has to ride the surviving device.

[i-kosarev]

Divided into #10538 + #10539 + #10540 + #10541

[i-kosarev]

Work transferred into #10538 + #10539 + #10540 + #10541