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()); +}