Fix multi-asic behaviour for ecnconfig (sonic-net#3062)

* Fixes multi-asic behaviour for ecnconfig.

Previously, ecnconfig -l was not behaving correctly on multi-asic devices,
as the '-n' namespace option was not available, and correct namespaces were
not traversed on multi-asic devices to retrieve correct results for ecnconfig.
This change fixes multi-asic behaviour of CLI commands that rely on ecnconfig.

* tests cleanup

* Remove wred on lossy

* Addendum to previous commit regarding wred lossy

* Enhancements to multi-asic support in ecnconfig

- Removed unix socket and fixed failing unit test
- Replace argparse with click
- Add multi-asic support for get and set queue
- Add multi-asic support for set threshold and prob
- Modify test framework to support multi-asic
- Use multi_asic decorators
- Resolve precommit errors

---------

Co-authored-by: rdjeric <rdjeric@arista.com>
Co-authored-by: arista-hpandya <hpandya@arista.com>

config/main.py

@@ -6399,7 +6399,8 @@ def remove_reasons(counter_name, reasons, verbose):
 @click.option('-ydrop', metavar='<yellow drop probability>', type=click.IntRange(0, 100), help="Set yellow drop probability")
 @click.option('-gdrop', metavar='<green drop probability>', type=click.IntRange(0, 100), help="Set green drop probability")
 @click.option('-v', '--verbose', is_flag=True, help="Enable verbose output")
-def ecn(profile, rmax, rmin, ymax, ymin, gmax, gmin, rdrop, ydrop, gdrop, verbose):
+@multi_asic_util.multi_asic_click_option_namespace
+def ecn(profile, rmax, rmin, ymax, ymin, gmax, gmin, rdrop, ydrop, gdrop, verbose, namespace):
  """ECN-related configuration tasks"""
  log.log_info("'ecn -profile {}' executing...".format(profile))
  command = ['ecnconfig', '-p', str(profile)]
@@ -6413,6 +6414,8 @@ def ecn(profile, rmax, rmin, ymax, ymin, gmax, gmin, rdrop, ydrop, gdrop, verbos
  if ydrop is not None: command += ['-ydrop', str(ydrop)]
  if gdrop is not None: command += ['-gdrop', str(gdrop)]
  if verbose: command += ["-vv"]
+ if namespace is not None:
+ command += ['-n', str(namespace)]
  clicommon.run_command(command, display_cmd=verbose)
 
 

show/main.py

@@ -2008,10 +2008,13 @@ def policer(policer_name, verbose):
 # 'ecn' command ("show ecn")
 #
 @cli.command('ecn')
+@multi_asic_util.multi_asic_click_option_namespace
 @click.option('--verbose', is_flag=True, help="Enable verbose output")
-def ecn(verbose):
+def ecn(namespace, verbose):
  """Show ECN configuration"""
  cmd = ['ecnconfig', '-l']
+ if namespace is not None:
+ cmd += ['-n', str(namespace)]
  run_command(cmd, display_cmd=verbose)
 
 

tests/ecn_test.py

@@ -6,25 +6,123 @@
 from click.testing import CliRunner
 
 import config.main as config
-from .ecn_input.ecn_test_vectors import *
+from .ecn_input.ecn_test_vectors import testData
 from .utils import get_result_and_return_code
 from utilities_common.db import Db
 import show.main as show
 
+# Constants
+ARGS_DELIMITER = ','
+NAMESPACE_DELIMITER = '-'
+
 test_path = os.path.dirname(os.path.abspath(__file__))
 modules_path = os.path.dirname(test_path)
 scripts_path = os.path.join(modules_path, "scripts")
 sys.path.insert(0, test_path)
 sys.path.insert(0, modules_path)
 
 
-class TestEcnConfig(object):
+class TestEcnConfigBase(object):
  @classmethod
  def setup_class(cls):
+ print("SETUP")
  os.environ["PATH"] += os.pathsep + scripts_path
  os.environ['UTILITIES_UNIT_TESTING'] = "2"
- print("SETUP")
 
+ def process_cmp_args(self, cmp_args):
+ """
+ The arguments are a string marked by delimiters
+ Arguments marked as 'None', are treated as None objects
+ First arg is always a collection of namespaces
+ """
+
+ args = cmp_args.split(ARGS_DELIMITER)
+ args = [None if arg == "None" else arg for arg in args]
+ args[0] = args[0].split(NAMESPACE_DELIMITER)
+ return args
+
+ def verify_profile(self, queue_db_entry, profile, value):
+ if profile is not None:
+ assert queue_db_entry[profile] == value
+ else:
+ assert profile not in queue_db_entry,\
+ "Profile needs to be fully removed from table to propagate NULL OID to SAI"
+
+ def executor(self, input):
+ runner = CliRunner()
+
+ if 'db_table' in input:
+ db = Db()
+ data_list = list(db.cfgdb.get_table(input['db_table']))
+ input['rc_msg'] = input['rc_msg'].format(",".join(data_list))
+
+ if 'show' in input['cmd']:
+ exec_cmd = show.cli.commands["ecn"]
+ result = runner.invoke(exec_cmd, input['args'])
+ exit_code = result.exit_code
+ output = result.output
+ elif 'q_cmd' in input['cmd'] or 'show_masic' in input['cmd'] or 'config_masic' in input['cmd']:
+ exit_code, output = get_result_and_return_code(["ecnconfig"] + input['args'])
+ else:
+ exec_cmd = config.config.commands["ecn"]
+ result = runner.invoke(exec_cmd, input['args'])
+ exit_code = result.exit_code
+ output = result.output
+
+ print(exit_code)
+ print(output)
+
+ if input['rc'] == 0:
+ assert exit_code == 0
+ else:
+ assert exit_code != 0
+
+ if 'cmp_args' in input:
+ fd = open('/tmp/ecnconfig', 'r')
+ cmp_data = json.load(fd)
+
+ # Verify queue assignments
+ if 'cmp_q_args' in input:
+ namespaces, profile, value = self.process_cmp_args(input['cmp_args'][0])
+ for namespace in namespaces:
+ for key in cmp_data[namespace]:
+ queue_idx = ast.literal_eval(key)[-1]
+ if queue_idx in input['cmp_q_args']:
+ self.verify_profile(cmp_data[namespace][key], profile, value)
+
+ # other_q helps verify two different queue assignments
+ if 'other_q' in input:
+ namespaces1, profile1, value1 = self.process_cmp_args(input['cmp_args'][-1])
+ for namespace1 in namespaces1:
+ for key in cmp_data[namespace1]:
+ queue_idx = ast.literal_eval(key)[-1]
+ if 'other_q' in input and queue_idx in input['other_q']:
+ self.verify_profile(cmp_data[namespace1][key], profile1, value1)
+ # Verify non-queue related assignments
+ else:
+ for args in input['cmp_args']:
+ namespaces, profile, name, value = self.process_cmp_args(args)
+ for namespace in namespaces:
+ assert(cmp_data[namespace][profile][name] == value)
+ fd.close()
+
+ if 'rc_msg' in input:
+ assert input['rc_msg'] in output
+
+ if 'rc_output' in input:
+ assert output == input['rc_output']
+
+ @classmethod
+ def teardown_class(cls):
+ os.environ['PATH'] = os.pathsep.join(os.environ['PATH'].split(os.pathsep)[:-1])
+ os.environ['UTILITIES_UNIT_TESTING'] = "0"
+
+ if os.path.isfile('/tmp/ecnconfig'):
+ os.remove('/tmp/ecnconfig')
+ print("TEARDOWN")
+
+
+class TestEcnConfig(TestEcnConfigBase):
  def test_ecn_show_config(self):
  self.executor(testData['ecn_show_config'])
 
@@ -123,77 +221,3 @@ def test_ecn_queue_set_all_on_verbose(self):
 
  def test_ecn_queue_set_lossy_q_on(self):
  self.executor(testData['ecn_cfg_lossy_q_on'])
-
- def process_cmp_args(self, cmp_args):
- if cmp_args is None:
- return (None, None)
- return cmp_args.split(',')
-
- def verify_profile(self, queue_db_entry, profile, value):
- if profile != None:
- assert queue_db_entry[profile] == value
- else:
- assert profile not in queue_db_entry,\
- "Profile needs to be fully removed from table to propagate NULL OID to SAI"
-
- def executor(self, input):
- runner = CliRunner()
-
- if 'db_table' in input:
- db = Db()
- data_list = list(db.cfgdb.get_table(input['db_table']))
- input['rc_msg'] = input['rc_msg'].format(",".join(data_list))
-
- if 'show' in input['cmd']:
- exec_cmd = show.cli.commands["ecn"]
- result = runner.invoke(exec_cmd, input['args'])
- exit_code = result.exit_code
- output = result.output
- elif 'q_cmd' in input['cmd'] :
- exit_code, output = get_result_and_return_code(["ecnconfig"] + input['args'])
- else:
- exec_cmd = config.config.commands["ecn"]
- result = runner.invoke(exec_cmd, input['args'])
- exit_code = result.exit_code
- output = result.output
-
- print(exit_code)
- print(output)
-
- if input['rc'] == 0:
- assert exit_code == 0
- else:
- assert exit_code != 0
-
- if 'cmp_args' in input:
- fd = open('/tmp/ecnconfig', 'r')
- cmp_data = json.load(fd)
- if 'cmp_q_args' in input:
- profile, value = self.process_cmp_args(input['cmp_args'][0])
- if 'other_q' in input:
- profile1, value1 = self.process_cmp_args(input['cmp_args'][-1])
- for key in cmp_data:
- queue_idx = ast.literal_eval(key)[-1]
- if queue_idx in input['cmp_q_args']:
- self.verify_profile(cmp_data[key], profile, value)
- if 'other_q' in input and queue_idx in input['other_q']:
- self.verify_profile(cmp_data[key], profile1, value1)
- else:
- for args in input['cmp_args']:
- profile, name, value = args.split(',')
- assert(cmp_data[profile][name] == value)
- fd.close()
-
- if 'rc_msg' in input:
- assert input['rc_msg'] in output
-
- if 'rc_output' in input:
- assert output == input['rc_output']
-
- @classmethod
- def teardown_class(cls):
- os.environ['PATH'] = os.pathsep.join(os.environ['PATH'].split(os.pathsep)[:-1])
- os.environ['UTILITIES_UNIT_TESTING'] = "0"
- if os.path.isfile('/tmp/ecnconfig'):
- os.remove('/tmp/ecnconfig')
- print("TEARDOWN")

tests/mock_tables/asic0/config_db.json

@@ -303,5 +303,28 @@
  "SYSLOG_CONFIG_FEATURE|database": {
  "rate_limit_interval": "222",
  "rate_limit_burst": "22222"
+ },
+ "WRED_PROFILE|AZURE_LOSSLESS": {
+ "red_max_threshold": "2097152",
+ "ecn": "ecn_all",
+ "green_min_threshold": "1048576",
+ "red_min_threshold": "1048576",
+ "yellow_min_threshold": "1048576",
+ "green_max_threshold": "2097152",
+ "green_drop_probability": "5",
+ "yellow_max_threshold": "2097152",
+ "yellow_drop_probability": "5",
+ "red_drop_probability": "5"
+ },
+ "DEVICE_NEIGHBOR|Ethernet4": {
+ "name": "Serverss0",
+ "port": "eth0"
+ },
+ "QUEUE|Ethernet4|0": {
+ "scheduler": "[SCHEDULAR|scheduler.0]"
+ },
+ "QUEUE|Ethernet4|1": {
+ "scheduler": "[SCHEDULAR|scheduler.0]",
+ "wred_profile": "AZURE_LOSSLESS"
  }
 }

tests/mock_tables/asic1/config_db.json

@@ -242,5 +242,25 @@
  "SYSLOG_CONFIG_FEATURE|database": {
  "rate_limit_interval": "555",
  "rate_limit_burst": "55555"
+ },
+ "WRED_PROFILE|AZURE_LOSSY": {
+ "red_max_threshold":"32760",
+ "red_min_threshold":"4095",
+ "yellow_max_threshold":"32760",
+ "yellow_min_threshold":"4095",
+ "green_max_threshold": "32760",
+ "green_min_threshold": "4095",
+ "yellow_drop_probability": "2"
+ },
+ "DEVICE_NEIGHBOR|Ethernet0": {
+ "name": "Servers",
+ "port": "eth0"
+ },
+ "QUEUE|Ethernet0|0": {
+ "scheduler": "[SCHEDULAR|scheduler.0]",
+ "wred_profile": "AZURE_LOSSLESS"
+ },
+ "QUEUE|Ethernet0|1": {
+ "scheduler": "[SCHEDULAR|scheduler.0]"
  }
 }

