diff --git a/syncd/FlexCounter.cpp b/syncd/FlexCounter.cpp index e3467f42d0f0..356668ab0dbc 100644 --- a/syncd/FlexCounter.cpp +++ b/syncd/FlexCounter.cpp @@ -239,14 +239,15 @@ void FlexCounter::setPortCounterList( { SWSS_LOG_ENTER(); - updateSupportedPortCounters(portId); + PortCountersSet supportedPortCounters; + updateSupportedPortCounters(portId, supportedPortCounters); // Remove unsupported counters std::vector supportedIds; for (auto &counter : counterIds) { - if (isPortCounterSupported(counter)) + if (supportedPortCounters.count(counter) != 0) { supportedIds.push_back(counter); } @@ -1361,13 +1362,6 @@ bool FlexCounter::allPluginsEmpty() const m_flowCounterPlugins.empty(); } -bool FlexCounter::isPortCounterSupported(sai_port_stat_t counter) const -{ - SWSS_LOG_ENTER(); - - return m_supportedPortCounters.count(counter) != 0; -} - bool FlexCounter::isQueueCounterSupported( _In_ sai_queue_stat_t counter) const { @@ -2437,7 +2431,8 @@ void FlexCounter::endFlexCounterThread(void) } sai_status_t FlexCounter::querySupportedPortCounters( - _In_ sai_object_id_t portRid) + _In_ sai_object_id_t portRid, + _Out_ PortCountersSet &supportedPortCounters) { SWSS_LOG_ENTER(); @@ -2463,15 +2458,16 @@ sai_status_t FlexCounter::querySupportedPortCounters( if (status != SAI_STATUS_SUCCESS) { - SWSS_LOG_INFO("Unable to get port supported counters for %s", - sai_serialize_object_id(portRid).c_str()); + SWSS_LOG_NOTICE("Unable to query port supported counters for %s: %s", + sai_serialize_object_id(portRid).c_str(), + sai_serialize_status(status).c_str()); } else { for (auto statCapability: statCapabilityList) { sai_port_stat_t counter = static_cast(statCapability.stat_enum); - m_supportedPortCounters.insert(counter); + supportedPortCounters.insert(counter); } } } @@ -2479,7 +2475,8 @@ sai_status_t FlexCounter::querySupportedPortCounters( } void FlexCounter::getSupportedPortCounters( - _In_ sai_object_id_t portRid) + _In_ sai_object_id_t portRid, + _Out_ PortCountersSet &supportedPortCounters) { SWSS_LOG_ENTER(); @@ -2493,7 +2490,7 @@ void FlexCounter::getSupportedPortCounters( if (status != SAI_STATUS_SUCCESS) { - SWSS_LOG_INFO("Counter %s is not supported on port RID %s: %s", + SWSS_LOG_NOTICE("Counter %s is not supported on port RID %s: %s", sai_serialize_port_stat(counter).c_str(), sai_serialize_object_id(portRid).c_str(), sai_serialize_status(status).c_str()); @@ -2501,26 +2498,22 @@ void FlexCounter::getSupportedPortCounters( continue; } - m_supportedPortCounters.insert(counter); + supportedPortCounters.insert(counter); } } void FlexCounter::updateSupportedPortCounters( - _In_ sai_object_id_t portRid) + _In_ sai_object_id_t portRid, + _Out_ PortCountersSet &supportedPortCounters) { SWSS_LOG_ENTER(); - if (m_supportedPortCounters.size()) - { - return; - } - /* Query SAI supported port counters */ - sai_status_t status = querySupportedPortCounters(portRid); + sai_status_t status = querySupportedPortCounters(portRid, supportedPortCounters); if (status != SAI_STATUS_SUCCESS) { /* Fallback to legacy approach */ - getSupportedPortCounters(portRid); + getSupportedPortCounters(portRid, supportedPortCounters); } } diff --git a/syncd/FlexCounter.h b/syncd/FlexCounter.h index 1eb5e8a62db6..cfc79b6604c0 100644 --- a/syncd/FlexCounter.h +++ b/syncd/FlexCounter.h @@ -213,9 +213,6 @@ namespace syncd private: // is counter supported - bool isPortCounterSupported( - _In_ sai_port_stat_t counter) const; - bool isPriorityGroupCounterSupported( _In_ sai_ingress_priority_group_stat_t counter) const; @@ -237,14 +234,18 @@ namespace syncd private: // update supported counters + typedef std::set PortCountersSet; sai_status_t querySupportedPortCounters( - _In_ sai_object_id_t portRid); + _In_ sai_object_id_t portRid, + _Out_ PortCountersSet &supportedPortCounters); void getSupportedPortCounters( - _In_ sai_object_id_t portRid); + _In_ sai_object_id_t portRid, + _Out_ PortCountersSet &supportedPortCounters); void updateSupportedPortCounters( - _In_ sai_object_id_t portRid); + _In_ sai_object_id_t portRid, + _Out_ PortCountersSet &supportedPortCounters); std::vector saiCheckSupportedPortDebugCounters( _In_ sai_object_id_t portRid, @@ -545,7 +546,6 @@ namespace syncd private: // supported counters - std::set m_supportedPortCounters; std::set m_supportedPriorityGroupCounters; std::set m_supportedQueueCounters; std::set m_supportedRifCounters; diff --git a/unittest/syncd/MockableSaiInterface.cpp b/unittest/syncd/MockableSaiInterface.cpp index 30b52a2d3205..c96764e3a40c 100644 --- a/unittest/syncd/MockableSaiInterface.cpp +++ b/unittest/syncd/MockableSaiInterface.cpp @@ -165,7 +165,7 @@ sai_status_t MockableSaiInterface::queryStatsCapability( return mock_queryStatsCapability(switch_id, object_type, stats_capability); } - return SAI_STATUS_SUCCESS; + return SAI_STATUS_NOT_SUPPORTED; } sai_status_t MockableSaiInterface::getStatsExt( diff --git a/unittest/syncd/TestFlexCounter.cpp b/unittest/syncd/TestFlexCounter.cpp index 66a9d60a7abf..090b7c533303 100644 --- a/unittest/syncd/TestFlexCounter.cpp +++ b/unittest/syncd/TestFlexCounter.cpp @@ -186,3 +186,93 @@ TEST(FlexCounter, addRemoveCounterForMACsecSA) } +TEST(FlexCounter, addRemoveCounterForPort) +{ + std::shared_ptr sai(new MockableSaiInterface()); + FlexCounter fc("test", sai, "COUNTERS_DB"); + + sai_object_id_t counterVid{100}; + sai_object_id_t counterRid{100}; + std::vector values; + values.emplace_back(PORT_COUNTER_ID_LIST, "SAI_PORT_STAT_IF_IN_OCTETS,SAI_PORT_STAT_IF_IN_ERRORS"); + + test_syncd::mockVidManagerObjectTypeQuery(SAI_OBJECT_TYPE_PORT); + sai->mock_getStats = [](sai_object_type_t, sai_object_id_t, uint32_t number_of_counters, const sai_stat_id_t *ids, uint64_t *counters) { + for (uint32_t i = 0; i < number_of_counters; i++) + { + if (ids[i] == SAI_PORT_STAT_IF_IN_OCTETS) + { + counters[i] = 100; + } + else if (ids[i] == SAI_PORT_STAT_IF_IN_ERRORS) + { + counters[i] = 200; + } + else + { + return SAI_STATUS_FAILURE; + } + } + return SAI_STATUS_SUCCESS; + }; + + fc.addCounter(counterVid, counterRid, values); + EXPECT_EQ(fc.isEmpty(), false); + + values.clear(); + values.emplace_back(POLL_INTERVAL_FIELD, "1000"); + values.emplace_back(FLEX_COUNTER_STATUS_FIELD, "enable"); + fc.addCounterPlugin(values); + + usleep(1000*1000); + swss::DBConnector db("COUNTERS_DB", 0); + swss::RedisPipeline pipeline(&db); + swss::Table countersTable(&pipeline, COUNTERS_TABLE, false); + + std::vector keys; + countersTable.getKeys(keys); + EXPECT_EQ(keys.size(), size_t(1)); + EXPECT_EQ(keys[0], "oid:0x64"); + + std::string value; + countersTable.hget("oid:0x64", "SAI_PORT_STAT_IF_IN_OCTETS", value); + EXPECT_EQ(value, "100"); + countersTable.hget("oid:0x64", "SAI_PORT_STAT_IF_IN_ERRORS", value); + EXPECT_EQ(value, "200"); + + fc.removeCounter(counterVid); + EXPECT_EQ(fc.isEmpty(), true); + countersTable.del("oid:0x64"); + countersTable.getKeys(keys); + ASSERT_TRUE(keys.empty()); + + // Test again with queryStatsCapability support + sai->mock_queryStatsCapability = [](sai_object_id_t, sai_object_type_t, sai_stat_capability_list_t *capability) { + if (capability->count < 2) + { + capability->count = 2; + return SAI_STATUS_BUFFER_OVERFLOW; + } + + capability->list[0].stat_enum = SAI_PORT_STAT_IF_IN_OCTETS; + capability->list[1].stat_enum = SAI_PORT_STAT_IF_IN_ERRORS; + return SAI_STATUS_SUCCESS; + }; + + values.clear(); + values.emplace_back(PORT_COUNTER_ID_LIST, "SAI_PORT_STAT_IF_IN_OCTETS,SAI_PORT_STAT_IF_IN_ERRORS"); + fc.addCounter(counterVid, counterRid, values); + EXPECT_EQ(fc.isEmpty(), false); + + usleep(1000*1000); + countersTable.hget("oid:0x64", "SAI_PORT_STAT_IF_IN_OCTETS", value); + EXPECT_EQ(value, "100"); + countersTable.hget("oid:0x64", "SAI_PORT_STAT_IF_IN_ERRORS", value); + EXPECT_EQ(value, "200"); + + fc.removeCounter(counterVid); + EXPECT_EQ(fc.isEmpty(), true); + countersTable.del("oid:0x64"); + countersTable.getKeys(keys); + ASSERT_TRUE(keys.empty()); +}