-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
[Mellanox] Add bitmap support for SFP error event #7605
Conversation
This is pending on the discussion at stephenxs/SONiC#12, with regard to making this a generic feature, if possible. |
@Junchao-Mellanox now that it is not single value how the user is expected to understand the errors? should we add some information in the CLI? we cannot assume users has the code :-) Please confirm which HLD we should refer in this PR. Is it sonic-net/SONiC#586 ? |
1. Update get_change_event - Return event in a bitmap format in case there is an error - Return an extra map indexed by sfp_error in case there is a Mellanox-specific error - Indicate whether the error is a blocking error 2. Expose the error status of SFP modules to CLI - A platform API calls SDK API to fetch the error status - Display the SFP module state: plugged, unplugged and plugged with error. - The SFP error state is displayed only if the SFP module state is plugged with error - The CLI will call platform API and provide user-friendly output 3. Redirect stderr to /dev/null in order to eliminate errors in case SFP module is not plugged 4. Beautify the code, making all the state/error/descriptions defined in a uniform place Signed-off-by: Stephen Sun <[email protected]>
Signed-off-by: Stephen Sun <[email protected]>
Signed-off-by: Stephen Sun <[email protected]>
Support handling the error status of SFP modules
This pull request introduces 4 alerts when merging cb5135f into 62a4603 - view on LGTM.com new alerts:
|
The LGTM warnings are not real issue. It looks like a bug of LGTM. |
Stephen has updated the HLD, and user could get the error status from CLI. |
Pending on PR sonic-net/sonic-platform-common#194 to be merged first. |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
@jleveque can you review the PR when you have time? thanks. |
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.
Looks good to me.
sonic-net/sonic-platform-daemons#184 has merged. Please update the submodule, then we can merge this PR.
Thank you. Will do. |
advancing submodule PR: #7961 |
#### Why I did it Currently, SONiC use a single value to represent SFP error, however, multiple SFP errors could exist at the same time. This PR is aimed to support it #### How I did it Return bitmap instead of single value when a SFP event occurs Signed-off-by: Stephen Sun <[email protected]>
Depends on sonic-net/sonic-platform-daemons#184
Why I did it
Currently, SONiC use a single value to represent SFP error, however, multiple SFP errors could exist at the same time. This PR is aimed to support it
How I did it
Return bitmap instead of single value when a SFP event occurs
How to verify it
Which release branch to backport (provide reason below if selected)
Description for the changelog
A picture of a cute animal (not mandatory but encouraged)