Optimize counter initialization by reducing the number of bulk counte…

…r poll calls and communication between swss/sairedis (#1527)

Optimize counter initialization by reducing the number of bulk counter poll calls and communication between swss(orchagent)/sairedis(syncd) during initialization.

Originally, orchagent notifies syncd to initialize the counter using an extended sairedis call SAI_REDIS_SWITCH_ATTR_FLEX_COUNTER for each SAI object with the object ID as the key, which means the number of the extended sairedis calls is identical as the number of objects. It takes time to finish all the extended sairedis calls.

Now, for counter groups that have many objects (e.g., port, PG, queues, etc), orchagent notifies syncd to initialize the counter using a single extend sairedis call with many objects' ID as the key (format: <key1>,<key2>,...<keyn>). So, it takes much less time to initialize the counters because fewer extend sairedis calls are required.

Details:

In sairedis, the bulk counter is supported for all counter groups except Buffer Pool Counter and DASH ENI counter.

In swss, bulk counter for the following counter groups

priority group watermark
priority group drop
queue watermark
queue stat
PFC watchdog
HLD sonic-net/SONiC#1862

syncd/FlexCounter.h

@@ -43,6 +43,12 @@ namespace syncd
                 _In_ const std::vector<std::string> &idStrings,
                 _In_ const std::string &per_object_stats_mode) = 0;
 
+        virtual void bulkAddObject(
+                _In_ const std::vector<sai_object_id_t>& vids,
+                _In_ const std::vector<sai_object_id_t>& rids,
+                _In_ const std::vector<std::string>& idStrings,
+                _In_ const std::string &per_object_stats_mode) = 0;
+
         virtual void removeObject(
                 _In_ sai_object_id_t vid) = 0;
 
@@ -95,6 +101,12 @@ namespace syncd
                     _In_ sai_object_id_t rid,
                     _In_ const std::vector<swss::FieldValueTuple>& values);
 
+            void bulkAddCounter(
+                    _In_ sai_object_type_t objectType,
+                    _In_ const std::vector<sai_object_id_t>& vids,
+                    _In_ const std::vector<sai_object_id_t>& rids,
+                    _In_ const std::vector<swss::FieldValueTuple>& values);
+
             void removeCounter(
                     _In_ sai_object_id_t vid);
 
@@ -188,5 +200,7 @@ namespace syncd
             bool m_noDoubleCheckBulkCapability;
 
             static const std::map<std::string, std::string> m_plugIn2CounterType;
+
+            static const std::map<std::tuple<sai_object_type_t, std::string>, std::string> m_objectTypeField2CounterType;
     };
 }

syncd/FlexCounterManager.cpp

@@ -1,4 +1,5 @@
 #include "FlexCounterManager.h"
+#include "VidManager.h"
 
 #include "swss/logger.h"
 
@@ -100,6 +101,26 @@ void FlexCounterManager::addCounter(
     }
 }
 
+void FlexCounterManager::bulkAddCounter(
+        _In_ const std::vector<sai_object_id_t> &vids,
+        _In_ const std::vector<sai_object_id_t> &rids,
+        _In_ const std::string& instanceId,
+        _In_ const std::vector<swss::FieldValueTuple>& values)
+{
+    SWSS_LOG_ENTER();
+
+    auto fc = getInstance(instanceId);
+
+    sai_object_type_t objectType = VidManager::objectTypeQuery(vids.at(0)); // VID and RID will have the same object type
+
+    fc->bulkAddCounter(objectType, vids, rids, values);
+
+    if (fc->isDiscarded())
+    {
+        removeInstance(instanceId);
+    }
+}
+
 void FlexCounterManager::removeCounter(
         _In_ sai_object_id_t vid,
         _In_ const std::string& instanceId)

syncd/FlexCounterManager.h

@@ -38,6 +38,12 @@ namespace syncd
                     _In_ const std::string& instanceId,
                     _In_ const std::vector<swss::FieldValueTuple>& values);
 
+            void bulkAddCounter(
+                _In_ const std::vector<sai_object_id_t> &vids,
+                _In_ const std::vector<sai_object_id_t> &rids,
+                _In_ const std::string& instanceId,
+                _In_ const std::vector<swss::FieldValueTuple>& values);
+
             void removeCounter(
                     _In_ sai_object_id_t vid,
                     _In_ const std::string& instanceId);

syncd/Syncd.cpp

@@ -2791,49 +2791,95 @@ sai_status_t Syncd::processFlexCounterEvent(
     }
 
     auto groupName = key.substr(0, delimiter);
-    auto strVid = key.substr(delimiter + 1);
+    auto strVids = key.substr(delimiter + 1);
+    auto vidStringVector = swss::tokenize(strVids, ',');
 
