Td2: Reclaim buffer from unused ports (sonic-net#1830)

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

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

Author: neethajohn

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<FieldValueTuple> 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)
                 {

cfgmgr/buffermgr.h

@@ -27,6 +27,7 @@ typedef std::map<std::string, pg_profile_t> speed_map_t;
 typedef std::map<std::string, speed_map_t> pg_profile_lookup_t;
 
 typedef std::map<std::string, std::string> port_cable_length_t;
+typedef std::map<std::string, std::string> 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);

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