From ece0295ee5df49ec5496570405381941c4db9f5f Mon Sep 17 00:00:00 2001 From: Justin Wong Date: Thu, 25 Jun 2026 18:00:33 +0000 Subject: [PATCH 1/4] Cast counter enhancement from 202511 Signed-off-by: Justin Wong --- syncd/FlexCounter.cpp | 807 ++++++++++++++++++++++++++++- syncd/FlexCounter.h | 13 + unittest/syncd/TestFlexCounter.cpp | 386 +++++++++++++- 3 files changed, 1183 insertions(+), 23 deletions(-) diff --git a/syncd/FlexCounter.cpp b/syncd/FlexCounter.cpp index 948c0b2b40..a2a88dbe59 100644 --- a/syncd/FlexCounter.cpp +++ b/syncd/FlexCounter.cpp @@ -538,7 +538,21 @@ class CounterContext : public BaseCounterContext protected: sai_object_id_t m_switchId = SAI_NULL_OBJECT_ID; - + struct CounterGroupRef + { + size_t idx; + size_t size; + }; + CounterGroupRef makeCounterGroupRef(size_t idx, size_t size) + { + SWSS_LOG_ENTER(); + return {idx, size}; + } + std::set* getSortedCGRef(CounterGroupRef const& cgr) + { + SWSS_LOG_ENTER(); + return &m_supportedCounterGroups[cgr.idx]; + } public: typedef CounterIds CounterIdsType; typedef BulkStatsContext BulkContextType; @@ -554,6 +568,121 @@ class CounterContext : public BaseCounterContext SWSS_LOG_ENTER(); } + virtual void addObjectWithCounterGroups( + _In_ sai_object_id_t vid, + _In_ sai_object_id_t rid, + _In_ const std::vector &idStrings, + _In_ const std::string &per_object_stats_mode) override + { + SWSS_LOG_ENTER(); + sai_stats_mode_t instance_stats_mode = SAI_STATS_MODE_READ_AND_CLEAR; + sai_stats_mode_t effective_stats_mode; + // TODO: use if const expression when c++17 is supported + if (HasStatsMode::value) + { + if (per_object_stats_mode == STATS_MODE_READ_AND_CLEAR) + { + instance_stats_mode = SAI_STATS_MODE_READ_AND_CLEAR; + } + else if (per_object_stats_mode == STATS_MODE_READ) + { + instance_stats_mode = SAI_STATS_MODE_READ; + } + else + { + SWSS_LOG_WARN("Stats mode %s not supported for flex counter. Using STATS_MODE_READ_AND_CLEAR", per_object_stats_mode.c_str()); + } + + effective_stats_mode = (m_groupStatsMode == SAI_STATS_MODE_READ_AND_CLEAR || + instance_stats_mode == SAI_STATS_MODE_READ_AND_CLEAR) ? SAI_STATS_MODE_READ_AND_CLEAR : SAI_STATS_MODE_READ; + } + else + { + effective_stats_mode = m_groupStatsMode; + } + + std::vector counter_ids; + for (const auto &str : idStrings) + { + StatType stat; + deserializeStat(str.c_str(), &stat); + counter_ids.push_back(stat); + } + + std::set counter_ids_set = setupBaseCounterGroup(rid, counter_ids, effective_stats_mode); + counter_ids = std::vector(counter_ids_set.begin(), counter_ids_set.end()); + updateSupportedCounterGroups(rid, vid, counter_ids, effective_stats_mode); + + if (m_objectSupportedCountersGroupMap.count(vid) == 0) + { + return; // This vid has no supported counters + } + size_t groupIndex = m_objectSupportedCountersGroupMap[vid]; + std::set& supportedIdsSet = m_supportedCounterGroups[groupIndex]; + if (supportedIdsSet.empty()) + { + return; + } + std::vector supportedIds(supportedIdsSet.begin(), supportedIdsSet.end()); + + if (double_confirm_supported_counters) + { + std::vector stats(supportedIds.size()); + + if (!collectData(rid, supportedIds, effective_stats_mode, false, stats)) + { + SWSS_LOG_ERROR("%s RID %s VID %s can't provide the statistic", m_name.c_str(), + sai_serialize_object_id(rid).c_str(), sai_serialize_object_id(vid).c_str()); + throw std::runtime_error("Test counter poll failed on populating m_objectIdsMap"); + } + } + + bool supportBulk; + // TODO: use if const expression when cpp17 is supported + if (HasStatsMode::value) + { + supportBulk = false; + } + else + { + supportBulk = no_double_check_bulk_capability || checkBulkCapability(vid, rid, supportedIds); + } + + if (!supportBulk) + { + auto counter_data = std::make_shared>(rid, supportedIds); + // TODO: use if const expression when cpp17 is supported + if (HasStatsMode::value) + { + counter_data->setStatsMode(instance_stats_mode); + } + m_objectIdsMap.emplace(vid, counter_data); + } + else if (m_counterChunkSizeMapFromPrefix.empty()) + { + std::sort(supportedIds.begin(), supportedIds.end()); + auto bulkContext = getBulkStatsContext(supportedIds, "default", default_bulk_chunk_size); + addBulkStatsContext(vid, rid, supportedIds, *bulkContext.get()); + } + else + { + std::map> counter_prefix_map; + std::vector default_partition; + createCounterBulkChunkSizePerPrefixPartition(supportedIds, counter_prefix_map, default_partition); + + for (auto &counterPrefix : counter_prefix_map) + { + std::sort(counterPrefix.second.begin(), counterPrefix.second.end()); + auto bulkContext = getBulkStatsContext(counterPrefix.second, counterPrefix.first, m_counterChunkSizeMapFromPrefix[counterPrefix.first]); + addBulkStatsContext(vid, rid, counterPrefix.second, *bulkContext.get()); + } + + std::sort(default_partition.begin(), default_partition.end()); + auto bulkContext = getBulkStatsContext(default_partition, "default", default_bulk_chunk_size); + addBulkStatsContext(vid, rid, default_partition, *bulkContext.get()); + } + } + // For those object type who support per object stats mode, e.g. buffer pool. virtual void addObject( _In_ sai_object_id_t vid, @@ -904,8 +1033,224 @@ class CounterContext : public BaseCounterContext } } + virtual void bulkAddObjectWithCounterGroups( + _In_ const std::vector& vids, + _In_ const std::vector& rids, + _In_ const std::vector& idStrings, + _In_ const std::string &per_object_stats_mode) override + { + SWSS_LOG_ENTER(); + sai_stats_mode_t effective_stats_mode; + // TODO: use if const expression when c++17 is supported + if (HasStatsMode::value) + { + // Bulk operation is not supported by the counter group. + SWSS_LOG_INFO("Counter group %s %s does not support bulk. Fallback to single call", m_name.c_str(), m_instanceId.c_str()); + + // Fall back to old way + for (size_t i = 0; i < vids.size(); i++) + { + auto rid = rids[i]; + auto vid = vids[i]; + addObjectWithCounterGroups(vid, rid, idStrings, per_object_stats_mode); + } + + return; + } + else + { + effective_stats_mode = m_groupStatsMode; + } + + std::vector allCounterIds; + for (const auto &str : idStrings) + { + StatType stat; + deserializeStat(str.c_str(), &stat); + allCounterIds.push_back(stat); + } + + std::set counter_ids_set = setupBaseCounterGroup(rids[0], allCounterIds, effective_stats_mode); + allCounterIds = std::vector(counter_ids_set.begin(), counter_ids_set.end()); + std::sort(allCounterIds.begin(), allCounterIds.end()); + for (size_t i = 0; i < vids.size(); i++) + { + updateSupportedCounterGroups(rids[i], vids[i], allCounterIds, effective_stats_mode); + } + + // Check if any counter group exists + if (m_objectSupportedCountersGroupMap.empty()) + { + SWSS_LOG_NOTICE("%s %s does not have supported counters", m_name.c_str(), m_instanceId.c_str()); + return; + } + + std::map, std::tuple> bulkUnsupportedCounters; + auto statsMode = m_groupStatsMode == SAI_STATS_MODE_READ ? SAI_STATS_MODE_BULK_READ : SAI_STATS_MODE_BULK_READ_AND_CLEAR; + auto checkAndUpdateBulkCapability = [&](const std::vector &counter_ids, const std::string &prefix, uint32_t bulk_chunk_size) + { + BulkContextType ctx; + // Check bulk capabilities again + std::vector supportedBulkIds; + sai_status_t status = SAI_STATUS_SUCCESS; + if (m_supportedBulkCounters.empty()) + { + status = querySupportedCounters(rids[0], statsMode, m_supportedBulkCounters); + } + if (status == SAI_STATUS_SUCCESS && !m_supportedBulkCounters.empty()) + { + for (auto stat : counter_ids) + { + if (m_supportedBulkCounters.count(stat) != 0) + { + supportedBulkIds.push_back(stat); + } + } + } + if (supportedBulkIds.size() < counter_ids.size()) + { + // Bulk polling is unsupported for the whole group but single polling is supported + // Add all objects to m_objectIdsMap so that they will be polled using single API + for (size_t i = 0; i < vids.size(); i++) + { + auto rid = rids[i]; + auto vid = vids[i]; + + size_t groupIndex = m_objectSupportedCountersGroupMap[vid]; + std::vector intf_counter_ids(m_supportedCounterGroups[groupIndex].begin(), m_supportedCounterGroups[groupIndex].end()); + if (intf_counter_ids.empty()) + { + continue; + } + std::vector stats(intf_counter_ids.size()); + if (!collectData(rid, intf_counter_ids, effective_stats_mode, false, stats)) + { + SWSS_LOG_ERROR("%s RID %s VID %s can't provide the statistic", m_name.c_str(), + sai_serialize_object_id(rid).c_str(), sai_serialize_object_id(vid).c_str()); + throw std::runtime_error("Test counter poll failed on populating m_objectIdsMap"); + } + auto it_vid = m_objectIdsMap.find(vid); + if (it_vid != m_objectIdsMap.end()) + { + // Remove and re-add if vid already exists + m_objectIdsMap.erase(it_vid); + } + + auto counter_data = std::make_shared>(rid, intf_counter_ids); + m_objectIdsMap.emplace(vid, counter_data); + SWSS_LOG_INFO("Fallback to single call for object 0x%" PRIx64, vid); + } + return; + } + + ctx.counter_ids = counter_ids; + addBulkStatsContext(vids, rids, counter_ids, ctx); + 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 (status == SAI_STATUS_SUCCESS) + { + auto bulkContext = getBulkStatsContext(counter_ids, prefix, bulk_chunk_size); + addBulkStatsContext(vids, rids, counter_ids, *bulkContext.get()); + } + else + { + // Bulk is not supported for this counter prefix + // Append it to bulkUnsupportedCounters + std::tuple value(prefix, bulk_chunk_size); + bulkUnsupportedCounters.emplace(counter_ids, value); + SWSS_LOG_INFO("Counters starting with %s do not support bulk. Fallback to single call for these counters", prefix.c_str()); + } + }; + + // Use counter group with the most counters + std::vector supportedIds(getSortedCGRef(m_counterGroupsSorted[0])->begin(), getSortedCGRef(m_counterGroupsSorted[0])->end()); + + if (m_counterChunkSizeMapFromPrefix.empty()) + { + std::sort(supportedIds.begin(), supportedIds.end()); + checkAndUpdateBulkCapability(supportedIds, "default", default_bulk_chunk_size); + } + else + { + std::map> counter_prefix_map; + std::vector default_partition; + createCounterBulkChunkSizePerPrefixPartition(supportedIds, counter_prefix_map, default_partition); + + for (auto &counterPrefix : counter_prefix_map) + { + std::sort(counterPrefix.second.begin(), counterPrefix.second.end()); + } + + std::sort(default_partition.begin(), default_partition.end()); + + for (auto &counterPrefix : counter_prefix_map) + { + checkAndUpdateBulkCapability(counterPrefix.second, counterPrefix.first, m_counterChunkSizeMapFromPrefix[counterPrefix.first]); + } + + checkAndUpdateBulkCapability(default_partition, "default", default_bulk_chunk_size); + } + + if (!bulkUnsupportedCounters.empty()) + { + SWSS_LOG_NOTICE("Partial counters do not support bulk. Re-check bulk capability for each object"); + + for (auto &it : bulkUnsupportedCounters) + { + std::vector bulkSupportedRIDs; + std::vector bulkSupportedVIDs; + for (size_t i = 0; i < vids.size(); i++) + { + auto rid = rids[i]; + auto vid = vids[i]; + std::vector stats(it.first.size()); + if (checkBulkCapability(vid, rid, it.first)) + { + bulkSupportedVIDs.push_back(vid); + bulkSupportedRIDs.push_back(rid); + } + else if (!double_confirm_supported_counters || collectData(rid, it.first, effective_stats_mode, false, stats)) + { + SWSS_LOG_INFO("Fallback to single call for object 0x%" PRIx64, vid); + + auto it_vid = m_objectIdsMap.find(vid); + if (it_vid != m_objectIdsMap.end()) + { + // Remove and re-add if vid already exists + m_objectIdsMap.erase(it_vid); + } + + size_t groupIndex = m_objectSupportedCountersGroupMap[vid]; + std::vector objCounterIds(m_supportedCounterGroups[groupIndex].begin(), + m_supportedCounterGroups[groupIndex].end()); + auto counter_data = std::make_shared>(rid, objCounterIds); + m_objectIdsMap.emplace(vid, counter_data); + } + else + { + SWSS_LOG_WARN("%s RID %s can't provide the statistic", m_name.c_str(), sai_serialize_object_id(rid).c_str()); + } + } + + if (!bulkSupportedVIDs.empty() && !bulkSupportedRIDs.empty()) + { + auto bulkContext = getBulkStatsContext(it.first, get<0>(it.second), get<1>(it.second)); + addBulkStatsContext(bulkSupportedVIDs, bulkSupportedRIDs, it.first, *bulkContext.get()); + } + } + } + } + virtual void bulkAddObject( - _In_ const std::vector& vids, + _In_ const std::vector& vids, _In_ const std::vector& rids, _In_ const std::vector& idStrings, _In_ const std::string &per_object_stats_mode) @@ -938,9 +1283,7 @@ class CounterContext : public BaseCounterContext { StatType stat; deserializeStat(str.c_str(), &stat); - { - allCounterIds.push_back(stat); - } + allCounterIds.push_back(stat); } updateSupportedCounters(rids[0]/*it is not really used*/, allCounterIds, effective_stats_mode); @@ -1119,6 +1462,45 @@ class CounterContext : public BaseCounterContext removeObject(vid, true); } + void cleanupCounterGroupMapping( + _In_ sai_object_id_t vid) + { + SWSS_LOG_ENTER(); + + auto groupIter = m_objectSupportedCountersGroupMap.find(vid); + if (groupIter == m_objectSupportedCountersGroupMap.end()) + { + return; + } + + size_t removedGroupIdx = groupIter->second; + m_objectSupportedCountersGroupMap.erase(groupIter); + + // Check if any other vid still references this group + bool referenced = false; + for (const auto& pair: m_objectSupportedCountersGroupMap) + { + if (pair.second == removedGroupIdx) + { + referenced = true; + break; + } + } + + if (!referenced) + { + // Remove from sorted list so updateSupportedCounterGroups won't match + m_counterGroupsSorted.erase( + std::remove_if(m_counterGroupsSorted.begin(), m_counterGroupsSorted.end(), + [removedGroupIdx](const CounterGroupRef& ref) { return ref.idx == removedGroupIdx; }), + m_counterGroupsSorted.end()); + + // Clear the group data (can't erase from vector without invalidating other indices) + m_supportedCounterGroups[removedGroupIdx].clear(); + } + } + + void removeObject( _In_ sai_object_id_t vid, _In_ bool log) @@ -1139,8 +1521,10 @@ class CounterContext : public BaseCounterContext sai_serialize_object_type(m_objectType).c_str(), sai_serialize_object_id(vid).c_str()); } + cleanupCounterGroupMapping(vid); } + virtual void collectData( _In_ swss::Table &countersTable) override { @@ -1159,9 +1543,24 @@ class CounterContext : public BaseCounterContext kv.second->getStatsMode() == SAI_STATS_MODE_READ_AND_CLEAR) ? SAI_STATS_MODE_READ_AND_CLEAR : SAI_STATS_MODE_READ; } - std::vector stats(statIds.size()); + std::vector stats(statIds.size(), 0); if (!collectData(rid, statIds, effective_stats_mode, true, stats)) { + if (m_failedPolls.find({rid, vid}) == m_failedPolls.end()) + { + m_failedPolls[{rid, vid}] = 1; + SWSS_LOG_DEBUG("counter read failed 1 time on RID 0x%" PRIx64 " on intf 0x%" PRIx64, rid, vid); + } + else if (m_failedPolls[{rid, vid}] < 3) + { + m_failedPolls[{rid, vid}] += 1; + SWSS_LOG_DEBUG("counter read failed %d times on RID 0x%" PRIx64 " on intf 0x%" PRIx64, m_failedPolls[{rid, vid}], rid, vid); + } + else if (m_failedPolls[{rid, vid}] == 3) + { + m_failedPolls[{rid, vid}] += 1; + SWSS_LOG_ERROR("counter read failed more than 3 times on RID 0x%" PRIx64 " on intf 0x%" PRIx64, rid, vid); + } continue; } @@ -1221,6 +1620,76 @@ class CounterContext : public BaseCounterContext } private: + std::set setupBaseCounterGroup( + _In_ sai_object_id_t rid, + _In_ const std::vector& counter_ids, + _In_ sai_stats_mode_t &stats_mode) + { + SWSS_LOG_ENTER(); + // Query supported counters if flag set + std::set counter_ids_set; + if (!use_sai_stats_capa_query || querySupportedCounters(rid, stats_mode, counter_ids_set) != SAI_STATUS_SUCCESS) + { + // Query disabled/failed, use supplied set + counter_ids_set = std::set(counter_ids.begin(), counter_ids.end()); + } + else + { + // Query succeeded, intersect with supplied set to not query unintended counters + std::set originalSet(counter_ids.begin(), counter_ids.end()); + std::set intersected; + std::set_intersection(counter_ids_set.begin(), counter_ids_set.end(), + originalSet.begin(), originalSet.end(), + std::inserter(intersected, intersected.begin())); + counter_ids_set = intersected; + } + + // Create base counter group if not yet instantiated + if (m_supportedCounterGroups.empty()) + { + m_supportedCounterGroups.push_back(counter_ids_set); + m_counterGroupsSorted.push_back(makeCounterGroupRef(0, counter_ids_set.size())); + } + // Replace base counter group if full counter set differs + else + { + bool groupExists = false; + for (size_t i = 0; i < m_supportedCounterGroups.size(); i++) + { + if (m_supportedCounterGroups[i] == counter_ids_set) + { + groupExists = true; + break; + } + } + if (!groupExists) + { + m_counterGroupsSorted.push_back(makeCounterGroupRef(m_supportedCounterGroups.size(), counter_ids_set.size())); + m_supportedCounterGroups.push_back(counter_ids_set); + // Must adhere to strict weak ordering + // Use lambda instead of static function to avoid SWSS_LOG_ENTER CI build requirement for better performance + std::sort(m_counterGroupsSorted.begin(), m_counterGroupsSorted.end(), + [](CounterGroupRef const& lhs, CounterGroupRef const& rhs) + { + if (lhs.idx == rhs.idx) + { + return false; // The sets are equivalent + } + else if (lhs.size == rhs.size) + { + return lhs.idx < rhs.idx; // order by earliest creation when size is same + } + else + { + return lhs.size > rhs.size; + } + }); + } + } + return counter_ids_set; + } + + bool isCounterSupported( _In_ StatType counter) const { @@ -1539,6 +2008,191 @@ class CounterContext : public BaseCounterContext return status == SAI_STATUS_SUCCESS; } + void updateSupportedCounterGroups( + _In_ sai_object_id_t rid, + _In_ sai_object_id_t vid, + _In_ const std::vector& counter_ids, + _In_ sai_stats_mode_t stats_mode) + { + SWSS_LOG_ENTER(); + if (m_objectSupportedCountersGroupMap.find(vid) != m_objectSupportedCountersGroupMap.end()) + { + if (!always_check_supported_counters) + { + SWSS_LOG_NOTICE("Ignore checking of supported counters"); + return; + } + // If there is already a counter group for this vid, and that counter group contains a counter that is not + // included in counter_ids, then it means we are handling a different set of counters. + // Remove the previous group and mappings to this vid + std::set existingGroup = m_supportedCounterGroups[m_objectSupportedCountersGroupMap[vid]]; + for (auto &counter : existingGroup) + { + if (find(counter_ids.begin(), counter_ids.end(), counter) == counter_ids.end()) + { + removeObject(vid, false); + break; + } + } + } + + // Check if a matching counter group already exists + for (size_t i = 0; i < m_counterGroupsSorted.size(); i++) + { + // Try match + std::set* counterSet = getSortedCGRef(m_counterGroupsSorted[i]); + std::vector values(counterSet->size(), 0); + std::vector countersToPoll(counterSet->begin(), counterSet->end()); + if (collectData(rid, countersToPoll, stats_mode, false, values)) + { + // Success - Check support for counters not in counter group (extra counters) + std::vector extraCounters; + std::set newCounters; + std::set intersectedCounters; + bool newGroup = false; + + // Make it such that the counter group is a subset of counter_ids + std::set_intersection(counter_ids.begin(), counter_ids.end(), countersToPoll.begin(), countersToPoll.end(), + std::inserter(intersectedCounters, intersectedCounters.begin())); + newGroup = intersectedCounters.size() < countersToPoll.size(); + + // Vectors need to be sorted for set_difference for defined behavior - sets are sorted in C++ + std::set_difference(counter_ids.begin(), counter_ids.end(), intersectedCounters.begin(), intersectedCounters.end(), + std::back_inserter(extraCounters)); + + for (const StatType &counter : extraCounters) + { + std::vector singleCounter {counter}; + std::vector singleValue(1); + if (collectData(rid, singleCounter, stats_mode, false, singleValue)) + { + newCounters.insert(counter); + newGroup = true; + } + else + { + SWSS_LOG_DEBUG("Counter %s not supported with rid %s, vid %s", + serializeStat(counter).c_str(), sai_serialize_object_id(rid).c_str(), + sai_serialize_object_id(vid).c_str()); + } + } + if (newGroup) + { + // New counters discovered, create new counter group + newCounters.insert(intersectedCounters.begin(), intersectedCounters.end()); + + // If vid already has assigned counter group, merge the two groups if dont_clear flag is set + if (m_objectSupportedCountersGroupMap.count(vid) && dont_clear_support_counter) + { + std::set oldGroup = m_supportedCounterGroups[m_objectSupportedCountersGroupMap[vid]]; + newCounters.insert(oldGroup.begin(), oldGroup.end()); + } + + m_objectSupportedCountersGroupMap[vid] = m_supportedCounterGroups.size(); + m_supportedCounterGroups.push_back(newCounters); + m_counterGroupsSorted.push_back(makeCounterGroupRef(m_supportedCounterGroups.size()-1, newCounters.size())); + // Must adhere to strict weak ordering + // Use lambda instead of static function to avoid SWSS_LOG_ENTER CI build requirement for better performance + std::sort(m_counterGroupsSorted.begin(), m_counterGroupsSorted.end(), + [](CounterGroupRef const& lhs, CounterGroupRef const& rhs) + { + if (lhs.idx == rhs.idx) + { + return false; // The sets are equivalent + } + else if (lhs.size == rhs.size) + { + return lhs.idx < rhs.idx; // order by earliest creation when size is same + } + else + { + return lhs.size > rhs.size; + } + }); + } + else + { + // Use existing counter group + // If vid already has assigned counter group, merge the two groups if dont_clear flag is set + if (m_objectSupportedCountersGroupMap.count(vid) && dont_clear_support_counter && + m_supportedCounterGroups[m_objectSupportedCountersGroupMap[vid]] != m_supportedCounterGroups[m_counterGroupsSorted[i].idx]) + { + std::set prevGroup = m_supportedCounterGroups[m_objectSupportedCountersGroupMap[vid]]; + newCounters.insert(prevGroup.begin(), prevGroup.end()); + std::set currGroup = m_supportedCounterGroups[m_counterGroupsSorted[i].idx]; + newCounters.insert(currGroup.begin(), currGroup.end()); + m_objectSupportedCountersGroupMap[vid] = m_supportedCounterGroups.size(); + m_supportedCounterGroups.push_back(newCounters); + m_counterGroupsSorted.push_back(makeCounterGroupRef(m_supportedCounterGroups.size()-1, newCounters.size())); + // Must adhere to strict weak ordering + // Use lambda instead of static function to avoid SWSS_LOG_ENTER CI build requirement for better performance + std::sort(m_counterGroupsSorted.begin(), m_counterGroupsSorted.end(), + [](CounterGroupRef const& lhs, CounterGroupRef const& rhs) + { + if (lhs.idx == rhs.idx) + { + return false; // The sets are equivalent + } + else if (lhs.size == rhs.size) + { + return lhs.idx < rhs.idx; // order by earliest creation when size is same + } + else + { + return lhs.size > rhs.size; + } + }); + } + else + { + m_objectSupportedCountersGroupMap[vid] = m_counterGroupsSorted[i].idx; + } + } + return; + } + } + + // No counter groups matched, check counter support individually + std::set supportedIds; + std::vector values(1); + for (const auto &counter : counter_ids) + { + std::vector tmp_counter_ids {counter}; + if (collectData(rid, tmp_counter_ids, stats_mode, false, values)) + { + supportedIds.insert(counter); + } + } + + if (supportedIds.empty()) + { + return; + } + + // Make new counter group and assign the index if not assigned + m_objectSupportedCountersGroupMap[vid] = m_supportedCounterGroups.size(); + m_supportedCounterGroups.push_back(supportedIds); + m_counterGroupsSorted.push_back(makeCounterGroupRef(m_supportedCounterGroups.size()-1, supportedIds.size())); + // Must adhere to strict weak ordering + // Use lambda instead of static function to avoid SWSS_LOG_ENTER CI build requirement for better performance + std::sort(m_counterGroupsSorted.begin(), m_counterGroupsSorted.end(), + [](CounterGroupRef const& lhs, CounterGroupRef const& rhs) + { + if (lhs.idx == rhs.idx) + { + return false; // The sets are equivalent + } + else if (lhs.size == rhs.size) + { + return lhs.idx < rhs.idx; // order by earliest creation when size is same + } + else + { + return lhs.size > rhs.size; + } + }); + } + void updateSupportedCounters( _In_ sai_object_id_t rid, _In_ const std::vector& counter_ids, @@ -1649,6 +2303,11 @@ class CounterContext : public BaseCounterContext sai_stats_mode_t& m_groupStatsMode; std::set m_supportedCounters; std::set m_supportedBulkCounters; + + std::map m_objectSupportedCountersGroupMap; + std::vector> m_supportedCounterGroups; + std::vector m_counterGroupsSorted; + std::map> m_objectIdsMap; std::map, std::shared_ptr> m_bulkContexts; }; @@ -1677,6 +2336,16 @@ class AttrContext : public CounterContext SWSS_LOG_ENTER(); } + // Wrapper to addObject + void addObjectWithCounterGroups( + _In_ sai_object_id_t vid, + _In_ sai_object_id_t rid, + _In_ const std::vector &idStrings, + _In_ const std::string &per_object_stats_mode) override + { + addObject(vid, rid, idStrings, per_object_stats_mode); + } + void addObject( _In_ sai_object_id_t vid, _In_ sai_object_id_t rid, @@ -1705,6 +2374,16 @@ class AttrContext : public CounterContext Base::m_objectIdsMap.emplace(vid, attr_ids); } + // Wrapper to bulkAddObject + void bulkAddObjectWithCounterGroups( + _In_ const std::vector& vids, + _In_ const std::vector& rids, + _In_ const std::vector& idStrings, + _In_ const std::string &per_object_stats_mode) override + { + bulkAddObject(vids, rids, idStrings, per_object_stats_mode); + } + void bulkAddObject( _In_ const std::vector& vids, _In_ const std::vector& rids, @@ -2655,6 +3334,16 @@ class DashMeterCounterContext : public BaseCounterContext SWSS_LOG_ENTER(); } + // Wrapper to addObject + void addObjectWithCounterGroups( + _In_ sai_object_id_t vid, + _In_ sai_object_id_t rid, + _In_ const std::vector &idStrings, + _In_ const std::string &per_object_stats_mode) override + { + addObject(vid, rid, idStrings, per_object_stats_mode); + } + void addObject( _In_ sai_object_id_t vid, _In_ sai_object_id_t rid, @@ -2774,6 +3463,16 @@ class DashMeterCounterContext : public BaseCounterContext } } + // Wrapper to bulkAddObject + void bulkAddObjectWithCounterGroups( + _In_ const std::vector& vids, + _In_ const std::vector& rids, + _In_ const std::vector& idStrings, + _In_ const std::string &per_object_stats_mode) override + { + bulkAddObject(vids, rids, idStrings, per_object_stats_mode); + } + void bulkAddObject( _In_ const std::vector& vids, _In_ const std::vector& rids, @@ -3801,11 +4500,32 @@ void FlexCounter::addCounter( const auto &counterGroupRef = m_objectTypeField2CounterType.find({objectType, field}); if (counterGroupRef != m_objectTypeField2CounterType.end()) { - getCounterContext(counterGroupRef->second)->addObject( - vid, - rid, - idStrings, - ""); + try { + getCounterContext(counterGroupRef->second)->addObjectWithCounterGroups( + vid, + rid, + idStrings, + ""); + + } + catch (const std::exception& e) + { + SWSS_LOG_WARN("Error initializing SAI objects with counter groups: %s, falling back", e.what()); + getCounterContext(counterGroupRef->second)->addObject( + vid, + rid, + idStrings, + ""); + } + catch (...) { + SWSS_LOG_WARN("Unknown error initializing SAI objects with counter groups, falling back"); + + getCounterContext(counterGroupRef->second)->addObject( + vid, + rid, + idStrings, + ""); + } } else if (objectType == SAI_OBJECT_TYPE_BUFFER_POOL && field == BUFFER_POOL_COUNTER_ID_LIST) { @@ -3862,11 +4582,33 @@ void FlexCounter::bulkAddCounter( const auto &counterGroupRef = m_objectTypeField2CounterType.find({objectType, field}); if (counterGroupRef != m_objectTypeField2CounterType.end()) { - getCounterContext(counterGroupRef->second)->bulkAddObject( - vids, - rids, - idStrings, - ""); + try + { + getCounterContext(counterGroupRef->second)->bulkAddObjectWithCounterGroups( + vids, + rids, + idStrings, + ""); + } + catch (const std::exception& e) + { + SWSS_LOG_WARN("Error initializing SAI objects with counter groups: %s, falling back", e.what()); + getCounterContext(counterGroupRef->second)->bulkAddObject( + vids, + rids, + idStrings, + ""); + } + catch (...) + { + SWSS_LOG_WARN("Unknown error initializing SAI objects with counter groups, falling back"); + getCounterContext(counterGroupRef->second)->bulkAddObject( + vids, + rids, + idStrings, + ""); + } + } else if (objectType == SAI_OBJECT_TYPE_BUFFER_POOL && field == BUFFER_POOL_COUNTER_ID_LIST) { @@ -3888,11 +4630,34 @@ void FlexCounter::bulkAddCounter( if (objectType == SAI_OBJECT_TYPE_BUFFER_POOL && counterIds.size()) { - getCounterContext(COUNTER_TYPE_BUFFER_POOL)->bulkAddObject( - vids, - rids, - counterIds, - statsMode); + try + { + getCounterContext(COUNTER_TYPE_BUFFER_POOL)->bulkAddObjectWithCounterGroups( + vids, + rids, + counterIds, + statsMode); + + } + catch (const std::exception& e) + { + SWSS_LOG_WARN("Error initializing SAI objects with counter groups: %s, falling back", e.what()); + getCounterContext(COUNTER_TYPE_BUFFER_POOL)->bulkAddObject( + vids, + rids, + counterIds, + statsMode); + } + catch (...) + { + SWSS_LOG_WARN("Unknown error initializing SAI objects with counter groups, falling back"); + getCounterContext(COUNTER_TYPE_BUFFER_POOL)->bulkAddObject( + vids, + rids, + counterIds, + statsMode); + + } } // notify thread to start polling diff --git a/syncd/FlexCounter.h b/syncd/FlexCounter.h index 1d28cb23f9..cedea07afa 100644 --- a/syncd/FlexCounter.h +++ b/syncd/FlexCounter.h @@ -60,12 +60,24 @@ namespace syncd void removePlugins() {m_plugins.clear();} + virtual void addObjectWithCounterGroups( + _In_ sai_object_id_t vid, + _In_ sai_object_id_t rid, + _In_ const std::vector &idStrings, + _In_ const std::string &per_object_stats_mode) = 0; + virtual void addObject( _In_ sai_object_id_t vid, _In_ sai_object_id_t rid, _In_ const std::vector &idStrings, _In_ const std::string &per_object_stats_mode) = 0; + virtual void bulkAddObjectWithCounterGroups( + _In_ const std::vector& vids, + _In_ const std::vector& rids, + _In_ const std::vector& idStrings, + _In_ const std::string &per_object_stats_mode) = 0; + virtual void bulkAddObject( _In_ const std::vector& vids, _In_ const std::vector& rids, @@ -89,6 +101,7 @@ namespace syncd std::string m_instanceId; std::set m_plugins; std::string m_bulkChunkSizePerPrefix; + std::map, uint32_t> m_failedPolls; public: bool always_check_supported_counters = false; diff --git a/unittest/syncd/TestFlexCounter.cpp b/unittest/syncd/TestFlexCounter.cpp index 745f64edc3..59c6281d78 100644 --- a/unittest/syncd/TestFlexCounter.cpp +++ b/unittest/syncd/TestFlexCounter.cpp @@ -1938,12 +1938,12 @@ TEST(FlexCounter, counterIdChange) // support bulk to not support bulk values.clear(); - values.emplace_back(PORT_COUNTER_ID_LIST, "SAI_PORT_STAT_IF_IN_NON_UCAST_PKTS,SAI_PORT_STAT_IF_IN_UCAST_PKTS"); + values.emplace_back(PORT_COUNTER_ID_LIST, "SAI_PORT_STAT_IF_IN_UCAST_PKTS,SAI_PORT_STAT_IF_IN_NON_UCAST_PKTS"); fc.addCounter(oid, oid, values); waitForCounterValues(countersTable, expectedKey, - {"SAI_PORT_STAT_IF_IN_NON_UCAST_PKTS", "SAI_PORT_STAT_IF_IN_UCAST_PKTS"}, + {"SAI_PORT_STAT_IF_IN_UCAST_PKTS", "SAI_PORT_STAT_IF_IN_NON_UCAST_PKTS"}, {"10", "20"}); // not support bulk but counter id changes @@ -2346,6 +2346,388 @@ TEST(FlexCounter, noEniDashMeterCounter) false); } +TEST(FlexCounter, dynamicCounterGroups) +{ + // This test tests counter group functionality. It ensures each interface only polls the counters they support. + + // All 6 counters are requested for every port, but getStats fails for + // unsupported ones, so each port's counter group only contains its subset. + // Port 0: IN_OCTETS, OUT_OCTETS, IN_ERRORS (3 of 6) + // Port 1: IN_UCAST_PKTS, OUT_UCAST_PKTS, OUT_ERRORS (3 of 6) + // Port 2: IN_OCTETS, IN_UCAST_PKTS, IN_ERRORS, OUT_ERRORS (4 of 6, overlaps both) + // Port 3: IN_OCTETS, OUT_OCTETS, IN_UCAST_PKTS, IN_ERRORS, OUT_ERRORS (5 of 6, super-set of Port 2) + // + // Port 3 matches Port 2's existing group, but supports an extra counter + // (OUT_OCTETS), so a new larger group must be created rather than reusing + // Port 2's group. + // + // Unsupported counters must not appear in Redis for any port. + + std::vector allCounterNames = { + "SAI_PORT_STAT_IF_IN_OCTETS", + "SAI_PORT_STAT_IF_OUT_OCTETS", + "SAI_PORT_STAT_IF_IN_UCAST_PKTS", + "SAI_PORT_STAT_IF_OUT_UCAST_PKTS", + "SAI_PORT_STAT_IF_IN_ERRORS", + "SAI_PORT_STAT_IF_OUT_ERRORS" + }; + + test_syncd::mockVidManagerObjectTypeQuery(SAI_OBJECT_TYPE_PORT); + auto oids = generateOids(4, SAI_OBJECT_TYPE_PORT); + ASSERT_EQ(oids.size(), 4u); + + // Per-RID supported counter sets (keyed by object_id since RID == VID in tests) + std::map> supportedMap; + supportedMap[oids[0]] = {SAI_PORT_STAT_IF_IN_OCTETS, SAI_PORT_STAT_IF_OUT_OCTETS, SAI_PORT_STAT_IF_IN_ERRORS}; + supportedMap[oids[1]] = {SAI_PORT_STAT_IF_IN_UCAST_PKTS, SAI_PORT_STAT_IF_OUT_UCAST_PKTS, SAI_PORT_STAT_IF_OUT_ERRORS}; + supportedMap[oids[2]] = {SAI_PORT_STAT_IF_IN_OCTETS, SAI_PORT_STAT_IF_IN_UCAST_PKTS, SAI_PORT_STAT_IF_IN_ERRORS, SAI_PORT_STAT_IF_OUT_ERRORS}; + supportedMap[oids[3]] = {SAI_PORT_STAT_IF_IN_OCTETS, SAI_PORT_STAT_IF_OUT_OCTETS, SAI_PORT_STAT_IF_IN_UCAST_PKTS, SAI_PORT_STAT_IF_IN_ERRORS, SAI_PORT_STAT_IF_OUT_ERRORS}; + + // Deterministic counter values: value = (port_index + 1) * 1000 + stat_enum + auto computeValue = [&](sai_object_id_t rid, sai_port_stat_t stat) -> uint64_t { + int portIdx = 0; + for (size_t i = 0; i < oids.size(); i++) + { + if (oids[i] == rid) { portIdx = static_cast(i); break; } + } + return static_cast((portIdx + 1) * 1000 + static_cast(stat)); + }; + + auto isAllSupported = [&](sai_object_id_t rid, uint32_t count, const sai_stat_id_t *ids) -> bool { + auto it = supportedMap.find(rid); + if (it == supportedMap.end()) return false; + for (uint32_t i = 0; i < count; i++) + { + if (it->second.count(static_cast(ids[i])) == 0) + return false; + } + return true; + }; + + // Succeed only when all requested counters are in the RID's supported set. + sai->mock_getStats = [&](sai_object_type_t, sai_object_id_t rid, + uint32_t count, const sai_stat_id_t *ids, + uint64_t *counters) -> sai_status_t + { + if (!isAllSupported(rid, count, ids)) + return SAI_STATUS_FAILURE; + for (uint32_t i = 0; i < count; i++) + counters[i] = computeValue(rid, static_cast(ids[i])); + return SAI_STATUS_SUCCESS; + }; + + // Same per-RID logic; PORT uses getStats, but mock this as a safety net. + sai->mock_getStatsExt = [&](sai_object_type_t, sai_object_id_t rid, + uint32_t count, const sai_stat_id_t *ids, + sai_stats_mode_t, uint64_t *counters) -> sai_status_t + { + if (!isAllSupported(rid, count, ids)) + return SAI_STATUS_FAILURE; + for (uint32_t i = 0; i < count; i++) + counters[i] = computeValue(rid, static_cast(ids[i])); + return SAI_STATUS_SUCCESS; + }; + + // Skip HW capability query; counter support is determined by getStats probing. + sai->mock_queryStatsCapability = [](sai_object_id_t, sai_object_type_t, + sai_stat_capability_list_t *) + { + return SAI_STATUS_FAILURE; + }; + + // Force non-bulk path so per-port counter groups are polled individually. + // (per-port counter group discovery cannot be validated by the bulk-path, + // it has different counter discovery logic.) + sai->mock_bulkGetStats = [](sai_object_id_t, sai_object_type_t, uint32_t, + const sai_object_key_t *, uint32_t, + const sai_stat_id_t *, sai_stats_mode_t, + sai_status_t *, uint64_t *) + { + return SAI_STATUS_FAILURE; + }; + + FlexCounter fc("test", sai, "COUNTERS_DB"); + + std::vector pluginValues; + pluginValues.emplace_back(POLL_INTERVAL_FIELD, "1000"); + pluginValues.emplace_back(FLEX_COUNTER_STATUS_FIELD, "enable"); + pluginValues.emplace_back(STATS_MODE_FIELD, STATS_MODE_READ); + fc.addCounterPlugin(pluginValues); + + std::vector counterValues; + counterValues.emplace_back(PORT_COUNTER_ID_LIST, join(allCounterNames)); + + fc.bulkAddCounter(SAI_OBJECT_TYPE_PORT, oids, oids, counterValues); + + EXPECT_FALSE(fc.isEmpty()); + + swss::DBConnector db("COUNTERS_DB", 0); + swss::RedisPipeline pipeline(&db); + swss::Table countersTable(&pipeline, COUNTERS_TABLE, false); + + waitForCounterKeys(countersTable, 4); + + // Verify each port has exactly its supported counters with correct values, + // and unsupported counters are absent. + std::set allStats = { + SAI_PORT_STAT_IF_IN_OCTETS, + SAI_PORT_STAT_IF_OUT_OCTETS, + SAI_PORT_STAT_IF_IN_UCAST_PKTS, + SAI_PORT_STAT_IF_OUT_UCAST_PKTS, + SAI_PORT_STAT_IF_IN_ERRORS, + SAI_PORT_STAT_IF_OUT_ERRORS + }; + + for (size_t p = 0; p < oids.size(); p++) + { + std::string key = toOid(oids[p]); + const auto &supported = supportedMap[oids[p]]; + + // Wait for one of the supported counters to be populated + sai_port_stat_t firstSupported = *supported.begin(); + std::string firstField = sai_serialize_port_stat(firstSupported); + std::string expectedFirstVal = std::to_string(computeValue(oids[p], firstSupported)); + waitForCounterValues(countersTable, key, {firstField}, {expectedFirstVal}); + + // Verify all supported counters have correct values + for (auto stat : supported) + { + std::string field = sai_serialize_port_stat(stat); + std::string value; + ASSERT_TRUE(countersTable.hget(key, field, value)) + << "Port " << p << " missing supported counter " << field; + std::string expected = std::to_string(computeValue(oids[p], stat)); + EXPECT_EQ(value, expected) + << "Port " << p << " counter " << field << " value mismatch"; + } + + // Verify unsupported counters are absent + for (auto stat : allStats) + { + if (supported.count(stat)) + continue; + std::string field = sai_serialize_port_stat(stat); + std::string value; + EXPECT_FALSE(countersTable.hget(key, field, value)) + << "Port " << p << " should NOT have unsupported counter " << field + << " but found value '" << value << "'"; + } + } + + // Cleanup + for (auto oid : oids) + { + fc.removeCounter(oid); + countersTable.del(toOid(oid)); + } + EXPECT_TRUE(fc.isEmpty()); + + std::vector keys; + countersTable.getKeys(keys); + removeTimeStamp(keys, countersTable); + ASSERT_TRUE(keys.empty()); +} + +TEST(FlexCounter, dynamicCounterGroupsBulkPath) +{ + // Bulk-path variant of dynamicCounterGroups. Uses + // bulkAddObjectWithCounterGroups, which selects the largest counter group + // for bulkGetStats and falls back to single-object polling for ports whose + // supported set is smaller. + + // All 6 counters are requested for every port, but getStats fails for + // unsupported ones, so each port's counter group only contains its subset. + // Port 0: IN_OCTETS, OUT_OCTETS, IN_ERRORS (3 of 6) + // Port 1: IN_UCAST_PKTS, OUT_UCAST_PKTS, OUT_ERRORS (3 of 6) + // Port 2: IN_OCTETS, IN_UCAST_PKTS, IN_ERRORS, OUT_ERRORS (4 of 6, overlaps both) + // Port 3: IN_OCTETS, OUT_OCTETS, IN_UCAST_PKTS, IN_ERRORS, OUT_ERRORS (5 of 6, super-set of Port 2) + // + // Port 3 matches Port 2's existing group, but supports an extra counter + // (OUT_OCTETS), so a new larger group must be created rather than reusing + // Port 2's group. + // + // Unsupported counters must not appear in Redis for any port. + + std::vector allCounterNames = { + "SAI_PORT_STAT_IF_IN_OCTETS", + "SAI_PORT_STAT_IF_OUT_OCTETS", + "SAI_PORT_STAT_IF_IN_UCAST_PKTS", + "SAI_PORT_STAT_IF_OUT_UCAST_PKTS", + "SAI_PORT_STAT_IF_IN_ERRORS", + "SAI_PORT_STAT_IF_OUT_ERRORS" + }; + + test_syncd::mockVidManagerObjectTypeQuery(SAI_OBJECT_TYPE_PORT); + auto oids = generateOids(4, SAI_OBJECT_TYPE_PORT); + ASSERT_EQ(oids.size(), 4u); + + // Per-RID supported counter sets (keyed by object_id since RID == VID in tests) + std::map> supportedMap; + supportedMap[oids[0]] = {SAI_PORT_STAT_IF_IN_OCTETS, SAI_PORT_STAT_IF_OUT_OCTETS, SAI_PORT_STAT_IF_IN_ERRORS}; + supportedMap[oids[1]] = {SAI_PORT_STAT_IF_IN_UCAST_PKTS, SAI_PORT_STAT_IF_OUT_UCAST_PKTS, SAI_PORT_STAT_IF_OUT_ERRORS}; + supportedMap[oids[2]] = {SAI_PORT_STAT_IF_IN_OCTETS, SAI_PORT_STAT_IF_IN_UCAST_PKTS, SAI_PORT_STAT_IF_IN_ERRORS, SAI_PORT_STAT_IF_OUT_ERRORS}; + supportedMap[oids[3]] = {SAI_PORT_STAT_IF_IN_OCTETS, SAI_PORT_STAT_IF_OUT_OCTETS, SAI_PORT_STAT_IF_IN_UCAST_PKTS, SAI_PORT_STAT_IF_IN_ERRORS, SAI_PORT_STAT_IF_OUT_ERRORS}; + + // Deterministic counter values: value = (port_index + 1) * 1000 + stat_enum + auto computeValue = [&](sai_object_id_t rid, sai_port_stat_t stat) -> uint64_t { + int portIdx = 0; + for (size_t i = 0; i < oids.size(); i++) + { + if (oids[i] == rid) { portIdx = static_cast(i); break; } + } + return static_cast((portIdx + 1) * 1000 + static_cast(stat)); + }; + + auto isAllSupported = [&](sai_object_id_t rid, uint32_t count, const sai_stat_id_t *ids) -> bool { + auto it = supportedMap.find(rid); + if (it == supportedMap.end()) return false; + for (uint32_t i = 0; i < count; i++) + { + if (it->second.count(static_cast(ids[i])) == 0) + return false; + } + return true; + }; + + // Succeed only when all requested counters are in the RID's supported set. + sai->mock_getStats = [&](sai_object_type_t, sai_object_id_t rid, + uint32_t count, const sai_stat_id_t *ids, + uint64_t *counters) -> sai_status_t + { + if (!isAllSupported(rid, count, ids)) + return SAI_STATUS_FAILURE; + for (uint32_t i = 0; i < count; i++) + counters[i] = computeValue(rid, static_cast(ids[i])); + return SAI_STATUS_SUCCESS; + }; + + // Same per-RID logic; PORT uses getStats, but mock this as a safety net. + sai->mock_getStatsExt = [&](sai_object_type_t, sai_object_id_t rid, + uint32_t count, const sai_stat_id_t *ids, + sai_stats_mode_t, uint64_t *counters) -> sai_status_t + { + if (!isAllSupported(rid, count, ids)) + return SAI_STATUS_FAILURE; + for (uint32_t i = 0; i < count; i++) + counters[i] = computeValue(rid, static_cast(ids[i])); + return SAI_STATUS_SUCCESS; + }; + + // Skip HW capability query; counter support is determined by getStats probing. + sai->mock_queryStatsCapability = [](sai_object_id_t, sai_object_type_t, + sai_stat_capability_list_t *) + { + return SAI_STATUS_FAILURE; + }; + + // Bulk path: succeed for any counter set so that the largest counter group + // is polled via bulkGetStats. Return the same deterministic values as the + // single-object path so verification is identical. + sai->mock_bulkGetStats = [&](sai_object_id_t, + sai_object_type_t, + uint32_t object_count, + const sai_object_key_t *object_keys, + uint32_t number_of_counters, + const sai_stat_id_t *counter_ids, + sai_stats_mode_t, + sai_status_t *object_statuses, + uint64_t *counters) -> sai_status_t + { + for (uint32_t i = 0; i < object_count; i++) + { + sai_object_id_t rid = object_keys[i].key.object_id; + object_statuses[i] = SAI_STATUS_SUCCESS; + for (uint32_t j = 0; j < number_of_counters; j++) + { + counters[i * number_of_counters + j] = + computeValue(rid, static_cast(counter_ids[j])); + } + } + return SAI_STATUS_SUCCESS; + }; + + FlexCounter fc("test", sai, "COUNTERS_DB"); + + std::vector pluginValues; + pluginValues.emplace_back(POLL_INTERVAL_FIELD, "1000"); + pluginValues.emplace_back(FLEX_COUNTER_STATUS_FIELD, "enable"); + pluginValues.emplace_back(STATS_MODE_FIELD, STATS_MODE_READ); + fc.addCounterPlugin(pluginValues); + + std::vector counterValues; + counterValues.emplace_back(PORT_COUNTER_ID_LIST, join(allCounterNames)); + + fc.bulkAddCounter(SAI_OBJECT_TYPE_PORT, oids, oids, counterValues); + + EXPECT_FALSE(fc.isEmpty()); + + swss::DBConnector db("COUNTERS_DB", 0); + swss::RedisPipeline pipeline(&db); + swss::Table countersTable(&pipeline, COUNTERS_TABLE, false); + + waitForCounterKeys(countersTable, 4); + + // Verify each port has exactly its supported counters with correct values, + // and unsupported counters are absent. + std::set allStats = { + SAI_PORT_STAT_IF_IN_OCTETS, + SAI_PORT_STAT_IF_OUT_OCTETS, + SAI_PORT_STAT_IF_IN_UCAST_PKTS, + SAI_PORT_STAT_IF_OUT_UCAST_PKTS, + SAI_PORT_STAT_IF_IN_ERRORS, + SAI_PORT_STAT_IF_OUT_ERRORS + }; + + for (size_t p = 0; p < oids.size(); p++) + { + std::string key = toOid(oids[p]); + const auto &supported = supportedMap[oids[p]]; + + // Wait for one of the supported counters to be populated + sai_port_stat_t firstSupported = *supported.begin(); + std::string firstField = sai_serialize_port_stat(firstSupported); + std::string expectedFirstVal = std::to_string(computeValue(oids[p], firstSupported)); + waitForCounterValues(countersTable, key, {firstField}, {expectedFirstVal}); + + // Verify all supported counters have correct values + for (auto stat : supported) + { + std::string field = sai_serialize_port_stat(stat); + std::string value; + ASSERT_TRUE(countersTable.hget(key, field, value)) + << "Port " << p << " missing supported counter " << field; + std::string expected = std::to_string(computeValue(oids[p], stat)); + EXPECT_EQ(value, expected) + << "Port " << p << " counter " << field << " value mismatch"; + } + + // Verify unsupported counters are absent + for (auto stat : allStats) + { + if (supported.count(stat)) + continue; + std::string field = sai_serialize_port_stat(stat); + std::string value; + EXPECT_FALSE(countersTable.hget(key, field, value)) + << "Port " << p << " should NOT have unsupported counter " << field + << " but found value '" << value << "'"; + } + } + + // Cleanup + for (auto oid : oids) + { + fc.removeCounter(oid); + countersTable.del(toOid(oid)); + } + EXPECT_TRUE(fc.isEmpty()); + + std::vector keys; + countersTable.getKeys(keys); + removeTimeStamp(keys, countersTable); + ASSERT_TRUE(keys.empty()); +} + class FlexCounterTcpFallback : public ::testing::Test { protected: From 6a45916d3f385c99ff6554c4178fd6a23255fcf6 Mon Sep 17 00:00:00 2001 From: Justin Wong Date: Fri, 3 Jul 2026 23:17:53 +0000 Subject: [PATCH 2/4] Address review comments List of changes made: - Added tracking of free indicies for reuse - Added m_failedPolls cleanup when object is removed - Improved m_failedPolls lookup logic to only use a single lookup - Added addGroup helper - Used const& refs for sets instead of copies where applicable - Removed getSortedCGRef() for to avoid performance impact from the SWSS_LOG_ENTER() requirement in functions - Better clarity by changing find() -> std::find() - Simplied erase-emplace logic by using built in [] operator logic - Normalized key check method when dealing with maps - Indentation fixes - Minor optimization when iterating through counter groups in setupBaseCounterGroup() - Added warning comments on parts of code where more care is required (consuming of const refs) - Added unit test for m_failedPolls logic Signed-off-by: Justin Wong --- syncd/FlexCounter.cpp | 229 ++++++++++------------------- unittest/syncd/TestFlexCounter.cpp | 116 +++++++++++++++ 2 files changed, 192 insertions(+), 153 deletions(-) diff --git a/syncd/FlexCounter.cpp b/syncd/FlexCounter.cpp index a2a88dbe59..0b54eab798 100644 --- a/syncd/FlexCounter.cpp +++ b/syncd/FlexCounter.cpp @@ -548,11 +548,6 @@ class CounterContext : public BaseCounterContext SWSS_LOG_ENTER(); return {idx, size}; } - std::set* getSortedCGRef(CounterGroupRef const& cgr) - { - SWSS_LOG_ENTER(); - return &m_supportedCounterGroups[cgr.idx]; - } public: typedef CounterIds CounterIdsType; typedef BulkStatsContext BulkContextType; @@ -1129,15 +1124,7 @@ class CounterContext : public BaseCounterContext sai_serialize_object_id(rid).c_str(), sai_serialize_object_id(vid).c_str()); throw std::runtime_error("Test counter poll failed on populating m_objectIdsMap"); } - auto it_vid = m_objectIdsMap.find(vid); - if (it_vid != m_objectIdsMap.end()) - { - // Remove and re-add if vid already exists - m_objectIdsMap.erase(it_vid); - } - - auto counter_data = std::make_shared>(rid, intf_counter_ids); - m_objectIdsMap.emplace(vid, counter_data); + m_objectIdsMap[vid] = std::make_shared>(rid, intf_counter_ids); SWSS_LOG_INFO("Fallback to single call for object 0x%" PRIx64, vid); } return; @@ -1171,7 +1158,8 @@ class CounterContext : public BaseCounterContext }; // Use counter group with the most counters - std::vector supportedIds(getSortedCGRef(m_counterGroupsSorted[0])->begin(), getSortedCGRef(m_counterGroupsSorted[0])->end()); + const std::set& largest_set = m_supportedCounterGroups[m_counterGroupsSorted[0].idx]; + std::vector supportedIds(largest_set.begin(), largest_set.end()); if (m_counterChunkSizeMapFromPrefix.empty()) { @@ -1221,18 +1209,10 @@ class CounterContext : public BaseCounterContext { SWSS_LOG_INFO("Fallback to single call for object 0x%" PRIx64, vid); - auto it_vid = m_objectIdsMap.find(vid); - if (it_vid != m_objectIdsMap.end()) - { - // Remove and re-add if vid already exists - m_objectIdsMap.erase(it_vid); - } - size_t groupIndex = m_objectSupportedCountersGroupMap[vid]; std::vector objCounterIds(m_supportedCounterGroups[groupIndex].begin(), m_supportedCounterGroups[groupIndex].end()); - auto counter_data = std::make_shared>(rid, objCounterIds); - m_objectIdsMap.emplace(vid, counter_data); + m_objectIdsMap[vid] = std::make_shared>(rid, objCounterIds); } else { @@ -1334,17 +1314,7 @@ class CounterContext : public BaseCounterContext auto vid = vids[i]; std::vector stats(counter_ids.size()); if (collectData(rid, counter_ids, effective_stats_mode, false, stats)) { - - auto it_vid = m_objectIdsMap.find(vid); - if (it_vid != m_objectIdsMap.end()) - { - // Remove and re-add if vid already exists - m_objectIdsMap.erase(it_vid); - } - - auto counter_data = std::make_shared>(rid, counter_ids); - m_objectIdsMap.emplace(vid, counter_data); - + m_objectIdsMap[vid] = std::make_shared>(rid, counter_ids); SWSS_LOG_INFO("Fallback to single call for object 0x%" PRIx64, vid); } else { SWSS_LOG_WARN("%s RID %s can't provide the statistic", m_name.c_str(), sai_serialize_object_id(rid).c_str()); @@ -1428,16 +1398,7 @@ class CounterContext : public BaseCounterContext else if (!double_confirm_supported_counters || collectData(rid, it.first, effective_stats_mode, false, stats)) { SWSS_LOG_INFO("Fallback to single call for object 0x%" PRIx64, vid); - - auto it_vid = m_objectIdsMap.find(vid); - if (it_vid != m_objectIdsMap.end()) - { - // Remove and re-add if vid already exists - m_objectIdsMap.erase(it_vid); - } - - auto counter_data = std::make_shared>(rid, supportedIds); - m_objectIdsMap.emplace(vid, counter_data); + m_objectIdsMap[vid] = std::make_shared>(rid, supportedIds); } else { @@ -1462,6 +1423,45 @@ class CounterContext : public BaseCounterContext removeObject(vid, true); } + size_t addGroup(std::set group) + { + SWSS_LOG_ENTER(); + size_t idx; + if (!m_freeGroupIndices.empty()) + { + idx = m_freeGroupIndices.back(); + m_freeGroupIndices.pop_back(); + m_supportedCounterGroups[idx] = std::move(group); + } + else + { + idx = m_supportedCounterGroups.size(); + m_supportedCounterGroups.push_back(std::move(group)); + } + m_counterGroupsSorted.push_back(makeCounterGroupRef(idx, m_supportedCounterGroups[idx].size())); + + // Must adhere to strict weak ordering + // Use lambda instead of static func to avoid SWSS_LOG_ENTER CI build requirement for better performance + std::sort(m_counterGroupsSorted.begin(), m_counterGroupsSorted.end(), + [](CounterGroupRef const& lhs, CounterGroupRef const& rhs) + { + if (lhs.idx == rhs.idx) + { + return false; // The sets are equivalent + } + else if (lhs.size == rhs.size) + { + return lhs.idx < rhs.idx; // order by earliest creation when size is same + } + else + { + return lhs.size > rhs.size; + } + }); + + return idx; + } + void cleanupCounterGroupMapping( _In_ sai_object_id_t vid) { @@ -1497,6 +1497,7 @@ class CounterContext : public BaseCounterContext // Clear the group data (can't erase from vector without invalidating other indices) m_supportedCounterGroups[removedGroupIdx].clear(); + m_freeGroupIndices.push_back(removedGroupIdx); } } @@ -1510,6 +1511,8 @@ class CounterContext : public BaseCounterContext auto iter = m_objectIdsMap.find(vid); if (iter != m_objectIdsMap.end()) { + auto rid = iter->second->rid; + m_failedPolls.erase({rid, vid}); m_objectIdsMap.erase(iter); } @@ -1546,19 +1549,17 @@ class CounterContext : public BaseCounterContext std::vector stats(statIds.size(), 0); if (!collectData(rid, statIds, effective_stats_mode, true, stats)) { - if (m_failedPolls.find({rid, vid}) == m_failedPolls.end()) + uint32_t n = ++m_failedPolls[{rid, vid}]; + if (n == 1) { - m_failedPolls[{rid, vid}] = 1; SWSS_LOG_DEBUG("counter read failed 1 time on RID 0x%" PRIx64 " on intf 0x%" PRIx64, rid, vid); } - else if (m_failedPolls[{rid, vid}] < 3) + else if (n <= 3) { - m_failedPolls[{rid, vid}] += 1; - SWSS_LOG_DEBUG("counter read failed %d times on RID 0x%" PRIx64 " on intf 0x%" PRIx64, m_failedPolls[{rid, vid}], rid, vid); + SWSS_LOG_DEBUG("counter read failed %u times on RID 0x%" PRIx64 " on intf 0x%" PRIx64, n, rid, vid); } - else if (m_failedPolls[{rid, vid}] == 3) + else if (n == 4) { - m_failedPolls[{rid, vid}] += 1; SWSS_LOG_ERROR("counter read failed more than 3 times on RID 0x%" PRIx64 " on intf 0x%" PRIx64, rid, vid); } continue; @@ -1647,8 +1648,7 @@ class CounterContext : public BaseCounterContext // Create base counter group if not yet instantiated if (m_supportedCounterGroups.empty()) { - m_supportedCounterGroups.push_back(counter_ids_set); - m_counterGroupsSorted.push_back(makeCounterGroupRef(0, counter_ids_set.size())); + addGroup(counter_ids_set); } // Replace base counter group if full counter set differs else @@ -1656,7 +1656,7 @@ class CounterContext : public BaseCounterContext bool groupExists = false; for (size_t i = 0; i < m_supportedCounterGroups.size(); i++) { - if (m_supportedCounterGroups[i] == counter_ids_set) + if (!m_supportedCounterGroups[i].empty() && m_supportedCounterGroups[i] == counter_ids_set) { groupExists = true; break; @@ -1664,26 +1664,7 @@ class CounterContext : public BaseCounterContext } if (!groupExists) { - m_counterGroupsSorted.push_back(makeCounterGroupRef(m_supportedCounterGroups.size(), counter_ids_set.size())); - m_supportedCounterGroups.push_back(counter_ids_set); - // Must adhere to strict weak ordering - // Use lambda instead of static function to avoid SWSS_LOG_ENTER CI build requirement for better performance - std::sort(m_counterGroupsSorted.begin(), m_counterGroupsSorted.end(), - [](CounterGroupRef const& lhs, CounterGroupRef const& rhs) - { - if (lhs.idx == rhs.idx) - { - return false; // The sets are equivalent - } - else if (lhs.size == rhs.size) - { - return lhs.idx < rhs.idx; // order by earliest creation when size is same - } - else - { - return lhs.size > rhs.size; - } - }); + addGroup(counter_ids_set); } } return counter_ids_set; @@ -2015,7 +1996,7 @@ class CounterContext : public BaseCounterContext _In_ sai_stats_mode_t stats_mode) { SWSS_LOG_ENTER(); - if (m_objectSupportedCountersGroupMap.find(vid) != m_objectSupportedCountersGroupMap.end()) + if (m_objectSupportedCountersGroupMap.count(vid)) { if (!always_check_supported_counters) { @@ -2025,10 +2006,10 @@ class CounterContext : public BaseCounterContext // If there is already a counter group for this vid, and that counter group contains a counter that is not // included in counter_ids, then it means we are handling a different set of counters. // Remove the previous group and mappings to this vid - std::set existingGroup = m_supportedCounterGroups[m_objectSupportedCountersGroupMap[vid]]; + const std::set& existingGroup = m_supportedCounterGroups[m_objectSupportedCountersGroupMap[vid]]; for (auto &counter : existingGroup) { - if (find(counter_ids.begin(), counter_ids.end(), counter) == counter_ids.end()) + if (std::find(counter_ids.begin(), counter_ids.end(), counter) == counter_ids.end()) { removeObject(vid, false); break; @@ -2040,9 +2021,9 @@ class CounterContext : public BaseCounterContext for (size_t i = 0; i < m_counterGroupsSorted.size(); i++) { // Try match - std::set* counterSet = getSortedCGRef(m_counterGroupsSorted[i]); - std::vector values(counterSet->size(), 0); - std::vector countersToPoll(counterSet->begin(), counterSet->end()); + const std::set& counterSet = m_supportedCounterGroups[m_counterGroupsSorted[i].idx]; + std::vector values(counterSet.size(), 0); + std::vector countersToPoll(counterSet.begin(), counterSet.end()); if (collectData(rid, countersToPoll, stats_mode, false, values)) { // Success - Check support for counters not in counter group (extra counters) @@ -2084,68 +2065,29 @@ class CounterContext : public BaseCounterContext // If vid already has assigned counter group, merge the two groups if dont_clear flag is set if (m_objectSupportedCountersGroupMap.count(vid) && dont_clear_support_counter) { - std::set oldGroup = m_supportedCounterGroups[m_objectSupportedCountersGroupMap[vid]]; - newCounters.insert(oldGroup.begin(), oldGroup.end()); + const std::set& oldGroup = m_supportedCounterGroups[m_objectSupportedCountersGroupMap[vid]]; + newCounters.insert(oldGroup.begin(), oldGroup.end()); } - - m_objectSupportedCountersGroupMap[vid] = m_supportedCounterGroups.size(); - m_supportedCounterGroups.push_back(newCounters); - m_counterGroupsSorted.push_back(makeCounterGroupRef(m_supportedCounterGroups.size()-1, newCounters.size())); - // Must adhere to strict weak ordering - // Use lambda instead of static function to avoid SWSS_LOG_ENTER CI build requirement for better performance - std::sort(m_counterGroupsSorted.begin(), m_counterGroupsSorted.end(), - [](CounterGroupRef const& lhs, CounterGroupRef const& rhs) - { - if (lhs.idx == rhs.idx) - { - return false; // The sets are equivalent - } - else if (lhs.size == rhs.size) - { - return lhs.idx < rhs.idx; // order by earliest creation when size is same - } - else - { - return lhs.size > rhs.size; - } - }); + // References to oldGroup and newCounters must be consumed before this call + m_objectSupportedCountersGroupMap[vid] = addGroup(newCounters); } else { // Use existing counter group // If vid already has assigned counter group, merge the two groups if dont_clear flag is set if (m_objectSupportedCountersGroupMap.count(vid) && dont_clear_support_counter && - m_supportedCounterGroups[m_objectSupportedCountersGroupMap[vid]] != m_supportedCounterGroups[m_counterGroupsSorted[i].idx]) + m_supportedCounterGroups[m_objectSupportedCountersGroupMap[vid]] != m_supportedCounterGroups[m_counterGroupsSorted[i].idx]) { - std::set prevGroup = m_supportedCounterGroups[m_objectSupportedCountersGroupMap[vid]]; - newCounters.insert(prevGroup.begin(), prevGroup.end()); - std::set currGroup = m_supportedCounterGroups[m_counterGroupsSorted[i].idx]; - newCounters.insert(currGroup.begin(), currGroup.end()); - m_objectSupportedCountersGroupMap[vid] = m_supportedCounterGroups.size(); - m_supportedCounterGroups.push_back(newCounters); - m_counterGroupsSorted.push_back(makeCounterGroupRef(m_supportedCounterGroups.size()-1, newCounters.size())); - // Must adhere to strict weak ordering - // Use lambda instead of static function to avoid SWSS_LOG_ENTER CI build requirement for better performance - std::sort(m_counterGroupsSorted.begin(), m_counterGroupsSorted.end(), - [](CounterGroupRef const& lhs, CounterGroupRef const& rhs) - { - if (lhs.idx == rhs.idx) - { - return false; // The sets are equivalent - } - else if (lhs.size == rhs.size) - { - return lhs.idx < rhs.idx; // order by earliest creation when size is same - } - else - { - return lhs.size > rhs.size; - } - }); + const std::set& prevGroup = m_supportedCounterGroups[m_objectSupportedCountersGroupMap[vid]]; + newCounters.insert(prevGroup.begin(), prevGroup.end()); + const std::set& currGroup = m_supportedCounterGroups[m_counterGroupsSorted[i].idx]; + newCounters.insert(currGroup.begin(), currGroup.end()); + // References to prevGroup, currGroup, and newCounters must be consumed before this call + m_objectSupportedCountersGroupMap[vid] = addGroup(newCounters); } else { - m_objectSupportedCountersGroupMap[vid] = m_counterGroupsSorted[i].idx; + m_objectSupportedCountersGroupMap[vid] = m_counterGroupsSorted[i].idx; } } return; @@ -2170,27 +2112,7 @@ class CounterContext : public BaseCounterContext } // Make new counter group and assign the index if not assigned - m_objectSupportedCountersGroupMap[vid] = m_supportedCounterGroups.size(); - m_supportedCounterGroups.push_back(supportedIds); - m_counterGroupsSorted.push_back(makeCounterGroupRef(m_supportedCounterGroups.size()-1, supportedIds.size())); - // Must adhere to strict weak ordering - // Use lambda instead of static function to avoid SWSS_LOG_ENTER CI build requirement for better performance - std::sort(m_counterGroupsSorted.begin(), m_counterGroupsSorted.end(), - [](CounterGroupRef const& lhs, CounterGroupRef const& rhs) - { - if (lhs.idx == rhs.idx) - { - return false; // The sets are equivalent - } - else if (lhs.size == rhs.size) - { - return lhs.idx < rhs.idx; // order by earliest creation when size is same - } - else - { - return lhs.size > rhs.size; - } - }); + m_objectSupportedCountersGroupMap[vid] = addGroup(supportedIds); } void updateSupportedCounters( @@ -2307,6 +2229,7 @@ class CounterContext : public BaseCounterContext std::map m_objectSupportedCountersGroupMap; std::vector> m_supportedCounterGroups; std::vector m_counterGroupsSorted; + std::vector m_freeGroupIndices; std::map> m_objectIdsMap; std::map, std::shared_ptr> m_bulkContexts; diff --git a/unittest/syncd/TestFlexCounter.cpp b/unittest/syncd/TestFlexCounter.cpp index 59c6281d78..b52e656e21 100644 --- a/unittest/syncd/TestFlexCounter.cpp +++ b/unittest/syncd/TestFlexCounter.cpp @@ -2828,4 +2828,120 @@ TEST_F(FlexCounterTcpFallback, tcpFallbackWhenNoUnixSocket) EXPECT_TRUE(fc.isEmpty()); countersTable.del(key); + +TEST(FlexCounter, failedPollsCountAndCleanUp) +{ + // This test verifies that: + // + // 1. When getStats starts failing after successful polls on an already-added object, + // counter DB values go stale (stop updating) and the poll continues. + // 2. After removing and re-adding the same object, the failure count resets + // (m_failedPolls cleanup on remove) so it polls successfully again. + + std::atomic failGetStats{false}; + std::atomic pollCycleCount{0}; + + sai->mock_queryStatsCapability = [](sai_object_id_t, sai_object_type_t, + sai_stat_capability_list_t *) + { + return SAI_STATUS_FAILURE; + }; + + sai->mock_getStats = [&](sai_object_type_t, sai_object_id_t, + uint32_t number_of_counters, const sai_stat_id_t *, + uint64_t *counters) -> sai_status_t + { + if (failGetStats.load()) + { + pollCycleCount++; + return SAI_STATUS_FAILURE; + } + for (uint32_t i = 0; i < number_of_counters; i++) + { + counters[i] = (i + 1) * 100; + } + return SAI_STATUS_SUCCESS; + }; + + sai->mock_bulkGetStats = [](sai_object_id_t, sai_object_type_t, uint32_t, + const sai_object_key_t *, uint32_t, + const sai_stat_id_t *, sai_stats_mode_t, + sai_status_t *, uint64_t *) + { + return SAI_STATUS_FAILURE; + }; + + test_syncd::mockVidManagerObjectTypeQuery(SAI_OBJECT_TYPE_PORT); + + sai_object_id_t oid{0x1000000000000}; + std::string expectedKey = toOid(oid); + + FlexCounter fc("test", sai, "COUNTERS_DB"); + + std::vector pluginValues; + pluginValues.emplace_back(POLL_INTERVAL_FIELD, "1000"); + pluginValues.emplace_back(FLEX_COUNTER_STATUS_FIELD, "enable"); + pluginValues.emplace_back(STATS_MODE_FIELD, STATS_MODE_READ); + fc.addCounterPlugin(pluginValues); + + swss::DBConnector db("COUNTERS_DB", 0); + swss::RedisPipeline pipeline(&db); + swss::Table countersTable(&pipeline, COUNTERS_TABLE, false); + + // Verify counters will stop updating DB after 3 or more failed polls + // Add object and verify counters in DB + std::vector counterValues; + counterValues.emplace_back(PORT_COUNTER_ID_LIST, "SAI_PORT_STAT_IF_IN_OCTETS,SAI_PORT_STAT_IF_IN_ERRORS"); + fc.addCounter(oid, oid, counterValues); + EXPECT_FALSE(fc.isEmpty()); + + waitForCounterKeys(countersTable, 1); + waitForCounterValues(countersTable, expectedKey, + {"SAI_PORT_STAT_IF_IN_OCTETS", "SAI_PORT_STAT_IF_IN_ERRORS"}, + {"100", "200"}); + + // getStats starts failing and DB values should go stale (not updated, not cleared). + failGetStats = true; + pollCycleCount = 0; + + auto deadline = std::chrono::steady_clock::now() + std::chrono::milliseconds(5000); + while (std::chrono::steady_clock::now() < deadline && pollCycleCount.load() < 3) + { + usleep(100 * 1000); + } + EXPECT_GE(pollCycleCount.load(), 3u); + + // Stale values should remain in the DB, unchanged + std::string value; + countersTable.hget(expectedKey, "SAI_PORT_STAT_IF_IN_OCTETS", value); + EXPECT_EQ(value, "100"); + countersTable.hget(expectedKey, "SAI_PORT_STAT_IF_IN_ERRORS", value); + EXPECT_EQ(value, "200"); + + // Verify m_failedPolls is cleaned up on removeObject so the + // re-added object can poll fresh without stale failing state. + // Remove and re-add with getStats succeeding + fc.removeCounter(oid); + countersTable.del(expectedKey); + EXPECT_TRUE(fc.isEmpty()); + + failGetStats = false; + + fc.addCounter(oid, oid, counterValues); + EXPECT_FALSE(fc.isEmpty()); + + waitForCounterKeys(countersTable, 1); + waitForCounterValues(countersTable, expectedKey, + {"SAI_PORT_STAT_IF_IN_OCTETS", "SAI_PORT_STAT_IF_IN_ERRORS"}, + {"100", "200"}); + + // Cleanup + fc.removeCounter(oid); + countersTable.del(expectedKey); + EXPECT_TRUE(fc.isEmpty()); + + std::vector keys; + countersTable.getKeys(keys); + removeTimeStamp(keys, countersTable); + ASSERT_TRUE(keys.empty()); } From f3d8e69dc1c594fcc1a8a4883bbfadb79850945e Mon Sep 17 00:00:00 2001 From: Justin Wong Date: Sat, 4 Jul 2026 01:08:33 +0000 Subject: [PATCH 3/4] func -> function Signed-off-by: Justin Wong --- syncd/FlexCounter.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/syncd/FlexCounter.cpp b/syncd/FlexCounter.cpp index 0b54eab798..76cc9d4c84 100644 --- a/syncd/FlexCounter.cpp +++ b/syncd/FlexCounter.cpp @@ -1441,7 +1441,7 @@ class CounterContext : public BaseCounterContext m_counterGroupsSorted.push_back(makeCounterGroupRef(idx, m_supportedCounterGroups[idx].size())); // Must adhere to strict weak ordering - // Use lambda instead of static func to avoid SWSS_LOG_ENTER CI build requirement for better performance + // Use lambda instead of a static function to avoid SWSS_LOG_ENTER CI build requirement for better performance std::sort(m_counterGroupsSorted.begin(), m_counterGroupsSorted.end(), [](CounterGroupRef const& lhs, CounterGroupRef const& rhs) { From f348a0145218f2fae7f171ce06abc1735327a4fa Mon Sep 17 00:00:00 2001 From: Justin Wong Date: Tue, 7 Jul 2026 17:21:21 +0000 Subject: [PATCH 4/4] fix missing brace from conflict resolution Signed-off-by: Justin Wong --- unittest/syncd/TestFlexCounter.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/unittest/syncd/TestFlexCounter.cpp b/unittest/syncd/TestFlexCounter.cpp index b52e656e21..c46fa4b99f 100644 --- a/unittest/syncd/TestFlexCounter.cpp +++ b/unittest/syncd/TestFlexCounter.cpp @@ -2828,6 +2828,7 @@ TEST_F(FlexCounterTcpFallback, tcpFallbackWhenNoUnixSocket) EXPECT_TRUE(fc.isEmpty()); countersTable.del(key); +} TEST(FlexCounter, failedPollsCountAndCleanUp) {