Fix flex counter out-of-order issue by notifying counter operations using SelectableChannel (#3076)

* Fix flow counter sequence issue using redis SAI switch API enable counter polling and create counter group
SAI switch API is invoked to enable counter polling and create counter group
CLI option to choose whether the new infra should be used

Author: stephenxs

orchagent/bufferorch.cpp

@@ -23,9 +23,6 @@ extern string gMySwitchType;
 extern string gMyHostName;
 extern string gMyAsicName;
 
-#define BUFFER_POOL_WATERMARK_FLEX_STAT_COUNTER_POLL_MSECS  "60000"
-
-
 static const vector<sai_buffer_pool_stat_t> bufferPoolWatermarkStatIds =
 {
     SAI_BUFFER_POOL_STAT_WATERMARK_BYTES,
@@ -52,9 +49,6 @@ std::map<string, std::map<size_t, string>> queue_port_flags;
 
 BufferOrch::BufferOrch(DBConnector *applDb, DBConnector *confDb, DBConnector *stateDb, vector<string> &tableNames) :
     Orch(applDb, tableNames),
-    m_flexCounterDb(new DBConnector("FLEX_COUNTER_DB", 0)),
-    m_flexCounterTable(new ProducerTable(m_flexCounterDb.get(), FLEX_COUNTER_TABLE)),
-    m_flexCounterGroupTable(new ProducerTable(m_flexCounterDb.get(), FLEX_COUNTER_GROUP_TABLE)),
     m_countersDb(new DBConnector("COUNTERS_DB", 0)),
     m_stateBufferMaximumValueTable(stateDb, STATE_BUFFER_MAXIMUM_VALUE_TABLE)
 {
@@ -229,22 +223,23 @@ void BufferOrch::initBufferConstants()
 void BufferOrch::initFlexCounterGroupTable(void)
 {
     string bufferPoolWmPluginName = "watermark_bufferpool.lua";
+    string bufferPoolWmSha;
 
     try
     {
         string bufferPoolLuaScript = swss::loadLuaScript(bufferPoolWmPluginName);
-        string bufferPoolWmSha = swss::loadRedisScript(m_countersDb.get(), bufferPoolLuaScript);
-
-        vector<FieldValueTuple> fvTuples;
-        fvTuples.emplace_back(BUFFER_POOL_PLUGIN_FIELD, bufferPoolWmSha);
-        fvTuples.emplace_back(POLL_INTERVAL_FIELD, BUFFER_POOL_WATERMARK_FLEX_STAT_COUNTER_POLL_MSECS);
-
-        m_flexCounterGroupTable->set(BUFFER_POOL_WATERMARK_STAT_COUNTER_FLEX_COUNTER_GROUP, fvTuples);
+        bufferPoolWmSha = swss::loadRedisScript(m_countersDb.get(), bufferPoolLuaScript);
     }
     catch (const runtime_error &e)
     {
         SWSS_LOG_ERROR("Buffer pool watermark lua script and/or flex counter group not set successfully. Runtime error: %s", e.what());
     }
+
+    setFlexCounterGroupParameter(BUFFER_POOL_WATERMARK_STAT_COUNTER_FLEX_COUNTER_GROUP,
+                                 BUFFER_POOL_WATERMARK_FLEX_STAT_COUNTER_POLL_MSECS,
+                                 "", // do not touch stats_mode
+                                 BUFFER_POOL_PLUGIN_FIELD,
+                                 bufferPoolWmSha);
 }
 
 bool BufferOrch::isPortReady(const std::string& port_name) const
@@ -275,7 +270,7 @@ void BufferOrch::clearBufferPoolWatermarkCounterIdList(const sai_object_id_t obj
     if (m_isBufferPoolWatermarkCounterIdListGenerated)
     {
         string key = BUFFER_POOL_WATERMARK_STAT_COUNTER_FLEX_COUNTER_GROUP ":" + sai_serialize_object_id(object_id);
-        m_flexCounterTable->del(key);
+        stopFlexCounterPolling(gSwitchId, key);
     }
 }
 
@@ -326,37 +321,32 @@ void BufferOrch::generateBufferPoolWatermarkCounterIdList(void)
 
     if (!noWmClrCapability)
     {
-        vector<FieldValueTuple> fvs;
-
-        fvs.emplace_back(STATS_MODE_FIELD, STATS_MODE_READ_AND_CLEAR);
-        m_flexCounterGroupTable->set(BUFFER_POOL_WATERMARK_STAT_COUNTER_FLEX_COUNTER_GROUP, fvs);
+        setFlexCounterGroupStatsMode(BUFFER_POOL_WATERMARK_STAT_COUNTER_FLEX_COUNTER_GROUP,
+                                     STATS_MODE_READ_AND_CLEAR);
     }
 
     // Push buffer pool watermark COUNTER_ID_LIST to FLEX_COUNTER_TABLE on a per buffer pool basis
-    vector<FieldValueTuple> fvTuples;
-    fvTuples.emplace_back(BUFFER_POOL_COUNTER_ID_LIST, statList);
+    string stats_mode;
+
     bitMask = 1;
+
     for (const auto &it : *(m_buffer_type_maps[APP_BUFFER_POOL_TABLE_NAME]))
     {
         string key = BUFFER_POOL_WATERMARK_STAT_COUNTER_FLEX_COUNTER_GROUP ":" + sai_serialize_object_id(it.second.m_saiObjectId);
 
+        stats_mode = "";
+
         if (noWmClrCapability)
         {
-            string stats_mode = STATS_MODE_READ_AND_CLEAR;
             if (noWmClrCapability & bitMask)
             {
                 stats_mode = STATS_MODE_READ;
             }
-            fvTuples.emplace_back(STATS_MODE_FIELD, stats_mode);
 
-            m_flexCounterTable->set(key, fvTuples);
-            fvTuples.pop_back();
             bitMask <<= 1;
         }
-        else
-        {
-            m_flexCounterTable->set(key, fvTuples);
-        }
+
+        startFlexCounterPolling(gSwitchId, key, statList, BUFFER_POOL_COUNTER_ID_LIST, stats_mode);
     }
 
     m_isBufferPoolWatermarkCounterIdListGenerated = true;

orchagent/bufferorch.h

@@ -9,6 +9,7 @@
 #include "redisapi.h"
 
 #define BUFFER_POOL_WATERMARK_STAT_COUNTER_FLEX_COUNTER_GROUP "BUFFER_POOL_WATERMARK_STAT_COUNTER"
+#define BUFFER_POOL_WATERMARK_FLEX_STAT_COUNTER_POLL_MSECS  "60000"
 
 const string buffer_size_field_name         = "size";
 const string buffer_pool_type_field_name    = "type";
@@ -63,10 +64,6 @@ class BufferOrch : public Orch
     std::unordered_map<std::string, bool> m_ready_list;
     std::unordered_map<std::string, std::vector<std::string>> m_port_ready_list_ref;
 
-    unique_ptr<DBConnector> m_flexCounterDb;
-    unique_ptr<ProducerTable> m_flexCounterGroupTable;
-    unique_ptr<ProducerTable> m_flexCounterTable;
-
     Table m_stateBufferMaximumValueTable;
 
     unique_ptr<DBConnector> m_countersDb;

orchagent/copporch.cpp

@@ -26,6 +26,7 @@ extern sai_object_id_t      gSwitchId;
 extern PortsOrch*           gPortsOrch;
 extern Directory<Orch*>     gDirectory;
 extern bool                 gIsNatSupported;
+extern bool                 gTraditionalFlexCounter;
 
 #define FLEX_COUNTER_UPD_INTERVAL 1
 
@@ -126,11 +127,9 @@ const uint HOSTIF_TRAP_COUNTER_POLLING_INTERVAL_MS = 10000;
 CoppOrch::CoppOrch(DBConnector* db, string tableName) :
     Orch(db, tableName),
     m_counter_db(std::shared_ptr<DBConnector>(new DBConnector("COUNTERS_DB", 0))),
-    m_flex_db(std::shared_ptr<DBConnector>(new DBConnector("FLEX_COUNTER_DB", 0))),
     m_asic_db(std::shared_ptr<DBConnector>(new DBConnector("ASIC_DB", 0))),
     m_counter_table(std::unique_ptr<Table>(new Table(m_counter_db.get(), COUNTERS_TRAP_NAME_MAP))),
     m_vidToRidTable(std::unique_ptr<Table>(new Table(m_asic_db.get(), "VIDTORID"))),
-    m_flex_counter_group_table(std::unique_ptr<ProducerTable>(new ProducerTable(m_flex_db.get(), FLEX_COUNTER_GROUP_TABLE))),
     m_trap_counter_manager(HOSTIF_TRAP_COUNTER_FLEX_COUNTER_GROUP, StatsMode::READ, HOSTIF_TRAP_COUNTER_POLLING_INTERVAL_MS, false)
 {
     SWSS_LOG_ENTER();
@@ -772,7 +771,7 @@ void CoppOrch::doTask(SelectableTimer &timer)
     for (auto it = m_pendingAddToFlexCntr.begin(); it != m_pendingAddToFlexCntr.end(); )
     {
         const auto id = sai_serialize_object_id(it->first);
-        if (m_vidToRidTable->hget("", id, value))
+        if (!gTraditionalFlexCounter || m_vidToRidTable->hget("", id, value))
         {
             SWSS_LOG_INFO("Registering %s, id %s", it->second.c_str(), id.c_str());
 
@@ -1205,20 +1204,22 @@ void CoppOrch::initTrapRatePlugin()
     }
 
     std::string trapRatePluginName = "trap_rates.lua";
+    std::string trapSha;
     try
     {
         std::string trapLuaScript = swss::loadLuaScript(trapRatePluginName);
-        std::string trapSha = swss::loadRedisScript(m_counter_db.get(), trapLuaScript);
-
-        vector<FieldValueTuple> fieldValues;
-        fieldValues.emplace_back(FLOW_COUNTER_PLUGIN_FIELD, trapSha);
-        fieldValues.emplace_back(STATS_MODE_FIELD, STATS_MODE_READ);
-        m_flex_counter_group_table->set(HOSTIF_TRAP_COUNTER_FLEX_COUNTER_GROUP, fieldValues);
+        trapSha = swss::loadRedisScript(m_counter_db.get(), trapLuaScript);
     }
     catch (const runtime_error &e)
     {
         SWSS_LOG_ERROR("Trap flex counter groups were not set successfully: %s", e.what());
     }
+
+    setFlexCounterGroupParameter(HOSTIF_TRAP_COUNTER_FLEX_COUNTER_GROUP,
+                                 "", // Do not touch poll interval
+                                 STATS_MODE_READ,
+                                 FLOW_COUNTER_PLUGIN_FIELD,
+                                 trapSha);
     m_trap_rate_plugin_loaded = true;
 }
 

orchagent/copporch.h

@@ -98,11 +98,9 @@ class CoppOrch : public Orch
     std::map<sai_object_id_t, std::string> m_pendingAddToFlexCntr;
 
     std::shared_ptr<DBConnector> m_counter_db;
-    std::shared_ptr<DBConnector> m_flex_db;
     std::shared_ptr<DBConnector> m_asic_db;
     std::unique_ptr<Table> m_counter_table;
     std::unique_ptr<Table> m_vidToRidTable;
-    std::unique_ptr<ProducerTable> m_flex_counter_group_table;
 
     FlexCounterManager m_trap_counter_manager;
 

orchagent/fabricportsorch.cpp

@@ -84,8 +84,6 @@ FabricPortsOrch::FabricPortsOrch(DBConnector *appl_db, vector<table_name_with_pr
     m_portNamePortCounterTable = unique_ptr<Table>(new Table(m_counter_db.get(), COUNTERS_FABRIC_PORT_NAME_MAP));
     m_fabricCounterTable = unique_ptr<Table>(new Table(m_counter_db.get(), COUNTERS_TABLE));
 
-    m_flex_db = shared_ptr<DBConnector>(new DBConnector("FLEX_COUNTER_DB", 0));
-    m_flexCounterTable = unique_ptr<ProducerTable>(new ProducerTable(m_flex_db.get(), APP_FABRIC_PORT_TABLE_NAME));
     m_appl_db = shared_ptr<DBConnector>(new DBConnector("APPL_DB", 0));
     m_applTable = unique_ptr<Table>(new Table(m_appl_db.get(), APP_FABRIC_MONITOR_PORT_TABLE_NAME));
     m_applMonitorConstTable = unique_ptr<Table>(new Table(m_appl_db.get(), APP_FABRIC_MONITOR_DATA_TABLE_NAME));

orchagent/fabricportsorch.h

@@ -26,7 +26,6 @@ class FabricPortsOrch : public Orch, public Subject
 
     shared_ptr<DBConnector> m_state_db;
     shared_ptr<DBConnector> m_counter_db;
-    shared_ptr<DBConnector> m_flex_db;
     shared_ptr<DBConnector> m_appl_db;
 
     unique_ptr<Table> m_stateTable;

orchagent/flex_counter/flex_counter_manager.cpp

@@ -18,6 +18,10 @@ using swss::DBConnector;
 using swss::FieldValueTuple;
 using swss::ProducerTable;
 
+extern sai_switch_api_t *sai_switch_api;
+
+extern sai_object_id_t gSwitchId;
+
 const string FLEX_COUNTER_ENABLE("enable");
 const string FLEX_COUNTER_DISABLE("disable");
 
@@ -89,13 +93,13 @@ FlexCounterManager::FlexCounterManager(
         const uint polling_interval,
         const bool enabled,
         FieldValueTuple fv_plugin) :
-    FlexCounterManager("FLEX_COUNTER_DB", group_name, stats_mode,
+    FlexCounterManager(false, group_name, stats_mode,
             polling_interval, enabled, fv_plugin)
 {
 }
 
 FlexCounterManager::FlexCounterManager(
-        const string& db_name,
+        const bool is_gearbox,
         const string& group_name,
         const StatsMode stats_mode,
         const uint polling_interval,
@@ -106,11 +110,7 @@ FlexCounterManager::FlexCounterManager(
     polling_interval(polling_interval),
     enabled(enabled),
     fv_plugin(fv_plugin),
-    flex_counter_db(new DBConnector(db_name, 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))
+    is_gearbox(is_gearbox)
 {
     SWSS_LOG_ENTER();
 
@@ -125,13 +125,10 @@ FlexCounterManager::~FlexCounterManager()
 
     for (const auto& counter: installed_counters)
     {
-        flex_counter_table->del(getFlexCounterTableKey(group_name, counter));
+        stopFlexCounterPolling(counter.second, getFlexCounterTableKey(group_name, counter.first));
     }
 
-    if (flex_counter_group_table != nullptr)
-    {
-        flex_counter_group_table->del(group_name);
-    }
+    delFlexCounterGroup(group_name, is_gearbox);
 
     SWSS_LOG_DEBUG("Deleted flex counter group '%s'.", group_name.c_str());
 }
@@ -140,31 +137,21 @@ void FlexCounterManager::applyGroupConfiguration()
 {
     SWSS_LOG_ENTER();
 
-    vector<FieldValueTuple> field_values =
-    {
-        FieldValueTuple(STATS_MODE_FIELD, stats_mode_lookup.at(stats_mode)),
-        FieldValueTuple(POLL_INTERVAL_FIELD, std::to_string(polling_interval)),
-        FieldValueTuple(FLEX_COUNTER_STATUS_FIELD, status_lookup.at(enabled))
-    };
-
-    if (!fvField(fv_plugin).empty())
-    {
-        field_values.emplace_back(fv_plugin);
-    }
-
-    flex_counter_group_table->set(group_name, field_values);
+    setFlexCounterGroupParameter(group_name,
+                                 std::to_string(polling_interval),
+                                 stats_mode_lookup.at(stats_mode),
+                                 fvField(fv_plugin),
+                                 fvValue(fv_plugin),
+                                 status_lookup.at(enabled),
+                                 is_gearbox);
 }
 
 void FlexCounterManager::updateGroupPollingInterval(
         const uint polling_interval)
 {
     SWSS_LOG_ENTER();
 
-    vector<FieldValueTuple> field_values =
-    {
-        FieldValueTuple(POLL_INTERVAL_FIELD, std::to_string(polling_interval))
-    };
-    flex_counter_group_table->set(group_name, field_values);
+    setFlexCounterGroupPollInterval(group_name, std::to_string(polling_interval), is_gearbox);
 
     SWSS_LOG_DEBUG("Set polling interval for flex counter group '%s' to %d ms.",
             group_name.c_str(), polling_interval);
@@ -181,11 +168,7 @@ void FlexCounterManager::enableFlexCounterGroup()
         return;
     }
 
-    vector<FieldValueTuple> field_values =
-    {
-        FieldValueTuple(FLEX_COUNTER_STATUS_FIELD, FLEX_COUNTER_ENABLE)
-    };
-    flex_counter_group_table->set(group_name, field_values);
+    setFlexCounterGroupOperation(group_name, FLEX_COUNTER_ENABLE, is_gearbox);
     enabled = true;
 
     SWSS_LOG_DEBUG("Enabling flex counters for group '%s'.",
@@ -203,11 +186,7 @@ void FlexCounterManager::disableFlexCounterGroup()
         return;
     }
 
-    vector<FieldValueTuple> field_values =
-    {
-        FieldValueTuple(FLEX_COUNTER_STATUS_FIELD, FLEX_COUNTER_DISABLE)
-    };
-    flex_counter_group_table->set(group_name, field_values);
+    setFlexCounterGroupOperation(group_name, FLEX_COUNTER_DISABLE, is_gearbox);
     enabled = false;
 
     SWSS_LOG_DEBUG("Disabling flex counters for group '%s'.",
@@ -219,7 +198,8 @@ void FlexCounterManager::disableFlexCounterGroup()
 void FlexCounterManager::setCounterIdList(
         const sai_object_id_t object_id,
         const CounterType counter_type,
-        const unordered_set<string>& counter_stats)
+        const unordered_set<string>& counter_stats,
+        const sai_object_id_t switch_id)
 {
     SWSS_LOG_ENTER();
 
@@ -231,12 +211,12 @@ void FlexCounterManager::setCounterIdList(
         return;
     }
 
-    std::vector<swss::FieldValueTuple> field_values =
-    {
-        FieldValueTuple(counter_type_it->second, serializeCounterStats(counter_stats))
-    };
-    flex_counter_table->set(getFlexCounterTableKey(group_name, object_id), field_values);
-    installed_counters.insert(object_id);
+    auto key = getFlexCounterTableKey(group_name, object_id);
+    auto counter_ids = serializeCounterStats(counter_stats);
+    auto effective_switch_id = switch_id == SAI_NULL_OBJECT_ID ? gSwitchId : switch_id;
+
+    startFlexCounterPolling(effective_switch_id, key, counter_ids, counter_type_it->second);
+    installed_counters[object_id] = effective_switch_id;
 
     SWSS_LOG_DEBUG("Updated flex counter id list for object '%" PRIu64 "' in group '%s'.",
             object_id,
@@ -258,7 +238,8 @@ void FlexCounterManager::clearCounterIdList(const sai_object_id_t object_id)
         return;
     }
 
-    flex_counter_table->del(getFlexCounterTableKey(group_name, object_id));
+    auto key = getFlexCounterTableKey(group_name, object_id);
+    stopFlexCounterPolling(installed_counters[object_id], key);
     installed_counters.erase(counter_it);
 
     SWSS_LOG_DEBUG("Cleared flex counter id list for object '%" PRIu64 "' in group '%s'.",
@@ -272,7 +253,7 @@ string FlexCounterManager::getFlexCounterTableKey(
 {
     SWSS_LOG_ENTER();
 
-    return group_name + flex_counter_table->getTableNameSeparator() + sai_serialize_object_id(object_id);
+    return group_name + ":" + sai_serialize_object_id(object_id);
 }
 
 // serializeCounterStats turns a set of stats into a format suitable for FLEX_COUNTER_DB.