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

[Mellanox] Enable get_rx_los API support in CMIS cable host mgmt mode #20743

Merged
merged 2 commits into from
Nov 13, 2024

Conversation

mihirpat1
Copy link
Contributor

@mihirpat1 mihirpat1 commented Nov 8, 2024

Why I did it

The show int transceiver status PORT CLI always displays Rx LOS as False for all the lanes.

root@sonic:/home/admin# show int transceiver status Ethernet0 | grep "Rx loss"
        Rx loss of signal flag on media lane 1: False
        Rx loss of signal flag on media lane 2: False
        Rx loss of signal flag on media lane 3: False
        Rx loss of signal flag on media lane 4: False
        Rx loss of signal flag on media lane 5: False
        Rx loss of signal flag on media lane 6: False
        Rx loss of signal flag on media lane 7: False
        Rx loss of signal flag on media lane 8: False
root@sonic:/home/admin# 
Work item tracking
  • Microsoft ADO (number only): 30180403

How I did it

On Mellanox platforms, the get_rx_los API is overridden through the platform implementation (

) rather than using the implementation defined through cmis.py (https://github.com/sonic-net/sonic-platform-common/blob/59babf59f9aa611b84082896391b01a39ffcb866/sonic_platform_base/sonic_xcvr/api/public/cmis.py#L404).

The Mellanox platform is overriding the get_rx_los due to the below code

if self._xcvr_api is not None:
self._xcvr_api.get_rx_los = self.get_rx_los

The current platform implementation assigns Rx LOS as False for all the lanes (irrespective of the actual status on the module) which in turn causes the show int transceiver status PORT CLI to display the Rx LOS as False for all the lanes.

Hence, removing the overriding part to ensure that the API is not overridden by the platform.

How to verify it

Verified the CLI on first 2 lanes which are in admin disabled state from the peer side.

root@sonic:/home/admin# show int transceiver status Ethernet0 | grep "Rx loss"
        Rx loss of signal flag on media lane 1: False
        Rx loss of signal flag on media lane 2: False
        Rx loss of signal flag on media lane 3: True
        Rx loss of signal flag on media lane 4: True
        Rx loss of signal flag on media lane 5: True
        Rx loss of signal flag on media lane 6: True
        Rx loss of signal flag on media lane 7: True
        Rx loss of signal flag on media lane 8: True
root@sonic:/home/admin# 

Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111
  • 202205
  • 202211
  • 202305

Tested branch (Please provide the tested image version)

Description for the changelog

Link to config_db schema for YANG module changes

A picture of a cute animal (not mandatory but encouraged)

prgeor
prgeor previously approved these changes Nov 8, 2024
@prgeor
Copy link
Contributor

prgeor commented Nov 8, 2024

@liat-grozovik @Junchao-Mellanox can you also review

Copy link
Collaborator

@keboliu keboliu left a comment

Choose a reason for hiding this comment

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

There is a limitation on the Mellanox platform to support RX LOS reading in FW control mode, this change will also enable the RX LOS reading in FW control mode, we should have a logic to differentiate between FW control mode and host mgmt mode. @Junchao-Mellanox

@Junchao-Mellanox
Copy link
Collaborator

There is a limitation on the Mellanox platform to support RX LOS reading in FW control mode, this change will also enable the RX LOS reading in FW control mode, we should have a logic to differentiate between FW control mode and host mgmt mode. @Junchao-Mellanox

Thanks for catching this. Indeed, we need do something like following:

def get_rx_los(self):
        """Accessing rx los is not supproted, return all False

        Returns:
            list: [False] * channels
        """
        api = self.get_xcvr_api()
        if self.is_sw_control():
                return api.get_rx_los()
        return [False] * api.NUM_CHANNELS if api else None

@mihirpat1
Copy link
Contributor Author

we should have a logic to differentiate between FW control mode and host mgmt mode. @Junchao-Mellanox

@Junchao-Mellanox @keboliu I have now addressed this. Can you please help in reviewing it?

@mihirpat1
Copy link
Contributor Author

@liat-grozovik Can you please help in merging this PR?

@liat-grozovik liat-grozovik merged commit 8dd1d98 into sonic-net:master Nov 13, 2024
12 checks passed
@mihirpat1
Copy link
Contributor Author

@bingwang-ms @yxieca Can you please help in merging this to 202405 and 202311?
MSFT ADO - 30180403

mssonicbld pushed a commit to mssonicbld/sonic-buildimage that referenced this pull request Nov 13, 2024
…sonic-net#20743)

- Why I did it
The show int transceiver status PORT CLI always displays Rx LOS as False for all the lanes.

- How I did it
The current platform implementation assigns Rx LOS as False for all the lanes (irrespective of the actual status on the module) which in turn causes the show int transceiver status PORT CLI to display the Rx LOS as False for all the lanes.

Hence, removing the overriding part to ensure that the API is not overridden by the platform.

- How to verify it
Verified the CLI on first 2 lanes which are in admin disabled state from the peer side.

---------
Signed-off-by: Mihir Patel <[email protected]>
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202405: #20790

mssonicbld pushed a commit that referenced this pull request Nov 14, 2024
…#20743)

- Why I did it
The show int transceiver status PORT CLI always displays Rx LOS as False for all the lanes.

- How I did it
The current platform implementation assigns Rx LOS as False for all the lanes (irrespective of the actual status on the module) which in turn causes the show int transceiver status PORT CLI to display the Rx LOS as False for all the lanes.

Hence, removing the overriding part to ensure that the API is not overridden by the platform.

- How to verify it
Verified the CLI on first 2 lanes which are in admin disabled state from the peer side.

---------
Signed-off-by: Mihir Patel <[email protected]>
mssonicbld pushed a commit to mssonicbld/sonic-buildimage that referenced this pull request Nov 14, 2024
…sonic-net#20743)

- Why I did it
The show int transceiver status PORT CLI always displays Rx LOS as False for all the lanes.

- How I did it
The current platform implementation assigns Rx LOS as False for all the lanes (irrespective of the actual status on the module) which in turn causes the show int transceiver status PORT CLI to display the Rx LOS as False for all the lanes.

Hence, removing the overriding part to ensure that the API is not overridden by the platform.

- How to verify it
Verified the CLI on first 2 lanes which are in admin disabled state from the peer side.

---------
Signed-off-by: Mihir Patel <[email protected]>
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202311: #20805

mssonicbld pushed a commit that referenced this pull request Nov 14, 2024
…#20743)

- Why I did it
The show int transceiver status PORT CLI always displays Rx LOS as False for all the lanes.

- How I did it
The current platform implementation assigns Rx LOS as False for all the lanes (irrespective of the actual status on the module) which in turn causes the show int transceiver status PORT CLI to display the Rx LOS as False for all the lanes.

Hence, removing the overriding part to ensure that the API is not overridden by the platform.

- How to verify it
Verified the CLI on first 2 lanes which are in admin disabled state from the peer side.

---------
Signed-off-by: Mihir Patel <[email protected]>
aidan-gallagher pushed a commit to aidan-gallagher/sonic-buildimage that referenced this pull request Nov 16, 2024
…sonic-net#20743)

- Why I did it
The show int transceiver status PORT CLI always displays Rx LOS as False for all the lanes.

- How I did it
The current platform implementation assigns Rx LOS as False for all the lanes (irrespective of the actual status on the module) which in turn causes the show int transceiver status PORT CLI to display the Rx LOS as False for all the lanes.

Hence, removing the overriding part to ensure that the API is not overridden by the platform.

- How to verify it
Verified the CLI on first 2 lanes which are in admin disabled state from the peer side.

---------
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.

8 participants