tests/multi_asic_ecnconfig_test.py

@@ -0,0 +1,64 @@
+import os
+import sys
+from .ecn_test import TestEcnConfigBase
+from .ecn_input.ecn_test_vectors import testData
+
+root_path = os.path.dirname(os.path.abspath(__file__))
+modules_path = os.path.dirname(root_path)
+scripts_path = os.path.join(modules_path, "scripts")
+sys.path.insert(0, root_path)
+sys.path.insert(0, modules_path)
+
+
+class TestEcnConfigMultiAsic(TestEcnConfigBase):
+ @classmethod
+ def setup_class(cls):
+ super().setup_class()
+ os.environ["UTILITIES_UNIT_TESTING_TOPOLOGY"] = "multi_asic"
+
+ def test_ecn_show_config_all_masic(self):
+ self.executor(testData['ecn_show_config_masic'])
+
+ def test_ecn_show_config_all_verbose_masic(self):
+ self.executor(testData['test_ecn_show_config_verbose_masic'])
+
+ def test_ecn_show_config_one_masic(self):
+ self.executor(testData['test_ecn_show_config_namespace'])
+
+ def test_ecn_show_config_one_verbose_masic(self):
+ self.executor(testData['test_ecn_show_config_namespace_verbose'])
+
+ def test_ecn_config_change_other_threshold_masic(self):
+ self.executor(testData['ecn_cfg_threshold_masic'])
+
+ def test_ecn_config_change_other_prob_masic(self):
+ self.executor(testData['ecn_cfg_probability_masic'])
+
+ def test_ecn_config_change_gdrop_verbose_all_masic(self):
+ self.executor(testData['ecn_cfg_gdrop_verbose_all_masic'])
+
+ def test_ecn_config_multi_set_verbose_all_masic(self):
+ self.executor(testData['ecn_cfg_multi_set_verbose_all_masic'])
+
+ def test_ecn_queue_get_masic(self):
+ self.executor(testData['ecn_q_get_masic'])
+
+ def test_ecn_queue_get_verbose_masic(self):
+ self.executor(testData['ecn_q_get_verbose_masic'])
+
+ def test_ecn_queue_get_all_masic(self):
+ self.executor(testData['ecn_q_get_all_ns_masic'])
+
+ def test_ecn_queue_get_all_verbose_masic(self):
+ self.executor(testData['ecn_q_get_all_ns_verbose_masic'])
+
+ def test_ecn_q_set_off_all_masic(self):
+ self.executor(testData['ecn_cfg_q_all_ns_off_masic'])
+
+ def test_ecn_q_set_off_one_masic(self):
+ self.executor(testData['ecn_cfg_q_one_ns_off_verbose_masic'])
+
+ @classmethod
+ def teardown_class(cls):
+ super().teardown_class()
+ os.environ["UTILITIES_UNIT_TESTING_TOPOLOGY"] = ""