Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimize counter polling interval by making it more accurate #1457

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Optimize counter polling interval by making it more accurate #1457

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8e773f6
Support setting bulk size
stephenxs Sep 5, 2024
d15f4c5
More precise timestamps
stephenxs Sep 30, 2024
f04641e
More debug information
stephenxs Oct 13, 2024
b82e233
Adjust code according to latest community
stephenxs Oct 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions lib/RedisRemoteSaiInterface.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -568,6 +568,7 @@ sai_status_t RedisRemoteSaiInterface::notifyCounterGroupOperations(
std::string key((const char*)flexCounterGroupParam->counter_group_name.list, flexCounterGroupParam->counter_group_name.count);

emplaceStrings(POLL_INTERVAL_FIELD, flexCounterGroupParam->poll_interval, entries);
emplaceStrings(BULK_SIZE_FIELD, flexCounterGroupParam->bulk_size, entries);
emplaceStrings(STATS_MODE_FIELD, flexCounterGroupParam->stats_mode, entries);
emplaceStrings(flexCounterGroupParam->plugin_name, flexCounterGroupParam->plugins, entries);
emplaceStrings(FLEX_COUNTER_STATUS_FIELD, flexCounterGroupParam->operation, entries);
Expand Down
7 changes: 7 additions & 0 deletions lib/sairedis.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,13 @@ typedef struct _sai_redis_flex_counter_group_parameter_t
*/
sai_s8_list_t plugins;

/**
* @brief The bulk size of the counter group
*
* It should be a number representing the bulk size.
*/
sai_s8_list_t bulk_size;

} sai_redis_flex_counter_group_parameter_t;

typedef struct _sai_redis_flex_counter_parameter_t
Expand Down
126 changes: 92 additions & 34 deletions syncd/FlexCounter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,9 @@ const std::map<std::string, std::string> FlexCounter::m_plugIn2CounterType = {
{TUNNEL_PLUGIN_FIELD, COUNTER_TYPE_TUNNEL},
{FLOW_COUNTER_PLUGIN_FIELD, COUNTER_TYPE_FLOW}};

BaseCounterContext::BaseCounterContext(const std::string &name):
m_name(name)
BaseCounterContext::BaseCounterContext(const std::string &name, const std::string &instance):
m_name(name),
m_instanceId(instance)
{
SWSS_LOG_ENTER();
}
Expand Down Expand Up @@ -72,6 +73,13 @@ void BaseCounterContext::setNoDoubleCheckBulkCapability(
no_double_check_bulk_capability = noDoubleCheckBulkCapability;
}

void BaseCounterContext::setBulkSize(
_In_ uint32_t bulkSize)
{
SWSS_LOG_ENTER();
default_bulk_size = bulkSize;
}

template <typename StatType,
typename Enable = void>
struct CounterIds
Expand Down Expand Up @@ -405,10 +413,11 @@ class CounterContext : public BaseCounterContext

CounterContext(
_In_ const std::string &name,
_In_ const std::string &instance,
_In_ sai_object_type_t object_type,
_In_ sairedis::SaiInterface *vendor_sai,
_In_ sai_stats_mode_t &stats_mode):
BaseCounterContext(name), m_objectType(object_type), m_vendorSai(vendor_sai), m_groupStatsMode(stats_mode)
BaseCounterContext(name, instance), m_objectType(object_type), m_vendorSai(vendor_sai), m_groupStatsMode(stats_mode)
{
SWSS_LOG_ENTER();
}
Expand Down Expand Up @@ -592,6 +601,9 @@ class CounterContext : public BaseCounterContext
{
return;
}

SWSS_LOG_INFO("Before running plugin %s %s", m_instanceId.c_str(), m_name.c_str());

std::vector<std::string> idStrings;
idStrings.reserve(m_objectIdsMap.size());
std::transform(m_objectIdsMap.begin(),
Expand All @@ -610,6 +622,8 @@ class CounterContext : public BaseCounterContext
std::for_each(m_plugins.begin(),
m_plugins.end(),
[&] (auto &sha) { runRedisScript(counters_db, sha, idStrings, argv); });

SWSS_LOG_INFO("After running plugin %s %s", m_instanceId.c_str(), m_name.c_str());
}

