From e4be7392c40cb8eda2f9f1e76b9d37c05414aedc Mon Sep 17 00:00:00 2001 From: Justin Wong Date: Tue, 4 Aug 2026 21:52:22 +0000 Subject: [PATCH] Cast new syncd arg logic from master Casting from: https://github.com/sonic-net/sonic-sairedis/pull/2000 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 | 11 +++- unittest/syncd/TestFlexCounter.cpp | 36 ++++++++++++ 8 files changed, 86 insertions(+), 51 deletions(-) diff --git a/syncd/CommandLineOptions.cpp b/syncd/CommandLineOptions.cpp index 02f026c5f8..aa2acddba2 100644 --- a/syncd/CommandLineOptions.cpp +++ b/syncd/CommandLineOptions.cpp @@ -47,6 +47,7 @@ CommandLineOptions::CommandLineOptions() #endif // SAITHRIFT m_supportingBulkCounterGroups = ""; + m_enablePerPortCounterDiscovery = false; m_enableAttrVersionCheck = false; } @@ -73,6 +74,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 96d362992e..b9117d741e 100644 --- a/syncd/CommandLineOptions.h +++ b/syncd/CommandLineOptions.h @@ -101,6 +101,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 08d681a0ed..ffcc11739f 100644 --- a/syncd/CommandLineOptionsParser.cpp +++ b/syncd/CommandLineOptionsParser.cpp @@ -23,9 +23,9 @@ std::shared_ptr CommandLineOptionsParser::parseCommandLine( bool initTimeSpanSeen = false; #ifdef SAITHRIFT - const char* const optstring = "dp:t:g:x:b:B:aw:W:uSUCsz:lrm:h"; + const char* const optstring = "dp:t:g:x:b:B:aw:W:uSUCsz:lGrm:h"; #else - const char* const optstring = "dp:t:g:x:b:B:aw:W:uSUCsz:lh"; + const char* const optstring = "dp:t:g:x:b:B:aw:W:uSUCsz:lGh"; #endif // SAITHRIFT while (true) @@ -48,6 +48,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' }, @@ -149,6 +150,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; @@ -182,9 +187,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] [-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] [-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] [-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] [-g idx] [-x contextConfig] [-b breakConfig] [-B supportingBulkCounters] [-G] [-h]" << std::endl; #endif // SAITHRIFT std::cout << " -d --diag" << std::endl; @@ -219,6 +224,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 d2bc0ba056..227bd4844f 100644 --- a/syncd/Syncd.cpp +++ b/syncd/Syncd.cpp @@ -122,6 +122,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 fbfffe016a..ab829556b2 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] [-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] [-g idx] [-x contextConfig] [-b breakConfig] [-B supportingBulkCounters] [-G] [-h] -d --diag Enable diagnostic shell -p --profile profile @@ -40,6 +40,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 @@ -55,7 +57,8 @@ TEST(CommandLineOptions, getCommandLineString) EXPECT_EQ(str, " EnableDiagShell=NO EnableTempView=NO DisableExitSleep=NO EnableUnittests=NO" " EnableConsistencyCheck=NO EnableSyncMode=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) @@ -83,12 +86,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_TRUE(opt->m_enablePerPortCounterDiscovery); } TEST(CommandLineOptionsParser, parseCommandLineInitTimeout) diff --git a/unittest/syncd/TestFlexCounter.cpp b/unittest/syncd/TestFlexCounter.cpp index 3069eadb92..10b7ff90a1 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,37 @@ 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) + { + SWSS_LOG_ENTER(); + 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() + { + SWSS_LOG_ENTER(); + sai->setOptions(VendorSaiOptions::OPTIONS_KEY, m_previous); + } + + std::shared_ptr m_previous; +}; + std::vector generateOids( unsigned int numOid, sai_object_type_t object_type) @@ -2450,6 +2482,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 @@ -2632,6 +2666,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