Skip to content

Commit

Permalink
[ecnconfig] Fix exception seen during display and add unit tests (#1784)
Browse files Browse the repository at this point in the history 
Fixes #8323 and added unit tests for all ecn queue configs. This regression was caused because the current ecn unit tests didn't cover the queue configs.

Signed-off-by: Neetha John <nejo@microsoft.com>

How to verify it
Ran tests/ecn_test.py and all cases passed
  • Loading branch information
@neethajohn
neethajohn committed Aug 30, 2021
1 parent 83309c6 commit 3e8626e
Show file tree
Hide file tree
Showing 4 changed files with 880 additions and 17 deletions.
22 changes: 18 additions & 4 deletions scripts/ecnconfig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -199,10 +199,11 @@ class EcnQ(object):
"""
Process ecn on/off on queues
"""
def __init__(self, queues, verbose):
def __init__(self, queues, filename, verbose):
self.ports_key = []
self.queues = queues.split(',')
self.validate_queues()
self.filename = filename
self.verbose = verbose

# Set up db connections
Expand All @@ -222,14 +223,20 @@ class EcnQ(object):
def gen_ports_key(self):
if self.ports_key is not None:
port_table = self.config_db.get_table(DEVICE_NEIGHBOR_TABLE_NAME)
self.ports_key = port_table.keys()
self.ports_key = list(port_table.keys())

# In multi-ASIC platforms backend ethernet ports are identified as
# 'Ethernet-BPxy'. Add 1024 to sort backend ports to the end.
self.ports_key.sort(
key = lambda k: int(k[8:]) if "BP" not in k else int(k[11:]) + 1024
)

def dump_table_info(self):
if self.filename is not None:
q_table = self.config_db.get_table(QUEUE_TABLE_NAME)
with open(self.filename, "w") as fd:
json.dump({repr(x):y for x, y in q_table.items()}, fd)

def set(self, enable):
chk_exec_privilege()

Expand All @@ -239,6 +246,7 @@ class EcnQ(object):
for port_key in self.ports_key:
key = '|'.join([port_key, queue])
self.config_db.mod_entry(QUEUE_TABLE_NAME, key, {FIELD: ON if enable else OFF})
self.dump_table_info()

def get(self):
print("ECN status:")
Expand All @@ -256,6 +264,7 @@ class EcnQ(object):
print("%s: on" % (out))
else:
print("%s: off" % (out))
self.dump_table_info()

def main():
parser = argparse.ArgumentParser(description='Show and change:\n'
Expand Down Expand Up @@ -357,10 +366,15 @@ def main():
prof_cfg.set_wred_prob(args.profile, "rdrop", args.red_drop_prob)

elif args.queue:
if len(sys.argv) < (4 if args.verbose else 3):
arg_len_min = 3
if args.filename:
arg_len_min += 1
if args.verbose:
arg_len_min += 1
if len(sys.argv) < arg_len_min:
raise Exception("Input arguments error. Specify at least one queue by index")

q_ecn = EcnQ(args.queue, args.verbose)
q_ecn = EcnQ(args.queue, args.filename, args.verbose)
if not args.command:
q_ecn.get()
else:
Expand Down
106 changes: 105 additions & 1 deletion tests/ecn_input/ecn_test_vectors.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,22 @@
'rc' : 0,
'rc_output': ecn_show_config_output
},
'ecn_show_config_verbose' : {'cmd' : ['q_cmd'],
'args' : ['-l', '-vv'],
'rc' : 0,
'rc_output': ecn_show_config_output + 'Total profiles: 1\n'
},
'ecn_cfg_gmin' : {'cmd' : ['config'],
'args' : ['-profile', 'AZURE_LOSSLESS', '-gmin', '1048600'],
'rc' : 0,
'cmp_args' : ['AZURE_LOSSLESS,green_min_threshold,1048600']
},
'ecn_cfg_gmin_verbose' : {'cmd' : ['config'],
'args' : ['-profile', 'AZURE_LOSSLESS', '-gmin', '1048600', '-vv'],
'rc' : 0,
'cmp_args' : ['AZURE_LOSSLESS,green_min_threshold,1048600'],
'rc_output' : 'Running command: ecnconfig -p AZURE_LOSSLESS -gmin 1048600 -vv\nSetting green_min_threshold value to 1048600\n'
},
'ecn_cfg_gmax' : {'cmd' : ['config'],
'args' : ['-profile', 'AZURE_LOSSLESS', '-gmax', '2097153'],
'rc' : 0,
Expand Down Expand Up @@ -69,6 +80,12 @@
'rc' : 0,
'cmp_args' : ['AZURE_LOSSLESS,green_drop_probability,12']
},
'ecn_cfg_gdrop_verbose' : {'cmd' : ['config'],
'args' : ['-profile', 'AZURE_LOSSLESS', '-gdrop', '12', '-vv'],
'rc' : 0,
'cmp_args' : ['AZURE_LOSSLESS,green_drop_probability,12'],
'rc_output' : 'Running command: ecnconfig -p AZURE_LOSSLESS -gdrop 12 -vv\nSetting green_drop_probability value to 12%\n'
},
'ecn_cfg_multi_set' : {'cmd' : ['config'],
'args' : ['-profile', 'AZURE_LOSSLESS', '-gdrop', '12', '-gmax', '2097153'],
'rc' : 0,
Expand Down Expand Up @@ -100,6 +117,93 @@
'args' : ['-profile', 'AZURE_LOSSLESS', '-rdrop', '105'],
'rc' : 1,
'rc_msg' : 'Invalid value for "-rdrop": 105 is not in the valid range of 0 to 100'
},
'ecn_q_get' : {'cmd' : ['q_cmd'],
'args' : ['-q', '3'],
'rc' : 0,
'rc_msg' : 'ECN status:\nqueue 3: on\n',
'cmp_args' : ['wred_profile,[WRED_PROFILE|AZURE_LOSSLESS]'],
'cmp_q_args' : ['3', '4']
},
'ecn_q_get_verbose' : {'cmd' : ['q_cmd'],
'args' : ['-q', '3', '-vv'],
'rc' : 0,
'rc_msg' : 'ECN status:\n{0} queue 3: on\n',
'cmp_args' : ['wred_profile,[WRED_PROFILE|AZURE_LOSSLESS]'],
'cmp_q_args' : ['3', '4'],
'db_table' : 'DEVICE_NEIGHBOR'
},
'ecn_q_all_get_verbose' : {'cmd' : ['q_cmd'],
'args' : ['-q', '3,4', '-vv'],
'rc' : 0,
'rc_msg' : 'ECN status:\n{0} queue 3: on\n{0} queue 4: on\n',
'cmp_args' : ['wred_profile,[WRED_PROFILE|AZURE_LOSSLESS]'],
'cmp_q_args' : ['3', '4'],
'db_table' : 'DEVICE_NEIGHBOR'
},
'ecn_q_all_get' : {'cmd' : ['q_cmd'],
'args' : ['-q', '3,4'],
'rc' : 0,
'rc_msg' : 'ECN status:\nqueue 3: on\nqueue 4: on\n',
'cmp_args' : ['wred_profile,[WRED_PROFILE|AZURE_LOSSLESS]'],
'cmp_q_args' : ['3', '4']
},
'ecn_cfg_q_all_off' : {'cmd' : ['q_cmd'],
'args' : ['-q', '3,4', 'off'],
'rc' : 0,
'cmp_args' : ['wred_profile,[]'],
'cmp_q_args' : ['3', '4']
},
'ecn_cfg_q_all_off_verbose' : {'cmd' : ['q_cmd'],
'args' : ['-q', '3,4', 'off', '-vv'],
'rc' : 0,
'cmp_args' : ['wred_profile,[]'],
'cmp_q_args' : ['3', '4'],
'db_table' : 'DEVICE_NEIGHBOR',
'rc_msg' : 'Disable ECN on {0} queue 3\nDisable ECN on {0} queue 4'
},
'ecn_cfg_q_off' : {'cmd' : ['q_cmd'],
'args' : ['-q', '3', 'off'],
'rc' : 0,
'cmp_args' : ['wred_profile,[]', 'wred_profile,[WRED_PROFILE|AZURE_LOSSLESS]'],
'cmp_q_args' : ['3'],
'other_q' : ['4']
},
'ecn_cfg_q_off_verbose' : {'cmd' : ['q_cmd'],
'args' : ['-q', '3', 'off', '-vv'],
'rc' : 0,
'cmp_args' : ['wred_profile,[]', 'wred_profile,[WRED_PROFILE|AZURE_LOSSLESS]'],
'cmp_q_args' : ['3'],
'other_q' : ['4'],
'db_table' : 'DEVICE_NEIGHBOR',
'rc_msg' : 'Disable ECN on {0} queue 3'
},
'ecn_cfg_q_all_on' : {'cmd' : ['q_cmd'],
'args' : ['-q', '3,4', 'on'],
'rc' : 0,
'cmp_args' : ['wred_profile,[WRED_PROFILE|AZURE_LOSSLESS]'],
'cmp_q_args' : ['3', '4']
},
'ecn_cfg_q_all_on_verbose' : {'cmd' : ['q_cmd'],
'args' : ['-q', '3,4', 'on', '-vv'],
'rc' : 0,
'cmp_args' : ['wred_profile,[WRED_PROFILE|AZURE_LOSSLESS]'],
'cmp_q_args' : ['3', '4'],
'db_table' : 'DEVICE_NEIGHBOR',
'rc_msg' : 'Enable ECN on {0} queue 3\nEnable ECN on {0} queue 4'
},
'ecn_cfg_q_on' : {'cmd' : ['q_cmd'],
'args' : ['-q', '4', 'on'],
'rc' : 0,
'cmp_args' : ['wred_profile,[WRED_PROFILE|AZURE_LOSSLESS]'],
'cmp_q_args' : ['3', '4']
},
'ecn_cfg_q_on_verbose' : {'cmd' : ['q_cmd'],
'args' : ['-q', '4', 'on', '-vv'],
'rc' : 0,
'cmp_args' : ['wred_profile,[WRED_PROFILE|AZURE_LOSSLESS]'],
'cmp_q_args' : ['3', '4'],
'db_table' : 'DEVICE_NEIGHBOR',
'rc_msg' : 'Enable ECN on {0} queue 4'
}

}
94 changes: 82 additions & 12 deletions tests/ecn_test.py
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import ast
import json
import os
import sys
Expand All @@ -6,6 +7,8 @@

import config.main as config
from .ecn_input.ecn_test_vectors import *
from .utils import get_result_and_return_code
from utilities_common.db import Db
import show.main as show

test_path = os.path.dirname(os.path.abspath(__file__))
Expand All @@ -25,9 +28,15 @@ def setup_class(cls):
def test_ecn_show_config(self):
self.executor(testData['ecn_show_config'])

def test_ecn_show_config_verbose(self):
self.executor(testData['ecn_show_config_verbose'])

def test_ecn_config_gmin(self):
self.executor(testData['ecn_cfg_gmin'])

def test_ecn_config_gmin_verbose(self):
self.executor(testData['ecn_cfg_gmin_verbose'])

def test_ecn_config_gmax(self):
self.executor(testData['ecn_cfg_gmax'])

Expand All @@ -46,6 +55,9 @@ def test_ecn_config_rmax(self):
def test_ecn_config_gdrop(self):
self.executor(testData['ecn_cfg_gdrop'])

def test_ecn_config_gdrop_verbose(self):
self.executor(testData['ecn_cfg_gdrop_verbose'])

def test_ecn_config_ydrop(self):
self.executor(testData['ecn_cfg_ydrop'])

Expand All @@ -70,37 +82,95 @@ def test_ecn_config_rmax_invalid(self):
def test_ecn_config_rdrop_invalid(self):
self.executor(testData['ecn_cfg_rdrop_invalid'])