bool hasObject() const override
Expand Down Expand Up @@ -712,20 +726,43 @@ class CounterContext : public BaseCounterContext
{
SWSS_LOG_ENTER();
auto statsMode = m_groupStatsMode == SAI_STATS_MODE_READ ? SAI_STATS_MODE_BULK_READ : SAI_STATS_MODE_BULK_READ_AND_CLEAR;
sai_status_t status = m_vendorSai->bulkGetStats(
SAI_NULL_OBJECT_ID,
m_objectType,
static_cast<uint32_t>(ctx.object_keys.size()),
ctx.object_keys.data(),
static_cast<uint32_t>(ctx.counter_ids.size()),
reinterpret_cast<const sai_stat_id_t *>(ctx.counter_ids.data()),
statsMode,
ctx.object_statuses.data(),
ctx.counters.data());
if (SAI_STATUS_SUCCESS != status)
uint32_t bulk_size = default_bulk_size;
uint32_t size = static_cast<uint32_t>(ctx.object_keys.size());
if (bulk_size > size || bulk_size == 0)
{
SWSS_LOG_WARN("Failed to bulk get stats for %s: %u", m_name.c_str(), status);
bulk_size = size;
}
uint32_t current = 0;

SWSS_LOG_INFO("Before getting bulk %s %s size %u bulk_size %u current %u", m_instanceId.c_str(), m_name.c_str(), size, bulk_size, current);

while (current < size)
{
sai_status_t status = m_vendorSai->bulkGetStats(
SAI_NULL_OBJECT_ID,
m_objectType,
bulk_size,
ctx.object_keys.data() + current,
static_cast<uint32_t>(ctx.counter_ids.size()),
reinterpret_cast<const sai_stat_id_t *>(ctx.counter_ids.data()),
statsMode,
ctx.object_statuses.data() + current,
ctx.counters.data() + current * ctx.counter_ids.size());
if (SAI_STATUS_SUCCESS != status)
{
SWSS_LOG_WARN("Failed to bulk get stats for %s: %u", m_name.c_str(), status);
}
current += bulk_size;

SWSS_LOG_INFO("After getting bulk %s %s index %u(advanced to %u) bulk_size %u", m_instanceId.c_str(), m_name.c_str(), current - bulk_size, current, bulk_size);

if (size - current < bulk_size)
{
bulk_size = size - current;
}
}

auto time_stamp = std::chrono::steady_clock::now().time_since_epoch().count();

std::vector<swss::FieldValueTuple> values;
for (size_t i = 0; i < ctx.object_keys.size(); i++)
Expand All @@ -741,9 +778,12 @@ class CounterContext : public BaseCounterContext
{
values.emplace_back(serializeStat(ctx.counter_ids[j]), std::to_string(ctx.counters[i * ctx.counter_ids.size() + j]));
}
values.emplace_back(m_instanceId + "_time_stamp", std::to_string(time_stamp));
countersTable.set(sai_serialize_object_id(vid), values, "");
values.clear();
}

SWSS_LOG_INFO("After pushing db %s %s", m_instanceId.c_str(), m_name.c_str());
}

