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

Fix issues in cmis.get_transceiver_bulk_status #351

Merged
merged 4 commits into from
Mar 15, 2023

Conversation

stephenxs
Copy link
Collaborator

@stephenxs stephenxs commented Mar 6, 2023

Description

Fix issues in cmis.get_transceiver_bulk_status

  1. In case it fails to read EEPROM, either self.get_rx_power() or self.get_tx_power() can be a list of 'N/A'. Need to test it before calling self.mw_to_dbm otherwise exception TypeError("'<' not supported between instances of 'str' and 'int'") will be thrown.
    Exception occured at DomInfoUpdateTask thread due to TypeError("'<' not supported between instances of 'str' and 'int'")
    Traceback (most recent call last):
    File "/usr/local/lib/python3.9/dist-packages/xcvrd/xcvrd.py", line 1696, in run
        self.task_worker()
    File "/usr/local/lib/python3.9/dist-packages/xcvrd/xcvrd.py", line 1681, in task_worker
        post_port_dom_info_to_db(logical_port_name, self.port_mapping, self.xcvr_table_helper.get_dom_tbl(asic_index), self.task_stopping_event, dom_info_cache=dom_info_cache)
    File "/usr/local/lib/python3.9/dist-packages/xcvrd/xcvrd.py", line 503, in post_port_dom_info_to_db
        dom_info_dict = _wrapper_get_transceiver_dom_info(physical_port)
    File "/usr/local/lib/python3.9/dist-packages/xcvrd/xcvrd.py", line 181, in _wrapper_get_transceiver_dom_info
        return platform_chassis.get_sfp(physical_port).get_transceiver_bulk_status()
    File "/usr/local/lib/python3.9/dist-packages/sonic_platform_base/sonic_xcvr/sfp_optoe_base.py", line 28, in get_transceiver_bulk_status
        return api.get_transceiver_bulk_status() if api is not None else None
    File "/usr/local/lib/python3.9/dist-packages/sonic_platform_base/sonic_xcvr/api/public/cmis.py", line 211, in get_transceiver_bulk_status
        bulk_status["rx%dpower" % i] = float("{:.3f}".format(self.mw_to_dbm(rx_power[i - 1]))) if self.get_rx_power_support() else 'N/A'
    File "/usr/local/lib/python3.9/dist-packages/sonic_platform_base/sonic_xcvr/api/xcvr_api.py", line 16, in mw_to_dbm
        elif mW < 0:
    TypeError: '<' not supported between instances of 'str' and 'int'
    Xcvrd: exception found at child thread DomInfoUpdateTask due to TypeError("'<' not supported between instances of 'str' and 'int'")
    
  2. It should be a valid case for either self.get_rx_power() or self.get_tx_power() to return None. Handle other fields instead of returning None in this case

Motivation and Context

How Has This Been Tested?

Additional Information (Optional)

1. In case it fails to read EEPROM, either self.get_rx_power() or self.get_tx_power() can be a list of 'N/A'. Need to test it before calling self.mw_to_dbm
2. It should be a valid case for either self.get_rx_power() or self.get_tx_power() to return None. Handle other fields instead of returning None in this case

Signed-off-by: Stephen Sun <stephens@nvidia.com>
@stephenxs stephenxs changed the title Fix issue in cmis.get_transceiver_bulk_status Fix issues in cmis.get_transceiver_bulk_status Mar 6, 2023
@stephenxs
Copy link
Collaborator Author

It can be cleanly cherry-picked to both 202205 and 202211
202205:

commit 16bc0c01a5ac610606c839c1a4bf5204520f4047
Author: Stephen Sun <stephens@nvidia.com>
Date:   Mon Mar 6 04:07:02 2023 +0000

    Fix issue in cmis.get_transceiver_bulk_status

    1. In case it fails to read EEPROM, either self.get_rx_power() or self.get_tx_power() can be a list of 'N/A'. Need to test it before calling self.mw_to_dbm
    2. It should be a valid case for either self.get_rx_power() or self.get_tx_power() to return None. Handle other fields instead of returning None in this case

    Signed-off-by: Stephen Sun <stephens@nvidia.com>

