Repository navigation
feat(fabric): add --telemetry and repoint --topology to config - #11084
Merged
Merged
Conversation
github-actions
Bot
added
documentation
✅ 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.
🔵 Needs a closer look
It introduces broad ABI/API surface changes (new struct layout selection, new sysfs write APIs, and CLI behavior changes) that warrant final human review for compatibility and operational safety.
Pull request overview
This PR expands the AMD SMI “fabric” surface to support separate configuration vs telemetry reporting in the CLI, and adds a new nested fabric-info layout plus new UALink sysfs-backed write APIs (PPoD/vPoD/station) with accompanying unit/functional tests and binding updates.
Changes:
- Add
--telemetryand repoint--topologyto configuration output, including a targeted warning on explicit--topologyusage. - Introduce
amdsmi_fabric_info_v2_t+ layout selection viaamdsmi_fabric_info_t::fabric_version, and add new fabric configuration write APIs (amdsmi_set_gpu_fabric_*_config). - Add new C++ unit/functional test coverage for layout compatibility, sysfs round-trips, and public API argument rejection; update Python CLI tests to lock in flag routing/behavior.
File summaries
| File | Description |
|---|---|
| projects/amdsmi/tests/python/cli/test_fabric.py | Adds CLI-level flag contract tests for --topology/--telemetry behavior and stderr warning expectations. |
| projects/amdsmi/tests/amd_smi_test/unit/gpu/fabric_public_api_test.cc | New unit test ensuring public fabric getter rejects null arguments. |
| projects/amdsmi/tests/amd_smi_test/unit/gpu/fabric_info_layout_test.cc | New layout/ABI-guard unit tests for v1/v2 fabric info union behavior and bounded writes. |
| projects/amdsmi/tests/amd_smi_test/unit/gpu/fabric_config_roundtrip_test.cc | New hardware-free sysfs round-trip tests for fabric config apply/query seams. |
| projects/amdsmi/tests/amd_smi_test/main.cc | Registers new functional fabric write test suite with privilege gating. |
| projects/amdsmi/tests/amd_smi_test/functional/ifoe/fabric/fabric_write.h | Declares new functional test harness for fabric write paths. |
| projects/amdsmi/tests/amd_smi_test/functional/ifoe/fabric/fabric_write_test.cc | Adds functional coverage for setters’ validation, support gating, and safe round-trips. |
| projects/amdsmi/tests/amd_smi_test/functional/ifoe/fabric/fabric_read_test.cc | Updates functional fabric read test to consume/print v2 layout and adds flat-vs-subtree diagnostics. |
| projects/amdsmi/src/CMakeLists.txt | Adds new fabric UALink implementation source to the build. |
| projects/amdsmi/src/amd_smi/amd_smi_ualoe.cc | Implements new public setter APIs that delegate into AMDSmiGPUDevice. |
| projects/amdsmi/src/amd_smi/amd_smi_gpu_device.cc | Refactors fabric info read path to v2-first gathering + bounded publish to requested layout; adds flatten/publish helpers. |
| projects/amdsmi/src/amd_smi/amd_smi_fabric_ualink.cc | New sysfs serialization/parsing implementation for apply/query of fabric config domains, with commit/divergence diagnostics. |
| projects/amdsmi/rust-interface/src/amdsmi_wrapper.rs | Updates Rust FFI bindings for fabric v2 layout and new fabric config setter APIs/types/constants. |
| projects/amdsmi/py-interface/amdsmi_wrapper.py | Updates Python ctypes bindings for v2 fabric layout and new fabric config APIs/types/constants. |
| projects/amdsmi/py-interface/amdsmi_interface.py | Updates Python high-level API to request/require fabric v2 layout and expose new fields/masks. |
| projects/amdsmi/include/amd_smi/impl/amd_smi_gpu_device.h | Extends device interface for fabric apply/query helpers and publishes bounded-layout helpers for tests. |
| projects/amdsmi/include/amd_smi/impl/amd_smi_fabric_ualink.h | New internal header exposing filesystem-seam apply/query cores for hardware-free testing. |
| projects/amdsmi/include/amd_smi/amdsmi.h | Adds fabric v2 layout + layout selector enum, new fabric config structures/enums, and new setter API declarations/docs. |
| projects/amdsmi/example/amd_smi_fabric_example.cc | Updates example to request and consume fabric v2 layout including station fields and masks. |
| projects/amdsmi/docs/reference/amdsmi-py-api.md | Updates Python API docs for fabric info to reflect v2 layout and additional keys. |
| projects/amdsmi/docs/how-to/amdsmi-cli-tool.md | Updates CLI docs to reflect --topology/--telemetry flag set and semantics. |
| projects/amdsmi/CHANGELOG.md | Documents the breaking CLI flag changes and the new fabric v2 layout + config APIs. |
| projects/amdsmi/amdsmi_cli/subcommands/fabric.py | Routes --topology to config and --telemetry to counters; updates defaults. |
| projects/amdsmi/amdsmi_cli/amdsmi_parser.py | Adds --telemetry, updates --topology help, and implements stderr warning action for explicit --topology. |
Review details
- Files reviewed: 24/24 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
bill-shuzhou-liu
approved these changes
Sep 2, 2026
amd-josnarlo
requested changes
Sep 2, 2026
adam360x
approved these changes
Sep 2, 2026
oliveiradan
force-pushed
the
users/daolivei/amdsmi_aifm_telemetry
branch
from
September 3, 2026 18:18
b93f66a to
5ec1fe8
Compare
amd-josnarlo
self-requested a review
September 4, 2026 14:07
amd-josnarlo
approved these changes
Sep 4, 2026
oliveiradan
force-pushed
the
users/daolivei/amdsmi_aifm_telemetry
branch
from
September 5, 2026 00:51
5ec1fe8 to
f14e1ab
Compare
marifamd
force-pushed
the
users/daolivei/amdsmi_aifm_telemetry
branch
from
September 14, 2026 18:42
f14e1ab to
e974f03
Compare
marifamd
force-pushed
the
users/daolivei/amdsmi_aifm_telemetry
branch
2 times, most recently
from
September 30, 2026 16:07
736b305 to
9178df7
Compare
marifamd
force-pushed
the
users/daolivei/amdsmi_aifm_telemetry
branch
from
October 6, 2026 19:34
9178df7 to
fa917c6
Compare
- Separate the two fabric datasets onto distinct flags so one flag no
longer serves two different outputs:
- ``--topology`` now reports device configuration
(``amdsmi_get_gpu_fabric_info()``), previously shown by ``--info``
- ``--telemetry`` reports the per-category counters
(``amdsmi_get_fabric_telemetry_data()``), previously shown by
``--topology``
- Remove ``-i``/``--info``. Its output moved to ``--topology`` under the
unchanged ``fabric_info`` output key
- Parser (``amdsmi_cli/amdsmi_parser.py``):
- Replace the ``-i``/``--info`` argument with ``-T``/``--telemetry``
and swap the two help strings
- Add ``_WarnTopologySemanticsChange``, an ``argparse.Action`` that
writes a warning to stderr when ``--topology`` is passed explicitly.
The flag changed meaning without failing to parse, so a script
written against the old flag would have silently read configuration
where it expected counters. Keeping the warning off stdout is what
leaves ``--json`` and ``--csv`` parseable. The no-flag default sets
both flags internally and stays quiet
- CLI (``amdsmi_cli/subcommands/fabric.py``):
- Rename the ``info`` override parameter and ``args.info`` to
``telemetry``
- Gate the ``fabric_info`` block on ``args.topology`` and the
``fabric_telemetry`` block on ``args.telemetry``
- Docs (``docs/how-to/amdsmi-cli-tool.md``):
- Update the ``fabric --help`` block for the new flag set
- ``CHANGELOG.md``:
- Record the flag rework as breaking, including the ``--info`` removal
and why ``--topology`` warns
- ``tests/python/cli/test_fabric.py``:
- Add ``test_flag_contract`` covering the flag set in ``--help``, that
``--info`` no longer parses, that each flag routes to its own output
key, and that the warning reaches stderr for an explicit
``--topology`` only
- Skip in ``setUp`` when the fabric subcommand is not registered, so an
uninitialized amdgpu driver does not read as a contract failure
Signed-off-by: Oliveira, Daniel <daniel.oliveira@amd.com>
marifamd
force-pushed
the
users/daolivei/amdsmi_aifm_telemetry
branch
from
October 12, 2026 17:26
fa917c6 to
67b15d2
Compare
1 task done
Contributor
TheRock Submodule Bump ActivityNewest first
|
1 task done
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
IFoE telemetry
amd-smi fabric --telemetryabsent from amd-smi outputTechnical Details
longer serves two different outputs: -
--topologynow shows device configuration (amdsmi_get_gpu_fabric_info) ---telemetryshows the per-category counters(
amdsmi_get_fabric_telemetry_data)-i/--infoas a deprecated hidden alias for--topology, slated forremoval in a future ROCm release.
--helpno longer lists it - Parser (amdsmi_cli/amdsmi_parser.py): - Add-T/--telemetry, repoint the-t/--topologyhelp text, and hide-i/--infowithargparse.SUPPRESS- CLI (amdsmi_cli/subcommands/fabric.py): - Route--topologytoamdsmi_get_gpu_fabric_info()and--telemetrytoamdsmi_get_fabric_telemetry_data()- Fold--infointo--topology. Keep thefabric_infooutput key unchanged - Docs (docs/how-to/amdsmi-cli-tool.md): - Update the fabric--helpblock for the new flag set -CHANGELOG.md: - Log the breaking fabric CLI flag rework -tests/python/cli/test_fabric.py: - Add a flag-contract test covering the new flag set and the hidden--infoaliasIssue Tracking
JIRA ID: ROCM-29630
Test Plan
tests/amd_smi_test/unit/gpu/fabric_info_layout_test.cc(new): hardware-free check that the version 1 offsets and size hold, that a version 1 or unrecognized request leaves every byte past the version 1 member untouched, that a version 2 request fills the nested member, and thatflatten_v2_to_v1()maps every version 1 fieldtests/amd_smi_test/unit/gpu/fabric_public_api_test.cc(new): null-handle rejection inamdsmi_get_gpu_fabric_info()tests/amd_smi_test/unit/gpu/fabric_config_roundtrip_test.cc(new): flat read,mask == 0rejection, non-committing request -tests/amd_smi_test/functional/ifoe/fabric/fabric_write_test.cc(new): null-handle, invalid-mask, and per-device hardware paths -tests/python/cli/test_fabric.py: flag set, each flag's routing to its output key, rejection of--info, which invocations warn, and a skip when the fabric subcommand is not registeredTest Result
pre-commit: all hooks pass -cli.test_fabric: 2 tests OK (13.9 s) with the amdgpu driver loaded; both skip cleanly when the fabric subcommand is not registeredamd-smi fabric -t --json: warning emitted once on stderr, stdout is valid JSON carryingfabric_infoonly.--telemetryand the no-flag default emit nothing on stderr.--infoand-iexit non-zeroamdsmitst --gtest_filter='*Fabric*': 29 tests from 3 suites, 27 passed, 2 skipped, 0 failed (18 ms). All hardware-free coverage passes: the 4FabricInfoLayout*offset/size/flatten checks,FabricPublicApiGetRejectsNullArguments, the 17FabricConfig*round-trip and validation checks, and the 3FabricTelemIdToString*checksIfoeFunctionalReadOnly.TestFabricReadskips with "Fabric (IFoE) not supported on this system",IfoeFunctionalReadWrite.TestFabricWriteskips with "Invalid permission - Must run as super user"N/ASubmission Checklist