From ec5cf301e0fbacfdb2644db055495922d1d958c9 Mon Sep 17 00:00:00 2001 From: Ashish Singh Date: Wed, 6 Dec 2023 01:59:21 -0800 Subject: [PATCH] Add libsai RedisInterface for link event damping. - This supports the link event damping config and algorithm set API. - Sends the link event damping config from libsai to syncd main thread. HLD: sonic-net/SONiC#1071 Signed-off-by: Ashish Singh --- lib/ClientSai.cpp | 3 +- lib/RedisRemoteSaiInterface.cpp | 96 ++++++++ lib/RedisRemoteSaiInterface.h | 19 ++ lib/Sai.cpp | 6 + lib/sairediscommon.h | 2 + syncd/Syncd.cpp | 33 +++ syncd/Syncd.h | 6 + syncd/tests/Makefile.am | 2 +- syncd/tests/TestSyncdLinkEventDamping.cpp | 253 ++++++++++++++++++++++ unittest/lib/TestClientServerSai.cpp | 68 ++++++ 10 files changed, 486 insertions(+), 2 deletions(-) create mode 100644 syncd/tests/TestSyncdLinkEventDamping.cpp diff --git a/lib/ClientSai.cpp b/lib/ClientSai.cpp index 809f50e73..be009a87f 100644 --- a/lib/ClientSai.cpp +++ b/lib/ClientSai.cpp @@ -240,7 +240,8 @@ sai_status_t ClientSai::set( SWSS_LOG_ENTER(); REDIS_CHECK_API_INITIALIZED(); - if (RedisRemoteSaiInterface::isRedisAttribute(objectType, attr)) + if (RedisRemoteSaiInterface::isRedisAttribute(objectType, attr) || + RedisRemoteSaiInterface::isRedisPortAttribute(objectType, attr)) { SWSS_LOG_ERROR("sairedis extension attributes are not supported in CLIENT mode"); diff --git a/lib/RedisRemoteSaiInterface.cpp b/lib/RedisRemoteSaiInterface.cpp index 12bcba7d9..72ac0d9e5 100644 --- a/lib/RedisRemoteSaiInterface.cpp +++ b/lib/RedisRemoteSaiInterface.cpp @@ -493,6 +493,83 @@ sai_status_t RedisRemoteSaiInterface::setRedisExtensionAttribute( return SAI_STATUS_FAILURE; } +sai_status_t RedisRemoteSaiInterface::setLinkEventDampingConfig( + _In_ sai_object_type_t objectType, + _In_ sai_object_id_t objectId, + _In_ const std::vector &values) +{ + SWSS_LOG_ENTER(); + + std::string key = sai_serialize_object_type(objectType) + ":" + sai_serialize_object_id(objectId); + + m_communicationChannel->set(key, values, REDIS_ASIC_STATE_COMMAND_DAMPING_CONFIG_SET); + + if (m_syncMode) + { + swss::KeyOpFieldsValuesTuple kco; + auto status = m_communicationChannel->wait(REDIS_ASIC_STATE_COMMAND_DAMPING_CONFIG_SET, kco); + + m_recorder->recordGenericSetResponse(status); + + return status; + } + + return SAI_STATUS_SUCCESS; +} + +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_INVALID_PARAMETER; + } + + std::string str_attr_id = sai_serialize_redis_port_attr_id( + static_cast(attr->id)); + + switch (attr->id) + { + case SAI_REDIS_PORT_ATTR_LINK_EVENT_DAMPING_ALGORITHM: + { + std::string str_attr_value = sai_serialize_redis_link_event_damping_algorithm( + static_cast(attr->value.s32)); + + return setLinkEventDampingConfig( + objectType, objectId, {swss::FieldValueTuple(str_attr_id, str_attr_value)}); + } + case SAI_REDIS_PORT_ATTR_LINK_EVENT_DAMPING_ALGO_AIED_CONFIG: + { + sai_redis_link_event_damping_algo_aied_config_t *config = + (sai_redis_link_event_damping_algo_aied_config_t *)attr->value.ptr; + + if (config == NULL) + { + SWSS_LOG_ERROR("invalid link damping config attr value NULL"); + + return SAI_STATUS_INVALID_PARAMETER; + } + + std::string str_attr_value = sai_serialize_redis_link_event_damping_aied_config(*config); + + return setLinkEventDampingConfig( + objectType, objectId, {swss::FieldValueTuple(str_attr_id, str_attr_value)}); + } + default: + break; + } + + SWSS_LOG_ERROR("unknown redis port extension attribute: %d", attr->id); + + return SAI_STATUS_INVALID_PARAMETER; +} + bool RedisRemoteSaiInterface::isSaiS8ListValidString( _In_ const sai_s8_list_t &s8list) { @@ -625,6 +702,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), @@ -1792,6 +1874,20 @@ bool RedisRemoteSaiInterface::isRedisAttribute( return true; } +bool RedisRemoteSaiInterface::isRedisPortAttribute( + _In_ sai_object_id_t objectType, + _In_ const sai_attribute_t* attr) +{ + SWSS_LOG_ENTER(); + + if ((objectType != SAI_OBJECT_TYPE_PORT) || (attr == nullptr) || (attr->id < SAI_PORT_ATTR_CUSTOM_RANGE_START)) + { + return false; + } + + return true; +} + void RedisRemoteSaiInterface::handleNotification( _In_ const std::string &name, _In_ const std::string &serializedNotification, diff --git a/lib/RedisRemoteSaiInterface.h b/lib/RedisRemoteSaiInterface.h index 4f19f9faa..a2f4552a7 100644 --- a/lib/RedisRemoteSaiInterface.h +++ b/lib/RedisRemoteSaiInterface.h @@ -207,6 +207,15 @@ namespace sairedis _In_ sai_object_id_t switchId, _In_ const sai_attribute_t* attr); + /** + * @brief Checks whether attribute is custom SAI_REDIS_PORT attribute. + * + * This function should only be used on port_api set function. + */ + static bool isRedisPortAttribute( + _In_ sai_object_id_t obejctType, + _In_ const sai_attribute_t* attr); + void setMeta( _In_ std::weak_ptr meta); @@ -343,6 +352,11 @@ 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); + bool isSaiS8ListValidString( _In_ const sai_s8_list_t &s8list); @@ -370,6 +384,11 @@ namespace sairedis _In_ sai_object_id_t switchId, _In_ const sai_attribute_t *attr); + sai_status_t setLinkEventDampingConfig( + _In_ sai_object_type_t objectType, + _In_ sai_object_id_t objectId, + _In_ const std::vector &values); + void clear_local_state(); sai_switch_notifications_t processNotification( diff --git a/lib/Sai.cpp b/lib/Sai.cpp index 15725dc24..b504e77ef 100644 --- a/lib/Sai.cpp +++ b/lib/Sai.cpp @@ -245,6 +245,12 @@ sai_status_t Sai::set( REDIS_CHECK_CONTEXT(objectId); + if (RedisRemoteSaiInterface::isRedisPortAttribute(objectType, attr)) + { + // skip metadata if attribute is redis extension port attribute. + return context->m_redisSai->set(objectType, objectId, attr); + } + return context->m_meta->set(objectType, objectId, attr); } diff --git a/lib/sairediscommon.h b/lib/sairediscommon.h index d09ddc0ab..309ee4395 100644 --- a/lib/sairediscommon.h +++ b/lib/sairediscommon.h @@ -52,6 +52,8 @@ #define REDIS_ASIC_STATE_COMMAND_OBJECT_TYPE_GET_AVAILABILITY_QUERY "object_type_get_availability_query" #define REDIS_ASIC_STATE_COMMAND_OBJECT_TYPE_GET_AVAILABILITY_RESPONSE "object_type_get_availability_response" +#define REDIS_ASIC_STATE_COMMAND_DAMPING_CONFIG_SET "link_event_damping_config_set" + #define REDIS_FLEX_COUNTER_COMMAND_START_POLL "start_poll" #define REDIS_FLEX_COUNTER_COMMAND_STOP_POLL "stop_poll" #define REDIS_FLEX_COUNTER_COMMAND_SET_GROUP "set_counter_group" diff --git a/syncd/Syncd.cpp b/syncd/Syncd.cpp index e29481c60..1910fb2dd 100644 --- a/syncd/Syncd.cpp +++ b/syncd/Syncd.cpp @@ -383,6 +383,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)); @@ -580,6 +583,36 @@ sai_status_t Syncd::processObjectTypeGetAvailabilityQuery( return status; } +sai_status_t Syncd::processLinkEventDampingConfigSet( + _In_ const swss::KeyOpFieldsValuesTuple &kco) +{ + SWSS_LOG_ENTER(); + + sendLinkEventDampingConfigResponse(SAI_STATUS_NOT_IMPLEMENTED); + + return SAI_STATUS_NOT_IMPLEMENTED; +} + +void Syncd::sendLinkEventDampingConfigResponse( + _In_ sai_status_t status) +{ + SWSS_LOG_ENTER(); + + // If sync mode is not enabled, do not send response. + if (!m_enableSyncMode) + { + return; + } + + std::string strStatus = sai_serialize_status(status); + + std::vector entry; + + SWSS_LOG_INFO("sending link event damping config response: %s", strStatus.c_str()); + + m_selectableChannel->set(strStatus, entry, REDIS_ASIC_STATE_COMMAND_DAMPING_CONFIG_SET); +} + sai_status_t Syncd::processFdbFlush( _In_ const swss::KeyOpFieldsValuesTuple &kco) { diff --git a/syncd/Syncd.h b/syncd/Syncd.h index 06eb84a62..cd2a5d329 100644 --- a/syncd/Syncd.h +++ b/syncd/Syncd.h @@ -210,6 +210,9 @@ namespace syncd _In_ const std::vector &values, _In_ bool fromAsicChannel=true); + sai_status_t processLinkEventDampingConfigSet( + _In_ const swss::KeyOpFieldsValuesTuple &kco); + private: // process quad oid sai_status_t processOidCreate( @@ -361,6 +364,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 25c114334..39bbf1608 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 + main.cpp TestSyncdBrcm.cpp TestSyncdMlnx.cpp TestSyncdNvdaBf.cpp TestSyncdLib.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 000000000..fb9bd0836 --- /dev/null +++ b/syncd/tests/TestSyncdLinkEventDamping.cpp @@ -0,0 +1,253 @@ +#include +#include +#include +#include + +#include + +#include + +#include +#include +#include "swss/select.h" + +#include "Sai.h" +#include "Syncd.h" +#include "MetadataLogger.h" + +#include "TestSyncdLib.h" + +#include "meta/sai_serialize.h" +#include "sairediscommon.h" +#include "meta/RedisSelectableChannel.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 = true; + 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 worker."); +} + +class LinkEventDampingTest : public ::testing::Test +{ +public: + LinkEventDampingTest() + { + SWSS_LOG_ENTER(); + + auto dbAsic = std::make_shared("ASIC_DB", 0); + + m_selectableChannel = std::make_shared( + dbAsic, + REDIS_TABLE_GETRESPONSE, + ASIC_STATE_TABLE, + TEMP_PREFIX, + false); + } + + virtual ~LinkEventDampingTest() = default; + +public: + virtual void SetUp() override + { + SWSS_LOG_ENTER(); + + m_switchId = SAI_NULL_OBJECT_ID; + + // flush ASIC DB + flushAsicDb(); + + syncdStart(); + createSwitch(); + } + + void syncdStart() + { + SWSS_LOG_ENTER(); + + // start syncd worker + m_worker = std::make_shared(syncdLinkEventDampingWorkerThread); + + // initialize SAI redis + m_sairedis = std::make_shared(); + + auto status = m_sairedis->initialize(0, &test_services); + ASSERT_EQ(status, SAI_STATUS_SUCCESS); + + // set communication mode + 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); + + // enable recording + 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); + + } + + void createSwitch() + { + SWSS_LOG_ENTER(); + + sai_attribute_t attr; + + // init view + 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); + + // apply view + attr.id = SAI_REDIS_SWITCH_ATTR_NOTIFY_SYNCD; + attr.value.s32 = SAI_REDIS_NOTIFY_SYNCD_APPLY_VIEW; + + status = m_sairedis->set(SAI_OBJECT_TYPE_SWITCH, SAI_NULL_OBJECT_ID, &attr); + ASSERT_EQ(status, SAI_STATUS_SUCCESS); + + // create switch + attr.id = SAI_SWITCH_ATTR_INIT_SWITCH; + attr.value.booldata = true; + + status = m_sairedis->create(SAI_OBJECT_TYPE_SWITCH, &m_switchId, SAI_NULL_OBJECT_ID, 1, &attr); + ASSERT_EQ(status, SAI_STATUS_SUCCESS); + } + + virtual void TearDown() override + { + SWSS_LOG_ENTER(); + + // uninitialize SAI redis + ASSERT_EQ(m_sairedis->uninitialize(), SAI_STATUS_SUCCESS); + + // stop syncd worker + sendSyncdShutdownNotification(); + m_worker->join(); + } + +protected: + std::shared_ptr m_worker; + std::shared_ptr m_sairedis; + sai_object_id_t m_switchId; + std::shared_ptr m_selectableChannel; +}; + +sai_status_t getResponseStatus( + _In_ const std::string& command, + _In_ sairedis::RedisSelectableChannel *selectable, + _In_ bool init_view_mode) +{ + SWSS_LOG_ENTER(); + + swss::Select s; + s.addSelectable(selectable); + + while (true) + { + swss::Selectable *sel; + int result = s.select(&sel, 1000); + + if (result == swss::Select::OBJECT) + { + swss::KeyOpFieldsValuesTuple kco; + selectable->pop(kco, init_view_mode); + + const std::string &op = kfvOp(kco); + const std::string &opkey = kfvKey(kco); + + if (op != command) + { + SWSS_LOG_WARN("got not expected response: %s:%s", opkey.c_str(), op.c_str()); + continue; + } + + sai_status_t status; + sai_deserialize_status(opkey, status); + + return status; + } + + SWSS_LOG_ERROR("SELECT operation result: %s on %s", swss::Select::resultToString(result).c_str(), command.c_str()); + break; + } + + return SAI_STATUS_FAILURE; +} + +TEST_F(LinkEventDampingTest, SetLinkEventDampingConfigNotImplemented) +{ + std::string key = sai_serialize_object_type(SAI_OBJECT_TYPE_PORT) + ":" + sai_serialize_object_id(SAI_NULL_OBJECT_ID); + + std::string str_attr_id = sai_serialize_redis_port_attr_id(SAI_REDIS_PORT_ATTR_LINK_EVENT_DAMPING_ALGORITHM); + + std::string str_attr_value = sai_serialize_redis_link_event_damping_algorithm(SAI_REDIS_LINK_EVENT_DAMPING_ALGORITHM_AIED); + + m_selectableChannel->set(key, {swss::FieldValueTuple(str_attr_id, str_attr_value)}, REDIS_ASIC_STATE_COMMAND_DAMPING_CONFIG_SET); + + EXPECT_EQ(getResponseStatus(REDIS_ASIC_STATE_COMMAND_DAMPING_CONFIG_SET, m_selectableChannel.get(), false), SAI_STATUS_NOT_IMPLEMENTED); +} diff --git a/unittest/lib/TestClientServerSai.cpp b/unittest/lib/TestClientServerSai.cpp index 67894f7c2..35566f3fb 100644 --- a/unittest/lib/TestClientServerSai.cpp +++ b/unittest/lib/TestClientServerSai.cpp @@ -105,6 +105,74 @@ TEST(ClientServerSai, logSet) EXPECT_EQ(SAI_STATUS_SUCCESS, css->logSet(SAI_API_PORT, SAI_LOG_LEVEL_NOTICE)); } +TEST(ClientServerSai, VerifySaiRedisPortAttrNotSupportedInClientMode) +{ + auto css = std::make_shared(); + + // Initialize as sairedis client. + EXPECT_EQ(SAI_STATUS_SUCCESS, css->initialize(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, SetLinkEventDampingAlgorithm) +{ + auto css = std::make_shared(); + + // Initialize as sairedis server. + EXPECT_EQ(SAI_STATUS_SUCCESS, css->initialize(0, &test_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_SUCCESS, css->set(SAI_OBJECT_TYPE_PORT, SAI_NULL_OBJECT_ID, &attr)); +} + +TEST(ClientServerSai, SetLinkEventDampingConfig) +{ + auto css = std::make_shared(); + + // Initialize as sairedis server. + EXPECT_EQ(SAI_STATUS_SUCCESS, css->initialize(0, &test_services)); + + // Failure when config is NULL. + 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)); + + sai_redis_link_event_damping_algo_aied_config_t config = { + .max_suppress_time = 5000, + .suppress_threshold = 1500, + .reuse_threshold = 1200, + .decay_half_life = 3000, + .flap_penalty = 1000}; + + attr.value.ptr = (void *) &config; + + EXPECT_EQ(SAI_STATUS_SUCCESS, css->set(SAI_OBJECT_TYPE_PORT, SAI_NULL_OBJECT_ID, &attr)); +} + +TEST(ClientServerSai, SetInvalidSaiRedisPortAttribute) +{ + auto css = std::make_shared(); + + // Initialize as sairedis server. + EXPECT_EQ(SAI_STATUS_SUCCESS, css->initialize(0, &test_services)); + + sai_attribute_t attr; + // Set an id that is not supported yet. + attr.id = SAI_REDIS_PORT_ATTR_LINK_EVENT_DAMPING_ALGO_AIED_CONFIG + 100; + + EXPECT_EQ(SAI_STATUS_INVALID_PARAMETER, css->set(SAI_OBJECT_TYPE_PORT, SAI_NULL_OBJECT_ID, &attr)); +} + TEST(ClientServerSai, bulkGetClearStats) { auto css = std::make_shared();