From 0de47b2a87724d289f287759b9bfc8c63a419718 Mon Sep 17 00:00:00 2001 From: xinyu Date: Sat, 10 Aug 2024 10:22:26 +0800 Subject: [PATCH 1/9] [sfputil] Configure the debug loopback mode only on the relevant lanes of the logical port Signed-off-by: xinyu --- sfputil/main.py | 37 +++++++++++++++++++++++++++++++++++-- tests/sfputil_test.py | 14 ++++++++++++++ 2 files changed, 49 insertions(+), 2 deletions(-) diff --git a/sfputil/main.py b/sfputil/main.py index 2c8f85d016..c804eb0fb0 100644 --- a/sfputil/main.py +++ b/sfputil/main.py @@ -18,7 +18,7 @@ import sonic_platform import sonic_platform_base.sonic_sfp.sfputilhelper from sonic_platform_base.sfp_base import SfpBase -from swsscommon.swsscommon import SonicV2Connector +from swsscommon.swsscommon import SonicV2Connector, ConfigDBConnector from natsort import natsorted from sonic_py_common import device_info, logger, multi_asic from utilities_common.sfp_helper import covert_application_advertisement_to_output_string @@ -1991,11 +1991,44 @@ def loopback(port_name, loopback_mode): click.echo("{}: This functionality is not implemented".format(port_name)) sys.exit(ERROR_NOT_IMPLEMENTED) + namespaces = multi_asic.get_front_end_namespaces() + for namespace in namespaces: + config_db = ConfigDBConnector(use_unix_socket_path=True, namespace=namespace) + if config_db is not None: + config_db.connect() + subport = int(config_db.get(config_db.CONFIG_DB, + 'PORT|{}'.format(port_name), 'subport')) + else: + click.echo("Failed to connect to CONFIG_DB") + sys.exit(EXIT_FAIL) + + state_db = SonicV2Connector(use_unix_socket_path=False, namespace=namespace) + if state_db is not None: + state_db.connect(state_db.STATE_DB) + host_lane_count = int(state_db.get(state_db.STATE_DB, + 'TRANSCEIVER_INFO|{}'.format(port_name), + 'host_lane_count')) + media_lane_count = int(state_db.get(state_db.STATE_DB, + 'TRANSCEIVER_INFO|{}'.format(port_name), + 'media_lane_count')) + else: + click.echo("Failed to connect to STATE_DB") + sys.exit(EXIT_FAIL) + + if loopback_mode in ("host-side-input", "host-side-output"): + lane_mask = ((1 << host_lane_count) - 1) << ((subport - 1) * host_lane_count) + elif loopback_mode in ("media-side-input", "host-side-output"): + lane_mask = ((1 << media_lane_count) - 1) << ((subport - 1) * media_lane_count) + else: + lane_mask = 0 + try: - status = api.set_loopback_mode(loopback_mode) + status = api.set_loopback_mode(loopback_mode, lane_mask=lane_mask) except AttributeError: click.echo("{}: Set loopback mode is not applicable for this module".format(port_name)) sys.exit(ERROR_NOT_IMPLEMENTED) + except TypeError: + status = api.set_loopback_mode(loopback_mode) if status: click.echo("{}: Set {} loopback".format(port_name, loopback_mode)) diff --git a/tests/sfputil_test.py b/tests/sfputil_test.py index 0e58daa18e..a9dd096290 100644 --- a/tests/sfputil_test.py +++ b/tests/sfputil_test.py @@ -1633,6 +1633,9 @@ def test_load_port_config(self, mock_is_multi_asic): @patch('sfputil.main.platform_chassis') @patch('sfputil.main.platform_sfputil', MagicMock(is_logical_port=MagicMock(return_value=1))) @patch('sfputil.main.logical_port_to_physical_port_index', MagicMock(return_value=1)) + @patch('sonic_py_common.multi_asic.get_front_end_namespaces', MagicMock(return_value=[''])) + @patch('sfputil.main.ConfigDBConnector', MagicMock()) + @patch('sfputil.main.SonicV2Connector', MagicMock()) def test_debug_loopback(self, mock_chassis): mock_sfp = MagicMock() mock_api = MagicMock() @@ -1659,6 +1662,12 @@ def test_debug_loopback(self, mock_chassis): assert result.output == 'Ethernet0: Set host-side-input loopback\n' assert result.exit_code != ERROR_NOT_IMPLEMENTED + mock_sfp.get_xcvr_api = MagicMock(return_value=mock_api) + result = runner.invoke(sfputil.cli.commands['debug'].commands['loopback'], + ["Ethernet0", "media-side-input"]) + assert result.output == 'Ethernet0: Set media-side-input loopback\n' + assert result.exit_code != ERROR_NOT_IMPLEMENTED + mock_api.set_loopback_mode.return_value = False result = runner.invoke(sfputil.cli.commands['debug'].commands['loopback'], ["Ethernet0", "none"]) @@ -1671,3 +1680,8 @@ def test_debug_loopback(self, mock_chassis): ["Ethernet0", "none"]) assert result.output == 'Ethernet0: Set loopback mode is not applicable for this module\n' assert result.exit_code == ERROR_NOT_IMPLEMENTED + + mock_api.set_loopback_mode.side_effect = [TypeError, True] + result = runner.invoke(sfputil.cli.commands['debug'].commands['loopback'], + ["Ethernet0", "none"]) + assert result.output == 'Ethernet0: Set none loopback\n' From c7010e2aab45693e193a45250aee4c9ad8c1b721 Mon Sep 17 00:00:00 2001 From: xinyu Date: Wed, 14 Aug 2024 09:56:00 +0800 Subject: [PATCH 2/9] [sfputil] Get namespace instance directly from the logical port Signed-off-by: xinyu --- sfputil/main.py | 43 +++++++++++++++++++++---------------------- 1 file changed, 21 insertions(+), 22 deletions(-) diff --git a/sfputil/main.py b/sfputil/main.py index c804eb0fb0..c0f2df697c 100644 --- a/sfputil/main.py +++ b/sfputil/main.py @@ -1991,29 +1991,28 @@ def loopback(port_name, loopback_mode): click.echo("{}: This functionality is not implemented".format(port_name)) sys.exit(ERROR_NOT_IMPLEMENTED) - namespaces = multi_asic.get_front_end_namespaces() - for namespace in namespaces: - config_db = ConfigDBConnector(use_unix_socket_path=True, namespace=namespace) - if config_db is not None: - config_db.connect() - subport = int(config_db.get(config_db.CONFIG_DB, - 'PORT|{}'.format(port_name), 'subport')) - else: - click.echo("Failed to connect to CONFIG_DB") - sys.exit(EXIT_FAIL) + namespace = multi_asic.get_namespace_for_port(port_name) + config_db = ConfigDBConnector(use_unix_socket_path=True, namespace=namespace) + if config_db is not None: + config_db.connect() + subport = int(config_db.get(config_db.CONFIG_DB, + 'PORT|{}'.format(port_name), 'subport')) + else: + click.echo("Failed to connect to CONFIG_DB") + sys.exit(EXIT_FAIL) - state_db = SonicV2Connector(use_unix_socket_path=False, namespace=namespace) - if state_db is not None: - state_db.connect(state_db.STATE_DB) - host_lane_count = int(state_db.get(state_db.STATE_DB, - 'TRANSCEIVER_INFO|{}'.format(port_name), - 'host_lane_count')) - media_lane_count = int(state_db.get(state_db.STATE_DB, - 'TRANSCEIVER_INFO|{}'.format(port_name), - 'media_lane_count')) - else: - click.echo("Failed to connect to STATE_DB") - sys.exit(EXIT_FAIL) + state_db = SonicV2Connector(use_unix_socket_path=False, namespace=namespace) + if state_db is not None: + state_db.connect(state_db.STATE_DB) + host_lane_count = int(state_db.get(state_db.STATE_DB, + 'TRANSCEIVER_INFO|{}'.format(port_name), + 'host_lane_count')) + media_lane_count = int(state_db.get(state_db.STATE_DB, + 'TRANSCEIVER_INFO|{}'.format(port_name), + 'media_lane_count')) + else: + click.echo("Failed to connect to STATE_DB") + sys.exit(EXIT_FAIL) if loopback_mode in ("host-side-input", "host-side-output"): lane_mask = ((1 << host_lane_count) - 1) << ((subport - 1) * host_lane_count) From a4362bea557ed9481a9017d90e62c2fd2e0e1dc5 Mon Sep 17 00:00:00 2001 From: xinyu Date: Wed, 14 Aug 2024 09:56:33 +0800 Subject: [PATCH 3/9] [sfputil] Assign subport to 1 if it is not defined or set to 0 in CONFIG_DB --- sfputil/main.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/sfputil/main.py b/sfputil/main.py index c0f2df697c..9ef6cbaa6b 100644 --- a/sfputil/main.py +++ b/sfputil/main.py @@ -1997,6 +1997,11 @@ def loopback(port_name, loopback_mode): config_db.connect() subport = int(config_db.get(config_db.CONFIG_DB, 'PORT|{}'.format(port_name), 'subport')) + + # If it is not defined (None) or if it is set to 0, assign a default value of 1 + # to ensure valid subport configuration. + if subport is None or subport == 0: + subport = 1 else: click.echo("Failed to connect to CONFIG_DB") sys.exit(EXIT_FAIL) From 4d85d225e44756ed3d5020e6e9cdf363fc4cbf71 Mon Sep 17 00:00:00 2001 From: xinyu Date: Sun, 18 Aug 2024 18:50:22 +0800 Subject: [PATCH 4/9] [sfputil] Update loopback_mode options to individually clear each mode Signed-off-by: xinyu --- sfputil/main.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/sfputil/main.py b/sfputil/main.py index 9ef6cbaa6b..af58c042ba 100644 --- a/sfputil/main.py +++ b/sfputil/main.py @@ -1469,10 +1469,10 @@ def is_fw_switch_done(port_name): status = -1 # Abnormal status. elif (ImageARunning == 1) and (ImageACommitted == 0): # ImageA is running, but not committed. click.echo("FW images switch successful : ImageA is running") - status = 1 # run_firmware is done. + status = 1 # run_firmware is done. elif (ImageBRunning == 1) and (ImageBCommitted == 0): # ImageB is running, but not committed. click.echo("FW images switch successful : ImageB is running") - status = 1 # run_firmware is done. + status = 1 # run_firmware is done. else: # No image is running, or running and committed image is same. click.echo("FW info error : Failed to switch into uncommitted image!") status = -1 # Failure for Switching images. @@ -1970,7 +1970,9 @@ def debug(): @click.argument('port_name', required=True, default=None) @click.argument('loopback_mode', required=True, default="none", type=click.Choice(["none", "host-side-input", "host-side-output", - "media-side-input", "media-side-output"])) + "media-side-input", "media-side-output", + "host-side-input-none", "host-side-output-none", + "media-side-input-none", "media-side-output-none"])) def loopback(port_name, loopback_mode): """Set module diagnostic loopback mode """ @@ -1998,7 +2000,7 @@ def loopback(port_name, loopback_mode): subport = int(config_db.get(config_db.CONFIG_DB, 'PORT|{}'.format(port_name), 'subport')) - # If it is not defined (None) or if it is set to 0, assign a default value of 1 + # If subport is not defined (None) or set to 0, assign a default value of 1 # to ensure valid subport configuration. if subport is None or subport == 0: subport = 1 @@ -2019,9 +2021,11 @@ def loopback(port_name, loopback_mode): click.echo("Failed to connect to STATE_DB") sys.exit(EXIT_FAIL) - if loopback_mode in ("host-side-input", "host-side-output"): + if loopback_mode in ("host-side-input", "host-side-output", + "host-side-input-none", "host-side-output-none"): lane_mask = ((1 << host_lane_count) - 1) << ((subport - 1) * host_lane_count) - elif loopback_mode in ("media-side-input", "host-side-output"): + elif loopback_mode in ("media-side-input", "media-side-output", + "media-side-input-none", "media-side-output-none"): lane_mask = ((1 << media_lane_count) - 1) << ((subport - 1) * media_lane_count) else: lane_mask = 0 From af0508c3d56b75614ed52db3d534c0531a8aec00 Mon Sep 17 00:00:00 2001 From: xinyu Date: Mon, 19 Aug 2024 20:35:17 +0800 Subject: [PATCH 5/9] [sfputil] Simplify the loopback condition to make it more compact and clean Signed-off-by: xinyu --- sfputil/main.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/sfputil/main.py b/sfputil/main.py index af58c042ba..992db42ac9 100644 --- a/sfputil/main.py +++ b/sfputil/main.py @@ -1469,10 +1469,10 @@ def is_fw_switch_done(port_name): status = -1 # Abnormal status. elif (ImageARunning == 1) and (ImageACommitted == 0): # ImageA is running, but not committed. click.echo("FW images switch successful : ImageA is running") - status = 1 # run_firmware is done. + status = 1 # run_firmware is done. elif (ImageBRunning == 1) and (ImageBCommitted == 0): # ImageB is running, but not committed. click.echo("FW images switch successful : ImageB is running") - status = 1 # run_firmware is done. + status = 1 # run_firmware is done. else: # No image is running, or running and committed image is same. click.echo("FW info error : Failed to switch into uncommitted image!") status = -1 # Failure for Switching images. @@ -2021,11 +2021,9 @@ def loopback(port_name, loopback_mode): click.echo("Failed to connect to STATE_DB") sys.exit(EXIT_FAIL) - if loopback_mode in ("host-side-input", "host-side-output", - "host-side-input-none", "host-side-output-none"): + if 'host-side' in loopback_mode: lane_mask = ((1 << host_lane_count) - 1) << ((subport - 1) * host_lane_count) - elif loopback_mode in ("media-side-input", "media-side-output", - "media-side-input-none", "media-side-output-none"): + elif 'media-side' in loopback_mode: lane_mask = ((1 << media_lane_count) - 1) << ((subport - 1) * media_lane_count) else: lane_mask = 0 From e1c7aa1fbbfeb2e8e1ea41dd61ab5169ffce9fbb Mon Sep 17 00:00:00 2001 From: xinyu Date: Tue, 20 Aug 2024 20:47:53 +0800 Subject: [PATCH 6/9] [sfputil] Reject the loopback command and report an error if subport, host_lane_count, or media_lane_count is not present Signed-off-by: xinyu --- doc/Command-Reference.md | 14 +++++++++----- sfputil/main.py | 37 ++++++++++++++++++++++++------------- tests/sfputil_test.py | 29 ++++++++++++++++++++++++++--- 3 files changed, 59 insertions(+), 21 deletions(-) diff --git a/doc/Command-Reference.md b/doc/Command-Reference.md index 689ca23b73..5625b04eab 100644 --- a/doc/Command-Reference.md +++ b/doc/Command-Reference.md @@ -3109,11 +3109,15 @@ This command is the standard CMIS diagnostic control used for troubleshooting li sfputil debug loopback PORT_NAME LOOPBACK_MODE Set the loopback mode - host-side-input: host side input loopback mode - host-side-output: host side output loopback mode - media-side-input: media side input loopback mode - media-side-output: media side output loopback mode - none: disable loopback mode + host-side-input: enable host side input loopback mode + host-side-output: enable host side output loopback mode + media-side-input: enable media side input loopback mode + media-side-output: enable media side output loopback mode + host-side-input-none: disable host side input loopback mode + host-side-output-none: disable host side output loopback mode + media-side-input-none: disable media side input loopback mode + media-side-output-none: disable media side output loopback mode + none: disable all loopback mode ``` - Example: diff --git a/sfputil/main.py b/sfputil/main.py index 992db42ac9..004a47ea9f 100644 --- a/sfputil/main.py +++ b/sfputil/main.py @@ -1997,28 +1997,39 @@ def loopback(port_name, loopback_mode): config_db = ConfigDBConnector(use_unix_socket_path=True, namespace=namespace) if config_db is not None: config_db.connect() - subport = int(config_db.get(config_db.CONFIG_DB, - 'PORT|{}'.format(port_name), 'subport')) + try: + subport = int(config_db.get(config_db.CONFIG_DB, f'PORT|{port_name}', 'subport')) + except TypeError: + click.echo(f"{port_name}: subport is not present in CONFIG_DB") + sys.exit(EXIT_FAIL) - # If subport is not defined (None) or set to 0, assign a default value of 1 - # to ensure valid subport configuration. - if subport is None or subport == 0: + # If subport is set to 0, assign a default value of 1 to ensure valid subport configuration + if subport == 0: subport = 1 else: - click.echo("Failed to connect to CONFIG_DB") + click.echo(f"{port_name}: Failed to connect to CONFIG_DB") sys.exit(EXIT_FAIL) state_db = SonicV2Connector(use_unix_socket_path=False, namespace=namespace) if state_db is not None: state_db.connect(state_db.STATE_DB) - host_lane_count = int(state_db.get(state_db.STATE_DB, - 'TRANSCEIVER_INFO|{}'.format(port_name), - 'host_lane_count')) - media_lane_count = int(state_db.get(state_db.STATE_DB, - 'TRANSCEIVER_INFO|{}'.format(port_name), - 'media_lane_count')) + try: + host_lane_count = int(state_db.get(state_db.STATE_DB, + f'TRANSCEIVER_INFO|{port_name}', + 'host_lane_count')) + except TypeError: + click.echo(f"{port_name}: host_lane_count is not present in STATE_DB") + sys.exit(EXIT_FAIL) + + try: + media_lane_count = int(state_db.get(state_db.STATE_DB, + f'TRANSCEIVER_INFO|{port_name}', + 'media_lane_count')) + except TypeError: + click.echo(f"{port_name}: media_lane_count is not present in STATE_DB") + sys.exit(EXIT_FAIL) else: - click.echo("Failed to connect to STATE_DB") + click.echo(f"{port_name}: Failed to connect to STATE_DB") sys.exit(EXIT_FAIL) if 'host-side' in loopback_mode: diff --git a/tests/sfputil_test.py b/tests/sfputil_test.py index a9dd096290..3d54d5d715 100644 --- a/tests/sfputil_test.py +++ b/tests/sfputil_test.py @@ -1631,14 +1631,16 @@ def test_load_port_config(self, mock_is_multi_asic): @patch('sfputil.main.is_port_type_rj45', MagicMock(return_value=False)) @patch('sfputil.main.platform_chassis') + @patch('sfputil.main.ConfigDBConnector') + @patch('sfputil.main.SonicV2Connector') @patch('sfputil.main.platform_sfputil', MagicMock(is_logical_port=MagicMock(return_value=1))) @patch('sfputil.main.logical_port_to_physical_port_index', MagicMock(return_value=1)) @patch('sonic_py_common.multi_asic.get_front_end_namespaces', MagicMock(return_value=[''])) - @patch('sfputil.main.ConfigDBConnector', MagicMock()) - @patch('sfputil.main.SonicV2Connector', MagicMock()) - def test_debug_loopback(self, mock_chassis): + def test_debug_loopback(self, mock_sonic_v2_connector, mock_config_db_connector, mock_chassis): mock_sfp = MagicMock() mock_api = MagicMock() + mock_config_db_connector.return_value = MagicMock() + mock_sonic_v2_connector.return_value = MagicMock() mock_chassis.get_sfp = MagicMock(return_value=mock_sfp) mock_sfp.get_presence.return_value = True mock_sfp.get_xcvr_api = MagicMock(return_value=mock_api) @@ -1685,3 +1687,24 @@ def test_debug_loopback(self, mock_chassis): result = runner.invoke(sfputil.cli.commands['debug'].commands['loopback'], ["Ethernet0", "none"]) assert result.output == 'Ethernet0: Set none loopback\n' + + mock_config_db = MagicMock() + mock_config_db.get.side_effect = TypeError + mock_config_db_connector.return_value = mock_config_db + result = runner.invoke(sfputil.cli.commands['debug'].commands['loopback'], + ["Ethernet0", "none"]) + assert result.output == 'Ethernet0: subport is not present in CONFIG_DB\n' + assert result.exit_code == EXIT_FAIL + + mock_config_db_connector.return_value = None + result = runner.invoke(sfputil.cli.commands['debug'].commands['loopback'], + ["Ethernet0", "none"]) + assert result.output == 'Ethernet0: Failed to connect to CONFIG_DB\n' + assert result.exit_code == EXIT_FAIL + + mock_config_db_connector.return_value = MagicMock() + mock_sonic_v2_connector.return_value = None + result = runner.invoke(sfputil.cli.commands['debug'].commands['loopback'], + ["Ethernet0", "none"]) + assert result.output == 'Ethernet0: Failed to connect to STATE_DB\n' + assert result.exit_code == EXIT_FAIL From 4ede7973d9d592ff84227f8fd3e7ae3b06b58e68 Mon Sep 17 00:00:00 2001 From: xinyu Date: Wed, 4 Sep 2024 14:26:38 +0800 Subject: [PATCH 7/9] [sfputil] Add an enable parameter to eliminate the loopback modes with the -none suffix Signed-off-by: xinyu --- doc/Command-Reference.md | 18 +++++++----------- sfputil/main.py | 20 +++++++++++--------- tests/sfputil_test.py | 25 +++++++++++++------------ 3 files changed, 31 insertions(+), 32 deletions(-) diff --git a/doc/Command-Reference.md b/doc/Command-Reference.md index 5625b04eab..ea34db9035 100644 --- a/doc/Command-Reference.md +++ b/doc/Command-Reference.md @@ -3106,23 +3106,19 @@ This command is the standard CMIS diagnostic control used for troubleshooting li - Usage: ``` - sfputil debug loopback PORT_NAME LOOPBACK_MODE + sfputil debug loopback PORT_NAME LOOPBACK_MODE Set the loopback mode - host-side-input: enable host side input loopback mode - host-side-output: enable host side output loopback mode - media-side-input: enable media side input loopback mode - media-side-output: enable media side output loopback mode - host-side-input-none: disable host side input loopback mode - host-side-output-none: disable host side output loopback mode - media-side-input-none: disable media side input loopback mode - media-side-output-none: disable media side output loopback mode - none: disable all loopback mode + host-side-input: host side input loopback mode + host-side-output: host side output loopback mode + media-side-input: media side input loopback mode + media-side-output: media side output loopback mode ``` - Example: ``` - admin@sonic:~$ sfputil debug loopback Ethernet88 host-side-input + admin@sonic:~$ sfputil debug loopback Ethernet88 host-side-input enable + admin@sonic:~$ sfputil debug loopback Ethernet88 media-side-output disable ``` ## DHCP Relay diff --git a/sfputil/main.py b/sfputil/main.py index 004a47ea9f..1e6416e14c 100644 --- a/sfputil/main.py +++ b/sfputil/main.py @@ -1967,13 +1967,12 @@ def debug(): # 'loopback' subcommand @debug.command() -@click.argument('port_name', required=True, default=None) -@click.argument('loopback_mode', required=True, default="none", - type=click.Choice(["none", "host-side-input", "host-side-output", - "media-side-input", "media-side-output", - "host-side-input-none", "host-side-output-none", - "media-side-input-none", "media-side-output-none"])) -def loopback(port_name, loopback_mode): +@click.argument('port_name', required=True) +@click.argument('loopback_mode', required=True, + type=click.Choice(["host-side-input", "host-side-output", + "media-side-input", "media-side-output"])) +@click.argument('enable', required=True, type=click.Choice(["enable", "disable"])) +def loopback(port_name, loopback_mode, enable): """Set module diagnostic loopback mode """ physical_port = logical_port_to_physical_port_index(port_name) @@ -2040,12 +2039,15 @@ def loopback(port_name, loopback_mode): lane_mask = 0 try: - status = api.set_loopback_mode(loopback_mode, lane_mask=lane_mask) + status = api.set_loopback_mode(loopback_mode, + lane_mask=lane_mask, + enable=enable == 'enable') except AttributeError: click.echo("{}: Set loopback mode is not applicable for this module".format(port_name)) sys.exit(ERROR_NOT_IMPLEMENTED) except TypeError: - status = api.set_loopback_mode(loopback_mode) + click.echo("{}: Set loopback mode failed. Parameter is not supported".format(port_name)) + sys.exit(EXIT_FAIL) if status: click.echo("{}: Set {} loopback".format(port_name, loopback_mode)) diff --git a/tests/sfputil_test.py b/tests/sfputil_test.py index 3d54d5d715..5f1374be12 100644 --- a/tests/sfputil_test.py +++ b/tests/sfputil_test.py @@ -1648,63 +1648,64 @@ def test_debug_loopback(self, mock_sonic_v2_connector, mock_config_db_connector, runner = CliRunner() mock_sfp.get_presence.return_value = False result = runner.invoke(sfputil.cli.commands['debug'].commands['loopback'], - ["Ethernet0", "host-side-input"]) + ["Ethernet0", "host-side-input", "enable"]) assert result.output == 'Ethernet0: SFP EEPROM not detected\n' mock_sfp.get_presence.return_value = True mock_sfp.get_xcvr_api = MagicMock(side_effect=NotImplementedError) result = runner.invoke(sfputil.cli.commands['debug'].commands['loopback'], - ["Ethernet0", "host-side-input"]) + ["Ethernet0", "host-side-input", "enable"]) assert result.output == 'Ethernet0: This functionality is not implemented\n' assert result.exit_code == ERROR_NOT_IMPLEMENTED mock_sfp.get_xcvr_api = MagicMock(return_value=mock_api) result = runner.invoke(sfputil.cli.commands['debug'].commands['loopback'], - ["Ethernet0", "host-side-input"]) + ["Ethernet0", "host-side-input", "enable"]) assert result.output == 'Ethernet0: Set host-side-input loopback\n' assert result.exit_code != ERROR_NOT_IMPLEMENTED mock_sfp.get_xcvr_api = MagicMock(return_value=mock_api) result = runner.invoke(sfputil.cli.commands['debug'].commands['loopback'], - ["Ethernet0", "media-side-input"]) + ["Ethernet0", "media-side-input", "enable"]) assert result.output == 'Ethernet0: Set media-side-input loopback\n' assert result.exit_code != ERROR_NOT_IMPLEMENTED mock_api.set_loopback_mode.return_value = False result = runner.invoke(sfputil.cli.commands['debug'].commands['loopback'], - ["Ethernet0", "none"]) - assert result.output == 'Ethernet0: Set none loopback failed\n' + ["Ethernet0", "media-side-output", "enable"]) + assert result.output == 'Ethernet0: Set media-side-output loopback failed\n' assert result.exit_code == EXIT_FAIL mock_api.set_loopback_mode.return_value = True mock_api.set_loopback_mode.side_effect = AttributeError result = runner.invoke(sfputil.cli.commands['debug'].commands['loopback'], - ["Ethernet0", "none"]) + ["Ethernet0", "host-side-input", "enable"]) assert result.output == 'Ethernet0: Set loopback mode is not applicable for this module\n' assert result.exit_code == ERROR_NOT_IMPLEMENTED mock_api.set_loopback_mode.side_effect = [TypeError, True] result = runner.invoke(sfputil.cli.commands['debug'].commands['loopback'], - ["Ethernet0", "none"]) - assert result.output == 'Ethernet0: Set none loopback\n' + ["Ethernet0", "host-side-input", "enable"]) + assert result.output == 'Ethernet0: Set loopback mode failed. Parameter is not supported\n' + assert result.exit_code == EXIT_FAIL mock_config_db = MagicMock() mock_config_db.get.side_effect = TypeError mock_config_db_connector.return_value = mock_config_db result = runner.invoke(sfputil.cli.commands['debug'].commands['loopback'], - ["Ethernet0", "none"]) + ["Ethernet0", "media-side-input", "enable"]) assert result.output == 'Ethernet0: subport is not present in CONFIG_DB\n' assert result.exit_code == EXIT_FAIL mock_config_db_connector.return_value = None result = runner.invoke(sfputil.cli.commands['debug'].commands['loopback'], - ["Ethernet0", "none"]) + ["Ethernet0", "media-side-input", "enable"]) assert result.output == 'Ethernet0: Failed to connect to CONFIG_DB\n' assert result.exit_code == EXIT_FAIL mock_config_db_connector.return_value = MagicMock() mock_sonic_v2_connector.return_value = None result = runner.invoke(sfputil.cli.commands['debug'].commands['loopback'], - ["Ethernet0", "none"]) + ["Ethernet0", "media-side-input", "enable"]) assert result.output == 'Ethernet0: Failed to connect to STATE_DB\n' assert result.exit_code == EXIT_FAIL From 2aaf92fd3747ee6881263d9aedcdb913fa5b9cc3 Mon Sep 17 00:00:00 2001 From: xinyu Date: Tue, 10 Sep 2024 22:29:38 +0800 Subject: [PATCH 8/9] [sfputil] Add generic function to get lane mask by given subport and lane count Signed-off-by: xinyu --- sfputil/main.py | 22 ++++++++++++++++++---- tests/sfputil_test.py | 17 ++++++++++++++--- 2 files changed, 32 insertions(+), 7 deletions(-) diff --git a/sfputil/main.py b/sfputil/main.py index 1e6416e14c..58c6855abe 100644 --- a/sfputil/main.py +++ b/sfputil/main.py @@ -2032,9 +2032,9 @@ def loopback(port_name, loopback_mode, enable): sys.exit(EXIT_FAIL) if 'host-side' in loopback_mode: - lane_mask = ((1 << host_lane_count) - 1) << ((subport - 1) * host_lane_count) + lane_mask = get_subport_lane_mask(subport, host_lane_count) elif 'media-side' in loopback_mode: - lane_mask = ((1 << media_lane_count) - 1) << ((subport - 1) * media_lane_count) + lane_mask = get_subport_lane_mask(subport, media_lane_count) else: lane_mask = 0 @@ -2050,10 +2050,24 @@ def loopback(port_name, loopback_mode, enable): sys.exit(EXIT_FAIL) if status: - click.echo("{}: Set {} loopback".format(port_name, loopback_mode)) + click.echo("{}: {} {} loopback".format(port_name, enable, loopback_mode)) else: - click.echo("{}: Set {} loopback failed".format(port_name, loopback_mode)) + click.echo("{}: {} {} loopback failed".format(port_name, enable, loopback_mode)) sys.exit(EXIT_FAIL) + +def get_subport_lane_mask(subport, lane_count): + """Get the lane mask for the given subport and lane count + + Args: + subport (int): Subport number + lane_count (int): Lane count for the subport + + Returns: + int: Lane mask for the given subport and lane count + """ + return ((1 << lane_count) - 1) << ((subport - 1) * lane_count) + + if __name__ == '__main__': cli() diff --git a/tests/sfputil_test.py b/tests/sfputil_test.py index 5f1374be12..d8d13df1c0 100644 --- a/tests/sfputil_test.py +++ b/tests/sfputil_test.py @@ -1661,19 +1661,19 @@ def test_debug_loopback(self, mock_sonic_v2_connector, mock_config_db_connector, mock_sfp.get_xcvr_api = MagicMock(return_value=mock_api) result = runner.invoke(sfputil.cli.commands['debug'].commands['loopback'], ["Ethernet0", "host-side-input", "enable"]) - assert result.output == 'Ethernet0: Set host-side-input loopback\n' + assert result.output == 'Ethernet0: enable host-side-input loopback\n' assert result.exit_code != ERROR_NOT_IMPLEMENTED mock_sfp.get_xcvr_api = MagicMock(return_value=mock_api) result = runner.invoke(sfputil.cli.commands['debug'].commands['loopback'], ["Ethernet0", "media-side-input", "enable"]) - assert result.output == 'Ethernet0: Set media-side-input loopback\n' + assert result.output == 'Ethernet0: enable media-side-input loopback\n' assert result.exit_code != ERROR_NOT_IMPLEMENTED mock_api.set_loopback_mode.return_value = False result = runner.invoke(sfputil.cli.commands['debug'].commands['loopback'], ["Ethernet0", "media-side-output", "enable"]) - assert result.output == 'Ethernet0: Set media-side-output loopback failed\n' + assert result.output == 'Ethernet0: enable media-side-output loopback failed\n' assert result.exit_code == EXIT_FAIL mock_api.set_loopback_mode.return_value = True @@ -1709,3 +1709,14 @@ def test_debug_loopback(self, mock_sonic_v2_connector, mock_config_db_connector, ["Ethernet0", "media-side-input", "enable"]) assert result.output == 'Ethernet0: Failed to connect to STATE_DB\n' assert result.exit_code == EXIT_FAIL + + @pytest.mark.parametrize("subport, lane_count, expected_mask", [ + (1, 1, 0x1), + (1, 4, 0xf), + (2, 1, 0x2), + (2, 4, 0xf0), + (3, 2, 0x30), + (4, 1, 0x8), + ]) + def test_get_subport_lane_mask(self, subport, lane_count, expected_mask): + assert sfputil.get_subport_lane_mask(subport, lane_count) == expected_mask From 96eac898ef0f9d3e6c79e2c608feb45e315a2faa Mon Sep 17 00:00:00 2001 From: xinyu Date: Wed, 11 Sep 2024 14:05:50 +0800 Subject: [PATCH 9/9] [sfputil] Update the loopback command reference documentation to improve clarity Signed-off-by: xinyu --- doc/Command-Reference.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/Command-Reference.md b/doc/Command-Reference.md index ea34db9035..817c9189ee 100644 --- a/doc/Command-Reference.md +++ b/doc/Command-Reference.md @@ -3108,7 +3108,7 @@ This command is the standard CMIS diagnostic control used for troubleshooting li ``` sfputil debug loopback PORT_NAME LOOPBACK_MODE - Set the loopback mode + Valid values for loopback mode host-side-input: host side input loopback mode host-side-output: host side output loopback mode media-side-input: media side input loopback mode