From bec24906343547bd2802c76bd9a7b545b0f898a5 Mon Sep 17 00:00:00 2001 From: DendroLabs Date: Mon, 8 Jun 2026 12:03:25 -0400 Subject: [PATCH 1/2] [Link Event Damping] Add RedisInterface plumbing for damping config Supersedes #1331 by @Ashish1805. Adds the Redis communication layer to carry link event damping configuration from orchagent to syncd. Addresses @kcudnik review feedback from #1331: - setLinkEventDampingConfig always synchronous (never checks m_syncMode) - sendLinkEventDampingConfigResponse always sends (no m_enableSyncMode guard) - isRedisPortAttribute simplified to single boolean expression lib/ changes: - sairediscommon.h: DAMPING_CONFIG_SET / DAMPING_CONFIG_RESPONSE commands - RedisRemoteSaiInterface: isRedisPortAttribute(), setRedisPortExtensionAttribute(), setLinkEventDampingConfig() with unconditional wait(), waitForLinkEventDampingConfigResponse() - ClientSai::set(): block port extension attrs in CLIENT mode - Sai::set(): route port extension attrs directly, bypass metadata validation syncd/ changes: - Syncd: dispatch DAMPING_CONFIG_SET in processSingleEvent() - processLinkEventDampingConfigSet(): stub returning NOT_IMPLEMENTED - sendLinkEventDampingConfigResponse(): always sends response syncd handler is a stub -- actual algorithm in #1334 supersede. Signed-off-by: DendroLabs --- lib/ClientSai.cpp | 7 + lib/RedisRemoteSaiInterface.cpp | 111 +++++++++++++ lib/RedisRemoteSaiInterface.h | 15 ++ lib/Sai.cpp | 7 + lib/sairediscommon.h | 3 + syncd/Syncd.cpp | 27 ++++ syncd/Syncd.h | 6 + syncd/tests/Makefile.am | 2 +- syncd/tests/TestSyncdLinkEventDamping.cpp | 187 ++++++++++++++++++++++ unittest/lib/TestClientServerSai.cpp | 39 +++++ 10 files changed, 403 insertions(+), 1 deletion(-) create mode 100644 syncd/tests/TestSyncdLinkEventDamping.cpp diff --git a/lib/ClientSai.cpp b/lib/ClientSai.cpp index 63bc2392e2..3190238043 100644 --- a/lib/ClientSai.cpp +++ b/lib/ClientSai.cpp @@ -247,6 +247,13 @@ sai_status_t ClientSai::set( return SAI_STATUS_FAILURE; } + if (RedisRemoteSaiInterface::isRedisPortAttribute(objectType, attr)) + { + SWSS_LOG_ERROR("redis port extension attributes are not supported in CLIENT mode"); + + return SAI_STATUS_FAILURE; + } + auto status = set( objectType, sai_serialize_object_id(objectId), diff --git a/lib/RedisRemoteSaiInterface.cpp b/lib/RedisRemoteSaiInterface.cpp index dabd7ad0b0..daf00f852b 100644 --- a/lib/RedisRemoteSaiInterface.cpp +++ b/lib/RedisRemoteSaiInterface.cpp @@ -666,6 +666,11 @@ sai_status_t RedisRemoteSaiInterface::set( return setRedisExtensionAttribute(objectType, objectId, attr); } + if (RedisRemoteSaiInterface::isRedisPortAttribute(objectType, attr)) + { + return setRedisPortExtensionAttribute(objectType, objectId, attr); + } + auto status = set( objectType, sai_serialize_object_id(objectId), @@ -2201,6 +2206,112 @@ bool RedisRemoteSaiInterface::isRedisAttribute( return true; } +bool RedisRemoteSaiInterface::isRedisPortAttribute( + _In_ sai_object_type_t objectType, + _In_ const sai_attribute_t* attr) +{ + SWSS_LOG_ENTER(); + + return objectType == SAI_OBJECT_TYPE_PORT + && attr != nullptr + && attr->id >= SAI_PORT_ATTR_CUSTOM_RANGE_START; +} + +sai_status_t RedisRemoteSaiInterface::setRedisPortExtensionAttribute( + _In_ sai_object_type_t objectType, + _In_ sai_object_id_t objectId, + _In_ const sai_attribute_t *attr) +{ + SWSS_LOG_ENTER(); + + if (attr == nullptr) + { + SWSS_LOG_ERROR("attr pointer is null"); + + return SAI_STATUS_FAILURE; + } + + switch ((sai_redis_port_attr_t)attr->id) + { + case SAI_REDIS_PORT_ATTR_LINK_EVENT_DAMPING_ALGORITHM: + case SAI_REDIS_PORT_ATTR_LINK_EVENT_DAMPING_ALGO_AIED_CONFIG: + return setLinkEventDampingConfig(objectId, attr); + + default: + break; + } + + SWSS_LOG_ERROR("unknown redis port extension attribute: %d", attr->id); + + return SAI_STATUS_INVALID_PARAMETER; +} + +sai_status_t RedisRemoteSaiInterface::setLinkEventDampingConfig( + _In_ sai_object_id_t objectId, + _In_ const sai_attribute_t *attr) +{ + SWSS_LOG_ENTER(); + + std::vector entries; + + std::string strAttrId = sai_serialize_redis_port_attr_id( + static_cast(attr->id)); + + switch ((sai_redis_port_attr_t)attr->id) + { + case SAI_REDIS_PORT_ATTR_LINK_EVENT_DAMPING_ALGORITHM: + { + std::string strAttrValue = sai_serialize_redis_link_event_damping_algorithm( + static_cast(attr->value.s32)); + + entries.emplace_back(strAttrId, strAttrValue); + break; + } + + case SAI_REDIS_PORT_ATTR_LINK_EVENT_DAMPING_ALGO_AIED_CONFIG: + { + auto *config = static_cast(attr->value.ptr); + + if (config == nullptr) + { + SWSS_LOG_ERROR("link event damping AIED config pointer is null"); + + return SAI_STATUS_INVALID_PARAMETER; + } + + std::string strAttrValue = sai_serialize_redis_link_event_damping_aied_config(*config); + + entries.emplace_back(strAttrId, strAttrValue); + break; + } + + default: + + SWSS_LOG_ERROR("unknown damping config attribute: %d", attr->id); + + return SAI_STATUS_INVALID_PARAMETER; + } + + std::string key = sai_serialize_object_type(SAI_OBJECT_TYPE_PORT) + ":" + sai_serialize_object_id(objectId); + + SWSS_LOG_DEBUG("damping config set key: %s", key.c_str()); + + m_communicationChannel->set(key, entries, REDIS_ASIC_STATE_COMMAND_DAMPING_CONFIG_SET); + + return waitForLinkEventDampingConfigResponse(); +} + +sai_status_t RedisRemoteSaiInterface::waitForLinkEventDampingConfigResponse() +{ + SWSS_LOG_ENTER(); + + swss::KeyOpFieldsValuesTuple kco; + + auto status = m_communicationChannel->wait(REDIS_ASIC_STATE_COMMAND_DAMPING_CONFIG_RESPONSE, kco); + + return status; +} + void RedisRemoteSaiInterface::handleNotification( _In_ const std::string &name, _In_ const std::string &serializedNotification, diff --git a/lib/RedisRemoteSaiInterface.h b/lib/RedisRemoteSaiInterface.h index 74019eccf8..4dd66bfcec 100644 --- a/lib/RedisRemoteSaiInterface.h +++ b/lib/RedisRemoteSaiInterface.h @@ -227,6 +227,10 @@ namespace sairedis _In_ sai_object_id_t obejctType, _In_ const sai_attribute_t* attr); + static bool isRedisPortAttribute( + _In_ sai_object_type_t objectType, + _In_ const sai_attribute_t* attr); + sai_status_t setRedisAttribute( _In_ sai_object_id_t switchId, _In_ const sai_attribute_t* attr); @@ -401,6 +405,17 @@ namespace sairedis _In_ sai_object_id_t objectId, _In_ const sai_attribute_t *attr); + sai_status_t setRedisPortExtensionAttribute( + _In_ sai_object_type_t objectType, + _In_ sai_object_id_t objectId, + _In_ const sai_attribute_t *attr); + + sai_status_t setLinkEventDampingConfig( + _In_ sai_object_id_t objectId, + _In_ const sai_attribute_t *attr); + + sai_status_t waitForLinkEventDampingConfigResponse(); + bool isSaiS8ListValidString( _In_ const sai_s8_list_t &s8list); diff --git a/lib/Sai.cpp b/lib/Sai.cpp index 22b33e4764..8a4639bd92 100644 --- a/lib/Sai.cpp +++ b/lib/Sai.cpp @@ -249,6 +249,13 @@ sai_status_t Sai::set( return success ? SAI_STATUS_SUCCESS : SAI_STATUS_FAILURE; } + if (RedisRemoteSaiInterface::isRedisPortAttribute(objectType, attr)) + { + REDIS_CHECK_CONTEXT(objectId); + + return context->m_redisSai->set(objectType, objectId, attr); + } + REDIS_CHECK_CONTEXT(objectId); return context->m_meta->set(objectType, objectId, attr); diff --git a/lib/sairediscommon.h b/lib/sairediscommon.h index 4594cab5b0..66685edfaa 100644 --- a/lib/sairediscommon.h +++ b/lib/sairediscommon.h @@ -64,6 +64,9 @@ #define REDIS_ASIC_STATE_COMMAND_STATS_ST_CAPABILITY_QUERY "stats_st_capability_query" #define REDIS_ASIC_STATE_COMMAND_STATS_ST_CAPABILITY_RESPONSE "stats_st_capability_response" +#define REDIS_ASIC_STATE_COMMAND_DAMPING_CONFIG_SET "link_event_damping_config_set" +#define REDIS_ASIC_STATE_COMMAND_DAMPING_CONFIG_RESPONSE "link_event_damping_config_response" + /** * @brief Redis virtual object id counter key name. * diff --git a/syncd/Syncd.cpp b/syncd/Syncd.cpp index d2bc0ba056..413ccab223 100644 --- a/syncd/Syncd.cpp +++ b/syncd/Syncd.cpp @@ -473,6 +473,9 @@ sai_status_t Syncd::processSingleEvent( if (op == REDIS_ASIC_STATE_COMMAND_OBJECT_TYPE_GET_AVAILABILITY_QUERY) return processObjectTypeGetAvailabilityQuery(kco); + if (op == REDIS_ASIC_STATE_COMMAND_DAMPING_CONFIG_SET) + return processLinkEventDampingConfigSet(kco); + if (op == REDIS_FLEX_COUNTER_COMMAND_START_POLL) return processFlexCounterEvent(key, SET_COMMAND, kfvFieldsValues(kco)); @@ -842,6 +845,30 @@ sai_status_t Syncd::processStatsStCapabilityQuery( return status; } +sai_status_t Syncd::processLinkEventDampingConfigSet( + _In_ const swss::KeyOpFieldsValuesTuple &kco) +{ + SWSS_LOG_ENTER(); + + SWSS_LOG_NOTICE("processLinkEventDampingConfigSet: stub, returning NOT_IMPLEMENTED"); + + sendLinkEventDampingConfigResponse(SAI_STATUS_NOT_IMPLEMENTED); + + return SAI_STATUS_NOT_IMPLEMENTED; +} + +void Syncd::sendLinkEventDampingConfigResponse( + _In_ sai_status_t status) +{ + SWSS_LOG_ENTER(); + + std::string strStatus = sai_serialize_status(status); + + SWSS_LOG_INFO("sending link event damping config response: %s", strStatus.c_str()); + + m_selectableChannel->set(strStatus, {}, REDIS_ASIC_STATE_COMMAND_DAMPING_CONFIG_RESPONSE); +} + sai_status_t Syncd::processFdbFlush( _In_ const swss::KeyOpFieldsValuesTuple &kco) { diff --git a/syncd/Syncd.h b/syncd/Syncd.h index d633c9196d..bb68e71887 100644 --- a/syncd/Syncd.h +++ b/syncd/Syncd.h @@ -145,6 +145,9 @@ namespace syncd sai_status_t processStatsStCapabilityQuery( _In_ const swss::KeyOpFieldsValuesTuple &kco); + sai_status_t processLinkEventDampingConfigSet( + _In_ const swss::KeyOpFieldsValuesTuple &kco); + sai_status_t processFdbFlush( _In_ const swss::KeyOpFieldsValuesTuple &kco); @@ -395,6 +398,9 @@ namespace syncd void sendNotifyResponse( _In_ sai_status_t status); + void sendLinkEventDampingConfigResponse( + _In_ sai_status_t status); + private: // snoop get response oids void snoopGetResponse( diff --git a/syncd/tests/Makefile.am b/syncd/tests/Makefile.am index 2630eecdc9..9d3f8b940d 100644 --- a/syncd/tests/Makefile.am +++ b/syncd/tests/Makefile.am @@ -5,7 +5,7 @@ LDADD_GTEST = -L/usr/src/gtest -lgtest -lgtest_main bin_PROGRAMS = tests tests_SOURCES = \ - main.cpp TestSyncdBrcm.cpp TestSyncdMlnx.cpp TestSyncdNvdaBf.cpp TestSyncdLib.cpp TestDisabledRedisClient.cpp + main.cpp TestSyncdBrcm.cpp TestSyncdMlnx.cpp TestSyncdNvdaBf.cpp TestSyncdLib.cpp TestDisabledRedisClient.cpp TestSyncdLinkEventDamping.cpp tests_CXXFLAGS = \ $(DBGFLAGS) $(AM_CXXFLAGS) $(CXXFLAGS_COMMON) tests_LDADD = \ diff --git a/syncd/tests/TestSyncdLinkEventDamping.cpp b/syncd/tests/TestSyncdLinkEventDamping.cpp new file mode 100644 index 0000000000..bfe91556c4 --- /dev/null +++ b/syncd/tests/TestSyncdLinkEventDamping.cpp @@ -0,0 +1,187 @@ +#include +#include +#include +#include + +#include + +#include + +#include "Sai.h" +#include "Syncd.h" +#include "MetadataLogger.h" + +#include "TestSyncdLib.h" + +using namespace syncd; + +static const char* profile_get_value( + _In_ sai_switch_profile_id_t profile_id, + _In_ const char* variable) +{ + SWSS_LOG_ENTER(); + + return NULL; +} + +static int profile_get_next_value( + _In_ sai_switch_profile_id_t profile_id, + _Out_ const char** variable, + _Out_ const char** value) +{ + SWSS_LOG_ENTER(); + + if (value == NULL) + { + SWSS_LOG_INFO("resetting profile map iterator"); + return 0; + } + + if (variable == NULL) + { + SWSS_LOG_WARN("variable is null"); + return -1; + } + + SWSS_LOG_INFO("iterator reached end"); + return -1; +} + +static sai_service_method_table_t test_services = { + profile_get_value, + profile_get_next_value +}; + +void syncdLinkEventDampingWorkerThread() +{ + SWSS_LOG_ENTER(); + + swss::Logger::getInstance().setMinPrio(swss::Logger::SWSS_NOTICE); + MetadataLogger::initialize(); + + auto vendorSai = std::make_shared(); + auto commandLineOptions = std::make_shared(); + auto isWarmStart = false; + + commandLineOptions->m_enableSyncMode = true; + commandLineOptions->m_enableTempView = true; + commandLineOptions->m_disableExitSleep = true; + commandLineOptions->m_enableUnittests = false; + commandLineOptions->m_enableSaiBulkSupport = true; + commandLineOptions->m_startType = SAI_START_TYPE_COLD_BOOT; + commandLineOptions->m_redisCommunicationMode = SAI_REDIS_COMMUNICATION_MODE_REDIS_SYNC; + commandLineOptions->m_profileMapFile = "./brcm/testprofile.ini"; + + auto syncd = std::make_shared(vendorSai, commandLineOptions, isWarmStart); + syncd->run(); + + SWSS_LOG_NOTICE("Started syncd link event damping worker"); +} + +class SyncdLinkEventDampingTest : public ::testing::Test +{ +public: + SyncdLinkEventDampingTest() = default; + virtual ~SyncdLinkEventDampingTest() = default; + +public: + virtual void SetUp() override + { + SWSS_LOG_ENTER(); + + flushAsicDb(); + + m_worker = std::make_shared(syncdLinkEventDampingWorkerThread); + + m_sairedis = std::make_shared(); + + auto status = m_sairedis->apiInitialize(0, &test_services); + ASSERT_EQ(status, SAI_STATUS_SUCCESS); + + sai_attribute_t attr; + + attr.id = SAI_REDIS_SWITCH_ATTR_REDIS_COMMUNICATION_MODE; + attr.value.s32 = SAI_REDIS_COMMUNICATION_MODE_REDIS_SYNC; + + status = m_sairedis->set(SAI_OBJECT_TYPE_SWITCH, SAI_NULL_OBJECT_ID, &attr); + ASSERT_EQ(status, SAI_STATUS_SUCCESS); + + attr.id = SAI_REDIS_SWITCH_ATTR_RECORD; + attr.value.booldata = true; + + status = m_sairedis->set(SAI_OBJECT_TYPE_SWITCH, SAI_NULL_OBJECT_ID, &attr); + ASSERT_EQ(status, SAI_STATUS_SUCCESS); + } + + virtual void TearDown() override + { + SWSS_LOG_ENTER(); + + auto status = m_sairedis->apiUninitialize(); + ASSERT_EQ(status, SAI_STATUS_SUCCESS); + + sendSyncdShutdownNotification(); + m_worker->join(); + } + +protected: + std::shared_ptr m_worker; + std::shared_ptr m_sairedis; +}; + +TEST_F(SyncdLinkEventDampingTest, SetLinkEventDampingAlgorithm) +{ + sai_attribute_t attr; + + attr.id = SAI_REDIS_SWITCH_ATTR_NOTIFY_SYNCD; + attr.value.s32 = SAI_REDIS_NOTIFY_SYNCD_INIT_VIEW; + + auto status = m_sairedis->set(SAI_OBJECT_TYPE_SWITCH, SAI_NULL_OBJECT_ID, &attr); + ASSERT_EQ(status, SAI_STATUS_SUCCESS); + + sai_object_id_t switchId; + + attr.id = SAI_SWITCH_ATTR_INIT_SWITCH; + attr.value.booldata = true; + + status = m_sairedis->create(SAI_OBJECT_TYPE_SWITCH, &switchId, SAI_NULL_OBJECT_ID, 1, &attr); + ASSERT_EQ(status, SAI_STATUS_SUCCESS); + + attr.id = SAI_REDIS_PORT_ATTR_LINK_EVENT_DAMPING_ALGORITHM; + attr.value.s32 = SAI_REDIS_LINK_EVENT_DAMPING_ALGORITHM_AIED; + + status = m_sairedis->set(SAI_OBJECT_TYPE_PORT, SAI_NULL_OBJECT_ID, &attr); + EXPECT_EQ(status, SAI_STATUS_NOT_IMPLEMENTED); +} + +TEST_F(SyncdLinkEventDampingTest, SetLinkEventDampingAiedConfig) +{ + sai_attribute_t attr; + + attr.id = SAI_REDIS_SWITCH_ATTR_NOTIFY_SYNCD; + attr.value.s32 = SAI_REDIS_NOTIFY_SYNCD_INIT_VIEW; + + auto status = m_sairedis->set(SAI_OBJECT_TYPE_SWITCH, SAI_NULL_OBJECT_ID, &attr); + ASSERT_EQ(status, SAI_STATUS_SUCCESS); + + sai_object_id_t switchId; + + attr.id = SAI_SWITCH_ATTR_INIT_SWITCH; + attr.value.booldata = true; + + status = m_sairedis->create(SAI_OBJECT_TYPE_SWITCH, &switchId, SAI_NULL_OBJECT_ID, 1, &attr); + ASSERT_EQ(status, SAI_STATUS_SUCCESS); + + sai_redis_link_event_damping_algo_aied_config_t config = {}; + config.max_suppress_time = 30000; + config.suppress_threshold = 3000; + config.reuse_threshold = 750; + config.decay_half_life = 5000; + config.flap_penalty = 1000; + + attr.id = SAI_REDIS_PORT_ATTR_LINK_EVENT_DAMPING_ALGO_AIED_CONFIG; + attr.value.ptr = &config; + + status = m_sairedis->set(SAI_OBJECT_TYPE_PORT, SAI_NULL_OBJECT_ID, &attr); + EXPECT_EQ(status, SAI_STATUS_NOT_IMPLEMENTED); +} diff --git a/unittest/lib/TestClientServerSai.cpp b/unittest/lib/TestClientServerSai.cpp index dc82ad152e..25d78ab134 100644 --- a/unittest/lib/TestClientServerSai.cpp +++ b/unittest/lib/TestClientServerSai.cpp @@ -264,6 +264,45 @@ TEST(ClientServerSai, bulk_ ## OT) SAIREDIS_DECLARE_EVERY_BULK_ENTRY(TEST_BULK_ENTRY) +TEST(ClientServerSai, VerifySaiRedisPortAttrNotSupportedInClientMode) +{ + auto css = std::make_shared(); + + EXPECT_EQ(SAI_STATUS_SUCCESS, css->apiInitialize(0, &test_client_services)); + + sai_attribute_t attr; + attr.id = SAI_REDIS_PORT_ATTR_LINK_EVENT_DAMPING_ALGORITHM; + attr.value.s32 = SAI_REDIS_LINK_EVENT_DAMPING_ALGORITHM_AIED; + + EXPECT_EQ(SAI_STATUS_FAILURE, css->set(SAI_OBJECT_TYPE_PORT, SAI_NULL_OBJECT_ID, &attr)); +} + +TEST(ClientServerSai, SetLinkEventDampingConfigNullPtr) +{ + auto css = std::make_shared(); + + EXPECT_EQ(SAI_STATUS_SUCCESS, css->apiInitialize(0, &test_services)); + + sai_attribute_t attr; + attr.id = SAI_REDIS_PORT_ATTR_LINK_EVENT_DAMPING_ALGO_AIED_CONFIG; + attr.value.ptr = nullptr; + + EXPECT_EQ(SAI_STATUS_INVALID_PARAMETER, css->set(SAI_OBJECT_TYPE_PORT, SAI_NULL_OBJECT_ID, &attr)); +} + +TEST(ClientServerSai, SetInvalidSaiRedisPortAttribute) +{ + auto css = std::make_shared(); + + EXPECT_EQ(SAI_STATUS_SUCCESS, css->apiInitialize(0, &test_services)); + + sai_attribute_t attr; + attr.id = SAI_PORT_ATTR_CUSTOM_RANGE_START + 99; + attr.value.s32 = 0; + + EXPECT_EQ(SAI_STATUS_INVALID_PARAMETER, css->set(SAI_OBJECT_TYPE_PORT, SAI_NULL_OBJECT_ID, &attr)); +} + TEST(ClientServerSai, bulkGet) { ClientServerSai sai; From 79a45bc7aa7b40cdbac7eea5607a6351fec37138 Mon Sep 17 00:00:00 2001 From: DendroLabs Date: Mon, 8 Jun 2026 13:37:30 -0400 Subject: [PATCH 2/2] [Link Event Damping] Clean up stub log message Replace development-facing "stub" wording in the processLinkEventDampingConfigSet NOTICE log with a professional message suitable for operator syslog. Signed-off-by: DendroLabs --- syncd/Syncd.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/syncd/Syncd.cpp b/syncd/Syncd.cpp index 413ccab223..4a77e3df98 100644 --- a/syncd/Syncd.cpp +++ b/syncd/Syncd.cpp @@ -850,7 +850,7 @@ sai_status_t Syncd::processLinkEventDampingConfigSet( { SWSS_LOG_ENTER(); - SWSS_LOG_NOTICE("processLinkEventDampingConfigSet: stub, returning NOT_IMPLEMENTED"); + SWSS_LOG_NOTICE("link event damping config set not yet wired to SAI"); sendLinkEventDampingConfigResponse(SAI_STATUS_NOT_IMPLEMENTED);