test(rccl): add host-only unit tests for transport/net_telemetry.cc and include/net_telemetry.h by dhanavikram · Pull Request #13835 · ROCm/rocm-systems

dhanavikram · 2026-10-20T18:45:06Z

Summary

Host-only coverage for network telemetry, src/transport/net_telemetry.cc and the inline hot-path hooks in src/include/net_telemetry.h (AICOMRCCL-2805), in rccl-UnitTestsMicro.

rcclTelemetryInit: RCCL_TELEMETRY_* env parsing, limits and defaults, latency-sample rounding/clamping, exit-handler registration, sampler start.
Device registration, driver/hardware-table resolution (ainic, mlx5, thor2), on-demand channel and QP slot allocation including allocation failures and untracked-QP accounting.
The counter and latency hooks (handle and index forms), histogram bucketing and min/max, SW capture.
HW counter collection from sysfs, ethtool (ioctl) and debugfs, first-use baselines and deltas, the sampler thread, the JSON report and flush.

The shared libc seam gains open, fopen, opendir, readdir, closedir, readlink, ioctl and atexit; their defaults fail (or record, for atexit) so results never depend on the host's /sys. open is the same seam #13755 adds, so whichever of the two merges second needs a trivial rebase.

Testing

164 tests, stable over 50 shuffled repeats.
Mutation testing, coverage, full host suite and in-tree build: results to follow in a comment.

AICOMRCCL-2805

therock-pr-bot · 2026-10-20T18:46:01Z

✅ 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?

amd-argus

ARGUS

Findings

5 more finding(s), not posted inline

projects/rccl/test/host/fakes/libc_fakes.h:117 (Low) libc_fakes.h:86 says every seam defaults to success, but the new g_open/g_fopen/g_opendir/g_readlink/g_ioctl default to failing
projects/rccl/test/host/net-telemetry-test.cc:2228 (Low) WarnUnresolvedSampled_ResolvedOnAnyDevice_DoesNotWarn checks only the stderr count; warned[] is available as the behavioural check
projects/rccl/test/host/net-telemetry-test.cc:1416 (Low) Out-of-range device tests (1403, 2045) check only in-range slots, so removing the bounds guards passes outside ASan builds
projects/rccl/test/host/net-telemetry-test.cc:797 (Low) The all-baselines-unavailable (-1) check block is duplicated at lines 439-462 and 797-818
projects/rccl/test/host/net-telemetry-test.cc:1885 (Nit) Nested for at 1884-1885 lacks outer braces (style guide: braces on all but the innermost)

This PR changes 2,902 lines across 7 files. A change this size is hard to review
well, for a person and for this bot alike: reviewers lose the thread, defects hide in the
volume, and the feedback arrives too late to be cheap to act on. Splitting it into a
stacked pull request
gives each piece a reviewable diff and keeps the history. This review covers the whole change
regardless.

Bottom line: Looks approval ready.

_{Comment @argus review to re-run, or @argus re-review after pushing.}

pvallem

Thanks for the thorough net_telemetry.cc coverage and for extending the shared libc seam. A few notes on sharing the remaining fakes and on coordinating with sibling PRs that touch the same seams.

amd-argus

ARGUS

This PR changes 2,923 lines across 8 files. A change this size is hard to review
well, for a person and for this bot alike: reviewers lose the thread, defects hide in the
volume, and the feedback arrives too late to be cheap to act on. Splitting it into a
stacked pull request
gives each piece a reviewable diff and keeps the history. This review covers the whole change
regardless.

Bottom line: Worth settling before approval: Flush_CollectsCountersOnlyForUsedDevices and ReadSysfsCounter_LongContent_ReadsAtMost63Bytes both still pass with the behaviour they target removed.

_{Comment @argus review to re-run, or @argus re-review after pushing.}

amd-argus

ARGUS

This PR changes 2,913 lines across 8 files. A change this size is hard to review
well, for a person and for this bot alike: reviewers lose the thread, defects hide in the
volume, and the feedback arrives too late to be cheap to act on. Splitting it into a
stacked pull request
gives each piece a reviewable diff and keeps the history. This review covers the whole change
regardless.

Bottom line: Worth settling before approval: three tests still pass with their target behaviour mutated (histogram bound at net_telemetry.h:806, qmin > 0 at net_telemetry.cc:1046, name-before-alternate lookup).

_{Comment @argus review to re-run, or @argus re-review after pushing.}

