From ea34b92b37c46b45b7aeba3efe1da605ff2c95d8 Mon Sep 17 00:00:00 2001
From: Andriy Moroz <c_andriym@mellanox.com>
Date: Tue, 24 Apr 2018 18:51:47 +0300
Subject: [PATCH] Fix tables handling race condition in buffermgr (#484)

* Fix tables handling race condition in buffermgr

Signed-off-by: Andriy Moroz <c_andriym@mellanox.com>

* Fixed status lose in loop

Signed-off-by: Andriy Moroz <c_andriym@mellanox.com>
---
 cfgmgr/buffermgr.cpp | 40 +++++++++++++++++++++++++++++-----------
 cfgmgr/buffermgr.h   |  4 ++--
 2 files changed, 31 insertions(+), 13 deletions(-)

diff --git a/cfgmgr/buffermgr.cpp b/cfgmgr/buffermgr.cpp
index 6e396700f350..eda11819892d 100644
--- a/cfgmgr/buffermgr.cpp
+++ b/cfgmgr/buffermgr.cpp
@@ -72,9 +72,10 @@ void BufferMgr::readPgProfileLookupFile(string file)
     infile.close();
 }
 
-void BufferMgr::doCableTask(string port, string cable_length)
+task_process_status BufferMgr::doCableTask(string port, string cable_length)
 {
     m_cableLenLookup[port] = cable_length;
+    return task_process_status::task_success;
 }
 
 string BufferMgr::getPgPoolMode()
@@ -108,15 +109,15 @@ Create/update two tables: profile (in m_cfgBufferProfileTable) and port buffer (
         }
     }
 */
-void BufferMgr::doSpeedUpdateTask(string port, string speed)
+task_process_status BufferMgr::doSpeedUpdateTask(string port, string speed)
 {
     vector<FieldValueTuple> fvVector;
     string cable;
 
     if (m_cableLenLookup.count(port) == 0)
     {
-        SWSS_LOG_ERROR("Unable to create/update PG profile for port %s. Cable length is not set", port.c_str());
-        return;
+        SWSS_LOG_WARN("Unable to create/update PG profile for port %s. Cable length is not set", port.c_str());
+        return task_process_status::task_need_retry;
     }
 
     cable = m_cableLenLookup[port];
@@ -125,7 +126,7 @@ void BufferMgr::doSpeedUpdateTask(string port, string speed)
     {
         SWSS_LOG_ERROR("Unable to create/update PG profile for port %s. No PG profile configured for speed %s and cable length %s",
                        port.c_str(), speed.c_str(), cable.c_str());
-        return;
+        return task_process_status::task_failed;
     }
 
     // Crete record in BUFFER_PROFILE table
@@ -143,8 +144,8 @@ void BufferMgr::doSpeedUpdateTask(string port, string speed)
         if (mode.empty())
         {
             // this should never happen if switch initialized properly
-            SWSS_LOG_ERROR("PG lossless pool is not yet created");
-            return;
+            SWSS_LOG_WARN("PG lossless pool is not yet created");
+            return task_process_status::task_need_retry;
         }
 
         // profile threshold field name
@@ -170,6 +171,7 @@ void BufferMgr::doSpeedUpdateTask(string port, string speed)
     string profile_ref = string("[") + CFG_BUFFER_PROFILE_TABLE_NAME + CONFIGDB_TABLE_NAME_SEPARATOR + buffer_profile_key + "]";
     fvVector.push_back(make_pair("profile", profile_ref));
     m_cfgBufferPgTable.set(buffer_pg_key, fvVector);
+    return task_process_status::task_success;
 }
 
 void BufferMgr::doTask(Consumer &consumer)
@@ -188,6 +190,7 @@ void BufferMgr::doTask(Consumer &consumer)
         string port(keys[0]);
 
         string op = kfvOp(t);
+        task_process_status task_status = task_process_status::task_success;
         if (op == SET_COMMAND)
         {
             for (auto i : kfvFieldsValues(t))
@@ -195,18 +198,33 @@ void BufferMgr::doTask(Consumer &consumer)
                 if (table_name == CFG_PORT_CABLE_LEN_TABLE_NAME)
                 {
                     // receive and cache cable length table
-                    doCableTask(fvField(i), fvValue(i));
+                    task_status = doCableTask(fvField(i), fvValue(i));
                 }
                 // In case of PORT table update, Buffer Manager is interested in speed update only
                 if (table_name == CFG_PORT_TABLE_NAME && fvField(i) == "speed")
                 {
                     // create/update profile for port
-                    doSpeedUpdateTask(port, fvValue(i));
+                    task_status = doSpeedUpdateTask(port, fvValue(i));
+                }
+                if (task_status != task_process_status::task_success)
+                {
+                    break;
                 }
             }
         }
 
-        it = consumer.m_toSync.erase(it);
-        continue;
+        switch (task_status)
+        {
+            case task_process_status::task_failed:
+                SWSS_LOG_ERROR("Failed to process table update");
+                return;
+            case task_process_status::task_need_retry:
+                SWSS_LOG_INFO("Unable to process table update. Will retry...");
+                ++it;
+                break;
+            default:
+                it = consumer.m_toSync.erase(it);
+                break;
+        }
     }
 }
diff --git a/cfgmgr/buffermgr.h b/cfgmgr/buffermgr.h
index 9263aa6a4f15..3c00988e65ad 100644
--- a/cfgmgr/buffermgr.h
+++ b/cfgmgr/buffermgr.h
@@ -44,8 +44,8 @@ class BufferMgr : public Orch
     port_cable_length_t m_cableLenLookup;
     std::string getPgPoolMode();
     void readPgProfileLookupFile(std::string);
-    void doCableTask(string port, string cable_length);
-    void doSpeedUpdateTask(string port, string speed);
+    task_process_status doCableTask(string port, string cable_length);
+    task_process_status doSpeedUpdateTask(string port, string speed);
 
     void doTask(Consumer &consumer);
 };