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

[CMIS] Catch Exception to avoid CMIS code crash #299

Merged
merged 2 commits into from
Aug 20, 2022

Conversation

keboliu
Copy link
Collaborator

@keboliu keboliu commented Aug 11, 2022

Signed-off-by: Kebo Liu kebol@nvidia.com

Description

To protect the code from crashing by catching the exception

Motivation and Context

This is to fix the issue sonic-net/sonic-buildimage#11525

How Has This Been Tested?

run sonic-mgmt sfp reset test case: platform_tests/sfp/test_sfputil.py::test_check_sfputil_reset to make sure xcvrd will not crash

Additional Information (Optional)

Signed-off-by: Kebo Liu <kebol@nvidia.com>
@keboliu keboliu marked this pull request as draft August 11, 2022 02:48
@keboliu keboliu marked this pull request as ready for review August 11, 2022 10:32
stephenxs
stephenxs previously approved these changes Aug 12, 2022
@liat-grozovik
Copy link
Collaborator

@prgeor this is critical fix for 202205. can you please approve?

@prgeor
Copy link
Collaborator

prgeor commented Aug 17, 2022

@keboliu could you paste the exception backtrace in the description?

@prgeor prgeor self-assigned this Aug 17, 2022
@prgeor prgeor added the Bug label Aug 17, 2022
self.vdm_dict = self.get_vdm()
try:
bulk_status['laser_temperature'] = laser_temp_dict['monitor value']
Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

not sure I understand, I moved bulk_status['laser_temperature'] = laser_temp_dict['monitor value'] to try section to avoid the case that laser_temp_dict is none

@@ -50,7 +50,9 @@ def get_vdm_page(self, page, VDM_flag_page):
if page not in [0x20, 0x21, 0x22, 0x23]:
raise ValueError('Page not in VDM Descriptor range!')
vdm_descriptor = self.xcvr_eeprom.read_raw(page * PAGE_SIZE + PAGE_OFFSET, PAGE_SIZE)

if not vdm_descriptor:
Copy link
Collaborator

Choose a reason for hiding this comment

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

There are other eeprom read in this function. can you take care of them also?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

Signed-off-by: Kebo Liu <kebol@nvidia.com>
@prgeor prgeor merged commit 211d6bc into sonic-net:master Aug 20, 2022
yxieca pushed a commit that referenced this pull request Aug 26, 2022
* catch Exception to avoid CMIS code crash

Signed-off-by: Kebo Liu <kebol@nvidia.com>

* fix review comments, add more UT test

Signed-off-by: Kebo Liu <kebol@nvidia.com>

Signed-off-by: Kebo Liu <kebol@nvidia.com>
@keboliu keboliu deleted the cmis_exception_fix branch August 29, 2022 08:18
dprital added a commit to dprital/sonic-buildimage that referenced this pull request Aug 29, 2022
Update sonic-platform-common submodule pointer to include the following:
* [CMIS] Catch Exception to avoid CMIS code crash ([sonic-net#299](sonic-net/sonic-platform-common#299))
* [Credo][Ycable] fix incorrect uart statistics ([sonic-net#296](sonic-net/sonic-platform-common#296))
* Add PSU input voltage and input current ([sonic-net#295](sonic-net/sonic-platform-common#295))

Signed-off-by: dprital <drorp@nvidia.com>
dprital added a commit to dprital/sonic-buildimage that referenced this pull request Sep 4, 2022
Update sonic-platform-common submodule pointer to include the following:
* [CMIS] 'get_transceiver_info' should return 'None' when CMIS cable EEPROM is not ready  ([sonic-net#305](sonic-net/sonic-platform-common#305))
* uplift code coverage 80% ([sonic-net#307](sonic-net/sonic-platform-common#307))
* [sonic-pcie] Add UT for pcie_common.py ([sonic-net#293](sonic-net/sonic-platform-common#293))
* [CMIS] Catch Exception to avoid CMIS code crash ([sonic-net#299](sonic-net/sonic-platform-common#299))
* [Credo][Ycable] fix incorrect uart statistics ([sonic-net#296](sonic-net/sonic-platform-common#296))
* Add PSU input voltage and input current ([sonic-net#295](sonic-net/sonic-platform-common#295))

Signed-off-by: dprital <drorp@nvidia.com>
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