Skip to content
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

Create TRANSCEIVER_DOM_THRESHOLD table in state DB #320

Merged
merged 3 commits into from
Dec 6, 2022

Conversation

mihirpat1
Copy link
Contributor

@mihirpat1 mihirpat1 commented Nov 29, 2022

Description

This PR is adding the required changes for creating a table for DOM threshold in state DB. Please refer to CMIS_and_C-CMIS_support_for_ZR.md document for field definitions for TRANSCEIVER_DOM_THRESHOLD.
Overall, TRANSCEIVER_DOM_SENSOR DB table currently comprised of fields for TRANSCEIVER_DOM_SENSOR as well as TRANSCEIVER_DOM_THRESHOLD. With the current PR, a separate entry in state DB has now been created for TRANSCEIVER_DOM_THRESHOLD related fields.
The new command to display the threshold relevant fields will be done separately and is currently being tracked using sonic-net/sonic-buildimage#12911.

PR sonic-net/sonic-utilities#2535 has now been created to ensure that the output from "show interface transceiver eeprom <port> -d" remains unchanged after the current PR is merged. Both the current PR and sonic-net/sonic-utilities#2535 needs to be merged at the same time to avoid running into issues since they are dependent.

Motivation and Context

How Has This Been Tested?

400ZR related testing
Unit-test_400ZR.txt

100G and 10G related testing
Unit-test_100G_10G.txt

Transceiver info o/p
Unit-test_transceiver_info.txt

400ZR UT for multi-asic switch
Unit-test_400ZR_Multiasic.txt

Additional Information (Optional)

Signed-off-by: Mihir Patel [email protected]

@mihirpat1 mihirpat1 marked this pull request as ready for review November 30, 2022 02:41
@mihirpat1 mihirpat1 requested a review from prgeor November 30, 2022 02:41
@shyam77git
Copy link

@mihirpat1
Looked into the UT logs.
Don't see output/display for 'show int transceiver eeprom' CLIs with this change.
Could you add them?

@mihirpat1
Copy link
Contributor Author

@mihirpat1 Looked into the UT logs. Don't see output/display for 'show int transceiver eeprom' CLIs with this change. Could you add them?

The current PR is only for supporting the state DB related changes. I have filed sonic-net/sonic-buildimage#12911 to track the CLI support for the same.

@jaganbal-a
Copy link

jaganbal-a commented Dec 1, 2022

I see the o/p of "sfpshow eeprom -d -p port_name" have "ModuleThresholdValues:" and looks incomplete.
Do you want to keep the CLI o/p this way until sonic-net/sonic-buildimage#12911 fix commit?
ModuleMonitorValues:
Temperature: 73.91C
Vcc: 3.347Volts
ModuleThresholdValues:

root@sonic:/home/admin# 

@mihirpat1
Copy link
Contributor Author

I see the o/p of "sfpshow eeprom -d -p port_name" have "ModuleThresholdValues:" and looks incomplete. Do you want to keep the CLI o/p this way until sonic-net/sonic-buildimage#12911 fix commit? ModuleMonitorValues: Temperature: 73.91C Vcc: 3.347Volts ModuleThresholdValues:

root@sonic:/home/admin# 

Yes, the corresponding changes will be done with the issue mentioned above

@jaganbal-a
Copy link

I see the o/p of "sfpshow eeprom -d -p port_name" have "ModuleThresholdValues:" and looks incomplete. Do you want to keep the CLI o/p this way until sonic-net/sonic-buildimage#12911 fix commit? ModuleMonitorValues: Temperature: 73.91C Vcc: 3.347Volts ModuleThresholdValues:

root@sonic:/home/admin# 

Yes, the corresponding changes will be done with the issue mentioned above

the show int trans CLI o/p will have the Module threshold values right?
otherwise it will be difficult to debug any signal out of range scenario.

@mihirpat1
Copy link
Contributor Author

I see the o/p of "sfpshow eeprom -d -p port_name" have "ModuleThresholdValues:" and looks incomplete. Do you want to keep the CLI o/p this way until sonic-net/sonic-buildimage#12911 fix commit? ModuleMonitorValues: Temperature: 73.91C Vcc: 3.347Volts ModuleThresholdValues:

root@sonic:/home/admin# 

Yes, the corresponding changes will be done with the issue mentioned above

the show int trans CLI o/p will have the Module threshold values right? otherwise it will be difficult to debug any signal out of range scenario.

Need access since part of SONiC teamI have now raised sonic-net/sonic-utilities#2535 to ensure that the CLI output remains unchanged with the current changes. We plan to commit both this and sonic-net/sonic-utilities#2535 at the same time to avoid running into dependency issues.

@mihirpat1 mihirpat1 changed the title Create state DB for TRANSCEIVER_DOM_THRESHOLD Create TRANSCEIVER_DOM_THRESHOLD table in state DB Dec 2, 2022
@shyam77git
Copy link

@mihirpat1
Have added comment to PR sonic-net/sonic-utilities#2535. Please take care of it.
Approving this one

Copy link

@shyam77git shyam77git left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mihirpat1
Copy link
Contributor Author

@mihirpat1 Have added comment to PR sonic-net/sonic-utilities#2535. Please take care of it. Approving this one

I have added the requested UT enclosure to this PR now.

@prgeor prgeor added the xcvrd label Dec 6, 2022
@prgeor prgeor merged commit adcd69b into sonic-net:master Dec 6, 2022
StormLiangMS pushed a commit that referenced this pull request Jan 5, 2023
* Create TRANSCEIVER_DOM_THRESHOLD table in state DB

Signed-off-by: Mihir Patel <[email protected]>

* Removed debug statement

Signed-off-by: Mihir Patel <[email protected]>

* Fixed test failure

Signed-off-by: Mihir Patel <[email protected]>

Signed-off-by: Mihir Patel <[email protected]>
yxieca pushed a commit that referenced this pull request Jan 19, 2023
* Create TRANSCEIVER_DOM_THRESHOLD table in state DB

Signed-off-by: Mihir Patel <[email protected]>

* Removed debug statement

Signed-off-by: Mihir Patel <[email protected]>

* Fixed test failure

Signed-off-by: Mihir Patel <[email protected]>

Signed-off-by: Mihir Patel <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants