-
Notifications
You must be signed in to change notification settings - Fork 664
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
[sfpshow/sfputil] Enhance sfpshow and sfputil to behavior correctly on RJ45 ports #2111
[sfpshow/sfputil] Enhance sfpshow and sfputil to behavior correctly on RJ45 ports #2111
Conversation
Signed-off-by: Kebo Liu <kebol@nvidia.com>
Signed-off-by: Kebo Liu <kebol@nvidia.com>
Signed-off-by: Kebo Liu <kebol@nvidia.com>
Signed-off-by: Kebo Liu <kebol@nvidia.com>
Signed-off-by: Stephen Sun <stephens@nvidia.com>
Signed-off-by: Stephen Sun <stephens@nvidia.com>
Signed-off-by: Stephen Sun <stephens@nvidia.com>
Signed-off-by: Stephen Sun <stephens@nvidia.com>
Signed-off-by: Stephen Sun <stephens@nvidia.com>
Signed-off-by: Kebo Liu <kebol@nvidia.com>
This pull request introduces 1 alert when merging cc540ed into 61b1396 - view on LGTM.com new alerts:
|
The building errors are caused by dependent mock dbs in #2110 has not yet been merged. |
Signed-off-by: Kebo Liu <kebol@nvidia.com>
/azpw run |
/AzurePipelines run |
Azure Pipelines successfully started running 1 pipeline(s). |
platform API will detect whether RJ45 port is present or not, and push the result into TRANSCEIVER_STATUS table.
|
According to the current logic, an SFP's info will be removed from TRANSCEIVER_INFO table once the port is absent (unplugged). In that case, we will lost the "type" information of the port.
|
@prgeor
By doing so,
|
@prgeor any additional points to discuss in order to signoff this PR? |
…port (#17) * Revert the logic to fetch presence status from error status Signed-off-by: Stephen Sun <stephens@nvidia.com> * Unit test Signed-off-by: Stephen Sun <stephens@nvidia.com> * Fix error Signed-off-by: Stephen Sun <stephens@nvidia.com>
@prgeor For now, we will follow the current solution to fetch the port type of RJ45 - reading it from |
Signed-off-by: Stephen Sun <stephens@nvidia.com>
This is not available anymore. it's the previous impelmetnation. |
def is_rj45_port_from_db(port_name, db): | ||
intf_type = db.get(db.STATE_DB, 'TRANSCEIVER_INFO|{}'.format(port_name), 'type') | ||
return intf_type == RJ45_PORT_TYPE | ||
|
||
|
||
def is_rj45_port_from_api(port_name): |
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.
@keboliu can put comments in the code why we need two different functions for getting the port type? Also when should one use each of them?
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.
@prgeor please check the latest commits for the comments on the usage of the functions.
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.
@keboliu as we discussed, lets have sfptuil always fetch the values by reading the eeprom and sfpshow utility use the DB to read the values. Hope this will be scoped and fixed in future PR.
Signed-off-by: Kebo Liu <kebol@nvidia.com>
e8eacb3
to
09a891a
Compare
Signed-off-by: Kebo Liu <kebol@nvidia.com>
Signed-off-by: Kebo Liu <kebol@nvidia.com>
…n RJ45 ports (#2111) * enhance show interface transceiver eeprom logic with RJ45 port support Signed-off-by: Kebo Liu <kebol@nvidia.com> * enhance sfputil to support RJ45 port, exclude error status * fix sfputil issue on RJ45 port Signed-off-by: Kebo Liu <kebol@nvidia.com> * [RJ45] change the way to judge port type and add more UT test case Signed-off-by: Kebo Liu <kebol@nvidia.com> * [sfputil] simplity the logic for RJ45 support Signed-off-by: Kebo Liu <kebol@nvidia.com> * Support sfputil show present Signed-off-by: Stephen Sun <stephens@nvidia.com> * Support rj45 in sfpshow Signed-off-by: Stephen Sun <stephens@nvidia.com> * Add test case for sfputil with RJ45 supported Signed-off-by: Stephen Sun <stephens@nvidia.com> * Add mock data for RJ45 ports into STATE_DB Signed-off-by: Stephen Sun <stephens@nvidia.com> * Add test for sfputil show for RJ45 ports Signed-off-by: Stephen Sun <stephens@nvidia.com> * remove debug code in sfputil test case Signed-off-by: Kebo Liu <kebol@nvidia.com> * remove unnecessary argument for format() Signed-off-by: Kebo Liu <kebol@nvidia.com> * Revert the logic to fetch presence status from error status for RJ45 port (#17) * Revert the logic to fetch presence status from error status Signed-off-by: Stephen Sun <stephens@nvidia.com> * Unit test Signed-off-by: Stephen Sun <stephens@nvidia.com> * Fix error Signed-off-by: Stephen Sun <stephens@nvidia.com> * Add test cases to cover lpmode and error status Signed-off-by: Stephen Sun <stephens@nvidia.com> * add comments to describe the usage of functions to judge the port type Signed-off-by: Kebo Liu <kebol@nvidia.com> * add more testcase for sfputil Signed-off-by: Kebo Liu <kebol@nvidia.com> * fix typo in testcase name Signed-off-by: Kebo Liu <kebol@nvidia.com> Co-authored-by: Stephen Sun <stephens@nvidia.com> Co-authored-by: Stephen Sun <5379172+stephenxs@users.noreply.github.com>
What I did
For RJ45 port, sfpshow and sfputil need to be enhanced to behavior correctly:
How I did it
check the port type, if it's RJ45 ports then handle it seperately.
How to verify it
run above command on platform with RJ45 ports
Previous command output (if the output of a command-line utility has changed)
New command output (if the output of a command-line utility has changed)