From c4aea50ed872e1b7710d09cb0f9749bcfd8059df 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 54443ed87..e77f982f4 100644 --- a/syncd/FlexCounter.cpp +++ b/syncd/FlexCounter.cpp @@ -31,6 +31,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) @@ -57,6 +65,13 @@ void BaseCounterContext::addPlugins( } } +void BaseCounterContext::setNoDoubleCheckBulkCapability( + _In_ bool noDoubleCheckBulkCapability) +{ + SWSS_LOG_ENTER(); + no_double_check_bulk_capability = noDoubleCheckBulkCapability; +} + template struct CounterIds @@ -478,7 +493,7 @@ class CounterContext : public BaseCounterContext } else { - supportBulk = checkBulkCapability(vid, rid, supportedIds); + supportBulk = no_double_check_bulk_capability || checkBulkCapability(vid, rid, supportedIds); } if (!supportBulk) @@ -1020,12 +1035,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(); @@ -1153,37 +1170,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 13c46c80d..9c2533c3a 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 c750f77f8..9a7a1c890 100644 --- a/syncd/scripts/syncd_init_common.sh +++ b/syncd/scripts/syncd_init_common.sh @@ -44,6 +44,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 1e81e3458..1b79e4d0d 100644 --- a/unittest/syncd/TestFlexCounter.cpp +++ b/unittest/syncd/TestFlexCounter.cpp @@ -456,7 +456,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");