From f06bc9dffff3407178233457b88bac8fa418bccd Mon Sep 17 00:00:00 2001 From: Sivakumar Thirukkanna Thevar Date: Mon, 22 Jun 2026 14:53:47 +0530 Subject: [PATCH 1/7] Fixing elastic test failures + code review comments of PR#1906 Signed-off-by: Sivakumar Thirukkanna Thevar --- syncd/LinkEventDamping.h | 63 ++++++++ syncd/Syncd.cpp | 209 +++++++++++++++++++++----- syncd/Syncd.h | 79 +++------- unittest/syncd/Makefile.am | 2 +- unittest/syncd/TestSyncd.cpp | 281 +++++++++++++++++++++++++++++++++++ 5 files changed, 536 insertions(+), 98 deletions(-) create mode 100644 syncd/LinkEventDamping.h diff --git a/syncd/LinkEventDamping.h b/syncd/LinkEventDamping.h new file mode 100644 index 0000000000..902b5a838f --- /dev/null +++ b/syncd/LinkEventDamping.h @@ -0,0 +1,63 @@ +#pragma once + +#include "sai.h" +#include "sairedis.h" + +namespace syncd +{ + /** + * @brief Link event damping configuration and state per port + */ + struct LinkEventDampingPortState + { + // Configuration parameters + sai_redis_link_event_damping_algorithm_t algorithm; + sai_redis_link_event_damping_algo_aied_config_t aied_config; + + // Runtime state for AIED algorithm + uint32_t current_penalty; // Current penalty value + uint64_t last_transition_time_ms; // Timestamp of last transition (milliseconds) + uint64_t last_decay_time_ms; // Timestamp of last decay calculation (milliseconds) + uint64_t damping_start_time_ms; // When damping state started (milliseconds) + bool is_damping_active; // Whether link is currently in damped state + sai_port_oper_status_t physical_status; // Physical port status + sai_port_oper_status_t advertised_status; // Last advertised status (may differ due to damping) + sai_port_oper_status_t last_suppressed_status; // Last event suppressed while damping + bool pending_state_sync; // Flag to indicate state mismatch needs propagation + + // Counters for monitoring + uint64_t pre_damping_link_transitions; + uint64_t pre_damping_up_events; + uint64_t pre_damping_down_events; + uint64_t post_damping_up_events; + uint64_t post_damping_down_events; + uint64_t post_damping_link_transitions; + + // Constructor with defaults + LinkEventDampingPortState() + : algorithm(SAI_REDIS_LINK_EVENT_DAMPING_ALGORITHM_DISABLED), + current_penalty(0), + last_transition_time_ms(0), + last_decay_time_ms(0), + damping_start_time_ms(0), + is_damping_active(false), + physical_status(SAI_PORT_OPER_STATUS_UNKNOWN), + advertised_status(SAI_PORT_OPER_STATUS_UNKNOWN), + last_suppressed_status(SAI_PORT_OPER_STATUS_UNKNOWN), + pending_state_sync(false), + pre_damping_link_transitions(0), + pre_damping_up_events(0), + pre_damping_down_events(0), + post_damping_up_events(0), + post_damping_down_events(0), + post_damping_link_transitions(0) + { + // Initialize AIED config with defaults + aied_config.max_suppress_time = 0; + aied_config.suppress_threshold = 0; + aied_config.reuse_threshold = 0; + aied_config.decay_half_life = 0; + aied_config.flap_penalty = 0; + } + }; +} diff --git a/syncd/Syncd.cpp b/syncd/Syncd.cpp index 64138af182..2dc30a293f 100644 --- a/syncd/Syncd.cpp +++ b/syncd/Syncd.cpp @@ -877,7 +877,6 @@ sai_status_t Syncd::processLinkEventDampingConfigSet( // Extract object type and object ID std::string strObjectType = key.substr(0, colon_pos); std::string strObjectId = key.substr(colon_pos + 1); - sai_object_type_t objectType; sai_deserialize_object_type(strObjectType, objectType); @@ -885,7 +884,7 @@ sai_status_t Syncd::processLinkEventDampingConfigSet( if (objectType != SAI_OBJECT_TYPE_PORT) { SWSS_LOG_ERROR("invalid object type for link event damping config: %s", - strObjectType.c_str()); + strObjectType.c_str()); sendLinkEventDampingConfigResponse(SAI_STATUS_INVALID_PARAMETER); return SAI_STATUS_INVALID_PARAMETER; } @@ -911,10 +910,11 @@ sai_status_t Syncd::processLinkEventDampingConfigSet( return SAI_STATUS_INVALID_PARAMETER; } - // Link event damping is a software-based feature implemented in syncd. - // Store the configuration parameters on the port object so that - // OnPortStateChange can apply the damping algorithm before forwarding notifications. - // The damping parameters will be used to decide whether to suppress link state changes. + /* Link event damping is a software-based feature implemented in syncd. + * Store the configuration parameters on the port object so that OnPortStateChange + * can apply the damping algorithm before forwarding notifications. The damping + * parameters will be used to decide whether to suppress link state changes. + */ sai_status_t status = SAI_STATUS_SUCCESS; // Acquire lock to protect damping state @@ -930,7 +930,7 @@ sai_status_t Syncd::processLinkEventDampingConfigSet( std::string strAttrValue = fvValue(v); SWSS_LOG_DEBUG("processing link event damping attribute: %s = %s", - strAttrId.c_str(), strAttrValue.c_str()); + strAttrId.c_str(), strAttrValue.c_str()); // Deserialize attribute ID sai_redis_port_attr_t attrId; @@ -947,11 +947,8 @@ sai_status_t Syncd::processLinkEventDampingConfigSet( SWSS_LOG_INFO("setting link event damping algorithm on port %s: %d", strObjectId.c_str(), algo); - // Link event damping is a software-only feature as of now - // Store the configuration locally for use in notification - // processing. + // Store the configuration locally for use in notification processing dampingState.algorithm = algo; - status = SAI_STATUS_SUCCESS; break; } @@ -960,7 +957,6 @@ sai_status_t Syncd::processLinkEventDampingConfigSet( { // Allocate temporary memory for the config structure sai_redis_link_event_damping_algo_aied_config_t config{}; - sai_deserialize_redis_link_event_damping_aied_config(strAttrValue, config); SWSS_LOG_INFO("setting link event damping AIED config on port %s: " @@ -970,11 +966,7 @@ sai_status_t Syncd::processLinkEventDampingConfigSet( config.suppress_threshold, config.reuse_threshold, config.decay_half_life, config.flap_penalty); - // Link event damping is a software-only feature as of now - // Store the configuration locally for use in notification - // processing. dampingState.aied_config = config; - status = SAI_STATUS_SUCCESS; break; } @@ -997,7 +989,6 @@ sai_status_t Syncd::processLinkEventDampingConfigSet( } sendLinkEventDampingConfigResponse(status); - return status; } @@ -1013,7 +1004,6 @@ void Syncd::sendLinkEventDampingConfigResponse( } std::string strStatus = sai_serialize_status(status); - std::vector entry; SWSS_LOG_INFO("sending link event damping config response: %s", strStatus.c_str()); @@ -1064,7 +1054,6 @@ void Syncd::decayPenalty( } // Penalty decay formula: P(t) = P0 * (0.5 ^ (t / half_life)) - // We use floating point for the calculation double half_lives = (double)elapsed_ms / state.aied_config.decay_half_life; double decay_factor = std::pow(0.5, half_lives); uint32_t decayed_penalty = (uint32_t)(state.current_penalty * decay_factor); @@ -1197,13 +1186,13 @@ bool Syncd::applyAiedAlgorithm( writeDampingCountersToStateDb(portVid, state); } - // Damping exits when EITHER: - // 1. Time-based: damping_duration_ms >= max_suppress_time - // 2. Penalty-based: current_penalty < reuse_threshold (decay-based recovery) + /* Damping exits when EITHER: + * 1. Time-based: damping_duration_ms >= max_suppress_time + * 2. Penalty-based: current_penalty < reuse_threshold (decay-based recovery) + */ if (state.is_damping_active) { // Check timeout - never suppress longer than max_suppress_time - // This is a hard timestamp-based limit to prevent infinite suppression uint64_t damping_duration_ms = currentTimeMs - state.damping_start_time_ms; if (damping_duration_ms >= state.aied_config.max_suppress_time) @@ -1238,7 +1227,7 @@ bool Syncd::applyAiedAlgorithm( // Penalty decays based on last_decay_time tracking else if (state.current_penalty < state.aied_config.reuse_threshold) { - // Store temporary strings to avoid dangling pointers + // Penalty decayed below resue threshold. Exit damping. std::string physicalStatusStr = sai_serialize_port_oper_status(state.physical_status); std::string advertisedStatusStr = sai_serialize_port_oper_status(state.advertised_status); SWSS_LOG_NOTICE("Port VID %s exiting damped state: penalty (%u) < " @@ -1270,8 +1259,9 @@ bool Syncd::applyAiedAlgorithm( // Determine if notification should be suppressed bool should_suppress = false; - // Only suppress when damping is active AND this is NOT the threshold-crossing event - // The threshold-crossing event itself should be propagated + /* Only suppress when damping is already active. When entering + * into damping active, the event should be propagated. + */ if (state.is_damping_active && was_damping_active_before) { // Calculate current suppression time based on damping algorithm @@ -1405,7 +1395,6 @@ void Syncd::checkDampedPortsTimeout() SWSS_LOG_ENTER(); std::lock_guard lock(m_linkEventDampingMutex); - uint64_t currentTimeMs = getCurrentTimeMs(); // Iterate through all ports with damping configured @@ -1451,8 +1440,8 @@ void Syncd::checkDampedPortsTimeout() { state.pending_state_sync = true; - SWSS_LOG_NOTICE("Marked pending sync (decay) for Port VID %s: " - "physical=%s advertised=%s", + SWSS_LOG_NOTICE("Marked pending sync (decay) for Port VID %s: " + "physical=%s advertised=%s", portVidStr.c_str(), physicalStatusStr.c_str(), advertisedStatusStr.c_str()); } @@ -1515,6 +1504,37 @@ void Syncd::checkDampedPortsTimeout() } } +bool Syncd::hasAnyValidDampingConfig() +{ + SWSS_LOG_ENTER(); + + std::lock_guard lock(m_linkEventDampingMutex); + + for (const auto& kv : m_portLinkEventDampingStates) + { + const auto& state = kv.second; + + // Check if algorithm is AIED + if (state.algorithm != SAI_REDIS_LINK_EVENT_DAMPING_ALGORITHM_AIED) + { + continue; + } + + /* Check if configuration is valid: + * 1. suppress_threshold > reuse_threshold + * 2. max_suppress_time > decay_half_life + */ + if (state.aied_config.suppress_threshold > state.aied_config.reuse_threshold && + state.aied_config.max_suppress_time > state.aied_config.decay_half_life) + { + // Found at least one port with valid damping configuration + return true; + } + } + + return false; +} + void Syncd::processPendingDampingSync() { SWSS_LOG_ENTER(); @@ -1533,7 +1553,6 @@ void Syncd::processPendingDampingSync() state.advertised_status != state.physical_status) { state.pending_state_sync = false; - state.advertised_status = state.physical_status; sai_port_oper_status_notification_t n; @@ -1546,14 +1565,90 @@ void Syncd::processPendingDampingSync() } } - // ALWAYS send from main thread (safe) + // Don't send - just enqueue if (!notifications.empty()) { - std::string s = sai_serialize_port_oper_status_ntf( - (uint32_t)notifications.size(), - notifications.data()); - std::vector entry; - m_notifications->send(SAI_SWITCH_NOTIFICATION_NAME_PORT_STATE_CHANGE, s, entry); + // Build port VID list for logging + std::string portVids; + portVids.reserve(notifications.size() * 24); + + for (size_t i = 0; i < notifications.size(); ++i) + { + if (i > 0) portVids += ", "; + portVids += sai_serialize_object_id(notifications[i].port_id); + } + + SWSS_LOG_NOTICE("processPendingDampingSync: enqueuing %zu port state notifications for ports: %s", + notifications.size(), portVids.c_str()); + + std::lock_guard lock(m_pendingNotificationsMutex); + + // Queue overflow protection + if (m_pendingNotifications.size() >= 1000) + { + SWSS_LOG_ERROR("Pending notification queue overflow: size=%zu, dropping oldest batch", + m_pendingNotifications.size()); + m_pendingNotifications.pop(); // Drop oldest + } + + m_pendingNotifications.push(notifications); + } +} + +void Syncd::flushPendingDampingNotifications() +{ + SWSS_LOG_ENTER(); + + std::queue> localQueue; + { + std::lock_guard lock(m_pendingNotificationsMutex); + std::swap(localQueue, m_pendingNotifications); + } + + if (!localQueue.empty()) + { + SWSS_LOG_NOTICE("flushPendingDampingNotifications: flushing %zu notification batches", localQueue.size()); + } + + while (!localQueue.empty()) + { + auto ¬ifications = localQueue.front(); + + // Build port VID list for logging + std::string portVids; + portVids.reserve(notifications.size() * 24); + + for (size_t i = 0; i < notifications.size(); ++i) + { + if (i > 0) portVids += ", "; + portVids += sai_serialize_object_id(notifications[i].port_id); + } + + SWSS_LOG_NOTICE("flushPendingDampingNotifications: sending %zu port state change notifications for ports: %s", + notifications.size(), portVids.c_str()); + + try + { + std::string s = sai_serialize_port_oper_status_ntf( + (uint32_t)notifications.size(), + notifications.data()); + + std::vector entry; + + m_notifications->send( + SAI_SWITCH_NOTIFICATION_NAME_PORT_STATE_CHANGE, + s, + entry); + } + + catch (const std::exception &e) + { + SWSS_LOG_ERROR("Failed to send port state notifications for ports %s: %s", + portVids.c_str(), e.what()); + // Continue processing remaining notifications instead of crashing + } + + localQueue.pop(); } } @@ -6718,6 +6813,33 @@ void Syncd::run() int result = s->select(&sel, 1000); + if (result == swss::Select::TIMEOUT) + { + SWSS_LOG_DEBUG("Select timeout"); + + // Only process damping functions if at least one port has valid damping config + if (hasAnyValidDampingConfig()) + { + // Process if any pending state sync due to link event damping + processPendingDampingSync(); + + // Flush in controlled manner + flushPendingDampingNotifications(); + } + continue; + } + else if (result == swss::Select::ERROR) + { + SWSS_LOG_ERROR("select errored, return value: %d", result); + continue; + } + else if (result == swss::Select::SIGNALINT) + { + SWSS_LOG_DEBUG("Select interrupted by a signal"); + continue; + } + + // result OBJECT case if (sel == m_restartQuery.get()) { /* @@ -6776,7 +6898,7 @@ void Syncd::run() if (status != SAI_STATUS_SUCCESS) { SWSS_LOG_ERROR("Failed to set SAI_SWITCH_ATTR_FAST_API_ENABLE=true: %s for express pre-shutdown. Fall back to cold restart", - sai_serialize_status(status).c_str()); + sai_serialize_status(status).c_str()); shutdownType = SYNCD_RESTART_TYPE_COLD; @@ -6830,11 +6952,18 @@ void Syncd::run() } else { - SWSS_LOG_ERROR("select failed: %d", result); + SWSS_LOG_ERROR("Select returned unknown selectable: %p", sel); } - // Process if any pending state sync due to link event damping - processPendingDampingSync(); + // Only process damping functions if at least one port has valid damping config + if (hasAnyValidDampingConfig()) + { + // Process if any pending state sync due to link event damping + processPendingDampingSync(); + + // Flush in controlled manner + flushPendingDampingNotifications(); + } } catch(const std::exception &e) { diff --git a/syncd/Syncd.h b/syncd/Syncd.h index e802c3b76c..0ebddc3a1d 100644 --- a/syncd/Syncd.h +++ b/syncd/Syncd.h @@ -18,6 +18,7 @@ #include "NotificationProducerBase.h" #include "TimerWatchdog.h" #include "MdioIpcServer.h" +#include "LinkEventDamping.h" #include "meta/SaiAttributeList.h" #include "meta/SelectableChannel.h" @@ -29,65 +30,10 @@ #include #include #include +#include namespace syncd { - /** - * @brief Link event damping configuration and state per port - */ - struct LinkEventDampingPortState - { - // Configuration parameters - sai_redis_link_event_damping_algorithm_t algorithm; - sai_redis_link_event_damping_algo_aied_config_t aied_config; - - // Runtime state for AIED algorithm - uint32_t current_penalty; // Current penalty value - uint64_t last_transition_time_ms; // Timestamp of last transition (milliseconds) - uint64_t last_decay_time_ms; // Timestamp of last decay calculation (milliseconds) - uint64_t damping_start_time_ms; // When damping state started (milliseconds) - bool is_damping_active; // Whether link is currently in damped state - sai_port_oper_status_t physical_status; // Physical port status - sai_port_oper_status_t advertised_status; // Last advertised status (may differ due to damping) - sai_port_oper_status_t last_suppressed_status; // Last event suppressed while damping - bool pending_state_sync; // Flag to indicate state mismatch needs propagation - - // Counters for monitoring - uint64_t pre_damping_link_transitions; - uint64_t pre_damping_up_events; - uint64_t pre_damping_down_events; - uint64_t post_damping_up_events; - uint64_t post_damping_down_events; - uint64_t post_damping_link_transitions; - - // Constructor with defaults - LinkEventDampingPortState() - : algorithm(SAI_REDIS_LINK_EVENT_DAMPING_ALGORITHM_DISABLED), - current_penalty(0), - last_transition_time_ms(0), - last_decay_time_ms(0), - damping_start_time_ms(0), - is_damping_active(false), - physical_status(SAI_PORT_OPER_STATUS_UNKNOWN), - advertised_status(SAI_PORT_OPER_STATUS_UNKNOWN), - last_suppressed_status(SAI_PORT_OPER_STATUS_UNKNOWN), - pending_state_sync(false), - pre_damping_link_transitions(0), - pre_damping_up_events(0), - pre_damping_down_events(0), - post_damping_up_events(0), - post_damping_down_events(0), - post_damping_link_transitions(0) - { - // Initialize AIED config with defaults - aied_config.max_suppress_time = 0; - aied_config.suppress_threshold = 0; - aied_config.reuse_threshold = 0; - aied_config.decay_half_life = 0; - aied_config.flap_penalty = 0; - } - }; - class Syncd { private: @@ -300,7 +246,7 @@ namespace syncd * @return true if should suppress, false if should propagate */ bool applyAiedAlgorithm( - _In_ sai_object_id_t portVid, + _In_ sai_object_id_t portVid, _In_ LinkEventDampingPortState& state, _In_ sai_port_oper_status_t newStatus, _In_ uint64_t currentTimeMs); @@ -348,6 +294,16 @@ namespace syncd */ void processPendingDampingSync(); + /** + * @brief link status flush notification after damping exit + */ + void flushPendingDampingNotifications(); + + /** + * @brief check any valid damping config exists in any port + */ + bool hasAnyValidDampingConfig(); + /** * @brief Write damping counters to STATE_DB for a specific port * @param portVid Virtual object ID of the port @@ -729,5 +685,14 @@ namespace syncd * @brief Mutex for damping timer thread synchronization */ std::mutex m_dampingTimerMutex; + + /** + * @brief queue for pending notifications + */ + std::queue> m_pendingNotifications; + /** + * @brief Mutex for pending notifications + */ + std::mutex m_pendingNotificationsMutex; }; } diff --git a/unittest/syncd/Makefile.am b/unittest/syncd/Makefile.am index 76c29e2466..49cec2fc36 100644 --- a/unittest/syncd/Makefile.am +++ b/unittest/syncd/Makefile.am @@ -28,7 +28,7 @@ tests_SOURCES = main.cpp \ TestVendorSai.cpp \ TestFlowDump.cpp -tests_CXXFLAGS = $(DBGFLAGS) $(AM_CXXFLAGS) $(CXXFLAGS_COMMON) +tests_CXXFLAGS = $(DBGFLAGS) $(AM_CXXFLAGS) $(CXXFLAGS_COMMON) -fno-access-control tests_LDFLAGS = -Wl,-rpath,$(top_srcdir)/lib/.libs -Wl,-rpath,$(top_srcdir)/meta/.libs tests_LDADD = $(LDADD_GTEST) $(top_srcdir)/syncd/libSyncdRequestShutdown.a $(top_srcdir)/syncd/libSyncd.a $(top_srcdir)/vslib/libSaiVS.a $(top_srcdir)/syncd/libMdioIpcClient.a \ -lhiredis -lswsscommon -lnl-genl-3 -lnl-nf-3 -lnl-route-3 -lnl-3 -lpthread -L$(top_srcdir)/lib/.libs -lsairedis -L$(top_srcdir)/meta/.libs -lsaimetadata -lsaimeta -lzmq -lz $(CODE_COVERAGE_LIBS) $(VPP_LIBS) diff --git a/unittest/syncd/TestSyncd.cpp b/unittest/syncd/TestSyncd.cpp index c041e29bcc..a7606a7625 100644 --- a/unittest/syncd/TestSyncd.cpp +++ b/unittest/syncd/TestSyncd.cpp @@ -879,6 +879,10 @@ class SyncdLinkEventDampingTest : public SyncdTest } }; +// Define static constant expression members for linkage +constexpr sai_object_id_t SyncdLinkEventDampingTest::PORT_VID; +constexpr sai_object_id_t SyncdLinkEventDampingTest::PORT_RID; + TEST_F(SyncdLinkEventDampingTest, flapsEnterDampingAndSuppress) { sai_redis_link_event_damping_algo_aied_config_t config; @@ -991,4 +995,281 @@ TEST_F(SyncdLinkEventDampingTest, noDampingConfiguredPropagates) // no STATE_DB entry is written for non-configured ports EXPECT_EQ(getDampingField("is_damping_active"), ""); } + +TEST_F(SyncdLinkEventDampingTest, processPendingDampingSyncWithNotifications) +{ + sai_redis_link_event_damping_algo_aied_config_t config; + config.max_suppress_time = 2000; + config.suppress_threshold = 100; + config.reuse_threshold = 50; + config.decay_half_life = 500; + config.flap_penalty = 1000; + + setDampingConfig(config); + + // Create the scenario: port goes DOWN (advertised), then UP (suppressed) + sendPortStateChange(SAI_PORT_OPER_STATUS_DOWN); + std::this_thread::sleep_for(std::chrono::milliseconds(50)); + + sendPortStateChange(SAI_PORT_OPER_STATUS_UP); + std::this_thread::sleep_for(std::chrono::milliseconds(50)); + + sendPortStateChange(SAI_PORT_OPER_STATUS_DOWN); + std::this_thread::sleep_for(std::chrono::milliseconds(50)); + + sendPortStateChange(SAI_PORT_OPER_STATUS_UP); + std::this_thread::sleep_for(std::chrono::milliseconds(50)); + + // Verify damping is active (UP was suppressed) + EXPECT_EQ(getDampingField("is_damping_active"), "true"); + + // Set up preconditions + { + std::lock_guard lock(m_syncd->m_linkEventDampingMutex); + + auto it = m_syncd->m_portLinkEventDampingStates.find(PORT_VID); + ASSERT_NE(it, m_syncd->m_portLinkEventDampingStates.end()) + << "Port damping state not found"; + + auto& state = it->second; + + // Simulate what the timer thread does when damping exits via timeout + state.pending_state_sync = true; + state.is_damping_active = false; + + // Verify the mismatch exists (this is what triggers the notification) + EXPECT_NE(state.advertised_status, state.physical_status) + << "Expected status mismatch for notification generation"; + } + + // Call the function + m_syncd->processPendingDampingSync(); + + // Verify the notification was enqueued + { + std::lock_guard lock(m_syncd->m_pendingNotificationsMutex); + EXPECT_EQ(m_syncd->m_pendingNotifications.size(), 1) + << "Expected 1 notification batch to be enqueued"; + + if (!m_syncd->m_pendingNotifications.empty()) + { + auto& batch = m_syncd->m_pendingNotifications.front(); + EXPECT_EQ(batch.size(), 1) << "Expected 1 notification in batch"; + EXPECT_EQ(batch[0].port_id, PORT_VID) << "Notification for wrong port"; + } + } + + // Verify pending_state_sync was cleared and advertised was updated + { + std::lock_guard lock(m_syncd->m_linkEventDampingMutex); + auto& state = m_syncd->m_portLinkEventDampingStates[PORT_VID]; + EXPECT_FALSE(state.pending_state_sync) << "pending_state_sync should be cleared"; + EXPECT_EQ(state.advertised_status, state.physical_status) + << "Advertised should match physical after sync"; + } +} + +TEST_F(SyncdLinkEventDampingTest, processPendingDampingSyncQueueOverflow) +{ + sai_redis_link_event_damping_algo_aied_config_t config; + config.max_suppress_time = 2000; + config.suppress_threshold = 100; + config.reuse_threshold = 50; + config.decay_half_life = 500; + config.flap_penalty = 1000; + + setDampingConfig(config); + + // Directly populate the queue to 1000 entries + { + std::lock_guard lock(m_syncd->m_pendingNotificationsMutex); + + sai_port_oper_status_notification_t dummy_ntf; + dummy_ntf.port_id = PORT_VID; + dummy_ntf.port_state = SAI_PORT_OPER_STATUS_UP; + + std::vector batch = {dummy_ntf}; + + // Fill queue to exactly 1000 entries + for (int i = 0; i < 1000; ++i) + { + m_syncd->m_pendingNotifications.push(batch); + } + + EXPECT_EQ(m_syncd->m_pendingNotifications.size(), 1000); + } + + // Set up the scenario to trigger one more notification + { + std::lock_guard lock(m_syncd->m_linkEventDampingMutex); + + auto& state = m_syncd->m_portLinkEventDampingStates.at(PORT_VID); + + // Set up the condition for processPendingDampingSync to queue a notification + state.pending_state_sync = true; + state.advertised_status = SAI_PORT_OPER_STATUS_DOWN; + state.physical_status = SAI_PORT_OPER_STATUS_UP; + } + + // This should trigger overflow protection + m_syncd->processPendingDampingSync(); + + // Verify the overflow protection worked + { + std::lock_guard lock(m_syncd->m_pendingNotificationsMutex); + + // Queue should still be 1000 (dropped oldest, added newest) + EXPECT_EQ(m_syncd->m_pendingNotifications.size(), 1000) + << "Queue should be capped at 1000 after overflow"; + } +} + +TEST_F(SyncdLinkEventDampingTest, flushPendingNotificationsWithBatches) +{ + // Directly populate the queue with multiple batches + { + std::lock_guard lock(m_syncd->m_pendingNotificationsMutex); + + // Create 3 different notification batches + for (int batch_num = 0; batch_num < 3; ++batch_num) + { + sai_port_oper_status_notification_t ntf; + ntf.port_id = PORT_VID; + ntf.port_state = (batch_num % 2 == 0) + ? SAI_PORT_OPER_STATUS_DOWN + : SAI_PORT_OPER_STATUS_UP; + + std::vector batch = {ntf}; + m_syncd->m_pendingNotifications.push(batch); + } + + EXPECT_EQ(m_syncd->m_pendingNotifications.size(), 3); + } + + m_syncd->flushPendingDampingNotifications(); + + // Verify all batches were flushed + { + std::lock_guard lock(m_syncd->m_pendingNotificationsMutex); + EXPECT_EQ(m_syncd->m_pendingNotifications.size(), 0) + << "All batches should be flushed"; + } +} + +TEST_F(SyncdLinkEventDampingTest, flushPendingNotificationsDrainsQueue) +{ + // Queue 2 notification batches + { + std::lock_guard lock(m_syncd->m_pendingNotificationsMutex); + + for (int i = 0; i < 2; ++i) + { + sai_port_oper_status_notification_t ntf; + ntf.port_id = PORT_VID; + ntf.port_state = SAI_PORT_OPER_STATUS_UP; + + std::vector batch = {ntf}; + m_syncd->m_pendingNotifications.push(batch); + } + } + + // Tests normal path + m_syncd->flushPendingDampingNotifications(); + + // Verify all batches were processed + { + std::lock_guard lock(m_syncd->m_pendingNotificationsMutex); + EXPECT_EQ(m_syncd->m_pendingNotifications.size(), 0); + } +} + +TEST_F(SyncdLinkEventDampingTest, fullNotificationFlowIntegrated) +{ + sai_redis_link_event_damping_algo_aied_config_t config; + config.max_suppress_time = 2000; + config.suppress_threshold = 100; + config.reuse_threshold = 50; + config.decay_half_life = 500; + config.flap_penalty = 1000; + + setDampingConfig(config); + + // Set up with pending notification + { + std::lock_guard lock(m_syncd->m_linkEventDampingMutex); + + auto& state = m_syncd->m_portLinkEventDampingStates.at(PORT_VID); + state.pending_state_sync = true; + state.advertised_status = SAI_PORT_OPER_STATUS_DOWN; + state.physical_status = SAI_PORT_OPER_STATUS_UP; + } + + // Step 1: processPendingDampingSync queues the notification + m_syncd->processPendingDampingSync(); + + // Verify notification was queued + { + std::lock_guard lock(m_syncd->m_pendingNotificationsMutex); + EXPECT_EQ(m_syncd->m_pendingNotifications.size(), 1); + } + + // Step 2: flushPendingDampingNotifications sends it + m_syncd->flushPendingDampingNotifications(); + + // Verify queue was flushed + { + std::lock_guard lock(m_syncd->m_pendingNotificationsMutex); + EXPECT_EQ(m_syncd->m_pendingNotifications.size(), 0); + } +} + +TEST_F(SyncdLinkEventDampingTest, flushPendingNotificationsEmptyQueue) +{ + // Ensure queue is empty + { + std::lock_guard lock(m_syncd->m_pendingNotificationsMutex); + EXPECT_EQ(m_syncd->m_pendingNotifications.size(), 0); + } + + // Call flush with empty queue + m_syncd->flushPendingDampingNotifications(); + + // Queue should still be empty + { + std::lock_guard lock(m_syncd->m_pendingNotificationsMutex); + EXPECT_EQ(m_syncd->m_pendingNotifications.size(), 0); + } +} + +TEST_F(SyncdLinkEventDampingTest, processPendingDampingSyncNoPending) +{ + sai_redis_link_event_damping_algo_aied_config_t config; + config.max_suppress_time = 2000; + config.suppress_threshold = 100; + config.reuse_threshold = 50; + config.decay_half_life = 500; + config.flap_penalty = 1000; + + setDampingConfig(config); + + // Set up state WITHOUT pending_state_sync + { + std::lock_guard lock(m_syncd->m_linkEventDampingMutex); + + auto& state = m_syncd->m_portLinkEventDampingStates.at(PORT_VID); + state.pending_state_sync = false; // No pending sync + state.advertised_status = SAI_PORT_OPER_STATUS_DOWN; + state.physical_status = SAI_PORT_OPER_STATUS_UP; + } + + // Call the function - should NOT queue any notifications + m_syncd->processPendingDampingSync(); + + // Verify NO notification was queued + { + std::lock_guard lock(m_syncd->m_pendingNotificationsMutex); + EXPECT_EQ(m_syncd->m_pendingNotifications.size(), 0) + << "No notification should be queued when pending_state_sync=false"; + } +} #endif From 5f322eeed1ebf579ca67c6251d7a7e7bff2cc54a Mon Sep 17 00:00:00 2001 From: Sivakumar Thirukkanna Thevar Date: Mon, 22 Jun 2026 15:37:34 +0530 Subject: [PATCH 2/7] spell check errro Signed-off-by: Sivakumar Thirukkanna Thevar --- syncd/Syncd.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/syncd/Syncd.cpp b/syncd/Syncd.cpp index 2dc30a293f..ced7b07f8f 100644 --- a/syncd/Syncd.cpp +++ b/syncd/Syncd.cpp @@ -1227,7 +1227,7 @@ bool Syncd::applyAiedAlgorithm( // Penalty decays based on last_decay_time tracking else if (state.current_penalty < state.aied_config.reuse_threshold) { - // Penalty decayed below resue threshold. Exit damping. + // Penalty decayed below reuse threshold. Exit damping. std::string physicalStatusStr = sai_serialize_port_oper_status(state.physical_status); std::string advertisedStatusStr = sai_serialize_port_oper_status(state.advertised_status); SWSS_LOG_NOTICE("Port VID %s exiting damped state: penalty (%u) < " From 81fea5d303d48f679d15979b6742692bb89af97d Mon Sep 17 00:00:00 2001 From: Sivakumar Thirukkanna Thevar Date: Tue, 23 Jun 2026 13:27:03 +0530 Subject: [PATCH 3/7] Addressed code review comments Signed-off-by: Sivakumar Thirukkanna Thevar --- syncd/Syncd.cpp | 93 +++++++++++------------------------- syncd/Syncd.h | 5 -- unittest/syncd/TestSyncd.cpp | 7 --- 3 files changed, 27 insertions(+), 78 deletions(-) diff --git a/syncd/Syncd.cpp b/syncd/Syncd.cpp index ced7b07f8f..ff79796a95 100644 --- a/syncd/Syncd.cpp +++ b/syncd/Syncd.cpp @@ -57,6 +57,21 @@ using namespace std::placeholders; #define WD_DELAY_FACTOR 1 #endif + +std::string serializePortVids(const std::vector& notifications) +{ + std::string portVids; + portVids.reserve(notifications.size() * 24); + + for (size_t i = 0; i < notifications.size(); ++i) + { + if (i > 0) portVids += ", "; + portVids += sai_serialize_object_id(notifications[i].port_id); + } + + return portVids; +} + Syncd::Syncd( _In_ std::shared_ptr vendorSai, _In_ std::shared_ptr cmd, @@ -1504,37 +1519,6 @@ void Syncd::checkDampedPortsTimeout() } } -bool Syncd::hasAnyValidDampingConfig() -{ - SWSS_LOG_ENTER(); - - std::lock_guard lock(m_linkEventDampingMutex); - - for (const auto& kv : m_portLinkEventDampingStates) - { - const auto& state = kv.second; - - // Check if algorithm is AIED - if (state.algorithm != SAI_REDIS_LINK_EVENT_DAMPING_ALGORITHM_AIED) - { - continue; - } - - /* Check if configuration is valid: - * 1. suppress_threshold > reuse_threshold - * 2. max_suppress_time > decay_half_life - */ - if (state.aied_config.suppress_threshold > state.aied_config.reuse_threshold && - state.aied_config.max_suppress_time > state.aied_config.decay_half_life) - { - // Found at least one port with valid damping configuration - return true; - } - } - - return false; -} - void Syncd::processPendingDampingSync() { SWSS_LOG_ENTER(); @@ -1569,15 +1553,7 @@ void Syncd::processPendingDampingSync() if (!notifications.empty()) { // Build port VID list for logging - std::string portVids; - portVids.reserve(notifications.size() * 24); - - for (size_t i = 0; i < notifications.size(); ++i) - { - if (i > 0) portVids += ", "; - portVids += sai_serialize_object_id(notifications[i].port_id); - } - + auto portVids = serializePortVids(notifications); SWSS_LOG_NOTICE("processPendingDampingSync: enqueuing %zu port state notifications for ports: %s", notifications.size(), portVids.c_str()); @@ -1591,7 +1567,8 @@ void Syncd::processPendingDampingSync() m_pendingNotifications.pop(); // Drop oldest } - m_pendingNotifications.push(notifications); + m_pendingNotifications.emplace(); + m_pendingNotifications.back().swap(notifications); } } @@ -1613,16 +1590,8 @@ void Syncd::flushPendingDampingNotifications() while (!localQueue.empty()) { auto ¬ifications = localQueue.front(); - // Build port VID list for logging - std::string portVids; - portVids.reserve(notifications.size() * 24); - - for (size_t i = 0; i < notifications.size(); ++i) - { - if (i > 0) portVids += ", "; - portVids += sai_serialize_object_id(notifications[i].port_id); - } + auto portVids = serializePortVids(notifications); SWSS_LOG_NOTICE("flushPendingDampingNotifications: sending %zu port state change notifications for ports: %s", notifications.size(), portVids.c_str()); @@ -6817,15 +6786,11 @@ void Syncd::run() { SWSS_LOG_DEBUG("Select timeout"); - // Only process damping functions if at least one port has valid damping config - if (hasAnyValidDampingConfig()) - { - // Process if any pending state sync due to link event damping - processPendingDampingSync(); + // Process if any pending state sync due to link event damping + processPendingDampingSync(); - // Flush in controlled manner - flushPendingDampingNotifications(); - } + // Flush in controlled manner + flushPendingDampingNotifications(); continue; } else if (result == swss::Select::ERROR) @@ -6955,15 +6920,11 @@ void Syncd::run() SWSS_LOG_ERROR("Select returned unknown selectable: %p", sel); } - // Only process damping functions if at least one port has valid damping config - if (hasAnyValidDampingConfig()) - { - // Process if any pending state sync due to link event damping - processPendingDampingSync(); + // Process if any pending state sync due to link event damping + processPendingDampingSync(); - // Flush in controlled manner - flushPendingDampingNotifications(); - } + // Flush in controlled manner + flushPendingDampingNotifications(); } catch(const std::exception &e) { diff --git a/syncd/Syncd.h b/syncd/Syncd.h index 0ebddc3a1d..333fa50521 100644 --- a/syncd/Syncd.h +++ b/syncd/Syncd.h @@ -299,11 +299,6 @@ namespace syncd */ void flushPendingDampingNotifications(); - /** - * @brief check any valid damping config exists in any port - */ - bool hasAnyValidDampingConfig(); - /** * @brief Write damping counters to STATE_DB for a specific port * @param portVid Virtual object ID of the port diff --git a/unittest/syncd/TestSyncd.cpp b/unittest/syncd/TestSyncd.cpp index a7606a7625..5c333f91f6 100644 --- a/unittest/syncd/TestSyncd.cpp +++ b/unittest/syncd/TestSyncd.cpp @@ -1009,16 +1009,9 @@ TEST_F(SyncdLinkEventDampingTest, processPendingDampingSyncWithNotifications) // Create the scenario: port goes DOWN (advertised), then UP (suppressed) sendPortStateChange(SAI_PORT_OPER_STATUS_DOWN); - std::this_thread::sleep_for(std::chrono::milliseconds(50)); - sendPortStateChange(SAI_PORT_OPER_STATUS_UP); - std::this_thread::sleep_for(std::chrono::milliseconds(50)); - sendPortStateChange(SAI_PORT_OPER_STATUS_DOWN); - std::this_thread::sleep_for(std::chrono::milliseconds(50)); - sendPortStateChange(SAI_PORT_OPER_STATUS_UP); - std::this_thread::sleep_for(std::chrono::milliseconds(50)); // Verify damping is active (UP was suppressed) EXPECT_EQ(getDampingField("is_damping_active"), "true"); From 162a34135046976bb540900864410c0dd9c15db3 Mon Sep 17 00:00:00 2001 From: Sivakumar Thirukkanna Thevar Date: Tue, 23 Jun 2026 16:20:01 +0530 Subject: [PATCH 4/7] Added missing SWSS_LOG_ENTER Signed-off-by: Sivakumar Thirukkanna Thevar --- syncd/Syncd.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/syncd/Syncd.cpp b/syncd/Syncd.cpp index ff79796a95..fc41868dc6 100644 --- a/syncd/Syncd.cpp +++ b/syncd/Syncd.cpp @@ -60,6 +60,8 @@ using namespace std::placeholders; std::string serializePortVids(const std::vector& notifications) { + SWSS_LOG_ENTER(); + std::string portVids; portVids.reserve(notifications.size() * 24); From 63a8ed2b1941d7ac998d32873e4ecfe3a1cf7737 Mon Sep 17 00:00:00 2001 From: Sivakumar Thirukkanna Thevar Date: Mon, 29 Jun 2026 12:07:55 +0530 Subject: [PATCH 5/7] Fixed a review comment Signed-off-by: Sivakumar Thirukkanna Thevar --- syncd/Syncd.cpp | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/syncd/Syncd.cpp b/syncd/Syncd.cpp index fc41868dc6..5ec6fcf6bf 100644 --- a/syncd/Syncd.cpp +++ b/syncd/Syncd.cpp @@ -1538,15 +1538,11 @@ void Syncd::processPendingDampingSync() if (state.pending_state_sync && state.advertised_status != state.physical_status) { - state.pending_state_sync = false; - state.advertised_status = state.physical_status; - sai_port_oper_status_notification_t n; n.port_id = port; n.port_state = state.physical_status; notifications.push_back(n); - writeDampingCountersToStateDb(port, state); } } } @@ -1610,6 +1606,19 @@ void Syncd::flushPendingDampingNotifications() SAI_SWITCH_NOTIFICATION_NAME_PORT_STATE_CHANGE, s, entry); + { + std::lock_guard lock(m_linkEventDampingMutex); + for (const auto &ntf : notifications) + { + auto it = m_portLinkEventDampingStates.find(ntf.port_id); + if (it != m_portLinkEventDampingStates.end()) + { + it->second.pending_state_sync = false; + it->second.advertised_status = ntf.port_state; + writeDampingCountersToStateDb(ntf.port_id, it->second); + } + } + } } catch (const std::exception &e) From 527f8ef2483009e2a8ba0d5dd826ed6f46d8cc38 Mon Sep 17 00:00:00 2001 From: Sivakumar Thirukkanna Thevar Date: Mon, 29 Jun 2026 14:12:02 +0530 Subject: [PATCH 6/7] Fixing the test failure Signed-off-by: Sivakumar Thirukkanna Thevar --- unittest/syncd/TestSyncd.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/unittest/syncd/TestSyncd.cpp b/unittest/syncd/TestSyncd.cpp index 5c333f91f6..3389f314c9 100644 --- a/unittest/syncd/TestSyncd.cpp +++ b/unittest/syncd/TestSyncd.cpp @@ -1052,6 +1052,9 @@ TEST_F(SyncdLinkEventDampingTest, processPendingDampingSyncWithNotifications) } } + // flush the notifications + m_syncd->flushPendingDampingNotifications(); + // Verify pending_state_sync was cleared and advertised was updated { std::lock_guard lock(m_syncd->m_linkEventDampingMutex); From 6fcb8c8ae9c2ee2a23adcc382a7cfbacda2299aa Mon Sep 17 00:00:00 2001 From: Sivakumar Thirukkanna Thevar Date: Mon, 29 Jun 2026 16:25:44 +0530 Subject: [PATCH 7/7] Removing -fno-access-control and adding friend class Signed-off-by: Sivakumar Thirukkanna Thevar --- syncd/Syncd.h | 6 ++ unittest/syncd/Makefile.am | 2 +- unittest/syncd/TestSyncd.cpp | 138 +++++++++++++++++++++++------------ 3 files changed, 97 insertions(+), 49 deletions(-) diff --git a/syncd/Syncd.h b/syncd/Syncd.h index 333fa50521..bf1fd88f83 100644 --- a/syncd/Syncd.h +++ b/syncd/Syncd.h @@ -32,10 +32,16 @@ #include #include +class SyncdTest; +class SyncdLinkEventDampingTest; + namespace syncd { class Syncd { + friend class ::SyncdTest; + friend class ::SyncdLinkEventDampingTest; + private: Syncd(const Syncd&) = delete; diff --git a/unittest/syncd/Makefile.am b/unittest/syncd/Makefile.am index 49cec2fc36..76c29e2466 100644 --- a/unittest/syncd/Makefile.am +++ b/unittest/syncd/Makefile.am @@ -28,7 +28,7 @@ tests_SOURCES = main.cpp \ TestVendorSai.cpp \ TestFlowDump.cpp -tests_CXXFLAGS = $(DBGFLAGS) $(AM_CXXFLAGS) $(CXXFLAGS_COMMON) -fno-access-control +tests_CXXFLAGS = $(DBGFLAGS) $(AM_CXXFLAGS) $(CXXFLAGS_COMMON) tests_LDFLAGS = -Wl,-rpath,$(top_srcdir)/lib/.libs -Wl,-rpath,$(top_srcdir)/meta/.libs tests_LDADD = $(LDADD_GTEST) $(top_srcdir)/syncd/libSyncdRequestShutdown.a $(top_srcdir)/syncd/libSyncd.a $(top_srcdir)/vslib/libSaiVS.a $(top_srcdir)/syncd/libMdioIpcClient.a \ -lhiredis -lswsscommon -lnl-genl-3 -lnl-nf-3 -lnl-route-3 -lnl-3 -lpthread -L$(top_srcdir)/lib/.libs -lsairedis -L$(top_srcdir)/meta/.libs -lsaimetadata -lsaimeta -lzmq -lz $(CODE_COVERAGE_LIBS) $(VPP_LIBS) diff --git a/unittest/syncd/TestSyncd.cpp b/unittest/syncd/TestSyncd.cpp index 3389f314c9..ed5465a7a0 100644 --- a/unittest/syncd/TestSyncd.cpp +++ b/unittest/syncd/TestSyncd.cpp @@ -877,6 +877,48 @@ class SyncdLinkEventDampingTest : public SyncdTest return found ? value : ""; } + + std::mutex& dampingStateMutex() + { + SWSS_LOG_ENTER(); + + return m_syncd->m_linkEventDampingMutex; + } + + std::map& portDampingStates() + { + SWSS_LOG_ENTER(); + + return m_syncd->m_portLinkEventDampingStates; + } + + std::mutex& pendingNotificationsMutex() + { + SWSS_LOG_ENTER(); + + return m_syncd->m_pendingNotificationsMutex; + } + + std::queue>& pendingNotifications() + { + SWSS_LOG_ENTER(); + + return m_syncd->m_pendingNotifications; + } + + void invokeProcessPendingDampingSync() + { + SWSS_LOG_ENTER(); + + m_syncd->processPendingDampingSync(); + } + + void invokeFlushPendingDampingNotifications() + { + SWSS_LOG_ENTER(); + + m_syncd->flushPendingDampingNotifications(); + } }; // Define static constant expression members for linkage @@ -1018,10 +1060,10 @@ TEST_F(SyncdLinkEventDampingTest, processPendingDampingSyncWithNotifications) // Set up preconditions { - std::lock_guard lock(m_syncd->m_linkEventDampingMutex); + std::lock_guard lock(dampingStateMutex()); - auto it = m_syncd->m_portLinkEventDampingStates.find(PORT_VID); - ASSERT_NE(it, m_syncd->m_portLinkEventDampingStates.end()) + auto it = portDampingStates().find(PORT_VID); + ASSERT_NE(it, portDampingStates().end()) << "Port damping state not found"; auto& state = it->second; @@ -1036,29 +1078,29 @@ TEST_F(SyncdLinkEventDampingTest, processPendingDampingSyncWithNotifications) } // Call the function - m_syncd->processPendingDampingSync(); + invokeProcessPendingDampingSync(); // Verify the notification was enqueued { - std::lock_guard lock(m_syncd->m_pendingNotificationsMutex); - EXPECT_EQ(m_syncd->m_pendingNotifications.size(), 1) + std::lock_guard lock(pendingNotificationsMutex()); + EXPECT_EQ(pendingNotifications().size(), 1) << "Expected 1 notification batch to be enqueued"; - if (!m_syncd->m_pendingNotifications.empty()) + if (!pendingNotifications().empty()) { - auto& batch = m_syncd->m_pendingNotifications.front(); + auto& batch = pendingNotifications().front(); EXPECT_EQ(batch.size(), 1) << "Expected 1 notification in batch"; EXPECT_EQ(batch[0].port_id, PORT_VID) << "Notification for wrong port"; } } // flush the notifications - m_syncd->flushPendingDampingNotifications(); + invokeFlushPendingDampingNotifications(); // Verify pending_state_sync was cleared and advertised was updated { - std::lock_guard lock(m_syncd->m_linkEventDampingMutex); - auto& state = m_syncd->m_portLinkEventDampingStates[PORT_VID]; + std::lock_guard lock(dampingStateMutex()); + auto& state = portDampingStates()[PORT_VID]; EXPECT_FALSE(state.pending_state_sync) << "pending_state_sync should be cleared"; EXPECT_EQ(state.advertised_status, state.physical_status) << "Advertised should match physical after sync"; @@ -1078,7 +1120,7 @@ TEST_F(SyncdLinkEventDampingTest, processPendingDampingSyncQueueOverflow) // Directly populate the queue to 1000 entries { - std::lock_guard lock(m_syncd->m_pendingNotificationsMutex); + std::lock_guard lock(pendingNotificationsMutex()); sai_port_oper_status_notification_t dummy_ntf; dummy_ntf.port_id = PORT_VID; @@ -1089,17 +1131,17 @@ TEST_F(SyncdLinkEventDampingTest, processPendingDampingSyncQueueOverflow) // Fill queue to exactly 1000 entries for (int i = 0; i < 1000; ++i) { - m_syncd->m_pendingNotifications.push(batch); + pendingNotifications().push(batch); } - EXPECT_EQ(m_syncd->m_pendingNotifications.size(), 1000); + EXPECT_EQ(pendingNotifications().size(), 1000); } // Set up the scenario to trigger one more notification { - std::lock_guard lock(m_syncd->m_linkEventDampingMutex); + std::lock_guard lock(dampingStateMutex()); - auto& state = m_syncd->m_portLinkEventDampingStates.at(PORT_VID); + auto& state = portDampingStates().at(PORT_VID); // Set up the condition for processPendingDampingSync to queue a notification state.pending_state_sync = true; @@ -1108,14 +1150,14 @@ TEST_F(SyncdLinkEventDampingTest, processPendingDampingSyncQueueOverflow) } // This should trigger overflow protection - m_syncd->processPendingDampingSync(); + invokeProcessPendingDampingSync(); // Verify the overflow protection worked { - std::lock_guard lock(m_syncd->m_pendingNotificationsMutex); + std::lock_guard lock(pendingNotificationsMutex()); // Queue should still be 1000 (dropped oldest, added newest) - EXPECT_EQ(m_syncd->m_pendingNotifications.size(), 1000) + EXPECT_EQ(pendingNotifications().size(), 1000) << "Queue should be capped at 1000 after overflow"; } } @@ -1124,7 +1166,7 @@ TEST_F(SyncdLinkEventDampingTest, flushPendingNotificationsWithBatches) { // Directly populate the queue with multiple batches { - std::lock_guard lock(m_syncd->m_pendingNotificationsMutex); + std::lock_guard lock(pendingNotificationsMutex()); // Create 3 different notification batches for (int batch_num = 0; batch_num < 3; ++batch_num) @@ -1136,18 +1178,18 @@ TEST_F(SyncdLinkEventDampingTest, flushPendingNotificationsWithBatches) : SAI_PORT_OPER_STATUS_UP; std::vector batch = {ntf}; - m_syncd->m_pendingNotifications.push(batch); + pendingNotifications().push(batch); } - EXPECT_EQ(m_syncd->m_pendingNotifications.size(), 3); + EXPECT_EQ(pendingNotifications().size(), 3); } - m_syncd->flushPendingDampingNotifications(); + invokeFlushPendingDampingNotifications(); // Verify all batches were flushed { - std::lock_guard lock(m_syncd->m_pendingNotificationsMutex); - EXPECT_EQ(m_syncd->m_pendingNotifications.size(), 0) + std::lock_guard lock(pendingNotificationsMutex()); + EXPECT_EQ(pendingNotifications().size(), 0) << "All batches should be flushed"; } } @@ -1156,7 +1198,7 @@ TEST_F(SyncdLinkEventDampingTest, flushPendingNotificationsDrainsQueue) { // Queue 2 notification batches { - std::lock_guard lock(m_syncd->m_pendingNotificationsMutex); + std::lock_guard lock(pendingNotificationsMutex()); for (int i = 0; i < 2; ++i) { @@ -1165,17 +1207,17 @@ TEST_F(SyncdLinkEventDampingTest, flushPendingNotificationsDrainsQueue) ntf.port_state = SAI_PORT_OPER_STATUS_UP; std::vector batch = {ntf}; - m_syncd->m_pendingNotifications.push(batch); + pendingNotifications().push(batch); } } // Tests normal path - m_syncd->flushPendingDampingNotifications(); + invokeFlushPendingDampingNotifications(); // Verify all batches were processed { - std::lock_guard lock(m_syncd->m_pendingNotificationsMutex); - EXPECT_EQ(m_syncd->m_pendingNotifications.size(), 0); + std::lock_guard lock(pendingNotificationsMutex()); + EXPECT_EQ(pendingNotifications().size(), 0); } } @@ -1192,30 +1234,30 @@ TEST_F(SyncdLinkEventDampingTest, fullNotificationFlowIntegrated) // Set up with pending notification { - std::lock_guard lock(m_syncd->m_linkEventDampingMutex); + std::lock_guard lock(dampingStateMutex()); - auto& state = m_syncd->m_portLinkEventDampingStates.at(PORT_VID); + auto& state = portDampingStates().at(PORT_VID); state.pending_state_sync = true; state.advertised_status = SAI_PORT_OPER_STATUS_DOWN; state.physical_status = SAI_PORT_OPER_STATUS_UP; } // Step 1: processPendingDampingSync queues the notification - m_syncd->processPendingDampingSync(); + invokeProcessPendingDampingSync(); // Verify notification was queued { - std::lock_guard lock(m_syncd->m_pendingNotificationsMutex); - EXPECT_EQ(m_syncd->m_pendingNotifications.size(), 1); + std::lock_guard lock(pendingNotificationsMutex()); + EXPECT_EQ(pendingNotifications().size(), 1); } // Step 2: flushPendingDampingNotifications sends it - m_syncd->flushPendingDampingNotifications(); + invokeFlushPendingDampingNotifications(); // Verify queue was flushed { - std::lock_guard lock(m_syncd->m_pendingNotificationsMutex); - EXPECT_EQ(m_syncd->m_pendingNotifications.size(), 0); + std::lock_guard lock(pendingNotificationsMutex()); + EXPECT_EQ(pendingNotifications().size(), 0); } } @@ -1223,17 +1265,17 @@ TEST_F(SyncdLinkEventDampingTest, flushPendingNotificationsEmptyQueue) { // Ensure queue is empty { - std::lock_guard lock(m_syncd->m_pendingNotificationsMutex); - EXPECT_EQ(m_syncd->m_pendingNotifications.size(), 0); + std::lock_guard lock(pendingNotificationsMutex()); + EXPECT_EQ(pendingNotifications().size(), 0); } // Call flush with empty queue - m_syncd->flushPendingDampingNotifications(); + invokeFlushPendingDampingNotifications(); // Queue should still be empty { - std::lock_guard lock(m_syncd->m_pendingNotificationsMutex); - EXPECT_EQ(m_syncd->m_pendingNotifications.size(), 0); + std::lock_guard lock(pendingNotificationsMutex()); + EXPECT_EQ(pendingNotifications().size(), 0); } } @@ -1250,21 +1292,21 @@ TEST_F(SyncdLinkEventDampingTest, processPendingDampingSyncNoPending) // Set up state WITHOUT pending_state_sync { - std::lock_guard lock(m_syncd->m_linkEventDampingMutex); + std::lock_guard lock(dampingStateMutex()); - auto& state = m_syncd->m_portLinkEventDampingStates.at(PORT_VID); + auto& state = portDampingStates().at(PORT_VID); state.pending_state_sync = false; // No pending sync state.advertised_status = SAI_PORT_OPER_STATUS_DOWN; state.physical_status = SAI_PORT_OPER_STATUS_UP; } // Call the function - should NOT queue any notifications - m_syncd->processPendingDampingSync(); + invokeProcessPendingDampingSync(); // Verify NO notification was queued { - std::lock_guard lock(m_syncd->m_pendingNotificationsMutex); - EXPECT_EQ(m_syncd->m_pendingNotifications.size(), 0) + std::lock_guard lock(pendingNotificationsMutex()); + EXPECT_EQ(pendingNotifications().size(), 0) << "No notification should be queued when pending_state_sync=false"; } }