From 0fe378f34d6a27b0bba815240637789da7f0c8fa Mon Sep 17 00:00:00 2001 From: Ashish Singh Date: Fri, 17 Nov 2023 14:25:49 -0800 Subject: [PATCH] [Link Event Damping] Add tracker to track the selectable timers used by link event damper. - Link event damper running on each port will have its own SelectableTimer. This class helps create a mapping of SelectableTimer and object that created the SelectableTimer so that when timer is fired, proper EventHandler object can be invoked. HLD: sonic-net/SONiC#1071 --- syncd/Makefile.am | 1 + syncd/SelectablesTracker.cpp | 103 ++++++++++++++ syncd/SelectablesTracker.h | 54 ++++++++ tests/aspell.en.pws | 1 + unittest/syncd/Makefile.am | 1 + unittest/syncd/MockSelectablesTracker.h | 23 ++++ unittest/syncd/TestSelectablesTracker.cpp | 161 ++++++++++++++++++++++ 7 files changed, 344 insertions(+) create mode 100644 syncd/SelectablesTracker.cpp create mode 100644 syncd/SelectablesTracker.h create mode 100644 unittest/syncd/MockSelectablesTracker.h create mode 100644 unittest/syncd/TestSelectablesTracker.cpp diff --git a/syncd/Makefile.am b/syncd/Makefile.am index 4982ea16c..fbb43a770 100644 --- a/syncd/Makefile.am +++ b/syncd/Makefile.am @@ -43,6 +43,7 @@ libSyncd_a_SOURCES = \ SaiObj.cpp \ SaiSwitch.cpp \ SaiSwitchInterface.cpp \ + SelectablesTracker.cpp \ ServiceMethodTable.cpp \ SingleReiniter.cpp \ SwitchNotifications.cpp \ diff --git a/syncd/SelectablesTracker.cpp b/syncd/SelectablesTracker.cpp new file mode 100644 index 000000000..041f586ea --- /dev/null +++ b/syncd/SelectablesTracker.cpp @@ -0,0 +1,103 @@ +#include "SelectablesTracker.h" + +#include "swss/logger.h" + +using namespace syncd; + +bool SelectablesTracker::addSelectableToTracker( + _In_ swss::Selectable *selectable, + _In_ SelectableEventHandler *eventHandler) +{ + SWSS_LOG_ENTER(); + + if (selectable == nullptr) + { + SWSS_LOG_ERROR("Invalid Selectable:Selectable is NULL."); + + return false; + } + + int fd = selectable->getFd(); + if (eventHandler == nullptr) + { + SWSS_LOG_ERROR("Event handler for Selectable with fd: %d is NULL.", fd); + + return false; + } + + if (m_selectableFdToEventHandlerMap.count(fd) != 0) + { + SWSS_LOG_ERROR("Selectable with fd %d is already in use.", fd); + + return false; + } + + SWSS_LOG_INFO("Adding the Selectable with fd: %d.", fd); + m_selectableFdToEventHandlerMap[fd] = eventHandler; + + return true; +} + +bool SelectablesTracker::removeSelectableFromTracker( + _In_ swss::Selectable *selectable) +{ + SWSS_LOG_ENTER(); + + if (selectable == nullptr) + { + SWSS_LOG_ERROR("Invalid Selectable:Selectable is NULL."); + + return false; + } + + int fd = selectable->getFd(); + + SWSS_LOG_INFO("Removing the Selectable with fd: %d.", fd); + if (m_selectableFdToEventHandlerMap.erase(fd) == 0) + { + SWSS_LOG_ERROR("Selectable with fd %d is not present in the map!", fd); + + return false; + } + + return true; +} + +bool SelectablesTracker::selectableIsTracked( + _In_ swss::Selectable *selectable) +{ + SWSS_LOG_ENTER(); + + if ((selectable == nullptr) || + (m_selectableFdToEventHandlerMap.count(selectable->getFd()) == 0)) + { + return false; + } + + return true; +} + +SelectableEventHandler *SelectablesTracker::getEventHandlerForSelectable( + _In_ swss::Selectable *selectable) +{ + SWSS_LOG_ENTER(); + + if (selectable == nullptr) + { + SWSS_LOG_ERROR("Invalid Selectable:Selectable is NULL."); + + return nullptr; + } + + int fd = selectable->getFd(); + auto it = m_selectableFdToEventHandlerMap.find(fd); + + if (it == m_selectableFdToEventHandlerMap.end()) + { + SWSS_LOG_ERROR("Selectable with fd %d is not present in the map!", fd); + + return nullptr; + } + + return it->second; +} diff --git a/syncd/SelectablesTracker.h b/syncd/SelectablesTracker.h new file mode 100644 index 000000000..e2fb9f8ed --- /dev/null +++ b/syncd/SelectablesTracker.h @@ -0,0 +1,54 @@ +#pragma once + +#include + +#include "SelectableEventHandler.h" +#include "swss/sal.h" +#include "swss/selectable.h" +#include "swss/selectabletimer.h" + +namespace syncd +{ + // This class keeps track of Selectable being used by an entity and their + // corresponding EventHandler objects. + class SelectablesTracker + { + private: + + // Not copyable or movable. + SelectablesTracker(const SelectablesTracker &) = delete; + SelectablesTracker(SelectablesTracker &&) = delete; + SelectablesTracker &operator=(const SelectablesTracker &) = delete; + SelectablesTracker &operator=(SelectablesTracker &&) = delete; + + public: + + SelectablesTracker() = default; + + virtual ~SelectablesTracker() = default; + + // Adds the mapping of Selectable and it's corresponding EventHandler object. + virtual bool addSelectableToTracker( + _In_ swss::Selectable *selectable, + _In_ SelectableEventHandler *eventHandler); + + // Removes a Selectable from the map. + virtual bool removeSelectableFromTracker( + _In_ swss::Selectable *selectable); + + // Checks if Selectable is present in the tracker map. + virtual bool selectableIsTracked( + _In_ swss::Selectable *selectable); + + // Gets the EventHandler for a Selectable. + virtual SelectableEventHandler *getEventHandlerForSelectable( + _In_ swss::Selectable *selectable); + + private: + + using SelectableFdToEventHandlerMap = std::unordered_map; + + SelectableFdToEventHandlerMap m_selectableFdToEventHandlerMap; + }; + +} // namespace syncd diff --git a/tests/aspell.en.pws b/tests/aspell.en.pws index f7832828d..b3e89149f 100644 --- a/tests/aspell.en.pws +++ b/tests/aspell.en.pws @@ -149,6 +149,7 @@ config const CONST consts +copyable counterName countOnly cout diff --git a/unittest/syncd/Makefile.am b/unittest/syncd/Makefile.am index 27c301e25..b42ed3ac7 100644 --- a/unittest/syncd/Makefile.am +++ b/unittest/syncd/Makefile.am @@ -17,6 +17,7 @@ tests_SOURCES = main.cpp \ TestNotificationHandler.cpp \ TestMdioIpcServer.cpp \ TestPortStateChangeHandler.cpp \ + TestSelectablesTracker.cpp \ TestVendorSai.cpp tests_CXXFLAGS = $(DBGFLAGS) $(AM_CXXFLAGS) $(CXXFLAGS_COMMON) diff --git a/unittest/syncd/MockSelectablesTracker.h b/unittest/syncd/MockSelectablesTracker.h new file mode 100644 index 000000000..1b08119f4 --- /dev/null +++ b/unittest/syncd/MockSelectablesTracker.h @@ -0,0 +1,23 @@ +#pragma once + +#include "SelectablesTracker.h" +#include "gmock/gmock.h" + +namespace syncd +{ + + class MockSelectablesTracker : public SelectablesTracker + { + public: + + MockSelectablesTracker() : SelectablesTracker() {} + + ~MockSelectablesTracker() override {} + + MOCK_METHOD2(addSelectableToTracker, bool(swss::Selectable *selectable, + SelectableEventHandler *eventHandler) override); + + MOCK_METHOD1(removeSelectableFromTracker, + bool(swss::Selectable *selectable) override); + }; +} // namespace syncd diff --git a/unittest/syncd/TestSelectablesTracker.cpp b/unittest/syncd/TestSelectablesTracker.cpp new file mode 100644 index 000000000..4b167e87a --- /dev/null +++ b/unittest/syncd/TestSelectablesTracker.cpp @@ -0,0 +1,161 @@ +#include + +#include "SelectablesTracker.h" + +using namespace syncd; + +class SelectableEventHandlerTestHelper : public SelectableEventHandler +{ + public: + + SelectableEventHandlerTestHelper() {} + ~SelectableEventHandlerTestHelper() {} + + void handleSelectableEvent() {} +}; + +class SelectablesTrackerTest : public ::testing::Test +{ + public: + + SelectablesTrackerTest() {} + + SelectablesTracker m_selectablesTracker_; +}; + +TEST_F(SelectablesTrackerTest, AddSelectableFailsIfSelectableNull) +{ + SelectableEventHandlerTestHelper eventHandler; + + EXPECT_FALSE(m_selectablesTracker_.addSelectableToTracker( + /*selectable=*/nullptr, (SelectableEventHandler *)&eventHandler)); +} + +TEST_F(SelectablesTrackerTest, AddSelectableFailsIfEventHandlerNull) +{ + swss::SelectableTimer timer(/*interval=*/{.tv_sec = 10, .tv_nsec = 20}); + + EXPECT_FALSE(m_selectablesTracker_.addSelectableToTracker( + (swss::Selectable *)&timer, /*eventHandler=*/nullptr)); +} + +TEST_F(SelectablesTrackerTest, AddSelectableSucceeds) +{ + swss::SelectableTimer timer(/*interval=*/{.tv_sec = 10, .tv_nsec = 20}); + SelectableEventHandlerTestHelper eventHandler; + + EXPECT_TRUE(m_selectablesTracker_.addSelectableToTracker( + (swss::Selectable *)&timer, (SelectableEventHandler *)&eventHandler)); +} + +TEST_F(SelectablesTrackerTest, AddSelectableFailsIfSelectableExists) +{ + // Add a selectable timer to tracker. + swss::SelectableTimer timer(/*interval=*/{.tv_sec = 10, .tv_nsec = 20}); + SelectableEventHandlerTestHelper eventHandler; + + EXPECT_TRUE(m_selectablesTracker_.addSelectableToTracker( + (swss::Selectable *)&timer, (SelectableEventHandler *)&eventHandler)); + + // Adding the same selectable timer to tracker should fail. + SelectableEventHandlerTestHelper eventHandler2; + EXPECT_FALSE(m_selectablesTracker_.addSelectableToTracker( + (swss::Selectable *)&timer, (SelectableEventHandler *)&eventHandler2)); +} + +TEST_F(SelectablesTrackerTest, RemoveSelectableFailsIfSelectableNull) +{ + EXPECT_FALSE( + m_selectablesTracker_.removeSelectableFromTracker(/*selectable=*/nullptr)); +} + +TEST_F(SelectablesTrackerTest, RemoveSelectableFailsIfSelectableNotExist) +{ + swss::SelectableTimer timer(/*interval=*/{.tv_sec = 10, .tv_nsec = 20}); + + EXPECT_FALSE(m_selectablesTracker_.removeSelectableFromTracker( + (swss::Selectable *)&timer)); +} + +TEST_F(SelectablesTrackerTest, RemoveSelectableSucceeds) +{ + // Add a selectable timer. + swss::SelectableTimer timer(/*interval=*/{.tv_sec = 10, .tv_nsec = 20}); + SelectableEventHandlerTestHelper eventHandler; + + EXPECT_TRUE(m_selectablesTracker_.addSelectableToTracker( + (swss::Selectable *)&timer, (SelectableEventHandler *)&eventHandler)); + + // Remove the selectable timer just added in tracker. + EXPECT_TRUE(m_selectablesTracker_.removeSelectableFromTracker( + (swss::Selectable *)&timer)); + + // Removing the selectable timer again should fail. + EXPECT_FALSE(m_selectablesTracker_.removeSelectableFromTracker( + (swss::Selectable *)&timer)); +} + +TEST_F(SelectablesTrackerTest, NullptrSelectableIsNotTracked) +{ + EXPECT_FALSE( + m_selectablesTracker_.selectableIsTracked(/*selectable=*/nullptr)); +} + +TEST_F(SelectablesTrackerTest, SelectableIsNotTracked) +{ + swss::SelectableTimer timer(/*interval=*/{.tv_sec = 10, .tv_nsec = 20}); + EXPECT_FALSE( + m_selectablesTracker_.selectableIsTracked((swss::Selectable *)&timer)); +} + +TEST_F(SelectablesTrackerTest, SelectableIsTracked) +{ + // Add the Selectable in the tracker. + swss::SelectableTimer timer(/*interval=*/{.tv_sec = 10, .tv_nsec = 20}); + SelectableEventHandlerTestHelper eventHandler; + + EXPECT_TRUE(m_selectablesTracker_.addSelectableToTracker( + (swss::Selectable *)&timer, (SelectableEventHandler *)&eventHandler)); + + // Verify the Selectable in the tracker. + EXPECT_TRUE( + m_selectablesTracker_.selectableIsTracked((swss::Selectable *)&timer)); + + // Remove the Selectable from tracker. + EXPECT_TRUE(m_selectablesTracker_.removeSelectableFromTracker( + (swss::Selectable *)&timer)); + + // Check the Selectable in tracker again, this time it should fail. + EXPECT_FALSE( + m_selectablesTracker_.selectableIsTracked((swss::Selectable *)&timer)); +} + +TEST_F(SelectablesTrackerTest, GetEventHandlerFailsIfSelectableNull) +{ + EXPECT_EQ( + m_selectablesTracker_.getEventHandlerForSelectable(/*selectable=*/nullptr), + nullptr); +} + +TEST_F(SelectablesTrackerTest, GetEventHandlerFailsIfSelectableNotExist) +{ + swss::SelectableTimer timer(/*interval=*/{.tv_sec = 10, .tv_nsec = 20}); + EXPECT_EQ(m_selectablesTracker_.getEventHandlerForSelectable( + (swss::Selectable *)&timer), + nullptr); +} + +TEST_F(SelectablesTrackerTest, GetEventHandlerSucceeds) +{ + // Add a selectable timer. + swss::SelectableTimer timer(/*interval=*/{.tv_sec = 10, .tv_nsec = 20}); + SelectableEventHandlerTestHelper eventHandler; + + EXPECT_TRUE(m_selectablesTracker_.addSelectableToTracker( + (swss::Selectable *)&timer, (SelectableEventHandler *)&eventHandler)); + + // Get the event handler for the selectable timer. + EXPECT_EQ(m_selectablesTracker_.getEventHandlerForSelectable( + (swss::Selectable *)&timer), + (SelectableEventHandler *)&eventHandler); +}