Add pmon configuration resolution HLD - #2362
Open
aditya-nexthop wants to merge 3 commits into
Open
Conversation
Add the High Level Design describing how xcvrd resolves its runtime tunables via a single XcvrdConfig object that reads the xcvrd section of pmon_daemon_control.json directly, replacing the per-tunable command-line-flag mechanism. Adding a new tunable becomes a one-field change to XcvrdConfig, and changing a flag at runtime only requires restarting xcvrd rather than having pmon regenerate the whole supervisord template. Link the new HLD from sonic_docs_toc.md under the xrcvd section. Signed-off-by: aditya-nexthop <aditya@nexthop.ai>
Collaborator
|
/azp run |
|
No pipelines are associated with this pull request. |
Contributor
Author
moshemos
previously approved these changes
Jul 14, 2026
Contributor
|
Hi @aditya-nexthop , it is a good idea to simplify the flow of adding parameters to xcvrd. Based on your proposal, I think we could do it more generic. How about changing XcvrdConfig to PmonDaemonConfig? Other daemons like thermalctld, psud could also be beneficial from this new desing. |
Contributor
@Junchao-Mellanox I have requested @aditya-nexthop for a community review |
tshalvi
reviewed
Jul 30, 2026
…nges Restructure the design around a shared PmonDaemonConfig base in sonic_py_common - the one library every pmon daemon already depends on - with per-daemon subclasses declaring a section name and their fields. The resolution mechanism is not xcvrd-specific: thermalctld already hand-plumbs five tunables through the same template -> argparse -> constructor path. xcvrd is the first adopter, other daemons migrate in their own changes, and thermalctld is written out as a worked example. Record why sonic_py_common beats an xcvrd-private resolver, a new shared wheel, or folding into DaemonBase. Add a FieldSpec validation layer that enforces a declared range after type coercion, plus a per-field table of types, ranges, and defaults. Coercion alone lets through values that are not valid configuration, and the consumers disagree today: DomInfoUpdateTask rejects a negative dom_update_interval, while DomThermalInfoUpdateTask never checks poll_interval, so a negative value leaves the next scheduled poll permanently in the past and the sweep runs back-to-back. Out-of-range values keep the built-in default and log; never fatal. Fields that are genuinely unbounded must say so explicitly. Add section 9.3 stating the boundary the platform files already follow but never documented: top-level skip_/delay_/capability keys are pmon orchestration settings consumed by the supervisord template, while the nested "<daemon>" object is the daemon's own runtime config. skip_xcvrd and delay_xcvrd can never move daemon-side; the two mgr capability flags could, and are left alone because migrating them relocates a key platforms already set. Signed-off-by: aditya-nexthop <aditya@nexthop.ai>
Collaborator
|
/azp run |
|
No pipelines are associated with this pull request. |
The design now defines a shared PmonDaemonConfig in sonic_py_common with xcvrd as its first adopter and thermalctld as a worked example, so doc/xrcvd hid it from the maintainers it is aimed at. doc/pmon already holds the pmon-wide designs. Rename to match the component and module name. Signed-off-by: aditya-nexthop <aditya@nexthop.ai>
Collaborator
|
/azp run |
|
No pipelines are associated with this pull request. |
Contributor
Author
|
I moved the file to the pmon directory as we have increased the scope. |
10 tasks
aditya-nexthop
changed the title
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
6 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.
Add the High Level Design for how pmon daemons resolve their runtime tunables from the per-platform
pmon_daemon_control.json, replacing the per-tunable command-line-flag mechanism.Today a tunable is plumbed end-to-end as a flag: the platform sets it in
pmon_daemon_control.json,sonic-cfggenloads that file while renderingdocker-pmon.supervisord.conf.j2, the template flattens it into--flag value,argparsere-parses it, and the daemon constructor grows a parameter. Adding one knob means editing four places, and it has to be redone per daemon —thermalctldcarries five tunables this way today,xcvrdtwo.The design defines a shared
PmonDaemonConfigresolver insonic_py_commonthat owns everything not specific to a daemon: locating the file (hwsku over platform, mirroringdocker_init.j2), extracting the daemon's section, layering it over the built-in defaults, coercing types, validating ranges, and degrading to defaults on any error. A daemon adopts it by declaring a dataclass subclass with a section name and one field per tunable. Adoption is per-daemon and independent, so a daemon that has not migrated is unaffected.xcvrdis the first adopter;thermalctldis written out as a worked example.Adding a tunable becomes one field plus one
FieldSpecentry, and changing a value only requires restarting the daemon rather than having pmon regenerate the whole supervisord template.Also covered:
intbut is not a valid cadence, and consumers today disagree about what happens next. An out-of-range value keeps the built-in default and logs a warning — never fatal, so a bad tunable cannot keep a daemon down.sonic_py_commonis already a universal dependency of every pmon daemon. Alternatives considered and rejected: keeping the resolver private toxcvrd, a new shared wheel insonic-platform-daemons, folding it intoDaemonBase, and sourcing tunables from Config DB.skip_*/delay_*/ capability keys are pmon orchestration settings consumed by the supervisord template; the nested"<daemon>"object is the daemon's own runtime config.skip_xcvrdanddelay_xcvrdgate process startup and can never move daemon-side; the two mgr capability flags could, and are deliberately left alone because migrating them relocates a key platforms already set.The doc lives at
doc/pmon/pmon_daemon_config_hld.mdand is linked fromsonic_docs_toc.mdunder the pmon section, matching its pmon-wide scope.Revision history: v0.1 initial; v0.2 generalizes the resolver to
PmonDaemonConfigand adds per-field range validation, following review feedback.Associated code PRs to be merged in order:
sonic-net/sonic-buildimage#28859
sonic-net/sonic-platform-daemons#854
sonic-net/sonic-buildimage#28306