Add per-port counter discovery toggle argument

syncd/CommandLineOptions.cpp

@@ -48,6 +48,7 @@ CommandLineOptions::CommandLineOptions()
 #endif // SAITHRIFT
 
     m_supportingBulkCounterGroups = "";
+    m_enablePerPortCounterDiscovery = false;
 
     m_enableAttrVersionCheck = false;
 }
@@ -75,6 +76,7 @@ std::string CommandLineOptions::getCommandLineString() const
     ss << " WatchdogWarnTimeSpan=" << m_watchdogWarnTimeSpan;
     ss << " WatchdogInitTimeSpan=" << m_watchdogInitTimeSpan;
     ss << " SupportingBulkCounters=" << m_supportingBulkCounterGroups;
+    ss << " EnablePerPortCounterDiscovery=" << (m_enablePerPortCounterDiscovery ? "YES" : "NO");
     ss << " EnableAttrVersionCheck=" << (m_enableAttrVersionCheck ? "YES" : "NO");
 
 #ifdef SAITHRIFT

syncd/CommandLineOptions.h

@@ -109,6 +109,7 @@ namespace syncd
 #endif // SAITHRIFT
 
             std::string m_supportingBulkCounterGroups;
+            bool m_enablePerPortCounterDiscovery;
 
             bool m_enableAttrVersionCheck;
     };

syncd/CommandLineOptionsParser.cpp