…#14299) ## Motivation #13863 skips the Ruby and OCI coco smoke runs for PRs to develop that only touch `projects/rccl/test/host/*`. But a PR that adds or extends a micro host binary also has to register it outside `test/host/`, so it still books both clusters: - `projects/rccl/test/test_categories_micro*.yaml`: the binary's CTest categories, read by the in-tree `test/host` CMake. - `projects/rccl/tools/scripts/test_runner/configs/ci-precheckin.json` and `lib/test_executor.py`: math-ci precheckin and coverage registration. Open examples that run coco only because of these files: #13548, #13755, #13835, #13857, #13858, #13642, #13116. ## Technical Details `rccl_change_sets.py` `COVERAGE_RULES`, develop only: - **"host-only unit tests"** also covers `projects/rccl/test/test_categories_micro.yaml` and `test_categories_micro_*.yaml`. The GPU suites' `test_categories.yaml`, `_fixtures`, `_fixtures_debug`, `_mpi`, `_amdsmi` and `_altrsmi` do not match and still trigger coco, as would a future `test_categories_microbench.yaml`. - **New "test runner" rule**: `projects/rccl/tools/scripts/test_runner/*`. This skips coco for any change confined to `test_runner/`, not only registrations. `rccl-host-unit-tests.yml` runs the test_runner unit tests on every PR to develop touching `projects/rccl/`. `ci-precheckin.json` itself is exercised only by math-ci precheckin. It is a separate rule from host-only tests so each keeps its own examples and justification. Why coco loses nothing here: - Its PR jobs in ROCm/tRCCL (`ruby64-`/`oci-rccl-pr-build`, then `-pr-smoke`) run harness-owned `smoke-test.yaml` and `rccl-perf-regression.yaml`, plus `pr-device-api.yaml` on Ruby: GPU `rccl-UnitTests`/`Fixtures`/`Gdr` gtests and rccl-tests. - Their `install.sh -t` build does configure `test/host` in-tree and parse the micro category files. A parse error there is only a CMake warning, and TheRock Multi-Arch CI (a required check, no path filter) runs the same in-tree configure. - No PR job runs the micro binaries, CTest labels, or anything under `tools/scripts`. No committed tRCCL job names a test_runner JSON, and the harness never executes `test_executor.py`. Also corrects #13863's comment that the in-tree `test/host` build is compiled only by TheRock (coco's build compiles it too), and updates the `rccl-host-unit-tests.yml` header for the second rule. Unchanged: TheRock Multi-Arch CI, math-ci precheckin, and the label-gated AINIC TW / dispatch-only MI350P workflows that use the test runner. ## Issue Tracking JIRA ID: N/A. Follow-up to #13863. ## Test Plan - `python3 -m pytest .github/scripts/tests/*rccl*_test.py` (what `rccl-cluster-runner-allowlist.yml` runs). - New cases: registration paths skip on develop; GPU category files still match; a full new-micro-binary file list emits `rccl=false`; look-alikes (`test_categories.yaml`, `test_categories_micro.yml`, `test_categories_microbench.yaml`, `test_runner_x/`, `tools/test_runner/`) stay uncovered. - Mutation check on a scratch copy: reverting, dropping or widening each pattern, or pointing a rule at another workflow, fails the tests. - Ran `coco_paths` on the changed files of the 7 PRs above and on every `test/test_categories*.yaml` on develop. ## Test Result - 120 passed, 149 subtests passed. - All 7 PRs above now skip coco; 42 micro category files are covered and the 6 GPU category files still trigger coco. ## Submission Checklist - [x] Look over the contributing guidelines at https://github.com/ROCm/rocm-systems/blob/develop/CONTRIBUTING.md.

Cover src/transport/net_telemetry.cc and the inline hooks in src/include/net_telemetry.h in rccl-UnitTestsMicro: env-driven init and its limits, device registration and hardware resolution, on-demand channel and QP slot allocation, the hot-path counter and latency hooks, SW capture, sysfs/ethtool/debugfs counter collection with first-use baselines, the sampler thread, and the JSON report and flush. The shared libc seam gains open, fopen, opendir, readdir, closedir, readlink, ioctl and atexit. Their defaults fail or record instead of touching the host, so results never depend on the machine's /sys. AICOMRCCL-2805

…bc_fakes JIRA ID: AICOMRCCL-2805 clock_gettime, posix_memalign, pthread_create and nanosleep join the shared libc seam (hooks in libc_fakes.{h,cc}, reset by ResetLibcFakes(), real-libc defaults), so other units can reuse them; net-telemetry-test.cc installs its fixed clock on g_clockGettime instead of keeping a private copy. MICROTEST_README lists net_telemetry.cc and the libc names it seams.

