[Buffer Manager][201911] Reclaim unused buffer for admin-down ports (#1837)

Depends on #1787

What I did
To reclaim reserved buffer.
As the way to do it differs among vendors, the environment ASIC_VENDOR is passed to swss docker and will be loaded when buffermgrd starts. After that, buffermgrd will:

Handle port admin down on Mellanox platform.
Not apply lossless buffer PG to an admin-down port
Remove lossless buffer PG (3-4) from a port when it is shut down.
Readd lossless buffer PG (3-4) to a port when a port is started up.

Why I did it
To support reclaiming reserved buffer when a port is shut down on Mellanox platform.

How I verified it
Regression test and vs test.

Signed-off-by: Stephen Sun <[email protected]>

Author: stephenxs

cfgmgr/buffermgr.cpp

@@ -22,6 +22,15 @@ BufferMgr::BufferMgr(DBConnector *cfgDb, DBConnector *stateDb, string pg_lookup_
         m_cfgLosslessPgPoolTable(cfgDb, CFG_BUFFER_POOL_TABLE_NAME)
 {
     readPgProfileLookupFile(pg_lookup_file);
+    char *platform = getenv("ASIC_VENDOR");
+    if (NULL == platform)
+    {
+        SWSS_LOG_WARN("Platform environment variable is not defined");
+    }
+    else
+    {
+        m_platform = platform;
+    }
 }
 
 //# speed, cable, size,    xon,  xoff, threshold,  xon_offset
@@ -112,11 +121,27 @@ Create/update two tables: profile (in m_cfgBufferProfileTable) and port buffer (
         }
     }
 */
-task_process_status BufferMgr::doSpeedUpdateTask(string port, string speed)
+task_process_status BufferMgr::doPortTableUpdateTask(string port, string speed, bool admin_up)
 {
-    vector<FieldValueTuple> fvVector;
+    vector<FieldValueTuple> fvVectorPg, fvVectorProfile;
     string cable;
 
+    string buffer_pg_key = port + m_cfgBufferPgTable.getTableNameSeparator() + LOSSLESS_PGS;
+
+    m_cfgBufferPgTable.get(buffer_pg_key, fvVectorPg);
+
+    if (!admin_up && m_platform == "mellanox")
+    {
+        // Remove the entry in BUFFER_PG table if any
+        if (!fvVectorPg.empty())
+        {
+            SWSS_LOG_NOTICE("Removing PG %s from port %s which is administrative down", buffer_pg_key.c_str(), port.c_str());
+            m_cfgBufferPgTable.del(buffer_pg_key);
+        }
+
+        return task_process_status::task_success;
+    }
+
     if (m_cableLenLookup.count(port) == 0)
     {
         SWSS_LOG_INFO("Unable to create/update PG profile for port %s. Cable length is not set", port.c_str());
@@ -137,8 +162,8 @@ task_process_status BufferMgr::doSpeedUpdateTask(string port, string speed)
     string buffer_profile_key = "pg_lossless_" + speed + "_" + cable + "_profile";
 
     // check if profile already exists - if yes - skip creation
-    m_cfgBufferProfileTable.get(buffer_profile_key, fvVector);
-    if (fvVector.size() == 0)
+    m_cfgBufferProfileTable.get(buffer_profile_key, fvVectorProfile);
+    if (fvVectorProfile.size() == 0)
     {
         SWSS_LOG_NOTICE("Creating new profile '%s'", buffer_profile_key.c_str());
 
@@ -156,49 +181,42 @@ task_process_status BufferMgr::doSpeedUpdateTask(string port, string speed)
                                    m_cfgBufferProfileTable.getTableNameSeparator() +
                                    INGRESS_LOSSLESS_PG_POOL_NAME;
 
-        fvVector.push_back(make_pair("pool", "[" + pg_pool_reference + "]"));
-        fvVector.push_back(make_pair("xon", m_pgProfileLookup[speed][cable].xon));
+        fvVectorProfile.push_back(make_pair("pool", "[" + pg_pool_reference + "]"));
+        fvVectorProfile.push_back(make_pair("xon", m_pgProfileLookup[speed][cable].xon));
         if (m_pgProfileLookup[speed][cable].xon_offset.length() > 0) {
-            fvVector.push_back(make_pair("xon_offset",
+            fvVectorProfile.push_back(make_pair("xon_offset",
                                          m_pgProfileLookup[speed][cable].xon_offset));
         }
-        fvVector.push_back(make_pair("xoff", m_pgProfileLookup[speed][cable].xoff));
-        fvVector.push_back(make_pair("size", m_pgProfileLookup[speed][cable].size));
-        fvVector.push_back(make_pair(mode, m_pgProfileLookup[speed][cable].threshold));
-        m_cfgBufferProfileTable.set(buffer_profile_key, fvVector);
+        fvVectorProfile.push_back(make_pair("xoff", m_pgProfileLookup[speed][cable].xoff));
+        fvVectorProfile.push_back(make_pair("size", m_pgProfileLookup[speed][cable].size));
+        fvVectorProfile.push_back(make_pair(mode, m_pgProfileLookup[speed][cable].threshold));
+        m_cfgBufferProfileTable.set(buffer_profile_key, fvVectorProfile);
     }
     else
     {
         SWSS_LOG_NOTICE("Reusing existing profile '%s'", buffer_profile_key.c_str());
     }
 
-    fvVector.clear();
-
-    string buffer_pg_key = port + m_cfgBufferPgTable.getTableNameSeparator() + LOSSLESS_PGS;
-
     string profile_ref = string("[") +
                          CFG_BUFFER_PROFILE_TABLE_NAME +
                          m_cfgBufferPgTable.getTableNameSeparator() +
                          buffer_profile_key +
                          "]";
 
     /* Check if PG Mapping is already then log message and return. */
-
-    m_cfgBufferPgTable.get(buffer_pg_key, fvVector);
-    
-    for (auto& prop : fvVector)
+    for (auto& prop : fvVectorPg)
     {
         if ((fvField(prop) == "profile") && (profile_ref == fvValue(prop)))
         {
             SWSS_LOG_NOTICE("PG to Buffer Profile Mapping %s already present", buffer_pg_key.c_str());
             return task_process_status::task_success;
         }
     }
-    
-    fvVector.clear();
- 
-    fvVector.push_back(make_pair("profile", profile_ref));
-    m_cfgBufferPgTable.set(buffer_pg_key, fvVector);
+
+    fvVectorPg.clear();
+
+    fvVectorPg.push_back(make_pair("profile", profile_ref));
+    m_cfgBufferPgTable.set(buffer_pg_key, fvVectorPg);
     return task_process_status::task_success;
 }
 
@@ -221,23 +239,34 @@ void BufferMgr::doTask(Consumer &consumer)
         task_process_status task_status = task_process_status::task_success;
         if (op == SET_COMMAND)
         {
-            for (auto i : kfvFieldsValues(t))
+            if (table_name == CFG_PORT_CABLE_LEN_TABLE_NAME)
             {
-                if (table_name == CFG_PORT_CABLE_LEN_TABLE_NAME)
+                // receive and cache cable length table
+                for (auto i : kfvFieldsValues(t))
                 {
-                    // 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")
-                {
-                    // create/update profile for port
-                    task_status = doSpeedUpdateTask(port, fvValue(i));
-                }
-                if (task_status != task_process_status::task_success)
+            }
+            else if (m_pgfile_processed && table_name == CFG_PORT_TABLE_NAME)
+            {
+                bool speed_updated = false, admin_status_updated = false, admin_up = false;
+                string speed;
+                for (auto i : kfvFieldsValues(t))
                 {
-                    break;
+                    if (fvField(i) == "speed")
+                    {
+                        speed_updated = true;
+                        speed = fvValue(i);
+                    }
+                    else if (fvField(i) == "admin_status")
+                    {
+                        admin_status_updated = true;
+                        admin_up = ("up" == fvValue(i));
+                    }
                 }
+
+                if (speed_updated || admin_status_updated)
+                    task_status = doPortTableUpdateTask(port, speed, admin_up);
             }
         }
 

cfgmgr/buffermgr.h

@@ -33,6 +33,8 @@ class BufferMgr : public Orch
     using Orch::doTask;
 
 private:
+    std::string m_platform;
+
     Table m_cfgPortTable;
     Table m_cfgCableLenTable;
     Table m_cfgBufferProfileTable;
@@ -45,7 +47,7 @@ class BufferMgr : public Orch
     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 doPortTableUpdateTask(std::string port, std::string speed, bool admin_up);
 
     void doTask(Consumer &consumer);
 };

tests/test_buffer_manager.py

@@ -0,0 +1,79 @@
+import pytest
+import time
+from swsscommon import swsscommon
+
+class TestBufferManager(object):
+    def make_dict(self, input_list):
+        return dict(input_list[1])
+
+    def setup_db(self, dvs):
+        self.config_db = swsscommon.DBConnector(4, dvs.redis_sock, 0)
+        self.buffer_pg_table = swsscommon.Table(self.config_db, "BUFFER_PG")
+        self.port_table = swsscommon.Table(self.config_db, "PORT")
+        cable_length_table = swsscommon.Table(self.config_db, "CABLE_LENGTH")
+        cable_length_key = cable_length_table.getKeys()[0]
+        self.cable_lengths = self.make_dict(cable_length_table.get(cable_length_key))
+        self.buffer_profile_table = swsscommon.Table(self.config_db, "BUFFER_PROFILE")
+
+    def load_pg_profile_lookup(self, dvs):
+        self.profile_lookup_info = {}
+        lines = dvs.runcmd('cat /usr/share/sonic/hwsku/pg_profile_lookup.ini')[1].split('\n')
+
+        SPEED = 0
+        CABLE_LENGTH = 1
+        SIZE = 2
+        XON = 3
+        XOFF = 4
+        THRESHOLD = 5
+        XON_OFFSET = 6
+
+        for line in lines:
+            if len(line) == 0 or line[0] == '#':
+                continue
+            tokens = line.split()
+            self.profile_lookup_info[(tokens[SPEED], tokens[CABLE_LENGTH])] = {
+                'size': tokens[SIZE],
+                'xon': tokens[XON],
+                'xoff': tokens[XOFF],
+                'dynamic_th': tokens[THRESHOLD]
+            }
+            if XON_OFFSET < len(tokens):
+                self.profile_lookup_info[(tokens[SPEED], tokens[CABLE_LENGTH])]['xon_offset'] = tokens[XON_OFFSET]
+
+    def test_buffer_pg(self, dvs):
+        self.setup_db(dvs)
+
+        port = 'Ethernet0'
+        pg = port + '|3-4'
+
+        port_info = self.make_dict(self.port_table.get(port))
+        if 'up' == port_info.get('admin_status'):
+            # By default, all ports should be admin down on VM.
+            # However, in case the port under test was admin up before the test, we just shut down it.
+            dvs.runcmd('config interface shutdown {}'.format(port))
+            time.sleep(1)
+
+        # Make sure no lossless PG exists on an admin down port
+        assert not self.buffer_pg_table.get(pg)[0]
+
+        try:
+            # Startup the port. The lossless PG should be created according to speed and cable length
+            dvs.runcmd('config interface startup {}'.format(port))
+
+            cable_length = self.cable_lengths.get(port)
+            speed = port_info.get('speed')
+
+            expected_profile_name = 'pg_lossless_{}_{}_profile'.format(speed, cable_length)
+            buffer_profile_info = self.make_dict(self.buffer_profile_table.get(expected_profile_name))
+            buffer_pg_info = self.make_dict(self.buffer_pg_table.get(pg))
+            assert buffer_pg_info['profile'] == '[BUFFER_PROFILE|{}]'.format(expected_profile_name)
+
+            self.load_pg_profile_lookup(dvs)
+            expected_profile_info = self.profile_lookup_info[(speed, cable_length)]
+            expected_profile_info['pool'] = '[BUFFER_POOL|ingress_lossless_pool]'
+            assert buffer_profile_info == expected_profile_info
+        finally:
+            # Shutdown the port. The lossless PG should be removed
+            dvs.runcmd('config interface shutdown {}'.format(port))
+            time.sleep(1)
+            assert not self.buffer_pg_table.get(pg)[0]