[watermarkorch] only perform periodic clear if the polling is on (sonic-net#781)

* [watermarkorch] only perform periodic clear if the polling is on

Signed-off-by: Mykola Faryma <[email protected]>

* [watermarkorch] only set timer interval on change

Signed-off-by: Mykola Faryma <[email protected]>

*  fix logic

Signed-off-by: Mykola Faryma <[email protected]>

* fix comments

Signed-off-by: Mykola Faryma <[email protected]>

* fix change timer

Signed-off-by: Mykola Faryma <[email protected]>

Author: mykolaf

orchagent/orchdaemon.cpp

@@ -148,7 +148,12 @@ bool OrchDaemon::init()
         CFG_DTEL_EVENT_TABLE_NAME
     };
 
-    WatermarkOrch *wm_orch = new WatermarkOrch(m_configDb, CFG_WATERMARK_TABLE_NAME);
+    vector<string> wm_tables = {
+        CFG_WATERMARK_TABLE_NAME,
+        CFG_FLEX_COUNTER_TABLE_NAME
+    };
+
+    WatermarkOrch *wm_orch = new WatermarkOrch(m_configDb, wm_tables);
 
     /*
      * The order of the orch list is important for state restore of warm start and

orchagent/watermarkorch.cpp

@@ -14,8 +14,8 @@
 extern PortsOrch *gPortsOrch;
 
 
-WatermarkOrch::WatermarkOrch(DBConnector *db, const string tableName):
-    Orch(db, tableName)
+WatermarkOrch::WatermarkOrch(DBConnector *db, const vector<string> &tables):
+    Orch(db, tables)
 {
     SWSS_LOG_ENTER();
 
@@ -36,9 +36,6 @@ WatermarkOrch::WatermarkOrch(DBConnector *db, const string tableName):
     m_telemetryTimer = new SelectableTimer(intervT);
     auto executorT = new ExecutableTimer(m_telemetryTimer, this, "WM_TELEMETRY_TIMER");
     Orch::addExecutor(executorT);
-    m_telemetryTimer->start();
-
-    m_telemetryInterval = DEFAULT_TELEMETRY_INTERVAL;
 }
 
 WatermarkOrch::~WatermarkOrch()
@@ -66,19 +63,13 @@ void WatermarkOrch::doTask(Consumer &consumer)
 
         if (op == SET_COMMAND)
         {
-            if (key == "TELEMETRY_INTERVAL")
+            if (consumer.getTableName() == CFG_WATERMARK_TABLE_NAME)
             {
-                for (std::pair<std::basic_string<char>, std::basic_string<char> > i: fvt)
-                {
-                    if (i.first == "interval")
-                    {
-                        m_telemetryInterval = to_uint<uint32_t>(i.second.c_str());
-                    }
-                    else
-                    {
-                        SWSS_LOG_WARN("Unsupported key: %s", i.first.c_str());
-                    }
-                }
+                handleWmConfigUpdate(key, fvt);
+            }
+            else if (consumer.getTableName() == CFG_FLEX_COUNTER_TABLE_NAME)
+            {
+                handleFcConfigUpdate(key, fvt);
             }
         }
         else if (op == DEL_COMMAND)
@@ -94,13 +85,74 @@ void WatermarkOrch::doTask(Consumer &consumer)
     }
 }
 
+void WatermarkOrch::handleWmConfigUpdate(const std::string &key, const std::vector<FieldValueTuple> &fvt)
+{
+    SWSS_LOG_ENTER();    
+    if (key == "TELEMETRY_INTERVAL")
+    {
+        for (std::pair<std::basic_string<char>, std::basic_string<char> > i: fvt)
+        {
+            if (i.first == "interval")
+            {
+                auto intervT = timespec { .tv_sec = to_uint<uint32_t>(i.second.c_str()) , .tv_nsec = 0 };
+                m_telemetryTimer->setInterval(intervT);
+                // reset the timer interval when current timer expires
+                m_timerChanged = true;
+            }
+            else
+            {
+                SWSS_LOG_WARN("Unsupported key: %s", i.first.c_str());
+            }
+        }
+    }
+}
+
+void WatermarkOrch::handleFcConfigUpdate(const std::string &key, const std::vector<FieldValueTuple> &fvt)
+{
+    SWSS_LOG_ENTER();
+    uint8_t prevStatus = m_wmStatus;
+    if (key == "QUEUE_WATERMARK" || key == "PG_WATERMARK")
+    {
+        for (std::pair<std::basic_string<char>, std::basic_string<char> > i: fvt)
+        {
+            if (i.first == "FLEX_COUNTER_STATUS")
+            {
+                if (i.second == "enable")
+                {
+                    m_wmStatus = (uint8_t) (m_wmStatus | groupToMask.at(key));
+                }
+                else if (i.second == "disable")
+                {
+                    m_wmStatus = (uint8_t) (m_wmStatus & ~(groupToMask.at(key)));
+                }
+            }
+        }
+        if (!prevStatus && m_wmStatus)
+        {
+            m_telemetryTimer->start();
+        }
+    SWSS_LOG_DEBUG("Status of WMs: %u", m_wmStatus);
+    }
+}
+
 void WatermarkOrch::doTask(NotificationConsumer &consumer)
 {
+    SWSS_LOG_ENTER();
     if (!gPortsOrch->isPortReady())
     {
         return;
     }
 
+    if (m_pg_ids.empty())
+    {
+        init_pg_ids();
+    }
+
+    if (m_multicast_queue_ids.empty() and m_unicast_queue_ids.empty())
+    {
+        init_queue_ids();
+    }
+
     std::string op;
     std::string data;
     std::vector<swss::FieldValueTuple> values;
@@ -170,16 +222,21 @@ void WatermarkOrch::doTask(SelectableTimer &timer)
 
     if (&timer == m_telemetryTimer)
     {
-        /* If the interval was changed */
-        auto intervT = timespec { .tv_sec = m_telemetryInterval , .tv_nsec = 0 };
-        m_telemetryTimer->setInterval(intervT);
-        m_telemetryTimer->reset();
+        if (m_timerChanged)
+        {
+            m_telemetryTimer->reset();
+            m_timerChanged = false;
+        }
+        if (!m_wmStatus)
+        {
+            m_telemetryTimer->stop();
+        }
 
         clearSingleWm(m_periodicWatermarkTable.get(), "SAI_INGRESS_PRIORITY_GROUP_STAT_XOFF_ROOM_WATERMARK_BYTES", m_pg_ids);
         clearSingleWm(m_periodicWatermarkTable.get(), "SAI_INGRESS_PRIORITY_GROUP_STAT_SHARED_WATERMARK_BYTES", m_pg_ids);
         clearSingleWm(m_periodicWatermarkTable.get(), "SAI_QUEUE_STAT_SHARED_WATERMARK_BYTES", m_unicast_queue_ids);
         clearSingleWm(m_periodicWatermarkTable.get(), "SAI_QUEUE_STAT_SHARED_WATERMARK_BYTES", m_multicast_queue_ids);
