From 64473080c82a01c599ba1e3b7152744e7f78d49b Mon Sep 17 00:00:00 2001 From: bktsim <144830673+bktsim-arista@users.noreply.github.com> Date: Fri, 31 May 2024 13:35:33 -0700 Subject: [PATCH] [chassis][multi-asic] Make PFC commands use a class (#3057) What I did This change puts contents originally in pfc/main.py into a class, to support the usage of the multi-asic helper in a future change. This change is required, as multi-asic helper being used expects members self.config_db and self.db to exist when a function with the decorator run_on_multi_asic is called. The multi-asic class helper will be used to add multi-asic support to pfc commands in a following pull request. This is a part of the set of changes being pushed for sonic-net/sonic-buildimage#15148 How I did it Moved contents of PFC commands into a class. There are no functional changes. Co-authored-by: rdjeric Co-authored-by: Kenneth Cheung --- pfc/main.py | 225 ++++++++++++++------------ tests/pfc_input/assert_show_output.py | 82 ++++++++++ tests/pfc_test.py | 81 ++++++++++ 3 files changed, 282 insertions(+), 106 deletions(-) create mode 100644 tests/pfc_input/assert_show_output.py create mode 100644 tests/pfc_test.py diff --git a/pfc/main.py b/pfc/main.py index b31d3c755e..f0b376e242 100644 --- a/pfc/main.py +++ b/pfc/main.py @@ -1,5 +1,4 @@ #!/usr/bin/env python3 - import click from swsscommon.swsscommon import ConfigDBConnector from tabulate import tabulate @@ -8,153 +7,167 @@ ALL_PRIORITIES = [str(x) for x in range(8)] PRIORITY_STATUS = ['on', 'off'] -def configPfcAsym(interface, pfc_asym): - """ - PFC handler to configure asymmentric PFC. - """ - configdb = ConfigDBConnector() - configdb.connect() - configdb.mod_entry("PORT", interface, {'pfc_asym': pfc_asym}) +class Pfc(object): + def __init__(self, cfgdb=None): + self.cfgdb = cfgdb + def configPfcAsym(self, interface, pfc_asym): + """ + PFC handler to configure asymmetric PFC. + """ + configdb = ConfigDBConnector() if self.cfgdb is None else self.cfgdb + configdb.connect() -def showPfcAsym(interface): - """ - PFC handler to display asymmetric PFC information. - """ - header = ('Interface', 'Asymmetric') + configdb.mod_entry("PORT", interface, {'pfc_asym': pfc_asym}) - configdb = ConfigDBConnector() - configdb.connect() + def showPfcAsym(self, interface): + """ + PFC handler to display asymmetric PFC information. + """ + header = ('Interface', 'Asymmetric') - if interface: - db_keys = configdb.keys(configdb.CONFIG_DB, 'PORT|{0}'.format(interface)) - else: - db_keys = configdb.keys(configdb.CONFIG_DB, 'PORT|*') + configdb = ConfigDBConnector() if self.cfgdb is None else self.cfgdb + configdb.connect() - table = [] - - for i in db_keys or [None]: - key = None - if i: - key = i.split('|')[-1] + if interface: + db_keys = configdb.keys(configdb.CONFIG_DB, 'PORT|{0}'.format(interface)) + else: + db_keys = configdb.keys(configdb.CONFIG_DB, 'PORT|*') - if key and key.startswith('Ethernet'): - entry = configdb.get_entry('PORT', key) - table.append([key, entry.get('pfc_asym', 'N/A')]) + table = [] - sorted_table = natsorted(table) + for i in db_keys or [None]: + key = None + if i: + key = i.split('|')[-1] - click.echo() - click.echo(tabulate(sorted_table, headers=header, tablefmt="simple", missingval="")) - click.echo() + if key and key.startswith('Ethernet'): + entry = configdb.get_entry('PORT', key) + table.append([key, entry.get('pfc_asym', 'N/A')]) -def configPfcPrio(status, interface, priority): - configdb = ConfigDBConnector() - configdb.connect() + sorted_table = natsorted(table) - if interface not in configdb.get_keys('PORT_QOS_MAP'): - click.echo('Cannot find interface {0}'.format(interface)) - return + click.echo() + click.echo(tabulate(sorted_table, headers=header, tablefmt="simple", missingval="")) + click.echo() - """Current lossless priorities on the interface""" - entry = configdb.get_entry('PORT_QOS_MAP', interface) - enable_prio = entry.get('pfc_enable').split(',') - - """Avoid '' in enable_prio""" - enable_prio = [x.strip() for x in enable_prio if x.strip()] - - if status == 'on' and priority in enable_prio: - click.echo('Priority {0} has already been enabled on {1}'.format(priority, interface)) - return - - if status == 'off' and priority not in enable_prio: - click.echo('Priority {0} is not enabled on {1}'.format(priority, interface)) - return - - if status == 'on': - enable_prio.append(priority) - - else: - enable_prio.remove(priority) - - enable_prio.sort() - configdb.mod_entry("PORT_QOS_MAP", interface, {'pfc_enable': ','.join(enable_prio)}) - - """Show the latest PFC configuration""" - showPfcPrio(interface) - -def showPfcPrio(interface): - """ - PFC handler to display PFC enabled priority information. - """ - header = ('Interface', 'Lossless priorities') - table = [] + def configPfcPrio(self, status, interface, priority): + configdb = ConfigDBConnector() if self.cfgdb is None else self.cfgdb + configdb.connect() + + if interface not in configdb.get_keys('PORT_QOS_MAP'): + click.echo('Cannot find interface {0}'.format(interface)) + return + + """Current lossless priorities on the interface""" + entry = configdb.get_entry('PORT_QOS_MAP', interface) + enable_prio = entry.get('pfc_enable').split(',') + + """Avoid '' in enable_prio""" + enable_prio = [x.strip() for x in enable_prio if x.strip()] + + if status == 'on' and priority in enable_prio: + click.echo('Priority {0} has already been enabled on {1}'.format(priority, interface)) + return + + if status == 'off' and priority not in enable_prio: + click.echo('Priority {0} is not enabled on {1}'.format(priority, interface)) + return + + if status == 'on': + enable_prio.append(priority) + + else: + enable_prio.remove(priority) + + enable_prio.sort() + configdb.mod_entry("PORT_QOS_MAP", interface, {'pfc_enable': ','.join(enable_prio)}) + + """Show the latest PFC configuration""" + self.showPfcPrio(interface) - configdb = ConfigDBConnector() - configdb.connect() - - """Get all the interfaces with QoS map information""" - intfs = configdb.get_keys('PORT_QOS_MAP') - - """The user specifies an interface but we cannot find it""" - if interface and interface not in intfs: - click.echo('Cannot find interface {0}'.format(interface)) - return - - if interface: - intfs = [interface] - - for intf in intfs: - entry = configdb.get_entry('PORT_QOS_MAP', intf) - table.append([intf, entry.get('pfc_enable', 'N/A')]) - - sorted_table = natsorted(table) - click.echo() - click.echo(tabulate(sorted_table, headers=header, tablefmt="simple", missingval="")) - click.echo() + def showPfcPrio(self, interface): + """ + PFC handler to display PFC enabled priority information. + """ + header = ('Interface', 'Lossless priorities') + table = [] + + configdb = ConfigDBConnector() if self.cfgdb is None else self.cfgdb + configdb.connect() + + """Get all the interfaces with QoS map information""" + intfs = configdb.get_keys('PORT_QOS_MAP') + + """The user specifies an interface but we cannot find it""" + if interface and interface not in intfs: + click.echo('Cannot find interface {0}'.format(interface)) + return + + if interface: + intfs = [interface] + + for intf in intfs: + entry = configdb.get_entry('PORT_QOS_MAP', intf) + table.append([intf, entry.get('pfc_enable', 'N/A')]) + + sorted_table = natsorted(table) + click.echo() + click.echo(tabulate(sorted_table, headers=header, tablefmt="simple", missingval="")) + click.echo() @click.group() -def cli(): +@click.pass_context +def cli(ctx): """PFC Command Line""" - pass + # Use the cfgdb object if given as input. + cfgdb = None if ctx.obj is None else ctx.obj.cfgdb + + ctx.obj = {'pfc': Pfc(cfgdb)} @cli.group() -def config(): +@click.pass_context +def config(ctx): """Config PFC""" pass @cli.group() -def show(): +@click.pass_context +def show(ctx): """Show PFC information""" pass @click.command() @click.argument('status', type=click.Choice(PRIORITY_STATUS)) @click.argument('interface', type=click.STRING) -def configAsym(status, interface): +@click.pass_context +def configAsym(ctx, status, interface): """Configure asymmetric PFC on a given port.""" - configPfcAsym(interface, status) + ctx.obj['pfc'].configPfcAsym(interface, status) @click.command() @click.argument('status', type=click.Choice(PRIORITY_STATUS)) @click.argument('interface', type=click.STRING) @click.argument('priority', type=click.Choice(ALL_PRIORITIES)) -def configPrio(status, interface, priority): +@click.pass_context +def configPrio(ctx, status, interface, priority): """Configure PFC on a given priority.""" - configPfcPrio(status, interface, priority) - + ctx.obj['pfc'].configPfcPrio(status, interface, priority) + @click.command() @click.argument('interface', type=click.STRING, required=False) -def showAsym(interface): +@click.pass_context +def showAsym(ctx, interface): """Show asymmetric PFC information""" - showPfcAsym(interface) + ctx.obj['pfc'].showPfcAsym(interface) @click.command() @click.argument('interface', type=click.STRING, required=False) -def showPrio(interface): +@click.pass_context +def showPrio(ctx, interface): """Show PFC priority information""" - showPfcPrio(interface) + ctx.obj['pfc'].showPfcPrio(interface) config.add_command(configAsym, "asymmetric") config.add_command(configPrio, "priority") diff --git a/tests/pfc_input/assert_show_output.py b/tests/pfc_input/assert_show_output.py new file mode 100644 index 0000000000..2406f8b49f --- /dev/null +++ b/tests/pfc_input/assert_show_output.py @@ -0,0 +1,82 @@ +pfc_asym_cannot_find_intf = """\ + +Interface Asymmetric +----------- ------------ + +""" + +pfc_cannot_find_intf = """\ +Cannot find interface Ethernet1234 +""" + +pfc_show_asymmetric_all = """\ + +Interface Asymmetric +----------- ------------ +Ethernet0 off +Ethernet4 off +Ethernet8 off +Ethernet12 off +Ethernet16 off +Ethernet20 off +Ethernet24 off +Ethernet28 off +Ethernet32 off +Ethernet36 off +Ethernet40 off +Ethernet44 off +Ethernet48 off +Ethernet52 off +Ethernet56 off +Ethernet60 off +Ethernet64 off +Ethernet68 off +Ethernet72 off +Ethernet76 off +Ethernet80 off +Ethernet84 off +Ethernet88 off +Ethernet92 off +Ethernet96 off +Ethernet100 off +Ethernet104 off +Ethernet108 off +Ethernet112 off +Ethernet116 off +Ethernet120 off +Ethernet124 off + +""" + +pfc_show_asymmetric_intf = """\ + +Interface Asymmetric +----------- ------------ +Ethernet0 off + +""" + +pfc_show_priority_all = """\ + +Interface Lossless priorities +----------- --------------------- +Ethernet0 3,4 +Ethernet4 3,4 + +""" + +pfc_show_priority_intf = """\ + +Interface Lossless priorities +----------- --------------------- +Ethernet0 3,4 + +""" + +pfc_config_priority_on = """\ + +Interface Lossless priorities +----------- --------------------- +Ethernet0 3,4,5 + +""" diff --git a/tests/pfc_test.py b/tests/pfc_test.py new file mode 100644 index 0000000000..101aa476cc --- /dev/null +++ b/tests/pfc_test.py @@ -0,0 +1,81 @@ +import os +import sys +import pfc.main as pfc +from .pfc_input.assert_show_output import pfc_cannot_find_intf, pfc_show_asymmetric_all, \ + pfc_show_asymmetric_intf, pfc_show_priority_all, pfc_show_priority_intf, \ + pfc_config_priority_on, pfc_asym_cannot_find_intf +from utilities_common.db import Db + +from click.testing import CliRunner +from importlib import reload + +test_path = os.path.dirname(os.path.abspath(__file__)) +modules_path = os.path.dirname(test_path) +scripts_path = os.path.join(modules_path, "pfc") +sys.path.insert(0, test_path) +sys.path.insert(0, modules_path) + + +class TestPfcBase(object): + + def executor(self, cliobj, command, expected_rc=0, expected_output=None, expected_cfgdb_entry=None, + runner=CliRunner()): + db = Db() + result = runner.invoke(cliobj, command, obj=db) + print(result.exit_code) + print(result.output) + + if result.exit_code != expected_rc: + print(result.exception) + assert result.exit_code == expected_rc + + if expected_output: + assert result.output == expected_output + + if expected_cfgdb_entry: + (table, key, field, expected_val) = expected_cfgdb_entry + configdb = db.cfgdb + entry = configdb.get_entry(table, key) + assert entry.get(field) == expected_val + + +class TestPfc(TestPfcBase): + + @classmethod + def setup_class(cls): + from mock_tables import dbconnector + from mock_tables import mock_single_asic + reload(mock_single_asic) + dbconnector.load_namespace_config() + + def test_pfc_show_asymmetric_all(self): + self.executor(pfc.cli, ['show', 'asymmetric'], + expected_output=pfc_show_asymmetric_all) + + def test_pfc_show_asymmetric_intf(self): + self.executor(pfc.cli, ['show', 'asymmetric', 'Ethernet0'], + expected_output=pfc_show_asymmetric_intf) + + def test_pfc_show_asymmetric_intf_fake(self): + self.executor(pfc.cli, ['show', 'asymmetric', 'Ethernet1234'], + expected_output=pfc_asym_cannot_find_intf) + + def test_pfc_show_priority_all(self): + self.executor(pfc.cli, ['show', 'priority'], + expected_output=pfc_show_priority_all) + + def test_pfc_show_priority_intf(self): + self.executor(pfc.cli, ['show', 'priority', 'Ethernet0'], + expected_output=pfc_show_priority_intf) + + def test_pfc_show_priority_intf_fake(self): + self.executor(pfc.cli, ['show', 'priority', 'Ethernet1234'], + expected_output=pfc_cannot_find_intf) + + def test_pfc_config_asymmetric(self): + self.executor(pfc.cli, ['config', 'asymmetric', 'on', 'Ethernet0'], + expected_cfgdb_entry=('PORT', 'Ethernet0', 'pfc_asym', 'on')) + + def test_pfc_config_priority(self): + self.executor(pfc.cli, ['config', 'priority', 'on', 'Ethernet0', '5'], + expected_output=pfc_config_priority_on)