JIRA ID: AICOMRCCL-2805 The flush test now checks that no counter file of an unused device is opened, and the sysfs-read test uses content whose value differs between a 63- and a 64-byte read, so both fail if the behaviour they name is removed. The unresolved-sampled test also checks warned[], the two copies of the "every counter unavailable" checks share a helper, and the libc_fakes.h banner no longer claims every default succeeds.

JIRA ID: AICOMRCCL-2805 GetNs checks that CLOCK_MONOTONIC is read. The past-histogram test uses the first bucket past the end and checks nothing is written there, the min capture test scans its unsampled QP last, and SampledTableIdx checks that thor2 resolves the primary name over the alternate. Restores develop's note on the seam include in libc_fakes.cc.

amd-argus

ARGUS

This PR changes 2,930 lines across 8 files. A change this size is hard to review
well, for a person and for this bot alike: reviewers lose the thread, defects hide in the
volume, and the feedback arrives too late to be cheap to act on. Splitting it into a
stacked pull request
gives each piece a reviewable diff and keeps the history. This review covers the whole change
regardless.

Bottom line: Worth settling before approval: the new #define fopen makes scheduler.cc:156 fail in net-ib-cast-scheduler-test.cc, so its two log-path tests break, and Flush_SamplerRunning_StopsSamplerFirst does not pin the stop-first order.

_{Comment @argus review to re-run, or @argus re-review after pushing.}

JIRA ID: AICOMRCCL-2805 The libc_fakes fopen default now fails, and these tests write a real log file under a temp directory.

amd-argus

ARGUS

This PR changes 2,932 lines across 9 files. A change this size is hard to review
well, for a person and for this bot alike: reviewers lose the thread, defects hide in the
volume, and the feedback arrives too late to be cheap to act on. Splitting it into a
stacked pull request
gives each piece a reviewable diff and keeps the history. This review covers the whole change
regardless.

Bottom line: Nothing new worth raising; still open from earlier: net-telemetry-test.cc:2726. Worth clearing before approval.

_{Comment @argus review to re-run, or @argus re-review after pushing.}

JIRA ID: AICOMRCCL-2805

amd-argus

ARGUS

This PR changes 2,937 lines across 9 files. A change this size is hard to review
well, for a person and for this bot alike: reviewers lose the thread, defects hide in the
volume, and the feedback arrives too late to be cheap to act on. Splitting it into a
stacked pull request
gives each piece a reviewable diff and keeps the history. This review covers the whole change
regardless.

Bottom line: LGTM.

_{Comment @argus review to re-run, or @argus re-review after pushing.}

