Repository navigation
test(net): add v11 plugin compat lazy-init unit test - #8633
Conversation
✅ All Policy Checks Passed
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
|
🚫 Please fix the failed policies before requesting reviews. The following policy checks failed:
The |
There was a problem hiding this comment.
Pull request overview
Adds a new regression unit test to guard the NCCL v11 network-plugin compatibility layer’s lazy-initialization contract, ensuring only ->init is populated before initialization and all other vtable entry points are deferred until after init() succeeds.
Changes:
- Introduces an in-process mock v11 net/collnet plugin and validates pre-/post-
init()vtable wiring behavior via GoogleTest. - Updates
rccl-UnitTestsbuild to include the new test source and exports executable symbols fordlsym(dlopen(NULL), ...)via-rdynamic.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| projects/rccl/test/PluginCompatV11Tests.cpp | Adds regression tests for v11 net/collnet compat-layer lazy initialization using in-process exported mock plugin symbols. |
| projects/rccl/test/CMakeLists.txt | Includes the new test in rccl-UnitTests and adds -rdynamic so dlsym can resolve in-process plugin symbols. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ffaa5a2 to
c884648
Compare
c884648 to
983952e
Compare
|
%RCCL_TEST_CONFIG |
|
%RCCL_TEST_CONFIG |
|
%RCCL_TEST_CONFIG |
1 similar comment
|
%RCCL_TEST_CONFIG |
|
%RCCL_TEST_CONFIG |
278bc7a to
a01bb82
Compare
|
%RCCL_TEST_CONFIG |
|
Checked manually |
RCCL Perf-Regression Gate: ✅ PASSMode: detect (reference vs candidate) Per-collective breakdown
|
Add PluginCompatV11_test.cpp, an in-process regression test for the NCCL v11 network-plugin compat layer's lazy initialization: getNcclNet_v11() / getNcclCollNet_v11() must populate only ncclNet.init and leave the remaining entry points null until init() runs. The test defines an in-process mock v11 net/collnet plugin exported under the symbols the compat layer resolves via dlsym, so no GPU or real plugin is required. It lives in the Debug-only rccl-UnitTestsFixturesDebug target since it calls internal symbols (getNcclNet_v11) that are hidden in Release builds, and exports the two mock symbols via --export-dynamic-symbol. Resolves AICOMRCCL-1532
Register the two PluginCompatV11 tests as a plugin_compat_v11 gtest group (rccl-UnitTestsFixturesDebug, 1 GPU) so they run in the MI300X + Mellanox IB test-runner config.
The compat-layer mock symbols are exported via --export-dynamic-symbol, not -rdynamic; update the comment to match.
64a8798 to
1d7490c
Compare
The plugin_compat_v11 test configuration had no matching test_suites entry, so the test_runner never enumerated the PluginCompatV11 lazy-init tests. Add the suite entry (Debug-only) so CI actually runs them.
1d7490c to
9448042
Compare
## Motivation
The NCCL v11 network-plugin compatibility layer must initialize
**lazily**:
`getNcclNet_v11()` / `getNcclCollNet_v11()` should populate only the
`init`
entry point of the public `ncclNet_t` / `ncclCollNet_t` vtable, and
defer
wiring up every other entry point (`devices`, `getProperties`, `listen`,
…)
until the plugin's `init()` callback has actually run. A v11 plugin is
only
allowed to expose its per-device entry points after `init()`; populating
the
vtable eagerly (the old direct-cast pattern) meant RCCL could call into
an
uninitialized plugin.
The fix already landed on `develop`
(`projects/rccl/src/plugin/net/net_v11.cc`), but there was no test
guarding
the behavior. This PR adds that regression test and wires it into CI.
## Technical Details
- New test `projects/rccl/test/PluginCompatV11_test.cpp`:
- Defines an in-process mock v11 net/collnet plugin and exports it under
the
exact symbols the compat layer resolves via `dlsym`
(`ncclNetPlugin_v11` / `ncclCollNetPlugin_v11`), so **no GPU or real
network plugin is required**.
- Hands the compat layer a `dlopen(NULL)` handle and asserts the
lazy-init
contract:
1. `getNcclNet_v11()` returns a non-null `ncclNet_t*`.
2. Before `init()`: only `->init` is set; every other entry point is
`nullptr`.
3. After `->init()` succeeds: the remaining entry points become
non-null.
- Covers both the net and collnet compat layers.
- `projects/rccl/test/CMakeLists.txt`:
- Adds `PluginCompatV11_test.cpp` to the **Debug-only**
`rccl-UnitTestsFixturesDebug` target — the test calls internal symbols
(`getNcclNet_v11`) that have hidden visibility in Release builds, so it
cannot live in the always-built `rccl-UnitTests` (that would break the
Release link).
- Exports only the two mock plugin symbols via
`--export-dynamic-symbol=ncclNetPlugin_v11 / =ncclCollNetPlugin_v11`
(rather
than the broad `-rdynamic`) so `dlsym`(`dlopen(NULL)`) can find them.
-
`projects/rccl/tools/scripts/test_runner/configs/mi300x_mellanox_ib.json`:
- Registers the two tests as a `plugin_compat_v11` gtest group
(`rccl-UnitTestsFixturesDebug`, 1 GPU) so they run in the MI300X +
Mellanox
IB config.
## JIRA ID
JIRA ID : AICOMRCCL-1532
## Test Plan
Built and ran on an AMD MI350X (gfx950) node (Debug build):
```
bash install.sh -l -t --debug --prefix ~/rccl-install-1532
./build/debug/test/rccl-UnitTestsFixturesDebug --gtest_filter=PluginCompatV11*
```
Validity of the test was confirmed by temporarily reverting
`getNcclNet_v11()` to the old eager/direct-cast pattern and re-running.
The
tests are also registered in the `mi300x_mellanox_ib` test-runner config
so
they execute in CI.
## Test Result
- Fixed (current) code — **PASS**:
```
[==========] Running 2 tests from 1 test suite.
[ RUN ] PluginCompatV11.NetLazyInitialization
[ OK ] PluginCompatV11.NetLazyInitialization (0 ms)
[ RUN ] PluginCompatV11.CollNetLazyInitialization
[ OK ] PluginCompatV11.CollNetLazyInitialization (0 ms)
[ PASSED ] 2 tests.
```
- With `getNcclNet_v11()` reverted to the eager pattern — **FAIL** (the
test
catches the bug), failing exactly on the pre-`init()` null checks
(`devices` / `getProperties` / `listen` / … set before `init()`).
Restoring the fix returns the suite to PASS.
## Submission Checklist
- [x] Look over the contributing guidelines at
https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests.
Motivation
The NCCL v11 network-plugin compatibility layer must initialize lazily:
getNcclNet_v11()/getNcclCollNet_v11()should populate only theinitentry point of the public
ncclNet_t/ncclCollNet_tvtable, and deferwiring up every other entry point (
devices,getProperties,listen, …)until the plugin's
init()callback has actually run. A v11 plugin is onlyallowed to expose its per-device entry points after
init(); populating thevtable eagerly (the old direct-cast pattern) meant RCCL could call into an
uninitialized plugin.
The fix already landed on
develop(
projects/rccl/src/plugin/net/net_v11.cc), but there was no test guardingthe behavior. This PR adds that regression test and wires it into CI.
Technical Details
projects/rccl/test/PluginCompatV11_test.cpp:exact symbols the compat layer resolves via
dlsym(
ncclNetPlugin_v11/ncclCollNetPlugin_v11), so no GPU or realnetwork plugin is required.
dlopen(NULL)handle and asserts the lazy-initcontract:
getNcclNet_v11()returns a non-nullncclNet_t*.init(): only->initis set; every other entry point isnullptr.->init()succeeds: the remaining entry points become non-null.projects/rccl/test/CMakeLists.txt:PluginCompatV11_test.cppto the Debug-onlyrccl-UnitTestsFixturesDebugtarget — the test calls internal symbols(
getNcclNet_v11) that have hidden visibility in Release builds, so itcannot live in the always-built
rccl-UnitTests(that would break theRelease link).
--export-dynamic-symbol=ncclNetPlugin_v11 / =ncclCollNetPlugin_v11(ratherthan the broad
-rdynamic) sodlsym(dlopen(NULL)) can find them.projects/rccl/tools/scripts/test_runner/configs/mi300x_mellanox_ib.json:plugin_compat_v11gtest group(
rccl-UnitTestsFixturesDebug, 1 GPU) so they run in the MI300X + MellanoxIB config.
JIRA ID
JIRA ID : AICOMRCCL-1532
Test Plan
Built and ran on an AMD MI350X (gfx950) node (Debug build):
Validity of the test was confirmed by temporarily reverting
getNcclNet_v11()to the old eager/direct-cast pattern and re-running. Thetests are also registered in the
mi300x_mellanox_ibtest-runner config sothey execute in CI.
Test Result
getNcclNet_v11()reverted to the eager pattern — FAIL (the testcatches the bug), failing exactly on the pre-
init()null checks(
devices/getProperties/listen/ … set beforeinit()).Restoring the fix returns the suite to PASS.
Submission Checklist