-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Add HLD for H/W capabilities fields in platform.json #768
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
jleveque
merged 6 commits into
sonic-net:master
from
ArunSaravananBalachandran:platform_json_capabilities_field
Jun 29, 2021
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
5111768
Add H/W capabilities fields in platform.json
ArunSaravananBalachandran c49bba1
Add field for maximum fan speed
ArunSaravananBalachandran 07258a6
Reorder fan speed fields
ArunSaravananBalachandran 5e94f28
Address review comment - Change 'control' to 'controllable'
ArunSaravananBalachandran b9477f8
Specify default value for 'controllable'
ArunSaravananBalachandran 7e736af
Reword note on controllable
ArunSaravananBalachandran File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,125 @@ | ||
| # Platform capability file enhancement # | ||
|
|
||
| ### Rev 0.1 | ||
|
|
||
| ## Table of Content | ||
|
|
||
| ## Revision | ||
|
|
||
| | Rev | Date | Author | Change Description | | ||
| |:---:|:-----------:|:----------------------------:|----------------------| | ||
| | 0.1 | | Arun Saravanan Balachandran | Initial version | | ||
|
|
||
| ## Scope | ||
|
|
||
| This document provides information on the enhancements for platform capability file `platform.json`. | ||
|
|
||
| ## Definitions/Abbreviations | ||
|
|
||
| | Definitions/Abbreviation | Description | | ||
| |--------------------------|-------------| | ||
| | BMC | Baseband Management Controller | | ||
| | NOS | Network Operating System | | ||
| | PSU | Power Supply Unit | | ||
|
|
||
| ## Overview | ||
|
|
||
| Each networking switch has a set of platform components (e.g: Fans, PSUs, LEDs, etc.) and these components in each platform can have different characteristics (like supported colors for a LED). In a given platform, the components could be controlled by a dedicated platform controller (like BMC) or the NOS running on the CPU is required to control it and in the former the control of the components from the NOS could be limited. | ||
|
|
||
| In SONiC the platform components' supported attributes are made available via Platform API, but certain platform specific capabilties for the components are not available for the applications. | ||
|
|
||
| This document provides the enhancement for `platform.json` to address the above issue. | ||
|
|
||
| ## Design | ||
|
|
||
| Currently, `platform.json` is used for providing the expected structure of the platform components and interface details for supporting dynamic port breakout. | ||
|
|
||
| ### Platform capabilities field | ||
|
|
||
| A new set of `capabilities` fields are introduced in platform.json, for providing platform specific capablities on control and characteristics of the components. | ||
|
|
||
| For each component's attribute, the defined `capabilities` fields are as follows: | ||
|
|
||
| - "controllable" : A boolean, 'true' if the given attribute can be controlled from the NOS, 'false' otherwise. Defaults to 'true'. | ||
| - Attribute specific fields: | ||
| - status led - "color" - A list of the supported colors. | ||
| - speed | ||
| - "minimum" - Minimum recommended fan speed that can be set. | ||
| - "maximum" - Maximum recommended fan speed that can be set. | ||
|
|
||
| Sample `capabilities` fields: | ||
|
|
||
| ``` | ||
| { | ||
| "chassis": { | ||
| "name": "PLATFORM", | ||
| "status_led": { | ||
| "controllable": true, | ||
| "colors": ["off", "amber", "green"] | ||
| }, | ||
| "fan_drawers":[ | ||
| { | ||
| "name": "FanTray1", | ||
| "status_led": { | ||
| "controllable": true, | ||
| "colors": ["red", "green"] | ||
| } | ||
| "fans": [ | ||
| { | ||
| "name": "FanTray1-Fan", | ||
| "speed": { | ||
| "controllable": true, | ||
| "minimum": 40, | ||
| "maximum": 100 | ||
| } | ||
| } | ||
| ] | ||
| }, | ||
| { | ||
| "name": "FanTray2", | ||
| "status_led": { | ||
| "controllable": true, | ||
| "colors": ["red", "green"] | ||
| } | ||
| "fans": [ | ||
| { | ||
| "name": "FanTray2-Fan", | ||
| "speed": { | ||
| "controllable": true, | ||
| "minimum": 40, | ||
| "maximum": 100 | ||
| } | ||
| } | ||
| ] | ||
| } | ||
| ], | ||
| "psus": [ | ||
| { | ||
| "name": "PSU1", | ||
| "status_led": { | ||
| "controllable": false | ||
| } | ||
| "fans": [ | ||
| { | ||
| "name": "PSU1 Fan", | ||
| "speed": { | ||
| "controllable": false | ||
| } | ||
| } | ||
| ], | ||
| } | ||
| ], | ||
| "thermals": [ | ||
| { | ||
| "name": "Thermal 1", | ||
| "controllable": false | ||
| }, | ||
| { | ||
| "name": "Thermal 2", | ||
| "controllable": false | ||
| }, | ||
| ], | ||
|
|
||
| ... | ||
| } | ||
| ``` | ||
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please elaborate on who will be the consumer of this info? Also as platform facts for the test cases?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently, these fields would be used as platform facts for platform API test suite.
But, these can also be consulted by any platform daemon before executing the respective APIs.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since platform daemons should be using the platform API, I think some of these things should just be taken care of by vendor-side logic. Like the fan speeds are completely handled by platform API instead of thermalctld.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@zzhiyuan: I'm not sure I understand your comment. The main usage of these values is for testing purposes to understand boundaries when testing the device. Can you please elaborate?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I meant more for the latter part, "these can also be consulted by any platform daemon before executing the respective APIs."
Since there is already the fantray abstraction, I believe it is better for platform vendor to control the min/max fan speed which may differ by model, via thermal manager. Having values in platform.json for purpose of testing is fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah. Yes, I agree with you. Platform daemons really shouldn't be referring to these values.