From 90c944d80654de19ca9baa1f65c539dfee905e0d Mon Sep 17 00:00:00 2001 From: Stephen Sun Date: Fri, 27 Sep 2024 12:18:57 +0000 Subject: [PATCH 1/5] Refactor the logic to map counter type from plugin Signed-off-by: Stephen Sun --- syncd/FlexCounter.cpp | 47 +++++++++++++++++-------------------------- 1 file changed, 18 insertions(+), 29 deletions(-) diff --git a/syncd/FlexCounter.cpp b/syncd/FlexCounter.cpp index 54443ed87..1bc28edb1 100644 --- a/syncd/FlexCounter.cpp +++ b/syncd/FlexCounter.cpp @@ -1153,37 +1153,26 @@ 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()); + std::map 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}}; + + auto counterTypeRef = plugIn2CounterType.find(field); + if (counterTypeRef != plugIn2CounterType.end()) + { + getCounterContext(counterTypeRef->second)->addPlugins(shaStrings); + } + else + { + SWSS_LOG_ERROR("Field is not supported %s", field.c_str()); + } } } From 3b26dcb32cb80fa5d2071c1d14da8522d04b37f6 Mon Sep 17 00:00:00 2001 From: Stephen Sun Date: Mon, 30 Sep 2024 09:20:21 +0000 Subject: [PATCH 2/5] Do not poll counters in bulk mode during initialization if the bulk is supported Signed-off-by: Stephen Sun --- syncd/CommandLineOptions.cpp | 2 ++ syncd/CommandLineOptions.h | 2 ++ syncd/CommandLineOptionsParser.cpp | 16 ++++++++++++---- syncd/FlexCounter.cpp | 20 +++++++++++++++++--- syncd/FlexCounter.h | 10 +++++++++- syncd/FlexCounterManager.cpp | 9 ++++++--- syncd/FlexCounterManager.h | 5 ++++- syncd/Syncd.cpp | 2 +- syncd/scripts/syncd_init_common.sh | 5 +++++ unittest/syncd/TestFlexCounter.cpp | 10 +++++----- 10 files changed, 63 insertions(+), 18 deletions(-) diff --git a/syncd/CommandLineOptions.cpp b/syncd/CommandLineOptions.cpp index 493af0a87..af3b11cc5 100644 --- a/syncd/CommandLineOptions.cpp +++ b/syncd/CommandLineOptions.cpp @@ -74,6 +74,8 @@ std::string CommandLineOptions::getCommandLineString() const #endif // SAITHRIFT + ss << " supportingBulkCounters" << m_supportingBulkCounterGroups; + return ss.str(); } 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..33ae434ec 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) @@ -46,6 +46,7 @@ std::shared_ptr CommandLineOptionsParser::parseCommandLine( { "rpcserver", no_argument, 0, 'r' }, { "portmap", required_argument, 0, 'm' }, #endif // SAITHRIFT + { "supportingBulkCounters", required_argument, 0, 'B' }, { "help", no_argument, 0, 'h' }, { 0, 0, 0, 0 } }; @@ -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] [-r] [-m portmap] [-B supportingBulkCounters] [-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; @@ -199,6 +204,9 @@ void CommandLineOptionsParser::printUsage() #endif // SAITHRIFT + std::cout << " -B --supportingBulkCounters" << std::endl; + std::cout << " Counter groups that support bulk polling" << std::endl; + std::cout << " -h --help" << std::endl; std::cout << " Print out this message" << std::endl; } diff --git a/syncd/FlexCounter.cpp b/syncd/FlexCounter.cpp index 1bc28edb1..65cf3925f 100644 --- a/syncd/FlexCounter.cpp +++ b/syncd/FlexCounter.cpp @@ -57,6 +57,12 @@ void BaseCounterContext::addPlugins( } } +void BaseCounterContext::setNoDoubleCheckBulkCapability(bool noDoubleCheckBulkCapability) +{ + SWSS_LOG_ENTER(); + no_double_check_bulk_capability = noDoubleCheckBulkCapability; +} + template struct CounterIds @@ -478,7 +484,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 +1026,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(); @@ -1168,6 +1176,12 @@ void FlexCounter::addCounterPlugin( if (counterTypeRef != 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 { diff --git a/syncd/FlexCounter.h b/syncd/FlexCounter.h index 0ed9c8be3..c63cf9572 100644 --- a/syncd/FlexCounter.h +++ b/syncd/FlexCounter.h @@ -24,6 +24,8 @@ namespace syncd void addPlugins( _In_ const std::vector& shaStrings); + void setNoDoubleCheckBulkCapability(bool); + bool hasPlugin() const {return !m_plugins.empty();} void removePlugins() {m_plugins.clear();} @@ -44,6 +46,8 @@ namespace syncd _In_ swss::DBConnector& counters_db, _In_ const std::vector& argv) = 0; + + virtual bool hasObject() const = 0; protected: @@ -55,6 +59,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 +70,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); virtual ~FlexCounter(); @@ -168,5 +174,7 @@ namespace syncd bool m_isDiscarded; std::map> m_counterContext; + + bool m_noDoubleCheckBulkCapability; }; } 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 c1d965328..c44883187 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/TestFlexCounter.cpp b/unittest/syncd/TestFlexCounter.cpp index 1e81e3458..d0b0f78f4 100644 --- a/unittest/syncd/TestFlexCounter.cpp +++ b/unittest/syncd/TestFlexCounter.cpp @@ -88,7 +88,7 @@ void testAddRemoveCounter( { SWSS_LOG_ENTER(); - FlexCounter fc("test", sai, "COUNTERS_DB"); + FlexCounter fc("test", sai, "COUNTERS_DB", false); test_syncd::mockVidManagerObjectTypeQuery(object_type); @@ -441,7 +441,7 @@ TEST(FlexCounter, noSupportedCounters) return SAI_STATUS_FAILURE; }; - FlexCounter fc("test", sai, "COUNTERS_DB"); + FlexCounter fc("test", sai, "COUNTERS_DB", false); std::vector values; values.emplace_back(PORT_COUNTER_ID_LIST, "SAI_PORT_STAT_IF_IN_OCTETS,SAI_PORT_STAT_IF_IN_UCAST_PKTS"); @@ -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", false); std::vector values; values.emplace_back(pluginFieldName, "dummy_sha_strings"); @@ -484,7 +484,7 @@ TEST(FlexCounter, addRemoveCounterPlugin) TEST(FlexCounter, addRemoveCounterForPort) { - FlexCounter fc("test", sai, "COUNTERS_DB"); + FlexCounter fc("test", sai, "COUNTERS_DB", false); sai_object_id_t counterVid{0x1000000000000}; sai_object_id_t counterRid{0x1000000000000}; @@ -799,7 +799,7 @@ TEST(FlexCounter, counterIdChange) } }; - FlexCounter fc("test", sai, "COUNTERS_DB"); + FlexCounter fc("test", sai, "COUNTERS_DB", false); test_syncd::mockVidManagerObjectTypeQuery(SAI_OBJECT_TYPE_PORT); From c861a3eada04ac71ee063644662695b0aeeb3fac Mon Sep 17 00:00:00 2001 From: Stephen Sun Date: Mon, 21 Oct 2024 02:59:12 +0000 Subject: [PATCH 3/5] Fix review comments Signed-off-by: Stephen Sun --- syncd/CommandLineOptions.cpp | 3 +-- syncd/CommandLineOptionsParser.cpp | 9 ++++---- syncd/FlexCounter.cpp | 26 ++++++++++++----------- syncd/FlexCounter.h | 9 ++++---- unittest/syncd/TestCommandLineOptions.cpp | 6 ++++-- unittest/syncd/TestFlexCounter.cpp | 10 ++++----- 6 files changed, 33 insertions(+), 30 deletions(-) diff --git a/syncd/CommandLineOptions.cpp b/syncd/CommandLineOptions.cpp index af3b11cc5..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 @@ -74,8 +75,6 @@ std::string CommandLineOptions::getCommandLineString() const #endif // SAITHRIFT - ss << " supportingBulkCounters" << m_supportingBulkCounterGroups; - return ss.str(); } diff --git a/syncd/CommandLineOptionsParser.cpp b/syncd/CommandLineOptionsParser.cpp index 33ae434ec..fc2997c43 100644 --- a/syncd/CommandLineOptionsParser.cpp +++ b/syncd/CommandLineOptionsParser.cpp @@ -42,11 +42,11 @@ 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' }, #endif // SAITHRIFT - { "supportingBulkCounters", required_argument, 0, 'B' }, { "help", no_argument, 0, 'h' }, { 0, 0, 0, 0 } }; @@ -161,7 +161,7 @@ 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] [-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] [-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; #endif // SAITHRIFT @@ -194,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 @@ -204,9 +206,6 @@ void CommandLineOptionsParser::printUsage() #endif // SAITHRIFT - std::cout << " -B --supportingBulkCounters" << std::endl; - std::cout << " Counter groups that support bulk polling" << std::endl; - std::cout << " -h --help" << std::endl; std::cout << " Print out this message" << std::endl; } diff --git a/syncd/FlexCounter.cpp b/syncd/FlexCounter.cpp index 65cf3925f..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,7 +65,8 @@ void BaseCounterContext::addPlugins( } } -void BaseCounterContext::setNoDoubleCheckBulkCapability(bool noDoubleCheckBulkCapability) +void BaseCounterContext::setNoDoubleCheckBulkCapability( + _In_ bool noDoubleCheckBulkCapability) { SWSS_LOG_ENTER(); no_double_check_bulk_capability = noDoubleCheckBulkCapability; @@ -1163,23 +1172,16 @@ void FlexCounter::addCounterPlugin( } else { - std::map 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}}; - - auto counterTypeRef = plugIn2CounterType.find(field); - if (counterTypeRef != plugIn2CounterType.end()) + 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()); } } diff --git a/syncd/FlexCounter.h b/syncd/FlexCounter.h index c63cf9572..18d3f3af1 100644 --- a/syncd/FlexCounter.h +++ b/syncd/FlexCounter.h @@ -24,7 +24,8 @@ namespace syncd void addPlugins( _In_ const std::vector& shaStrings); - void setNoDoubleCheckBulkCapability(bool); + void setNoDoubleCheckBulkCapability( + _In_ bool); bool hasPlugin() const {return !m_plugins.empty();} @@ -46,8 +47,6 @@ namespace syncd _In_ swss::DBConnector& counters_db, _In_ const std::vector& argv) = 0; - - virtual bool hasObject() const = 0; protected: @@ -71,7 +70,7 @@ namespace syncd _In_ const std::string& instanceId, _In_ std::shared_ptr vendorSai, _In_ const std::string& dbCounters, - _In_ const bool noDoubleCheckBulkCapability); + _In_ const bool noDoubleCheckBulkCapability=false); virtual ~FlexCounter(); @@ -176,5 +175,7 @@ namespace syncd std::map> m_counterContext; bool m_noDoubleCheckBulkCapability; + + static const std::map m_plugIn2CounterType; }; } diff --git a/unittest/syncd/TestCommandLineOptions.cpp b/unittest/syncd/TestCommandLineOptions.cpp index 508418767..ad624eb50 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) diff --git a/unittest/syncd/TestFlexCounter.cpp b/unittest/syncd/TestFlexCounter.cpp index d0b0f78f4..1e81e3458 100644 --- a/unittest/syncd/TestFlexCounter.cpp +++ b/unittest/syncd/TestFlexCounter.cpp @@ -88,7 +88,7 @@ void testAddRemoveCounter( { SWSS_LOG_ENTER(); - FlexCounter fc("test", sai, "COUNTERS_DB", false); + FlexCounter fc("test", sai, "COUNTERS_DB"); test_syncd::mockVidManagerObjectTypeQuery(object_type); @@ -441,7 +441,7 @@ TEST(FlexCounter, noSupportedCounters) return SAI_STATUS_FAILURE; }; - FlexCounter fc("test", sai, "COUNTERS_DB", false); + FlexCounter fc("test", sai, "COUNTERS_DB"); std::vector values; values.emplace_back(PORT_COUNTER_ID_LIST, "SAI_PORT_STAT_IF_IN_OCTETS,SAI_PORT_STAT_IF_IN_UCAST_PKTS"); @@ -456,7 +456,7 @@ void testAddRemovePlugin(const std::string& pluginFieldName) { SWSS_LOG_ENTER(); - FlexCounter fc("test", sai, "COUNTERS_DB", false); + FlexCounter fc("test", sai, "COUNTERS_DB"); std::vector values; values.emplace_back(pluginFieldName, "dummy_sha_strings"); @@ -484,7 +484,7 @@ TEST(FlexCounter, addRemoveCounterPlugin) TEST(FlexCounter, addRemoveCounterForPort) { - FlexCounter fc("test", sai, "COUNTERS_DB", false); + FlexCounter fc("test", sai, "COUNTERS_DB"); sai_object_id_t counterVid{0x1000000000000}; sai_object_id_t counterRid{0x1000000000000}; @@ -799,7 +799,7 @@ TEST(FlexCounter, counterIdChange) } }; - FlexCounter fc("test", sai, "COUNTERS_DB", false); + FlexCounter fc("test", sai, "COUNTERS_DB"); test_syncd::mockVidManagerObjectTypeQuery(SAI_OBJECT_TYPE_PORT); From 4ba0d3bf60a28e4f12200c339ebf550f0647bf8b Mon Sep 17 00:00:00 2001 From: Stephen Sun Date: Mon, 21 Oct 2024 07:43:19 +0000 Subject: [PATCH 4/5] Fix comments Signed-off-by: Stephen Sun --- syncd/CommandLineOptionsParser.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/syncd/CommandLineOptionsParser.cpp b/syncd/CommandLineOptionsParser.cpp index fc2997c43..d49624336 100644 --- a/syncd/CommandLineOptionsParser.cpp +++ b/syncd/CommandLineOptionsParser.cpp @@ -42,7 +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' }, + { "supportingBulkCounters", required_argument, 0, 'B' }, #ifdef SAITHRIFT { "rpcserver", no_argument, 0, 'r' }, { "portmap", required_argument, 0, 'm' }, From e0f8b07393941758445d207c866625c068c591bf Mon Sep 17 00:00:00 2001 From: Stephen Sun Date: Fri, 25 Oct 2024 01:59:03 +0000 Subject: [PATCH 5/5] Fix coverage Signed-off-by: Stephen Sun --- unittest/syncd/TestCommandLineOptions.cpp | 5 ++++- unittest/syncd/TestFlexCounter.cpp | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/unittest/syncd/TestCommandLineOptions.cpp b/unittest/syncd/TestCommandLineOptions.cpp index ad624eb50..7c9088bf0 100644 --- a/unittest/syncd/TestCommandLineOptions.cpp +++ b/unittest/syncd/TestCommandLineOptions.cpp @@ -77,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");