auto getBulkStatsContext(
Expand Down Expand Up @@ -950,10 +990,11 @@ class AttrContext : public CounterContext<AttrType>
typedef CounterContext<AttrType> Base;
AttrContext(
_In_ const std::string &name,
_In_ const std::string &instance,
_In_ sai_object_type_t object_type,
_In_ sairedis::SaiInterface *vendor_sai,
_In_ sai_stats_mode_t &stats_mode):
CounterContext<AttrType>(name, object_type, vendor_sai, stats_mode)
CounterContext<AttrType>(name, instance, object_type, vendor_sai, stats_mode)
{
SWSS_LOG_ENTER();
}
Expand Down Expand Up @@ -1150,6 +1191,7 @@ void FlexCounter::addCounterPlugin(
SWSS_LOG_ENTER();

m_isDiscarded = false;
uint32_t bulkSize = 0;

for (auto& fvt: values)
{
Expand All @@ -1162,6 +1204,15 @@ void FlexCounter::addCounterPlugin(
{
setPollInterval(stoi(value));
}
else if (field == BULK_SIZE_FIELD)
{
bulkSize = stoi(value);
for (auto &context : m_counterContext)
{
context.second->setBulkSize(bulkSize);
SWSS_LOG_NOTICE("Set counter context %s %s bulk size %u", m_instanceId.c_str(), COUNTER_TYPE_PORT.c_str(), bulkSize);
}
}
else if (field == FLEX_COUNTER_STATUS_FIELD)
{
setStatus(value);
Expand All @@ -1184,6 +1235,12 @@ void FlexCounter::addCounterPlugin(

SWSS_LOG_NOTICE("Do not double check bulk capability counter context %s %s", m_instanceId.c_str(), counterTypeRef->second.c_str());
}

if (bulkSize > 0)
{
getCounterContext(counterTypeRef->second)->setBulkSize(bulkSize);
SWSS_LOG_NOTICE("Create counter context %s %s with bulk size %u", m_instanceId.c_str(), counterTypeRef->second.c_str(), bulkSize);
}
}
else
{
Expand Down Expand Up @@ -1243,18 +1300,19 @@ bool FlexCounter::allPluginsEmpty() const
}

std::shared_ptr<BaseCounterContext> FlexCounter::createCounterContext(
_In_ const std::string& context_name)
_In_ const std::string& context_name,
_In_ const std::string& instance)
{
SWSS_LOG_ENTER();
if (context_name == COUNTER_TYPE_PORT)
{
auto context = std::make_shared<CounterContext<sai_port_stat_t>>(context_name, SAI_OBJECT_TYPE_PORT, m_vendorSai.get(), m_statsMode);
auto context = std::make_shared<CounterContext<sai_port_stat_t>>(context_name, instance, SAI_OBJECT_TYPE_PORT, m_vendorSai.get(), m_statsMode);
context->always_check_supported_counters = true;
return context;
}
else if (context_name == COUNTER_TYPE_PORT_DEBUG)
{
auto context = std::make_shared<CounterContext<sai_port_stat_t>>(context_name, SAI_OBJECT_TYPE_PORT, m_vendorSai.get(), m_statsMode);
auto context = std::make_shared<CounterContext<sai_port_stat_t>>(context_name, instance, SAI_OBJECT_TYPE_PORT, m_vendorSai.get(), m_statsMode);
context->always_check_supported_counters = true;
context->use_sai_stats_capa_query = false;
context->use_sai_stats_ext = true;
Expand All @@ -1263,25 +1321,25 @@ std::shared_ptr<BaseCounterContext> FlexCounter::createCounterContext(
}
else if (context_name == COUNTER_TYPE_QUEUE)
{
auto context = std::make_shared<CounterContext<sai_queue_stat_t>>(context_name, SAI_OBJECT_TYPE_QUEUE, m_vendorSai.get(), m_statsMode);
auto context = std::make_shared<CounterContext<sai_queue_stat_t>>(context_name, instance, SAI_OBJECT_TYPE_QUEUE, m_vendorSai.get(), m_statsMode);
context->always_check_supported_counters = true;
context->double_confirm_supported_counters = true;
return context;
}
else if (context_name == COUNTER_TYPE_PG)
{
auto context = std::make_shared<CounterContext<sai_ingress_priority_group_stat_t>>(context_name, SAI_OBJECT_TYPE_INGRESS_PRIORITY_GROUP, m_vendorSai.get(), m_statsMode);
auto context = std::make_shared<CounterContext<sai_ingress_priority_group_stat_t>>(context_name, instance, SAI_OBJECT_TYPE_INGRESS_PRIORITY_GROUP, m_vendorSai.get(), m_statsMode);
context->always_check_supported_counters = true;
context->double_confirm_supported_counters = true;
return context;
}
else if (context_name == COUNTER_TYPE_RIF)
{
return std::make_shared<CounterContext<sai_router_interface_stat_t>>(context_name, SAI_OBJECT_TYPE_ROUTER_INTERFACE, m_vendorSai.get(), m_statsMode);
return std::make_shared<CounterContext<sai_router_interface_stat_t>>(context_name, instance, SAI_OBJECT_TYPE_ROUTER_INTERFACE, m_vendorSai.get(), m_statsMode);
}
else if (context_name == COUNTER_TYPE_SWITCH_DEBUG)
{
auto context = std::make_shared<CounterContext<sai_switch_stat_t>>(context_name, SAI_OBJECT_TYPE_SWITCH, m_vendorSai.get(), m_statsMode);
auto context = std::make_shared<CounterContext<sai_switch_stat_t>>(context_name, instance, SAI_OBJECT_TYPE_SWITCH, m_vendorSai.get(), m_statsMode);
context->always_check_supported_counters = true;
context->use_sai_stats_capa_query = false;
context->use_sai_stats_ext = true;
Expand All @@ -1290,57 +1348,57 @@ std::shared_ptr<BaseCounterContext> FlexCounter::createCounterContext(
}
else if (context_name == COUNTER_TYPE_MACSEC_FLOW)
{
auto context = std::make_shared<CounterContext<sai_macsec_flow_stat_t>>(context_name, SAI_OBJECT_TYPE_MACSEC_FLOW, m_vendorSai.get(), m_statsMode);
auto context = std::make_shared<CounterContext<sai_macsec_flow_stat_t>>(context_name, instance, SAI_OBJECT_TYPE_MACSEC_FLOW, m_vendorSai.get(), m_statsMode);
context->use_sai_stats_capa_query = false;
return context;
}
else if (context_name == COUNTER_TYPE_MACSEC_SA)
{
auto context = std::make_shared<CounterContext<sai_macsec_sa_stat_t>>(context_name, SAI_OBJECT_TYPE_MACSEC_SA, m_vendorSai.get(), m_statsMode);
auto context = std::make_shared<CounterContext<sai_macsec_sa_stat_t>>(context_name, instance, SAI_OBJECT_TYPE_MACSEC_SA, m_vendorSai.get(), m_statsMode);
context->use_sai_stats_capa_query = false;
return context;
}
else if (context_name == COUNTER_TYPE_FLOW)
{
auto context = std::make_shared<CounterContext<sai_counter_stat_t>>(context_name, SAI_OBJECT_TYPE_COUNTER, m_vendorSai.get(), m_statsMode);
auto context = std::make_shared<CounterContext<sai_counter_stat_t>>(context_name, instance, SAI_OBJECT_TYPE_COUNTER, m_vendorSai.get(), m_statsMode);
context->use_sai_stats_capa_query = false;
context->use_sai_stats_ext = true;

return context;
}
else if (context_name == COUNTER_TYPE_TUNNEL)
{
auto context = std::make_shared<CounterContext<sai_tunnel_stat_t>>(context_name, SAI_OBJECT_TYPE_TUNNEL, m_vendorSai.get(), m_statsMode);
auto context = std::make_shared<CounterContext<sai_tunnel_stat_t>>(context_name, instance, SAI_OBJECT_TYPE_TUNNEL, m_vendorSai.get(), m_statsMode);
context->use_sai_stats_capa_query = false;
return context;
}
else if (context_name == COUNTER_TYPE_BUFFER_POOL)
{
auto context = std::make_shared<CounterContext<sai_buffer_pool_stat_t>>(context_name, SAI_OBJECT_TYPE_BUFFER_POOL, m_vendorSai.get(), m_statsMode);
auto context = std::make_shared<CounterContext<sai_buffer_pool_stat_t>>(context_name, instance, SAI_OBJECT_TYPE_BUFFER_POOL, m_vendorSai.get(), m_statsMode);
context->always_check_supported_counters = true;
return context;
}
else if (context_name == COUNTER_TYPE_ENI)
{
auto context = std::make_shared<CounterContext<sai_eni_stat_t>>(context_name, (sai_object_type_t)SAI_OBJECT_TYPE_ENI, m_vendorSai.get(), m_statsMode);
auto context = std::make_shared<CounterContext<sai_eni_stat_t>>(context_name, instance, (sai_object_type_t)SAI_OBJECT_TYPE_ENI, m_vendorSai.get(), m_statsMode);
context->always_check_supported_counters = true;
return context;
}
else if (context_name == ATTR_TYPE_QUEUE)
{
return std::make_shared<AttrContext<sai_queue_attr_t>>(context_name, SAI_OBJECT_TYPE_QUEUE, m_vendorSai.get(), m_statsMode);
return std::make_shared<AttrContext<sai_queue_attr_t>>(context_name, instance, SAI_OBJECT_TYPE_QUEUE, m_vendorSai.get(), m_statsMode);
}
else if (context_name == ATTR_TYPE_PG)
{
return std::make_shared<AttrContext<sai_ingress_priority_group_attr_t>>(context_name, SAI_OBJECT_TYPE_INGRESS_PRIORITY_GROUP, m_vendorSai.get(), m_statsMode);
return std::make_shared<AttrContext<sai_ingress_priority_group_attr_t>>(context_name, instance, SAI_OBJECT_TYPE_INGRESS_PRIORITY_GROUP, m_vendorSai.get(), m_statsMode);
}
else if (context_name == ATTR_TYPE_MACSEC_SA)
{
return std::make_shared<AttrContext<sai_macsec_sa_attr_t>>(context_name, SAI_OBJECT_TYPE_MACSEC_SA, m_vendorSai.get(), m_statsMode);
return std::make_shared<AttrContext<sai_macsec_sa_attr_t>>(context_name, instance, SAI_OBJECT_TYPE_MACSEC_SA, m_vendorSai.get(), m_statsMode);
}
else if (context_name == ATTR_TYPE_ACL_COUNTER)
{
return std::make_shared<AttrContext<sai_acl_counter_attr_t>>(context_name, SAI_OBJECT_TYPE_ACL_COUNTER, m_vendorSai.get(), m_statsMode);
return std::make_shared<AttrContext<sai_acl_counter_attr_t>>(context_name, instance, SAI_OBJECT_TYPE_ACL_COUNTER, m_vendorSai.get(), m_statsMode);
}

SWSS_LOG_THROW("Invalid counter type %s", context_name.c_str());
Expand All @@ -1359,7 +1417,7 @@ std::shared_ptr<BaseCounterContext> FlexCounter::getCounterContext(
return iter->second;
}

auto ret = m_counterContext.emplace(name, createCounterContext(name));
auto ret = m_counterContext.emplace(name, createCounterContext(name, m_instanceId));
return ret.first->second;
}

Expand Down
10 changes: 8 additions & 2 deletions syncd/FlexCounter.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,16 @@ namespace syncd
class BaseCounterContext
{
public:
BaseCounterContext(const std::string &name);
BaseCounterContext(const std::string &name, const std::string &instance);
void addPlugins(
_In_ const std::vector<std::string>& shaStrings);

void setNoDoubleCheckBulkCapability(
_In_ bool);

void setBulkSize(
_In_ uint32_t bulkSize);

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

void removePlugins() {m_plugins.clear();}
Expand All @@ -51,6 +54,7 @@ namespace syncd

protected:
std::string m_name;
std::string m_instanceId;
std::set<std::string> m_plugins;

public:
Expand All @@ -59,6 +63,7 @@ namespace syncd
bool use_sai_stats_ext = false;
bool double_confirm_supported_counters = false;
bool no_double_check_bulk_capability = false;
uint32_t default_bulk_size = 0;
};
class FlexCounter
{
Expand Down Expand Up @@ -118,7 +123,8 @@ namespace syncd
_In_ const std::string &name);

std::shared_ptr<BaseCounterContext> createCounterContext(
_In_ const std::string &name);
_In_ const std::string &name,
_In_ const std::string &instance);

void removeCounterContext(
_In_ const std::string &name);
Expand Down
Loading