-    auto effective_op = op;
-
-    sai_object_id_t vid;
-    sai_deserialize_object_id(strVid, vid);
+    if (fromAsicChannel && op == SET_COMMAND && (!vidStringVector.empty()))
+    {
+        std::vector<sai_object_id_t> vids;
+        std::vector<sai_object_id_t> rids;
+        std::vector<std::string> keys;
 
-    sai_object_id_t rid;
+        vids.reserve(vidStringVector.size());
+        rids.reserve(vidStringVector.size());
+        keys.reserve(vidStringVector.size());
 
-    if (!m_translator->tryTranslateVidToRid(vid, rid))
-    {
-        if (fromAsicChannel)
+        for (auto &strVid: vidStringVector)
         {
-            SWSS_LOG_ERROR("port VID %s, was not found (probably port was removed/splitted) and will remove from counters now",
-                           sai_serialize_object_id(vid).c_str());
+            sai_object_id_t vid, rid;
+            sai_deserialize_object_id(strVid, vid);
+            vids.emplace_back(vid);
+
+            if (!m_translator->tryTranslateVidToRid(vid, rid))
+            {
+                SWSS_LOG_ERROR("port VID %s, was not found (probably port was removed/splitted) and will remove from counters now",
+                               sai_serialize_object_id(vid).c_str());
+            }
+
+            rids.emplace_back(rid);
+            keys.emplace_back(groupName + ":" + strVid);
         }
-        else
+
+        m_manager->bulkAddCounter(vids, rids, groupName, values);
+
+        for (auto &singleKey: keys)
         {
-            SWSS_LOG_WARN("port VID %s, was not found (probably port was removed/splitted) and will remove from counters now",
-                          sai_serialize_object_id(vid).c_str());
+            m_flexCounterTable->set(singleKey, values);
         }
-        effective_op = DEL_COMMAND;
-    }
 
-    if (effective_op == SET_COMMAND)
-    {
-        m_manager->addCounter(vid, rid, groupName, values);
         if (fromAsicChannel)
         {
-            m_flexCounterTable->set(key, values);
+            sendApiResponse(SAI_COMMON_API_SET, SAI_STATUS_SUCCESS);
         }
+
+        return SAI_STATUS_SUCCESS;
     }
-    else if (effective_op == DEL_COMMAND)
+
+    for(auto &strVid : vidStringVector)
     {
-        if (fromAsicChannel)
+        auto effective_op = op;
+        auto singleKey = groupName + ":" + strVid;
+
+        sai_object_id_t vid;
+        sai_deserialize_object_id(strVid, vid);
+
+        sai_object_id_t rid;
+
+        if (!m_translator->tryTranslateVidToRid(vid, rid))
         {
-            m_flexCounterTable->del(key);
+            if (fromAsicChannel)
+            {
+                SWSS_LOG_ERROR("port VID %s, was not found (probably port was removed/splitted) and will remove from counters now",
+                               sai_serialize_object_id(vid).c_str());
+            }
+            else
+            {
+                SWSS_LOG_WARN("port VID %s, was not found (probably port was removed/splitted) and will remove from counters now",
+                              sai_serialize_object_id(vid).c_str());
+            }
+            effective_op = DEL_COMMAND;
+        }
+
+        if (effective_op == SET_COMMAND)
+        {
+            m_manager->addCounter(vid, rid, groupName, values);
+            if (fromAsicChannel)
+            {
+                m_flexCounterTable->set(singleKey, values);
+            }
+        }
+        else if (effective_op == DEL_COMMAND)
+        {
+            if (fromAsicChannel)
+            {
+                m_flexCounterTable->del(singleKey);
+            }
+            m_manager->removeCounter(vid, groupName);
+        }
+        else
+        {
+            SWSS_LOG_ERROR("unknown command: %s", op.c_str());
         }
-        m_manager->removeCounter(vid, groupName);
-    }
-    else
-    {
-        SWSS_LOG_ERROR("unknown command: %s", op.c_str());
     }
 
     if (fromAsicChannel)

syncd/tests/TestSyncdMlnx.cpp

@@ -222,29 +222,40 @@ TEST_F(SyncdMlnxTest, queryAttrEnumValuesCapability)
 
 TEST_F(SyncdMlnxTest, portBulkAddRemove)
 {
-    const std::uint32_t portCount = 1;
+    const std::uint32_t portCount = 4;
     const std::uint32_t laneCount = 4;
 
     // Generate port config
-    std::array<std::uint32_t, laneCount> laneList = { 1000, 1001, 1002, 1003 };
-
     sai_attribute_t attr;
-    std::vector<sai_attribute_t> attrList;
-
-    attr.id = SAI_PORT_ATTR_HW_LANE_LIST;
-    attr.value.u32list.count = static_cast<std::uint32_t>(laneList.size());
-    attr.value.u32list.list = laneList.data();
-    attrList.push_back(attr);
-
-    attr.id = SAI_PORT_ATTR_SPEED;
-    attr.value.u32 = 1000;
-    attrList.push_back(attr);
+    std::array<std::vector<std::uint32_t>, portCount> laneLists;
+    std::array<std::vector<sai_attribute_t>, portCount> attrLists;
+    std::array<std::uint32_t, portCount> attrCountList;
+    std::array<const sai_attribute_t*, portCount> attrPtrList;
 
-    std::array<std::uint32_t, portCount> attrCountList = { static_cast<std::uint32_t>(attrList.size()) };
-    std::array<const sai_attribute_t*, portCount> attrPtrList = { attrList.data() };
+    uint32_t lane = 1000;
+    for (auto i = 0u; i < portCount; i++)
+    {
+        auto &laneList = laneLists[i];
+        auto &attrList = attrLists[i];
+        for (auto j = 0u; j < laneCount; j++)
+        {
+            laneList.push_back(lane++);
+        }
+        attr.id = SAI_PORT_ATTR_HW_LANE_LIST;
+        attr.value.u32list.count = static_cast<std::uint32_t>(laneList.size());
+        attr.value.u32list.list = laneList.data();
+        attrList.push_back(attr);
+
+        attr.id = SAI_PORT_ATTR_SPEED;
+        attr.value.u32 = 1000;
+        attrList.push_back(attr);
+
+        attrPtrList[i] = attrList.data();
+        attrCountList[i] = static_cast<std::uint32_t>(attrList.size());
+    }
 
-    std::array<sai_object_id_t, portCount> oidList = { SAI_NULL_OBJECT_ID };
-    std::array<sai_status_t, portCount> statusList = { SAI_STATUS_SUCCESS };
+    std::vector<sai_object_id_t> oidList(portCount, SAI_NULL_OBJECT_ID);
+    std::vector<sai_status_t> statusList(portCount, SAI_STATUS_SUCCESS);
 
     // Validate port bulk add
     auto status = m_sairedis->bulkCreate(
@@ -334,15 +345,41 @@ TEST_F(SyncdMlnxTest, portBulkAddRemove)
     fvVectorExpected.emplace_back(counter_field_name, counters);
     ASSERT_EQ(fvVectorExpected, fvVector);
 
+    // Try with bulk initialization
+    key = "PORT_STAT_COUNTER:";
+    for (auto i = 1u; i < portCount; i++)
+    {
+        key += sai_serialize_object_id(oidList[i]) + ",";
+    }
+    key.pop_back();
+
+    flexCounterParam.counter_key.list = (int8_t*)const_cast<char *>(key.c_str());
+    flexCounterParam.counter_key.count = (uint32_t)key.length();
+    status = m_sairedis->set(SAI_OBJECT_TYPE_SWITCH, m_switchId, &attr);
+    ASSERT_EQ(status, SAI_STATUS_SUCCESS);
+
+    for (auto i = 1u; i < portCount; i++)
+    {
+        key = "PORT_STAT_COUNTER:" + sai_serialize_object_id(oidList[i]);
+        ASSERT_TRUE(m_flexCounterTable->get(key, fvVector));
+        ASSERT_EQ(fvVectorExpected, fvVector);
+    }
+
     flexCounterParam.counter_ids.list = nullptr;
     flexCounterParam.counter_ids.count = 0;
     flexCounterParam.counter_field_name.list = nullptr;
     flexCounterParam.counter_field_name.count = 0;
 
     // 3. Stop counter polling for the port
-    status = m_sairedis->set(SAI_OBJECT_TYPE_SWITCH, m_switchId, &attr);
-    ASSERT_EQ(status, SAI_STATUS_SUCCESS);
-    ASSERT_FALSE(m_flexCounterTable->get(key, fvVector));
+    for (auto i = 0u; i < portCount; i++)
+    {
+        key = "PORT_STAT_COUNTER:" + sai_serialize_object_id(oidList[i]);
+        flexCounterParam.counter_key.list = (int8_t*)const_cast<char *>(key.c_str());
+        flexCounterParam.counter_key.count = (uint32_t)key.length();
+        status = m_sairedis->set(SAI_OBJECT_TYPE_SWITCH, m_switchId, &attr);
+        ASSERT_EQ(status, SAI_STATUS_SUCCESS);
+        ASSERT_FALSE(m_flexCounterTable->get(key, fvVector));
+    }
 
     // 4. Disable counter polling for the group
     flexCounterGroupParam.poll_interval.list = nullptr;