[portsorch] Refactor portsorch to use FlexCounterManager to setup por…

…t and queue stats (sonic-net#1170) - Updates portsorch to use FlexCounterManager for port and queue stats instead of direct redis operations - Updates FlexCounterManager to allow clients to enable the group in the constructor, for convenience - Updates the makefile for cfgmgr to include new flex_counter dependency from portsorch Signed-off-by: Danny Allen <[email protected]>
kktheballer · Dec 23, 2020 · f11ccfc · f11ccfc
1 parent 6f3e10d
commit f11ccfc
Show file tree

Hide file tree

Showing 8 changed files with 66 additions and 59 deletions.
diff --git a/cfgmgr/Makefile.am b/cfgmgr/Makefile.am
@@ -1,4 +1,4 @@
-INCLUDES = -I $(top_srcdir) -I $(top_srcdir)/orchagent -I $(top_srcdir)/warmrestart
+INCLUDES = -I $(top_srcdir) -I $(top_srcdir)/orchagent -I $(top_srcdir)/warmrestart -I $(top_srcdir)/orchagent/flex_counter
 CFLAGS_SAI = -I /usr/include/sai
 LIBNL_CFLAGS = -I/usr/include/libnl3
 LIBNL_LIBS = -lnl-genl-3 -lnl-route-3 -lnl-3

diff --git a/orchagent/debugcounterorch.cpp b/orchagent/debugcounterorch.cpp
@@ -25,7 +25,7 @@ static const unordered_map<string, CounterType> flex_counter_type_lookup = {
 // object should only be initialized once.
 DebugCounterOrch::DebugCounterOrch(DBConnector *db, const vector<string>& table_names, int poll_interval) :
         Orch(db, table_names),
-        flex_counter_manager(DEBUG_COUNTER_FLEX_COUNTER_GROUP, StatsMode::READ, poll_interval),
+        flex_counter_manager(DEBUG_COUNTER_FLEX_COUNTER_GROUP, StatsMode::READ, poll_interval, true),
         m_stateDb(new DBConnector("STATE_DB", 0)),
         m_debugCapabilitiesTable(new Table(m_stateDb.get(), STATE_DEBUG_COUNTER_CAPABILITIES_NAME)),
         m_countersDb(new DBConnector("COUNTERS_DB", 0)),
@@ -34,7 +34,6 @@ DebugCounterOrch::DebugCounterOrch(DBConnector *db, const vector<string>& table_
 {
     SWSS_LOG_ENTER();
     publishDropCounterCapabilities();
-    flex_counter_manager.enableFlexCounterGroup();
 }
 
 DebugCounterOrch::~DebugCounterOrch(void)

diff --git a/orchagent/flex_counter/flex_counter_manager.cpp b/orchagent/flex_counter/flex_counter_manager.cpp
@@ -34,17 +34,19 @@ const unordered_map<CounterType, string> FlexCounterManager::counter_id_field_lo
 {
     { CounterType::PORT_DEBUG,   PORT_DEBUG_COUNTER_ID_LIST },
     { CounterType::SWITCH_DEBUG, SWITCH_DEBUG_COUNTER_ID_LIST },
+    { CounterType::PORT,         PORT_COUNTER_ID_LIST },
+    { CounterType::QUEUE,        QUEUE_COUNTER_ID_LIST }
 };
 
-// This constructor will create a group that is disabled by default.
 FlexCounterManager::FlexCounterManager(
         const string& group_name,
         const StatsMode stats_mode,
-        const uint polling_interval) :
+        const uint polling_interval,
+        const bool enabled) :
     group_name(group_name),
     stats_mode(stats_mode),
     polling_interval(polling_interval),
-    enabled(false),
+    enabled(enabled),
     flex_counter_db(new DBConnector("FLEX_COUNTER_DB", 0)),
     flex_counter_group_table(new ProducerTable(flex_counter_db.get(), FLEX_COUNTER_GROUP_TABLE)),
     flex_counter_table(new ProducerTable(flex_counter_db.get(), FLEX_COUNTER_TABLE))
@@ -72,6 +74,8 @@ FlexCounterManager::~FlexCounterManager()
 
 void FlexCounterManager::applyGroupConfiguration()
 {
+    SWSS_LOG_ENTER();
+
     vector<FieldValueTuple> field_values =
     {
         FieldValueTuple(STATS_MODE_FIELD, stats_mode_lookup.at(stats_mode)),

diff --git a/orchagent/flex_counter/flex_counter_manager.h b/orchagent/flex_counter/flex_counter_manager.h
@@ -18,6 +18,8 @@ enum class StatsMode
 
 enum class CounterType
 {
+    PORT,
+    QUEUE,
     PORT_DEBUG,
     SWITCH_DEBUG
 };
@@ -33,7 +35,8 @@ class FlexCounterManager
         FlexCounterManager(
                 const std::string& group_name,
                 const StatsMode stats_mode,
-                const uint polling_interval);
+                const uint polling_interval,
+                const bool enabled);
 
         FlexCounterManager(const FlexCounterManager&) = delete;
         FlexCounterManager& operator=(const FlexCounterManager&) = delete;

diff --git a/orchagent/flex_counter/flex_counter_stat_manager.cpp b/orchagent/flex_counter/flex_counter_stat_manager.cpp
@@ -13,8 +13,9 @@ using swss::FieldValueTuple;
 FlexCounterStatManager::FlexCounterStatManager(
         const string& group_name,
         const StatsMode stats_mode,
-        const int polling_interval) :
-    FlexCounterManager(group_name, stats_mode, polling_interval)
+        const int polling_interval,
+        const bool enabled) :
+    FlexCounterManager(group_name, stats_mode, polling_interval, enabled)
 {
     SWSS_LOG_ENTER();
 }

diff --git a/orchagent/flex_counter/flex_counter_stat_manager.h b/orchagent/flex_counter/flex_counter_stat_manager.h
@@ -11,7 +11,8 @@ class FlexCounterStatManager : public FlexCounterManager
         FlexCounterStatManager(
                 const std::string& group_name,
                 const StatsMode stats_mode,
-                const int polling_interval);
+                const int polling_interval,
+                const bool enabled);
 
         FlexCounterStatManager(const FlexCounterStatManager&) = delete;
         FlexCounterStatManager& operator=(const FlexCounterStatManager&) = delete;

diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp
@@ -11,6 +11,7 @@
 #include <algorithm>
 #include <tuple>
 #include <sstream>
+#include <unordered_set>
 
 #include <netinet/if_ether.h>
 #include "net/if.h"
@@ -47,8 +48,11 @@ extern FdbOrch *gFdbOrch;
 #define PORT_FLEX_STAT_COUNTER_POLL_MSECS "1000"
 #define PORT_BUFFER_DROP_STAT_POLLING_INTERVAL_MS     "60000"
 #define QUEUE_FLEX_STAT_COUNTER_POLL_MSECS "10000"
+
+#define PORT_STAT_FLEX_COUNTER_POLLING_INTERVAL_MS     1000
+#define QUEUE_STAT_FLEX_COUNTER_POLLING_INTERVAL_MS   10000
 #define QUEUE_WATERMARK_FLEX_STAT_COUNTER_POLL_MSECS "10000"
-#define PG_WATERMARK_FLEX_STAT_COUNTER_POLL_MSECS "10000"
+#define PG_WATERMARK_FLEX_STAT_COUNTER_POLL_MSECS    "10000"
 
 
 static map<string, sai_port_fec_mode_t> fec_mode_map =
@@ -74,7 +78,7 @@ static map<string, sai_bridge_port_fdb_learning_mode_t> learn_mode_map =
     { "notification", SAI_BRIDGE_PORT_FDB_LEARNING_MODE_FDB_NOTIFICATION}
 };
 
