From 9b1e3b8af51b8534d4b5309cc3cb9bb07296a4ef Mon Sep 17 00:00:00 2001 From: Dror Prital Date: Wed, 23 Mar 2022 17:44:30 +0000 Subject: [PATCH 1/6] Apply PR of Dynamic port configuration - add port buffer cfg to the port ref counter #2022 --- orchagent/bufferorch.cpp | 70 ++++++++++++++++++++++++++++++++++- tests/test_port_add_remove.py | 56 ++++++++++++++++++++++++++++ 2 files changed, 125 insertions(+), 1 deletion(-) create mode 100644 tests/test_port_add_remove.py diff --git a/orchagent/bufferorch.cpp b/orchagent/bufferorch.cpp index f9b91e7a16..3e1c0ebb3b 100644 --- a/orchagent/bufferorch.cpp +++ b/orchagent/bufferorch.cpp @@ -42,6 +42,9 @@ map buffer_to_ref_table_map = { {buffer_profile_list_field_name, APP_BUFFER_PROFILE_TABLE_NAME} }; +std::map> pg_port_flags; +std::map> queue_port_flags; + BufferOrch::BufferOrch(DBConnector *applDb, DBConnector *confDb, DBConnector *stateDb, vector &tableNames) : Orch(applDb, tableNames), m_flexCounterDb(new DBConnector("FLEX_COUNTER_DB", 0)), @@ -949,6 +952,38 @@ task_process_status BufferOrch::processQueue(KeyOpFieldsValuesTuple &tuple) } } } + + /* when we apply buffer configuration we need to increase the ref counter of this port + * or decrease the ref counter for this port when we remove buffer cfg + * so for each priority cfg in each port we will increase/decrease the ref counter + * also we need to know when the set command is for creating a buffer cfg or modifying buffer cfg - + * we need to increase ref counter only on create flow. + * so we added a map that will help us to know what was the last command for this port and priority - + * if the last command was set command then it is a modify command and we dont need to increase the buffer counter + * all other cases (no last command exist or del command was the last command) it means that we need to increase the ref counter */ + if (op == SET_COMMAND) + { + if (queue_port_flags[port_name][ind] != SET_COMMAND) + { + /* if the last operation was not "set" then it's create and not modify - need to increase ref counter */ + gPortsOrch->increasePortRefCount(port_name); + } + } + else if (op == DEL_COMMAND) + { + if (queue_port_flags[port_name][ind] == SET_COMMAND) { + /* we need to decrease ref counter only if the last operation was "SET_COMMAND" */ + gPortsOrch->decreasePortRefCount(port_name); + } + } + else + { + SWSS_LOG_ERROR("operation value is not SET or DEL (op = %s)", op.c_str()); + return task_process_status::task_invalid_entry; + } + /* save the last command (set or delete) */ + queue_port_flags[port_name][ind] = op; + } } @@ -1007,7 +1042,7 @@ task_process_status BufferOrch::processPriorityGroup(KeyOpFieldsValuesTuple &tup if (op == SET_COMMAND) { ref_resolve_status resolve_result = resolveFieldRefValue(m_buffer_type_maps, buffer_profile_field_name, - buffer_to_ref_table_map.at(buffer_profile_field_name), tuple, + buffer_to_ref_table_map.at(buffer_profile_field_name), tuple, sai_buffer_profile, buffer_profile_name); if (ref_resolve_status::success != resolve_result) { @@ -1089,6 +1124,39 @@ task_process_status BufferOrch::processPriorityGroup(KeyOpFieldsValuesTuple &tup } } } + + /* when we apply buffer configuration we need to increase the ref counter of this port + * or decrease the ref counter for this port when we remove buffer cfg + * so for each priority cfg in each port we will increase/decrease the ref counter + * also we need to know when the set command is for creating a buffer cfg or modifying buffer cfg - + * we need to increase ref counter only on create flow. + * so we added a map that will help us to know what was the last command for this port and priority - + * if the last command was set command then it is a modify command and we dont need to increase the buffer counter + * all other cases (no last command exist or del command was the last command) it means that we need to increase the ref counter */ + if (op == SET_COMMAND) + { + if (pg_port_flags[port_name][ind] != SET_COMMAND) + { + /* if the last operation was not "set" then it's create and not modify - need to increase ref counter */ + gPortsOrch->increasePortRefCount(port_name); + } + } + else if (op == DEL_COMMAND) + { + if (pg_port_flags[port_name][ind] == SET_COMMAND) + { + /* we need to decrease ref counter only if the last operation was "SET_COMMAND" */ + gPortsOrch->decreasePortRefCount(port_name); + } + } + else + { + SWSS_LOG_ERROR("operation value is not SET or DEL (op = %s)", op.c_str()); + return task_process_status::task_invalid_entry; + } + /* save the last command (set or delete) */ + pg_port_flags[port_name][ind] = op; + } if (portUpdated) { diff --git a/tests/test_port_add_remove.py b/tests/test_port_add_remove.py new file mode 100644 index 0000000000..3131528aa2 --- /dev/null +++ b/tests/test_port_add_remove.py @@ -0,0 +1,56 @@ +import pytest +import time +from dvslib.dvs_common import PollingConfig + +# the port to be removed and add +PORT = "Ethernet0" + +class TestPortAddRemove(object): + + def test_remove_port_with_buffer_cfg(self, dvs, testlog): + config_db = dvs.get_config_db() + asic_db = dvs.get_asic_db() + + # get port info + port_info = config_db.get_entry("PORT", PORT) + + # get the number of ports before removal + num_of_ports = len(asic_db.get_keys("ASIC_STATE:SAI_OBJECT_TYPE_PORT")) + + # try to remove this port + config_db.delete_entry('PORT', PORT) + num = asic_db.wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_PORT", + num_of_ports-1, + polling_config = PollingConfig(polling_interval = 1, timeout = 5.00, strict = False)) + + # verify that the port wasn't removed since we still have buffer cfg + assert len(num) == num_of_ports + + # remove buffer pg cfg for the port + pgs = config_db.get_keys('BUFFER_PG') + for key in pgs: + if PORT in key: + config_db.delete_entry('BUFFER_PG', key) + + num = asic_db.wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_PORT", + num_of_ports-1, + polling_config = PollingConfig(polling_interval = 1, timeout = 5.00, strict = False)) + + # verify that the port wasn't removed since we still have buffer cfg + assert len(num) == num_of_ports + + # modify buffer queue entry to egress_lossless_profile instead of egress_lossy_profile + config_db.update_entry("BUFFER_QUEUE", "%s|0-2"%PORT, {"profile": "egress_lossless_profile"}) + + # remove buffer queue cfg for the port + pgs = config_db.get_keys('BUFFER_QUEUE') + for key in pgs: + if PORT in key: + config_db.delete_entry('BUFFER_QUEUE', key) + + num = asic_db.wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_PORT", + num_of_ports-1, + polling_config = PollingConfig(polling_interval = 1, timeout = 5.00, strict = True)) + + # verify that the port was removed properly since all buffer configuration was removed also + assert len(num) == num_of_ports - 1 From eb5d3a1ed6b6d7c484a7735b3766dd840ba1f9e9 Mon Sep 17 00:00:00 2001 From: Dror Prital Date: Wed, 23 Mar 2022 17:47:08 +0000 Subject: [PATCH 2/6] Update test due to comments: check admin status to set it back and add port --- tests/test_port_add_remove.py | 106 ++++++++++++++++++++++++++++++++-- 1 file changed, 100 insertions(+), 6 deletions(-) mode change 100644 => 100755 tests/test_port_add_remove.py diff --git a/tests/test_port_add_remove.py b/tests/test_port_add_remove.py old mode 100644 new mode 100755 index 3131528aa2..fb4e36dc81 --- a/tests/test_port_add_remove.py +++ b/tests/test_port_add_remove.py @@ -3,8 +3,16 @@ from dvslib.dvs_common import PollingConfig # the port to be removed and add -PORT = "Ethernet0" +PORT_A = "Ethernet64" +PORT_B = "Ethernet68" +""" +DELETE_CREATE_ITERATIONS defines the number of iteration of delete and create to ports, +we add different timeouts between delete/create to catch potential race condition that can lead to system crush. +""" +DELETE_CREATE_ITERATIONS = 10 + +@pytest.mark.usefixtures('dvs_port_manager') class TestPortAddRemove(object): def test_remove_port_with_buffer_cfg(self, dvs, testlog): @@ -12,13 +20,13 @@ def test_remove_port_with_buffer_cfg(self, dvs, testlog): asic_db = dvs.get_asic_db() # get port info - port_info = config_db.get_entry("PORT", PORT) + port_info = config_db.get_entry("PORT", PORT_A) # get the number of ports before removal num_of_ports = len(asic_db.get_keys("ASIC_STATE:SAI_OBJECT_TYPE_PORT")) # try to remove this port - config_db.delete_entry('PORT', PORT) + config_db.delete_entry('PORT', PORT_A) num = asic_db.wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_PORT", num_of_ports-1, polling_config = PollingConfig(polling_interval = 1, timeout = 5.00, strict = False)) @@ -29,7 +37,7 @@ def test_remove_port_with_buffer_cfg(self, dvs, testlog): # remove buffer pg cfg for the port pgs = config_db.get_keys('BUFFER_PG') for key in pgs: - if PORT in key: + if PORT_A in key: config_db.delete_entry('BUFFER_PG', key) num = asic_db.wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_PORT", @@ -40,12 +48,12 @@ def test_remove_port_with_buffer_cfg(self, dvs, testlog): assert len(num) == num_of_ports # modify buffer queue entry to egress_lossless_profile instead of egress_lossy_profile - config_db.update_entry("BUFFER_QUEUE", "%s|0-2"%PORT, {"profile": "egress_lossless_profile"}) + config_db.update_entry("BUFFER_QUEUE", "%s|0-2"%PORT_A, {"profile": "egress_lossless_profile"}) # remove buffer queue cfg for the port pgs = config_db.get_keys('BUFFER_QUEUE') for key in pgs: - if PORT in key: + if PORT_A in key: config_db.delete_entry('BUFFER_QUEUE', key) num = asic_db.wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_PORT", @@ -54,3 +62,89 @@ def test_remove_port_with_buffer_cfg(self, dvs, testlog): # verify that the port was removed properly since all buffer configuration was removed also assert len(num) == num_of_ports - 1 + + config_db.create_entry("PORT", PORT_A, port_info) + + @pytest.mark.parametrize("scenario", ["one_port", "all_ports"]) + def test_add_remove_all_the_ports(self, dvs, testlog, scenario): + config_db = dvs.get_config_db() + state_db = dvs.get_state_db() + asic_db = dvs.get_asic_db() + + # get the number of ports before removal + num_of_ports = len(asic_db.get_keys("ASIC_STATE:SAI_OBJECT_TYPE_PORT")) + + # remove buffer pg cfg for the port + if scenario == "all_ports": + ports = config_db.get_keys('PORT') + elif scenario == "one_port": + ports = [PORT_A] + else: + assert False + + ports_info = {} + + for i in range(DELETE_CREATE_ITERATIONS): + # remove ports + for key in ports: + # read port info and save it + ports_info[key] = config_db.get_entry("PORT", key) + + # remove a port + self.dvs_port.remove_port(key) + + # verify remove port + num = asic_db.wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_PORT", + num_of_ports-len(ports)) + assert len(num) == num_of_ports-len(ports) + + # add port + """ + DELETE_CREATE_ITERATIONS defines the number of iteration of delete and create to ports, + we add different timeouts between delete/create to catch potential race condition that can lead to system crush. + """ + time.sleep(i%3) + for key in ports: + config_db.create_entry("PORT", key, ports_info[key]) + + # verify add port + num = asic_db.wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_PORT", + num_of_ports) + assert len(num) == num_of_ports + + time.sleep((i%2)+1) + + # run ping + dvs.setup_db() + dvs.create_vlan("6") + dvs.create_vlan_member("6", PORT_A) + dvs.create_vlan_member("6", PORT_B) + + port_entry_a = state_db.get_entry("PORT_TABLE",PORT_A) + port_entry_b = state_db.get_entry("PORT_TABLE",PORT_B) + port_admin_a = port_entry_a['admin_status'] + port_admin_b = port_entry_b['admin_status'] + + dvs.set_interface_status("Vlan6", "up") + dvs.add_ip_address("Vlan6", "6.6.6.1/24") + dvs.set_interface_status(PORT_A, "up") + dvs.set_interface_status(PORT_B, "up") + + dvs.servers[16].runcmd("ifconfig eth0 6.6.6.6/24 up") + dvs.servers[16].runcmd("ip route add default via 6.6.6.1") + dvs.servers[17].runcmd("ifconfig eth0 6.6.6.7/24 up") + dvs.servers[17].runcmd("ip route add default via 6.6.6.1") + + time.sleep(2) + + rc = dvs.servers[16].runcmd("ping -c 1 6.6.6.7") + assert rc == 0 + + rc = dvs.servers[17].runcmd("ping -c 1 6.6.6.6") + assert rc == 0 + + dvs.set_interface_status(PORT_A, port_admin_a) + dvs.set_interface_status(PORT_B, port_admin_b) + dvs.remove_vlan_member("6", PORT_A) + dvs.remove_vlan_member("6", PORT_B) + dvs.remove_vlan("6") From 51de623158bd3aa468ebe1cb41edd1122f0c4365 Mon Sep 17 00:00:00 2001 From: Dror Prital Date: Thu, 24 Mar 2022 08:42:53 +0000 Subject: [PATCH 3/6] Move { for line consistent due to comment on PR --- orchagent/bufferorch.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/orchagent/bufferorch.cpp b/orchagent/bufferorch.cpp index 3e1c0ebb3b..751708739d 100644 --- a/orchagent/bufferorch.cpp +++ b/orchagent/bufferorch.cpp @@ -971,7 +971,8 @@ task_process_status BufferOrch::processQueue(KeyOpFieldsValuesTuple &tuple) } else if (op == DEL_COMMAND) { - if (queue_port_flags[port_name][ind] == SET_COMMAND) { + if (queue_port_flags[port_name][ind] == SET_COMMAND) + { /* we need to decrease ref counter only if the last operation was "SET_COMMAND" */ gPortsOrch->decreasePortRefCount(port_name); } From 39aa90ac8c625a177e24f7248427f2504f4a2e6d Mon Sep 17 00:00:00 2001 From: Stephen Sun Date: Wed, 30 Mar 2022 14:14:39 +0000 Subject: [PATCH 4/6] Address comment: add a flow for port remove/add/remove Signed-off-by: Stephen Sun --- tests/test_port_add_remove.py | 77 +++++++++++++++++++++++++---------- 1 file changed, 56 insertions(+), 21 deletions(-) diff --git a/tests/test_port_add_remove.py b/tests/test_port_add_remove.py index fb4e36dc81..8a9069fe3b 100755 --- a/tests/test_port_add_remove.py +++ b/tests/test_port_add_remove.py @@ -12,10 +12,9 @@ """ DELETE_CREATE_ITERATIONS = 10 -@pytest.mark.usefixtures('dvs_port_manager') class TestPortAddRemove(object): - def test_remove_port_with_buffer_cfg(self, dvs, testlog): + def test_remove_add_remove_port_with_buffer_cfg(self, dvs, testlog): config_db = dvs.get_config_db() asic_db = dvs.get_asic_db() @@ -25,45 +24,81 @@ def test_remove_port_with_buffer_cfg(self, dvs, testlog): # get the number of ports before removal num_of_ports = len(asic_db.get_keys("ASIC_STATE:SAI_OBJECT_TYPE_PORT")) - # try to remove this port - config_db.delete_entry('PORT', PORT_A) - num = asic_db.wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_PORT", - num_of_ports-1, - polling_config = PollingConfig(polling_interval = 1, timeout = 5.00, strict = False)) - - # verify that the port wasn't removed since we still have buffer cfg - assert len(num) == num_of_ports - # remove buffer pg cfg for the port + # record the buffer pgs before removing them pgs = config_db.get_keys('BUFFER_PG') + buffer_pgs = {} for key in pgs: if PORT_A in key: + buffer_pgs[key] = config_db.get_entry('BUFFER_PG', key) config_db.delete_entry('BUFFER_PG', key) - num = asic_db.wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_PORT", - num_of_ports-1, - polling_config = PollingConfig(polling_interval = 1, timeout = 5.00, strict = False)) - - # verify that the port wasn't removed since we still have buffer cfg - assert len(num) == num_of_ports - # modify buffer queue entry to egress_lossless_profile instead of egress_lossy_profile config_db.update_entry("BUFFER_QUEUE", "%s|0-2"%PORT_A, {"profile": "egress_lossless_profile"}) # remove buffer queue cfg for the port - pgs = config_db.get_keys('BUFFER_QUEUE') - for key in pgs: + queues = config_db.get_keys('BUFFER_QUEUE') + buffer_queues = {} + for key in queues: if PORT_A in key: + buffer_queues[key] = config_db.get_entry('BUFFER_QUEUE', key) config_db.delete_entry('BUFFER_QUEUE', key) + # try to remove this port + config_db.delete_entry('PORT', PORT_A) num = asic_db.wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_PORT", num_of_ports-1, polling_config = PollingConfig(polling_interval = 1, timeout = 5.00, strict = True)) # verify that the port was removed properly since all buffer configuration was removed also assert len(num) == num_of_ports - 1 - + config_db.create_entry("PORT", PORT_A, port_info) + # verify that the port has been readded + num = asic_db.wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_PORT", + num_of_ports, + polling_config = PollingConfig(polling_interval = 1, timeout = 5.00, strict = True)) + + assert len(num) == num_of_ports + + # re-add buffer pg and queue cfg to the port + for key, pg in buffer_pgs.items(): + config_db.update_entry("BUFFER_PG", key, pg) + + for key, queue in buffer_queues.items(): + config_db.update_entry("BUFFER_QUEUE", key, queue) + + # Remove the port with buffer configuration + config_db.delete_entry('PORT', PORT_A) + num = asic_db.wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_PORT", + num_of_ports-1, + polling_config = PollingConfig(polling_interval = 1, timeout = 5.00, strict = False)) + + # verify that the port wasn't removed since we still have buffer cfg + assert len(num) == num_of_ports + + # Remove buffer pgs + for key in buffer_pgs.keys(): + config_db.delete_entry('BUFFER_PG', key) + + num = asic_db.wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_PORT", + num_of_ports-1, + polling_config = PollingConfig(polling_interval = 1, timeout = 5.00, strict = False)) + + # verify that the port wasn't removed since we still have buffer cfg + assert len(num) == num_of_ports + + # Remove buffer queue + for key in buffer_queues.keys(): + config_db.delete_entry('BUFFER_QUEUE', key) + + num = asic_db.wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_PORT", + num_of_ports-1, + polling_config = PollingConfig(polling_interval = 1, timeout = 5.00, strict = True)) + + # verify that the port wasn't removed since we still have buffer cfg + assert len(num) == num_of_ports - 1 + @pytest.mark.parametrize("scenario", ["one_port", "all_ports"]) def test_add_remove_all_the_ports(self, dvs, testlog, scenario): From 0a5db6fa0418443361e87605665bb94cf5b31474 Mon Sep 17 00:00:00 2001 From: dprital Date: Tue, 23 Aug 2022 17:17:29 +0000 Subject: [PATCH 5/6] Fix test failures --- tests/test_port_add_remove.py | 81 +++++++++++++++++++++++++++++------ 1 file changed, 69 insertions(+), 12 deletions(-) diff --git a/tests/test_port_add_remove.py b/tests/test_port_add_remove.py index 8a9069fe3b..be27a83f8f 100755 --- a/tests/test_port_add_remove.py +++ b/tests/test_port_add_remove.py @@ -1,5 +1,6 @@ import pytest import time +import buffer_model from dvslib.dvs_common import PollingConfig # the port to be removed and add @@ -8,15 +9,35 @@ """ DELETE_CREATE_ITERATIONS defines the number of iteration of delete and create to ports, -we add different timeouts between delete/create to catch potential race condition that can lead to system crush. +we add different timeouts between delete/create to catch potential race condition that can lead to system crush + +Add \ Remove of Buffers can be done only when the model is dynamic. """ DELETE_CREATE_ITERATIONS = 10 +@pytest.yield_fixture +def dynamic_buffer(dvs): + buffer_model.enable_dynamic_buffer(dvs.get_config_db(), dvs.runcmd) + yield + buffer_model.disable_dynamic_buffer(dvs.get_config_db(), dvs.runcmd) + +@pytest.mark.usefixtures('dvs_port_manager') +@pytest.mark.usefixtures("dynamic_buffer") class TestPortAddRemove(object): def test_remove_add_remove_port_with_buffer_cfg(self, dvs, testlog): config_db = dvs.get_config_db() asic_db = dvs.get_asic_db() + state_db = dvs.get_state_db() + app_db = dvs.get_app_db() + + # set mmu size + fvs = {"mmu_size": "12766208"} + state_db.create_entry("BUFFER_MAX_PARAM_TABLE", "global", fvs) + bufferMaxParameter = state_db.wait_for_entry("BUFFER_MAX_PARAM_TABLE", PORT_A) + + # Startup interface + dvs.port_admin_set(PORT_A, 'up') # get port info port_info = config_db.get_entry("PORT", PORT_A) @@ -24,14 +45,14 @@ def test_remove_add_remove_port_with_buffer_cfg(self, dvs, testlog): # get the number of ports before removal num_of_ports = len(asic_db.get_keys("ASIC_STATE:SAI_OBJECT_TYPE_PORT")) - # remove buffer pg cfg for the port - # record the buffer pgs before removing them + # remove buffer pg cfg for the port (record the buffer pgs before removing them) pgs = config_db.get_keys('BUFFER_PG') buffer_pgs = {} for key in pgs: if PORT_A in key: buffer_pgs[key] = config_db.get_entry('BUFFER_PG', key) config_db.delete_entry('BUFFER_PG', key) + app_db.wait_for_deleted_entry("BUFFER_PG_TABLE", key) # modify buffer queue entry to egress_lossless_profile instead of egress_lossy_profile config_db.update_entry("BUFFER_QUEUE", "%s|0-2"%PORT_A, {"profile": "egress_lossless_profile"}) @@ -43,7 +64,11 @@ def test_remove_add_remove_port_with_buffer_cfg(self, dvs, testlog): if PORT_A in key: buffer_queues[key] = config_db.get_entry('BUFFER_QUEUE', key) config_db.delete_entry('BUFFER_QUEUE', key) + app_db.wait_for_deleted_entry('BUFFER_QUEUE_TABLE', key) + # Shutdown interface + dvs.port_admin_set(PORT_A, 'down') + # try to remove this port config_db.delete_entry('PORT', PORT_A) num = asic_db.wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_PORT", @@ -53,7 +78,9 @@ def test_remove_add_remove_port_with_buffer_cfg(self, dvs, testlog): # verify that the port was removed properly since all buffer configuration was removed also assert len(num) == num_of_ports - 1 - config_db.create_entry("PORT", PORT_A, port_info) + # set back the port + config_db.update_entry("PORT", PORT_A, port_info) + # verify that the port has been readded num = asic_db.wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_PORT", num_of_ports, @@ -68,6 +95,8 @@ def test_remove_add_remove_port_with_buffer_cfg(self, dvs, testlog): for key, queue in buffer_queues.items(): config_db.update_entry("BUFFER_QUEUE", key, queue) + time.sleep(5) + # Remove the port with buffer configuration config_db.delete_entry('PORT', PORT_A) num = asic_db.wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_PORT", @@ -80,6 +109,7 @@ def test_remove_add_remove_port_with_buffer_cfg(self, dvs, testlog): # Remove buffer pgs for key in buffer_pgs.keys(): config_db.delete_entry('BUFFER_PG', key) + app_db.wait_for_deleted_entry("BUFFER_PG_TABLE", key) num = asic_db.wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_PORT", num_of_ports-1, @@ -91,6 +121,7 @@ def test_remove_add_remove_port_with_buffer_cfg(self, dvs, testlog): # Remove buffer queue for key in buffer_queues.keys(): config_db.delete_entry('BUFFER_QUEUE', key) + app_db.wait_for_deleted_entry('BUFFER_QUEUE_TABLE', key) num = asic_db.wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_PORT", num_of_ports-1, @@ -99,13 +130,18 @@ def test_remove_add_remove_port_with_buffer_cfg(self, dvs, testlog): # verify that the port wasn't removed since we still have buffer cfg assert len(num) == num_of_ports - 1 + # set back the port as it is required for next test + config_db.update_entry("PORT", PORT_A, port_info) + + @pytest.mark.parametrize("scenario", ["one_port", "all_ports"]) def test_add_remove_all_the_ports(self, dvs, testlog, scenario): config_db = dvs.get_config_db() state_db = dvs.get_state_db() asic_db = dvs.get_asic_db() - + app_db = dvs.get_app_db() + # get the number of ports before removal num_of_ports = len(asic_db.get_keys("ASIC_STATE:SAI_OBJECT_TYPE_PORT")) @@ -117,20 +153,38 @@ def test_add_remove_all_the_ports(self, dvs, testlog, scenario): else: assert False + # delete all PGs and QUEUEs from the relevant ports + pgs = config_db.get_keys('BUFFER_PG') + queues = config_db.get_keys('BUFFER_QUEUE') + + for port in ports: + for key in pgs: + if port in key: + config_db.delete_entry('BUFFER_PG', key) + app_db.wait_for_deleted_entry('BUFFER_PG_TABLE', key) + + for key in queues: + if port in key: + config_db.delete_entry('BUFFER_QUEUE', key) + app_db.wait_for_deleted_entry('BUFFER_QUEUE_TABLE', key) + ports_info = {} - + + for key in ports: + # read port info and save it + ports_info[key] = config_db.get_entry("PORT", key) + + for i in range(DELETE_CREATE_ITERATIONS): # remove ports for key in ports: - # read port info and save it - ports_info[key] = config_db.get_entry("PORT", key) - - # remove a port - self.dvs_port.remove_port(key) + config_db.delete_entry('PORT',key) + app_db.wait_for_deleted_entry("PORT_TABLE", key) # verify remove port num = asic_db.wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_PORT", num_of_ports-len(ports)) + assert len(num) == num_of_ports-len(ports) # add port @@ -140,17 +194,20 @@ def test_add_remove_all_the_ports(self, dvs, testlog, scenario): """ time.sleep(i%3) for key in ports: - config_db.create_entry("PORT", key, ports_info[key]) + config_db.update_entry("PORT", key, ports_info[key]) + app_db.wait_for_entry('PORT_TABLE',key) # verify add port num = asic_db.wait_for_n_keys("ASIC_STATE:SAI_OBJECT_TYPE_PORT", num_of_ports) + assert len(num) == num_of_ports time.sleep((i%2)+1) # run ping dvs.setup_db() + dvs.create_vlan("6") dvs.create_vlan_member("6", PORT_A) dvs.create_vlan_member("6", PORT_B) From 35b5ddab81986ee42fb2d706d2e0ed0b1afbae5c Mon Sep 17 00:00:00 2001 From: dprital Date: Wed, 24 Aug 2022 08:17:13 +0000 Subject: [PATCH 6/6] Update test due to review feedback, move mmu_set to common place --- tests/test_port_add_remove.py | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/tests/test_port_add_remove.py b/tests/test_port_add_remove.py index be27a83f8f..bfc3074d83 100755 --- a/tests/test_port_add_remove.py +++ b/tests/test_port_add_remove.py @@ -21,10 +21,18 @@ def dynamic_buffer(dvs): yield buffer_model.disable_dynamic_buffer(dvs.get_config_db(), dvs.runcmd) + @pytest.mark.usefixtures('dvs_port_manager') @pytest.mark.usefixtures("dynamic_buffer") class TestPortAddRemove(object): + def set_mmu(self,dvs): + state_db = dvs.get_state_db() + # set mmu size + fvs = {"mmu_size": "12766208"} + state_db.create_entry("BUFFER_MAX_PARAM_TABLE", "global", fvs) + + def test_remove_add_remove_port_with_buffer_cfg(self, dvs, testlog): config_db = dvs.get_config_db() asic_db = dvs.get_asic_db() @@ -32,9 +40,7 @@ def test_remove_add_remove_port_with_buffer_cfg(self, dvs, testlog): app_db = dvs.get_app_db() # set mmu size - fvs = {"mmu_size": "12766208"} - state_db.create_entry("BUFFER_MAX_PARAM_TABLE", "global", fvs) - bufferMaxParameter = state_db.wait_for_entry("BUFFER_MAX_PARAM_TABLE", PORT_A) + self.set_mmu(dvs) # Startup interface dvs.port_admin_set(PORT_A, 'up') @@ -142,6 +148,9 @@ def test_add_remove_all_the_ports(self, dvs, testlog, scenario): asic_db = dvs.get_asic_db() app_db = dvs.get_app_db() + # set mmu size + self.set_mmu(dvs) + # get the number of ports before removal num_of_ports = len(asic_db.get_keys("ASIC_STATE:SAI_OBJECT_TYPE_PORT"))