From 7c0535af382fa089e1f60d93d9584406c29ba93b Mon Sep 17 00:00:00 2001 From: Stephen Sun <5379172+stephenxs@users.noreply.github.com> Date: Thu, 7 Nov 2024 16:29:43 +0800 Subject: [PATCH] Do not poll counters in bulk mode during initialization for objects that support bulk per CLI option (#1437) Do not poll counters in bulk mode during initialization for objects that support bulk per CLI option Read the counter groups that support bulk mode from CONFIG_DB.DEVICE_METADATA|localhost.supporting_bulk_counter_groups Generate CLI options to syncd Based on CLI options syncd records the counter groups that support bulk mode. For those objects, we do not need to poll them in bulk mode during initialization --- syncd/CommandLineOptions.cpp | 1 + syncd/CommandLineOptions.h | 2 + syncd/CommandLineOptionsParser.cpp | 15 +++-- syncd/FlexCounter.cpp | 69 ++++++++++++----------- syncd/FlexCounter.h | 11 +++- syncd/FlexCounterManager.cpp | 9 ++- syncd/FlexCounterManager.h | 5 +- syncd/Syncd.cpp | 2 +- syncd/scripts/syncd_init_common.sh | 5 ++ unittest/syncd/TestCommandLineOptions.cpp | 11 +++- unittest/syncd/TestFlexCounter.cpp | 2 +- 11 files changed, 86 insertions(+), 46 deletions(-) diff --git a/syncd/CommandLineOptions.cpp b/syncd/CommandLineOptions.cpp index 493af0a87..0c25c8cad 100644 --- a/syncd/CommandLineOptions.cpp +++ b/syncd/CommandLineOptions.cpp @@ -66,6 +66,7 @@ std::string CommandLineOptions::getCommandLineString() const ss << " ContextConfig=" << m_contextConfig; ss << " BreakConfig=" << m_breakConfig; ss << " WatchdogWarnTimeSpan=" << m_watchdogWarnTimeSpan; + ss << " SupportingBulkCounters=" << m_supportingBulkCounterGroups; #ifdef SAITHRIFT diff --git a/syncd/CommandLineOptions.h b/syncd/CommandLineOptions.h index f3243e747..99d0827d6 100644 --- a/syncd/CommandLineOptions.h +++ b/syncd/CommandLineOptions.h @@ -98,5 +98,7 @@ namespace syncd std::string m_portMapFile; #endif // SAITHRIFT + std::string m_supportingBulkCounterGroups; + }; } diff --git a/syncd/CommandLineOptionsParser.cpp b/syncd/CommandLineOptionsParser.cpp index a7c034d73..d49624336 100644 --- a/syncd/CommandLineOptionsParser.cpp +++ b/syncd/CommandLineOptionsParser.cpp @@ -19,9 +19,9 @@ std::shared_ptr CommandLineOptionsParser::parseCommandLine( auto options = std::make_shared(); #ifdef SAITHRIFT - const char* const optstring = "dp:t:g:x:b:w:uSUCsz:lrm:h"; + const char* const optstring = "dp:t:g:x:b:B:w:uSUCsz:lrm:h"; #else - const char* const optstring = "dp:t:g:x:b:w:uSUCsz:lh"; + const char* const optstring = "dp:t:g:x:b:B:w:uSUCsz:lh"; #endif // SAITHRIFT while (true) @@ -42,6 +42,7 @@ std::shared_ptr CommandLineOptionsParser::parseCommandLine( { "contextContig", required_argument, 0, 'x' }, { "breakConfig", required_argument, 0, 'b' }, { "watchdogWarnTimeSpan", optional_argument, 0, 'w' }, + { "supportingBulkCounters", required_argument, 0, 'B' }, #ifdef SAITHRIFT { "rpcserver", no_argument, 0, 'r' }, { "portmap", required_argument, 0, 'm' }, @@ -133,6 +134,10 @@ std::shared_ptr CommandLineOptionsParser::parseCommandLine( break; #endif // SAITHRIFT + case 'B': + options->m_supportingBulkCounterGroups = std::string(optarg); + break; + case 'h': printUsage(); exit(EXIT_SUCCESS); @@ -156,9 +161,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] [-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] [-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] [-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] [-h]" << std::endl; #endif // SAITHRIFT std::cout << " -d --diag" << std::endl; @@ -189,6 +194,8 @@ void CommandLineOptionsParser::printUsage() std::cout << " Comparison logic 'break before make' configuration file" << std::endl; std::cout << " -w --watchdogWarnTimeSpan" << std::endl; std::cout << " Watchdog time span (in microseconds) to watch for execution" << std::endl; + std::cout << " -B --supportingBulkCounters" << std::endl; + std::cout << " Counter groups those support bulk polling" << std::endl; #ifdef SAITHRIFT diff --git a/syncd/FlexCounter.cpp b/syncd/FlexCounter.cpp index 4352efded..6424ef195 100644 --- a/syncd/FlexCounter.cpp +++ b/syncd/FlexCounter.cpp @@ -30,6 +30,14 @@ static const std::string ATTR_TYPE_QUEUE = "Queue Attribute"; static const std::string ATTR_TYPE_PG = "Priority Group Attribute"; static const std::string ATTR_TYPE_MACSEC_SA = "MACSEC SA Attribute"; static const std::string ATTR_TYPE_ACL_COUNTER = "ACL Counter Attribute"; +const std::map FlexCounter::m_plugIn2CounterType = { + {QUEUE_PLUGIN_FIELD, COUNTER_TYPE_QUEUE}, + {PG_PLUGIN_FIELD, COUNTER_TYPE_PG}, + {PORT_PLUGIN_FIELD, COUNTER_TYPE_PORT}, + {RIF_PLUGIN_FIELD, COUNTER_TYPE_RIF}, + {BUFFER_POOL_PLUGIN_FIELD, COUNTER_TYPE_BUFFER_POOL}, + {TUNNEL_PLUGIN_FIELD, COUNTER_TYPE_TUNNEL}, + {FLOW_COUNTER_PLUGIN_FIELD, COUNTER_TYPE_FLOW}}; BaseCounterContext::BaseCounterContext(const std::string &name): m_name(name) @@ -56,6 +64,13 @@ void BaseCounterContext::addPlugins( } } +void BaseCounterContext::setNoDoubleCheckBulkCapability( + _In_ bool noDoubleCheckBulkCapability) +{ + SWSS_LOG_ENTER(); + no_double_check_bulk_capability = noDoubleCheckBulkCapability; +} + template struct CounterIds @@ -460,7 +475,7 @@ class CounterContext : public BaseCounterContext } else { - supportBulk = checkBulkCapability(vid, rid, supportedIds); + supportBulk = no_double_check_bulk_capability || checkBulkCapability(vid, rid, supportedIds); } if (!supportBulk) @@ -1002,12 +1017,14 @@ class AttrContext : public CounterContext FlexCounter::FlexCounter( _In_ const std::string& instanceId, _In_ std::shared_ptr vendorSai, - _In_ const std::string& dbCounters): + _In_ const std::string& dbCounters, + _In_ const bool noDoubleCheckBulkCapability): m_readyToPoll(false), m_pollInterval(0), m_instanceId(instanceId), m_vendorSai(vendorSai), - m_dbCounters(dbCounters) + m_dbCounters(dbCounters), + m_noDoubleCheckBulkCapability(noDoubleCheckBulkCapability) { SWSS_LOG_ENTER(); @@ -1135,37 +1152,25 @@ void FlexCounter::addCounterPlugin( { setStatsMode(value); } - else if (field == QUEUE_PLUGIN_FIELD) - { - getCounterContext(COUNTER_TYPE_QUEUE)->addPlugins(shaStrings); - } - else if (field == PG_PLUGIN_FIELD) - { - getCounterContext(COUNTER_TYPE_PG)->addPlugins(shaStrings); - } - else if (field == PORT_PLUGIN_FIELD) - { - getCounterContext(COUNTER_TYPE_PORT)->addPlugins(shaStrings); - } - else if (field == RIF_PLUGIN_FIELD) - { - getCounterContext(COUNTER_TYPE_RIF)->addPlugins(shaStrings); - } - else if (field == BUFFER_POOL_PLUGIN_FIELD) - { - getCounterContext(COUNTER_TYPE_BUFFER_POOL)->addPlugins(shaStrings); - } - else if (field == TUNNEL_PLUGIN_FIELD) - { - getCounterContext(COUNTER_TYPE_TUNNEL)->addPlugins(shaStrings); - } - else if (field == FLOW_COUNTER_PLUGIN_FIELD) - { - getCounterContext(COUNTER_TYPE_FLOW)->addPlugins(shaStrings); - } else { - SWSS_LOG_ERROR("Field is not supported %s", field.c_str()); + auto counterTypeRef = m_plugIn2CounterType.find(field); + + if (counterTypeRef != m_plugIn2CounterType.end()) + { + getCounterContext(counterTypeRef->second)->addPlugins(shaStrings); + + if (m_noDoubleCheckBulkCapability) + { + getCounterContext(counterTypeRef->second)->setNoDoubleCheckBulkCapability(true); + + SWSS_LOG_NOTICE("Do not double check bulk capability counter context %s %s", m_instanceId.c_str(), counterTypeRef->second.c_str()); + } + } + else + { + SWSS_LOG_ERROR("Field is not supported %s", field.c_str()); + } } } diff --git a/syncd/FlexCounter.h b/syncd/FlexCounter.h index 0ed9c8be3..18d3f3af1 100644 --- a/syncd/FlexCounter.h +++ b/syncd/FlexCounter.h @@ -24,6 +24,9 @@ namespace syncd void addPlugins( _In_ const std::vector& shaStrings); + void setNoDoubleCheckBulkCapability( + _In_ bool); + bool hasPlugin() const {return !m_plugins.empty();} void removePlugins() {m_plugins.clear();} @@ -55,6 +58,7 @@ namespace syncd bool use_sai_stats_capa_query = true; bool use_sai_stats_ext = false; bool double_confirm_supported_counters = false; + bool no_double_check_bulk_capability = false; }; class FlexCounter { @@ -65,7 +69,8 @@ namespace syncd FlexCounter( _In_ const std::string& instanceId, _In_ std::shared_ptr vendorSai, - _In_ const std::string& dbCounters); + _In_ const std::string& dbCounters, + _In_ const bool noDoubleCheckBulkCapability=false); virtual ~FlexCounter(); @@ -168,5 +173,9 @@ namespace syncd bool m_isDiscarded; std::map> m_counterContext; + + bool m_noDoubleCheckBulkCapability; + + static const std::map m_plugIn2CounterType; }; } diff --git a/syncd/FlexCounterManager.cpp b/syncd/FlexCounterManager.cpp index 134ebc9db..5088a19c5 100644 --- a/syncd/FlexCounterManager.cpp +++ b/syncd/FlexCounterManager.cpp @@ -8,9 +8,11 @@ using namespace syncd; FlexCounterManager::FlexCounterManager( _In_ std::shared_ptr vendorSai, - _In_ const std::string& dbCounters): + _In_ const std::string& dbCounters, + _In_ const std::string& supportingBulkInstances): m_vendorSai(vendorSai), - m_dbCounters(dbCounters) + m_dbCounters(dbCounters), + m_supportingBulkGroups(supportingBulkInstances) { SWSS_LOG_ENTER(); @@ -26,7 +28,8 @@ std::shared_ptr FlexCounterManager::getInstance( if (m_flexCounters.count(instanceId) == 0) { - auto counter = std::make_shared(instanceId, m_vendorSai, m_dbCounters); + bool supportingBulk = (m_supportingBulkGroups.find(instanceId) != std::string::npos); + auto counter = std::make_shared(instanceId, m_vendorSai, m_dbCounters, supportingBulk); m_flexCounters[instanceId] = counter; } diff --git a/syncd/FlexCounterManager.h b/syncd/FlexCounterManager.h index df63522c9..5ba2f9992 100644 --- a/syncd/FlexCounterManager.h +++ b/syncd/FlexCounterManager.h @@ -10,7 +10,8 @@ namespace syncd FlexCounterManager( _In_ std::shared_ptr vendorSai, - _In_ const std::string& dbCounters); + _In_ const std::string& dbCounters, + _In_ const std::string& supportingBulkInstances); virtual ~FlexCounterManager() = default; @@ -50,6 +51,8 @@ namespace syncd std::shared_ptr m_vendorSai; std::string m_dbCounters; + + std::string m_supportingBulkGroups; }; } diff --git a/syncd/Syncd.cpp b/syncd/Syncd.cpp index d5116732a..6dd4cad50 100644 --- a/syncd/Syncd.cpp +++ b/syncd/Syncd.cpp @@ -109,7 +109,7 @@ Syncd::Syncd( m_enableSyncMode = true; } - m_manager = std::make_shared(m_vendorSai, m_contextConfig->m_dbCounters); + m_manager = std::make_shared(m_vendorSai, m_contextConfig->m_dbCounters, m_commandLineOptions->m_supportingBulkCounterGroups); loadProfileMap(); diff --git a/syncd/scripts/syncd_init_common.sh b/syncd/scripts/syncd_init_common.sh index e066afa5b..f7aad9795 100644 --- a/syncd/scripts/syncd_init_common.sh +++ b/syncd/scripts/syncd_init_common.sh @@ -42,6 +42,11 @@ if [ "$SYNC_MODE" == "enable" ]; then CMD_ARGS+=" -s" fi +SUPPORTING_BULK_COUNTER_GROUPS=$(echo $SYNCD_VARS | jq -r '.supporting_bulk_counter_groups') +if [ "$SUPPORTING_BULK_COUNTER_GROUPS" != "" ]; then + CMD_ARGS+=" -B $SUPPORTING_BULK_COUNTER_GROUPS" +fi + case "$(cat /proc/cmdline)" in *SONIC_BOOT_TYPE=fastfast*) if [ -e /var/warmboot/warm-starting ]; then diff --git a/unittest/syncd/TestCommandLineOptions.cpp b/unittest/syncd/TestCommandLineOptions.cpp index 508418767..7c9088bf0 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] [-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] [-h] -d --diag Enable diagnostic shell -p --profile profile @@ -36,6 +36,8 @@ R"(Usage: syncd [-d] [-p profile] [-t type] [-u] [-S] [-U] [-C] [-s] [-z mode] [ Comparison logic 'break before make' configuration file -w --watchdogWarnTimeSpan Watchdog time span (in microseconds) to watch for execution + -B --supportingBulkCounters + Counter groups those support bulk polling -h --help Print out this message )"; @@ -49,7 +51,7 @@ 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"); + " WatchdogWarnTimeSpan=30000000 SupportingBulkCounters="); } TEST(CommandLineOptions, startTypeStringToStartType) @@ -75,8 +77,11 @@ TEST(CommandLineOptionsParser, parseCommandLine) char arg1[] = "test"; char arg2[] = "-w"; char arg3[] = "1000"; - std::vector args = {arg1, arg2, arg3}; + char arg4[] = "-B"; + char arg5[] = "WATERMARK"; + std::vector args = {arg1, arg2, arg3, arg4, arg5}; auto opt = syncd::CommandLineOptionsParser::parseCommandLine((int)args.size(), args.data()); EXPECT_EQ(opt->m_watchdogWarnTimeSpan, 1000); + EXPECT_EQ(opt->m_supportingBulkCounterGroups, "WATERMARK"); } diff --git a/unittest/syncd/TestFlexCounter.cpp b/unittest/syncd/TestFlexCounter.cpp index 482fc6c1c..246f9b0c4 100644 --- a/unittest/syncd/TestFlexCounter.cpp +++ b/unittest/syncd/TestFlexCounter.cpp @@ -407,7 +407,7 @@ void testAddRemovePlugin(const std::string& pluginFieldName) { SWSS_LOG_ENTER(); - FlexCounter fc("test", sai, "COUNTERS_DB"); + FlexCounter fc("test", sai, "COUNTERS_DB", true); std::vector values; values.emplace_back(pluginFieldName, "dummy_sha_strings");