-const vector<sai_port_stat_t> portStatIds =
+const vector<sai_port_stat_t> port_stat_ids =
 {
     SAI_PORT_STAT_IF_IN_OCTETS,
     SAI_PORT_STAT_IF_IN_UCAST_PKTS,
@@ -117,6 +121,7 @@ const vector<sai_port_stat_t> portStatIds =
     SAI_PORT_STAT_ETHER_IN_PKTS_128_TO_255_OCTETS,
 };
 
+
 static const vector<sai_port_stat_t> port_buffer_drop_stat_ids =
 {
     SAI_PORT_STAT_IN_DROPPED_PKTS,
@@ -131,6 +136,14 @@ static const vector<sai_queue_stat_t> queueStatIds =
     SAI_QUEUE_STAT_DROPPED_BYTES,
 };
 
+static const vector<sai_queue_stat_t> queue_stat_ids =
+{
+    SAI_QUEUE_STAT_PACKETS,
+    SAI_QUEUE_STAT_BYTES,
+    SAI_QUEUE_STAT_DROPPED_PACKETS,
+    SAI_QUEUE_STAT_DROPPED_BYTES,
+};
+
 static const vector<sai_queue_stat_t> queueWatermarkStatIds =
 {
     SAI_QUEUE_STAT_SHARED_WATERMARK_BYTES,
@@ -161,7 +174,9 @@ static char* hostif_vlan_tag[] = {
  *    default VLAN and all ports removed from .1Q bridge.
  */
 PortsOrch::PortsOrch(DBConnector *db, vector<table_name_with_pri_t> &tableNames) :
-        Orch(db, tableNames)
+        Orch(db, tableNames),
+        port_stat_manager(PORT_STAT_COUNTER_FLEX_COUNTER_GROUP, StatsMode::READ, PORT_STAT_FLEX_COUNTER_POLLING_INTERVAL_MS, true),
+        queue_stat_manager(QUEUE_STAT_COUNTER_FLEX_COUNTER_GROUP, StatsMode::READ, QUEUE_STAT_FLEX_COUNTER_POLLING_INTERVAL_MS, true)
 {
     SWSS_LOG_ENTER();
 
@@ -1499,6 +1514,7 @@ bool PortsOrch::removePort(sai_object_id_t port_id)
     return true;
 }
 
+//candidate for omission
 string PortsOrch::getPortFlexCounterTableKey(string key)
 {
     return string(PORT_STAT_COUNTER_FLEX_COUNTER_GROUP) + ":" + key;
@@ -1508,11 +1524,15 @@ string PortsOrch::getPortBuffDropFlexCounterTableKey(string key)
 {
     return string(PORT_DROP_STAT_COUNTER_FLEX_COUNTER_GROUP) + ":" + key;
 }
+//candidate for omission
+
+//string PortsOrch::getQueueFlexCounterTableKey(string key)
+//{
+//    return string(QUEUE_STAT_COUNTER_FLEX_COUNTER_GROUP) + ":" + key;
+//}
+
+//candidate for omission
 
-string PortsOrch::getQueueFlexCounterTableKey(string key)
-{
-    return string(QUEUE_STAT_COUNTER_FLEX_COUNTER_GROUP) + ":" + key;
-}
 
 string PortsOrch::getQueueWatermarkFlexCounterTableKey(string key)
 {
@@ -1557,35 +1577,17 @@ bool PortsOrch::initPort(const string &alias, const set<int> &lane_set)
                 fields.push_back(tuple);
                 m_counterTable->set("", fields);
 
-                /* Add port to flex_counter for updating stat counters  */
-                string key = getPortFlexCounterTableKey(sai_serialize_object_id(p.m_port_id));
-                std::string delimiter = "";
-                std::ostringstream counters_stream;
-                for (const auto &id: portStatIds)
+                // Install a flex counter for this port to track stats
+                std::unordered_set<std::string> counter_stats;
+                for (const auto& it: port_stat_ids)
                 {
-                    counters_stream << delimiter << sai_serialize_port_stat(id);
-                    delimiter = comma;
+                    counter_stats.emplace(sai_serialize_port_stat(it));
                 }
+                port_stat_manager.setCounterIdList(p.m_port_id, CounterType::PORT, counter_stats);
 
-                fields.clear();
-                fields.emplace_back(PORT_COUNTER_ID_LIST, counters_stream.str());
-
-                m_flexCounterTable->set(key, fields);
-
-                delimiter = "";
-                string port_drop_key = getPortBuffDropFlexCounterTableKey(sai_serialize_object_id(p.m_port_id));
-                std::ostringstream port_buffer_drop_stream;
-                for (const auto& it: port_buffer_drop_stat_ids)
-                {
-                    port_buffer_drop_stream << delimiter << sai_serialize_port_stat(it);
-                    delimiter = comma;
-                }
 
-                fields.clear();
-                fields.emplace_back(PORT_COUNTER_ID_LIST, port_buffer_drop_stream.str());
-                m_flexCounterTable->set(port_drop_key, fields);
+                PortUpdate update = { p, true };
 
-                PortUpdate update = {p, true };
                 notify(SUBJECT_TYPE_PORT_CHANGE, static_cast<void *>(&update));
 
                 SWSS_LOG_NOTICE("Initialized port %s", alias.c_str());
@@ -3632,34 +3634,26 @@ void PortsOrch::generateQueueMapPerPort(const Port& port)
             queueIndexVector.emplace_back(id, to_string(queueRealIndex));
         }
 
-        /* add ordinary Queue stat counters */
-        string key = getQueueFlexCounterTableKey(id);
-
-        std::string delimiter = "";
-        std::ostringstream counters_stream;
-        for (const auto& it: queueStatIds)
+        // Install a flex counter for this queue to track stats
+        std::unordered_set<string> counter_stats;
+        for (const auto& it: queue_stat_ids)
         {
-            counters_stream << delimiter << sai_serialize_queue_stat(it);
-            delimiter = comma;
+            counter_stats.emplace(sai_serialize_queue_stat(it));
         }
-
-        vector<FieldValueTuple> fieldValues;
-        fieldValues.emplace_back(QUEUE_COUNTER_ID_LIST, counters_stream.str());
-
-        m_flexCounterTable->set(key, fieldValues);
+        queue_stat_manager.setCounterIdList(port.m_queue_ids[queueIndex], CounterType::QUEUE, counter_stats);
 
         /* add watermark queue counters */
-        key = getQueueWatermarkFlexCounterTableKey(id);
+        string key = getQueueWatermarkFlexCounterTableKey(id);
 
-        delimiter = "";
-        counters_stream.str("");
+        string delimiter("");
+        std::ostringstream counters_stream;
         for (const auto& it: queueWatermarkStatIds)
         {
             counters_stream << delimiter << sai_serialize_queue_stat(it);
             delimiter = comma;
         }
 
-        fieldValues.clear();
+        vector<FieldValueTuple> fieldValues;
         fieldValues.emplace_back(QUEUE_COUNTER_ID_LIST, counters_stream.str());
 
         m_flexCounterTable->set(key, fieldValues);

diff --git a/orchagent/portsorch.h b/orchagent/portsorch.h
@@ -9,6 +9,7 @@
 #include "observer.h"
 #include "macaddress.h"
 #include "producertable.h"
+#include "flex_counter_manager.h"
 
 #define FCS_LEN 4
 #define VLAN_TAG_LEN 4
@@ -106,15 +107,19 @@ class PortsOrch : public Orch, public Subject
     unique_ptr<ProducerTable> m_flexCounterTable;
     unique_ptr<ProducerTable> m_flexCounterGroupTable;
 
-    std::string getQueueFlexCounterTableKey(std::string s);
     std::string getQueueWatermarkFlexCounterTableKey(std::string s);
+    //candidate for omission
     std::string getPortFlexCounterTableKey(std::string s);
     std::string getPortBuffDropFlexCounterTableKey(std::string s);
+    //candidate for omission
     std::string getPriorityGroupWatermarkFlexCounterTableKey(std::string s);
 
     shared_ptr<DBConnector> m_counter_db;
     shared_ptr<DBConnector> m_flex_db;
 
+    FlexCounterManager port_stat_manager;
+    FlexCounterManager queue_stat_manager;
+
     std::map<sai_object_id_t, PortSupportedSpeeds> m_portSupportedSpeeds;
 
     bool m_initDone = false;