Skip to content

Commit

Permalink
Do not poll counters in bulk mode during initialization for objects t…
Browse files Browse the repository at this point in the history
…hat 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
  • Loading branch information
stephenxs authored and mssonicbld committed Dec 3, 2024
1 parent 497bd05 commit 0bd015e
Show file tree
Hide file tree
Showing 11 changed files with 86 additions and 46 deletions.
1 change: 1 addition & 0 deletions syncd/CommandLineOptions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 2 additions & 0 deletions syncd/CommandLineOptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -98,5 +98,7 @@ namespace syncd
std::string m_portMapFile;
#endif // SAITHRIFT

std::string m_supportingBulkCounterGroups;

};
}
15 changes: 11 additions & 4 deletions syncd/CommandLineOptionsParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ std::shared_ptr<CommandLineOptions> CommandLineOptionsParser::parseCommandLine(
auto options = std::make_shared<CommandLineOptions>();

#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)
Expand All @@ -42,6 +42,7 @@ std::shared_ptr<CommandLineOptions> 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' },
Expand Down Expand Up @@ -133,6 +134,10 @@ std::shared_ptr<CommandLineOptions> CommandLineOptionsParser::parseCommandLine(
break;
#endif // SAITHRIFT

case 'B':
options->m_supportingBulkCounterGroups = std::string(optarg);
break;

case 'h':
printUsage();
exit(EXIT_SUCCESS);
Expand All @@ -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;
Expand Down Expand Up @@ -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

Expand Down
69 changes: 37 additions & 32 deletions syncd/FlexCounter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<std::string, std::string> 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)
Expand All @@ -56,6 +64,13 @@ void BaseCounterContext::addPlugins(
}
}

void BaseCounterContext::setNoDoubleCheckBulkCapability(
_In_ bool noDoubleCheckBulkCapability)
{
SWSS_LOG_ENTER();
no_double_check_bulk_capability = noDoubleCheckBulkCapability;
}

template <typename StatType,
typename Enable = void>
struct CounterIds
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -1002,12 +1017,14 @@ class AttrContext : public CounterContext<AttrType>
FlexCounter::FlexCounter(
_In_ const std::string& instanceId,
_In_ std::shared_ptr<sairedis::SaiInterface> 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();

Expand Down Expand Up @@ -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());
}
}
}

Expand Down
11 changes: 10 additions & 1 deletion syncd/FlexCounter.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ namespace syncd
void addPlugins(
_In_ const std::vector<std::string>& shaStrings);

void setNoDoubleCheckBulkCapability(
_In_ bool);

bool hasPlugin() const {return !m_plugins.empty();}

void removePlugins() {m_plugins.clear();}
Expand Down Expand Up @@ -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
{
Expand All @@ -65,7 +69,8 @@ namespace syncd
FlexCounter(
_In_ const std::string& instanceId,
_In_ std::shared_ptr<sairedis::SaiInterface> vendorSai,
_In_ const std::string& dbCounters);
_In_ const std::string& dbCounters,
_In_ const bool noDoubleCheckBulkCapability=false);

virtual ~FlexCounter();

Expand Down Expand Up @@ -168,5 +173,9 @@ namespace syncd
bool m_isDiscarded;

std::map<std::string, std::shared_ptr<BaseCounterContext>> m_counterContext;

bool m_noDoubleCheckBulkCapability;

static const std::map<std::string, std::string> m_plugIn2CounterType;
};
}
9 changes: 6 additions & 3 deletions syncd/FlexCounterManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,11 @@ using namespace syncd;

FlexCounterManager::FlexCounterManager(
_In_ std::shared_ptr<sairedis::SaiInterface> 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();

Expand All @@ -26,7 +28,8 @@ std::shared_ptr<FlexCounter> FlexCounterManager::getInstance(

if (m_flexCounters.count(instanceId) == 0)
{
auto counter = std::make_shared<FlexCounter>(instanceId, m_vendorSai, m_dbCounters);
bool supportingBulk = (m_supportingBulkGroups.find(instanceId) != std::string::npos);
auto counter = std::make_shared<FlexCounter>(instanceId, m_vendorSai, m_dbCounters, supportingBulk);

m_flexCounters[instanceId] = counter;
}
Expand Down
5 changes: 4 additions & 1 deletion syncd/FlexCounterManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ namespace syncd

FlexCounterManager(
_In_ std::shared_ptr<sairedis::SaiInterface> vendorSai,
_In_ const std::string& dbCounters);
_In_ const std::string& dbCounters,
_In_ const std::string& supportingBulkInstances);

virtual ~FlexCounterManager() = default;

Expand Down Expand Up @@ -50,6 +51,8 @@ namespace syncd
std::shared_ptr<sairedis::SaiInterface> m_vendorSai;

std::string m_dbCounters;

std::string m_supportingBulkGroups;
};
}

2 changes: 1 addition & 1 deletion syncd/Syncd.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ Syncd::Syncd(
m_enableSyncMode = true;
}

m_manager = std::make_shared<FlexCounterManager>(m_vendorSai, m_contextConfig->m_dbCounters);
m_manager = std::make_shared<FlexCounterManager>(m_vendorSai, m_contextConfig->m_dbCounters, m_commandLineOptions->m_supportingBulkCounterGroups);

loadProfileMap();

Expand Down
5 changes: 5 additions & 0 deletions syncd/scripts/syncd_init_common.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 8 additions & 3 deletions unittest/syncd/TestCommandLineOptions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
)";
Expand All @@ -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)
Expand All @@ -75,8 +77,11 @@ TEST(CommandLineOptionsParser, parseCommandLine)
char arg1[] = "test";
char arg2[] = "-w";
char arg3[] = "1000";
std::vector<char *> args = {arg1, arg2, arg3};
char arg4[] = "-B";
char arg5[] = "WATERMARK";
std::vector<char *> 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");
}
2 changes: 1 addition & 1 deletion unittest/syncd/TestFlexCounter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<swss::FieldValueTuple> values;
values.emplace_back(pluginFieldName, "dummy_sha_strings");
Expand Down

0 comments on commit 0bd015e

Please sign in to comment.