-        SWSS_LOG_INFO("Periodic watermark cleared by timer!");
+        SWSS_LOG_DEBUG("Periodic watermark cleared by timer!");
     }
 }
 

orchagent/watermarkorch.h

@@ -1,17 +1,27 @@
 #ifndef WATERMARKORCH_H
 #define WATERMARKORCH_H
 
+#include <map>
+
 #include "orch.h"
 #include "port.h"
 
 #include "notificationconsumer.h"
 #include "timer.h"
 
+const uint8_t queue_wm_status_mask = 1 << 0;
+const uint8_t pg_wm_status_mask = 1 << 1;
+
+static const map<string, const uint8_t> groupToMask =
+{
+    { "QUEUE_WATERMARK",     queue_wm_status_mask },
+    { "PG_WATERMARK",        pg_wm_status_mask }
+};
 
 class WatermarkOrch : public Orch
 {
 public:
-    WatermarkOrch(DBConnector *db, const std::string tableName);
+    WatermarkOrch(DBConnector *db, const vector<string> &tables);
     virtual ~WatermarkOrch(void);
 
     void doTask(Consumer &consumer);
@@ -21,6 +31,9 @@ class WatermarkOrch : public Orch
     void init_pg_ids();
     void init_queue_ids();
 
+    void handleWmConfigUpdate(const std::string &key, const std::vector<FieldValueTuple> &fvt);
+    void handleFcConfigUpdate(const std::string &key, const std::vector<FieldValueTuple> &fvt);
+
     void clearSingleWm(Table *table, string wm_name, vector<sai_object_id_t> &obj_ids);
 
     shared_ptr<Table> getCountersTable(void)
@@ -34,6 +47,14 @@ class WatermarkOrch : public Orch
     }
 
 private:
+    /*
+    [7-2] - unused
+    [1] - pg wm status
+    [0] - queue wm status (least significant bit) 
+    */
+    uint8_t m_wmStatus = 0;
+    bool m_timerChanged = false;
+
     shared_ptr<DBConnector> m_countersDb = nullptr;
     shared_ptr<DBConnector> m_appDb = nullptr;
     shared_ptr<Table> m_countersTable = nullptr;
@@ -47,8 +68,6 @@ class WatermarkOrch : public Orch
     vector<sai_object_id_t> m_unicast_queue_ids;
     vector<sai_object_id_t> m_multicast_queue_ids;
     vector<sai_object_id_t> m_pg_ids;
-
-    int m_telemetryInterval;
 };
 
 #endif // WATERMARKORCH_H