…nd include/net_telemetry.h (#13835) ## Summary Host-only coverage for network telemetry, `src/transport/net_telemetry.cc` and the inline hot-path hooks in `src/include/net_telemetry.h` (AICOMRCCL-2805), in `rccl-UnitTestsMicro`. - `rcclTelemetryInit`: `RCCL_TELEMETRY_*` env parsing, limits and defaults, latency-sample rounding/clamping, exit-handler registration, sampler start. - Device registration, driver/hardware-table resolution (ainic, mlx5, thor2), on-demand channel and QP slot allocation including allocation failures and untracked-QP accounting. - The counter and latency hooks (handle and index forms), histogram bucketing and min/max, SW capture. - HW counter collection from sysfs, ethtool (ioctl) and debugfs, first-use baselines and deltas, the sampler thread, the JSON report and flush. The shared libc seam gains `open`, `fopen`, `opendir`, `readdir`, `closedir`, `readlink`, `ioctl` and `atexit`; their defaults fail (or record, for `atexit`) so results never depend on the host's `/sys`. `open` is the same seam #13755 adds, so whichever of the two merges second needs a trivial rebase. ## Testing - 164 tests, stable over 50 shuffled repeats. - Mutation testing, coverage, full host suite and in-tree build: results to follow in a comment. AICOMRCCL-2805

systems-assistant · 2026-10-28T16:29:21Z

TheRock Submodule Bump Activity

Newest first

2026-10-28 — Included in TheRock via ROCm/TheRock#9555.

…3755, #13835 and #14381, one verbs recorder #13835 landed the same clock_gettime seam (g_clockGettime, micro_clock_gettime) on develop, so the four libc hunks here go and the scheduler and wqe_lat_mon tests use develop's. #13918 folded fakes/ibv_ops_fakes into IbVerbsScene.h, so GinDataScene builds on a scene: its data and flush QPs, CQ and PD are the scene's, and the recorder checks read scene.verbs.

#13980) ## Motivation `src/transport/net_ib/gin.cc`, `wqe_lat_mon.cc`, and `wqe_lat_mon.h` implement the host-side IB GIN proxy backend and per-QP WQE post-to-poll latency monitor. They had no host-only microtest coverage, so collective setup, RMA batching, completion behavior, latency state transitions, and failure paths required the larger runtime or RDMA hardware. This PR is stacked directly on [PR 13973](#13973). ## Technical Details Extends the existing `rccl-UnitTestsMicroTransport` binary and mirrors both production units under `test/host/transport/net_ib/`. `NetIbGinMicrotest` covers: - GDR capability gating, proxy initialization/finalization, properties, and backend vtable wiring - ring all-gather retry/completion behavior, all-to-all extraction, barrier, connect retry, and teardown - context creation, traffic class, rank stride, WR batch clamping, symmetric MR registration, and cleanup - QP-switch/capacity batching and exact IPut, IGet, IPutSignal, Test, and IFlush WR contracts - request/sequence completion, poll/error propagation, unconnected peers, local-rank flush keys, and signaling flags `NetIbWqeLatMonMicrotest` covers: - process-isolated initialization, scripted monotonic time, signaled-WR stamping, backlog tracking, and completion windows - strict slow/stall thresholds, one-second report limiters, opcode filtering, shared CQ timestamps, and failover scanning - Welford mean/stddev/max and exact P² percentiles for small, ascending, descending, mixed, 10k uniform, and bimodal distributions - slow/stall/summary report output and every summary enable/null/empty guard The tests reuse the existing transport binary and the generic NCCL and socket fakes. The GIN data path posts and polls on an `IbVerbsScene`, the unit's `ncclIbMalloc`/`ncclCalloc`/`free` go through `HostAllocLedger.h` (which gains a page-aligned `Malloc` and a fail-on-Nth-call knob), and its GDR, PeerMem and DMA-BUF probes rename onto `transport/net_ib/NetIbSupportSeams.h`. Hooks are `ScopedHook`s; `HookOf`, the member form, moves into `ScopedHook.h`. TU-local seams isolate only dependencies whose real definitions are already owned by sibling transport tests. `clock_gettime` goes through develop's `g_clockGettime` seam (#13835) in `fakes/libc_seam.h`, which `net-ib-cast-scheduler-test.cc` now uses in place of its own TU-local seam. Runner, category, and coverage-object registration come from PR #13858 because this uses the same `rccl-UnitTestsMicroTransport` binary; like the other host-only microtests it stays out of `ci-precheckin.json` after #14381. Production defects discovered by these tests are preserved as disabled forward specifications and tracked only under [AICOMRCCL-2397](https://amd-hub.atlassian.net/browse/AICOMRCCL-2397), the designated host-only incidental-findings epic. ## Issue Tracking JIRA ID: AICOMRCCL-2811 ## Test Plan - `run_host_tests.sh all` on the [PR 13973](#13973) head. - Standalone and in-tree `rccl-UnitTestsMicroTransport` builds. - Transport binary with `--gtest_shuffle --gtest_repeat=100`. - Five shuffled repetitions under ASan/UBSan/LSan. - One shuffled run under host-only ThreadSanitizer. - Eighteen focused production mutations covering WQE backlog/threshold/report guards, the summary count and gates, the exact-percentile path, and GIN ring, batching, addressing, atomic, flush, peer-selection, and device-bound behavior. - 33 more production mutations for the second review round, one or more per new assertion. - Explicit execution of all 12 disabled forward specifications. ## Test Result - All host binaries and guards pass; Transport: 477 tests. - New suites: 71 enabled tests; 12 disabled production-defect reproducers fail as intended. - 18 of 18 focused mutations caught. - Review follow-up: 16 of 16 mutations caught (GIN handle, traffic-class, listen, batch-clamp, peer/stride and flush-QP; WQE slow/stall reports, interleaved layout, summary gates and empty-QP guard). - Second review round: 33 of 33 caught (GIN closes, barriers, stride 0, connect TC and handle slot, published context, all-gather comms, all-to-all size, flush gate and drain, signal lkey, ledger frees; WQE targets, report param, P² markers, summary order, zero-device scan, limiter, outputs, opcodes, Init). - ASan/UBSan/LSan and TSan report no issues. - Scoped line coverage: 98.85% for `gin.cc`, 96.93% for `wqe_lat_mon.cc`, 98.23% combined. ## Submission Checklist - [x] Look over the contributing guidelines at https://github.com/ROCm/rocm-systems/blob/develop/CONTRIBUTING.md. [AICOMRCCL-2397]: https://amd-hub-sandbox-20240925.atlassian.net/browse/AICOMRCCL-2397?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ --------- Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: Dhanavikram Sekar <dhanavikram@users.noreply.github.com>

dhanavikram requested review from a team October 20, 2026 18:45

github-actions Bot added the project: rccl label Oct 20, 2026

systems-assistant Bot added the organization: ROCm label Oct 20, 2026

dhanavikram changed the title ~~rccl: add host-only tests for net telemetry~~ test(rccl): add host-only tests for net telemetry Oct 20, 2026

dhanavikram changed the title ~~test(rccl): add host-only tests for net telemetry~~ test(rccl): add host-only unit tests for transport/net_telemetry.cc and include/net_telemetry.h Oct 20, 2026

amd-argus Bot reviewed Oct 20, 2026

View reviewed changes

dhanavikram mentioned this pull request Oct 20, 2026

ci(rccl): skip cluster CI for host-only test changes on develop #13863

Merged

1 task done

pvallem mentioned this pull request Oct 21, 2026

test(rccl): add host-only unit tests for transport/net_socket.cc and net_socket/common.cc #13755

Merged

pvallem reviewed Oct 21, 2026

View reviewed changes

This was referenced Oct 21, 2026

test(rccl): add host-only unit tests for misc/ibv*, *dv* and gdr_peermem.cc #13857

Merged

test(rccl): add host-only unit tests for net_ib #13865

Merged

pvallem approved these changes Oct 21, 2026

View reviewed changes

amd-argus Bot reviewed Oct 21, 2026

View reviewed changes

Comment thread projects/rccl/test/host/net-telemetry-test.cc Resolved

Comment thread projects/rccl/test/host/net-telemetry-test.cc Resolved

amd-argus Bot reviewed Oct 21, 2026

View reviewed changes

Comment thread projects/rccl/test/host/net-telemetry-test.cc Resolved

Comment thread projects/rccl/test/host/net-telemetry-test.cc Resolved

Comment thread projects/rccl/test/host/net-telemetry-test.cc Resolved

aduforest approved these changes Oct 21, 2026

View reviewed changes

pvallem reviewed Oct 24, 2026

View reviewed changes

Comment thread projects/rccl/test/host/net-telemetry-test.cc Resolved

Comment thread projects/rccl/test/host/fakes/libc_fakes.cc Resolved

dhanavikram mentioned this pull request Oct 26, 2026

ci(rccl): skip cluster CI for host-test registration files on develop #14299

Merged

1 task done

dhanavikram added 4 commits October 27, 2026 14:37

dhanavikram force-pushed the users/dhanavikram/net-telemetry-host-tests branch from 9429343 to 757e395 Compare October 27, 2026 20:01

amd-argus Bot reviewed Oct 27, 2026

View reviewed changes

Comment thread projects/rccl/test/host/fakes/libc_seam.h Resolved

Comment thread projects/rccl/test/host/net-telemetry-test.cc Resolved

rccl: give the IB-CAST scheduler log tests a real fopen

894a270

JIRA ID: AICOMRCCL-2805 The libc_fakes fopen default now fails, and these tests write a real log file under a temp directory.

amd-argus Bot reviewed Oct 27, 2026

View reviewed changes

rccl: check the telemetry sampler is stopped before the report opens

7303ae5

JIRA ID: AICOMRCCL-2805

amd-argus Bot reviewed Oct 27, 2026

View reviewed changes

dhanavikram merged commit b0a6ded into develop Oct 27, 2026
61 of 64 checks passed

dhanavikram deleted the users/dhanavikram/net-telemetry-host-tests branch October 27, 2026 23:44

pvallem mentioned this pull request Oct 28, 2026

test(rccl): add host-only unit tests for gin.cc and wqe_lat_mon.{cc,h} #13980

Merged

1 task done

Conversation

dhanavikram commented Oct 20, 2026

Summary

Testing

Uh oh!

therock-pr-bot Bot commented Oct 20, 2026

✅ All Policy Checks Passed

Uh oh!

amd-argus Bot left a comment

Choose a reason for hiding this comment

Findings

Uh oh!

pvallem left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

amd-argus Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

amd-argus Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

amd-argus Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

amd-argus Bot left a comment

Choose a reason for hiding this comment

Uh oh!

amd-argus Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

systems-assistant Bot commented Oct 28, 2026

TheRock Submodule Bump Activity

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants