Skip to content

Commit

Permalink
Several fixes and updates for ecnconfig (sonic-net#2889)
Browse files Browse the repository at this point in the history 
* Update DB entry table ref syntax.

Allow non-lossless queues to be queried and updated.

Remove entire wred_profile entry to activate a change to the SAI layer.

If wred_profile was the only value and was removed, delete entire key entry.

Check that a port is available before attempting an index access.

* Update mock DB syntax, fix UT.

Update mock DB syntax for wred_profile entries.

Add UT support for expecting no wred profile to exist.

Fix UT expectations.

* Add new lossy queue tests.

* rm comment.

* rm another comment.

---------

Co-authored-by: Kevin Wang <[email protected]>
  • Loading branch information
@rbpittman @kevinskwang @pdhruv-marvell
2 people authored and pdhruv-marvell committed Aug 23, 2023
1 parent 02e18aa commit 083e536
Show file tree
Hide file tree
Showing 4 changed files with 126 additions and 89 deletions.
28 changes: 17 additions & 11 deletions scripts/ecnconfig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,10 +86,7 @@ PORT_TABLE_NAME = "PORT"
QUEUE_TABLE_NAME = "QUEUE"
DEVICE_NEIGHBOR_TABLE_NAME = "DEVICE_NEIGHBOR"
FIELD = "wred_profile"
ON = "[WRED_PROFILE|AZURE_LOSSLESS]"
OFF = "[]"

lossless_queues = ['3', '4']
ON = "AZURE_LOSSLESS"

def chk_exec_privilege():
if os.geteuid() != 0 and os.environ.get("UTILITIES_UNIT_TESTING", "0") != "2":
Expand Down Expand Up @@ -202,7 +199,6 @@ class EcnQ(object):
def __init__(self, queues, filename, verbose):
self.ports_key = []
self.queues = queues.split(',')
self.validate_queues()
self.filename = filename
self.verbose = verbose

Expand All @@ -215,16 +211,15 @@ class EcnQ(object):

self.gen_ports_key()

def validate_queues(self):
for q in self.queues:
if q not in lossless_queues:
sys.exit('Invalid queue index: %s' % q)

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 = list(port_table.keys())

# Verify at least one port is available
if len(self.ports_key) == 0:
raise Exception("No active ports detected in table '{}'".format(DEVICE_NEIGHBOR_TABLE_NAME))

# 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(
Expand All @@ -245,7 +240,18 @@ class EcnQ(object):
print("%s ECN on %s queue %s" % ("Enable" if enable else "Disable", ','.join(self.ports_key), queue))
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})
entry = self.config_db.get_entry(QUEUE_TABLE_NAME, key)
if enable:
entry[FIELD] = ON
else:
# Remove entry to propagate SAI change
if FIELD in entry:
del entry[FIELD]
# If entry is now empty, remove the key
if entry == {}:
self.config_db.mod_entry(QUEUE_TABLE_NAME, key, None)
else:
self.config_db.set_entry(QUEUE_TABLE_NAME, key, entry)
self.dump_table_info()

def get(self):
Expand Down
39 changes: 26 additions & 13 deletions tests/ecn_input/ecn_test_vectors.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,57 +122,64 @@
'args' : ['-q', '3'],
'rc' : 0,
'rc_msg' : 'ECN status:\nqueue 3: on\n',
'cmp_args' : ['wred_profile,[WRED_PROFILE|AZURE_LOSSLESS]'],
'cmp_args' : ['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_args' : ['wred_profile,AZURE_LOSSLESS'],
'cmp_q_args' : ['3', '4'],
'db_table' : 'DEVICE_NEIGHBOR'
},
'ecn_lossy_q_get' : {'cmd' : ['q_cmd'],
'args' : ['-q', '2'],
'rc' : 0,
'rc_msg' : 'ECN status:\nqueue 2: off\n',
'cmp_args' : [None],
'cmp_q_args' : ['2']
},
'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_args' : ['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_args' : ['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_args' : [None],
'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_args' : [None],
'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_args' : [None, '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_args' : [None, 'wred_profile,AZURE_LOSSLESS'],
'cmp_q_args' : ['3'],
'other_q' : ['4'],
'db_table' : 'DEVICE_NEIGHBOR',
Expand All @@ -181,29 +188,35 @@
'ecn_cfg_q_all_on' : {'cmd' : ['q_cmd'],
'args' : ['-q', '3,4', 'on'],
'rc' : 0,
'cmp_args' : ['wred_profile,[WRED_PROFILE|AZURE_LOSSLESS]'],
'cmp_args' : ['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_args' : ['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_args' : ['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_args' : ['wred_profile,AZURE_LOSSLESS'],
'cmp_q_args' : ['3', '4'],
'db_table' : 'DEVICE_NEIGHBOR',
'rc_msg' : 'Enable ECN on {0} queue 4'
}
},
'ecn_cfg_lossy_q_on' : {'cmd' : ['q_cmd'],
'args' : ['-q', '0,1,2,5,6,7', 'on'],
'rc' : 0,
'cmp_args' : ['wred_profile,AZURE_LOSSLESS'],
'cmp_q_args' : ['0', '1', '2', '5', '6', '7']
}
}
32 changes: 25 additions & 7 deletions tests/ecn_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,9 @@ def test_ecn_queue_get(self):
def test_ecn_queue_get_verbose(self):
self.executor(testData['ecn_q_get_verbose'])

def test_ecn_queue_get_lossy(self):
self.executor(testData['ecn_lossy_q_get'])

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

Expand Down Expand Up @@ -118,6 +121,21 @@ def test_ecn_queue_set_all_on(self):
def test_ecn_queue_set_all_on_verbose(self):
self.executor(testData['ecn_cfg_q_all_on_verbose'])

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()

Expand Down Expand Up @@ -150,16 +168,16 @@ def executor(self, input):
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 = input['cmp_args'][-1].split(',')
profile, value = input['cmp_args'][0].split(',')
profile1, value1 = self.process_cmp_args(input['cmp_args'][-1])
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)
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(',')
Expand Down
Loading

0 comments on commit 083e536

Please sign in to comment.