From ba98b06b6bf3de7d4b1ae0f3cd74a46e67ccc0bd Mon Sep 17 00:00:00 2001 From: Justin Wong Date: Sun, 19 Jul 2026 06:12:10 +0000 Subject: [PATCH 1/4] Add per-port counter discovery toggle argument Add an argument to toggle the per-port counter discovery feature in syncd_init_common.sh Signed-off-by: Justin Wong --- syncd/CommandLineOptions.cpp | 2 + syncd/CommandLineOptions.h | 1 + syncd/CommandLineOptionsParser.cpp | 15 +++-- syncd/FlexCounter.cpp | 70 +++++++++-------------- syncd/Syncd.cpp | 1 + syncd/VendorSaiOptions.h | 1 + unittest/syncd/TestCommandLineOptions.cpp | 13 +++-- unittest/syncd/TestFlexCounter.cpp | 34 +++++++++++ 8 files changed, 85 insertions(+), 52 deletions(-) diff --git a/syncd/CommandLineOptions.cpp b/syncd/CommandLineOptions.cpp index 66d98f7d10..d468531115 100644 --- a/syncd/CommandLineOptions.cpp +++ b/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 diff --git a/syncd/CommandLineOptions.h b/syncd/CommandLineOptions.h index 883b427bd7..e23f07b502 100644 --- a/syncd/CommandLineOptions.h +++ b/syncd/CommandLineOptions.h @@ -109,6 +109,7 @@ namespace syncd #endif // SAITHRIFT std::string m_supportingBulkCounterGroups; + bool m_enablePerPortCounterDiscovery; bool m_enableAttrVersionCheck; }; diff --git a/syncd/CommandLineOptionsParser.cpp b/syncd/CommandLineOptionsParser.cpp index 99af4f6d42..bab3ea4e51 100644 --- a/syncd/CommandLineOptionsParser.cpp +++ b/syncd/CommandLineOptionsParser.cpp @@ -26,9 +26,9 @@ std::shared_ptr 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 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 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; diff --git a/syncd/FlexCounter.cpp b/syncd/FlexCounter.cpp index 11f3e4e5c2..ab93f7ce53 100644 --- a/syncd/FlexCounter.cpp +++ b/syncd/FlexCounter.cpp @@ -6,6 +6,7 @@ #include #include "FlexCounter.h" +#include "VendorSaiOptions.h" #include "VidManager.h" #include @@ -4411,6 +4412,10 @@ void FlexCounter::addCounter( std::vector counterIds; std::string statsMode; + auto vso = std::dynamic_pointer_cast( + 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 counterIds; std::string statsMode; + auto vso = std::dynamic_pointer_cast( + 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); - } } diff --git a/syncd/Syncd.cpp b/syncd/Syncd.cpp index 1bd8083964..1b36c52e17 100644 --- a/syncd/Syncd.cpp +++ b/syncd/Syncd.cpp @@ -150,6 +150,7 @@ Syncd::Syncd( auto vso = std::make_shared(); vso->m_checkAttrVersion = m_commandLineOptions->m_enableAttrVersionCheck; + vso->m_enablePerPortCounterDiscovery = m_commandLineOptions->m_enablePerPortCounterDiscovery; m_vendorSai->setOptions(VendorSaiOptions::OPTIONS_KEY, vso); diff --git a/syncd/VendorSaiOptions.h b/syncd/VendorSaiOptions.h index 4c66e81d32..0a3fdc1016 100644 --- a/syncd/VendorSaiOptions.h +++ b/syncd/VendorSaiOptions.h @@ -13,5 +13,6 @@ namespace syncd public: bool m_checkAttrVersion = false; + bool m_enablePerPortCounterDiscovery = false; }; } diff --git a/unittest/syncd/TestCommandLineOptions.cpp b/unittest/syncd/TestCommandLineOptions.cpp index edfcf04c08..e6b10fd791 100644 --- a/unittest/syncd/TestCommandLineOptions.cpp +++ b/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 args = {arg1, arg2, arg3, arg4, arg5}; + char arg6[] "-G"; + std::vector 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_EQ(opt->supportingBulkCounterGroups, "WATERMARK"); + EXPECT_TRUE(opt->m_enablePerPortCounterDiscovery); } TEST(CommandLineOptionsParser, parseCommandLineAsyncRec) diff --git a/unittest/syncd/TestFlexCounter.cpp b/unittest/syncd/TestFlexCounter.cpp index 2029370af8..7704ef09f3 100644 --- a/unittest/syncd/TestFlexCounter.cpp +++ b/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,35 @@ std::string toOid(T value) std::shared_ptr sai(new MockableSaiInterface()); typedef std::function& counterIdNames, const std::vector& expectedValues)> VerifyStatsFunc; +class ScopedPerPortCounterDiscovery +{ + public: + + explicit ScopedPerPortCounterDiscovery( + _In_ bool enabled) + { + m_previous = sai->getOptions(VendorSaiOptions::OPTIONS_KEY); + + auto options = std::make_shared(); + + if (auto previousVendorOptions = std::dynamic_pointer_cast(m_previous)) + { + *options = *previousVendorOptions; + } + + options->m_enablePerPortCounterDiscovery = enabled; + + sai->setOptions(VendorSaiOptions::OPTIONS_KEY, options); + } + + ~ScopedPerPortCounterDiscovery() + { + sai->setOptions(VendorSaiOptions::OPTIONS_KEY, m_previous); + } + + std::shared_ptr m_previous; +}; + std::vector generateOids( unsigned int numOid, sai_object_type_t object_type) @@ -2456,6 +2486,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 +2670,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 From 5fa0ff9871b670b100472fcf3c9dc54bc79e6fd6 Mon Sep 17 00:00:00 2001 From: Justin Wong Date: Mon, 20 Jul 2026 18:48:51 +0000 Subject: [PATCH 2/4] fix patch cast typo Signed-off-by: Justin Wong --- unittest/syncd/TestCommandLineOptions.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/unittest/syncd/TestCommandLineOptions.cpp b/unittest/syncd/TestCommandLineOptions.cpp index e6b10fd791..27e9687f12 100644 --- a/unittest/syncd/TestCommandLineOptions.cpp +++ b/unittest/syncd/TestCommandLineOptions.cpp @@ -88,7 +88,7 @@ TEST(CommandLineOptionsParser, parseCommandLine) char arg3[] = "1000"; char arg4[] = "-B"; char arg5[] = "WATERMARK"; - char arg6[] "-G"; + char arg6[] = "-G"; std::vector args = {arg1, arg2, arg3, arg4, arg5, arg6}; auto opt = syncd::CommandLineOptionsParser::parseCommandLine((int)args.size(), args.data()); From 2b6791682124aa6a7698e0b98b810da00622a38c Mon Sep 17 00:00:00 2001 From: Justin Wong Date: Mon, 20 Jul 2026 19:38:11 +0000 Subject: [PATCH 3/4] fix patch cast typo Signed-off-by: Justin Wong --- unittest/syncd/TestCommandLineOptions.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/unittest/syncd/TestCommandLineOptions.cpp b/unittest/syncd/TestCommandLineOptions.cpp index 27e9687f12..001ec8f3bc 100644 --- a/unittest/syncd/TestCommandLineOptions.cpp +++ b/unittest/syncd/TestCommandLineOptions.cpp @@ -94,7 +94,7 @@ TEST(CommandLineOptionsParser, parseCommandLine) 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->supportingBulkCounterGroups, "WATERMARK"); + EXPECT_EQ(opt->m_supportingBulkCounterGroups, "WATERMARK"); EXPECT_TRUE(opt->m_enablePerPortCounterDiscovery); } From 183e44c8c6285735ca810bf51ce653f4568fd5a2 Mon Sep 17 00:00:00 2001 From: Justin Wong Date: Mon, 20 Jul 2026 20:05:42 +0000 Subject: [PATCH 4/4] Add SWSS_LOG_ENTER() to new functions to fulfil CI requirements Signed-off-by: Justin Wong --- unittest/syncd/TestFlexCounter.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/unittest/syncd/TestFlexCounter.cpp b/unittest/syncd/TestFlexCounter.cpp index 7704ef09f3..709ed5c7ea 100644 --- a/unittest/syncd/TestFlexCounter.cpp +++ b/unittest/syncd/TestFlexCounter.cpp @@ -55,6 +55,7 @@ class ScopedPerPortCounterDiscovery explicit ScopedPerPortCounterDiscovery( _In_ bool enabled) { + SWSS_LOG_ENTER(); m_previous = sai->getOptions(VendorSaiOptions::OPTIONS_KEY); auto options = std::make_shared(); @@ -71,6 +72,7 @@ class ScopedPerPortCounterDiscovery ~ScopedPerPortCounterDiscovery() { + SWSS_LOG_ENTER(); sai->setOptions(VendorSaiOptions::OPTIONS_KEY, m_previous); }