From 4c98df2e0803dcf276d4557fb54acc91037e2615 Mon Sep 17 00:00:00 2001 From: Stephen Sun Date: Mon, 22 Jun 2020 14:01:17 +0800 Subject: [PATCH 1/9] [Dynamic buffer calc] Regression test for dynamic buffer calculation 1. Add test cases for dynamic buffer calculation - changing speed/cable length test - headroom override - additional lossless PG - exceeding the accumulative headroom size of a port - default buffer model 2. Adjust qos test, fetching configuration from: - APPL_DB if there is buffer_model defined, this is the case in master - CONF_DB otherwise, for 201911 3. Additional checking script to test whether the buffer model is as expected. This can be used by warm-reboot Signed-off-by: Stephen Sun --- tests/qos/qos_sai_base.py | 40 +- tests/qos/test_buffer.py | 475 ++++++++++++++++++++++ tests/scripts/check_buffer_dynamic.sh | 9 + tests/scripts/check_buffer_traditional.sh | 9 + 4 files changed, 524 insertions(+), 9 deletions(-) create mode 100644 tests/qos/test_buffer.py create mode 100644 tests/scripts/check_buffer_dynamic.sh create mode 100644 tests/scripts/check_buffer_traditional.sh diff --git a/tests/qos/qos_sai_base.py b/tests/qos/qos_sai_base.py index ec4f60cc906..6d8aad56b00 100644 --- a/tests/qos/qos_sai_base.py +++ b/tests/qos/qos_sai_base.py @@ -21,6 +21,16 @@ class QosSaiBase: TARGET_QUEUE_WRED = 3 TARGET_LOSSY_QUEUE_SCHED = 0 TARGET_LOSSLESS_QUEUE_SCHED = 3 + buffer_model_initialized = False + buffer_model = None + + def __isBufferInApplDb(self, duthost): + if not self.buffer_model_initialized: + self.buffer_model = duthost.shell('redis-cli -n 4 hget "DEVICE_METADATA|localhost" buffer_model')["stdout"] + self.buffer_model_initialized = True + logger.info("Buffer model is {}, buffer tables will be fetched from {}". + format(self.buffer_model or "not defined", "APPL_DB" if self.buffer_model else "CONFIG_DB")) + return self.buffer_model def __runRedisCommandOrAssert(self, duthost, argv=[]): """ @@ -52,10 +62,11 @@ def __computeBufferThreshold(self, duthost, bufferProfile): Returns: Updates bufferProfile with computed buffer threshold """ + db = "0" if self.__isBufferInApplDb(duthost) else "4" pool = bufferProfile["pool"].encode("utf-8").translate(None, "[]") bufferSize = int(self.__runRedisCommandOrAssert( duthost, - argv = ["redis-cli", "-n", "4", "HGET", pool, "size"] + argv = ["redis-cli", "-n", db, "HGET", pool, "size"] )[0]) bufferScale = 2**float(bufferProfile["dynamic_th"]) bufferScale /= (bufferScale + 1) @@ -72,7 +83,7 @@ def __updateVoidRoidParams(self, duthost, bufferProfile): Returns: Updates bufferProfile with VOID/ROID obtained from Redis db """ - bufferPoolName = bufferProfile["pool"].encode("utf-8").translate(None, "[]").replace("BUFFER_POOL|",'') + bufferPoolName = bufferProfile["pool"].encode("utf-8").translate(None, "[]").replace("BUFFER_POOL_TABLE:",'') bufferPoolVoid = self.__runRedisCommandOrAssert( duthost, @@ -100,14 +111,21 @@ def __getBufferProfile(self, request, duthost, table, port, priorityGroup): Returns: bufferProfile (dict): Map of buffer profile attributes """ + + if self.__isBufferInApplDb(duthost): + db = "0" + keystr = "{0}:{1}:{2}".format(table, port, priorityGroup) + else: + db = "4" + keystr = "{0}|{1}|{2}".format(table, port, priorityGroup) bufferProfileName = self.__runRedisCommandOrAssert( duthost, - argv = ["redis-cli", "-n", "4", "HGET", "{0}|{1}|{2}".format(table, port, priorityGroup), "profile"] + argv = ["redis-cli", "-n", db, "HGET", keystr, "profile"] )[0].encode("utf-8").translate(None, "[]") result = self.__runRedisCommandOrAssert( duthost, - argv = ["redis-cli", "-n", "4", "HGETALL", bufferProfileName] + argv = ["redis-cli", "-n", db, "HGETALL", bufferProfileName] ) it = iter(result) bufferProfile = dict(zip(it, it)) @@ -539,7 +557,11 @@ def dutQosConfig(self, duthosts, rand_one_dut_hostname, ingressLosslessProfile, pytest_assert("minigraph_hwsku" in mgFacts, "Could not find DUT SKU") profileName = ingressLosslessProfile["profileName"] - m = re.search("^BUFFER_PROFILE\|pg_lossless_(.*)_profile$", profileName) + if self.__isBufferInApplDb(duthost): + profile_pattern = "^BUFFER_PROFILE_TABLE\:pg_lossless_(.*)_profile$" + else: + profile_pattern = "^BUFFER_PROFILE\|pg_lossless_(.*)_profile" + m = re.search(profile_pattern, profileName) pytest_assert(m.group(1), "Cannot find port speed/cable length") portSpeedCableLength = m.group(1) @@ -680,7 +702,7 @@ def ingressLosslessProfile(self, request, duthosts, rand_one_dut_hostname, dutCo yield self.__getBufferProfile( request, duthost, - "BUFFER_PG", + "BUFFER_PG_TABLE" if self.__isBufferInApplDb(duthost) else "BUFFER_PG", dutConfig["dutInterfaces"][dutConfig["testPorts"]["src_port_id"]], "3-4" ) @@ -703,7 +725,7 @@ def ingressLossyProfile(self, request, duthosts, rand_one_dut_hostname, dutConfi yield self.__getBufferProfile( request, duthost, - "BUFFER_PG", + "BUFFER_PG_TABLE" if self.__isBufferInApplDb(duthost) else "BUFFER_PG", dutConfig["dutInterfaces"][dutConfig["testPorts"]["src_port_id"]], "0" ) @@ -726,7 +748,7 @@ def egressLosslessProfile(self, request, duthosts, rand_one_dut_hostname, dutCon yield self.__getBufferProfile( request, duthost, - "BUFFER_QUEUE", + "BUFFER_QUEUE_TABLE" if self.__isBufferInApplDb(duthost) else "BUFFER_QUEUE", dutConfig["dutInterfaces"][dutConfig["testPorts"]["src_port_id"]], "3-4" ) @@ -749,7 +771,7 @@ def egressLossyProfile(self, request, duthosts, rand_one_dut_hostname, dutConfig yield self.__getBufferProfile( request, duthost, - "BUFFER_QUEUE", + "BUFFER_QUEUE_TABLE" if self.__isBufferInApplDb(duthost) else "BUFFER_QUEUE", dutConfig["dutInterfaces"][dutConfig["testPorts"]["src_port_id"]], "0-2" ) diff --git a/tests/qos/test_buffer.py b/tests/qos/test_buffer.py new file mode 100644 index 00000000000..84c40955128 --- /dev/null +++ b/tests/qos/test_buffer.py @@ -0,0 +1,475 @@ +import logging +import os +import sys +import time +import re + +import pytest + +from tests.common import config_reload + +profile_format = 'pg_lossless_{}_{}_profile' + +default_cable_length_list = ['5m', '40m', '300m'] +default_mtu = '9100' + +def check_pool_size(duthost, expected_pool_size): + pool_size = duthost.shell('redis-cli hget "BUFFER_POOL_TABLE:ingress_lossless_pool" size')['stdout'] + assert int(pool_size) == expected_pool_size, "Pool size isn't correct: expected {} but got {}".format(expected_pool_size, pool_size) + + +def check_profile(duthost, pg, expected_profile): + profile = duthost.shell('redis-cli hget {} profile'.format(pg))['stdout'][1:-1] + assert profile == 'BUFFER_PROFILE_TABLE:' + expected_profile, 'Expected profile {} not found'.format(expected_profile) + + +def check_pfc_enable(duthost, port, expected_pfc_enable_map): + pfc_enable = duthost.shell('redis-cli -n 4 hget "PORT_QOS_MAP|{}" pfc_enable'.format(port))['stdout'] + assert expected_pfc_enable_map == pfc_enable, \ + "Expected pfc enable map {} doesn't match {}".format(expected_pfc_enable_map, pfc_enable) + + +def detect_ingress_pool_number(duthost): + pools = duthost.shell('redis-cli -n 4 keys "BUFFER_POOL|ingress*"')['stdout'] + return len(pools.split()) + + +def check_lossless_profile_removed(duthost, profile): + time.sleep(10) + profile_info = duthost.shell('redis-cli -n 6 hgetall "BUFFER_PROFILE_TABLE|{}"'.format(profile))['stdout'] + assert not profile_info, "Profile {} isn't removed from STATE_DB".format(profile) + profile_info = duthost.shell('redis-cli hgetall "BUFFER_PROFILE_TABLE:{}"'.format(profile))['stdout'] + assert not profile_info, "Profile {} isn't removed from APPL_DB".format(profile) + logging.info('Profile {} has been removed from STATE_DB and APPL_DB'.format(profile)) + + +def check_dynamic_th_in_appldb(duthost, profile, expected_dynamic_th, must_exist = False): + time.sleep(10) + dynamic_th = duthost.shell('redis-cli hget "BUFFER_PROFILE_TABLE:{}" dynamic_th'.format(profile))['stdout'] + assert not must_exist and not dynamic_th or dynamic_th == expected_dynamic_th, "dynamic_th of profile {} in APPL_DB isn't correct".format(profile) + + +@pytest.fixture(params=['50000', '10000']) +def speed_to_test(request): + ''' + @summary: used to parametrized test cases + @param request: pytest request object + @return: speed_to_test + ''' + return request.param + + +@pytest.fixture(params=['15m', '40m']) +def cable_len_to_test(request): + ''' + @summary: used to parametrized test cases + @param request: pytest request object + @return: cable_len_to_test + ''' + return request.param + + +@pytest.fixture(params=['1500', '9100']) +def mtu_to_test(request): + ''' + @summary: used to parametrized test cases + @param request: pytest request object + @return: cable_len_to_test + ''' + return request.param + + +lag_member_checked = False +lag_interface_port_belongs_to = None +port_under_test = None + +@pytest.fixture(params=['Ethernet8']) +def port_to_test(request, duthost): + ''' + @summary: used to parametrized test cases + @param request: pytest request object + @return: port_under_test + ''' + global lag_member_checked + global lag_interface_port_belongs_to + global port_under_test + + port_under_test = request.param + if not lag_member_checked: + portchannel_member_key = duthost.shell('redis-cli -n 4 keys "PORTCHANNEL_MEMBER|*|{}"'.format(port_under_test))['stdout'] + if portchannel_member_key: + portchannel = portchannel_member_key.split('|')[1] + duthost.shell('config portchannel member del {} {}'.format(portchannel, port_under_test)) + logging.info("Preparing: remove port {} from port channel {}".format(port_under_test, portchannel)) + lag_interface_port_belongs_to = portchannel + + return port_under_test + + +@pytest.fixture(params=['3-4', '6']) +def pg_to_test(request): + return request.param + + +@pytest.fixture(scope="module", autouse=True) +def teardown_module(duthost): + yield + + if lag_interface_port_belongs_to: + logging.info("Tearing down: restore the port channel configuration") + duthost.shell('config portchannel member add {} {}'.format(lag_interface_port_belongs_to, port_under_test)) + + +def test_change_speed_cable(duthost, conn_graph_facts, port_to_test, speed_to_test, mtu_to_test, cable_len_to_test): + ''' + @summary: change speed in different ways and observe whether the DUT behaves correctly + if all of the speed_to_test, mtu_to_test and cable_len_to_test match the current value, the test will be skipped + @port_to_test: on which port will the test be performed + @speed_to_test: to what speed will the port's be changed + @mtu_to_test: to what mtu will the port's be changed + @cable_len_to_test: to what cable length will the port's be changed + ''' + original_speed = duthost.shell('redis-cli -n 4 hget "PORT|{}" speed'.format(port_to_test))['stdout'] + original_cable_len = duthost.shell('redis-cli -n 4 hget "CABLE_LENGTH|AZURE" {}'.format(port_to_test))['stdout'] + profile = duthost.shell('redis-cli hget "BUFFER_PG_TABLE:{}:3-4" profile'.format(port_to_test))['stdout'][1:-1] + original_headroom_size = duthost.shell('redis-cli hget "{}" size'.format(profile))['stdout'] + original_pool_size = duthost.shell('redis-cli hget BUFFER_POOL_TABLE:ingress_lossless_pool size')['stdout'] + number_of_pools = detect_ingress_pool_number(duthost) + + if speed_to_test == original_speed and cable_len_to_test == original_cable_len and mtu_to_test == default_mtu: + pytest.skip('Speed, MTU and cable length matches the default value, nothing to test, skip') + + try: + if not speed_to_test == original_speed: + logging.info("Changing port's speed to {}".format(speed_to_test)) + duthost.shell('config interface speed {} {}'.format(port_to_test, speed_to_test)) + if not mtu_to_test == default_mtu: + logging.info("Changing port's mtu to {}".format(mtu_to_test)) + duthost.shell('config interface mtu {} {}'.format(port_to_test, mtu_to_test)) + if not cable_len_to_test == original_cable_len: + logging.info("Changing port's cable length to {}".format(cable_len_to_test)) + duthost.shell('config interface cable-length {} {}'.format(port_to_test, cable_len_to_test)) + + check_profile_removed = cable_len_to_test not in default_cable_length_list + + time.sleep(10) + # check whether profile is correct in PG table + if mtu_to_test != default_mtu: + expected_profile = 'pg_lossless_{}_{}_mtu{}_profile'.format(speed_to_test, cable_len_to_test, mtu_to_test) + check_profile_removed = True + else: + expected_profile = 'pg_lossless_{}_{}_profile'.format(speed_to_test, cable_len_to_test) + logging.info('[speed and/or cable-len and/or MTU updated] Checking whether new profile {} has been created and pfc_enable has been updated'.format(expected_profile)) + check_profile(duthost, 'BUFFER_PG_TABLE:{}:3-4'.format(port_to_test), expected_profile) + check_pfc_enable(duthost, port_to_test, '3,4') + + # check whether profile exist + headroom_size = duthost.shell('redis-cli hget "BUFFER_PROFILE_TABLE:{}" size'.format(expected_profile))['stdout'] + check_pool_size(duthost, (int(original_pool_size) * number_of_pools + (int(original_headroom_size) - int(headroom_size)) * 2) / number_of_pools) + + # Remove all the lossless profile on the port + logging.info('[remove all lossless PGs] Checking pool size and pfc_enable') + duthost.shell('config interface buffer priority-group lossless remove {} 3-4'.format(port_to_test)) + time.sleep(10) + check_pool_size(duthost, (int(original_pool_size) * number_of_pools + int(original_headroom_size) * 2) / number_of_pools) + check_pfc_enable(duthost, port_to_test, '') + if check_profile_removed: + logging.info('[remove dynamic profile on PG removed] Checking whether the profile {} is removed on receiving all lossless PG removed'.format(expected_profile)) + check_lossless_profile_removed(duthost, expected_profile) + + # Re-add another lossless profile + logging.info('Re-add a lossless_pg and check pool size and pfc_enable') + duthost.shell('config interface buffer priority-group lossless add {} 6'.format(port_to_test)) + time.sleep(10) + check_pool_size(duthost, (int(original_pool_size) * number_of_pools + int(original_headroom_size) * 2 - int(headroom_size)) / number_of_pools) + check_pfc_enable(duthost, port_to_test, '6') + + if cable_len_to_test != original_cable_len: + duthost.shell('config interface cable-length {} {}'.format(port_to_test, original_cable_len)) + if mtu_to_test != default_mtu: + duthost.shell('config interface mtu {} {}'.format(port_to_test, default_mtu)) + # remove old profile on cable length change + logging.info('[remove dynamic profile on cable length and/or MTU updated] Checking whether the old profile is removed') + check_lossless_profile_removed(duthost, expected_profile) + expected_profile = 'pg_lossless_{}_{}_profile'.format(speed_to_test, original_cable_len) + check_profile(duthost, 'BUFFER_PG_TABLE:{}:6'.format(port_to_test), expected_profile) + headroom_size = duthost.shell('redis-cli hget "BUFFER_PROFILE_TABLE:{}" size'.format(expected_profile))['stdout'] + check_pool_size(duthost, (int(original_pool_size) * number_of_pools + int(original_headroom_size) * 2 - int(headroom_size)) / number_of_pools) + + duthost.shell('config interface buffer priority-group lossless remove {} 6'.format(port_to_test)) + time.sleep(10) + check_pool_size(duthost, (int(original_pool_size) * number_of_pools + int(original_headroom_size) * 2) / number_of_pools) + check_pfc_enable(duthost, port_to_test, '') + else: + if cable_len_to_test != original_cable_len: + logging.info('[update cable length without any lossless pg configured]') + duthost.shell('config interface cable-length {} {}'.format(port_to_test, original_cable_len)) + if mtu_to_test != default_mtu: + logging.info('[update mtu without any lossless pg configured]') + duthost.shell('config interface mtu {} {}'.format(port_to_test, default_mtu)) + + if speed_to_test != original_speed: + logging.info('[update speed without any lossless pg configured]') + duthost.shell('config interface speed {} {}'.format(port_to_test, original_speed)) + + logging.info('[add lossless pg with speed and cable length ready]') + duthost.shell('config interface buffer priority-group lossless add {} 3-4'.format(port_to_test)) + time.sleep(10) + expected_profile = 'pg_lossless_{}_{}_profile'.format(original_speed, original_cable_len) + check_profile(duthost, 'BUFFER_PG_TABLE:{}:3-4'.format(port_to_test), expected_profile) + check_pfc_enable(duthost, port_to_test, '3,4') + + headroom_size = duthost.shell('redis-cli hget "BUFFER_PROFILE_TABLE:{}" size'.format(expected_profile))['stdout'] + check_pool_size(duthost, int(original_pool_size)) + + logging.info('[extra lossless PG]') + duthost.shell('config interface buffer priority-group lossless add {} 6'.format(port_to_test)) + time.sleep(10) + check_profile(duthost, 'BUFFER_PG_TABLE:{}:6'.format(port_to_test), expected_profile) + check_pfc_enable(duthost, port_to_test, '3,4,6') + check_pool_size(duthost, (int(original_pool_size) * number_of_pools - int(original_headroom_size)) / number_of_pools) + + logging.info('[restore config]') + duthost.shell('config interface buffer priority-group lossless remove {} 6'.format(port_to_test)) + time.sleep(10) + check_pfc_enable(duthost, port_to_test, '3,4') + check_pool_size(duthost, int(original_pool_size)) + finally: + duthost.shell('config interface buffer priority-group lossless remove {}'.format(port_to_test), module_ignore_errors = True) + duthost.shell('config interface speed {} {}'.format(port_to_test, original_speed), module_ignore_errors = True) + duthost.shell('config interface mtu {} {}'.format(port_to_test, default_mtu), module_ignore_errors = True) + duthost.shell('config interface cable-length {} {}'.format(port_to_test, original_cable_len), module_ignore_errors = True) + duthost.shell('config interface buffer priority-group lossless add {} 3-4'.format(port_to_test), module_ignore_errors = True) + + +def test_headroom_override(duthost, conn_graph_facts, port_to_test): + ''' + @summary: headroom override test + @port_to_test: on which port will the test be performed + ''' + original_speed = duthost.shell('redis-cli -n 4 hget "PORT|{}" speed'.format(port_to_test))['stdout'] + original_cable_len = duthost.shell('redis-cli -n 4 hget "CABLE_LENGTH|AZURE" {}'.format(port_to_test))['stdout'] + original_profile = duthost.shell('redis-cli hget "BUFFER_PG_TABLE:{}:3-4" profile'.format(port_to_test))['stdout'][1:-1] + original_headroom_size = duthost.shell('redis-cli hget "{}" size'.format(original_profile))['stdout'] + original_pool_size = duthost.shell('redis-cli hget BUFFER_POOL_TABLE:ingress_lossless_pool size')['stdout'] + number_of_pools = detect_ingress_pool_number(duthost) + + try: + # Configure a static profile + logging.info("[prepare configuration]") + duthost.shell('config buffer profile add headroom-override --xon 18432 --xoff 18432 --dynamic_th 1') + time.sleep(10) + + logging.info("[test: headroom override on lossless PG 3-4] apply the profile on the PG and check pool size") + duthost.shell('config interface buffer priority-group lossless set {} 3-4 headroom-override'.format(port_to_test)) + time.sleep(10) + check_profile(duthost, 'BUFFER_PG_TABLE:{}:3-4'.format(port_to_test), 'headroom-override') + check_pfc_enable(duthost, port_to_test, '3,4') + check_pool_size(duthost, (int(original_pool_size) * number_of_pools + 2 * int(original_headroom_size) - 73728) / number_of_pools) + + # Add another headroom override + logging.info("[test: headroom override on more lossless PGs 6] apply the profile on the PG and check pool size") + duthost.shell('config interface buffer priority-group lossless add {} 6 headroom-override'.format(port_to_test)) + time.sleep(10) + check_profile(duthost, 'BUFFER_PG_TABLE:{}:6'.format(port_to_test), 'headroom-override') + check_pfc_enable(duthost, port_to_test, '3,4,6') + check_pool_size(duthost, (int(original_pool_size) * number_of_pools + 2 * int(original_headroom_size) - 110592) / number_of_pools) + + logging.info("[test: update headroom-override profile] update the profile and check pool size") + duthost.shell('config buffer profile set headroom-override --xon 18432 --xoff 36864') + time.sleep(10) + check_pool_size(duthost, (int(original_pool_size) * number_of_pools + 2 * int(original_headroom_size) - 165888) / number_of_pools) + + # Recover configuration + logging.info("[test: static headroom being referenced can not be removed]") + duthost.shell('config buffer profile remove headroom-override', module_ignore_errors = True) + time.sleep(20) + profile = duthost.shell('redis-cli hgetall "BUFFER_PROFILE_TABLE:headroom-override"')['stdout'] + assert profile, 'Headroom override profile has been removed when being referenced' + logging.info("[recover configuration]") + duthost.shell('config interface buffer priority-group lossless remove {}'.format(port_to_test)) + duthost.shell('config interface buffer priority-group lossless add {} 3-4'.format(port_to_test)) + time.sleep(10) + check_profile(duthost, 'BUFFER_PG_TABLE:{}:3-4'.format(port_to_test), original_profile.split(':')[1]) + check_pfc_enable(duthost, port_to_test, '3,4') + check_pool_size(duthost, int(original_pool_size)) + finally: + duthost.shell('config interface buffer priority-group lossless remove {}'.format(port_to_test), module_ignore_errors = True) + duthost.shell('config interface buffer priority-group lossless add {} 3-4'.format(port_to_test), module_ignore_errors = True) + duthost.shell('config buffer profile remove headroom-override', module_ignore_errors = True) + + +def test_lossless_pg(duthost, conn_graph_facts, port_to_test, pg_to_test): + ''' + @summary: non default dynamic th test + @port_to_test: on which port will the test be performed + @speed_to_test: to what speed will the port's be changed + ''' + original_speed = duthost.shell('redis-cli -n 4 hget "PORT|{}" speed'.format(port_to_test))['stdout'] + original_cable_len = duthost.shell('redis-cli -n 4 hget "CABLE_LENGTH|AZURE" {}'.format(port_to_test))['stdout'] + + # create profiles + logging.info('[preparing]: create static buffer profile for headroom override and non default dynamic_th') + duthost.shell('config buffer profile add headroom-override --xon 18432 --xoff 32768') + duthost.shell('config buffer profile add non-default-dynamic_th --dynamic_th 2') + + # update cable length to 15m + logging.info('[preparing]: update cable length') + duthost.shell('config interface cable-length {} 15m'.format(port_to_test)) + expected_profile = 'pg_lossless_{}_15m_profile'.format(original_speed, original_cable_len) + check_profile(duthost, 'BUFFER_PG_TABLE:{}:3-4'.format(port_to_test), expected_profile) + + set_command = 'config interface buffer priority-group lossless set {} {} '.format(port_to_test, pg_to_test) + add_command = 'config interface buffer priority-group lossless add {} {} '.format(port_to_test, pg_to_test) + if pg_to_test == '3-4': + first_command = set_command + else: + first_command = add_command + + buffer_pg = 'BUFFER_PG_TABLE:{}:{}'.format(port_to_test, pg_to_test) + + try: + # 1. original it should be a dynamic PG, update it to override + logging.info('[testcase: dynamic headroom => headroom override]') + duthost.shell(first_command + 'headroom-override') + # check whether lossless dynamic profile is removed + check_profile(duthost, buffer_pg, 'headroom-override') + if pg_to_test == '3-4': + check_lossless_profile_removed(duthost, expected_profile) + + # update it to non-default dynamic_th + logging.info('[testcase: headroom override => dynamically calculated headroom with non-default dynamic_th]') + duthost.shell(set_command + 'non-default-dynamic_th') + expected_nondef_profile = 'pg_lossless_{}_15m_th2_profile'.format(original_speed) + check_profile(duthost, buffer_pg, expected_nondef_profile) + + # update it to dynamic PG + logging.info('[testcase: dynamically calculated headroom with non-default dynamic_th => dynamic headroom]') + duthost.shell(set_command) + check_profile(duthost, buffer_pg, expected_profile) + check_lossless_profile_removed(duthost, expected_nondef_profile) + + # update it to non-default dynamic_th + logging.info('[testcase: dynamic headroom => [dynamically calculated headroom with non-default dynamic_th]') + duthost.shell(set_command + 'non-default-dynamic_th') + check_profile(duthost, buffer_pg, expected_nondef_profile) + if pg_to_test == '3-4': + check_lossless_profile_removed(duthost, expected_profile) + + # update it to headroom override + logging.info('[testcase: dynamically calculated headroom with non-default dynamic_th => headroom override]') + duthost.shell(set_command + 'headroom-override') + check_profile(duthost, buffer_pg, 'headroom-override') + check_lossless_profile_removed(duthost, expected_nondef_profile) + + # update it to dynamic PG, recover + logging.info('[testcase: headroom override => dynamic headroom]') + duthost.shell(set_command) + check_profile(duthost, buffer_pg, expected_profile) + + # remove all static profiles + logging.info('[restoring configuration]') + duthost.shell('config buffer profile remove headroom-override') + duthost.shell('config buffer profile remove non-default-dynamic_th') + check_lossless_profile_removed(duthost, 'headroom-override') + check_lossless_profile_removed(duthost, 'non-default-dynamic_th') + finally: + if pg_to_test == '3-4': + duthost.shell(set_command, module_ignore_errors = True) + else: + duthost.shell('config interface buffer priority-group lossless remove {} {} '.format(port_to_test, pg_to_test), module_ignore_errors = True) + duthost.shell('config interface cable-length {} {}'.format(port_to_test, original_cable_len), module_ignore_errors = True) + duthost.shell('config buffer profile remove headroom-override', module_ignore_errors = True) + duthost.shell('config buffer profile remove non-default-dynamic_th', module_ignore_errors = True) + + +def test_exceeding_headroom(duthost, conn_graph_facts, port_to_test): + ''' + @summary: + ''' + max_headroom_size = duthost.shell('redis-cli -n 6 hget "BUFFER_MAX_PARAM_TABLE|{}" max_headroom_size'.format(port_to_test))['stdout'] + if not max_headroom_size: + pytest.skip('No max headroom found on port {}, skip'.format(port_to_test)) + + original_cable_len = duthost.shell('redis-cli -n 4 hget "CABLE_LENGTH|AZURE" {}'.format(port_to_test))['stdout'] + original_speed = duthost.shell('redis-cli -n 4 hget "PORT|{}" speed'.format(port_to_test))['stdout'] + original_profile = 'pg_lossless_{}_{}_profile'.format(original_speed, original_cable_len) + + try: + # set to super long cable length + logging.info('[config a super long cable length]') + duthost.shell('config interface cable-length {} 10000m'.format(port_to_test)) + time.sleep(20) + logging.info('verify the profile isn\'t changed') + check_profile(duthost, 'BUFFER_PG_TABLE:{}:3-4'.format(port_to_test), original_profile) + duthost.shell('config interface cable-length {} {}'.format(port_to_test, original_cable_len)) + + # add additional PG + logging.info('[config the cable length on the port]') + duthost.shell('config interface cable-length {} 300m'.format(port_to_test)) + time.sleep(20) + logging.info('verify the profile has been changed') + expected_profile = 'pg_lossless_{}_{}_profile'.format(original_speed, '300m') + check_profile(duthost, 'BUFFER_PG_TABLE:{}:3-4'.format(port_to_test), expected_profile) + logging.info('add another PG and make sure the system isn\'t broken') + duthost.shell('config interface buffer priority-group lossless add {} {}'.format(port_to_test, '5-7')) + time.sleep(20) + # we can't say whether this will accumulative headroom exceed the limit, but the system should not crash + # leverage sanity check to verify that + duthost.shell('config interface buffer priority-group lossless remove {} {}'.format(port_to_test, '5-7')) + duthost.shell('config interface cable-length {} {}'.format(port_to_test, original_cable_len)) + + # static profile + logging.info('[config headroom override to PG 3-4]') + duthost.shell('config buffer profile add test-headroom --xon 18432 --xoff 50000 -headroom 68432') + duthost.shell('config interface buffer priority-group lossless set {} {} {}'.format(port_to_test, '3-4', 'test-headroom')) + time.sleep(20) + logging.info('verify the profile is applied') + check_profile(duthost, 'BUFFER_PG_TABLE:{}:3-4'.format(port_to_test), 'test-headroom') + duthost.shell('config interface buffer priority-group lossless add {} {} {}'.format(port_to_test, '5-7', 'test-headroom')) + time.sleep(20) + # again, we can't say for sure whether the accumulative headroom exceeding. + # just make sure the system doesn't crash + duthost.shell('config interface buffer priority-group lossless remove {} {}'.format(port_to_test, '5-7')) + + logging.info('[update headroom override to a lager size]') + duthost.shell('config buffer profile set test-headroom --xon 18432 --xoff 860160 -headroom 878592') + time.sleep(20) + # this should make it exceed the limit, so the profile should not applied to the APPL_DB + size_in_appldb = duthost.shell('redis-cli hget "BUFFER_PROFILE_TABLE:test-headroom" size')['stdout'] + assert size_in_appldb == '68432', 'The profile with a large size was applied to APPL_DB, which can make headroom exceeding' + duthost.shell('config interface buffer priority-group lossless set {} {}'.format(port_to_test, '3-4')) + duthost.shell('config buffer profile remove test-headroom') + logging.info('[clean up]') + finally: + duthost.shell('config interface cable-length {} {}'.format(port_to_test, original_cable_len), module_ignore_errors = True) + duthost.shell('config interface buffer priority-group lossless remove {} 5-7'.format(port_to_test), module_ignore_errors = True) + duthost.shell('config interface buffer priority-group lossless set {} 3-4'.format(port_to_test), module_ignore_errors = True) + duthost.shell('config buffer profile remove test-headroom', module_ignore_errors = True) + + +def _recovery_to_dynamic_buffer_model(duthost): + duthost.shell('kill $(pgrep buffermgrd)') + duthost.shell('config qos reload') + duthost.shell('config save -y') + config_reload(duthost, config_source='config_db') + + +def test_buffer_model_test(duthost, conn_graph_facts): + ''' + @summary: verify whether the buffer model is expected after configuration operations: + - whether the buffer model is traditional after executing config load_minigraph + - whether the buffer model is dynamic after recovering the buffer model to dynamic + ''' + try: + logging.info('[config load_minigraph]') + config_reload(duthost, config_source='minigraph') + buffer_model = duthost.shell('redis-cli -n 4 hget "DEVICE_METADATA|localhost" buffer_model')['stdout'] + assert buffer_model == 'traditional', 'Got buffer model {} after executing config load_minigraph, traditional expected' + + logging.info('[Recover the DUT to default buffer model]') + _recovery_to_dynamic_buffer_model(duthost) + buffer_model = duthost.shell('redis-cli -n 4 hget "DEVICE_METADATA|localhost" buffer_model')['stdout'] + assert buffer_model == 'dynamic', 'Got buffer model {} after executing recovering the buffer model to dynamic' + finally: + _recovery_to_dynamic_buffer_model(duthost) diff --git a/tests/scripts/check_buffer_dynamic.sh b/tests/scripts/check_buffer_dynamic.sh new file mode 100644 index 00000000000..d34f5ee3858 --- /dev/null +++ b/tests/scripts/check_buffer_dynamic.sh @@ -0,0 +1,9 @@ +#!/bin/bash + +BUFFER_MODEL=$(redis-cli -n 4 hget "DEVICE_METADATA|localhost" buffer_model) + +if [[ "$BUFFER_MODEL" == "dynamic" ]]; then + exit 0 +else + exit 1 +fi diff --git a/tests/scripts/check_buffer_traditional.sh b/tests/scripts/check_buffer_traditional.sh new file mode 100644 index 00000000000..98165905fd0 --- /dev/null +++ b/tests/scripts/check_buffer_traditional.sh @@ -0,0 +1,9 @@ +#!/bin/bash + +BUFFER_MODEL=$(redis-cli -n 4 hget "DEVICE_METADATA|localhost" buffer_model) + +if [[ "$BUFFER_MODEL" != "dynamic" ]]; then + exit 0 +else + exit 1 +fi From 8ee5033be9e7b5b090a80ee3e39f1580e8131a76 Mon Sep 17 00:00:00 2001 From: Stephen Sun Date: Sat, 28 Nov 2020 12:44:15 +0000 Subject: [PATCH 2/9] Address comments: use pytest_assert and google style doc strings verify buffer profile and pool against ASIC_DB move hardcoded parameters to a predefined json file add comments to make it easy to understand Signed-off-by: Stephen Sun --- tests/qos/files/dynamic_buffer_param.json | 29 + tests/qos/qos_sai_base.py | 22 +- tests/qos/test_buffer.py | 651 +++++++++++++++++----- 3 files changed, 554 insertions(+), 148 deletions(-) create mode 100644 tests/qos/files/dynamic_buffer_param.json diff --git a/tests/qos/files/dynamic_buffer_param.json b/tests/qos/files/dynamic_buffer_param.json new file mode 100644 index 00000000000..98dab966197 --- /dev/null +++ b/tests/qos/files/dynamic_buffer_param.json @@ -0,0 +1,29 @@ +{ + "mellanox": { + "default_cable_length": ["5m", "40m", "300m"], + "testparam_cable_length": ["15m", "40m"], + "headroom-override": { + "add": { + "xon": "18432", + "xoff": "18432", + "size": "36864", + "dynamic_th": "1" + }, + "set": { + "xon": "18432", + "xoff": "36864", + "size": "55296" + } + }, + "lossless_pg": { + "headroom-override": { + "xon": "18432", + "xoff": "32768", + "size": "51200" + }, + "non-default-dynamic_th": { + "dynamic_th": "2" + } + } + } +} diff --git a/tests/qos/qos_sai_base.py b/tests/qos/qos_sai_base.py index 6d8aad56b00..cba4d2d2ff0 100644 --- a/tests/qos/qos_sai_base.py +++ b/tests/qos/qos_sai_base.py @@ -24,7 +24,8 @@ class QosSaiBase: buffer_model_initialized = False buffer_model = None - def __isBufferInApplDb(self, duthost): + @pytest.fixture(scope='class', autouse=True) + def isBufferInApplDb(self, duthost): if not self.buffer_model_initialized: self.buffer_model = duthost.shell('redis-cli -n 4 hget "DEVICE_METADATA|localhost" buffer_model')["stdout"] self.buffer_model_initialized = True @@ -62,7 +63,7 @@ def __computeBufferThreshold(self, duthost, bufferProfile): Returns: Updates bufferProfile with computed buffer threshold """ - db = "0" if self.__isBufferInApplDb(duthost) else "4" + db = "0" if self.isBufferInApplDb(duthost) else "4" pool = bufferProfile["pool"].encode("utf-8").translate(None, "[]") bufferSize = int(self.__runRedisCommandOrAssert( duthost, @@ -83,7 +84,10 @@ def __updateVoidRoidParams(self, duthost, bufferProfile): Returns: Updates bufferProfile with VOID/ROID obtained from Redis db """ - bufferPoolName = bufferProfile["pool"].encode("utf-8").translate(None, "[]").replace("BUFFER_POOL_TABLE:",'') + if self.isBufferInApplDb(duthost): + bufferPoolName = bufferProfile["pool"].encode("utf-8").translate(None, "[]").replace("BUFFER_POOL_TABLE:",'') + else: + bufferPoolName = bufferProfile["pool"].encode("utf-8").translate(None, "[]").replace("BUFFER_POOL|",'') bufferPoolVoid = self.__runRedisCommandOrAssert( duthost, @@ -112,7 +116,7 @@ def __getBufferProfile(self, request, duthost, table, port, priorityGroup): bufferProfile (dict): Map of buffer profile attributes """ - if self.__isBufferInApplDb(duthost): + if self.isBufferInApplDb(duthost): db = "0" keystr = "{0}:{1}:{2}".format(table, port, priorityGroup) else: @@ -557,7 +561,7 @@ def dutQosConfig(self, duthosts, rand_one_dut_hostname, ingressLosslessProfile, pytest_assert("minigraph_hwsku" in mgFacts, "Could not find DUT SKU") profileName = ingressLosslessProfile["profileName"] - if self.__isBufferInApplDb(duthost): + if self.isBufferInApplDb(duthost): profile_pattern = "^BUFFER_PROFILE_TABLE\:pg_lossless_(.*)_profile$" else: profile_pattern = "^BUFFER_PROFILE\|pg_lossless_(.*)_profile" @@ -702,7 +706,7 @@ def ingressLosslessProfile(self, request, duthosts, rand_one_dut_hostname, dutCo yield self.__getBufferProfile( request, duthost, - "BUFFER_PG_TABLE" if self.__isBufferInApplDb(duthost) else "BUFFER_PG", + "BUFFER_PG_TABLE" if self.isBufferInApplDb(duthost) else "BUFFER_PG", dutConfig["dutInterfaces"][dutConfig["testPorts"]["src_port_id"]], "3-4" ) @@ -725,7 +729,7 @@ def ingressLossyProfile(self, request, duthosts, rand_one_dut_hostname, dutConfi yield self.__getBufferProfile( request, duthost, - "BUFFER_PG_TABLE" if self.__isBufferInApplDb(duthost) else "BUFFER_PG", + "BUFFER_PG_TABLE" if self.isBufferInApplDb(duthost) else "BUFFER_PG", dutConfig["dutInterfaces"][dutConfig["testPorts"]["src_port_id"]], "0" ) @@ -748,7 +752,7 @@ def egressLosslessProfile(self, request, duthosts, rand_one_dut_hostname, dutCon yield self.__getBufferProfile( request, duthost, - "BUFFER_QUEUE_TABLE" if self.__isBufferInApplDb(duthost) else "BUFFER_QUEUE", + "BUFFER_QUEUE_TABLE" if self.isBufferInApplDb(duthost) else "BUFFER_QUEUE", dutConfig["dutInterfaces"][dutConfig["testPorts"]["src_port_id"]], "3-4" ) @@ -771,7 +775,7 @@ def egressLossyProfile(self, request, duthosts, rand_one_dut_hostname, dutConfig yield self.__getBufferProfile( request, duthost, - "BUFFER_QUEUE_TABLE" if self.__isBufferInApplDb(duthost) else "BUFFER_QUEUE", + "BUFFER_QUEUE_TABLE" if self.isBufferInApplDb(duthost) else "BUFFER_QUEUE", dutConfig["dutInterfaces"][dutConfig["testPorts"]["src_port_id"]], "0-2" ) diff --git a/tests/qos/test_buffer.py b/tests/qos/test_buffer.py index 84c40955128..48cc9c4bef0 100644 --- a/tests/qos/test_buffer.py +++ b/tests/qos/test_buffer.py @@ -3,107 +3,268 @@ import sys import time import re +import json import pytest from tests.common import config_reload +from tests.common.utilities import wait_until +from tests.common.helpers.assertions import pytest_assert profile_format = 'pg_lossless_{}_{}_profile' +lossless_profile_pattern = 'pg_lossless_([1-9][0-9]*000)_([1-9][0-9]*m)_profile' -default_cable_length_list = ['5m', '40m', '300m'] +default_cable_length_list = None +default_lossless_headroom_data = None +default_ingress_pool_number = 0 default_mtu = '9100' -def check_pool_size(duthost, expected_pool_size): - pool_size = duthost.shell('redis-cli hget "BUFFER_POOL_TABLE:ingress_lossless_pool" size')['stdout'] - assert int(pool_size) == expected_pool_size, "Pool size isn't correct: expected {} but got {}".format(expected_pool_size, pool_size) +testparam_headroom_override = None +testparam_lossless_pg = None +buffer_model_dynamic = True -def check_profile(duthost, pg, expected_profile): +def detect_buffer_model(duthost): + global buffer_model_dynamic + buffer_model = duthost.shell('redis-cli -n 4 hget "DEVICE_METADATA|localhost" buffer_model')['stdout'] + buffer_model_dynamic = buffer_model == 'dynamic' + + +def detect_ingress_pool_number(duthost): + global default_ingress_pool_number + pools = duthost.shell('redis-cli -n 4 keys "BUFFER_POOL|ingress*"')['stdout'] + default_ingress_pool_number = len(pools.split()) + + +def load_lossless_headroom_data(duthost): + global default_lossless_headroom_data + if not default_lossless_headroom_data: + dut_hwsku = duthost.facts["hwsku"] + dut_platform = duthost.facts["platform"] + skudir = "/usr/share/sonic/device/{}/{}/".format(dut_platform, dut_hwsku) + lines = duthost.shell('cat {}/pg_profile_lookup.ini'.format(skudir))["stdout"] + default_lossless_headroom_data = {} + for line in lines.split('\n'): + if line[0] == '#': + continue + tokens = line.split() + speed = tokens[0] + cable_length = tokens[1] + size = tokens[2] + xon = tokens[3] + xoff = tokens[4] + if not default_lossless_headroom_data.get(speed): + default_lossless_headroom_data[speed] = {} + default_lossless_headroom_data[speed][cable_length] = {'size': size, 'xon': xon, 'xoff': xoff} + default_lossless_headroom_data = default_lossless_headroom_data + + +def load_test_parameters(duthost): + global default_cable_length_list + global testparam_headroom_override + global testparam_lossless_pg + with open(r"qos/files/dynamic_buffer_param.json") as file: + params = json.load(file) + logging.info("Got test parameters {}".format(params)) + asic_type = duthost.facts['asic_type'] + vendor_specific_param = params[asic_type] + default_cable_length_list = vendor_specific_param['default_cable_length'] + testparam_headroom_override = vendor_specific_param['headroom-override'] + testparam_lossless_pg = vendor_specific_param['lossless_pg'] + + +@pytest.fixture(scope="module", autouse=True) +def setup_module(duthost): + detect_buffer_model(duthost) + if buffer_model_dynamic: + detect_ingress_pool_number(duthost) + load_lossless_headroom_data(duthost) + load_test_parameters(duthost) + + logging.info("Got cable length: default {}".format(default_cable_length_list)) + logging.info("Got ingress pool number {}".format(default_ingress_pool_number)) + logging.info("Got lossless headroom data {}".format(default_lossless_headroom_data)) + else: + pytest.skip("Dynamic buffer isn't enabled, skip the test") + + yield + + +def check_pool_size(duthost, ingress_lossless_pool_oid, **kwargs): + if duthost.facts['asic_type'] == 'mellanox': + old_headroom = int(kwargs["old_headroom"]) + + if "old_pg_number" in kwargs: + old_pg_number = int(kwargs["old_pg_number"]) + else: + old_pg_number = 2 + + if "new_pg_number" in kwargs: + new_pg_number = int(kwargs["new_pg_number"]) + else: + new_pg_number = old_pg_number + + if new_pg_number: + if "new_headroom" in kwargs: + new_headroom = int(kwargs["new_headroom"]) + else: + new_headroom = old_headroom + new_reserved = new_pg_number * new_headroom + else: + new_reserved = 0 + + curr_pool_size = int(kwargs["pool_size"]) + + original_memory = curr_pool_size * default_ingress_pool_number + old_headroom * old_pg_number + expected_pool_size = (original_memory - new_reserved) / default_ingress_pool_number + pool_size = duthost.shell('redis-cli hget "BUFFER_POOL_TABLE:ingress_lossless_pool" size')['stdout'] + pytest_assert(int(pool_size) == expected_pool_size, "Pool size isn't correct: expected {} but got {}".format(expected_pool_size, pool_size)) + + if ingress_lossless_pool_oid: + pool_sai = _compose_dict_from_cli(duthost.shell('redis-cli -n 1 hgetall ASIC_STATE:SAI_OBJECT_TYPE_BUFFER_POOL:{}'.format(ingress_lossless_pool_oid))['stdout'].split('\n')) + pytest_assert(int(pool_sai['SAI_BUFFER_POOL_ATTR_SIZE']) == expected_pool_size) + + +def check_pg_profile(duthost, pg, expected_profile): profile = duthost.shell('redis-cli hget {} profile'.format(pg))['stdout'][1:-1] - assert profile == 'BUFFER_PROFILE_TABLE:' + expected_profile, 'Expected profile {} not found'.format(expected_profile) + pytest_assert(profile == 'BUFFER_PROFILE_TABLE:' + expected_profile, 'Expected profile {} not found'.format(expected_profile)) def check_pfc_enable(duthost, port, expected_pfc_enable_map): pfc_enable = duthost.shell('redis-cli -n 4 hget "PORT_QOS_MAP|{}" pfc_enable'.format(port))['stdout'] - assert expected_pfc_enable_map == pfc_enable, \ - "Expected pfc enable map {} doesn't match {}".format(expected_pfc_enable_map, pfc_enable) + pytest_assert(expected_pfc_enable_map == pfc_enable, \ + "Expected pfc enable map {} doesn't match {}".format(expected_pfc_enable_map, pfc_enable)) -def detect_ingress_pool_number(duthost): - pools = duthost.shell('redis-cli -n 4 keys "BUFFER_POOL|ingress*"')['stdout'] - return len(pools.split()) - - -def check_lossless_profile_removed(duthost, profile): +def check_lossless_profile_removed(duthost, profile, sai_oid = None): time.sleep(10) profile_info = duthost.shell('redis-cli -n 6 hgetall "BUFFER_PROFILE_TABLE|{}"'.format(profile))['stdout'] - assert not profile_info, "Profile {} isn't removed from STATE_DB".format(profile) + pytest_assert(not profile_info, "Profile {} isn't removed from STATE_DB".format(profile)) profile_info = duthost.shell('redis-cli hgetall "BUFFER_PROFILE_TABLE:{}"'.format(profile))['stdout'] - assert not profile_info, "Profile {} isn't removed from APPL_DB".format(profile) - logging.info('Profile {} has been removed from STATE_DB and APPL_DB'.format(profile)) + pytest_assert(not profile_info, "Profile {} isn't removed from APPL_DB".format(profile)) + logging.debug('Profile {} has been removed from STATE_DB and APPL_DB'.format(profile)) + if sai_oid: + profile_info = duthost.shell('redis-cli -n 1 hgetall {}'.format(sai_oid)) + pytest_assert('Profile {} has been removed from ASIC_DB'.format(sai_oid)) def check_dynamic_th_in_appldb(duthost, profile, expected_dynamic_th, must_exist = False): time.sleep(10) dynamic_th = duthost.shell('redis-cli hget "BUFFER_PROFILE_TABLE:{}" dynamic_th'.format(profile))['stdout'] - assert not must_exist and not dynamic_th or dynamic_th == expected_dynamic_th, "dynamic_th of profile {} in APPL_DB isn't correct".format(profile) + pytest_assert(not must_exist and not dynamic_th or dynamic_th == expected_dynamic_th, "dynamic_th of profile {} in APPL_DB isn't correct".format(profile)) + + +def fetch_initial_asic_db(duthost): + profiles_in_asicdb = duthost.shell('redis-cli -n 1 keys "ASIC_STATE:SAI_OBJECT_TYPE_BUFFER_PROFILE*"')['stdout'] + return set(profiles_in_asicdb.split('\n')) + + +def _compose_dict_from_cli(fields_list): + return dict(zip(fields_list[0::2], fields_list[1::2])) + + +def check_buffer_profile_details(duthost, initial_profiles, profile_name, profile_oid, pool_oid): + profile_appldb = _compose_dict_from_cli(duthost.shell('redis-cli hgetall BUFFER_PROFILE_TABLE:{}'.format(profile_name))['stdout'].split('\n')) + logging.debug("appl db profile {}".format(profile_appldb)) + + # check the profile against the standard value + m = re.search(lossless_profile_pattern, profile_name) + if m: + # this means it's a dynamic profile + speed = m.group(1) + cable_length = m.group(2) + std_profiles_for_speed = default_lossless_headroom_data.get(speed) + std_profile = std_profiles_for_speed.get(cable_length) + if std_profile: + # this means it's a profile with std speed and cable length. we can check whether the headroom data is correct + pytest_assert(profile_appldb['xon'] == std_profile['xon'] and profile_appldb['xoff'] == std_profile['xoff'] and profile_appldb['size'] == std_profile['size'], + "Generated profile {} doesn't match the std profile {}".format(profile_appldb, std_profile)) + else: + for std_cable_len, std_profile in std_profiles_for_speed.items(): + if int(std_cable_len[:-1]) > int(cable_length[:-1]): + pytest_assert(int(std_profile['xoff']) >= int(profile_appldb['xoff']), + "XOFF of generated profile {} is greater than standard profile {} while its cable length is less".format(profile_appldb, std_profile)) + else: + pytest_assert(int(std_profile['xoff']) <= int(profile_appldb['xoff']), + "XOFF of generated profile {} is less than standard profile {} while its cable length is greater".format(profile_appldb, std_profile)) + + profiles_in_asicdb = set(duthost.shell('redis-cli -n 1 keys "ASIC_STATE:SAI_OBJECT_TYPE_BUFFER_PROFILE*"')['stdout'].split('\n')) + diff = profiles_in_asicdb - initial_profiles + if len(diff): + profile_oid = diff.pop() + + logging.debug("initial profiles {} current profiles {} difference {}".format(initial_profiles, profiles_in_asicdb, diff)) + + profile_sai = _compose_dict_from_cli(duthost.shell('redis-cli -n 1 hgetall {}'.format(profile_oid))['stdout'].split('\n')) + + logging.debug("SAI object for new profile {}: oid {} content {}".format(profile_name, profile_oid, profile_sai)) + + if pool_oid == None: + pool_oid = profile_sai['SAI_BUFFER_PROFILE_ATTR_POOL_ID'] + if profile_appldb.get('dynamic_th'): + sai_threshold_value = profile_appldb['dynamic_th'] + sai_threshold_mode = 'SAI_BUFFER_PROFILE_THRESHOLD_MODE_DYNAMIC' + else: + sai_threshold_value = profile_appldb['static_th'] + sai_threshold_mode = 'SAI_BUFFER_PROFILE_THRESHOLD_MODE_STATIC' + assert profile_sai == {'SAI_BUFFER_PROFILE_ATTR_XON_TH': profile_appldb['xon'], + 'SAI_BUFFER_PROFILE_ATTR_XOFF_TH': profile_appldb['xoff'], + 'SAI_BUFFER_PROFILE_ATTR_RESERVED_BUFFER_SIZE': profile_appldb['size'], + 'SAI_BUFFER_PROFILE_ATTR_POOL_ID': pool_oid, + 'SAI_BUFFER_PROFILE_ATTR_THRESHOLD_MODE': sai_threshold_mode, + 'SAI_BUFFER_PROFILE_ATTR_SHARED_DYNAMIC_TH': sai_threshold_value} + + return profile_oid, pool_oid @pytest.fixture(params=['50000', '10000']) def speed_to_test(request): - ''' + """ @summary: used to parametrized test cases @param request: pytest request object @return: speed_to_test - ''' + """ return request.param @pytest.fixture(params=['15m', '40m']) def cable_len_to_test(request): - ''' + """ @summary: used to parametrized test cases @param request: pytest request object @return: cable_len_to_test - ''' + """ return request.param @pytest.fixture(params=['1500', '9100']) def mtu_to_test(request): - ''' + """ @summary: used to parametrized test cases @param request: pytest request object @return: cable_len_to_test - ''' + """ return request.param -lag_member_checked = False -lag_interface_port_belongs_to = None -port_under_test = None - -@pytest.fixture(params=['Ethernet8']) +@pytest.fixture() def port_to_test(request, duthost): - ''' + """ @summary: used to parametrized test cases @param request: pytest request object @return: port_under_test - ''' - global lag_member_checked - global lag_interface_port_belongs_to - global port_under_test + """ + dutLagInterfaces = [] + mgFacts = duthost.minigraph_facts(host=duthost.hostname)['ansible_facts'] + ports = mgFacts['minigraph_ports'].keys() + + for _, lag in mgFacts["minigraph_portchannels"].items(): + dutLagInterfaces += lag["members"] - port_under_test = request.param - if not lag_member_checked: - portchannel_member_key = duthost.shell('redis-cli -n 4 keys "PORTCHANNEL_MEMBER|*|{}"'.format(port_under_test))['stdout'] - if portchannel_member_key: - portchannel = portchannel_member_key.split('|')[1] - duthost.shell('config portchannel member del {} {}'.format(portchannel, port_under_test)) - logging.info("Preparing: remove port {} from port channel {}".format(port_under_test, portchannel)) - lag_interface_port_belongs_to = portchannel + testPort = set(mgFacts["minigraph_ports"].keys()) + testPort -= set(dutLagInterfaces) - return port_under_test + return list(testPort)[0] @pytest.fixture(params=['3-4', '6']) @@ -111,30 +272,47 @@ def pg_to_test(request): return request.param -@pytest.fixture(scope="module", autouse=True) -def teardown_module(duthost): - yield - - if lag_interface_port_belongs_to: - logging.info("Tearing down: restore the port channel configuration") - duthost.shell('config portchannel member add {} {}'.format(lag_interface_port_belongs_to, port_under_test)) - - def test_change_speed_cable(duthost, conn_graph_facts, port_to_test, speed_to_test, mtu_to_test, cable_len_to_test): - ''' - @summary: change speed in different ways and observe whether the DUT behaves correctly - if all of the speed_to_test, mtu_to_test and cable_len_to_test match the current value, the test will be skipped - @port_to_test: on which port will the test be performed - @speed_to_test: to what speed will the port's be changed - @mtu_to_test: to what mtu will the port's be changed - @cable_len_to_test: to what cable length will the port's be changed - ''' + """The testcase for changing the speed and cable length of a port + + Change the variables of the port, including speed, mtu and cable length, in different ways and observe whether the DUT behaves correctly + For any of the variable, if it matches the current port configuration, we will skip configuring it. + If all of the speed_to_test, mtu_to_test and cable_len_to_test match the current value, the test will be skipped + + The flow of the test case: + 1. Update the port configuration according to input parameters + 2. Determine whether the profile removing behavior can be verifyed: + If neither mtu nor cable length is default value, they will be applied on the port_to_test only, + and the generated profile will be removed after the configuration change because the profile is referenced + by this port only. + For example: + The mtu_to_test 1500 only applied on the port_to_test, thus the *_mtu1500_* profile is referenced by the port only + The *_mtu1500_* mtu will be removed after the mtu of the port is updated to default value. + In this case, we are able to verify whether the buffer profile is removed after mtu reverted or all PGs are removed. + Other the other hand, if the mtu is 9100, the buffer profile can be referenced by many other ports and it's less possible for us to verify the removing behavior. + We will remove and readd an extra PG 6 to verify the removing behavior as well. + 3. Each time the port configuration updated, the following items will be checked as much as possible: + - whether the new profile is generated in APPL_DB, STATE_DB and ASIC_DB + - whether the pool size is updated in APPL_DB and ASIC_DB + 4. Each time the PG on a port is added or removed, the following items will be checked: + - whether the profile referenced by PGs is as expected according to the port configuration + - whether the profile is removed if all PGs are removed and we are able to check removing behavior (result of step 2) + - whether the pfc_enable filed of the port has been updated accordingly + + Args: + port_to_test: on which port will the test be performed + speed_to_test: to what speed will the port's be changed + mtu_to_test: to what mtu will the port's be changed + cable_len_to_test: to what cable length will the port's be changed + """ original_speed = duthost.shell('redis-cli -n 4 hget "PORT|{}" speed'.format(port_to_test))['stdout'] original_cable_len = duthost.shell('redis-cli -n 4 hget "CABLE_LENGTH|AZURE" {}'.format(port_to_test))['stdout'] profile = duthost.shell('redis-cli hget "BUFFER_PG_TABLE:{}:3-4" profile'.format(port_to_test))['stdout'][1:-1] - original_headroom_size = duthost.shell('redis-cli hget "{}" size'.format(profile))['stdout'] - original_pool_size = duthost.shell('redis-cli hget BUFFER_POOL_TABLE:ingress_lossless_pool size')['stdout'] - number_of_pools = detect_ingress_pool_number(duthost) + + original_headroom_size = int(duthost.shell('redis-cli hget "{}" size'.format(profile))['stdout']) + original_pool_size = int(duthost.shell('redis-cli hget BUFFER_POOL_TABLE:ingress_lossless_pool size')['stdout']) + + initial_asic_db_profiles = fetch_initial_asic_db(duthost) if speed_to_test == original_speed and cable_len_to_test == original_cable_len and mtu_to_test == default_mtu: pytest.skip('Speed, MTU and cable length matches the default value, nothing to test, skip') @@ -152,53 +330,90 @@ def test_change_speed_cable(duthost, conn_graph_facts, port_to_test, speed_to_te check_profile_removed = cable_len_to_test not in default_cable_length_list - time.sleep(10) + time.sleep(20) # check whether profile is correct in PG table if mtu_to_test != default_mtu: expected_profile = 'pg_lossless_{}_{}_mtu{}_profile'.format(speed_to_test, cable_len_to_test, mtu_to_test) check_profile_removed = True else: expected_profile = 'pg_lossless_{}_{}_profile'.format(speed_to_test, cable_len_to_test) + logging.info('[speed and/or cable-len and/or MTU updated] Checking whether new profile {} has been created and pfc_enable has been updated'.format(expected_profile)) - check_profile(duthost, 'BUFFER_PG_TABLE:{}:3-4'.format(port_to_test), expected_profile) + check_pg_profile(duthost, 'BUFFER_PG_TABLE:{}:3-4'.format(port_to_test), expected_profile) check_pfc_enable(duthost, port_to_test, '3,4') + time.sleep(10) + profile_oid, pool_oid = check_buffer_profile_details(duthost, initial_asic_db_profiles, expected_profile, None, None) + logging.info('got sai oid for newly created profile {} ingress lossless pool {}'.format(profile_oid, pool_oid)) # check whether profile exist - headroom_size = duthost.shell('redis-cli hget "BUFFER_PROFILE_TABLE:{}" size'.format(expected_profile))['stdout'] - check_pool_size(duthost, (int(original_pool_size) * number_of_pools + (int(original_headroom_size) - int(headroom_size)) * 2) / number_of_pools) + headroom_size = int(duthost.shell('redis-cli hget "BUFFER_PROFILE_TABLE:{}" size'.format(expected_profile))['stdout']) + check_pool_size(duthost, + pool_oid, + pool_size = original_pool_size, + old_headroom = original_headroom_size, + new_headroom = headroom_size) # Remove all the lossless profile on the port logging.info('[remove all lossless PGs] Checking pool size and pfc_enable') duthost.shell('config interface buffer priority-group lossless remove {} 3-4'.format(port_to_test)) time.sleep(10) - check_pool_size(duthost, (int(original_pool_size) * number_of_pools + int(original_headroom_size) * 2) / number_of_pools) + check_pool_size(duthost, + pool_oid, + pool_size = original_pool_size, + old_headroom = original_headroom_size, + new_pg_number = 0) + check_pfc_enable(duthost, port_to_test, '') + if check_profile_removed: logging.info('[remove dynamic profile on PG removed] Checking whether the profile {} is removed on receiving all lossless PG removed'.format(expected_profile)) - check_lossless_profile_removed(duthost, expected_profile) + check_lossless_profile_removed(duthost, expected_profile, profile_oid) - # Re-add another lossless profile + # Re-add another lossless priority logging.info('Re-add a lossless_pg and check pool size and pfc_enable') duthost.shell('config interface buffer priority-group lossless add {} 6'.format(port_to_test)) time.sleep(10) - check_pool_size(duthost, (int(original_pool_size) * number_of_pools + int(original_headroom_size) * 2 - int(headroom_size)) / number_of_pools) + + check_pool_size(duthost, + pool_oid, + pool_size = original_pool_size, + old_headroom = original_headroom_size, + new_headroom = headroom_size, + new_pg_number = 1) + check_pfc_enable(duthost, port_to_test, '6') + profile_oid, _ = check_buffer_profile_details(duthost, initial_asic_db_profiles, expected_profile, None, pool_oid) if cable_len_to_test != original_cable_len: + logging.info('[revert the cable length to the default value] Checking whether the profile is updated') duthost.shell('config interface cable-length {} {}'.format(port_to_test, original_cable_len)) + if mtu_to_test != default_mtu: + logging.info('[revert the mtu to the default value] Checking whether the profile is updated') duthost.shell('config interface mtu {} {}'.format(port_to_test, default_mtu)) + # remove old profile on cable length change logging.info('[remove dynamic profile on cable length and/or MTU updated] Checking whether the old profile is removed') - check_lossless_profile_removed(duthost, expected_profile) + check_lossless_profile_removed(duthost, expected_profile, profile_oid) expected_profile = 'pg_lossless_{}_{}_profile'.format(speed_to_test, original_cable_len) - check_profile(duthost, 'BUFFER_PG_TABLE:{}:6'.format(port_to_test), expected_profile) - headroom_size = duthost.shell('redis-cli hget "BUFFER_PROFILE_TABLE:{}" size'.format(expected_profile))['stdout'] - check_pool_size(duthost, (int(original_pool_size) * number_of_pools + int(original_headroom_size) * 2 - int(headroom_size)) / number_of_pools) + check_pg_profile(duthost, 'BUFFER_PG_TABLE:{}:6'.format(port_to_test), expected_profile) + + headroom_size = int(duthost.shell('redis-cli hget "BUFFER_PROFILE_TABLE:{}" size'.format(expected_profile))['stdout']) + check_pool_size(duthost, + pool_oid, + pool_size = original_pool_size, + old_headroom = original_headroom_size, + new_headroom = headroom_size, + new_pg_number = 1) duthost.shell('config interface buffer priority-group lossless remove {} 6'.format(port_to_test)) time.sleep(10) - check_pool_size(duthost, (int(original_pool_size) * number_of_pools + int(original_headroom_size) * 2) / number_of_pools) + + check_pool_size(duthost, + pool_oid, + pool_size = original_pool_size, + old_headroom = original_headroom_size, + new_pg_number = 0) check_pfc_enable(duthost, port_to_test, '') else: if cable_len_to_test != original_cable_len: @@ -216,24 +431,35 @@ def test_change_speed_cable(duthost, conn_graph_facts, port_to_test, speed_to_te duthost.shell('config interface buffer priority-group lossless add {} 3-4'.format(port_to_test)) time.sleep(10) expected_profile = 'pg_lossless_{}_{}_profile'.format(original_speed, original_cable_len) - check_profile(duthost, 'BUFFER_PG_TABLE:{}:3-4'.format(port_to_test), expected_profile) + check_pg_profile(duthost, 'BUFFER_PG_TABLE:{}:3-4'.format(port_to_test), expected_profile) check_pfc_enable(duthost, port_to_test, '3,4') - headroom_size = duthost.shell('redis-cli hget "BUFFER_PROFILE_TABLE:{}" size'.format(expected_profile))['stdout'] - check_pool_size(duthost, int(original_pool_size)) + check_pool_size(duthost, + pool_oid, + pool_size = original_pool_size, + old_headroom = original_headroom_size) logging.info('[extra lossless PG]') duthost.shell('config interface buffer priority-group lossless add {} 6'.format(port_to_test)) time.sleep(10) - check_profile(duthost, 'BUFFER_PG_TABLE:{}:6'.format(port_to_test), expected_profile) + check_pg_profile(duthost, 'BUFFER_PG_TABLE:{}:6'.format(port_to_test), expected_profile) check_pfc_enable(duthost, port_to_test, '3,4,6') - check_pool_size(duthost, (int(original_pool_size) * number_of_pools - int(original_headroom_size)) / number_of_pools) + + check_pool_size(duthost, + pool_oid, + pool_size = original_pool_size, + old_headroom = original_headroom_size, + new_pg_number = 3) logging.info('[restore config]') duthost.shell('config interface buffer priority-group lossless remove {} 6'.format(port_to_test)) time.sleep(10) check_pfc_enable(duthost, port_to_test, '3,4') - check_pool_size(duthost, int(original_pool_size)) + + check_pool_size(duthost, + pool_oid, + pool_size = original_pool_size, + old_headroom = original_headroom_size) finally: duthost.shell('config interface buffer priority-group lossless remove {}'.format(port_to_test), module_ignore_errors = True) duthost.shell('config interface speed {} {}'.format(port_to_test, original_speed), module_ignore_errors = True) @@ -242,57 +468,152 @@ def test_change_speed_cable(duthost, conn_graph_facts, port_to_test, speed_to_te duthost.shell('config interface buffer priority-group lossless add {} 3-4'.format(port_to_test), module_ignore_errors = True) +def _parse_buffer_profile_params(param, cmd, name): + """A helper for test_headroom_override, parsing the parameters from the pre-provided json file + + Args: + param: the dict containing test parameters parsed from dynamic_buffer_param.json + return: tuple: new headroom size and cli string + + Return: + a tuple consists of: + the CLI string by which a headroom-override profile can be configured + the size of new profile + """ + cli_str = "config buffer profile {} {}".format(cmd, name) + xon = "" + if 'xon' in param: + xon = param['xon'] + cli_str += " --xon " + xon + + xoff = "" + if 'xoff' in param: + xoff = param['xoff'] + cli_str += " --xoff " + xoff + + size = "" + if 'size' in param: + size = param['size'] + cli_str += " --size " + size + new_headroom = int(size) + elif xoff and xon: + new_headroom = int(xon) + int(xoff) + else: + new_headroom = None + + if 'dynamic_th' in param: + cli_str += " --dynamic_th " + param['dynamic_th'] + return cli_str, new_headroom + + def test_headroom_override(duthost, conn_graph_facts, port_to_test): - ''' - @summary: headroom override test - @port_to_test: on which port will the test be performed - ''' + """Test case for headroom override + + Verify the headroom override behavior. + All arguments required for testing are fetched from a predefined json file on a per-vendor basis. + The test will be skipped in case the arguments are not provided. + + The flow of the test case: + 1. Fetch the parameters + 2. Add the headroom override profile and apply it to PG 3-4 on port_to_test + 3. Verify: + - whether the profile referenced by PG is correct + - whether the pfc_enable matches the PG + - whether the buffer profile is correct deployed in APPL_DB, STATE_DB and ASIC_DB + - whether the pool size has been updated correctly + 4. Add PG 6, verify the related info + 5. Update the headroom override profile and verify the related info + 6. Negative test: try to remove the headroom override profile. + Verify it is not removed because it is still being referenced. + 7. Revert the PG configurations, verify the related info + + Args: + port_to_test: on which port will the test be performed + """ + if not testparam_headroom_override: + pytest.skip("headroom override test skipped due to no parameters provided") + original_speed = duthost.shell('redis-cli -n 4 hget "PORT|{}" speed'.format(port_to_test))['stdout'] original_cable_len = duthost.shell('redis-cli -n 4 hget "CABLE_LENGTH|AZURE" {}'.format(port_to_test))['stdout'] original_profile = duthost.shell('redis-cli hget "BUFFER_PG_TABLE:{}:3-4" profile'.format(port_to_test))['stdout'][1:-1] original_headroom_size = duthost.shell('redis-cli hget "{}" size'.format(original_profile))['stdout'] original_pool_size = duthost.shell('redis-cli hget BUFFER_POOL_TABLE:ingress_lossless_pool size')['stdout'] - number_of_pools = detect_ingress_pool_number(duthost) + + initial_asic_db_profiles = fetch_initial_asic_db(duthost) try: # Configure a static profile - logging.info("[prepare configuration]") - duthost.shell('config buffer profile add headroom-override --xon 18432 --xoff 18432 --dynamic_th 1') + param = testparam_headroom_override.get("add") + if not param: + pytest.skip('headroom override test skipped due to no parameters for "add" command provided') + else: + cli_str, new_headroom = _parse_buffer_profile_params(param, "add", "headroom-override") + + logging.info("[prepare configuration] {}".format(cli_str)) + duthost.shell(cli_str) time.sleep(10) logging.info("[test: headroom override on lossless PG 3-4] apply the profile on the PG and check pool size") duthost.shell('config interface buffer priority-group lossless set {} 3-4 headroom-override'.format(port_to_test)) time.sleep(10) - check_profile(duthost, 'BUFFER_PG_TABLE:{}:3-4'.format(port_to_test), 'headroom-override') + check_pg_profile(duthost, 'BUFFER_PG_TABLE:{}:3-4'.format(port_to_test), 'headroom-override') check_pfc_enable(duthost, port_to_test, '3,4') - check_pool_size(duthost, (int(original_pool_size) * number_of_pools + 2 * int(original_headroom_size) - 73728) / number_of_pools) + profile_oid, pool_oid = check_buffer_profile_details(duthost, initial_asic_db_profiles, "headroom-override", None, None) + + check_pool_size(duthost, + pool_oid, + pool_size = original_pool_size, + old_headroom = original_headroom_size, + new_headroom = new_headroom) # Add another headroom override logging.info("[test: headroom override on more lossless PGs 6] apply the profile on the PG and check pool size") duthost.shell('config interface buffer priority-group lossless add {} 6 headroom-override'.format(port_to_test)) time.sleep(10) - check_profile(duthost, 'BUFFER_PG_TABLE:{}:6'.format(port_to_test), 'headroom-override') + check_pg_profile(duthost, 'BUFFER_PG_TABLE:{}:6'.format(port_to_test), 'headroom-override') check_pfc_enable(duthost, port_to_test, '3,4,6') - check_pool_size(duthost, (int(original_pool_size) * number_of_pools + 2 * int(original_headroom_size) - 110592) / number_of_pools) + profile_oid, _ = check_buffer_profile_details(duthost, initial_asic_db_profiles, "headroom-override", profile_oid, pool_oid) + + check_pool_size(duthost, + pool_oid, + pool_size = original_pool_size, + old_headroom = original_headroom_size, + new_headroom = 36864, + new_pg_number = 3) + + param = testparam_headroom_override.get("set") + if not param: + pytest.skip('headroom override test skipped due to no parameters for "set" command provided') + else: + cli_str, new_headroom = _parse_buffer_profile_params(param, "set", "headroom-override") + logging.info("[test: update headroom-override profile] update the profile and check pool size: {}".format(cli_str)) + duthost.shell(cli_str) - logging.info("[test: update headroom-override profile] update the profile and check pool size") - duthost.shell('config buffer profile set headroom-override --xon 18432 --xoff 36864') time.sleep(10) - check_pool_size(duthost, (int(original_pool_size) * number_of_pools + 2 * int(original_headroom_size) - 165888) / number_of_pools) + check_pool_size(duthost, + pool_oid, + pool_size = original_pool_size, + old_headroom = original_headroom_size, + new_headroom = new_headroom, + new_pg_number = 3) # Recover configuration logging.info("[test: static headroom being referenced can not be removed]") duthost.shell('config buffer profile remove headroom-override', module_ignore_errors = True) time.sleep(20) profile = duthost.shell('redis-cli hgetall "BUFFER_PROFILE_TABLE:headroom-override"')['stdout'] - assert profile, 'Headroom override profile has been removed when being referenced' + pytest_assert(profile, 'Headroom override profile has been removed when being referenced') logging.info("[recover configuration]") duthost.shell('config interface buffer priority-group lossless remove {}'.format(port_to_test)) duthost.shell('config interface buffer priority-group lossless add {} 3-4'.format(port_to_test)) time.sleep(10) - check_profile(duthost, 'BUFFER_PG_TABLE:{}:3-4'.format(port_to_test), original_profile.split(':')[1]) + check_pg_profile(duthost, 'BUFFER_PG_TABLE:{}:3-4'.format(port_to_test), original_profile.split(':')[1]) check_pfc_enable(duthost, port_to_test, '3,4') - check_pool_size(duthost, int(original_pool_size)) + check_pool_size(duthost, + pool_oid, + pool_size = original_pool_size, + old_headroom = original_headroom_size, + new_pg_number = 2) finally: duthost.shell('config interface buffer priority-group lossless remove {}'.format(port_to_test), module_ignore_errors = True) duthost.shell('config interface buffer priority-group lossless add {} 3-4'.format(port_to_test), module_ignore_errors = True) @@ -300,24 +621,32 @@ def test_headroom_override(duthost, conn_graph_facts, port_to_test): def test_lossless_pg(duthost, conn_graph_facts, port_to_test, pg_to_test): - ''' - @summary: non default dynamic th test - @port_to_test: on which port will the test be performed - @speed_to_test: to what speed will the port's be changed - ''' + """Test case for non default dynamic th + + Test case to verify the static profile with non default dynamic th + The buffer profile will be generated automatically after the profile has been applied to the port + The arguments required for the test are fetched from a predefiend json file on a per vendor basis. + Not providing any of the arguments results in the test case skipped. + + The flow of the test case: + 1. Configure a headroom override profile and check it in the APPL_DB, STATE_DB and ASIC_DB + 2. Configure a non default dynamic th profile + 3. Apply the nondefault dynamic th profile to PG 3-4 and update cable length + 4. Check whether a new buffer profile is created accordingly in the APPL_DB, STATE_DB and ASIC_DB + 5. Update the PG 3-4 to the default mode: dynamic profile + Verify whether the profile created in step 4 is removed + 6. Reconfigure it as non default dynamic th profile and check related info + 7. Update it to a headroom override profile and check related info + 8. Recover the configuration + + Args: + port_to_test: on which port will the test be performed + pg_to_test: to what PG will the profiles be applied + """ original_speed = duthost.shell('redis-cli -n 4 hget "PORT|{}" speed'.format(port_to_test))['stdout'] original_cable_len = duthost.shell('redis-cli -n 4 hget "CABLE_LENGTH|AZURE" {}'.format(port_to_test))['stdout'] - # create profiles - logging.info('[preparing]: create static buffer profile for headroom override and non default dynamic_th') - duthost.shell('config buffer profile add headroom-override --xon 18432 --xoff 32768') - duthost.shell('config buffer profile add non-default-dynamic_th --dynamic_th 2') - - # update cable length to 15m - logging.info('[preparing]: update cable length') - duthost.shell('config interface cable-length {} 15m'.format(port_to_test)) - expected_profile = 'pg_lossless_{}_15m_profile'.format(original_speed, original_cable_len) - check_profile(duthost, 'BUFFER_PG_TABLE:{}:3-4'.format(port_to_test), expected_profile) + initial_asic_db_profiles = fetch_initial_asic_db(duthost) set_command = 'config interface buffer priority-group lossless set {} {} '.format(port_to_test, pg_to_test) add_command = 'config interface buffer priority-group lossless add {} {} '.format(port_to_test, pg_to_test) @@ -329,49 +658,85 @@ def test_lossless_pg(duthost, conn_graph_facts, port_to_test, pg_to_test): buffer_pg = 'BUFFER_PG_TABLE:{}:{}'.format(port_to_test, pg_to_test) try: - # 1. original it should be a dynamic PG, update it to override + param = testparam_lossless_pg.get("headroom-override") + if not param: + pytest.skip('lossless pg test skipped due to no parameters for "headroom-override" command provided') + else: + cli_str, new_headroom = _parse_buffer_profile_params(param, "add", "headroom-override") + + # create profiles + logging.info('[preparing]: create static buffer profile for headroom override') + duthost.shell(cli_str) + headroom_override_profile_oid, pool_oid = check_buffer_profile_details(duthost, initial_asic_db_profiles, "headroom-override", None, None) + + initial_asic_db_profiles = fetch_initial_asic_db(duthost) + + # this is a dynamic profile with non default dynamic-th. + # profile won't be created until configured on some pg + param = testparam_lossless_pg.get("non-default-dynamic_th") + if not param: + pytest.skip('lossless pg test skipped due to no parameters for "non-default-dynamic_th" command provided') + else: + cli_str, new_headroom = _parse_buffer_profile_params(param, "add", "non-default-dynamic_th") + + logging.info('[preparing]: create static buffer profile for non default dynamic_th') + duthost.shell(cli_str) + + # update cable length to 15m + logging.info('[preparing]: update cable length') + duthost.shell('config interface cable-length {} 15m'.format(port_to_test)) + expected_profile = 'pg_lossless_{}_15m_profile'.format(original_speed) + check_pg_profile(duthost, 'BUFFER_PG_TABLE:{}:3-4'.format(port_to_test), expected_profile) + profile_oid, _ = check_buffer_profile_details(duthost, initial_asic_db_profiles, expected_profile, None, pool_oid) + + # original it should be a dynamic PG, update it to override logging.info('[testcase: dynamic headroom => headroom override]') duthost.shell(first_command + 'headroom-override') # check whether lossless dynamic profile is removed - check_profile(duthost, buffer_pg, 'headroom-override') + check_pg_profile(duthost, buffer_pg, 'headroom-override') if pg_to_test == '3-4': - check_lossless_profile_removed(duthost, expected_profile) + check_lossless_profile_removed(duthost, expected_profile, profile_oid) # update it to non-default dynamic_th logging.info('[testcase: headroom override => dynamically calculated headroom with non-default dynamic_th]') duthost.shell(set_command + 'non-default-dynamic_th') expected_nondef_profile = 'pg_lossless_{}_15m_th2_profile'.format(original_speed) - check_profile(duthost, buffer_pg, expected_nondef_profile) + check_pg_profile(duthost, buffer_pg, expected_nondef_profile) + # a new profile should be created in ASIC DB + profile_oid, _ = check_buffer_profile_details(duthost, initial_asic_db_profiles, expected_nondef_profile, None, pool_oid) # update it to dynamic PG logging.info('[testcase: dynamically calculated headroom with non-default dynamic_th => dynamic headroom]') duthost.shell(set_command) - check_profile(duthost, buffer_pg, expected_profile) - check_lossless_profile_removed(duthost, expected_nondef_profile) + check_pg_profile(duthost, buffer_pg, expected_profile) + check_lossless_profile_removed(duthost, expected_nondef_profile, profile_oid) # update it to non-default dynamic_th logging.info('[testcase: dynamic headroom => [dynamically calculated headroom with non-default dynamic_th]') duthost.shell(set_command + 'non-default-dynamic_th') - check_profile(duthost, buffer_pg, expected_nondef_profile) + check_pg_profile(duthost, buffer_pg, expected_nondef_profile) + # a new profile should be created in ASIC DB + profile_oid, _ = check_buffer_profile_details(duthost, initial_asic_db_profiles, expected_nondef_profile, None, pool_oid) if pg_to_test == '3-4': + # the oid can be reused by SAI. so don't check whether profile_oid is removed. check_lossless_profile_removed(duthost, expected_profile) # update it to headroom override logging.info('[testcase: dynamically calculated headroom with non-default dynamic_th => headroom override]') duthost.shell(set_command + 'headroom-override') - check_profile(duthost, buffer_pg, 'headroom-override') - check_lossless_profile_removed(duthost, expected_nondef_profile) + check_pg_profile(duthost, buffer_pg, 'headroom-override') + check_lossless_profile_removed(duthost, expected_nondef_profile, profile_oid) # update it to dynamic PG, recover logging.info('[testcase: headroom override => dynamic headroom]') duthost.shell(set_command) - check_profile(duthost, buffer_pg, expected_profile) + check_pg_profile(duthost, buffer_pg, expected_profile) # remove all static profiles logging.info('[restoring configuration]') duthost.shell('config buffer profile remove headroom-override') duthost.shell('config buffer profile remove non-default-dynamic_th') - check_lossless_profile_removed(duthost, 'headroom-override') + check_lossless_profile_removed(duthost, 'headroom-override', headroom_override_profile_oid) check_lossless_profile_removed(duthost, 'non-default-dynamic_th') finally: if pg_to_test == '3-4': @@ -384,9 +749,17 @@ def test_lossless_pg(duthost, conn_graph_facts, port_to_test, pg_to_test): def test_exceeding_headroom(duthost, conn_graph_facts, port_to_test): - ''' - @summary: - ''' + """The test case for maximum headroom + + If the accumulative headroom of a port exceeds the maximum value, + the new configuation causing the violation should not be applied to prevent orchagent from exiting + + The idea is to configure a super long cable which can cause a super large headroom thus exceeding the maximum value. + Afterthat, verify the profile of the PG isn't changed + + Args: + port_to_test: Port to run the test + """ max_headroom_size = duthost.shell('redis-cli -n 6 hget "BUFFER_MAX_PARAM_TABLE|{}" max_headroom_size'.format(port_to_test))['stdout'] if not max_headroom_size: pytest.skip('No max headroom found on port {}, skip'.format(port_to_test)) @@ -401,7 +774,7 @@ def test_exceeding_headroom(duthost, conn_graph_facts, port_to_test): duthost.shell('config interface cable-length {} 10000m'.format(port_to_test)) time.sleep(20) logging.info('verify the profile isn\'t changed') - check_profile(duthost, 'BUFFER_PG_TABLE:{}:3-4'.format(port_to_test), original_profile) + check_pg_profile(duthost, 'BUFFER_PG_TABLE:{}:3-4'.format(port_to_test), original_profile) duthost.shell('config interface cable-length {} {}'.format(port_to_test, original_cable_len)) # add additional PG @@ -410,7 +783,7 @@ def test_exceeding_headroom(duthost, conn_graph_facts, port_to_test): time.sleep(20) logging.info('verify the profile has been changed') expected_profile = 'pg_lossless_{}_{}_profile'.format(original_speed, '300m') - check_profile(duthost, 'BUFFER_PG_TABLE:{}:3-4'.format(port_to_test), expected_profile) + check_pg_profile(duthost, 'BUFFER_PG_TABLE:{}:3-4'.format(port_to_test), expected_profile) logging.info('add another PG and make sure the system isn\'t broken') duthost.shell('config interface buffer priority-group lossless add {} {}'.format(port_to_test, '5-7')) time.sleep(20) @@ -425,7 +798,7 @@ def test_exceeding_headroom(duthost, conn_graph_facts, port_to_test): duthost.shell('config interface buffer priority-group lossless set {} {} {}'.format(port_to_test, '3-4', 'test-headroom')) time.sleep(20) logging.info('verify the profile is applied') - check_profile(duthost, 'BUFFER_PG_TABLE:{}:3-4'.format(port_to_test), 'test-headroom') + check_pg_profile(duthost, 'BUFFER_PG_TABLE:{}:3-4'.format(port_to_test), 'test-headroom') duthost.shell('config interface buffer priority-group lossless add {} {} {}'.format(port_to_test, '5-7', 'test-headroom')) time.sleep(20) # again, we can't say for sure whether the accumulative headroom exceeding. @@ -437,7 +810,7 @@ def test_exceeding_headroom(duthost, conn_graph_facts, port_to_test): time.sleep(20) # this should make it exceed the limit, so the profile should not applied to the APPL_DB size_in_appldb = duthost.shell('redis-cli hget "BUFFER_PROFILE_TABLE:test-headroom" size')['stdout'] - assert size_in_appldb == '68432', 'The profile with a large size was applied to APPL_DB, which can make headroom exceeding' + pytest_assert(size_in_appldb == '68432', 'The profile with a large size was applied to APPL_DB, which can make headroom exceeding') duthost.shell('config interface buffer priority-group lossless set {} {}'.format(port_to_test, '3-4')) duthost.shell('config buffer profile remove test-headroom') logging.info('[clean up]') @@ -456,20 +829,20 @@ def _recovery_to_dynamic_buffer_model(duthost): def test_buffer_model_test(duthost, conn_graph_facts): - ''' - @summary: verify whether the buffer model is expected after configuration operations: - - whether the buffer model is traditional after executing config load_minigraph - - whether the buffer model is dynamic after recovering the buffer model to dynamic - ''' + """verify whether the buffer model is expected after configuration operations: + The following items are verified + - whether the buffer model is traditional after executing config load_minigraph + - whether the buffer model is dynamic after recovering the buffer model to dynamic + """ try: logging.info('[config load_minigraph]') config_reload(duthost, config_source='minigraph') buffer_model = duthost.shell('redis-cli -n 4 hget "DEVICE_METADATA|localhost" buffer_model')['stdout'] - assert buffer_model == 'traditional', 'Got buffer model {} after executing config load_minigraph, traditional expected' + pytest_assert(buffer_model == 'traditional', 'Got buffer model {} after executing config load_minigraph, traditional expected') logging.info('[Recover the DUT to default buffer model]') _recovery_to_dynamic_buffer_model(duthost) buffer_model = duthost.shell('redis-cli -n 4 hget "DEVICE_METADATA|localhost" buffer_model')['stdout'] - assert buffer_model == 'dynamic', 'Got buffer model {} after executing recovering the buffer model to dynamic' + pytest_assert(buffer_model == 'dynamic', 'Got buffer model {} after executing recovering the buffer model to dynamic') finally: _recovery_to_dynamic_buffer_model(duthost) From aa5c07277831f739c2ee3cb8a2d69f90dce17b90 Mon Sep 17 00:00:00 2001 From: Stephen Sun Date: Wed, 9 Dec 2020 15:12:15 +0000 Subject: [PATCH 3/9] Fix review comments Signed-off-by: Stephen Sun --- tests/qos/test_buffer.py | 352 +++++++++++++++++++++++++++------------ 1 file changed, 244 insertions(+), 108 deletions(-) diff --git a/tests/qos/test_buffer.py b/tests/qos/test_buffer.py index 48cc9c4bef0..e6084e252ee 100644 --- a/tests/qos/test_buffer.py +++ b/tests/qos/test_buffer.py @@ -17,7 +17,7 @@ default_cable_length_list = None default_lossless_headroom_data = None default_ingress_pool_number = 0 -default_mtu = '9100' +default_mtu = None testparam_headroom_override = None testparam_lossless_pg = None @@ -25,18 +25,45 @@ buffer_model_dynamic = True def detect_buffer_model(duthost): + """detect the current buffer model (dynamic or traditional) and store it for futher use. called only once when the module is initialized + + Args: + duthost: the DUT host object + """ global buffer_model_dynamic buffer_model = duthost.shell('redis-cli -n 4 hget "DEVICE_METADATA|localhost" buffer_model')['stdout'] - buffer_model_dynamic = buffer_model == 'dynamic' + buffer_model_dynamic = (buffer_model == 'dynamic') def detect_ingress_pool_number(duthost): + """detect the number of ingress buffer pools and store it for futher use. called only once when the module is initialized + + Args: + duthost: the DUT host object + """ global default_ingress_pool_number pools = duthost.shell('redis-cli -n 4 keys "BUFFER_POOL|ingress*"')['stdout'] default_ingress_pool_number = len(pools.split()) +def detect_default_mtu(duthost, port_to_test): + """detect the mtu and store it for futher use. called only once when the module is initialized + + Args: + duthost: the DUT host object + """ + global default_mtu + if not default_mtu: + logging.info("Default MTU {}".format(default_mtu)) + default_mtu = duthost.shell('redis-cli -n 4 hget "PORT|{}" mtu'.format(port_to_test))['stdout'] + + def load_lossless_headroom_data(duthost): + """load test parameters from the json file. called only once when the module is initialized + + Args: + duthost: the DUT host object + """ global default_lossless_headroom_data if not default_lossless_headroom_data: dut_hwsku = duthost.facts["hwsku"] @@ -60,12 +87,19 @@ def load_lossless_headroom_data(duthost): def load_test_parameters(duthost): + """load test parameters from the json file. called only once when the module is initialized + + Args: + duthost: the DUT host object + """ global default_cable_length_list global testparam_headroom_override global testparam_lossless_pg - with open(r"qos/files/dynamic_buffer_param.json") as file: + + param_file_name = "qos/files/dynamic_buffer_param.json" + with open(param_file_name) as file: params = json.load(file) - logging.info("Got test parameters {}".format(params)) + logging.info("Loaded test parameters {} from {}".format(params, param_file_name)) asic_type = duthost.facts['asic_type'] vendor_specific_param = params[asic_type] default_cable_length_list = vendor_specific_param['default_cable_length'] @@ -75,15 +109,20 @@ def load_test_parameters(duthost): @pytest.fixture(scope="module", autouse=True) def setup_module(duthost): + """Set up module, called only once when the module is initialized + + Args: + duthost: the DUT host object + """ detect_buffer_model(duthost) if buffer_model_dynamic: detect_ingress_pool_number(duthost) load_lossless_headroom_data(duthost) load_test_parameters(duthost) - logging.info("Got cable length: default {}".format(default_cable_length_list)) - logging.info("Got ingress pool number {}".format(default_ingress_pool_number)) - logging.info("Got lossless headroom data {}".format(default_lossless_headroom_data)) + logging.info("Cable length: default {}".format(default_cable_length_list)) + logging.info("Ingress pool number {}".format(default_ingress_pool_number)) + logging.info("Lossless headroom data {}".format(default_lossless_headroom_data)) else: pytest.skip("Dynamic buffer isn't enabled, skip the test") @@ -91,6 +130,23 @@ def setup_module(duthost): def check_pool_size(duthost, ingress_lossless_pool_oid, **kwargs): + """Check whether the pool size has been updated correctedly + + The expected pool size will be calculated based on the input arguments on a per-vendor basis + After that, it will check the expected value against the buffer pool size in BUFFER_POOL_TABLE + and in the ASIC_DB + + Args: + ingress_lossless_pool_oid: the SAI OID of the ingress lossless pool in ASIC_DB + kwargs: the parameters based on which the expected pool size is calculated. + they are represeted in form of kwargs because different vendor can require different parameters + for Mellanox, it includes: + - old / new pg size + - old / new pg numbers + - current pool size + - the expected pool size is calculated as: + current_pool_size + old_pg_num * old_pg_size - new_pg_num * new_pg_size + """ if duthost.facts['asic_type'] == 'mellanox': old_headroom = int(kwargs["old_headroom"]) @@ -117,41 +173,81 @@ def check_pool_size(duthost, ingress_lossless_pool_oid, **kwargs): original_memory = curr_pool_size * default_ingress_pool_number + old_headroom * old_pg_number expected_pool_size = (original_memory - new_reserved) / default_ingress_pool_number + + def _get_pool_size_from_asic_db(duthost, ingress_lossless_pool_oid): + pool_sai = _compose_dict_from_cli(duthost.shell('redis-cli -n 1 hgetall ASIC_STATE:SAI_OBJECT_TYPE_BUFFER_POOL:{}'.format(ingress_lossless_pool_oid))['stdout'].split('\n')) + return pool_sai['SAI_BUFFER_POOL_ATTR_SIZE'] + + def _check_pool_size(duthost, expected_pool_size, ingress_lossless_pool_oid): pool_size = duthost.shell('redis-cli hget "BUFFER_POOL_TABLE:ingress_lossless_pool" size')['stdout'] - pytest_assert(int(pool_size) == expected_pool_size, "Pool size isn't correct: expected {} but got {}".format(expected_pool_size, pool_size)) + #pytest_assert(int(pool_size) == expected_pool_size, "Pool size isn't correct: expected {} but got {}".format(expected_pool_size, pool_size)) + + if int(pool_size) != expected_pool_size: + return False if ingress_lossless_pool_oid: - pool_sai = _compose_dict_from_cli(duthost.shell('redis-cli -n 1 hgetall ASIC_STATE:SAI_OBJECT_TYPE_BUFFER_POOL:{}'.format(ingress_lossless_pool_oid))['stdout'].split('\n')) - pytest_assert(int(pool_sai['SAI_BUFFER_POOL_ATTR_SIZE']) == expected_pool_size) + pool_size = _get_pool_size_from_asic_db(duthost, ingress_lossless_pool_oid) + if int(pool_size) != expected_pool_size: + return False + #pytest_assert(int(pool_sai['SAI_BUFFER_POOL_ATTR_SIZE']) == expected_pool_size) + + return True + + pytest_assert(wait_until(20, 2, _check_pool_size, duthost, expected_pool_size, ingress_lossless_pool_oid), + "Pool size isn't correct in database: expected {}, size in APPL_DB {}, size in ASIC_DB {}".format( + expected_pool_size, + duthost.shell('redis-cli hget "BUFFER_POOL_TABLE:ingress_lossless_pool" size')['stdout'], + _get_pool_size_from_asic_db(duthost, ingress_lossless_pool_oid))) def check_pg_profile(duthost, pg, expected_profile): - profile = duthost.shell('redis-cli hget {} profile'.format(pg))['stdout'][1:-1] - pytest_assert(profile == 'BUFFER_PROFILE_TABLE:' + expected_profile, 'Expected profile {} not found'.format(expected_profile)) + """Check whether the profile in BUFFER_PG match the expected value in a wait_until loop with maximum timeout as 10 seconds + + Args: + pg: the key of buffer pg in BUFFER_PG table. Format: BUFFER_PG|| + expected_profile: the name of the expected profile + """ + def _check_pg_profile(duthost, pg, expected_profile): + profile = duthost.shell('redis-cli hget {} profile'.format(pg))['stdout'][1:-1] + return (profile == 'BUFFER_PROFILE_TABLE:' + expected_profile) + + pytest_assert(wait_until(10, 2, _check_pg_profile, duthost, pg, expected_profile), "Profile in PG {} isn't {}".format(pg, expected_profile)) def check_pfc_enable(duthost, port, expected_pfc_enable_map): - pfc_enable = duthost.shell('redis-cli -n 4 hget "PORT_QOS_MAP|{}" pfc_enable'.format(port))['stdout'] - pytest_assert(expected_pfc_enable_map == pfc_enable, \ - "Expected pfc enable map {} doesn't match {}".format(expected_pfc_enable_map, pfc_enable)) + """Check whether the pfc_enable map in port table is correct in a wait_until loop with maximum timeout as 10 seconds + + Args: + port: the port to be checked + expected_pfc_enable_map: the expected pfc_enable map + """ + def _check_pfc_enable(duthost, port, expected_pfc_enable_map): + pfc_enable = duthost.shell('redis-cli -n 4 hget "PORT_QOS_MAP|{}" pfc_enable'.format(port))['stdout'] + return (expected_pfc_enable_map == pfc_enable) + + pytest_assert(wait_until(10, 2, _check_pfc_enable, duthost, port, expected_pfc_enable_map), + "Port {} pfc enable check failed expected: {} got: {}".format( + port, + expected_pfc_enable_map, + duthost.shell('redis-cli -n 4 hget "PORT_QOS_MAP|{}" pfc_enable'.format(port))['stdout'])) def check_lossless_profile_removed(duthost, profile, sai_oid = None): - time.sleep(10) + """Check whether the lossless profile has been removed from APPL_DB, STATE_DB and ASIC_DB (if sai_oid provided) + + Args: + profile: the name of the buffer profile to be checked + sai_oid: the SAI OID in ASIC_DB of the buffer profile + if it is None the ASIC_DB won't be checked + """ profile_info = duthost.shell('redis-cli -n 6 hgetall "BUFFER_PROFILE_TABLE|{}"'.format(profile))['stdout'] pytest_assert(not profile_info, "Profile {} isn't removed from STATE_DB".format(profile)) profile_info = duthost.shell('redis-cli hgetall "BUFFER_PROFILE_TABLE:{}"'.format(profile))['stdout'] pytest_assert(not profile_info, "Profile {} isn't removed from APPL_DB".format(profile)) logging.debug('Profile {} has been removed from STATE_DB and APPL_DB'.format(profile)) if sai_oid: - profile_info = duthost.shell('redis-cli -n 1 hgetall {}'.format(sai_oid)) - pytest_assert('Profile {} has been removed from ASIC_DB'.format(sai_oid)) - - -def check_dynamic_th_in_appldb(duthost, profile, expected_dynamic_th, must_exist = False): - time.sleep(10) - dynamic_th = duthost.shell('redis-cli hget "BUFFER_PROFILE_TABLE:{}" dynamic_th'.format(profile))['stdout'] - pytest_assert(not must_exist and not dynamic_th or dynamic_th == expected_dynamic_th, "dynamic_th of profile {} in APPL_DB isn't correct".format(profile)) + profile_info = duthost.shell('redis-cli -n 1 hgetall {}'.format(sai_oid))['stdout'] + pytest_assert(not profile_info, "Profile {} hasn't been removed from ASIC_DB".format(sai_oid)) def fetch_initial_asic_db(duthost): @@ -160,12 +256,30 @@ def fetch_initial_asic_db(duthost): def _compose_dict_from_cli(fields_list): + """Convert the out put of hgetall command to a dict object containing the field, key pairs of the database table content + + Args: + fields_list: a list of lines, the output of redis-cli hgetall command + """ return dict(zip(fields_list[0::2], fields_list[1::2])) def check_buffer_profile_details(duthost, initial_profiles, profile_name, profile_oid, pool_oid): + """Check buffer profile details. + + The following items are tested: + - whether the headroom information, like xoff, is correct. + this is tested by comparing with standard profile in pg_profile_lookup table + - whether the profile information in APPL_DB matches that in ASIC_DB + + Args: + initial_profiles: the keys of buffer profiles in ASIC_DB at the beginning of the test + profile_name: name of the profile + profile_oid: SAI OID of the profile + pool_oid: SIA OID of ingress lossless pool + """ profile_appldb = _compose_dict_from_cli(duthost.shell('redis-cli hgetall BUFFER_PROFILE_TABLE:{}'.format(profile_name))['stdout'].split('\n')) - logging.debug("appl db profile {}".format(profile_appldb)) + logging.debug("APPL_DB buffer profile {}: {} ".format(profile_name, profile_appldb)) # check the profile against the standard value m = re.search(lossless_profile_pattern, profile_name) @@ -190,10 +304,12 @@ def check_buffer_profile_details(duthost, initial_profiles, profile_name, profil profiles_in_asicdb = set(duthost.shell('redis-cli -n 1 keys "ASIC_STATE:SAI_OBJECT_TYPE_BUFFER_PROFILE*"')['stdout'].split('\n')) diff = profiles_in_asicdb - initial_profiles - if len(diff): + if len(diff) == 1: profile_oid = diff.pop() + pytest_assert(profile_oid, "Unable to fetch SAI OID for profile {}, initial SAI OID set {} current set {}".format( + profile_name, initial_profiles, profiles_in_asicdb)) - logging.debug("initial profiles {} current profiles {} difference {}".format(initial_profiles, profiles_in_asicdb, diff)) + logging.debug("Initial profiles {} and current profiles {} has the following difference(s) {}".format(initial_profiles, profiles_in_asicdb, diff)) profile_sai = _compose_dict_from_cli(duthost.shell('redis-cli -n 1 hgetall {}'.format(profile_oid))['stdout'].split('\n')) @@ -219,40 +335,52 @@ def check_buffer_profile_details(duthost, initial_profiles, profile_name, profil @pytest.fixture(params=['50000', '10000']) def speed_to_test(request): - """ - @summary: used to parametrized test cases - @param request: pytest request object - @return: speed_to_test + """used to parametrized test cases for speeds + + Args: + param request: pytest request object + + Return: + speed_to_test """ return request.param @pytest.fixture(params=['15m', '40m']) def cable_len_to_test(request): - """ - @summary: used to parametrized test cases - @param request: pytest request object - @return: cable_len_to_test + """used to parametrized test cases for cable length + + Args: + request: pytest request object + + Return: + cable_len_to_test """ return request.param @pytest.fixture(params=['1500', '9100']) def mtu_to_test(request): - """ - @summary: used to parametrized test cases - @param request: pytest request object - @return: cable_len_to_test + """used to parametrized test cases for mtu + + Args: + request: pytest request object + + Return: + cable_len_to_test """ return request.param @pytest.fixture() def port_to_test(request, duthost): - """ - @summary: used to parametrized test cases - @param request: pytest request object - @return: port_under_test + """used to parametrized test cases for port + + Args: + request: pytest request object + + Return: + port_to_test """ dutLagInterfaces = [] mgFacts = duthost.minigraph_facts(host=duthost.hostname)['ansible_facts'] @@ -269,10 +397,18 @@ def port_to_test(request, duthost): @pytest.fixture(params=['3-4', '6']) def pg_to_test(request): + """used to parametrized test cases for PGs under test + + Args: + request: pytest request object + + Return: + pg_to_test + """ return request.param -def test_change_speed_cable(duthost, conn_graph_facts, port_to_test, speed_to_test, mtu_to_test, cable_len_to_test): +def test_change_speed_cable(duthosts, rand_one_dut_hostname, conn_graph_facts, port_to_test, speed_to_test, mtu_to_test, cable_len_to_test): """The testcase for changing the speed and cable length of a port Change the variables of the port, including speed, mtu and cable length, in different ways and observe whether the DUT behaves correctly @@ -305,9 +441,11 @@ def test_change_speed_cable(duthost, conn_graph_facts, port_to_test, speed_to_te mtu_to_test: to what mtu will the port's be changed cable_len_to_test: to what cable length will the port's be changed """ + duthost = duthosts[rand_one_dut_hostname] original_speed = duthost.shell('redis-cli -n 4 hget "PORT|{}" speed'.format(port_to_test))['stdout'] original_cable_len = duthost.shell('redis-cli -n 4 hget "CABLE_LENGTH|AZURE" {}'.format(port_to_test))['stdout'] profile = duthost.shell('redis-cli hget "BUFFER_PG_TABLE:{}:3-4" profile'.format(port_to_test))['stdout'][1:-1] + detect_default_mtu(duthost, port_to_test) original_headroom_size = int(duthost.shell('redis-cli hget "{}" size'.format(profile))['stdout']) original_pool_size = int(duthost.shell('redis-cli hget BUFFER_POOL_TABLE:ingress_lossless_pool size')['stdout']) @@ -330,7 +468,6 @@ def test_change_speed_cable(duthost, conn_graph_facts, port_to_test, speed_to_te check_profile_removed = cable_len_to_test not in default_cable_length_list - time.sleep(20) # check whether profile is correct in PG table if mtu_to_test != default_mtu: expected_profile = 'pg_lossless_{}_{}_mtu{}_profile'.format(speed_to_test, cable_len_to_test, mtu_to_test) @@ -338,12 +475,11 @@ def test_change_speed_cable(duthost, conn_graph_facts, port_to_test, speed_to_te else: expected_profile = 'pg_lossless_{}_{}_profile'.format(speed_to_test, cable_len_to_test) - logging.info('[speed and/or cable-len and/or MTU updated] Checking whether new profile {} has been created and pfc_enable has been updated'.format(expected_profile)) + logging.info('[Speed and/or cable-len and/or MTU updated] Checking whether new profile {} has been created and pfc_enable has been updated'.format(expected_profile)) check_pg_profile(duthost, 'BUFFER_PG_TABLE:{}:3-4'.format(port_to_test), expected_profile) check_pfc_enable(duthost, port_to_test, '3,4') - time.sleep(10) profile_oid, pool_oid = check_buffer_profile_details(duthost, initial_asic_db_profiles, expected_profile, None, None) - logging.info('got sai oid for newly created profile {} ingress lossless pool {}'.format(profile_oid, pool_oid)) + logging.info('SAI OID for newly created profile {} ingress lossless pool {}'.format(profile_oid, pool_oid)) # check whether profile exist headroom_size = int(duthost.shell('redis-cli hget "BUFFER_PROFILE_TABLE:{}" size'.format(expected_profile))['stdout']) @@ -354,9 +490,9 @@ def test_change_speed_cable(duthost, conn_graph_facts, port_to_test, speed_to_te new_headroom = headroom_size) # Remove all the lossless profile on the port - logging.info('[remove all lossless PGs] Checking pool size and pfc_enable') + logging.info('[Remove all lossless PGs] Checking pool size and pfc_enable') duthost.shell('config interface buffer priority-group lossless remove {} 3-4'.format(port_to_test)) - time.sleep(10) + check_pool_size(duthost, pool_oid, pool_size = original_pool_size, @@ -366,13 +502,12 @@ def test_change_speed_cable(duthost, conn_graph_facts, port_to_test, speed_to_te check_pfc_enable(duthost, port_to_test, '') if check_profile_removed: - logging.info('[remove dynamic profile on PG removed] Checking whether the profile {} is removed on receiving all lossless PG removed'.format(expected_profile)) + logging.info('[Remove dynamic profile on PG removed] Checking whether the profile {} is removed on receiving all lossless PG removed'.format(expected_profile)) check_lossless_profile_removed(duthost, expected_profile, profile_oid) # Re-add another lossless priority logging.info('Re-add a lossless_pg and check pool size and pfc_enable') duthost.shell('config interface buffer priority-group lossless add {} 6'.format(port_to_test)) - time.sleep(10) check_pool_size(duthost, pool_oid, @@ -385,15 +520,15 @@ def test_change_speed_cable(duthost, conn_graph_facts, port_to_test, speed_to_te profile_oid, _ = check_buffer_profile_details(duthost, initial_asic_db_profiles, expected_profile, None, pool_oid) if cable_len_to_test != original_cable_len: - logging.info('[revert the cable length to the default value] Checking whether the profile is updated') + logging.info('[Revert the cable length to the default value] Checking whether the profile is updated') duthost.shell('config interface cable-length {} {}'.format(port_to_test, original_cable_len)) if mtu_to_test != default_mtu: - logging.info('[revert the mtu to the default value] Checking whether the profile is updated') + logging.info('[Revert the mtu to the default value] Checking whether the profile is updated') duthost.shell('config interface mtu {} {}'.format(port_to_test, default_mtu)) # remove old profile on cable length change - logging.info('[remove dynamic profile on cable length and/or MTU updated] Checking whether the old profile is removed') + logging.info('[Remove dynamic profile on cable length and/or MTU updated] Checking whether the old profile is removed') check_lossless_profile_removed(duthost, expected_profile, profile_oid) expected_profile = 'pg_lossless_{}_{}_profile'.format(speed_to_test, original_cable_len) check_pg_profile(duthost, 'BUFFER_PG_TABLE:{}:6'.format(port_to_test), expected_profile) @@ -407,7 +542,6 @@ def test_change_speed_cable(duthost, conn_graph_facts, port_to_test, speed_to_te new_pg_number = 1) duthost.shell('config interface buffer priority-group lossless remove {} 6'.format(port_to_test)) - time.sleep(10) check_pool_size(duthost, pool_oid, @@ -417,19 +551,19 @@ def test_change_speed_cable(duthost, conn_graph_facts, port_to_test, speed_to_te check_pfc_enable(duthost, port_to_test, '') else: if cable_len_to_test != original_cable_len: - logging.info('[update cable length without any lossless pg configured]') + logging.info('[Update cable length without any lossless pg configured]') duthost.shell('config interface cable-length {} {}'.format(port_to_test, original_cable_len)) if mtu_to_test != default_mtu: - logging.info('[update mtu without any lossless pg configured]') + logging.info('[Update mtu without any lossless pg configured]') duthost.shell('config interface mtu {} {}'.format(port_to_test, default_mtu)) if speed_to_test != original_speed: - logging.info('[update speed without any lossless pg configured]') + logging.info('[Update speed without any lossless pg configured]') duthost.shell('config interface speed {} {}'.format(port_to_test, original_speed)) - logging.info('[add lossless pg with speed and cable length ready]') + logging.info('[Add lossless pg with speed and cable length ready]') duthost.shell('config interface buffer priority-group lossless add {} 3-4'.format(port_to_test)) - time.sleep(10) + expected_profile = 'pg_lossless_{}_{}_profile'.format(original_speed, original_cable_len) check_pg_profile(duthost, 'BUFFER_PG_TABLE:{}:3-4'.format(port_to_test), expected_profile) check_pfc_enable(duthost, port_to_test, '3,4') @@ -439,9 +573,9 @@ def test_change_speed_cable(duthost, conn_graph_facts, port_to_test, speed_to_te pool_size = original_pool_size, old_headroom = original_headroom_size) - logging.info('[extra lossless PG]') + logging.info('[Extra lossless PG]') duthost.shell('config interface buffer priority-group lossless add {} 6'.format(port_to_test)) - time.sleep(10) + check_pg_profile(duthost, 'BUFFER_PG_TABLE:{}:6'.format(port_to_test), expected_profile) check_pfc_enable(duthost, port_to_test, '3,4,6') @@ -451,9 +585,9 @@ def test_change_speed_cable(duthost, conn_graph_facts, port_to_test, speed_to_te old_headroom = original_headroom_size, new_pg_number = 3) - logging.info('[restore config]') + logging.info('[Restore config]') duthost.shell('config interface buffer priority-group lossless remove {} 6'.format(port_to_test)) - time.sleep(10) + check_pfc_enable(duthost, port_to_test, '3,4') check_pool_size(duthost, @@ -506,7 +640,7 @@ def _parse_buffer_profile_params(param, cmd, name): return cli_str, new_headroom -def test_headroom_override(duthost, conn_graph_facts, port_to_test): +def test_headroom_override(duthosts, rand_one_dut_hostname, conn_graph_facts, port_to_test): """Test case for headroom override Verify the headroom override behavior. @@ -530,6 +664,7 @@ def test_headroom_override(duthost, conn_graph_facts, port_to_test): Args: port_to_test: on which port will the test be performed """ + duthost = duthosts[rand_one_dut_hostname] if not testparam_headroom_override: pytest.skip("headroom override test skipped due to no parameters provided") @@ -549,13 +684,12 @@ def test_headroom_override(duthost, conn_graph_facts, port_to_test): else: cli_str, new_headroom = _parse_buffer_profile_params(param, "add", "headroom-override") - logging.info("[prepare configuration] {}".format(cli_str)) + logging.info("[Prepare configuration] {}".format(cli_str)) duthost.shell(cli_str) - time.sleep(10) - logging.info("[test: headroom override on lossless PG 3-4] apply the profile on the PG and check pool size") + logging.info("[Test: headroom override on lossless PG 3-4] apply the profile on the PG and check pool size") duthost.shell('config interface buffer priority-group lossless set {} 3-4 headroom-override'.format(port_to_test)) - time.sleep(10) + check_pg_profile(duthost, 'BUFFER_PG_TABLE:{}:3-4'.format(port_to_test), 'headroom-override') check_pfc_enable(duthost, port_to_test, '3,4') profile_oid, pool_oid = check_buffer_profile_details(duthost, initial_asic_db_profiles, "headroom-override", None, None) @@ -567,9 +701,9 @@ def test_headroom_override(duthost, conn_graph_facts, port_to_test): new_headroom = new_headroom) # Add another headroom override - logging.info("[test: headroom override on more lossless PGs 6] apply the profile on the PG and check pool size") + logging.info("[Test: headroom override on more lossless PGs 6] apply the profile on the PG and check pool size") duthost.shell('config interface buffer priority-group lossless add {} 6 headroom-override'.format(port_to_test)) - time.sleep(10) + check_pg_profile(duthost, 'BUFFER_PG_TABLE:{}:6'.format(port_to_test), 'headroom-override') check_pfc_enable(duthost, port_to_test, '3,4,6') profile_oid, _ = check_buffer_profile_details(duthost, initial_asic_db_profiles, "headroom-override", profile_oid, pool_oid) @@ -586,10 +720,9 @@ def test_headroom_override(duthost, conn_graph_facts, port_to_test): pytest.skip('headroom override test skipped due to no parameters for "set" command provided') else: cli_str, new_headroom = _parse_buffer_profile_params(param, "set", "headroom-override") - logging.info("[test: update headroom-override profile] update the profile and check pool size: {}".format(cli_str)) + logging.info("[Test: update headroom-override profile] update the profile and check pool size: {}".format(cli_str)) duthost.shell(cli_str) - time.sleep(10) check_pool_size(duthost, pool_oid, pool_size = original_pool_size, @@ -598,15 +731,15 @@ def test_headroom_override(duthost, conn_graph_facts, port_to_test): new_pg_number = 3) # Recover configuration - logging.info("[test: static headroom being referenced can not be removed]") + logging.info("[Test: static headroom being referenced can not be removed]") duthost.shell('config buffer profile remove headroom-override', module_ignore_errors = True) - time.sleep(20) + profile = duthost.shell('redis-cli hgetall "BUFFER_PROFILE_TABLE:headroom-override"')['stdout'] pytest_assert(profile, 'Headroom override profile has been removed when being referenced') - logging.info("[recover configuration]") + logging.info("[Recover configuration]") duthost.shell('config interface buffer priority-group lossless remove {}'.format(port_to_test)) duthost.shell('config interface buffer priority-group lossless add {} 3-4'.format(port_to_test)) - time.sleep(10) + check_pg_profile(duthost, 'BUFFER_PG_TABLE:{}:3-4'.format(port_to_test), original_profile.split(':')[1]) check_pfc_enable(duthost, port_to_test, '3,4') check_pool_size(duthost, @@ -620,7 +753,7 @@ def test_headroom_override(duthost, conn_graph_facts, port_to_test): duthost.shell('config buffer profile remove headroom-override', module_ignore_errors = True) -def test_lossless_pg(duthost, conn_graph_facts, port_to_test, pg_to_test): +def test_lossless_pg(duthosts, rand_one_dut_hostname, conn_graph_facts, port_to_test, pg_to_test): """Test case for non default dynamic th Test case to verify the static profile with non default dynamic th @@ -643,6 +776,7 @@ def test_lossless_pg(duthost, conn_graph_facts, port_to_test, pg_to_test): port_to_test: on which port will the test be performed pg_to_test: to what PG will the profiles be applied """ + duthost = duthosts[rand_one_dut_hostname] original_speed = duthost.shell('redis-cli -n 4 hget "PORT|{}" speed'.format(port_to_test))['stdout'] original_cable_len = duthost.shell('redis-cli -n 4 hget "CABLE_LENGTH|AZURE" {}'.format(port_to_test))['stdout'] @@ -660,12 +794,12 @@ def test_lossless_pg(duthost, conn_graph_facts, port_to_test, pg_to_test): try: param = testparam_lossless_pg.get("headroom-override") if not param: - pytest.skip('lossless pg test skipped due to no parameters for "headroom-override" command provided') + pytest.skip('Lossless pg test skipped due to no parameters for "headroom-override" command provided') else: cli_str, new_headroom = _parse_buffer_profile_params(param, "add", "headroom-override") # create profiles - logging.info('[preparing]: create static buffer profile for headroom override') + logging.info('[Preparing]: create static buffer profile for headroom override') duthost.shell(cli_str) headroom_override_profile_oid, pool_oid = check_buffer_profile_details(duthost, initial_asic_db_profiles, "headroom-override", None, None) @@ -679,18 +813,18 @@ def test_lossless_pg(duthost, conn_graph_facts, port_to_test, pg_to_test): else: cli_str, new_headroom = _parse_buffer_profile_params(param, "add", "non-default-dynamic_th") - logging.info('[preparing]: create static buffer profile for non default dynamic_th') + logging.info('[Preparing]: create static buffer profile for non default dynamic_th') duthost.shell(cli_str) # update cable length to 15m - logging.info('[preparing]: update cable length') + logging.info('[Preparing]: update cable length') duthost.shell('config interface cable-length {} 15m'.format(port_to_test)) expected_profile = 'pg_lossless_{}_15m_profile'.format(original_speed) check_pg_profile(duthost, 'BUFFER_PG_TABLE:{}:3-4'.format(port_to_test), expected_profile) profile_oid, _ = check_buffer_profile_details(duthost, initial_asic_db_profiles, expected_profile, None, pool_oid) # original it should be a dynamic PG, update it to override - logging.info('[testcase: dynamic headroom => headroom override]') + logging.info('[Testcase: dynamic headroom => headroom override]') duthost.shell(first_command + 'headroom-override') # check whether lossless dynamic profile is removed check_pg_profile(duthost, buffer_pg, 'headroom-override') @@ -698,7 +832,7 @@ def test_lossless_pg(duthost, conn_graph_facts, port_to_test, pg_to_test): check_lossless_profile_removed(duthost, expected_profile, profile_oid) # update it to non-default dynamic_th - logging.info('[testcase: headroom override => dynamically calculated headroom with non-default dynamic_th]') + logging.info('[Testcase: headroom override => dynamically calculated headroom with non-default dynamic_th]') duthost.shell(set_command + 'non-default-dynamic_th') expected_nondef_profile = 'pg_lossless_{}_15m_th2_profile'.format(original_speed) check_pg_profile(duthost, buffer_pg, expected_nondef_profile) @@ -706,13 +840,13 @@ def test_lossless_pg(duthost, conn_graph_facts, port_to_test, pg_to_test): profile_oid, _ = check_buffer_profile_details(duthost, initial_asic_db_profiles, expected_nondef_profile, None, pool_oid) # update it to dynamic PG - logging.info('[testcase: dynamically calculated headroom with non-default dynamic_th => dynamic headroom]') + logging.info('[Testcase: dynamically calculated headroom with non-default dynamic_th => dynamic headroom]') duthost.shell(set_command) check_pg_profile(duthost, buffer_pg, expected_profile) check_lossless_profile_removed(duthost, expected_nondef_profile, profile_oid) # update it to non-default dynamic_th - logging.info('[testcase: dynamic headroom => [dynamically calculated headroom with non-default dynamic_th]') + logging.info('[Testcase: dynamic headroom => dynamically calculated headroom with non-default dynamic_th]') duthost.shell(set_command + 'non-default-dynamic_th') check_pg_profile(duthost, buffer_pg, expected_nondef_profile) # a new profile should be created in ASIC DB @@ -722,18 +856,18 @@ def test_lossless_pg(duthost, conn_graph_facts, port_to_test, pg_to_test): check_lossless_profile_removed(duthost, expected_profile) # update it to headroom override - logging.info('[testcase: dynamically calculated headroom with non-default dynamic_th => headroom override]') + logging.info('[Testcase: dynamically calculated headroom with non-default dynamic_th => headroom override]') duthost.shell(set_command + 'headroom-override') check_pg_profile(duthost, buffer_pg, 'headroom-override') check_lossless_profile_removed(duthost, expected_nondef_profile, profile_oid) # update it to dynamic PG, recover - logging.info('[testcase: headroom override => dynamic headroom]') + logging.info('[Testcase: headroom override => dynamic headroom]') duthost.shell(set_command) check_pg_profile(duthost, buffer_pg, expected_profile) # remove all static profiles - logging.info('[restoring configuration]') + logging.info('[Restoring configuration]') duthost.shell('config buffer profile remove headroom-override') duthost.shell('config buffer profile remove non-default-dynamic_th') check_lossless_profile_removed(duthost, 'headroom-override', headroom_override_profile_oid) @@ -748,7 +882,7 @@ def test_lossless_pg(duthost, conn_graph_facts, port_to_test, pg_to_test): duthost.shell('config buffer profile remove non-default-dynamic_th', module_ignore_errors = True) -def test_exceeding_headroom(duthost, conn_graph_facts, port_to_test): +def test_exceeding_headroom(duthosts, rand_one_dut_hostname, conn_graph_facts, port_to_test): """The test case for maximum headroom If the accumulative headroom of a port exceeds the maximum value, @@ -760,6 +894,7 @@ def test_exceeding_headroom(duthost, conn_graph_facts, port_to_test): Args: port_to_test: Port to run the test """ + duthost = duthosts[rand_one_dut_hostname] max_headroom_size = duthost.shell('redis-cli -n 6 hget "BUFFER_MAX_PARAM_TABLE|{}" max_headroom_size'.format(port_to_test))['stdout'] if not max_headroom_size: pytest.skip('No max headroom found on port {}, skip'.format(port_to_test)) @@ -770,50 +905,50 @@ def test_exceeding_headroom(duthost, conn_graph_facts, port_to_test): try: # set to super long cable length - logging.info('[config a super long cable length]') + logging.info('[Config a super long cable length]') duthost.shell('config interface cable-length {} 10000m'.format(port_to_test)) - time.sleep(20) - logging.info('verify the profile isn\'t changed') + + logging.info('Verify the profile isn\'t changed') check_pg_profile(duthost, 'BUFFER_PG_TABLE:{}:3-4'.format(port_to_test), original_profile) duthost.shell('config interface cable-length {} {}'.format(port_to_test, original_cable_len)) # add additional PG - logging.info('[config the cable length on the port]') + logging.info('[Config the cable length on the port]') duthost.shell('config interface cable-length {} 300m'.format(port_to_test)) - time.sleep(20) - logging.info('verify the profile has been changed') + + logging.info('Verify the profile has been changed') expected_profile = 'pg_lossless_{}_{}_profile'.format(original_speed, '300m') check_pg_profile(duthost, 'BUFFER_PG_TABLE:{}:3-4'.format(port_to_test), expected_profile) - logging.info('add another PG and make sure the system isn\'t broken') + logging.info('Add another PG and make sure the system isn\'t broken') duthost.shell('config interface buffer priority-group lossless add {} {}'.format(port_to_test, '5-7')) - time.sleep(20) + # we can't say whether this will accumulative headroom exceed the limit, but the system should not crash # leverage sanity check to verify that duthost.shell('config interface buffer priority-group lossless remove {} {}'.format(port_to_test, '5-7')) duthost.shell('config interface cable-length {} {}'.format(port_to_test, original_cable_len)) # static profile - logging.info('[config headroom override to PG 3-4]') + logging.info('[Config headroom override to PG 3-4]') duthost.shell('config buffer profile add test-headroom --xon 18432 --xoff 50000 -headroom 68432') duthost.shell('config interface buffer priority-group lossless set {} {} {}'.format(port_to_test, '3-4', 'test-headroom')) - time.sleep(20) - logging.info('verify the profile is applied') + + logging.info('Verify the profile is applied') check_pg_profile(duthost, 'BUFFER_PG_TABLE:{}:3-4'.format(port_to_test), 'test-headroom') duthost.shell('config interface buffer priority-group lossless add {} {} {}'.format(port_to_test, '5-7', 'test-headroom')) - time.sleep(20) + # again, we can't say for sure whether the accumulative headroom exceeding. # just make sure the system doesn't crash duthost.shell('config interface buffer priority-group lossless remove {} {}'.format(port_to_test, '5-7')) - logging.info('[update headroom override to a lager size]') + logging.info('[Update headroom override to a larger size]') duthost.shell('config buffer profile set test-headroom --xon 18432 --xoff 860160 -headroom 878592') - time.sleep(20) + # this should make it exceed the limit, so the profile should not applied to the APPL_DB size_in_appldb = duthost.shell('redis-cli hget "BUFFER_PROFILE_TABLE:test-headroom" size')['stdout'] pytest_assert(size_in_appldb == '68432', 'The profile with a large size was applied to APPL_DB, which can make headroom exceeding') duthost.shell('config interface buffer priority-group lossless set {} {}'.format(port_to_test, '3-4')) duthost.shell('config buffer profile remove test-headroom') - logging.info('[clean up]') + logging.info('[Clean up]') finally: duthost.shell('config interface cable-length {} {}'.format(port_to_test, original_cable_len), module_ignore_errors = True) duthost.shell('config interface buffer priority-group lossless remove {} 5-7'.format(port_to_test), module_ignore_errors = True) @@ -828,14 +963,15 @@ def _recovery_to_dynamic_buffer_model(duthost): config_reload(duthost, config_source='config_db') -def test_buffer_model_test(duthost, conn_graph_facts): +def test_buffer_model_test(duthosts, rand_one_dut_hostname, conn_graph_facts): """verify whether the buffer model is expected after configuration operations: The following items are verified - whether the buffer model is traditional after executing config load_minigraph - whether the buffer model is dynamic after recovering the buffer model to dynamic """ + duthost = duthosts[rand_one_dut_hostname] try: - logging.info('[config load_minigraph]') + logging.info('[Config load_minigraph]') config_reload(duthost, config_source='minigraph') buffer_model = duthost.shell('redis-cli -n 4 hget "DEVICE_METADATA|localhost" buffer_model')['stdout'] pytest_assert(buffer_model == 'traditional', 'Got buffer model {} after executing config load_minigraph, traditional expected') From b1ea1e988640bcae6cae0f61dce92fe9d8dab250 Mon Sep 17 00:00:00 2001 From: Stephen Sun Date: Wed, 16 Dec 2020 17:33:48 +0800 Subject: [PATCH 4/9] Fix a typo and remove unused code Signed-off-by: Stephen Sun --- tests/qos/test_buffer.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/qos/test_buffer.py b/tests/qos/test_buffer.py index e6084e252ee..444b13e5b19 100644 --- a/tests/qos/test_buffer.py +++ b/tests/qos/test_buffer.py @@ -180,7 +180,6 @@ def _get_pool_size_from_asic_db(duthost, ingress_lossless_pool_oid): def _check_pool_size(duthost, expected_pool_size, ingress_lossless_pool_oid): pool_size = duthost.shell('redis-cli hget "BUFFER_POOL_TABLE:ingress_lossless_pool" size')['stdout'] - #pytest_assert(int(pool_size) == expected_pool_size, "Pool size isn't correct: expected {} but got {}".format(expected_pool_size, pool_size)) if int(pool_size) != expected_pool_size: return False @@ -189,7 +188,6 @@ def _check_pool_size(duthost, expected_pool_size, ingress_lossless_pool_oid): pool_size = _get_pool_size_from_asic_db(duthost, ingress_lossless_pool_oid) if int(pool_size) != expected_pool_size: return False - #pytest_assert(int(pool_sai['SAI_BUFFER_POOL_ATTR_SIZE']) == expected_pool_size) return True @@ -712,7 +710,7 @@ def test_headroom_override(duthosts, rand_one_dut_hostname, conn_graph_facts, po pool_oid, pool_size = original_pool_size, old_headroom = original_headroom_size, - new_headroom = 36864, + new_headroom = new_headroom, new_pg_number = 3) param = testparam_headroom_override.get("set") From 5bbf23a27db6037f4316b14a4fce44b5b340edbd Mon Sep 17 00:00:00 2001 From: Stephen Sun Date: Mon, 28 Dec 2020 18:15:51 +0800 Subject: [PATCH 5/9] Fix a syntax error by replacing "has" with "have" Signed-off-by: Stephen Sun --- tests/qos/test_buffer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/qos/test_buffer.py b/tests/qos/test_buffer.py index 444b13e5b19..e6e46bc2c72 100644 --- a/tests/qos/test_buffer.py +++ b/tests/qos/test_buffer.py @@ -307,7 +307,7 @@ def check_buffer_profile_details(duthost, initial_profiles, profile_name, profil pytest_assert(profile_oid, "Unable to fetch SAI OID for profile {}, initial SAI OID set {} current set {}".format( profile_name, initial_profiles, profiles_in_asicdb)) - logging.debug("Initial profiles {} and current profiles {} has the following difference(s) {}".format(initial_profiles, profiles_in_asicdb, diff)) + logging.debug("Initial profiles {} and current profiles {} have the following difference(s) {}".format(initial_profiles, profiles_in_asicdb, diff)) profile_sai = _compose_dict_from_cli(duthost.shell('redis-cli -n 1 hgetall {}'.format(profile_oid))['stdout'].split('\n')) From 6d7e3a2a97a838f4b66f1bc663353ee3f49da4d5 Mon Sep 17 00:00:00 2001 From: Stephen Sun Date: Mon, 28 Dec 2020 18:59:06 +0800 Subject: [PATCH 6/9] Use the capital letters at the beginning of the sentence. Signed-off-by: Stephen Sun --- tests/qos/test_buffer.py | 205 +++++++++++++++++++-------------------- 1 file changed, 102 insertions(+), 103 deletions(-) diff --git a/tests/qos/test_buffer.py b/tests/qos/test_buffer.py index e6e46bc2c72..a7ab90387e7 100644 --- a/tests/qos/test_buffer.py +++ b/tests/qos/test_buffer.py @@ -25,10 +25,10 @@ buffer_model_dynamic = True def detect_buffer_model(duthost): - """detect the current buffer model (dynamic or traditional) and store it for futher use. called only once when the module is initialized + """Detect the current buffer model (dynamic or traditional) and store it for futher use. Called only once when the module is initialized Args: - duthost: the DUT host object + duthost: The DUT host object """ global buffer_model_dynamic buffer_model = duthost.shell('redis-cli -n 4 hget "DEVICE_METADATA|localhost" buffer_model')['stdout'] @@ -36,10 +36,10 @@ def detect_buffer_model(duthost): def detect_ingress_pool_number(duthost): - """detect the number of ingress buffer pools and store it for futher use. called only once when the module is initialized + """Detect the number of ingress buffer pools and store it for futher use. Called only once when the module is initialized Args: - duthost: the DUT host object + duthost: The DUT host object """ global default_ingress_pool_number pools = duthost.shell('redis-cli -n 4 keys "BUFFER_POOL|ingress*"')['stdout'] @@ -47,10 +47,10 @@ def detect_ingress_pool_number(duthost): def detect_default_mtu(duthost, port_to_test): - """detect the mtu and store it for futher use. called only once when the module is initialized + """Detect the mtu and store it for futher use. Called only once when the module is initialized Args: - duthost: the DUT host object + duthost: The DUT host object """ global default_mtu if not default_mtu: @@ -59,7 +59,7 @@ def detect_default_mtu(duthost, port_to_test): def load_lossless_headroom_data(duthost): - """load test parameters from the json file. called only once when the module is initialized + """Load test parameters from the json file. Called only once when the module is initialized Args: duthost: the DUT host object @@ -87,10 +87,10 @@ def load_lossless_headroom_data(duthost): def load_test_parameters(duthost): - """load test parameters from the json file. called only once when the module is initialized + """Load test parameters from the json file. Called only once when the module is initialized Args: - duthost: the DUT host object + duthost: The DUT host object """ global default_cable_length_list global testparam_headroom_override @@ -109,10 +109,10 @@ def load_test_parameters(duthost): @pytest.fixture(scope="module", autouse=True) def setup_module(duthost): - """Set up module, called only once when the module is initialized + """Set up module. Called only once when the module is initialized Args: - duthost: the DUT host object + duthost: The DUT host object """ detect_buffer_model(duthost) if buffer_model_dynamic: @@ -137,10 +137,10 @@ def check_pool_size(duthost, ingress_lossless_pool_oid, **kwargs): and in the ASIC_DB Args: - ingress_lossless_pool_oid: the SAI OID of the ingress lossless pool in ASIC_DB - kwargs: the parameters based on which the expected pool size is calculated. - they are represeted in form of kwargs because different vendor can require different parameters - for Mellanox, it includes: + ingress_lossless_pool_oid: The SAI OID of the ingress lossless pool in ASIC_DB + kwargs: The parameters based on which the expected pool size is calculated. + They are represeted in form of kwargs because different vendor can require different parameters + For Mellanox, it includes: - old / new pg size - old / new pg numbers - current pool size @@ -202,8 +202,8 @@ def check_pg_profile(duthost, pg, expected_profile): """Check whether the profile in BUFFER_PG match the expected value in a wait_until loop with maximum timeout as 10 seconds Args: - pg: the key of buffer pg in BUFFER_PG table. Format: BUFFER_PG|| - expected_profile: the name of the expected profile + pg: The key of buffer pg in BUFFER_PG table. Format: BUFFER_PG|| + expected_profile: The name of the expected profile """ def _check_pg_profile(duthost, pg, expected_profile): profile = duthost.shell('redis-cli hget {} profile'.format(pg))['stdout'][1:-1] @@ -216,8 +216,8 @@ def check_pfc_enable(duthost, port, expected_pfc_enable_map): """Check whether the pfc_enable map in port table is correct in a wait_until loop with maximum timeout as 10 seconds Args: - port: the port to be checked - expected_pfc_enable_map: the expected pfc_enable map + port: The port to be checked + expected_pfc_enable_map: The expected pfc_enable map """ def _check_pfc_enable(duthost, port, expected_pfc_enable_map): pfc_enable = duthost.shell('redis-cli -n 4 hget "PORT_QOS_MAP|{}" pfc_enable'.format(port))['stdout'] @@ -234,9 +234,9 @@ def check_lossless_profile_removed(duthost, profile, sai_oid = None): """Check whether the lossless profile has been removed from APPL_DB, STATE_DB and ASIC_DB (if sai_oid provided) Args: - profile: the name of the buffer profile to be checked - sai_oid: the SAI OID in ASIC_DB of the buffer profile - if it is None the ASIC_DB won't be checked + profile: The name of the buffer profile to be checked + sai_oid: The SAI OID in ASIC_DB of the buffer profile + If it is None the ASIC_DB won't be checked """ profile_info = duthost.shell('redis-cli -n 6 hgetall "BUFFER_PROFILE_TABLE|{}"'.format(profile))['stdout'] pytest_assert(not profile_info, "Profile {} isn't removed from STATE_DB".format(profile)) @@ -257,7 +257,7 @@ def _compose_dict_from_cli(fields_list): """Convert the out put of hgetall command to a dict object containing the field, key pairs of the database table content Args: - fields_list: a list of lines, the output of redis-cli hgetall command + fields_list: A list of lines, the output of redis-cli hgetall command """ return dict(zip(fields_list[0::2], fields_list[1::2])) @@ -266,29 +266,29 @@ def check_buffer_profile_details(duthost, initial_profiles, profile_name, profil """Check buffer profile details. The following items are tested: - - whether the headroom information, like xoff, is correct. - this is tested by comparing with standard profile in pg_profile_lookup table - - whether the profile information in APPL_DB matches that in ASIC_DB + - Whether the headroom information, like xoff, is correct. + This is tested by comparing with standard profile in pg_profile_lookup table + - Whether the profile information in APPL_DB matches that in ASIC_DB Args: - initial_profiles: the keys of buffer profiles in ASIC_DB at the beginning of the test - profile_name: name of the profile + initial_profiles: The keys of buffer profiles in ASIC_DB at the beginning of the test + profile_name: Name of the profile profile_oid: SAI OID of the profile - pool_oid: SIA OID of ingress lossless pool + pool_oid: SAI OID of ingress lossless pool """ profile_appldb = _compose_dict_from_cli(duthost.shell('redis-cli hgetall BUFFER_PROFILE_TABLE:{}'.format(profile_name))['stdout'].split('\n')) logging.debug("APPL_DB buffer profile {}: {} ".format(profile_name, profile_appldb)) - # check the profile against the standard value + # Check the profile against the standard value m = re.search(lossless_profile_pattern, profile_name) if m: - # this means it's a dynamic profile + # This means it's a dynamic profile speed = m.group(1) cable_length = m.group(2) std_profiles_for_speed = default_lossless_headroom_data.get(speed) std_profile = std_profiles_for_speed.get(cable_length) if std_profile: - # this means it's a profile with std speed and cable length. we can check whether the headroom data is correct + # This means it's a profile with std speed and cable length. We can check whether the headroom data is correct pytest_assert(profile_appldb['xon'] == std_profile['xon'] and profile_appldb['xoff'] == std_profile['xoff'] and profile_appldb['size'] == std_profile['size'], "Generated profile {} doesn't match the std profile {}".format(profile_appldb, std_profile)) else: @@ -333,10 +333,10 @@ def check_buffer_profile_details(duthost, initial_profiles, profile_name, profil @pytest.fixture(params=['50000', '10000']) def speed_to_test(request): - """used to parametrized test cases for speeds + """Used to parametrized test cases for speeds Args: - param request: pytest request object + param request: The pytest request object Return: speed_to_test @@ -346,10 +346,10 @@ def speed_to_test(request): @pytest.fixture(params=['15m', '40m']) def cable_len_to_test(request): - """used to parametrized test cases for cable length + """Used to parametrized test cases for cable length Args: - request: pytest request object + request: The pytest request object Return: cable_len_to_test @@ -359,10 +359,10 @@ def cable_len_to_test(request): @pytest.fixture(params=['1500', '9100']) def mtu_to_test(request): - """used to parametrized test cases for mtu + """Used to parametrized test cases for mtu Args: - request: pytest request object + request: The pytest request object Return: cable_len_to_test @@ -372,10 +372,10 @@ def mtu_to_test(request): @pytest.fixture() def port_to_test(request, duthost): - """used to parametrized test cases for port + """Used to parametrized test cases for port Args: - request: pytest request object + request: The pytest request object Return: port_to_test @@ -395,10 +395,10 @@ def port_to_test(request, duthost): @pytest.fixture(params=['3-4', '6']) def pg_to_test(request): - """used to parametrized test cases for PGs under test + """Used to parametrized test cases for PGs under test Args: - request: pytest request object + request: The pytest request object Return: pg_to_test @@ -417,8 +417,7 @@ def test_change_speed_cable(duthosts, rand_one_dut_hostname, conn_graph_facts, p 1. Update the port configuration according to input parameters 2. Determine whether the profile removing behavior can be verifyed: If neither mtu nor cable length is default value, they will be applied on the port_to_test only, - and the generated profile will be removed after the configuration change because the profile is referenced - by this port only. + and the generated profile will be removed after the configuration change because the profile is referenced by this port only. For example: The mtu_to_test 1500 only applied on the port_to_test, thus the *_mtu1500_* profile is referenced by the port only The *_mtu1500_* mtu will be removed after the mtu of the port is updated to default value. @@ -426,18 +425,18 @@ def test_change_speed_cable(duthosts, rand_one_dut_hostname, conn_graph_facts, p Other the other hand, if the mtu is 9100, the buffer profile can be referenced by many other ports and it's less possible for us to verify the removing behavior. We will remove and readd an extra PG 6 to verify the removing behavior as well. 3. Each time the port configuration updated, the following items will be checked as much as possible: - - whether the new profile is generated in APPL_DB, STATE_DB and ASIC_DB - - whether the pool size is updated in APPL_DB and ASIC_DB + - Whether the new profile is generated in APPL_DB, STATE_DB and ASIC_DB. + - Whether the pool size is updated in APPL_DB and ASIC_DB. 4. Each time the PG on a port is added or removed, the following items will be checked: - - whether the profile referenced by PGs is as expected according to the port configuration - - whether the profile is removed if all PGs are removed and we are able to check removing behavior (result of step 2) - - whether the pfc_enable filed of the port has been updated accordingly + - Whether the profile referenced by PGs is as expected according to the port configuration. + - Whether the profile is removed if all PGs are removed and we are able to check removing behavior (result of step 2). + - Whether the pfc_enable filed of the port has been updated accordingly. Args: - port_to_test: on which port will the test be performed - speed_to_test: to what speed will the port's be changed - mtu_to_test: to what mtu will the port's be changed - cable_len_to_test: to what cable length will the port's be changed + port_to_test: On which port will the test be performed + speed_to_test: To what speed will the port's be changed + mtu_to_test: To what mtu will the port's be changed + cable_len_to_test: To what cable length will the port's be changed """ duthost = duthosts[rand_one_dut_hostname] original_speed = duthost.shell('redis-cli -n 4 hget "PORT|{}" speed'.format(port_to_test))['stdout'] @@ -466,7 +465,7 @@ def test_change_speed_cable(duthosts, rand_one_dut_hostname, conn_graph_facts, p check_profile_removed = cable_len_to_test not in default_cable_length_list - # check whether profile is correct in PG table + # Check whether profile is correct in PG table if mtu_to_test != default_mtu: expected_profile = 'pg_lossless_{}_{}_mtu{}_profile'.format(speed_to_test, cable_len_to_test, mtu_to_test) check_profile_removed = True @@ -479,7 +478,7 @@ def test_change_speed_cable(duthosts, rand_one_dut_hostname, conn_graph_facts, p profile_oid, pool_oid = check_buffer_profile_details(duthost, initial_asic_db_profiles, expected_profile, None, None) logging.info('SAI OID for newly created profile {} ingress lossless pool {}'.format(profile_oid, pool_oid)) - # check whether profile exist + # Check whether profile exist headroom_size = int(duthost.shell('redis-cli hget "BUFFER_PROFILE_TABLE:{}" size'.format(expected_profile))['stdout']) check_pool_size(duthost, pool_oid, @@ -525,7 +524,7 @@ def test_change_speed_cable(duthosts, rand_one_dut_hostname, conn_graph_facts, p logging.info('[Revert the mtu to the default value] Checking whether the profile is updated') duthost.shell('config interface mtu {} {}'.format(port_to_test, default_mtu)) - # remove old profile on cable length change + # Remove old profile on cable length change logging.info('[Remove dynamic profile on cable length and/or MTU updated] Checking whether the old profile is removed') check_lossless_profile_removed(duthost, expected_profile, profile_oid) expected_profile = 'pg_lossless_{}_{}_profile'.format(speed_to_test, original_cable_len) @@ -604,13 +603,13 @@ def _parse_buffer_profile_params(param, cmd, name): """A helper for test_headroom_override, parsing the parameters from the pre-provided json file Args: - param: the dict containing test parameters parsed from dynamic_buffer_param.json - return: tuple: new headroom size and cli string + param: The dict containing test parameters parsed from dynamic_buffer_param.json + return: A tuple consisting of new headroom size and cli string Return: - a tuple consists of: - the CLI string by which a headroom-override profile can be configured - the size of new profile + A tuple consists of: + - The CLI string by which a headroom-override profile can be configured + - The size of new profile """ cli_str = "config buffer profile {} {}".format(cmd, name) xon = "" @@ -649,10 +648,10 @@ def test_headroom_override(duthosts, rand_one_dut_hostname, conn_graph_facts, po 1. Fetch the parameters 2. Add the headroom override profile and apply it to PG 3-4 on port_to_test 3. Verify: - - whether the profile referenced by PG is correct - - whether the pfc_enable matches the PG - - whether the buffer profile is correct deployed in APPL_DB, STATE_DB and ASIC_DB - - whether the pool size has been updated correctly + - Whether the profile referenced by PG is correct + - Whether the pfc_enable matches the PG + - Whether the buffer profile is correct deployed in APPL_DB, STATE_DB and ASIC_DB + - Whether the pool size has been updated correctly 4. Add PG 6, verify the related info 5. Update the headroom override profile and verify the related info 6. Negative test: try to remove the headroom override profile. @@ -660,11 +659,11 @@ def test_headroom_override(duthosts, rand_one_dut_hostname, conn_graph_facts, po 7. Revert the PG configurations, verify the related info Args: - port_to_test: on which port will the test be performed + port_to_test: On which port will the test be performed """ duthost = duthosts[rand_one_dut_hostname] if not testparam_headroom_override: - pytest.skip("headroom override test skipped due to no parameters provided") + pytest.skip("Headroom override test skipped due to no parameters provided") original_speed = duthost.shell('redis-cli -n 4 hget "PORT|{}" speed'.format(port_to_test))['stdout'] original_cable_len = duthost.shell('redis-cli -n 4 hget "CABLE_LENGTH|AZURE" {}'.format(port_to_test))['stdout'] @@ -678,14 +677,14 @@ def test_headroom_override(duthosts, rand_one_dut_hostname, conn_graph_facts, po # Configure a static profile param = testparam_headroom_override.get("add") if not param: - pytest.skip('headroom override test skipped due to no parameters for "add" command provided') + pytest.skip('Headroom override test skipped due to no parameters for "add" command provided') else: cli_str, new_headroom = _parse_buffer_profile_params(param, "add", "headroom-override") logging.info("[Prepare configuration] {}".format(cli_str)) duthost.shell(cli_str) - logging.info("[Test: headroom override on lossless PG 3-4] apply the profile on the PG and check pool size") + logging.info("[Test: headroom override on lossless PG 3-4] Apply the profile on the PG and check pool size") duthost.shell('config interface buffer priority-group lossless set {} 3-4 headroom-override'.format(port_to_test)) check_pg_profile(duthost, 'BUFFER_PG_TABLE:{}:3-4'.format(port_to_test), 'headroom-override') @@ -699,7 +698,7 @@ def test_headroom_override(duthosts, rand_one_dut_hostname, conn_graph_facts, po new_headroom = new_headroom) # Add another headroom override - logging.info("[Test: headroom override on more lossless PGs 6] apply the profile on the PG and check pool size") + logging.info("[Test: headroom override on more lossless PGs 6] Apply the profile on the PG and check pool size") duthost.shell('config interface buffer priority-group lossless add {} 6 headroom-override'.format(port_to_test)) check_pg_profile(duthost, 'BUFFER_PG_TABLE:{}:6'.format(port_to_test), 'headroom-override') @@ -715,10 +714,10 @@ def test_headroom_override(duthosts, rand_one_dut_hostname, conn_graph_facts, po param = testparam_headroom_override.get("set") if not param: - pytest.skip('headroom override test skipped due to no parameters for "set" command provided') + pytest.skip('Headroom override test skipped due to no parameters for "set" command provided') else: cli_str, new_headroom = _parse_buffer_profile_params(param, "set", "headroom-override") - logging.info("[Test: update headroom-override profile] update the profile and check pool size: {}".format(cli_str)) + logging.info("[Test: update headroom-override profile] Update the profile and check pool size: {}".format(cli_str)) duthost.shell(cli_str) check_pool_size(duthost, @@ -771,8 +770,8 @@ def test_lossless_pg(duthosts, rand_one_dut_hostname, conn_graph_facts, port_to_ 8. Recover the configuration Args: - port_to_test: on which port will the test be performed - pg_to_test: to what PG will the profiles be applied + port_to_test: On which port will the test be performed + pg_to_test: To what PG will the profiles be applied """ duthost = duthosts[rand_one_dut_hostname] original_speed = duthost.shell('redis-cli -n 4 hget "PORT|{}" speed'.format(port_to_test))['stdout'] @@ -796,75 +795,75 @@ def test_lossless_pg(duthosts, rand_one_dut_hostname, conn_graph_facts, port_to_ else: cli_str, new_headroom = _parse_buffer_profile_params(param, "add", "headroom-override") - # create profiles - logging.info('[Preparing]: create static buffer profile for headroom override') + # Create profiles + logging.info('[Preparing]: Create static buffer profile for headroom override') duthost.shell(cli_str) headroom_override_profile_oid, pool_oid = check_buffer_profile_details(duthost, initial_asic_db_profiles, "headroom-override", None, None) initial_asic_db_profiles = fetch_initial_asic_db(duthost) - # this is a dynamic profile with non default dynamic-th. - # profile won't be created until configured on some pg + # This is a dynamic profile with non default dynamic-th. + # Profile won't be created until configured on some pg param = testparam_lossless_pg.get("non-default-dynamic_th") if not param: - pytest.skip('lossless pg test skipped due to no parameters for "non-default-dynamic_th" command provided') + pytest.skip('Lossless pg test skipped due to no parameters for "non-default-dynamic_th" command provided') else: cli_str, new_headroom = _parse_buffer_profile_params(param, "add", "non-default-dynamic_th") - logging.info('[Preparing]: create static buffer profile for non default dynamic_th') + logging.info('[Preparing]: Create static buffer profile for non default dynamic_th') duthost.shell(cli_str) - # update cable length to 15m - logging.info('[Preparing]: update cable length') + # Update cable length to 15m + logging.info('[Preparing]: Update cable length') duthost.shell('config interface cable-length {} 15m'.format(port_to_test)) expected_profile = 'pg_lossless_{}_15m_profile'.format(original_speed) check_pg_profile(duthost, 'BUFFER_PG_TABLE:{}:3-4'.format(port_to_test), expected_profile) profile_oid, _ = check_buffer_profile_details(duthost, initial_asic_db_profiles, expected_profile, None, pool_oid) - # original it should be a dynamic PG, update it to override + # Originally, it should be a dynamic PG, update it to override logging.info('[Testcase: dynamic headroom => headroom override]') duthost.shell(first_command + 'headroom-override') - # check whether lossless dynamic profile is removed + # Check whether lossless dynamic profile is removed check_pg_profile(duthost, buffer_pg, 'headroom-override') if pg_to_test == '3-4': check_lossless_profile_removed(duthost, expected_profile, profile_oid) - # update it to non-default dynamic_th + # Update it to non-default dynamic_th logging.info('[Testcase: headroom override => dynamically calculated headroom with non-default dynamic_th]') duthost.shell(set_command + 'non-default-dynamic_th') expected_nondef_profile = 'pg_lossless_{}_15m_th2_profile'.format(original_speed) check_pg_profile(duthost, buffer_pg, expected_nondef_profile) - # a new profile should be created in ASIC DB + # A new profile should be created in ASIC DB profile_oid, _ = check_buffer_profile_details(duthost, initial_asic_db_profiles, expected_nondef_profile, None, pool_oid) - # update it to dynamic PG + # Update it to dynamic PG logging.info('[Testcase: dynamically calculated headroom with non-default dynamic_th => dynamic headroom]') duthost.shell(set_command) check_pg_profile(duthost, buffer_pg, expected_profile) check_lossless_profile_removed(duthost, expected_nondef_profile, profile_oid) - # update it to non-default dynamic_th + # Update it to non-default dynamic_th logging.info('[Testcase: dynamic headroom => dynamically calculated headroom with non-default dynamic_th]') duthost.shell(set_command + 'non-default-dynamic_th') check_pg_profile(duthost, buffer_pg, expected_nondef_profile) - # a new profile should be created in ASIC DB + # A new profile should be created in ASIC DB profile_oid, _ = check_buffer_profile_details(duthost, initial_asic_db_profiles, expected_nondef_profile, None, pool_oid) if pg_to_test == '3-4': - # the oid can be reused by SAI. so don't check whether profile_oid is removed. + # The oid can be reused by SAI. So we don't check whether profile_oid is removed. check_lossless_profile_removed(duthost, expected_profile) - # update it to headroom override + # Update it to headroom override logging.info('[Testcase: dynamically calculated headroom with non-default dynamic_th => headroom override]') duthost.shell(set_command + 'headroom-override') check_pg_profile(duthost, buffer_pg, 'headroom-override') check_lossless_profile_removed(duthost, expected_nondef_profile, profile_oid) - # update it to dynamic PG, recover + # Update it to dynamic PG, recover logging.info('[Testcase: headroom override => dynamic headroom]') duthost.shell(set_command) check_pg_profile(duthost, buffer_pg, expected_profile) - # remove all static profiles + # Remove all static profiles logging.info('[Restoring configuration]') duthost.shell('config buffer profile remove headroom-override') duthost.shell('config buffer profile remove non-default-dynamic_th') @@ -902,7 +901,7 @@ def test_exceeding_headroom(duthosts, rand_one_dut_hostname, conn_graph_facts, p original_profile = 'pg_lossless_{}_{}_profile'.format(original_speed, original_cable_len) try: - # set to super long cable length + # Set to super long cable length logging.info('[Config a super long cable length]') duthost.shell('config interface cable-length {} 10000m'.format(port_to_test)) @@ -920,12 +919,12 @@ def test_exceeding_headroom(duthosts, rand_one_dut_hostname, conn_graph_facts, p logging.info('Add another PG and make sure the system isn\'t broken') duthost.shell('config interface buffer priority-group lossless add {} {}'.format(port_to_test, '5-7')) - # we can't say whether this will accumulative headroom exceed the limit, but the system should not crash - # leverage sanity check to verify that + # We can't say whether this will accumulative headroom exceed the limit, but the system should not crash + # Leverage sanity check to verify that duthost.shell('config interface buffer priority-group lossless remove {} {}'.format(port_to_test, '5-7')) duthost.shell('config interface cable-length {} {}'.format(port_to_test, original_cable_len)) - # static profile + # Static profile logging.info('[Config headroom override to PG 3-4]') duthost.shell('config buffer profile add test-headroom --xon 18432 --xoff 50000 -headroom 68432') duthost.shell('config interface buffer priority-group lossless set {} {} {}'.format(port_to_test, '3-4', 'test-headroom')) @@ -934,14 +933,14 @@ def test_exceeding_headroom(duthosts, rand_one_dut_hostname, conn_graph_facts, p check_pg_profile(duthost, 'BUFFER_PG_TABLE:{}:3-4'.format(port_to_test), 'test-headroom') duthost.shell('config interface buffer priority-group lossless add {} {} {}'.format(port_to_test, '5-7', 'test-headroom')) - # again, we can't say for sure whether the accumulative headroom exceeding. - # just make sure the system doesn't crash + # Again, we can't say for sure whether the accumulative headroom exceeding. + # Just make sure the system doesn't crash duthost.shell('config interface buffer priority-group lossless remove {} {}'.format(port_to_test, '5-7')) logging.info('[Update headroom override to a larger size]') duthost.shell('config buffer profile set test-headroom --xon 18432 --xoff 860160 -headroom 878592') - # this should make it exceed the limit, so the profile should not applied to the APPL_DB + # This should make it exceed the limit, so the profile should not applied to the APPL_DB size_in_appldb = duthost.shell('redis-cli hget "BUFFER_PROFILE_TABLE:test-headroom" size')['stdout'] pytest_assert(size_in_appldb == '68432', 'The profile with a large size was applied to APPL_DB, which can make headroom exceeding') duthost.shell('config interface buffer priority-group lossless set {} {}'.format(port_to_test, '3-4')) @@ -962,10 +961,10 @@ def _recovery_to_dynamic_buffer_model(duthost): def test_buffer_model_test(duthosts, rand_one_dut_hostname, conn_graph_facts): - """verify whether the buffer model is expected after configuration operations: + """Verify whether the buffer model is expected after configuration operations: The following items are verified - - whether the buffer model is traditional after executing config load_minigraph - - whether the buffer model is dynamic after recovering the buffer model to dynamic + - Whether the buffer model is traditional after executing config load_minigraph + - Whether the buffer model is dynamic after recovering the buffer model to dynamic """ duthost = duthosts[rand_one_dut_hostname] try: From 25b7e18b46462169a3dec03547c1883deaee112b Mon Sep 17 00:00:00 2001 From: Stephen Sun Date: Wed, 6 Jan 2021 00:12:54 +0000 Subject: [PATCH 7/9] Don't make isBufferInApplDb a fixture. It's more convenent to leave it as a class member for users Signed-off-by: Stephen Sun --- tests/qos/qos_sai_base.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/qos/qos_sai_base.py b/tests/qos/qos_sai_base.py index cba4d2d2ff0..5567a863398 100644 --- a/tests/qos/qos_sai_base.py +++ b/tests/qos/qos_sai_base.py @@ -24,7 +24,6 @@ class QosSaiBase: buffer_model_initialized = False buffer_model = None - @pytest.fixture(scope='class', autouse=True) def isBufferInApplDb(self, duthost): if not self.buffer_model_initialized: self.buffer_model = duthost.shell('redis-cli -n 4 hget "DEVICE_METADATA|localhost" buffer_model')["stdout"] From 6a0600cb256b96c6b1dd91d30a32a86ad3bfd10a Mon Sep 17 00:00:00 2001 From: Stephen Sun Date: Wed, 6 Jan 2021 09:06:48 +0000 Subject: [PATCH 8/9] Fix review comments - Use capital letters for global variable names - Remove spaces between "=" in argument list Signed-off-by: Stephen Sun --- tests/qos/test_buffer.py | 108 +++++++++++++++++++-------------------- 1 file changed, 54 insertions(+), 54 deletions(-) diff --git a/tests/qos/test_buffer.py b/tests/qos/test_buffer.py index a7ab90387e7..e106dfffc46 100644 --- a/tests/qos/test_buffer.py +++ b/tests/qos/test_buffer.py @@ -12,17 +12,17 @@ from tests.common.helpers.assertions import pytest_assert profile_format = 'pg_lossless_{}_{}_profile' -lossless_profile_pattern = 'pg_lossless_([1-9][0-9]*000)_([1-9][0-9]*m)_profile' +LOSSLESS_PROFILE_PATTERN = 'pg_lossless_([1-9][0-9]*000)_([1-9][0-9]*m)_profile' -default_cable_length_list = None -default_lossless_headroom_data = None -default_ingress_pool_number = 0 -default_mtu = None +DEFAULT_CABLE_LENGTH_LIST = None +DEFAULT_LOSSLESS_HEADROOM_DATA = None +DEFAULT_INGRESS_POOL_NUMBER = 0 +DEFAULT_MTU = None -testparam_headroom_override = None -testparam_lossless_pg = None +TESTPARAM_HEADROOM_OVERRIDE = None +TESTPARAM_LOSSLESS_PG = None -buffer_model_dynamic = True +BUFFER_MODEL_DYNAMIC = True def detect_buffer_model(duthost): """Detect the current buffer model (dynamic or traditional) and store it for futher use. Called only once when the module is initialized @@ -30,9 +30,9 @@ def detect_buffer_model(duthost): Args: duthost: The DUT host object """ - global buffer_model_dynamic + global BUFFER_MODEL_DYNAMIC buffer_model = duthost.shell('redis-cli -n 4 hget "DEVICE_METADATA|localhost" buffer_model')['stdout'] - buffer_model_dynamic = (buffer_model == 'dynamic') + BUFFER_MODEL_DYNAMIC = (buffer_model == 'dynamic') def detect_ingress_pool_number(duthost): @@ -41,21 +41,21 @@ def detect_ingress_pool_number(duthost): Args: duthost: The DUT host object """ - global default_ingress_pool_number + global DEFAULT_INGRESS_POOL_NUMBER pools = duthost.shell('redis-cli -n 4 keys "BUFFER_POOL|ingress*"')['stdout'] - default_ingress_pool_number = len(pools.split()) + DEFAULT_INGRESS_POOL_NUMBER = len(pools.split()) -def detect_default_mtu(duthost, port_to_test): +def detect_DEFAULT_MTU(duthost, port_to_test): """Detect the mtu and store it for futher use. Called only once when the module is initialized Args: duthost: The DUT host object """ - global default_mtu - if not default_mtu: - logging.info("Default MTU {}".format(default_mtu)) - default_mtu = duthost.shell('redis-cli -n 4 hget "PORT|{}" mtu'.format(port_to_test))['stdout'] + global DEFAULT_MTU + if not DEFAULT_MTU: + logging.info("Default MTU {}".format(DEFAULT_MTU)) + DEFAULT_MTU = duthost.shell('redis-cli -n 4 hget "PORT|{}" mtu'.format(port_to_test))['stdout'] def load_lossless_headroom_data(duthost): @@ -64,13 +64,13 @@ def load_lossless_headroom_data(duthost): Args: duthost: the DUT host object """ - global default_lossless_headroom_data - if not default_lossless_headroom_data: + global DEFAULT_LOSSLESS_HEADROOM_DATA + if not DEFAULT_LOSSLESS_HEADROOM_DATA: dut_hwsku = duthost.facts["hwsku"] dut_platform = duthost.facts["platform"] skudir = "/usr/share/sonic/device/{}/{}/".format(dut_platform, dut_hwsku) lines = duthost.shell('cat {}/pg_profile_lookup.ini'.format(skudir))["stdout"] - default_lossless_headroom_data = {} + DEFAULT_LOSSLESS_HEADROOM_DATA = {} for line in lines.split('\n'): if line[0] == '#': continue @@ -80,10 +80,10 @@ def load_lossless_headroom_data(duthost): size = tokens[2] xon = tokens[3] xoff = tokens[4] - if not default_lossless_headroom_data.get(speed): - default_lossless_headroom_data[speed] = {} - default_lossless_headroom_data[speed][cable_length] = {'size': size, 'xon': xon, 'xoff': xoff} - default_lossless_headroom_data = default_lossless_headroom_data + if not DEFAULT_LOSSLESS_HEADROOM_DATA.get(speed): + DEFAULT_LOSSLESS_HEADROOM_DATA[speed] = {} + DEFAULT_LOSSLESS_HEADROOM_DATA[speed][cable_length] = {'size': size, 'xon': xon, 'xoff': xoff} + DEFAULT_LOSSLESS_HEADROOM_DATA = DEFAULT_LOSSLESS_HEADROOM_DATA def load_test_parameters(duthost): @@ -92,9 +92,9 @@ def load_test_parameters(duthost): Args: duthost: The DUT host object """ - global default_cable_length_list - global testparam_headroom_override - global testparam_lossless_pg + global DEFAULT_CABLE_LENGTH_LIST + global TESTPARAM_HEADROOM_OVERRIDE + global TESTPARAM_LOSSLESS_PG param_file_name = "qos/files/dynamic_buffer_param.json" with open(param_file_name) as file: @@ -102,9 +102,9 @@ def load_test_parameters(duthost): logging.info("Loaded test parameters {} from {}".format(params, param_file_name)) asic_type = duthost.facts['asic_type'] vendor_specific_param = params[asic_type] - default_cable_length_list = vendor_specific_param['default_cable_length'] - testparam_headroom_override = vendor_specific_param['headroom-override'] - testparam_lossless_pg = vendor_specific_param['lossless_pg'] + DEFAULT_CABLE_LENGTH_LIST = vendor_specific_param['default_cable_length'] + TESTPARAM_HEADROOM_OVERRIDE = vendor_specific_param['headroom-override'] + TESTPARAM_LOSSLESS_PG = vendor_specific_param['lossless_pg'] @pytest.fixture(scope="module", autouse=True) @@ -115,14 +115,14 @@ def setup_module(duthost): duthost: The DUT host object """ detect_buffer_model(duthost) - if buffer_model_dynamic: + if BUFFER_MODEL_DYNAMIC: detect_ingress_pool_number(duthost) load_lossless_headroom_data(duthost) load_test_parameters(duthost) - logging.info("Cable length: default {}".format(default_cable_length_list)) - logging.info("Ingress pool number {}".format(default_ingress_pool_number)) - logging.info("Lossless headroom data {}".format(default_lossless_headroom_data)) + logging.info("Cable length: default {}".format(DEFAULT_CABLE_LENGTH_LIST)) + logging.info("Ingress pool number {}".format(DEFAULT_INGRESS_POOL_NUMBER)) + logging.info("Lossless headroom data {}".format(DEFAULT_LOSSLESS_HEADROOM_DATA)) else: pytest.skip("Dynamic buffer isn't enabled, skip the test") @@ -171,8 +171,8 @@ def check_pool_size(duthost, ingress_lossless_pool_oid, **kwargs): curr_pool_size = int(kwargs["pool_size"]) - original_memory = curr_pool_size * default_ingress_pool_number + old_headroom * old_pg_number - expected_pool_size = (original_memory - new_reserved) / default_ingress_pool_number + original_memory = curr_pool_size * DEFAULT_INGRESS_POOL_NUMBER + old_headroom * old_pg_number + expected_pool_size = (original_memory - new_reserved) / DEFAULT_INGRESS_POOL_NUMBER def _get_pool_size_from_asic_db(duthost, ingress_lossless_pool_oid): pool_sai = _compose_dict_from_cli(duthost.shell('redis-cli -n 1 hgetall ASIC_STATE:SAI_OBJECT_TYPE_BUFFER_POOL:{}'.format(ingress_lossless_pool_oid))['stdout'].split('\n')) @@ -230,7 +230,7 @@ def _check_pfc_enable(duthost, port, expected_pfc_enable_map): duthost.shell('redis-cli -n 4 hget "PORT_QOS_MAP|{}" pfc_enable'.format(port))['stdout'])) -def check_lossless_profile_removed(duthost, profile, sai_oid = None): +def check_lossless_profile_removed(duthost, profile, sai_oid=None): """Check whether the lossless profile has been removed from APPL_DB, STATE_DB and ASIC_DB (if sai_oid provided) Args: @@ -280,12 +280,12 @@ def check_buffer_profile_details(duthost, initial_profiles, profile_name, profil logging.debug("APPL_DB buffer profile {}: {} ".format(profile_name, profile_appldb)) # Check the profile against the standard value - m = re.search(lossless_profile_pattern, profile_name) + m = re.search(LOSSLESS_PROFILE_PATTERN, profile_name) if m: # This means it's a dynamic profile speed = m.group(1) cable_length = m.group(2) - std_profiles_for_speed = default_lossless_headroom_data.get(speed) + std_profiles_for_speed = DEFAULT_LOSSLESS_HEADROOM_DATA.get(speed) std_profile = std_profiles_for_speed.get(cable_length) if std_profile: # This means it's a profile with std speed and cable length. We can check whether the headroom data is correct @@ -442,31 +442,31 @@ def test_change_speed_cable(duthosts, rand_one_dut_hostname, conn_graph_facts, p original_speed = duthost.shell('redis-cli -n 4 hget "PORT|{}" speed'.format(port_to_test))['stdout'] original_cable_len = duthost.shell('redis-cli -n 4 hget "CABLE_LENGTH|AZURE" {}'.format(port_to_test))['stdout'] profile = duthost.shell('redis-cli hget "BUFFER_PG_TABLE:{}:3-4" profile'.format(port_to_test))['stdout'][1:-1] - detect_default_mtu(duthost, port_to_test) + detect_DEFAULT_MTU(duthost, port_to_test) original_headroom_size = int(duthost.shell('redis-cli hget "{}" size'.format(profile))['stdout']) original_pool_size = int(duthost.shell('redis-cli hget BUFFER_POOL_TABLE:ingress_lossless_pool size')['stdout']) initial_asic_db_profiles = fetch_initial_asic_db(duthost) - if speed_to_test == original_speed and cable_len_to_test == original_cable_len and mtu_to_test == default_mtu: + if speed_to_test == original_speed and cable_len_to_test == original_cable_len and mtu_to_test == DEFAULT_MTU: pytest.skip('Speed, MTU and cable length matches the default value, nothing to test, skip') try: if not speed_to_test == original_speed: logging.info("Changing port's speed to {}".format(speed_to_test)) duthost.shell('config interface speed {} {}'.format(port_to_test, speed_to_test)) - if not mtu_to_test == default_mtu: + if not mtu_to_test == DEFAULT_MTU: logging.info("Changing port's mtu to {}".format(mtu_to_test)) duthost.shell('config interface mtu {} {}'.format(port_to_test, mtu_to_test)) if not cable_len_to_test == original_cable_len: logging.info("Changing port's cable length to {}".format(cable_len_to_test)) duthost.shell('config interface cable-length {} {}'.format(port_to_test, cable_len_to_test)) - check_profile_removed = cable_len_to_test not in default_cable_length_list + check_profile_removed = cable_len_to_test not in DEFAULT_CABLE_LENGTH_LIST # Check whether profile is correct in PG table - if mtu_to_test != default_mtu: + if mtu_to_test != DEFAULT_MTU: expected_profile = 'pg_lossless_{}_{}_mtu{}_profile'.format(speed_to_test, cable_len_to_test, mtu_to_test) check_profile_removed = True else: @@ -520,9 +520,9 @@ def test_change_speed_cable(duthosts, rand_one_dut_hostname, conn_graph_facts, p logging.info('[Revert the cable length to the default value] Checking whether the profile is updated') duthost.shell('config interface cable-length {} {}'.format(port_to_test, original_cable_len)) - if mtu_to_test != default_mtu: + if mtu_to_test != DEFAULT_MTU: logging.info('[Revert the mtu to the default value] Checking whether the profile is updated') - duthost.shell('config interface mtu {} {}'.format(port_to_test, default_mtu)) + duthost.shell('config interface mtu {} {}'.format(port_to_test, DEFAULT_MTU)) # Remove old profile on cable length change logging.info('[Remove dynamic profile on cable length and/or MTU updated] Checking whether the old profile is removed') @@ -550,9 +550,9 @@ def test_change_speed_cable(duthosts, rand_one_dut_hostname, conn_graph_facts, p if cable_len_to_test != original_cable_len: logging.info('[Update cable length without any lossless pg configured]') duthost.shell('config interface cable-length {} {}'.format(port_to_test, original_cable_len)) - if mtu_to_test != default_mtu: + if mtu_to_test != DEFAULT_MTU: logging.info('[Update mtu without any lossless pg configured]') - duthost.shell('config interface mtu {} {}'.format(port_to_test, default_mtu)) + duthost.shell('config interface mtu {} {}'.format(port_to_test, DEFAULT_MTU)) if speed_to_test != original_speed: logging.info('[Update speed without any lossless pg configured]') @@ -594,7 +594,7 @@ def test_change_speed_cable(duthosts, rand_one_dut_hostname, conn_graph_facts, p finally: duthost.shell('config interface buffer priority-group lossless remove {}'.format(port_to_test), module_ignore_errors = True) duthost.shell('config interface speed {} {}'.format(port_to_test, original_speed), module_ignore_errors = True) - duthost.shell('config interface mtu {} {}'.format(port_to_test, default_mtu), module_ignore_errors = True) + duthost.shell('config interface mtu {} {}'.format(port_to_test, DEFAULT_MTU), module_ignore_errors = True) duthost.shell('config interface cable-length {} {}'.format(port_to_test, original_cable_len), module_ignore_errors = True) duthost.shell('config interface buffer priority-group lossless add {} 3-4'.format(port_to_test), module_ignore_errors = True) @@ -662,7 +662,7 @@ def test_headroom_override(duthosts, rand_one_dut_hostname, conn_graph_facts, po port_to_test: On which port will the test be performed """ duthost = duthosts[rand_one_dut_hostname] - if not testparam_headroom_override: + if not TESTPARAM_HEADROOM_OVERRIDE: pytest.skip("Headroom override test skipped due to no parameters provided") original_speed = duthost.shell('redis-cli -n 4 hget "PORT|{}" speed'.format(port_to_test))['stdout'] @@ -675,7 +675,7 @@ def test_headroom_override(duthosts, rand_one_dut_hostname, conn_graph_facts, po try: # Configure a static profile - param = testparam_headroom_override.get("add") + param = TESTPARAM_HEADROOM_OVERRIDE.get("add") if not param: pytest.skip('Headroom override test skipped due to no parameters for "add" command provided') else: @@ -712,7 +712,7 @@ def test_headroom_override(duthosts, rand_one_dut_hostname, conn_graph_facts, po new_headroom = new_headroom, new_pg_number = 3) - param = testparam_headroom_override.get("set") + param = TESTPARAM_HEADROOM_OVERRIDE.get("set") if not param: pytest.skip('Headroom override test skipped due to no parameters for "set" command provided') else: @@ -789,7 +789,7 @@ def test_lossless_pg(duthosts, rand_one_dut_hostname, conn_graph_facts, port_to_ buffer_pg = 'BUFFER_PG_TABLE:{}:{}'.format(port_to_test, pg_to_test) try: - param = testparam_lossless_pg.get("headroom-override") + param = TESTPARAM_LOSSLESS_PG.get("headroom-override") if not param: pytest.skip('Lossless pg test skipped due to no parameters for "headroom-override" command provided') else: @@ -804,7 +804,7 @@ def test_lossless_pg(duthosts, rand_one_dut_hostname, conn_graph_facts, port_to_ # This is a dynamic profile with non default dynamic-th. # Profile won't be created until configured on some pg - param = testparam_lossless_pg.get("non-default-dynamic_th") + param = TESTPARAM_LOSSLESS_PG.get("non-default-dynamic_th") if not param: pytest.skip('Lossless pg test skipped due to no parameters for "non-default-dynamic_th" command provided') else: From bbbe22fe2739199adb854a1f1316e1de7eb139ef Mon Sep 17 00:00:00 2001 From: Stephen Sun Date: Wed, 6 Jan 2021 11:01:55 +0000 Subject: [PATCH 9/9] detect_DEFAULT_MTU => detect_default_mtu Signed-off-by: Stephen Sun --- tests/qos/test_buffer.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/qos/test_buffer.py b/tests/qos/test_buffer.py index e106dfffc46..0b7be8d50ed 100644 --- a/tests/qos/test_buffer.py +++ b/tests/qos/test_buffer.py @@ -46,7 +46,7 @@ def detect_ingress_pool_number(duthost): DEFAULT_INGRESS_POOL_NUMBER = len(pools.split()) -def detect_DEFAULT_MTU(duthost, port_to_test): +def detect_default_mtu(duthost, port_to_test): """Detect the mtu and store it for futher use. Called only once when the module is initialized Args: @@ -442,7 +442,7 @@ def test_change_speed_cable(duthosts, rand_one_dut_hostname, conn_graph_facts, p original_speed = duthost.shell('redis-cli -n 4 hget "PORT|{}" speed'.format(port_to_test))['stdout'] original_cable_len = duthost.shell('redis-cli -n 4 hget "CABLE_LENGTH|AZURE" {}'.format(port_to_test))['stdout'] profile = duthost.shell('redis-cli hget "BUFFER_PG_TABLE:{}:3-4" profile'.format(port_to_test))['stdout'][1:-1] - detect_DEFAULT_MTU(duthost, port_to_test) + detect_default_mtu(duthost, port_to_test) original_headroom_size = int(duthost.shell('redis-cli hget "{}" size'.format(profile))['stdout']) original_pool_size = int(duthost.shell('redis-cli hget BUFFER_POOL_TABLE:ingress_lossless_pool size')['stdout'])