def test_ecn_queue_get(self):
self.executor(testData['ecn_q_get'])

def test_ecn_queue_get_verbose(self):
self.executor(testData['ecn_q_get_verbose'])

def test_ecn_all_queue_get(self):
self.executor(testData['ecn_q_all_get'])

def test_ecn_queue_all_get_verbose(self):
self.executor(testData['ecn_q_all_get_verbose'])

def test_ecn_queue_set_q_off(self):
self.executor(testData['ecn_cfg_q_off'])

def test_ecn_queue_set_q_off_verbose(self):
self.executor(testData['ecn_cfg_q_off_verbose'])

def test_ecn_queue_set_all_off(self):
self.executor(testData['ecn_cfg_q_all_off'])

def test_ecn_queue_set_all_off_verbose(self):
self.executor(testData['ecn_cfg_q_all_off_verbose'])

def test_ecn_queue_set_q_on(self):
self.executor(testData['ecn_cfg_q_on'])

def test_ecn_queue_set_q_on_verbose(self):
self.executor(testData['ecn_cfg_q_on_verbose'])

def test_ecn_queue_set_all_on(self):
self.executor(testData['ecn_cfg_q_all_on'])

def test_ecn_queue_set_all_on_verbose(self):
self.executor(testData['ecn_cfg_q_all_on_verbose'])

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 {}".format(" ".join(input['args'])))
else:
exec_cmd = config.config.commands["ecn"]
result = runner.invoke(exec_cmd, input['args'])
exit_code = result.exit_code
output = result.output

result = runner.invoke(exec_cmd, input['args'])

print(result.exit_code)
print(result.output)
print(exit_code)
print(output)

if input['rc'] == 0:
assert result.exit_code == 0
assert exit_code == 0
else:
assert result.exit_code != 0
assert exit_code != 0

if 'cmp_args' in input:
fd = open('/tmp/ecnconfig', 'r')
prof_data = json.load(fd)
for args in input['cmp_args']:
profile, name, value = args.split(',')
assert(prof_data[profile][name] == value)
cmp_data = json.load(fd)

if 'cmp_q_args' in input:
if 'other_q' in input:
profile1, value1 = input['cmp_args'][-1].split(',')
profile, value = input['cmp_args'][0].split(',')
for key in cmp_data:
if ast.literal_eval(key)[-1] in input['cmp_q_args']:
assert(cmp_data[key][profile] == value)
if 'other_q' in input and ast.literal_eval(key)[-1] in input['other_q']:
assert(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 result.output
assert input['rc_msg'] in output

if 'rc_output' in input:
assert result.output == input['rc_output']
assert output == input['rc_output']

@classmethod
def teardown_class(cls):
Expand Down
Loading

0 comments on commit 3e8626e

Please sign in to comment.