From 1b3fd36582f47b6b29f403602f79c0d3f8f1b9f7 Mon Sep 17 00:00:00 2001 From: Sonic Build Admin Date: Thu, 16 Jul 2026 02:52:35 +0000 Subject: [PATCH] [master] Smart Counter Poll to allow counters to work properly on Broadcom platforms. #### Summary This is change is to address https://github.com/sonic-net/sonic-sairedis/issues/1753 and the master implementation of the HLD added in https://github.com/sonic-net/SONiC/pull/2190. `FlexCounter.cpp` assumes all ports support the same counter capabilities. This causes issues on most Broadcom platform switches as there are different types of ports on a switch that does not support the same set of counters. Fix by dynamically discovering what each interface is capable of during initialization of `syncd`. For more detials please refer to the above issue and HLD. #### Testing Master testing is also done, but due to frequent changes in master tests fail on other issues depending which commit is used for testing. However, none of the issues seem related to this change. More comprehensive testing is done on the 202511 stable branch: Testing is done on both a `Arista-7060X6-16PE-384C-B-O128S2` and `Arista-7260CX3-D108C8` on 202511 with the tests: ``` sonic-mgmt/tests/dhcp_relay/test_dhcp_counter_stress.py sonic-mgmt/tests/drop_packets/test_drop_counters.py sonic-mgmt/tests/drop_packets/test_configurable_drop_counters.py sonic-mgmt/tests/gnmi/test_gnmi_countersdb.py sonic-mgmt/tests/snmp/test_snmp_queue_counters.py ``` A full `sonic-mgmt` test suite run has also been ran (202511). There are no notable fallout compared to `sonic-mgmt` runs without this change. This is ran on both XGS and DNX Broadcom platforms. **EDIT: Jul 13, 2026** **202605 Testing** Additional 202605 testing has been done. Comparing full 202605 `sonic-mgmt` test results on TH5 (using `Arista-7060X6-64PE`) with and without these changes, there is no difference in the test results. #### Performance Impact: All logic change is only done in the counter initialisation stage of FlexCounters.cpp - there is no polling logic change at all. **Therefore, any performance impact is limited to any new execution of `syncd` - i.e. reboot / `config reload` / `systemctl restart`.** Tested on several topologies: image This impact seems reasonable for what this offers. The fastest operation that would cause a `syncd` restart is a `systemctl restart swss`, that is a operation that spans minutes. The worst real life scenario on a HwSKU with older CPU and many interfaces takes additional ~17 seconds. New, high interface-count HwSKUs only takes an extra ~2 seconds. As for memory usage - the impact is on the order of kilobytes, the impact is negligible as the system has GBs of RAM. Signed-off-by: Sonic Build Admin --- syncd/FlexCounter.cpp | 786 +++++++++++++++++++++++++++-- syncd/FlexCounter.h | 13 + unittest/syncd/TestFlexCounter.cpp | 505 +++++++++++++++++- 3 files changed, 1250 insertions(+), 54 deletions(-) diff --git a/syncd/FlexCounter.cpp b/syncd/FlexCounter.cpp index 15d7d971de..11f3e4e5c2 100644 --- a/syncd/FlexCounter.cpp +++ b/syncd/FlexCounter.cpp @@ -538,7 +538,16 @@ 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}; + } public: typedef CounterIds CounterIdsType; typedef BulkStatsContext BulkContextType; @@ -554,6 +563,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 +1028,209 @@ 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"); + } + m_objectIdsMap[vid] = std::make_shared>(rid, intf_counter_ids); + 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 + const std::set& largest_set = m_supportedCounterGroups[m_counterGroupsSorted[0].idx]; + std::vector supportedIds(largest_set.begin(), largest_set.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); + + size_t groupIndex = m_objectSupportedCountersGroupMap[vid]; + std::vector objCounterIds(m_supportedCounterGroups[groupIndex].begin(), + m_supportedCounterGroups[groupIndex].end()); + m_objectIdsMap[vid] = std::make_shared>(rid, objCounterIds); + } + 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 +1263,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); @@ -990,23 +1313,12 @@ class CounterContext : public BaseCounterContext auto rid = rids[i]; auto vid = vids[i]; std::vector stats(counter_ids.size()); - if (!collectData(rid, counter_ids, effective_stats_mode, false, stats)) - { - // Workaround for N/A Counters on Broadcom management ports - // Adding VIDs with unsupported counters to objectIdsMap to unblock testing - // Please see https://github.com/sonic-net/sonic-sairedis/issues/1753 for details - SWSS_LOG_INFO("counter read failed on RID 0x%" PRIx64 " on intf 0x%" PRIx64 ", adding to objectIdsMap regardless", rid, 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); + if (collectData(rid, counter_ids, effective_stats_mode, false, stats)) { + 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()); } - - auto counter_data = std::make_shared>(rid, counter_ids); - m_objectIdsMap.emplace(vid, counter_data); - SWSS_LOG_INFO("Fallback to single call for object 0x%" PRIx64, vid); } return; } @@ -1085,16 +1397,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 { @@ -1119,6 +1422,85 @@ 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 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) + { + 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) + { + 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(); + m_freeGroupIndices.push_back(removedGroupIdx); + } + } + + void removeObject( _In_ sai_object_id_t vid, _In_ bool log) @@ -1128,6 +1510,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); } @@ -1139,8 +1523,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,13 +1545,23 @@ class CounterContext : public BaseCounterContext kv.second->getStatsMode() == SAI_STATS_MODE_READ_AND_CLEAR) ? SAI_STATS_MODE_READ_AND_CLEAR : SAI_STATS_MODE_READ; } - // Workaround for N/A Counters on Broadcom management ports - // Using '0' for unsupported counters to unblock testing - // Please see https://github.com/sonic-net/sonic-sairedis/issues/1753 for details std::vector stats(statIds.size(), 0); - if (!collectData(rid, statIds, effective_stats_mode, false, stats)) + if (!collectData(rid, statIds, effective_stats_mode, true, stats)) { - SWSS_LOG_INFO("counter read failed on RID 0x%" PRIx64 " on intf 0x%" PRIx64 ", filling with '0' value", rid, vid); + uint32_t n = ++m_failedPolls[{rid, vid}]; + if (n == 1) + { + SWSS_LOG_DEBUG("counter read failed 1 time on RID 0x%" PRIx64 " on intf 0x%" PRIx64, rid, vid); + } + else if (n <= 3) + { + SWSS_LOG_DEBUG("counter read failed %u times on RID 0x%" PRIx64 " on intf 0x%" PRIx64, n, rid, vid); + } + else if (n == 4) + { + SWSS_LOG_ERROR("counter read failed more than 3 times on RID 0x%" PRIx64 " on intf 0x%" PRIx64, rid, vid); + } + continue; } std::vector values; @@ -1224,6 +1620,56 @@ 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()) + { + addGroup(counter_ids_set); + } + // 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].empty() && m_supportedCounterGroups[i] == counter_ids_set) + { + groupExists = true; + break; + } + } + if (!groupExists) + { + addGroup(counter_ids_set); + } + } + return counter_ids_set; + } + + bool isCounterSupported( _In_ StatType counter) const { @@ -1542,6 +1988,132 @@ 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.count(vid)) + { + 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 + const std::set& existingGroup = m_supportedCounterGroups[m_objectSupportedCountersGroupMap[vid]]; + for (auto &counter : existingGroup) + { + if (std::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 + 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) + 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) + { + const std::set& oldGroup = m_supportedCounterGroups[m_objectSupportedCountersGroupMap[vid]]; + newCounters.insert(oldGroup.begin(), oldGroup.end()); + } + // 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]) + { + 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; + } + } + 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] = addGroup(supportedIds); + } + void updateSupportedCounters( _In_ sai_object_id_t rid, _In_ const std::vector& counter_ids, @@ -1652,6 +2224,12 @@ 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::vector m_freeGroupIndices; + std::map> m_objectIdsMap; std::map, std::shared_ptr> m_bulkContexts; }; @@ -1680,6 +2258,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, @@ -1708,6 +2296,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, @@ -2658,6 +3256,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, @@ -2777,6 +3385,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, @@ -3804,11 +4422,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) { @@ -3865,11 +4504,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) { @@ -3891,11 +4552,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..3069eadb92 100644 --- a/unittest/syncd/TestFlexCounter.cpp +++ b/unittest/syncd/TestFlexCounter.cpp @@ -917,7 +917,7 @@ TEST(FlexCounter, queryCounterCapability) } }; - 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->mock_getStats = [](sai_object_type_t, sai_object_id_t, uint32_t number_of_counters, const sai_stat_id_t *counter_ids, uint64_t *counters) { for (uint32_t i = 0; i < number_of_counters; i++) { counters[i] = 1000; @@ -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 @@ -2447,3 +2447,502 @@ TEST_F(FlexCounterTcpFallback, tcpFallbackWhenNoUnixSocket) countersTable.del(key); } + +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()); +} + +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()); +}