sonic-swss icon indicating copy to clipboard operation
sonic-swss copied to clipboard

Published 20 hours ago • verified publisher icon sonic-net

Add support for configuring loopback mode (PHY and MAC)

Open royyi8 opened this issue 10 months ago • 11 comments

What I did: Add a new port attribute loopback_mode to PORT_TABLE in CONFIG_DB. Portsorch will use the enum value to set the sai attribute SAI_PORT_ATTR_LOOPBACK_MODE.

Why I did it: This PR adds support for setting loopback mode for an interface in CONFIG DB. It can be set at runtime or during initialization in config_db.json.

How I verified it: Run component tests to set loopback mode in CONFIG DB PORT TABLE and check the corresponding loopback attribute is set in ASIC DB.

Details if related: There is no impact to warmboot since the PR adds support for new port loopback config.

royyi8 avatar Mar 29 '24 00:03 royyi8

Is there an HLD for this PR? If so, please link it. Modify the PR description to follow the template and give details of the changes. Please mention any impact on Warmboot.

prsunny avatar Apr 10 '24 22:04 prsunny

Nit: Update the title to reflect it is phy loopback

prsunny avatar Apr 10 '24 22:04 prsunny

Hi Prince, thanks for your review. I included more details for the feature below. I have not created an HLD yet, but please let me know if you would like me to open an HLD PR.

Overview: Interfaces on a switch can be configured into loopback mode, which is useful for port management and diagnostic purposes. This PR allows port loopback mode to be configured directly from CONFIG DB, and supports PHY and MAC (local/remote) loopback modes.

Design: This PR introduces a new port attribute (loopback_mode) that SWSS will use to configure loopback mode. The attribute can be specified in PORT TABLE in config_db.json to be processed during initialization, or set in PORT_TABLE CONFIG DB during runtime. Portsorch will set the attribute SAI_PORT_ATTR_LOOPBACK_MODE through sai_port_api->set_port_attribute according to the enum defined in the CONFIG DB port table.

CONFIG DB Schema:

PORT_TABLE:Ethernet0
"loopback_mode": "<enum_value>"

_sai_port_loopback_mode_t enum:

DB Schema SAI Attribute
none SAI_PORT_LOOPBACK_MODE_NONE
phy_local SAI_PORT_LOOPBACK_MODE_PHY
mac_local SAI_PORT_LOOPBACK_MODE_MAC
phy_remote SAI_PORT_LOOPBACK_MODE_PHY_REMOTE
mac_remote SAI_PORT_LOOPBACK_MODE_MAC_REMOTE

Warmboot: This PR has no impact on warmboot, since it adds support for new loopback port config.

Testing: The component tests verify that setting loopback mode in CONFIG DB port table results in the corresponding SAI attribute set in the ASIC DB.

royyi8 avatar Apr 13 '24 00:04 royyi8

Hi @prgeor @prsunny could you please review the PR again? Thanks!

royyi8 avatar Apr 24 '24 17:04 royyi8

Friendly ping for review @prgeor @prsunny

royyi8 avatar May 01 '24 01:05 royyi8

Friendly ping for review @prgeor @prsunny

royyi8 avatar May 06 '24 22:05 royyi8

@prgeor @prsunny Could you please let me know if you have more comments/suggestions? Thanks!

royyi8 avatar May 17 '24 22:05 royyi8

@royyi8 , can you please resolve conflict and rebase?

prsunny avatar May 23 '24 17:05 prsunny

@royyi8 , @prgeor , can you confirm there is a yang model defined for this new attribute?

@dgsudharsan for viz

prsunny avatar Jun 05 '24 19:06 prsunny

@royyi8 , @prgeor , can you confirm there is a yang model defined for this new attribute?

@dgsudharsan for viz

@prsunny, I have not made changes to the sonic yang model. Could you please let me know if the new attribute needs to be added in sonic-buildimage sonic-port.yang?

royyi8 avatar Jun 05 '24 23:06 royyi8

@royyi8 , @prgeor , can you confirm there is a yang model defined for this new attribute? @dgsudharsan for viz

@prsunny, I have not made changes to the sonic yang model. Could you please let me know if the new attribute needs to be added in sonic-buildimage sonic-port.yang?

Yes, also we need for the link-damping feature.

prsunny avatar Jun 06 '24 21:06 prsunny