Skip to content

[sfputil] Use host lane mask as part of rx-output enable/disable#3911

Merged
prgeor merged 7 commits intosonic-net:masterfrom
mihirpat1:debug_tx_rx_output
Jun 13, 2025
Merged

[sfputil] Use host lane mask as part of rx-output enable/disable#3911
prgeor merged 7 commits intosonic-net:masterfrom
mihirpat1:debug_tx_rx_output

Conversation

@mihirpat1
Copy link
Copy Markdown
Contributor

@mihirpat1 mihirpat1 commented Jun 10, 2025

What I did

We need to use host lane mask instead of media lane mask as part of sfputil debug rx-output EthernetXX enable/disable CLI.
In addition to the above, following needs to be enhanced

  1. Need to fail CLI if the tx_disable_channel or rx_disable_channel fails
  2. Verify the Rx and Tx output status after enabling/disabling Rx/Tx
  3. Modify get_subport function to return subport as 1 if the subport field is not found in PORT table of CONFIG_DB

How I did it

Addressed the above issues in What I did section

How to verify it

  1. Verified rx-output disable and enable
  2. Verified tx-output disable and enable
  3. Verified rx-output disable and enable CLI on a device which did not have subport as part of the PORT table in CONFIG_DB

Previous command output (if the output of a command-line utility has changed)

The CLI used to fail if subport field is not defined in the PORT table of CONFIG_DB

root@sonic:/home/admin# sfputil debug rx-output Ethernet144 enable 
Ethernet144: subport is not present in CONFIG_DB

root@sonic:/home/admin# redis-cli -n 4 hgetall "PORT|Ethernet144"
 1) "admin_status"
 2) "up"
 3) "alias"
 4) "Ethernet19/1"
 5) "index"
 6) "19"
 7) "lanes"
 8) "145,146,147,148,149,150,151,152"
 9) "mtu"
10) "9100"
11) "speed"
12) "400000"
root@sonic:/home/admin# 

New command output (if the output of a command-line utility has changed)

With the new changes, the CLI is now supported

root@sonic:/home/admin# sfputil debug rx-output Ethernet144 enable 
Field 'subport' not found in table 'PORT' for key 'Ethernet144' in CONFIG_DB.
Ethernet144: RX output enabled on subport 1
root@sonic:/home/admin# 

@mihirpat1 mihirpat1 requested a review from Copilot June 10, 2025 00:44
@mssonicbld
Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

This comment was marked as outdated.

@mssonicbld
Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@mihirpat1 mihirpat1 requested a review from Copilot June 10, 2025 17:13

This comment was marked as outdated.

@mssonicbld
Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@mihirpat1 mihirpat1 requested a review from Copilot June 10, 2025 17:29
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR updates the sfputil debug commands to use the correct host lane mask for RX output and adds robust failure detection, status verification, and a default subport fallback.

  • Handle empty string values when retrieving host/media lane counts
  • Enhance set_output to verify TX/RX output status after enable/disable, restore on failure, and exit on errors
  • Default missing subport in CONFIG_DB to 1 and refactor tests into a parameterized suite

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
utilities_common/platform_sfputil_helper.py Added checks for empty lane count values and improved error handling when a field is missing
sfputil/debug.py Switched to host lane counts for RX, implemented post-action status checks, and error recovery
tests/sfputil_test.py Consolidated TX/RX output tests into a single parameterized test covering enable/disable scenarios
Comments suppressed due to low confidence (3)

utilities_common/platform_sfputil_helper.py:193

  • Fix the spelling of 'retreive' to 'retrieve'.
click.echo(f"{port_name}: unable to retreive correct host lane count")

utilities_common/platform_sfputil_helper.py:205

  • Fix the spelling of 'retreive' to 'retrieve'.
click.echo(f"{port_name}: unable to retreive correct media lane count")

tests/sfputil_test.py:1824

  • Add a test case for when sfp.get_xcvr_api() raises NotImplementedError to verify the CLI exits with ERROR_NOT_IMPLEMENTED and prints the expected message.
@pytest.mark.parametrize(

@mihirpat1 mihirpat1 marked this pull request as ready for review June 10, 2025 18:15
@mssonicbld
Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@mihirpat1 mihirpat1 requested review from prgeor and vdahiya12 June 10, 2025 18:37
@mssonicbld
Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Copy Markdown
Collaborator

Cherry-pick PR to msft-202412: Azure/sonic-utilities.msft#185

nmoray pushed a commit to nmoray/sonic-utilities that referenced this pull request Jun 25, 2025
…ic-net#3911)

* [sfputil] Use host lane mask as part of rx-output enable/disable

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

* Fixed pre-commit failures

* Fixed pre-commit failure

* Modified the state restore logic in case of failure

* Removed output restoration in case of an error

* Modified get_subport to return 0 if subport field does not exists

* Returning integer in get_subport

---------

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

Cherry-pick PR to 202505: #4018

YairRaviv pushed a commit to YairRaviv/sonic-utilities that referenced this pull request Jan 12, 2026
…ic-net#3911)

* [sfputil] Use host lane mask as part of rx-output enable/disable

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

* Fixed pre-commit failures

* Fixed pre-commit failure

* Modified the state restore logic in case of failure

* Removed output restoration in case of an error

* Modified get_subport to return 0 if subport field does not exists

* Returning integer in get_subport

---------

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.

7 participants