@@ -26,9 +26,9 @@ std::shared_ptr<CommandLineOptions> CommandLineOptionsParser::parseCommandLine(
     bool initTimeSpanSeen = false;
 
 #ifdef SAITHRIFT
-    const char* const optstring = "dp:t:g:x:b:B:aw:W:uSUCsz:lRrm:h";
+    const char* const optstring = "dp:t:g:x:b:B:aw:W:uSUCsz:lGRrm:h";
 #else
-    const char* const optstring = "dp:t:g:x:b:B:aw:W:uSUCsz:lRh";
+    const char* const optstring = "dp:t:g:x:b:B:aw:W:uSUCsz:lGRh";
 #endif // SAITHRIFT
 
     while (true)
@@ -52,6 +52,7 @@ std::shared_ptr<CommandLineOptions> CommandLineOptionsParser::parseCommandLine(
             { "watchdogWarnTimeSpan",    optional_argument, 0, 'w' },
             { "watchdogInitTimeSpan",    optional_argument, 0, 'W' },
             { "supportingBulkCounters",  required_argument, 0, 'B' },
+            { "enablePerPortCounterDiscovery", no_argument, 0, 'G' },
             { "enableAttrVersionCheck",  no_argument,       0, 'a' },
 #ifdef SAITHRIFT
             { "rpcserver",               no_argument,       0, 'r' },
@@ -157,6 +158,10 @@ std::shared_ptr<CommandLineOptions> CommandLineOptionsParser::parseCommandLine(
                 options->m_supportingBulkCounterGroups = std::string(optarg);
                 break;
 
+            case 'G':
+                options->m_enablePerPortCounterDiscovery = true;
+                break;
+
             case 'a':
                 options->m_enableAttrVersionCheck = true;
                 break;
@@ -190,9 +195,9 @@ void CommandLineOptionsParser::printUsage()
     SWSS_LOG_ENTER();
 
 #ifdef SAITHRIFT
-    std::cout << "Usage: syncd [-d] [-p profile] [-t type] [-u] [-S] [-U] [-C] [-s] [-z mode] [-l] [-R] [-g idx] [-x contextConfig] [-b breakConfig] [-B supportingBulkCounters] [-r] [-m portmap] [-h]" << std::endl;
+    std::cout << "Usage: syncd [-d] [-p profile] [-t type] [-u] [-S] [-U] [-C] [-s] [-z mode] [-l] [-R] [-g idx] [-x contextConfig] [-b breakConfig] [-B supportingBulkCounters] [-G] [-r] [-m portmap] [-h]" << std::endl;
 #else
-    std::cout << "Usage: syncd [-d] [-p profile] [-t type] [-u] [-S] [-U] [-C] [-s] [-z mode] [-l] [-R] [-g idx] [-x contextConfig] [-b breakConfig] [-B supportingBulkCounters] [-h]" << std::endl;
+    std::cout << "Usage: syncd [-d] [-p profile] [-t type] [-u] [-S] [-U] [-C] [-s] [-z mode] [-l] [-R] [-g idx] [-x contextConfig] [-b breakConfig] [-B supportingBulkCounters] [-G] [-h]" << std::endl;
 #endif // SAITHRIFT
 
     std::cout << "    -d --diag" << std::endl;
@@ -229,6 +234,8 @@ void CommandLineOptionsParser::printUsage()
     std::cout << "        Watchdog time span (in microseconds) for init phase (default: same as -w)" << std::endl;
     std::cout << "    -B --supportingBulkCounters" << std::endl;
     std::cout << "        Counter groups those support bulk polling" << std::endl;
+    std::cout << "    -G --enablePerPortCounterDiscovery" << std::endl;
+    std::cout << "        Enable counter-group discovery during counter add operations" << std::endl;
     std::cout << "    -a --enableAttrVersionCheck" << std::endl;
     std::cout << "        Enable attribute SAI version check when performing SAI discovery" << std::endl;
 

syncd/FlexCounter.cpp

@@ -6,6 +6,7 @@
 #include <mutex>
 
 #include "FlexCounter.h"
+#include "VendorSaiOptions.h"
 #include "VidManager.h"
 
 #include <chrono>
@@ -4411,6 +4412,10 @@ void FlexCounter::addCounter(
     std::vector<std::string> counterIds;
 
     std::string statsMode;
+    auto vso = std::dynamic_pointer_cast<VendorSaiOptions>(
+            m_vendorSai->getOptions(VendorSaiOptions::OPTIONS_KEY));
+    const bool enablePerPortCounterDiscovery =
+            vso && vso->m_enablePerPortCounterDiscovery;
 
     for (const auto& valuePair: values)
     {
@@ -4422,27 +4427,19 @@ void FlexCounter::addCounter(
         const auto &counterGroupRef = m_objectTypeField2CounterType.find({objectType, field});
         if (counterGroupRef != m_objectTypeField2CounterType.end())
         {
-            try {
-                getCounterContext(counterGroupRef->second)->addObjectWithCounterGroups(
-                        vid,
-                        rid,
-                        idStrings,
-                        "");
+            auto counterContext = getCounterContext(counterGroupRef->second);
 
-            }
-            catch (const std::exception& e)
+            if (enablePerPortCounterDiscovery)
             {
-                SWSS_LOG_WARN("Error initializing SAI objects with counter groups: %s, falling back", e.what());
-                getCounterContext(counterGroupRef->second)->addObject(
+                counterContext->addObjectWithCounterGroups(
                         vid,
                         rid,
                         idStrings,
                         "");
             }
-            catch (...) {
-                SWSS_LOG_WARN("Unknown error initializing SAI objects with counter groups, falling back");
-
-                getCounterContext(counterGroupRef->second)->addObject(
+            else
+            {
+                counterContext->addObject(
                         vid,
                         rid,
                         idStrings,
@@ -4493,6 +4490,10 @@ void FlexCounter::bulkAddCounter(
     std::vector<std::string> counterIds;
 
     std::string statsMode;
+    auto vso = std::dynamic_pointer_cast<VendorSaiOptions>(
+            m_vendorSai->getOptions(VendorSaiOptions::OPTIONS_KEY));
+    const bool enablePerPortCounterDiscovery =
+            vso && vso->m_enablePerPortCounterDiscovery;
 
     for (const auto& valuePair: values)
     {
@@ -4504,33 +4505,24 @@ void FlexCounter::bulkAddCounter(
         const auto &counterGroupRef = m_objectTypeField2CounterType.find({objectType, field});
         if (counterGroupRef != m_objectTypeField2CounterType.end())
         {
-            try
-            {
-                getCounterContext(counterGroupRef->second)->bulkAddObjectWithCounterGroups(
-                        vids,
-                        rids,
-                        idStrings,
-                        "");
-            }
-            catch (const std::exception& e)
+            auto counterContext = getCounterContext(counterGroupRef->second);
+
+            if (enablePerPortCounterDiscovery)
             {
-                SWSS_LOG_WARN("Error initializing SAI objects with counter groups: %s, falling back", e.what());
-                getCounterContext(counterGroupRef->second)->bulkAddObject(
+               counterContext->bulkAddObjectWithCounterGroups(
                         vids,
                         rids,
                         idStrings,
                         "");
             }
-            catch (...)
+            else
             {
-                SWSS_LOG_WARN("Unknown error initializing SAI objects with counter groups, falling back");
-                getCounterContext(counterGroupRef->second)->bulkAddObject(
+                counterContext->bulkAddObject(
                         vids,
                         rids,
                         idStrings,
                         "");
             }
-
         }
         else if (objectType == SAI_OBJECT_TYPE_BUFFER_POOL && field == BUFFER_POOL_COUNTER_ID_LIST)
         {
@@ -4552,33 +4544,23 @@ void FlexCounter::bulkAddCounter(
 
     if (objectType == SAI_OBJECT_TYPE_BUFFER_POOL && counterIds.size())
     {
-        try
-        {
-            getCounterContext(COUNTER_TYPE_BUFFER_POOL)->bulkAddObjectWithCounterGroups(
-                    vids,
-                    rids,
-                    counterIds,
-                    statsMode);
+        auto counterContext = getCounterContext(COUNTER_TYPE_BUFFER_POOL);
 
-        }
-        catch (const std::exception& e)
+        if (enablePerPortCounterDiscovery)
         {
-            SWSS_LOG_WARN("Error initializing SAI objects with counter groups: %s, falling back", e.what());
-            getCounterContext(COUNTER_TYPE_BUFFER_POOL)->bulkAddObject(
+            counterContext->bulkAddObjectWithCounterGroups(
                     vids,
                     rids,
                     counterIds,
                     statsMode);
         }
-        catch (...)
+        else
         {
-            SWSS_LOG_WARN("Unknown error initializing SAI objects with counter groups, falling back");
-            getCounterContext(COUNTER_TYPE_BUFFER_POOL)->bulkAddObject(
+            counterContext->bulkAddObject(
                     vids,
                     rids,
                     counterIds,
                     statsMode);
-
         }
     }
 

syncd/Syncd.cpp

@@ -150,6 +150,7 @@ Syncd::Syncd(
     auto vso = std::make_shared<VendorSaiOptions>();
 
     vso->m_checkAttrVersion = m_commandLineOptions->m_enableAttrVersionCheck;
+    vso->m_enablePerPortCounterDiscovery = m_commandLineOptions->m_enablePerPortCounterDiscovery;
 
     m_vendorSai->setOptions(VendorSaiOptions::OPTIONS_KEY, vso);
 

syncd/VendorSaiOptions.h

@@ -13,5 +13,6 @@ namespace syncd
         public:
 
             bool m_checkAttrVersion = false;
+            bool m_enablePerPortCounterDiscovery = false;
     };
 }

unittest/syncd/TestCommandLineOptions.cpp

@@ -7,7 +7,7 @@
 using namespace syncd;
 
 const std::string expected_usage =
-R"(Usage: syncd [-d] [-p profile] [-t type] [-u] [-S] [-U] [-C] [-s] [-z mode] [-l] [-R] [-g idx] [-x contextConfig] [-b breakConfig] [-B supportingBulkCounters] [-h]
+R"(Usage: syncd [-d] [-p profile] [-t type] [-u] [-S] [-U] [-C] [-s] [-z mode] [-l] [-R] [-g idx] [-x contextConfig] [-b breakConfig] [-B supportingBulkCounters] [-G] [-h]
     -d --diag
         Enable diagnostic shell
     -p --profile profile
@@ -42,6 +42,8 @@ R"(Usage: syncd [-d] [-p profile] [-t type] [-u] [-S] [-U] [-C] [-s] [-z mode] [
         Watchdog time span (in microseconds) for init phase (default: same as -w)
     -B --supportingBulkCounters
         Counter groups those support bulk polling
+    -G --enablePerPortCounterDiscovery
+        Enable counter-group discovery during counter add operations
     -a --enableAttrVersionCheck
         Enable attribute SAI version check when performing SAI discovery
     -h --help
@@ -57,7 +59,8 @@ TEST(CommandLineOptions, getCommandLineString)
     EXPECT_EQ(str, " EnableDiagShell=NO EnableTempView=NO DisableExitSleep=NO EnableUnittests=NO"
             " EnableConsistencyCheck=NO EnableSyncMode=NO EnableAsyncRec=NO RedisCommunicationMode=redis_async"
             " EnableSaiBulkSuport=NO StartType=cold ProfileMapFile= GlobalContext=0 ContextConfig= BreakConfig="
-            " WatchdogWarnTimeSpan=30000000 WatchdogInitTimeSpan=30000000 SupportingBulkCounters= EnableAttrVersionCheck=NO");
+            " WatchdogWarnTimeSpan=30000000 WatchdogInitTimeSpan=30000000 SupportingBulkCounters="
+            " EnablePerPortCounterDiscovery=NO EnableAttrVersionCheck=NO");
 }
 
 TEST(CommandLineOptions, startTypeStringToStartType)
@@ -85,12 +88,14 @@ TEST(CommandLineOptionsParser, parseCommandLine)
     char arg3[] = "1000";
     char arg4[] = "-B";
     char arg5[] = "WATERMARK";
-    std::vector<char *> args = {arg1, arg2, arg3, arg4, arg5};
+    char arg6[] = "-G";
+    std::vector<char *> args = {arg1, arg2, arg3, arg4, arg5, arg6};
 
     auto opt = syncd::CommandLineOptionsParser::parseCommandLine((int)args.size(), args.data());
     EXPECT_EQ(opt->m_watchdogWarnTimeSpan, 1000);
     EXPECT_EQ(opt->m_watchdogInitTimeSpan, 1000);
     EXPECT_EQ(opt->m_supportingBulkCounterGroups, "WATERMARK");
+    EXPECT_TRUE(opt->m_enablePerPortCounterDiscovery);
 }
 
 TEST(CommandLineOptionsParser, parseCommandLineAsyncRec)

unittest/syncd/TestFlexCounter.cpp

@@ -1,4 +1,5 @@
 #include "FlexCounter.h"
+#include "VendorSaiOptions.h"
 #include "sai_serialize.h"
 #include "MockableSaiInterface.h"
 #include "MockHelper.h"
@@ -47,6 +48,37 @@ std::string toOid(T value)
 std::shared_ptr<MockableSaiInterface> sai(new MockableSaiInterface());
 typedef std::function<void(swss::Table &countersTable, const std::string& key, const std::vector<std::string>& counterIdNames, const std::vector<std::string>& expectedValues)> VerifyStatsFunc;
 
+class ScopedPerPortCounterDiscovery
+{
+    public:
+
+        explicit ScopedPerPortCounterDiscovery(
+                _In_ bool enabled)
+        {
+            SWSS_LOG_ENTER();
+            m_previous = sai->getOptions(VendorSaiOptions::OPTIONS_KEY);
+
+            auto options = std::make_shared<VendorSaiOptions>();
+
+            if (auto previousVendorOptions = std::dynamic_pointer_cast<VendorSaiOptions>(m_previous))
+            {
+                *options = *previousVendorOptions;
+            }
+
+            options->m_enablePerPortCounterDiscovery = enabled;
+
+            sai->setOptions(VendorSaiOptions::OPTIONS_KEY, options);
+        }
+
+        ~ScopedPerPortCounterDiscovery()
+        {
+            SWSS_LOG_ENTER();
+            sai->setOptions(VendorSaiOptions::OPTIONS_KEY, m_previous);
+        }
+
+        std::shared_ptr<sairedis::SaiOptions> m_previous;
+};
+
 std::vector<sai_object_id_t> generateOids(
         unsigned int numOid,
         sai_object_type_t object_type)
@@ -2456,6 +2488,8 @@ TEST_F(FlexCounterTcpFallback, tcpFallbackWhenNoUnixSocket)
 
 TEST(FlexCounter, dynamicCounterGroups)
 {
+    ScopedPerPortCounterDiscovery enablePerPortCounterDiscovery(true);
+
     // This test tests counter group functionality. It ensures each interface only polls the counters they support.
 
     // All 6 counters are requested for every port, but getStats fails for
@@ -2638,6 +2672,8 @@ TEST(FlexCounter, dynamicCounterGroups)
 
 TEST(FlexCounter, dynamicCounterGroupsBulkPath)
 {
+    ScopedPerPortCounterDiscovery enablePerPortCounterDiscovery(true);
+
     // Bulk-path variant of dynamicCounterGroups. Uses
     // bulkAddObjectWithCounterGroups, which selects the largest counter group
     // for bulkGetStats and falls back to single-object polling for ports whose