From 7f80f06a98e2a18c4e5db4d5be25a62f6ac95bf6 Mon Sep 17 00:00:00 2001 From: Neetha John Date: Mon, 26 Jul 2021 10:42:27 -0700 Subject: [PATCH] Td2: Reclaim buffer from unused ports (#1830) Signed-off-by: Neetha John By default, lossless profiles are attached to PGs 3, 4 of all ports which results in a lot of buffer wastage when most of those ports are unused (not associated with a DEVICE_NEIGHBOR). In TD2, ingress pool size comprises of the reserved space as well. Hence making use of a special cable length of '0m' to identify unused ports and skip reserving space for those ports What I did * In buffermgr, if port with cable len '0m' is identified, return immediately without creating pg lossless profile or attaching profile to the lossless pgs of that port * Listen to 'admin_status' update as well from the PORT table and update port-speed mapping. This is to handle add-rack scenario where a port is added later - The port starts of with cable length 0m - Configlet to add a port is applied. The order of operations specifc to the PORT/CABLE_LENGTH table are - port is initially set to admin down, cable length is updated for that port, port table attributes are defined and port is set to admin up - speed update might not be seen when the port is set to admin up. Hence port-speed mapping will capture the speed update whenever its seen and once the cable length is updated while the port is brought back up, profiles can be attached to the lossless pgs of the port How I verified it - Manual tests done. Verified that no space is reserved for unused ports - Verified that when a port is added using 'add-rack' scenario, profile is attached to pgs of that port - New VS test added --- cfgmgr/buffermgr.cpp | 25 ++++-- cfgmgr/buffermgr.h | 4 +- tests/test_buffer_traditional.py | 145 +++++++++++++++++++++++++++++++ 3 files changed, 168 insertions(+), 6 deletions(-) create mode 100644 tests/test_buffer_traditional.py diff --git a/cfgmgr/buffermgr.cpp b/cfgmgr/buffermgr.cpp index 7b1b73faffb3..684b17f31de9 100644 --- a/cfgmgr/buffermgr.cpp +++ b/cfgmgr/buffermgr.cpp @@ -86,6 +86,7 @@ void BufferMgr::readPgProfileLookupFile(string file) task_process_status BufferMgr::doCableTask(string port, string cable_length) { m_cableLenLookup[port] = cable_length; + SWSS_LOG_INFO("Cable length set to %s for port %s", m_cableLenLookup[port].c_str(), port.c_str()); return task_process_status::task_success; } @@ -120,10 +121,11 @@ Create/update two tables: profile (in m_cfgBufferProfileTable) and port buffer ( } } */ -task_process_status BufferMgr::doSpeedUpdateTask(string port, string speed) +task_process_status BufferMgr::doSpeedUpdateTask(string port) { vector fvVector; string cable; + string speed; if (m_cableLenLookup.count(port) == 0) { @@ -132,7 +134,13 @@ task_process_status BufferMgr::doSpeedUpdateTask(string port, string speed) } cable = m_cableLenLookup[port]; + if (cable == "0m") + { + SWSS_LOG_NOTICE("Not creating/updating PG profile for port %s. Cable length is set to %s", port.c_str(), cable.c_str()); + return task_process_status::task_success; + } + speed = m_speedLookup[port]; if (m_pgProfileLookup.count(speed) == 0 || m_pgProfileLookup[speed].count(cable) == 0) { SWSS_LOG_ERROR("Unable to create/update PG profile for port %s. No PG profile configured for speed %s and cable length %s", @@ -368,11 +376,18 @@ void BufferMgr::doTask(Consumer &consumer) // receive and cache cable length table task_status = doCableTask(fvField(i), fvValue(i)); } - // In case of PORT table update, Buffer Manager is interested in speed update only - if (m_pgfile_processed && table_name == CFG_PORT_TABLE_NAME && fvField(i) == "speed") + if (m_pgfile_processed && table_name == CFG_PORT_TABLE_NAME && (fvField(i) == "speed" || fvField(i) == "admin_status")) { - // create/update profile for port - task_status = doSpeedUpdateTask(port, fvValue(i)); + if (fvField(i) == "speed") + { + m_speedLookup[port] = fvValue(i); + } + + if (m_speedLookup.count(port) != 0) + { + // create/update profile for port + task_status = doSpeedUpdateTask(port); + } } if (task_status != task_process_status::task_success) { diff --git a/cfgmgr/buffermgr.h b/cfgmgr/buffermgr.h index 5ad88f8901b2..c9777d291857 100644 --- a/cfgmgr/buffermgr.h +++ b/cfgmgr/buffermgr.h @@ -27,6 +27,7 @@ typedef std::map speed_map_t; typedef std::map pg_profile_lookup_t; typedef std::map port_cable_length_t; +typedef std::map port_speed_t; class BufferMgr : public Orch { @@ -52,10 +53,11 @@ class BufferMgr : public Orch pg_profile_lookup_t m_pgProfileLookup; port_cable_length_t m_cableLenLookup; + port_speed_t m_speedLookup; std::string getPgPoolMode(); void readPgProfileLookupFile(std::string); task_process_status doCableTask(std::string port, std::string cable_length); - task_process_status doSpeedUpdateTask(std::string port, std::string speed); + task_process_status doSpeedUpdateTask(std::string port); void doBufferTableTask(Consumer &consumer, ProducerStateTable &applTable); void transformSeperator(std::string &name); diff --git a/tests/test_buffer_traditional.py b/tests/test_buffer_traditional.py new file mode 100644 index 000000000000..b21862c29638 --- /dev/null +++ b/tests/test_buffer_traditional.py @@ -0,0 +1,145 @@ +import pytest +import time + + +class TestBuffer(object): + LOSSLESS_PGS = [3, 4] + INTF = "Ethernet0" + + def setup_db(self, dvs): + self.app_db = dvs.get_app_db() + self.asic_db = dvs.get_asic_db() + self.config_db = dvs.get_config_db() + self.counter_db = dvs.get_counters_db() + + # enable PG watermark + self.set_pg_wm_status('enable') + + def get_pg_oid(self, pg): + fvs = dict() + fvs = self.counter_db.get_entry("COUNTERS_PG_NAME_MAP", "") + return fvs[pg] + + def set_pg_wm_status(self, state): + fvs = {'FLEX_COUNTER_STATUS': state} + self.config_db.update_entry("FLEX_COUNTER_TABLE", "PG_WATERMARK", fvs) + time.sleep(1) + + def teardown(self): + # disable PG watermark + self.set_pg_wm_status('disable') + + def get_asic_buf_profile(self): + return set(self.asic_db.get_keys("ASIC_STATE:SAI_OBJECT_TYPE_BUFFER_PROFILE")) + + def check_new_profile_in_asic_db(self, profile): + retry_count = 0 + self.new_profile = None + while retry_count < 5: + retry_count += 1 + diff = set(self.asic_db.get_keys("ASIC_STATE:SAI_OBJECT_TYPE_BUFFER_PROFILE")) - self.orig_profiles + if len(diff) == 1: + self.new_profile = diff.pop() + break + else: + time.sleep(1) + assert self.new_profile, "Can't get SAI OID for newly created profile {} after retry {} times".format(profile, retry_count) + + def get_asic_buf_pg_profiles(self): + self.buf_pg_profile = dict() + for pg in self.pg_name_map: + buf_pg_entries = self.asic_db.get_entry("ASIC_STATE:SAI_OBJECT_TYPE_INGRESS_PRIORITY_GROUP", self.pg_name_map[pg]) + self.buf_pg_profile[pg] = buf_pg_entries["SAI_INGRESS_PRIORITY_GROUP_ATTR_BUFFER_PROFILE"] + + def change_cable_len(self, cable_len): + fvs = dict() + fvs[self.INTF] = cable_len + self.config_db.update_entry("CABLE_LENGTH", "AZURE", fvs) + + @pytest.fixture + def setup_teardown_test(self, dvs): + try: + self.setup_db(dvs) + pg_name_map = dict() + for pg in self.LOSSLESS_PGS: + pg_name = "{}:{}".format(self.INTF, pg) + pg_name_map[pg_name] = self.get_pg_oid(pg_name) + yield pg_name_map + finally: + self.teardown() + + def test_zero_cable_len_profile_update(self, dvs, setup_teardown_test): + self.pg_name_map = setup_teardown_test + orig_cable_len = None + orig_speed = None + try: + dvs.runcmd("config interface startup {}".format(self.INTF)) + self.orig_profiles = self.get_asic_buf_profile() + # get orig cable length and speed + fvs = self.config_db.get_entry("CABLE_LENGTH", "AZURE") + orig_cable_len = fvs[self.INTF] + fvs = self.config_db.get_entry("PORT", self.INTF) + orig_speed = fvs["speed"] + + if orig_speed == "100000": + test_speed = "40000" + elif orig_speed == "40000": + test_speed = "100000" + test_cable_len = "0m" + + # check if the lossless profile for the test speed is already present + fvs = dict() + new_lossless_profile = "pg_lossless_{}_{}_profile".format(test_speed, orig_cable_len) + fvs = self.app_db.get_entry("BUFFER_PROFILE_TABLE", new_lossless_profile) + if len(fvs): + profile_exp_cnt_diff = 0 + else: + profile_exp_cnt_diff = 1 + + # get the orig buf profiles attached to the pgs + self.get_asic_buf_pg_profiles() + + # change cable length to 'test_cable_len' + self.change_cable_len(test_cable_len) + + # change intf speed to 'test_speed' + dvs.runcmd("config interface speed {} {}".format(self.INTF, test_speed)) + test_lossless_profile = "pg_lossless_{}_{}_profile".format(test_speed, test_cable_len) + # buffer profile should not get created + self.app_db.wait_for_deleted_entry("BUFFER_PROFILE_TABLE", test_lossless_profile) + + # buffer pgs should still point to the original buffer profile + orig_lossless_profile = "pg_lossless_{}_{}_profile".format(orig_speed, orig_cable_len) + self.app_db.wait_for_field_match("BUFFER_PG_TABLE", self.INTF + ":3-4", {"profile": "[BUFFER_PROFILE_TABLE:{}]".format(orig_lossless_profile)}) + fvs = dict() + for pg in self.pg_name_map: + fvs["SAI_INGRESS_PRIORITY_GROUP_ATTR_BUFFER_PROFILE"] = self.buf_pg_profile[pg] + self.asic_db.wait_for_field_match("ASIC_STATE:SAI_OBJECT_TYPE_INGRESS_PRIORITY_GROUP", self.pg_name_map[pg], fvs) + + # change cable length to 'orig_cable_len' + self.change_cable_len(orig_cable_len) + + # change intf speed to 'test_speed' + dvs.runcmd("config interface speed {} {}".format(self.INTF, test_speed)) + if profile_exp_cnt_diff != 0: + # new profile will get created + self.app_db.wait_for_entry("BUFFER_PROFILE_TABLE", new_lossless_profile) + self.check_new_profile_in_asic_db(new_lossless_profile) + # verify that buffer pgs point to the new profile + fvs = dict() + for pg in self.pg_name_map: + fvs["SAI_INGRESS_PRIORITY_GROUP_ATTR_BUFFER_PROFILE"] = self.new_profile + self.asic_db.wait_for_field_match("ASIC_STATE:SAI_OBJECT_TYPE_INGRESS_PRIORITY_GROUP", self.pg_name_map[pg], fvs) + + else: + # verify that buffer pgs do not point to the old profile since we cannot deduce the new profile oid + fvs = dict() + for pg in self.pg_name_map: + fvs["SAI_INGRESS_PRIORITY_GROUP_ATTR_BUFFER_PROFILE"] = self.buf_pg_profile[pg] + self.asic_db.wait_for_field_negative_match("ASIC_STATE:SAI_OBJECT_TYPE_INGRESS_PRIORITY_GROUP", self.pg_name_map[pg], fvs) + finally: + if orig_cable_len: + self.change_cable_len(orig_cable_len) + if orig_speed: + dvs.runcmd("config interface speed {} {}".format(self.INTF, orig_speed)) + dvs.runcmd("config interface shutdown {}".format(self.INTF))