Optimize counter initialization by reducing the number of bulk counter poll calls and communication between swss/sairedis

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;