[Buffermgr]Graceful handling of buffer model change (#1956)

Signed-off-by: Sudharsan Dhamal Gopalarathnam [email protected] What I did Handled buffer model changes gracefully. The use case is when config_db.json is loaded initially with no buffer configurations and when user executes 'config qos reload', it would result in buffer model changing from traditional to dynamic. This transition requires swss restart which will be shown as message to user. Why I did it When config qos reload is given in platforms with dynamic buffer model, swss restart is required. However, if swss is not restarted the buffermgrd will stay in static model and will program the orchagent with 'size' field not set in buffer pool due to dynamic mode checks in jinja2 template. This will result in orchagent calling SAI without SAI_BUFFER_POOL_ATTR_SIZE which is mandatory attribute. Since this results in a SAI create API failure, it will result in orchagent crash. So when the mode is changed to dynamic, buffermgrd will not process any configurations when running in static mode. How I verified it Added UT. Manually verified that config qos reload doesn't crash.
sonic-net · Oct 20, 2021 · d95823d · d95823d
1 parent b0aa6a0
commit d95823d
Show file tree

Hide file tree

Showing 4 changed files with 75 additions and 1 deletion.
diff --git a/cfgmgr/buffermgr.cpp b/cfgmgr/buffermgr.cpp
@@ -30,6 +30,7 @@ BufferMgr::BufferMgr(DBConnector *cfgDb, DBConnector *applDb, string pg_lookup_f
         m_applBufferEgressProfileListTable(applDb, APP_BUFFER_PORT_EGRESS_PROFILE_LIST_NAME)
 {
     readPgProfileLookupFile(pg_lookup_file);
+    dynamic_buffer_model = false;
 }
 
 //# speed, cable, size,    xon,  xoff, threshold,  xon_offset
@@ -273,12 +274,62 @@ void BufferMgr::doBufferTableTask(Consumer &consumer, ProducerStateTable &applTa
     }
 }
 
+void BufferMgr::doBufferMetaTask(Consumer &consumer)
+{
+    SWSS_LOG_ENTER();
+
+    auto it = consumer.m_toSync.begin();
+    while (it != consumer.m_toSync.end())
+    {
+        KeyOpFieldsValuesTuple t = it->second;
+        string key = kfvKey(t);
+
+        string op = kfvOp(t);
+        if (op == SET_COMMAND)
+        {
+            vector<FieldValueTuple> fvVector;
+
+            for (auto i : kfvFieldsValues(t))
+            {
+                if (fvField(i) == "buffer_model")
+                {
+                    if (fvValue(i) == "dynamic")
+                    {
+                        dynamic_buffer_model = true;
+                    }
+                    else
+                    {
+                        dynamic_buffer_model = false;
+                    }
+                    break;
+                }
+            }
+        }
+        else if (op == DEL_COMMAND)
+        {
+            dynamic_buffer_model = false;
+        }
+        it = consumer.m_toSync.erase(it);
+    }
+}
+
 void BufferMgr::doTask(Consumer &consumer)
 {
     SWSS_LOG_ENTER();
 
     string table_name = consumer.getTableName();
 
+    if (table_name == CFG_DEVICE_METADATA_TABLE_NAME)
+    {
+        doBufferMetaTask(consumer);
+        return;
+    }
+
+    if (dynamic_buffer_model)
+    {
+         SWSS_LOG_DEBUG("Dynamic buffer model enabled. Skipping further processing");
+         return;
+    }
     if (table_name == CFG_BUFFER_POOL_TABLE_NAME)
     {
         doBufferTableTask(consumer, m_applBufferPoolTable);

diff --git a/cfgmgr/buffermgr.h b/cfgmgr/buffermgr.h
@@ -50,6 +50,7 @@ class BufferMgr : public Orch
     ProducerStateTable m_applBufferEgressProfileListTable;
 
     bool m_pgfile_processed;
+    bool dynamic_buffer_model;
 
     pg_profile_lookup_t m_pgProfileLookup;
     port_cable_length_t m_cableLenLookup;
@@ -63,6 +64,7 @@ class BufferMgr : public Orch
     void transformSeperator(std::string &name);
 
     void doTask(Consumer &consumer);
+    void doBufferMetaTask(Consumer &consumer);
 };
 
 }

diff --git a/cfgmgr/buffermgrd.cpp b/cfgmgr/buffermgrd.cpp
@@ -195,7 +195,8 @@ int main(int argc, char **argv)
                 CFG_BUFFER_PG_TABLE_NAME,
                 CFG_BUFFER_QUEUE_TABLE_NAME,
                 CFG_BUFFER_PORT_INGRESS_PROFILE_LIST_NAME,
-                CFG_BUFFER_PORT_EGRESS_PROFILE_LIST_NAME
+                CFG_BUFFER_PORT_EGRESS_PROFILE_LIST_NAME,
+                CFG_DEVICE_METADATA_TABLE_NAME
             };
             cfgOrchList.emplace_back(new BufferMgr(&cfgDb, &applDb, pg_lookup_file, cfg_buffer_tables));
         }

diff --git a/tests/test_buffer_mode.py b/tests/test_buffer_mode.py
@@ -1,7 +1,27 @@
 import pytest
+import time
 
 class TestBufferModel(object):
     def test_bufferModel(self, dvs, testlog):
         config_db = dvs.get_config_db()
         metadata = config_db.get_entry("DEVICE_METADATA", "localhost")
         assert metadata["buffer_model"] == "traditional"
+
+    def test_update_bufferModel(self, dvs, testlog):
+        config_db = dvs.get_config_db()
+        app_db = dvs.get_app_db()
+        keys = app_db.get_keys("BUFFER_POOL_TABLE")
+        num_keys =  len(keys)
+
+        try:
+            fvs = {'buffer_model' : 'dynamic'}
+            config_db.update_entry("DEVICE_METADATA", "localhost", fvs)
+            fvs = {'mode':'dynamic', 'type':'egress'}
+            config_db.update_entry("BUFFER_POOL", "temp_pool", fvs)
+            time.sleep(2)
+            app_db.wait_for_n_keys("BUFFER_POOL_TABLE", num_keys)
+
+        finally:
+            config_db.delete_entry("BUFFER_POOL", "temp_pool")
+            fvs = {'buffer_model' : 'traditional'}
+            config_db.update_entry("DEVICE_METADATA", "localhost", fvs)