diff --git a/syncd/FlexCounter.cpp b/syncd/FlexCounter.cpp index 79be74eaa..e1e8b7c91 100644 --- a/syncd/FlexCounter.cpp +++ b/syncd/FlexCounter.cpp @@ -10,6 +10,7 @@ #include using namespace syncd; +using namespace std; #define MUTEX std::unique_lock _lock(m_mtx); #define MUTEX_UNLOCK _lock.unlock(); @@ -18,6 +19,7 @@ FlexCounter::FlexCounter( _In_ const std::string& instanceId, _In_ std::shared_ptr vendorSai, _In_ const std::string& dbCounters): + m_readyToPoll(false), m_pollInterval(0), m_instanceId(instanceId), m_vendorSai(vendorSai), @@ -633,7 +635,14 @@ void FlexCounter::setTunnelCounterList( for (auto &counter : counterIds) { if (isTunnelCounterSupported(counter)) - SWSS_LOG_NOTICE("Tunnel %s does not have supported counters", sai_serialize_object_id(tunnelRid).c_str()); + { + supportedIds.push_back(counter); + } + } + + if (supportedIds.empty()) + { + SWSS_LOG_NOTICE("Tunnel %s does not have supported counters", sai_serialize_object_id(tunnelRid).c_str()); return; } @@ -1190,7 +1199,7 @@ void FlexCounter::addCounterPlugin( } // notify thread to start polling - m_pollCond.notify_all(); + notifyPoll(); } bool FlexCounter::isEmpty() @@ -2138,10 +2147,7 @@ void FlexCounter::flexCounterThreadRunFunction() MUTEX_UNLOCK; // explicit unlock // nothing to collect, wait until notified - - std::unique_lock lk(m_mtxSleep); - - m_pollCond.wait(lk); // wait on mutex + waitPoll(); } } @@ -2169,7 +2175,7 @@ void FlexCounter::endFlexCounterThread(void) m_runFlexCounterThread = false; - m_pollCond.notify_all(); + notifyPoll(); m_cvSleep.notify_all(); @@ -3136,5 +3142,19 @@ void FlexCounter::addCounter( } // notify thread to start polling + notifyPoll(); +} + +void FlexCounter::waitPoll() +{ + std::unique_lock lk(m_mtxSleep); + m_pollCond.wait(lk, [&](){return m_readyToPoll;}); + m_readyToPoll = false; +} + +void FlexCounter::notifyPoll() +{ + std::unique_lock lk(m_mtxSleep); + m_readyToPoll = true; m_pollCond.notify_all(); } diff --git a/syncd/FlexCounter.h b/syncd/FlexCounter.h index 702bf370a..fad045485 100644 --- a/syncd/FlexCounter.h +++ b/syncd/FlexCounter.h @@ -489,6 +489,11 @@ namespace syncd void removeCollectCountersHandler( _In_ const std::string& key); + private: + void waitPoll(); + + void notifyPoll(); + private: // plugins std::set m_queuePlugins; @@ -540,6 +545,8 @@ namespace syncd std::condition_variable m_pollCond; + bool m_readyToPoll; + uint32_t m_pollInterval; std::string m_instanceId; diff --git a/unittest/meta/TestSaiSerialize.cpp b/unittest/meta/TestSaiSerialize.cpp index 2fc26d937..8a8ceca29 100644 --- a/unittest/meta/TestSaiSerialize.cpp +++ b/unittest/meta/TestSaiSerialize.cpp @@ -338,6 +338,11 @@ TEST(SaiSerialize, sai_serialize_tunnel_stat) EXPECT_EQ(sai_serialize_tunnel_stat(SAI_TUNNEL_STAT_IN_OCTETS), "SAI_TUNNEL_STAT_IN_OCTETS"); } +TEST(SaiSerialize, sai_serialize_counter_stat) +{ + EXPECT_EQ(sai_serialize_counter_stat(SAI_COUNTER_STAT_PACKETS), "SAI_COUNTER_STAT_PACKETS"); +} + TEST(SaiSerialize, sai_serialize_queue_attr) { EXPECT_EQ(sai_serialize_queue_attr(SAI_QUEUE_ATTR_TYPE), "SAI_QUEUE_ATTR_TYPE"); diff --git a/unittest/syncd/Makefile.am b/unittest/syncd/Makefile.am index 8a76873df..999405282 100644 --- a/unittest/syncd/Makefile.am +++ b/unittest/syncd/Makefile.am @@ -1,11 +1,16 @@ -AM_CXXFLAGS = $(SAIINC) -I$(top_srcdir)/syncd -I$(top_srcdir)/lib -I$(top_srcdir)/vslib +AM_CXXFLAGS = $(SAIINC) -I$(top_srcdir)/syncd -I$(top_srcdir)/lib -I$(top_srcdir)/vslib -I$(top_srcdir)/meta bin_PROGRAMS = tests LDADD_GTEST = -L/usr/src/gtest -lgtest -lgtest_main tests_SOURCES = main.cpp \ - TestCommandLineOptions.cpp + ../../meta/DummySaiInterface.cpp \ + MockHelper.cpp \ + TestCommandLineOptions.cpp \ + TestFlexCounter.cpp tests_CXXFLAGS = $(DBGFLAGS) $(AM_CXXFLAGS) $(CXXFLAGS_COMMON) -tests_LDADD = $(LDADD_GTEST) $(top_srcdir)/syncd/libSyncd.a -lswsscommon -lpthread -L$(top_srcdir)/lib/.libs -lsairedis -L$(top_srcdir)/meta/.libs -lsaimetadata -lsaimeta -lzmq $(CODE_COVERAGE_LIBS) +tests_LDADD = $(LDADD_GTEST) $(top_srcdir)/syncd/libSyncd.a -lhiredis -lswsscommon -lpthread -L$(top_srcdir)/lib/.libs -lsairedis -L$(top_srcdir)/meta/.libs -lsaimetadata -lsaimeta -lzmq $(CODE_COVERAGE_LIBS) + +TESTS = tests diff --git a/unittest/syncd/MockHelper.cpp b/unittest/syncd/MockHelper.cpp new file mode 100644 index 000000000..ff7192cf8 --- /dev/null +++ b/unittest/syncd/MockHelper.cpp @@ -0,0 +1,36 @@ +#include "VidManager.h" +#include "swss/dbconnector.h" +#include "swss/table.h" + +namespace test_syncd { + sai_object_type_t mock_objectTypeQuery_result; + void mockVidManagerObjectTypeQuery(sai_object_type_t mock_result) + { + mock_objectTypeQuery_result = mock_result; + } +} + +namespace syncd { + sai_object_type_t VidManager::objectTypeQuery(_In_ sai_object_id_t objectId) + { + return test_syncd::mock_objectTypeQuery_result; + } +} + +/* +namespace swss { + DBConnector::DBConnector(const std::string& dbName, unsigned int timeout, bool isTcpConn) + { + } + + DBConnector *DBConnector::newConnector(unsigned int timeout) const + { + return nullptr; + } + + Table::Table(RedisPipeline *pipeline, const std::string &tableName, bool buffered) + { + } + + +}*/ diff --git a/unittest/syncd/MockHelper.h b/unittest/syncd/MockHelper.h new file mode 100644 index 000000000..21e56c74a --- /dev/null +++ b/unittest/syncd/MockHelper.h @@ -0,0 +1,5 @@ +#pragma once + +namespace test_syncd { + void mockVidManagerObjectTypeQuery(sai_object_type_t); +} \ No newline at end of file diff --git a/unittest/syncd/MockableSaiInterface.h b/unittest/syncd/MockableSaiInterface.h new file mode 100644 index 000000000..9ca6bc62d --- /dev/null +++ b/unittest/syncd/MockableSaiInterface.h @@ -0,0 +1,324 @@ +#pragma once + +#include + +#include "DummySaiInterface.h" + +// Use gmock? +class MockableSaiInterface: public saimeta::DummySaiInterface +{ + public: + + MockableSaiInterface() {} + + virtual ~MockableSaiInterface() {} + + public: + + virtual sai_status_t initialize( + _In_ uint64_t flags, + _In_ const sai_service_method_table_t *service_method_table) override { return SAI_STATUS_SUCCESS; } + + virtual sai_status_t uninitialize(void) override { return SAI_STATUS_SUCCESS; } + + public: // SAI interface overrides + + virtual sai_status_t create( + _In_ sai_object_type_t objectType, + _Out_ sai_object_id_t* objectId, + _In_ sai_object_id_t switchId, + _In_ uint32_t attr_count, + _In_ const sai_attribute_t *attr_list) override + { + if (mock_create) + { + return mock_create(objectType, objectId, switchId, attr_count, attr_list); + } + + return SAI_STATUS_SUCCESS; + } + + std::function mock_create; + + virtual sai_status_t remove( + _In_ sai_object_type_t objectType, + _In_ sai_object_id_t objectId) override + { + if (mock_remove) + { + return mock_remove(objectType, objectId); + } + + return SAI_STATUS_SUCCESS; + } + + std::function mock_remove; + + virtual sai_status_t set( + _In_ sai_object_type_t objectType, + _In_ sai_object_id_t objectId, + _In_ const sai_attribute_t *attr) override + { + if (mock_set) + { + return mock_set(objectType, objectId, attr); + } + + return SAI_STATUS_SUCCESS; + } + + std::function mock_set; + + virtual sai_status_t get( + _In_ sai_object_type_t objectType, + _In_ sai_object_id_t objectId, + _In_ uint32_t attr_count, + _Inout_ sai_attribute_t *attr_list) override + { + if (mock_get) + { + return mock_get(objectType, objectId, attr_count, attr_list); + } + + return SAI_STATUS_SUCCESS; + } + + std::function mock_get; + + public: // bulk QUAD oid + + virtual sai_status_t bulkCreate( + _In_ sai_object_type_t object_type, + _In_ sai_object_id_t switch_id, + _In_ uint32_t object_count, + _In_ const uint32_t *attr_count, + _In_ const sai_attribute_t **attr_list, + _In_ sai_bulk_op_error_mode_t mode, + _Out_ sai_object_id_t *object_id, + _Out_ sai_status_t *object_statuses) override + { + if (mock_bulkCreate) + { + return mock_bulkCreate(object_type, switch_id, object_count, attr_count, attr_list, mode, object_id, object_statuses); + } + + return SAI_STATUS_SUCCESS; + } + + std::function mock_bulkCreate; + + virtual sai_status_t bulkRemove( + _In_ sai_object_type_t object_type, + _In_ uint32_t object_count, + _In_ const sai_object_id_t *object_id, + _In_ sai_bulk_op_error_mode_t mode, + _Out_ sai_status_t *object_statuses) override + { + if (mock_bulkRemove) + { + return mock_bulkRemove(object_type, object_count, object_id, mode, object_statuses); + } + + return SAI_STATUS_SUCCESS; + } + + std::function mock_bulkRemove; + + virtual sai_status_t bulkSet( + _In_ sai_object_type_t object_type, + _In_ uint32_t object_count, + _In_ const sai_object_id_t *object_id, + _In_ const sai_attribute_t *attr_list, + _In_ sai_bulk_op_error_mode_t mode, + _Out_ sai_status_t *object_statuses) override + { + if (mock_bulkSet) + { + return mock_bulkSet(object_type, object_count, object_id, attr_list, mode, object_statuses); + } + + return SAI_STATUS_SUCCESS; + } + + std::function mock_bulkSet; + + public: // stats API + + virtual sai_status_t getStats( + _In_ sai_object_type_t object_type, + _In_ sai_object_id_t object_id, + _In_ uint32_t number_of_counters, + _In_ const sai_stat_id_t *counter_ids, + _Out_ uint64_t *counters) override + { + if (mock_getStats) + { + return mock_getStats(object_type, object_id, number_of_counters, counter_ids, counters); + } + + return SAI_STATUS_SUCCESS; + } + + std::function mock_getStats; + + virtual sai_status_t queryStatsCapability( + _In_ sai_object_id_t switch_id, + _In_ sai_object_type_t object_type, + _Inout_ sai_stat_capability_list_t *stats_capability) override + { + if (mock_queryStatsCapability) + { + return mock_queryStatsCapability(switch_id, object_type, stats_capability); + } + + return SAI_STATUS_SUCCESS; + } + + std::function mock_queryStatsCapability; + + virtual sai_status_t getStatsExt( + _In_ sai_object_type_t object_type, + _In_ sai_object_id_t object_id, + _In_ uint32_t number_of_counters, + _In_ const sai_stat_id_t *counter_ids, + _In_ sai_stats_mode_t mode, + _Out_ uint64_t *counters) override + { + if (mock_getStatsExt) + { + return mock_getStatsExt(object_type, object_id, number_of_counters, counter_ids, mode, counters); + } + + return SAI_STATUS_SUCCESS; + } + + std::function mock_getStatsExt; + + virtual sai_status_t clearStats( + _In_ sai_object_type_t object_type, + _In_ sai_object_id_t object_id, + _In_ uint32_t number_of_counters, + _In_ const sai_stat_id_t *counter_ids) override + { + if (mock_clearStats) + { + return mock_clearStats(object_type, object_id, number_of_counters, counter_ids); + } + + return SAI_STATUS_SUCCESS; + } + + std::function mock_clearStats; + + + public: // non QUAD API + + virtual sai_status_t flushFdbEntries( + _In_ sai_object_id_t switchId, + _In_ uint32_t attrCount, + _In_ const sai_attribute_t *attrList) override + { + if (mock_flushFdbEntries) + { + return mock_flushFdbEntries(switchId, attrCount, attrList); + } + + return SAI_STATUS_SUCCESS; + } + + std::function mock_flushFdbEntries; + + public: // SAI API + + virtual sai_status_t objectTypeGetAvailability( + _In_ sai_object_id_t switchId, + _In_ sai_object_type_t objectType, + _In_ uint32_t attrCount, + _In_ const sai_attribute_t *attrList, + _Out_ uint64_t *count) override + { + if (mock_objectTypeGetAvailability) + { + return mock_objectTypeGetAvailability(switchId, objectType, attrCount, attrList, count); + } + + return SAI_STATUS_SUCCESS; + } + + std::function mock_objectTypeGetAvailability; + + + virtual sai_status_t queryAttributeCapability( + _In_ sai_object_id_t switch_id, + _In_ sai_object_type_t object_type, + _In_ sai_attr_id_t attr_id, + _Out_ sai_attr_capability_t *capability) override + { + if (mock_queryAttributeCapability) + { + return mock_queryAttributeCapability(switch_id, object_type, attr_id, capability); + } + + return SAI_STATUS_SUCCESS; + } + + std::function mock_queryAttributeCapability; + + + virtual sai_status_t queryAattributeEnumValuesCapability( + _In_ sai_object_id_t switch_id, + _In_ sai_object_type_t object_type, + _In_ sai_attr_id_t attr_id, + _Inout_ sai_s32_list_t *enum_values_capability) override + { + if (mock_queryAattributeEnumValuesCapability) + { + return mock_queryAattributeEnumValuesCapability(switch_id, object_type, attr_id, enum_values_capability); + } + + return SAI_STATUS_SUCCESS; + } + + std::function mock_queryAattributeEnumValuesCapability; + + virtual sai_object_type_t objectTypeQuery( + _In_ sai_object_id_t objectId) override + { + if (mock_objectTypeQuery) + { + return mock_objectTypeQuery(objectId); + } + + return SAI_OBJECT_TYPE_NULL; + } + + std::function mock_objectTypeQuery; + + + virtual sai_object_id_t switchIdQuery( + _In_ sai_object_id_t objectId) override + { + if (mock_switchIdQuery) + { + return mock_switchIdQuery(objectId); + } + + return 0; + } + + std::function mock_switchIdQuery; + + virtual sai_status_t logSet( + _In_ sai_api_t api, + _In_ sai_log_level_t log_level) override + { + if (mock_logSet) + { + return mock_logSet(api, log_level); + } + + return SAI_STATUS_SUCCESS; + } + + std::function mock_logSet; +}; diff --git a/unittest/syncd/TestFlexCounter.cpp b/unittest/syncd/TestFlexCounter.cpp new file mode 100644 index 000000000..a98483d08 --- /dev/null +++ b/unittest/syncd/TestFlexCounter.cpp @@ -0,0 +1,77 @@ +#include "FlexCounter.h" +#include "MockableSaiInterface.h" +#include "MockHelper.h" + +#include + + +using namespace syncd; +using namespace std; + +TEST(FlexCounter, addRemoveCounterForFlowCounter) +{ + 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(FLOW_COUNTER_ID_LIST, "SAI_COUNTER_STAT_PACKETS,SAI_COUNTER_STAT_BYTES"); + + test_syncd::mockVidManagerObjectTypeQuery(SAI_OBJECT_TYPE_COUNTER); + sai->mock_getStatsExt = [](sai_object_type_t, sai_object_id_t, uint32_t number_of_counters, const sai_stat_id_t *, sai_stats_mode_t, uint64_t *counters) { + for (uint32_t i = 0; i < number_of_counters; i++) + { + counters[i] = (i + 1) * 100; + } + return SAI_STATUS_SUCCESS; + }; + + fc.addCounter(counterVid, counterRid, values); + EXPECT_EQ(fc.isEmpty(), false); + + values.clear(); + values.emplace_back(POLL_INTERVAL_FIELD, "1000"); + fc.addCounterPlugin(values); + + values.clear(); + 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_COUNTER_STAT_PACKETS", value); + EXPECT_EQ(value, "100"); + countersTable.hget("oid:0x64", "SAI_COUNTER_STAT_BYTES", value); + EXPECT_EQ(value, "200"); + + fc.removeCounter(counterVid); + EXPECT_EQ(fc.isEmpty(), true); + countersTable.getKeys(keys); + + ASSERT_TRUE(keys.empty()); + +} + +TEST(FlexCounter, addRemoveCounterPluginForFlowCounter) +{ + std::shared_ptr sai(new MockableSaiInterface()); + FlexCounter fc("test", sai, "COUNTERS_DB"); + + std::vector values; + values.emplace_back(FLOW_COUNTER_PLUGIN_FIELD, "dummy_sha_strings"); + fc.addCounterPlugin(values); + EXPECT_EQ(fc.isEmpty(), false); + + fc.removeCounterPlugins(); + EXPECT_EQ(fc.isEmpty(), true); +}