From 39473ea7894c1ed96a1844376e471deb351b1a40 Mon Sep 17 00:00:00 2001 From: Jie Feng Date: Fri, 22 Mar 2024 18:50:24 -0700 Subject: [PATCH 1/3] Add fabric monitoring commands. --- config/fabric.py | 55 +++++++++++++++++++++++++++++++++------- scripts/fabricstat | 40 +++++++++++++++++++++++++++++ show/fabric.py | 12 +++++++++ tests/fabricstat_test.py | 45 ++++++++++++++++++++++++++++++++ 4 files changed, 143 insertions(+), 9 deletions(-) diff --git a/config/fabric.py b/config/fabric.py index a3870589ae..48d4e98ae5 100644 --- a/config/fabric.py +++ b/config/fabric.py @@ -2,7 +2,10 @@ import utilities_common.cli as clicommon import utilities_common.multi_asic as multi_asic_util from sonic_py_common import multi_asic -from swsscommon.swsscommon import SonicV2Connector, ConfigDBConnector +from swsscommon.swsscommon import SonicV2Connector, ConfigDBConnector, APP_FABRIC_PORT_TABLE_NAME + +FABRIC_PORT_STATUS_TABLE_PREFIX = APP_FABRIC_PORT_TABLE_NAME+"|" + # # 'config fabric ...' @@ -66,19 +69,13 @@ def isolate(portid, namespace): # @port.command() @click.argument('portid', metavar='', required=True) +@click.option('-f', '--force', is_flag=True, help='Force to unisolate a link even if it is auto isolated.') @multi_asic_util.multi_asic_click_option_namespace -def unisolate(portid, namespace): +def unisolate(portid, namespace, force): """FABRIC PORT unisolate """ ctx = click.get_current_context() - if not portid.isdigit(): - ctx.fail("Invalid portid") - - n_asics = multi_asic.get_num_asics() - if n_asics > 1 and namespace is None: - ctx.fail('Must specify asic') - # Connect to config database config_db = ConfigDBConnector(use_unix_socket_path=True, namespace=namespace) config_db.connect() @@ -87,6 +84,37 @@ def unisolate(portid, namespace): state_db = SonicV2Connector(use_unix_socket_path=True, namespace=namespace) state_db.connect(state_db.STATE_DB, False) + n_asics = multi_asic.get_num_asics() + if n_asics > 1 and namespace is None: + ctx.fail( 'Must specify asic' ) + + # If "all" is specified then unisolate all ports. + if portid == "all": + port_keys = state_db.keys(state_db.STATE_DB, FABRIC_PORT_STATUS_TABLE_PREFIX + '*') + for port_key in port_keys: + port_data = state_db.get_all(state_db.STATE_DB, port_key) + if "REMOTE_PORT" in port_data: + port_number = int( port_key.replace( "FABRIC_PORT_TABLE|PORT", "" ) ) + + # Make sure configuration data exists + portName = f'Fabric{port_number}' + portConfigData = config_db.get_all(config_db.CONFIG_DB, "FABRIC_PORT|" + portName) + if not bool( portConfigData ): + ctx.fail( "Fabric monitor configuration data not present" ) + + # Update entry + config_db.mod_entry( "FABRIC_PORT", portName, {'isolateStatus': False} ) + if force: + forceShutCnt = int( portConfigData['forceUnisolateStatus'] ) + forceShutCnt += 1 + config_db.mod_entry( "FABRIC_PORT", portName, + {'forceUnisolateStatus': forceShutCnt}) + + return + + if not portid.isdigit(): + ctx.fail( "Invalid portid" ) + # check if the port is actually in use portName = f'PORT{portid}' portStateData = state_db.get_all(state_db.STATE_DB, "FABRIC_PORT_TABLE|" + portName) @@ -102,6 +130,15 @@ def unisolate(portid, namespace): # Update entry config_db.mod_entry("FABRIC_PORT", portName, {'isolateStatus': False}) + if force: + forceShutCnt = int( portConfigData['forceUnisolateStatus'] ) + forceShutCnt += 1 + config_db.mod_entry( "FABRIC_PORT", portName, + {'forceUnisolateStatus': forceShutCnt}) + + print("Force unisolate the link.") + print("It will clear all fabric link monitoring status for this link!") + # # 'config fabric port monitor ...' # diff --git a/scripts/fabricstat b/scripts/fabricstat index 205e3170bc..29b8ffdbe0 100755 --- a/scripts/fabricstat +++ b/scripts/fabricstat @@ -307,6 +307,40 @@ class FabricReachability(FabricStat): print(tabulate(body, header, tablefmt='simple', stralign='right')) return +class FabricIsolation(FabricStat): + def isolation_print(self): + # Connect to database + self.db = multi_asic.connect_to_all_dbs_for_ns(self.namespace) + # Get the set of all fabric ports + port_keys = self.db.keys(self.db.STATE_DB, FABRIC_PORT_STATUS_TABLE_PREFIX + '*') + # Create a new dictionary. The keys are the local port values in integer format. + # Only fabric ports that have remote port data are added. + port_dict = {} + for port_key in port_keys: + port_data = self.db.get_all(self.db.STATE_DB, port_key) + if "REMOTE_PORT" in port_data: + port_number = int(port_key.replace("FABRIC_PORT_TABLE|PORT", "")) + port_dict.update({port_number: port_data}) + # Create ordered table of fabric ports. + header = ["Local Link", "Auto Isolated", "Manual Isolated", "Isolated"] + auto_isolated = 0 + manual_isolated = 0 + isolated = 0 + body = [] + for port_number in sorted(port_dict.keys()): + port_data = port_dict[port_number] + if "AUTO_ISOLATED" in port_data: + auto_isolated = port_data["AUTO_ISOLATED"] + if "CONFIG_ISOLATED" in port_data: + manual_isolated = port_data["CONFIG_ISOLATED"] + if "ISOLATED" in port_data: + isolated = port_data["ISOLATED"] + body.append((port_number, auto_isolated, manual_isolated, isolated)); + if self.namespace: + print(f"\n{self.namespace}") + print(tabulate(body, header, tablefmt='simple', stralign='right')) + return + def main(): global cnstat_dir global cnstat_fqn_file_port @@ -329,12 +363,14 @@ Examples: parser.add_argument('-r','--reachability', action='store_true', help='Display reachability, otherwise port stat') parser.add_argument('-n','--namespace', default=None, help='Display fabric ports counters for specific namespace') parser.add_argument('-e', '--errors', action='store_true', help='Display errors') + parser.add_argument('-i','--isolation', action='store_true', help='Display fabric ports isolation status') parser.add_argument('-C','--clear', action='store_true', help='Copy & clear fabric counters') parser.add_argument('-D','--delete', action='store_true', help='Delete saved stats') args = parser.parse_args() queue = args.queue reachability = args.reachability + isolation_status = args.isolation namespace = args.namespace errors_only = args.errors @@ -362,6 +398,10 @@ Examples: stat = FabricReachability(ns) stat.reachability_print() return + elif isolation_status: + stat = FabricIsolation(ns) + stat.isolation_print() + return else: stat = FabricPortStat(ns) cnstat_dict = stat.get_cnstat_dict() diff --git a/show/fabric.py b/show/fabric.py index c8dc956e44..c67a28ac15 100644 --- a/show/fabric.py +++ b/show/fabric.py @@ -13,6 +13,18 @@ def counters(): """Show fabric port counters""" pass +@fabric.group(invoke_without_command=True) +@multi_asic_util.multi_asic_click_option_namespace +@click.option('-e', '--errors', is_flag=True) +def isolation(namespace, errors): + """Show fabric isolation status""" + cmd = ['fabricstat', '-i'] + if namespace is not None: + cmd += ['-n', str(namespace)] + if errors: + cmd += ["-e"] + clicommon.run_command(cmd) + @fabric.group(invoke_without_command=True) @multi_asic_util.multi_asic_click_option_namespace @click.option('-e', '--errors', is_flag=True) diff --git a/tests/fabricstat_test.py b/tests/fabricstat_test.py index 625c1d14a0..cc06701d81 100644 --- a/tests/fabricstat_test.py +++ b/tests/fabricstat_test.py @@ -151,6 +151,37 @@ 7 0 93 up """ +multi_asic_fabric_isolation = """\ + +asic0 + Local Link Auto Isolated Manual Isolated Isolated +------------ --------------- ----------------- ---------- + 0 0 0 0 + 2 0 0 0 + 4 0 0 0 + 6 0 0 0 + 7 0 0 0 + +asic1 + Local Link Auto Isolated Manual Isolated Isolated +------------ --------------- ----------------- ---------- + 0 0 0 0 + 4 0 0 0 +""" + +multi_asic_fabric_isolation_asic0 = """\ + +asic0 + Local Link Auto Isolated Manual Isolated Isolated +------------ --------------- ----------------- ---------- + 0 0 0 0 + 2 0 0 0 + 4 0 0 0 + 6 0 0 0 + 7 0 0 0 +""" + + class TestFabricStat(object): @classmethod def setup_class(cls): @@ -271,6 +302,20 @@ def test_multi_show_fabric_reachability_asic(self): assert return_code == 0 assert result == multi_asic_fabric_reachability_asic0 + def test_multi_show_fabric_isolation(self): + return_code, result = get_result_and_return_code(['fabricstat', '-i']) + print("return_code: {}".format(return_code)) + print("result = {}".format(result)) + assert return_code == 0 + assert result == multi_asic_fabric_isolation + + def test_multi_show_fabric_isolation_asic(self): + return_code, result = get_result_and_return_code(['fabricstat', '-i', '-n', 'asic0']) + print("return_code: {}".format(return_code)) + print("result = {}".format(result)) + assert return_code == 0 + assert result == multi_asic_fabric_isolation_asic0 + @classmethod def teardown_class(cls): print("TEARDOWN") From 44d1288f42203e3af136da6c77f6ae5ddc0eec4d Mon Sep 17 00:00:00 2001 From: Jie Feng Date: Mon, 25 Mar 2024 10:04:47 -0700 Subject: [PATCH 2/3] Update test to cover more code --- tests/config_fabric_test.py | 23 ++++++++++++++++++----- tests/mock_tables/config_db.json | 30 ++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 5 deletions(-) diff --git a/tests/config_fabric_test.py b/tests/config_fabric_test.py index 1f56ea416a..f39e896aa5 100644 --- a/tests/config_fabric_test.py +++ b/tests/config_fabric_test.py @@ -41,22 +41,35 @@ def test_config_isolation(self, ctx): expect_result = 0 assert operator.eq(result.exit_code, expect_result) - # Issue command "config fabric port isolate 1", - # check if the result has the error message as port 1 is not in use. - result = self.basic_check("port", ["isolate", "1"], ctx) - assert "Port 1 is not in use" in result.output - # Issue command "config fabric port unisolate 0", # check if the result is expected. result = self.basic_check("port", ["unisolate", "0"], ctx) expect_result = 0 assert operator.eq(result.exit_code, expect_result) + # Issue command "config fabric port unisolate 0", + # check if the result is expected. + result = self.basic_check("port", ["unisolate", "0", "--force"], ctx) + expect_result = 0 + assert operator.eq(result.exit_code, expect_result) + assert "Force unisolate the link" in result.output + + # Issue command "config fabric port isolate 1", + # check if the result has the error message as port 1 is not in use. + result = self.basic_check("port", ["isolate", "1"], ctx) + assert "Port 1 is not in use" in result.output + # Issue command "config fabric port unisolate 1", # check if the result has the error message as port 1 is not in use. result = self.basic_check("port", ["unisolate", "1"], ctx) assert "Port 1 is not in use" in result.output + # Issue command "config fabric port unisolate all -n asic1" + # check if the result has the warning message + result = self.basic_check("port", ["unisolate", "all", "--force"], ctx) + expect_result = 0 + assert operator.eq(result.exit_code, expect_result) + def test_config_fabric_monitor_threshold(self, ctx): # Issue command "config fabric port monitor error threshold <#> <#>" # with an out of range number, check if the result has the error message. diff --git a/tests/mock_tables/config_db.json b/tests/mock_tables/config_db.json index 0ef506c288..094993db03 100644 --- a/tests/mock_tables/config_db.json +++ b/tests/mock_tables/config_db.json @@ -2738,6 +2738,36 @@ "isolateStatus": "False", "lanes": "2" }, + "FABRIC_PORT|Fabric3": { + "alias": "Fabric3", + "forceUnisolateStatus": "0", + "isolateStatus": "False", + "lanes": "3" + }, + "FABRIC_PORT|Fabric4": { + "alias": "Fabric4", + "forceUnisolateStatus": "0", + "isolateStatus": "False", + "lanes": "4" + }, + "FABRIC_PORT|Fabric5": { + "alias": "Fabric5", + "forceUnisolateStatus": "0", + "isolateStatus": "False", + "lanes": "5" + }, + "FABRIC_PORT|Fabric6": { + "alias": "Fabric6", + "forceUnisolateStatus": "0", + "isolateStatus": "False", + "lanes": "6" + }, + "FABRIC_PORT|Fabric7": { + "alias": "Fabric7", + "forceUnisolateStatus": "0", + "isolateStatus": "False", + "lanes": "7" + }, "DHCP_RELAY|Vlan1000": { "dhcpv6_servers": [ "fc02:2000::1" From 9f61bf2c989c8c41cbf3c76dccc8b8d7a4a5c5e6 Mon Sep 17 00:00:00 2001 From: Jie Feng Date: Tue, 2 Apr 2024 12:42:04 -0700 Subject: [PATCH 3/3] Update with review comments --- config/fabric.py | 6 +++--- tests/mock_tables/config_db.json | 3 +++ 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/config/fabric.py b/config/fabric.py index 48d4e98ae5..56552ae7e0 100644 --- a/config/fabric.py +++ b/config/fabric.py @@ -69,7 +69,7 @@ def isolate(portid, namespace): # @port.command() @click.argument('portid', metavar='', required=True) -@click.option('-f', '--force', is_flag=True, help='Force to unisolate a link even if it is auto isolated.') +@click.option('-f', '--force', is_flag=True, default=False, help='Force to unisolate a link even if it is auto isolated.') @multi_asic_util.multi_asic_click_option_namespace def unisolate(portid, namespace, force): """FABRIC PORT unisolate """ @@ -136,8 +136,8 @@ def unisolate(portid, namespace, force): config_db.mod_entry( "FABRIC_PORT", portName, {'forceUnisolateStatus': forceShutCnt}) - print("Force unisolate the link.") - print("It will clear all fabric link monitoring status for this link!") + click.echo("Force unisolate the link.") + click.echo("It will clear all fabric link monitoring status for this link!") # # 'config fabric port monitor ...' diff --git a/tests/mock_tables/config_db.json b/tests/mock_tables/config_db.json index 094993db03..6a0f3e89b1 100644 --- a/tests/mock_tables/config_db.json +++ b/tests/mock_tables/config_db.json @@ -2725,16 +2725,19 @@ }, "FABRIC_PORT|Fabric0": { "alias": "Fabric0", + "forceUnisolateStatus": "0", "isolateStatus": "False", "lanes": "0" }, "FABRIC_PORT|Fabric1": { "alias": "Fabric1", + "forceUnisolateStatus": "0", "isolateStatus": "False", "lanes": "1" }, "FABRIC_PORT|Fabric2": { "alias": "Fabric2", + "forceUnisolateStatus": "0", "isolateStatus": "False", "lanes": "2" },