[rust] Add crates/thermalctld, a Rust thermalctld beside the Python daemon - #883
Open
ganglyu wants to merge 2 commits into
Open
[rust] Add crates/thermalctld, a Rust thermalctld beside the Python daemon#883ganglyu wants to merge 2 commits into
ganglyu wants to merge 2 commits into
Conversation
…aemon
A cargo workspace beside the Python daemons, the way sonic-swss-common carries
`crates/` beside its C++. The Python `sonic-thermalctld/` is untouched and
still installed; which one runs is a key in `pmon_daemon_control.json` and a
pmon restart, with no rebuild. Selecting it, and the vendor platform API it
reaches hardware through, are in the sonic-buildimage change that depends on
this one.
What it does is what the Python daemon does: publish TEMPERATURE_INFO,
FAN_INFO, FAN_DRAWER_INFO, PHYSICAL_ENTITY_INFO and the liquid-cooling tables,
mirror thermals to a Switch-BMC, and hand the platform its start and stop hooks
so hw-management-tc is suspended on every exit path including a panic --
`panic = "abort"` in the workspace profile is load-bearing for that, since the
hook exists because unwinding is skipped.
Hardware is reached through the `platform-traits` crate in
sonic-platform-common, never directly, so no vendor is named here. `Cargo.toml`
depends on the fixed crate name `sonic-platform` through a `vendor-platform`
symlink that `debian/rules` and the Makefile point at whatever
`platform/<vendor>/rules.mk` supplies. `clean` deliberately does not go through
cargo: a workspace cannot be enumerated before that symlink exists.
Six behavioural divergences from Python were found and fixed while writing it,
three by reading the Python source and three on hardware:
* The fan-speed policy ran on the monitor's cadence. Python runs it on a
second loop at the manager's own interval, and not on the first cycle --
it would be acting on readings the box has only just started producing.
* A pass could not be interrupted. Python checks its stopping event between
devices at five points, and returns before the sweep that deletes vanished
rows; sweeping an interrupted pass would delete every sensor and fan after
the interruption point.
* Fourteen messages Python sends at NOTICE were published a severity lower,
because the log crate has no NOTICE and syslog::BasicLogger maps four
levels. A `notice!` macro routes those records past the level map.
* `platform.json` asks for a 3 second ASIC thermal against a 60 second cycle.
A gate can only make a component slower, so Python replaces the cycle with
the fastest interval anything asks for (thermalctld:1283-1291) and pins
everything else back to the old cycle. The community case
`test_thermal_sensors_update_at_configured_intervals` caught this one.
* A leak row was cached before the write, so a dropped write was remembered
as published and never retried. Python caches only after `Table.set`
returns.
* Clearing the cache on a dropped aggregate write also cleared the record of
what the alarm had announced, so one leak was logged as two. The two jobs
are now separate fields.
Measured on an SN5640 against the Python daemon on the same device: 3.3 MB of
anonymous memory against 23.3 MB, two threads, and identical key counts and
field values across all four tables.
174 unit tests, clippy clean with `-D warnings`. The tests drive the real
shapes rather than mocks of them: STATE_DB access sits behind `TableLike` the
way Python shadows the swsscommon package with a dictionary, and the polling
loop's ordering, gating and wait arithmetic run under `tokio::time::pause()`
with no test sleeping. Regression on an SN5640: 12 community cases and 9
canonical cases pass against this daemon, none fail.
Signed-off-by: ganglyu <glv@nvidia.com>
Collaborator
|
/azp run |
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
rustfmt.toml, a .pre-commit-config.yaml and ci-* targets in the Makefile, so the crate carries its own checks rather than relying on review to catch formatting. The policy and the target names are sonic-dash-ha's, which is the only other Rust in SONiC. `make ci-format` is the one that runs in a bare checkout of this repository. It calls rustfmt directly rather than `cargo fmt`, because cargo resolves every dependency in the manifest before it will format a line, and three of this crate's live outside this repository: platform-traits in sonic-platform-common, swss-common in sonic-swss-common, and the vendor's platform API behind the vendor-platform symlink that debian/rules creates at build time. In a bare checkout `cargo fmt` therefore fails on `cargo metadata` before formatting anything. rustfmt only parses, so it works anywhere, and both read the same rustfmt.toml. `format` moves to rustfmt for the same reason. The rest of ci-all compiles and so needs those three resolved; with them, clippy (clippy::all denied), build and doc (warnings denied) and the 174 tests all pass, each in debug and release. Release matters here beyond habit: this workspace's release profile sets panic = "abort", which the hw-management-tc panic hook depends on. ci-lint and the rest use -p rather than --workspace. The vendor-platform symlink resolves inside the workspace directory, so cargo counts the vendor's crate as a member, and --workspace would lint a vendor's code from here. The check target's comment said `cargo fmt --check` was deliberately not used because these files align columns by hand. That is no longer true of them, so check now runs it. The rest of the diff is rustfmt output over hand-formatted code: re-wrapping, the trailing commas rustfmt adds when it breaks a literal, and the mod/use reordering reorder_imports asks for. No behaviour changes and the suite passes unchanged. Signed-off-by: ganglyu <glv@nvidia.com>
Collaborator
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
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
2 participants
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.
Why I did it
thermalctldruns on every SONiC switch and spends its life doing sysfs reads and STATE_DB writes. In Python that costs a full interpreter and the platform plugin it imports, resident for the life of the box — 23.3 MB of anonymous memory measured against 3.3 MB for the Rust daemon on the same device.It is also the first pmon daemon to go through the vendor-neutral Rust platform API, so it is what proves that contract carries a real daemon.
Nothing is switched on here: the Python
sonic-thermalctld/is untouched, still installed and still the default. Which one runs is a key inpmon_daemon_control.jsonand a pmon restart, with no rebuild.Work item tracking
How I did it
Added
crates/thermalctld, a cargo workspace beside the Python daemons — the way sonic-swss-common carriescrates/beside its C++. No Python file is touched.It publishes what the Python daemon publishes (
TEMPERATURE_INFO,FAN_INFO,FAN_DRAWER_INFO,PHYSICAL_ENTITY_INFOand the liquid-cooling tables), mirrors thermals to a Switch-BMC, and hands the platform its start and stop hooks sohw-management-tcis suspended on every exit path including a panic.No vendor is named anywhere in the crate: hardware is reached through the
platform-traitscrate in sonic-platform-common, STATE_DB throughswss-common, and the fixed crate namesonic-platformresolves through avendor-platformsymlink thatdebian/rulesand theMakefilepoint at whateverplatform/<vendor>/rules.mksupplies.Six behavioural divergences from Python were found and fixed while writing it:
logcrate has no NOTICE.thermalctld:1283-1291).thermalctld:751-752).Two review findings are deliberately not fixed and say so in a comment where they live, because Python has both shapes: the guard that makes
set_temperature(None)unreachable, and the overrun fallback that can exceed the shrunken cycle.How to verify it
174 unit tests, clippy clean with
-D warnings. STATE_DB access sits behindTableLike, so a test asserts the value that reachesFAN_INFOrather than that a setter was called; the polling loop runs undertokio::time::pause()with no test sleeping. On hardware the daemon publishes the same key counts and field values as the Python one across all four tables. Existing Pythonsonic-thermalctldtests are unaffected.Which release branch to backport (provide reason below if selected)
N/A — new feature, no backport requested.
Tested branch
Description for the changelog
Add
crates/thermalctld, a Rust thermalctld built on the vendor-neutral Rust platform API; the Python daemon stays installed and stays the default.Link to config_db schema for YANG module changes
N/A — no CONFIG_DB schema change. The STATE_DB tables written are the ones the Python
thermalctldalready writes.