commit 321a8e77131ce567ec015ba26912b4fd1d0b47e1 (origin/202205)
Author: CliveNi <clive.ni@cloudlight.com.hk>
Date:   Fri Sep 23 09:29:13 2022 +0800

    Cdb fw upgrade (#308)

    * [Cloudlight] QSFP-DD FW upgrade doesn't work (#257)

    - Description

202211:

commit 0430ae52a98d47daa05de347c0510fa3c0966869 (HEAD)
Author: Stephen Sun <stephens@nvidia.com>
Date:   Mon Mar 6 04:07:02 2023 +0000

    Fix issue in cmis.get_transceiver_bulk_status

    1. In case it fails to read EEPROM, either self.get_rx_power() or self.get_tx_power() can be a list of 'N/A'. Need to test it before calling self.mw_to_dbm
    2. It should be a valid case for either self.get_rx_power() or self.get_tx_power() to return None. Handle other fields instead of returning None in this case

    Signed-off-by: Stephen Sun <stephens@nvidia.com>

commit 2dbc0ea7ccecad1677e2f863cadcdfe451d8fd2a (origin/202211)
Author: mihirpat1 <112018033+mihirpat1@users.noreply.github.com>
Date:   Wed Jan 25 09:49:05 2023 -0800

    Change get_tx_bias return type to list (#342)

    * Change get_tx_bias return type to list

    Signed-off-by: Mihir Patel <patelmi@microsoft.com>

@stephenxs stephenxs marked this pull request as ready for review March 6, 2023 10:26
@liat-grozovik liat-grozovik requested a review from prgeor March 6, 2023 12:58
@liat-grozovik
Copy link
Collaborator

@prgeor could you please help review or assign someone?

@prgeor
Copy link
Collaborator

prgeor commented Mar 7, 2023

@stephenxs please provide more context in the description. What is failing? Which CLI? If the i2c read is failing then we should fix the optics or platform issue?

@stephenxs
Copy link
Collaborator Author

@stephenxs please provide more context in the description. What is failing? Which CLI? If the i2c read is failing then we should fix the optics or platform issue?

@prgeor Updated in the description.

keboliu
keboliu previously approved these changes Mar 7, 2023
@stephenxs stephenxs requested a review from prgeor March 8, 2023 01:09
@liat-grozovik
Copy link
Collaborator

@prgeor as this issue is for 202205 please review so we can have it in soon. thanks.

…ading failure

Signed-off-by: Stephen Sun <stephens@nvidia.com>
@stephenxs stephenxs dismissed stale reviews from keboliu and Junchao-Mellanox via f02c287 March 15, 2023 03:04
Signed-off-by: Stephen Sun <stephens@nvidia.com>
Signed-off-by: Stephen Sun <stephens@nvidia.com>
@prgeor prgeor merged commit b205400 into sonic-net:master Mar 15, 2023
@prgeor
Copy link
Collaborator

prgeor commented Mar 15, 2023

@yxieca please cherry pick to 202205

@prgeor
Copy link
Collaborator

prgeor commented Mar 15, 2023

@StormLiangMS please help cherry pick to 202211

yxieca pushed a commit that referenced this pull request Mar 15, 2023
* Fix issue in cmis.get_transceiver_bulk_status

1. In case it fails to read EEPROM, either self.get_rx_power() or self.get_tx_power() can be a list of 'N/A'. Need to test it before calling self.mw_to_dbm
2. It should be a valid case for either self.get_rx_power() or self.get_tx_power() to return None. Handle other fields instead of returning None in this case

Signed-off-by: Stephen Sun <stephens@nvidia.com>

* Address comments: distinguish scenarios between not supporting and reading failure

Signed-off-by: Stephen Sun <stephens@nvidia.com>

* Adjust unit test case

Signed-off-by: Stephen Sun <stephens@nvidia.com>

* Remove redundant code

Signed-off-by: Stephen Sun <stephens@nvidia.com>

---------

Signed-off-by: Stephen Sun <stephens@nvidia.com>
@stephenxs stephenxs deleted the fix-cmis-get-bulk branch March 15, 2023 23:37
StormLiangMS pushed a commit that referenced this pull request Mar 19, 2023
* Fix issue in cmis.get_transceiver_bulk_status

1. In case it fails to read EEPROM, either self.get_rx_power() or self.get_tx_power() can be a list of 'N/A'. Need to test it before calling self.mw_to_dbm
2. It should be a valid case for either self.get_rx_power() or self.get_tx_power() to return None. Handle other fields instead of returning None in this case

Signed-off-by: Stephen Sun <stephens@nvidia.com>

* Address comments: distinguish scenarios between not supporting and reading failure

Signed-off-by: Stephen Sun <stephens@nvidia.com>

* Adjust unit test case

Signed-off-by: Stephen Sun <stephens@nvidia.com>

* Remove redundant code

Signed-off-by: Stephen Sun <stephens@nvidia.com>

---------

Signed-off-by: Stephen Sun <stephens@nvidia.com>
@keboliu
Copy link
Collaborator

keboliu commented Mar 31, 2023

commits already included in 202211, so I add the label.

oleksandrivantsiv pushed a commit to oleksandrivantsiv/sonic-platform-common that referenced this pull request Oct 25, 2024
sonic-net#351)

* PSUD-Delete or update CHASSIS_INFO table PSU/Modules data if added or removed.

Signed-off-by: prem-nokia <premnath.saravanan@nokia.com>

* fix swsscommon.py hdel function.

* implemented code optimization.

Signed-off-by: premsara <premnath.saravanan@nokia.com>

---------

Signed-off-by: prem-nokia <premnath.saravanan@nokia.com>
Signed-off-by: premsara <premnath.saravanan@nokia.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.

6 participants