[xcvrd] Replace dom_* configuration flags with XcvrdConfig resolver - #854
Open
aditya-nexthop wants to merge 2 commits into
Open
[xcvrd] Replace dom_* configuration flags with XcvrdConfig resolver#854aditya-nexthop wants to merge 2 commits into
aditya-nexthop wants to merge 2 commits into
Conversation
Adds XcvrdConfig, which resolves dom_temperature_poll_interval and dom_update_interval from the "xcvrd" section of pmon_daemon_control.json (hwsku file taking precedence over platform file), falling back to built-in defaults (None) when unset. This replaces the old --dom_* argparse flags and threading a new command-line flag through the supervisord template, so adding a new tunable only requires a new dataclass field. DaemonXcvrd now builds self.config = XcvrdConfig() (or resolves it) instead of taking dom_temperature_poll_interval/dom_update_interval constructor args; call sites read from self.config instead. Signed-off-by: aditya-nexthop <aditya@nexthop.ai>
Collaborator
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
This was referenced Jul 8, 2026
aditya-nexthop
marked this pull request as ready for review
July 8, 2026 22:04
10 tasks
XcvrdConfig owned the whole resolution mechanism: locating pmon_daemon_control.json, extracting the section, merging, coercing, and error handling. None of that is xcvrd-specific, and thermalctld would have to copy it verbatim to get the same benefit, so it moves to sonic_py_common.pmon_daemon_config.PmonDaemonConfig. This module is left as xcvrd's schema: the section it owns, its fields, and their specs. Replace _FIELD_CASTERS with _FIELD_SPECS so each tunable declares a valid range alongside its caster. Coercion alone let through values that are not valid configuration, and the two consumers disagreed about what happened next: DomInfoUpdateTask rejected a negative dom_update_interval and fell back to its 60s default, while DomThermalInfoUpdateTask never checked poll_interval at all. A negative value there left the next scheduled poll permanently in the past, so the sweep ran back-to-back with no delay: a typo like -60 produced maximum transceiver I2C load instead of one poll per minute. Bounds are now enforced in one place, after coercion, so the stringified form is caught too. A rejected value keeps the built-in default and logs a warning; it never stops xcvrd from starting. The DomInfoUpdateTask guard stays as defense-in-depth for direct constructor callers. Depends on the sonic-py-common change adding PmonDaemonConfig (sonic-net/sonic-buildimage#28859), which must merge first. Signed-off-by: aditya-nexthop <aditya@nexthop.ai>
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.
Description
HLD Link
Adds
XcvrdConfig, which resolvesdom_temperature_poll_intervalanddom_update_intervalfrom the"xcvrd"section ofpmon_daemon_control.json(hwsku file takes precedence over platform file, no cross-file merge — mirrorsdocker_init.j2), falling back to built-in defaults (None) when unset.DaemonXcvrdnow buildsself.config = XcvrdConfig()(resolved viaXcvrdConfig.resolve()) instead of takingdom_temperature_poll_interval/dom_update_intervalas constructor args; call sites read fromself.configinstead of instance attributes.Motivation and Context
Previously, adding a new xcvrd tunable required threading a new
--flagthrough argparse, the supervisord template, and theDaemonXcvrdconstructor. Moving tunables into the per-platformpmon_daemon_control.jsonfile (which platform owners already maintain for other daemons) means a new knob only requires a new dataclass field onXcvrdConfig— no template or constructor changes.How Has This Been Tested?
Ran the full
sonic-xcvrdpytest suite (tests/test_xcvrd_config.py,tests/test_xcvrd.py) inside the SONiC build slave container:New tests in
tests/test_xcvrd_config.pycover: default values, platform-section overrides, partial overrides,None/0handling, string-to-int coercion, invalid/unknown keys, hwsku-vs-platform file precedence, missing/malformed files, and end-to-endresolve().tests/test_xcvrd.pyupdated to verifyDaemonXcvrdwiresself.configfromXcvrdConfig.resolve().Additional Information (Optional)
After this PR merges, sonic-net/sonic-buildimage#28306 removes the no longer necessary lines from the Jinja template.