From 8e773f69a943f8d76148fa7acb1177a16996aa90 Mon Sep 17 00:00:00 2001 From: Stephen Sun Date: Thu, 5 Sep 2024 10:54:49 +0000 Subject: [PATCH 1/4] Support setting bulk size Signed-off-by: Stephen Sun --- lib/RedisRemoteSaiInterface.cpp | 1 + lib/sairedis.h | 7 +++ syncd/FlexCounter.cpp | 75 +++++++++++++++++++++++++++------ syncd/FlexCounter.h | 4 ++ 4 files changed, 75 insertions(+), 12 deletions(-) diff --git a/lib/RedisRemoteSaiInterface.cpp b/lib/RedisRemoteSaiInterface.cpp index 4c0247109..bb7c7ac60 100644 --- a/lib/RedisRemoteSaiInterface.cpp +++ b/lib/RedisRemoteSaiInterface.cpp @@ -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); diff --git a/lib/sairedis.h b/lib/sairedis.h index 56a21bd4d..e3b8ab850 100644 --- a/lib/sairedis.h +++ b/lib/sairedis.h @@ -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 diff --git a/syncd/FlexCounter.cpp b/syncd/FlexCounter.cpp index e77f982f4..7625dbabb 100644 --- a/syncd/FlexCounter.cpp +++ b/syncd/FlexCounter.cpp @@ -72,6 +72,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 struct CounterIds @@ -592,6 +599,9 @@ class CounterContext : public BaseCounterContext { return; } + + SWSS_LOG_INFO("Before running plugin %s", m_name.c_str()); + std::vector idStrings; idStrings.reserve(m_objectIdsMap.size()); std::transform(m_objectIdsMap.begin(), @@ -610,6 +620,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", m_name.c_str()); } bool hasObject() const override @@ -712,19 +724,40 @@ 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(ctx.object_keys.size()), - ctx.object_keys.data(), - static_cast(ctx.counter_ids.size()), - reinterpret_cast(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(ctx.object_keys.size()); + if (bulk_size > size || bulk_size == 0) + { + bulk_size = size; + } + uint32_t current = 0; + + SWSS_LOG_INFO("Before getting bulk %s size %u bulk_size %u current %u", m_name.c_str(), size, bulk_size, current); + + while (current < size) { - SWSS_LOG_WARN("Failed to bulk get stats for %s: %u", m_name.c_str(), status); + sai_status_t status = m_vendorSai->bulkGetStats( + SAI_NULL_OBJECT_ID, + m_objectType, + bulk_size, + ctx.object_keys.data() + current, + static_cast(ctx.counter_ids.size()), + reinterpret_cast(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 index %u(advanced to %u) bulk_size %u", m_name.c_str(), current - bulk_size, current, bulk_size); + + if (size - current < bulk_size) + { + bulk_size = size - current; + } } std::vector values; @@ -744,6 +777,8 @@ class CounterContext : public BaseCounterContext countersTable.set(sai_serialize_object_id(vid), values, ""); values.clear(); } + + SWSS_LOG_INFO("After pushing db %s", m_name.c_str()); } auto getBulkStatsContext( @@ -1150,6 +1185,7 @@ void FlexCounter::addCounterPlugin( SWSS_LOG_ENTER(); m_isDiscarded = false; + uint32_t bulkSize = 0; for (auto& fvt: values) { @@ -1162,6 +1198,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); @@ -1184,6 +1229,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 { diff --git a/syncd/FlexCounter.h b/syncd/FlexCounter.h index 18d3f3af1..805d07c3d 100644 --- a/syncd/FlexCounter.h +++ b/syncd/FlexCounter.h @@ -27,6 +27,9 @@ namespace syncd void setNoDoubleCheckBulkCapability( _In_ bool); + void setBulkSize( + _In_ uint32_t bulkSize); + bool hasPlugin() const {return !m_plugins.empty();} void removePlugins() {m_plugins.clear();} @@ -59,6 +62,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 { From d15f4c558068fc488771e44022fbf4db0681a0b2 Mon Sep 17 00:00:00 2001 From: Stephen Sun Date: Mon, 30 Sep 2024 09:13:36 +0300 Subject: [PATCH 2/4] More precise timestamps Signed-off-by: Stephen Sun --- syncd/FlexCounter.cpp | 49 ++++++++++++++++++++++++------------------- syncd/FlexCounter.h | 6 ++++-- 2 files changed, 32 insertions(+), 23 deletions(-) diff --git a/syncd/FlexCounter.cpp b/syncd/FlexCounter.cpp index 7625dbabb..19626f74f 100644 --- a/syncd/FlexCounter.cpp +++ b/syncd/FlexCounter.cpp @@ -40,8 +40,9 @@ const std::map 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(); } @@ -412,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(); } @@ -760,6 +762,8 @@ class CounterContext : public BaseCounterContext } } + auto time_stamp = std::chrono::steady_clock::now().time_since_epoch().count(); + std::vector values; for (size_t i = 0; i < ctx.object_keys.size(); i++) { @@ -774,6 +778,7 @@ 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(); } @@ -985,10 +990,11 @@ class AttrContext : public CounterContext typedef CounterContext 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(name, object_type, vendor_sai, stats_mode) + CounterContext(name, instance, object_type, vendor_sai, stats_mode) { SWSS_LOG_ENTER(); } @@ -1294,18 +1300,19 @@ bool FlexCounter::allPluginsEmpty() const } std::shared_ptr 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>(context_name, SAI_OBJECT_TYPE_PORT, m_vendorSai.get(), m_statsMode); + auto context = std::make_shared>(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>(context_name, SAI_OBJECT_TYPE_PORT, m_vendorSai.get(), m_statsMode); + auto context = std::make_shared>(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; @@ -1314,25 +1321,25 @@ std::shared_ptr FlexCounter::createCounterContext( } else if (context_name == COUNTER_TYPE_QUEUE) { - auto context = std::make_shared>(context_name, SAI_OBJECT_TYPE_QUEUE, m_vendorSai.get(), m_statsMode); + auto context = std::make_shared>(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>(context_name, SAI_OBJECT_TYPE_INGRESS_PRIORITY_GROUP, m_vendorSai.get(), m_statsMode); + auto context = std::make_shared>(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>(context_name, SAI_OBJECT_TYPE_ROUTER_INTERFACE, m_vendorSai.get(), m_statsMode); + return std::make_shared>(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>(context_name, SAI_OBJECT_TYPE_SWITCH, m_vendorSai.get(), m_statsMode); + auto context = std::make_shared>(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; @@ -1341,19 +1348,19 @@ std::shared_ptr FlexCounter::createCounterContext( } else if (context_name == COUNTER_TYPE_MACSEC_FLOW) { - auto context = std::make_shared>(context_name, SAI_OBJECT_TYPE_MACSEC_FLOW, m_vendorSai.get(), m_statsMode); + auto context = std::make_shared>(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>(context_name, SAI_OBJECT_TYPE_MACSEC_SA, m_vendorSai.get(), m_statsMode); + auto context = std::make_shared>(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>(context_name, SAI_OBJECT_TYPE_COUNTER, m_vendorSai.get(), m_statsMode); + auto context = std::make_shared>(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; @@ -1361,13 +1368,13 @@ std::shared_ptr FlexCounter::createCounterContext( } else if (context_name == COUNTER_TYPE_TUNNEL) { - auto context = std::make_shared>(context_name, SAI_OBJECT_TYPE_TUNNEL, m_vendorSai.get(), m_statsMode); + auto context = std::make_shared>(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>(context_name, SAI_OBJECT_TYPE_BUFFER_POOL, m_vendorSai.get(), m_statsMode); + auto context = std::make_shared>(context_name, instance, SAI_OBJECT_TYPE_BUFFER_POOL, m_vendorSai.get(), m_statsMode); context->always_check_supported_counters = true; return context; } @@ -1379,19 +1386,19 @@ std::shared_ptr FlexCounter::createCounterContext( } else if (context_name == ATTR_TYPE_QUEUE) { - return std::make_shared>(context_name, SAI_OBJECT_TYPE_QUEUE, m_vendorSai.get(), m_statsMode); + return std::make_shared>(context_name, instance, SAI_OBJECT_TYPE_QUEUE, m_vendorSai.get(), m_statsMode); } else if (context_name == ATTR_TYPE_PG) { - return std::make_shared>(context_name, SAI_OBJECT_TYPE_INGRESS_PRIORITY_GROUP, m_vendorSai.get(), m_statsMode); + return std::make_shared>(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>(context_name, SAI_OBJECT_TYPE_MACSEC_SA, m_vendorSai.get(), m_statsMode); + return std::make_shared>(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>(context_name, SAI_OBJECT_TYPE_ACL_COUNTER, m_vendorSai.get(), m_statsMode); + return std::make_shared>(context_name, instance, SAI_OBJECT_TYPE_ACL_COUNTER, m_vendorSai.get(), m_statsMode); } SWSS_LOG_THROW("Invalid counter type %s", context_name.c_str()); @@ -1410,7 +1417,7 @@ std::shared_ptr 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; } diff --git a/syncd/FlexCounter.h b/syncd/FlexCounter.h index 805d07c3d..9858c1eac 100644 --- a/syncd/FlexCounter.h +++ b/syncd/FlexCounter.h @@ -20,7 +20,7 @@ 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& shaStrings); @@ -54,6 +54,7 @@ namespace syncd protected: std::string m_name; + std::string m_instanceId; std::set m_plugins; public: @@ -122,7 +123,8 @@ namespace syncd _In_ const std::string &name); std::shared_ptr createCounterContext( - _In_ const std::string &name); + _In_ const std::string &name, + _In_ const std::string &instance); void removeCounterContext( _In_ const std::string &name); From f04641efe311365bbfdf8a831ff13d7224db22e3 Mon Sep 17 00:00:00 2001 From: Stephen Sun Date: Sun, 13 Oct 2024 09:30:03 +0000 Subject: [PATCH 3/4] More debug information Signed-off-by: Stephen Sun --- syncd/FlexCounter.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/syncd/FlexCounter.cpp b/syncd/FlexCounter.cpp index 19626f74f..6dfd1a13c 100644 --- a/syncd/FlexCounter.cpp +++ b/syncd/FlexCounter.cpp @@ -602,7 +602,7 @@ class CounterContext : public BaseCounterContext return; } - SWSS_LOG_INFO("Before running plugin %s", m_name.c_str()); + SWSS_LOG_INFO("Before running plugin %s %s", m_instanceId.c_str(), m_name.c_str()); std::vector idStrings; idStrings.reserve(m_objectIdsMap.size()); @@ -623,7 +623,7 @@ class CounterContext : public BaseCounterContext m_plugins.end(), [&] (auto &sha) { runRedisScript(counters_db, sha, idStrings, argv); }); - SWSS_LOG_INFO("After running plugin %s", m_name.c_str()); + SWSS_LOG_INFO("After running plugin %s %s", m_instanceId.c_str(), m_name.c_str()); } bool hasObject() const override @@ -734,7 +734,7 @@ class CounterContext : public BaseCounterContext } uint32_t current = 0; - SWSS_LOG_INFO("Before getting bulk %s size %u bulk_size %u current %u", m_name.c_str(), size, bulk_size, current); + 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) { @@ -754,7 +754,7 @@ class CounterContext : public BaseCounterContext } current += bulk_size; - SWSS_LOG_INFO("After getting bulk %s index %u(advanced to %u) bulk_size %u", m_name.c_str(), current - bulk_size, 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) { @@ -783,7 +783,7 @@ class CounterContext : public BaseCounterContext values.clear(); } - SWSS_LOG_INFO("After pushing db %s", m_name.c_str()); + SWSS_LOG_INFO("After pushing db %s %s", m_instanceId.c_str(), m_name.c_str()); } auto getBulkStatsContext( From b82e2339019d6115bf9ce68b70bc36fb3cb478fa Mon Sep 17 00:00:00 2001 From: Stephen Sun Date: Tue, 29 Oct 2024 16:28:19 +0200 Subject: [PATCH 4/4] Adjust code according to latest community Signed-off-by: Stephen Sun --- syncd/FlexCounter.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/syncd/FlexCounter.cpp b/syncd/FlexCounter.cpp index 6dfd1a13c..edafa0d38 100644 --- a/syncd/FlexCounter.cpp +++ b/syncd/FlexCounter.cpp @@ -1380,7 +1380,7 @@ std::shared_ptr FlexCounter::createCounterContext( } else if (context_name == COUNTER_TYPE_ENI) { - auto context = std::make_shared>(context_name, (sai_object_type_t)SAI_OBJECT_TYPE_ENI, m_vendorSai.get(), m_statsMode); + auto context = std::make_shared>(context_name, instance, (sai_object_type_t)SAI_OBJECT_TYPE_ENI, m_vendorSai.get(), m_statsMode); context->always_check_supported_counters = true; return context; }