From a8c6b921359801b76732d8263796d16dfe15ae4f Mon Sep 17 00:00:00 2001 From: Oleksandr Ivantsiv Date: Fri, 20 Dec 2024 23:08:38 +0200 Subject: [PATCH] Revert "Revert back to SAI version 1 15 (#1481)" This reverts commit 4f7eed0aac95288077fa9959dc61eacd2f3938d6. --- SAI | 2 +- meta/Meta.cpp | 86 +++++++++++--- meta/Meta.h | 10 ++ meta/SaiInterface.cpp | 17 +++ meta/SaiInterface.h | 19 ++++ meta/SaiOptions.h | 11 ++ meta/SaiSerialize.cpp | 35 ++++++ meta/sai_serialize.h | 7 ++ syncd/AttrVersionChecker.cpp | 107 ++++++++++++++++++ syncd/AttrVersionChecker.h | 40 +++++++ syncd/CommandLineOptions.cpp | 4 + syncd/CommandLineOptions.h | 1 + syncd/CommandLineOptionsParser.cpp | 11 +- syncd/Makefile.am | 1 + syncd/SaiDiscovery.cpp | 34 +++++- syncd/SaiDiscovery.h | 4 + syncd/SaiSwitch.h | 2 +- syncd/SingleReiniter.cpp | 2 +- syncd/Syncd.cpp | 11 +- syncd/VendorSai.cpp | 64 +++++++++++ syncd/VendorSaiOptions.h | 17 +++ tests/aspell.en.pws | 2 + tests/utils.pm | 10 +- unittest/meta/TestMeta.cpp | 33 ++++++ unittest/meta/TestSaiInterface.cpp | 34 ++++++ unittest/meta/TestSaiSerialize.cpp | 11 ++ unittest/syncd/Makefile.am | 1 + unittest/syncd/TestAttrVersionChecker.cpp | 132 ++++++++++++++++++++++ unittest/syncd/TestCommandLineOptions.cpp | 4 +- unittest/syncd/TestVendorSai.cpp | 15 +++ 30 files changed, 697 insertions(+), 30 deletions(-) create mode 100644 meta/SaiOptions.h create mode 100644 syncd/AttrVersionChecker.cpp create mode 100644 syncd/AttrVersionChecker.h create mode 100644 syncd/VendorSaiOptions.h create mode 100644 unittest/syncd/TestAttrVersionChecker.cpp diff --git a/SAI b/SAI index f214adef0..054085547 160000 --- a/SAI +++ b/SAI @@ -1 +1 @@ -Subproject commit f214adef0f336cb991bad4461e626dc039f01878 +Subproject commit 054085547c2042b95ab0fa1a75a44da3288b26bf diff --git a/meta/Meta.cpp b/meta/Meta.cpp index a9851d201..d632eadea 100644 --- a/meta/Meta.cpp +++ b/meta/Meta.cpp @@ -8,6 +8,8 @@ #include +#include + #include // TODO add validation for all oids belong to the same switch @@ -3154,6 +3156,25 @@ sai_status_t Meta::meta_sai_validate_meter_bucket_entry( return SAI_STATUS_NOT_IMPLEMENTED; } +sai_status_t Meta::meta_sai_validate_prefix_compression_entry( + _In_ const sai_prefix_compression_entry_t* prefix_compression_entry, + _In_ bool create, + _In_ bool get) +{ + SWSS_LOG_ENTER(); + + if (prefix_compression_entry == NULL) + { + SWSS_LOG_ERROR("prefix_compression_entry pointer is NULL"); + + return SAI_STATUS_INVALID_PARAMETER; + } + + // TODO FIX ME + + return SAI_STATUS_NOT_IMPLEMENTED; +} + sai_status_t Meta::meta_generic_validation_create( _In_ const sai_object_meta_key_t& meta_key, _In_ sai_object_id_t switch_id, @@ -6675,24 +6696,14 @@ void Meta::meta_sai_on_port_state_change_single( auto ot = objectTypeQuery(data.port_id); - bool valid = false; + bool valid = isPortObjectIdValid(ot); - switch (ot) + if (!valid) { - // TODO hardcoded types, must advance SAI repository commit to get metadata for this - case SAI_OBJECT_TYPE_PORT: - case SAI_OBJECT_TYPE_BRIDGE_PORT: - case SAI_OBJECT_TYPE_LAG: - - valid = true; - break; - - default: - - SWSS_LOG_ERROR("data.port_id %s has unexpected type: %s, expected PORT, BRIDGE_PORT or LAG", - sai_serialize_object_id(data.port_id).c_str(), - sai_serialize_object_type(ot).c_str()); - break; + SWSS_LOG_ERROR("data.port_id %s has unexpected type: %s, expected: %s", + sai_serialize_object_id(data.port_id).c_str(), + sai_serialize_object_type(ot).c_str(), + boost::algorithm::join(getValidPortObjectTypes(), ",").c_str()); } if (valid && !m_oids.objectReferenceExists(data.port_id)) @@ -7112,3 +7123,46 @@ void Meta::populate( } } } + +bool Meta::isPortObjectIdValid( + _In_ sai_object_type_t object_type) +{ + SWSS_LOG_ENTER(); + + auto members = sai_metadata_struct_members_sai_port_oper_status_notification_t; + + for (size_t i = 0; members[i]; i++) + { + auto* mb = members[i]; + + if (mb->membername != std::string("port_id")) + continue; + + for (size_t idx = 0; idx < mb->allowedobjecttypeslength; idx++) + { + if (mb->allowedobjecttypes[idx] == object_type) + return true; + } + + return false; + } + + SWSS_LOG_THROW("port_id member not found on sai_port_oper_status_notification"); +} + +std::vector Meta::getValidPortObjectTypes() +{ + SWSS_LOG_ENTER(); + + auto md = sai_metadata_enum_sai_object_type_t; + + std::vector v; + + for (size_t i = 0; i < md.valuescount; i++) + { + if (isPortObjectIdValid((sai_object_type_t)md.values[i])) + v.push_back(md.valuesshortnames[i]); + } + + return v; +} diff --git a/meta/Meta.h b/meta/Meta.h index ce8a0a2dd..aac696a68 100644 --- a/meta/Meta.h +++ b/meta/Meta.h @@ -315,6 +315,11 @@ namespace saimeta static bool is_ipv6_mask_valid( _In_ const uint8_t* mask); + static bool isPortObjectIdValid( + _In_ sai_object_type_t object_type); + + static std::vector getValidPortObjectTypes(); + private: // unit tests helpers bool meta_unittests_get_and_erase_set_readonly_flag( @@ -563,6 +568,11 @@ namespace saimeta _In_ bool create, _In_ bool get = false); + sai_status_t meta_sai_validate_prefix_compression_entry( + _In_ const sai_prefix_compression_entry_t* prefix_compression_entry, + _In_ bool create, + _In_ bool get = false); + public: /* diff --git a/meta/SaiInterface.cpp b/meta/SaiInterface.cpp index 3a95bc017..c91d08275 100644 --- a/meta/SaiInterface.cpp +++ b/meta/SaiInterface.cpp @@ -378,3 +378,20 @@ sai_status_t SaiInterface::clearStats( return SAI_STATUS_NOT_IMPLEMENTED; } + +std::shared_ptr SaiInterface::getOptions( + _In_ const std::string& key) +{ + SWSS_LOG_ENTER(); + + return m_optionsMap[key]; +} + +void SaiInterface::setOptions( + _In_ const std::string& key, + _In_ std::shared_ptr options) +{ + SWSS_LOG_ENTER(); + + m_optionsMap[key] = options; +} diff --git a/meta/SaiInterface.h b/meta/SaiInterface.h index fb78e1569..1f694c040 100644 --- a/meta/SaiInterface.h +++ b/meta/SaiInterface.h @@ -5,6 +5,12 @@ extern "C" { #include "saimetadata.h" } +#include "SaiOptions.h" + +#include +#include +#include + #define SAIREDIS_DECLARE_EVERY_ENTRY(_X) \ SAI_METADATA_DECLARE_EVERY_ENTRY(_X) @@ -340,5 +346,18 @@ namespace sairedis virtual sai_log_level_t logGet( _In_ sai_api_t api); + + public: // non SAI API - options helper + + std::shared_ptr getOptions( + _In_ const std::string& key); + + void setOptions( + _In_ const std::string& key, + _In_ std::shared_ptr options); + + private: + + std::map> m_optionsMap; }; } diff --git a/meta/SaiOptions.h b/meta/SaiOptions.h new file mode 100644 index 000000000..2d053c6c5 --- /dev/null +++ b/meta/SaiOptions.h @@ -0,0 +1,11 @@ +#pragma once + +namespace sairedis +{ + class SaiOptions + { + public: + + virtual ~SaiOptions() = default; + }; +} diff --git a/meta/SaiSerialize.cpp b/meta/SaiSerialize.cpp index 0916cac02..ca528b4ff 100644 --- a/meta/SaiSerialize.cpp +++ b/meta/SaiSerialize.cpp @@ -975,6 +975,20 @@ std::string sai_serialize_meter_bucket_entry( return j.dump(); } +std::string sai_serialize_prefix_compression_entry( + _In_ const sai_prefix_compression_entry_t &prefix_compression_entry) +{ + SWSS_LOG_ENTER(); + + json j; + + j["switch_id"] = sai_serialize_object_id(prefix_compression_entry.switch_id); + j["prefix_table_id"] = sai_serialize_object_id(prefix_compression_entry.prefix_table_id); + j["prefix"] = sai_serialize_ip_prefix(prefix_compression_entry.prefix); + + return j.dump(); +} + std::string sai_serialize_flow_entry( _In_ const sai_flow_entry_t &flow_entry) { @@ -2670,6 +2684,10 @@ static bool sai_serialize_object_entry( key = sai_serialize_mcast_fdb_entry(key_entry.mcast_fdb_entry); return true; + case SAI_OBJECT_TYPE_PREFIX_COMPRESSION_ENTRY: + key = sai_serialize_prefix_compression_entry(key_entry.prefix_compression_entry); + return true; + default: return false; } @@ -4459,6 +4477,19 @@ void sai_deserialize_meter_bucket_entry( sai_deserialize_number(j["meter_class"], meter_bucket_entry.meter_class); } +void sai_deserialize_prefix_compression_entry( + _In_ const std::string& s, + _Out_ sai_prefix_compression_entry_t& prefix_compression_entry) +{ + SWSS_LOG_ENTER(); + + json j = json::parse(s); + + sai_deserialize_object_id(j["switch_id"], prefix_compression_entry.switch_id); + sai_deserialize_object_id(j["prefix_table_id"], prefix_compression_entry.prefix_table_id); + sai_deserialize_ip_prefix(j["prefix"], prefix_compression_entry.prefix); +} + void sai_deserialize_flow_entry( _In_ const std::string& s, _Out_ sai_flow_entry_t &flow_entry) @@ -4910,6 +4941,10 @@ bool sai_deserialize_object_entry( sai_deserialize_mcast_fdb_entry(object_id, meta_key.objectkey.key.mcast_fdb_entry); return true; + case SAI_OBJECT_TYPE_PREFIX_COMPRESSION_ENTRY: + sai_deserialize_prefix_compression_entry(object_id, meta_key.objectkey.key.prefix_compression_entry); + return true; + default: return false; } diff --git a/meta/sai_serialize.h b/meta/sai_serialize.h index 0ef79e9cb..b1733bf33 100644 --- a/meta/sai_serialize.h +++ b/meta/sai_serialize.h @@ -72,6 +72,9 @@ std::string sai_serialize_fdb_entry( std::string sai_serialize_meter_bucket_entry( _In_ const sai_meter_bucket_entry_t &meter_bucket_entry); +std::string sai_serialize_prefix_compression_entry( + _In_ const sai_prefix_compression_entry_t &prefix_compression_entry); + std::string sai_serialize_flow_entry( _In_ const sai_flow_entry_t &flow_entry); @@ -443,6 +446,10 @@ void sai_deserialize_meter_bucket_entry( _In_ const std::string& s, _Out_ sai_meter_bucket_entry_t& meter_bucket_entry); +void sai_deserialize_prefix_compression_entry( + _In_ const std::string& s, + _Out_ sai_prefix_compression_entry_t& prefix_compression_entry); + void sai_deserialize_flow_entry( _In_ const std::string& s, _Out_ sai_flow_entry_t &flow_entry); diff --git a/syncd/AttrVersionChecker.cpp b/syncd/AttrVersionChecker.cpp new file mode 100644 index 000000000..320afcaf5 --- /dev/null +++ b/syncd/AttrVersionChecker.cpp @@ -0,0 +1,107 @@ +#include "AttrVersionChecker.h" + +#include "swss/logger.h" + +using namespace syncd; + +AttrVersionChecker::AttrVersionChecker(): + m_enabled(false), + m_saiApiVersion(SAI_VERSION(0,0,0)) +{ + SWSS_LOG_ENTER(); + + // empty +} + +void AttrVersionChecker::enable( + _In_ bool enable) +{ + SWSS_LOG_ENTER(); + + m_enabled = enable; +} + +void AttrVersionChecker::setSaiApiVersion( + _In_ sai_api_version_t version) +{ + SWSS_LOG_ENTER(); + + m_saiApiVersion = version; +} + +void AttrVersionChecker::reset() +{ + SWSS_LOG_ENTER(); + + m_visitedAttributes.clear(); +} + +bool AttrVersionChecker::isSufficientVersion( + _In_ const sai_attr_metadata_t *md) +{ + SWSS_LOG_ENTER(); + + if (md == nullptr) + { + SWSS_LOG_ERROR("md is NULL"); + + return false; + } + + if (!m_enabled) + { + return true; + } + + if (SAI_METADATA_HAVE_ATTR_VERSION == 0) + { + // metadata does not contain attr versions, no check will be preformed + return true; + } + + // check attr version if metadata have version defined + + if (m_saiApiVersion > md->apiversion) + { + // ok, SAI version is bigger than attribute release version + + return true; + } + + if (m_saiApiVersion < md->apiversion) + { + // skip, SAI version is not sufficient + + if (m_visitedAttributes.find(md->attridname) == m_visitedAttributes.end()) + { + m_visitedAttributes.insert(md->attridname); + + // log only once + + SWSS_LOG_WARN("SAI version %lu, not sufficient to discover %s", m_saiApiVersion, md->attridname); + } + + return false; + } + + // m_saiApiVersion == md->apiversion + + if (md->nextrelease == false) + { + // ok, SAI version is equal to attribute version + return true; + } + + // next release == true + + if (m_visitedAttributes.find(md->attridname) == m_visitedAttributes.end()) + { + m_visitedAttributes.insert(md->attridname); + + // warn only once + + SWSS_LOG_WARN("%s is ment for next release after %lu, will not discover", md->attridname, m_saiApiVersion); + } + + return false; +} diff --git a/syncd/AttrVersionChecker.h b/syncd/AttrVersionChecker.h new file mode 100644 index 000000000..f2b65cddd --- /dev/null +++ b/syncd/AttrVersionChecker.h @@ -0,0 +1,40 @@ +#pragma once + +extern "C" { +#include "sai.h" +#include "saimetadata.h" +} + +#include +#include + +namespace syncd +{ + class AttrVersionChecker + { + public: + + AttrVersionChecker(); + + public: + + void enable( + _In_ bool enable); + + void setSaiApiVersion( + _In_ sai_api_version_t version); + + void reset(); + + bool isSufficientVersion( + _In_ const sai_attr_metadata_t *md); + + private: + + bool m_enabled; + + sai_api_version_t m_saiApiVersion; + + std::set m_visitedAttributes; + }; +} diff --git a/syncd/CommandLineOptions.cpp b/syncd/CommandLineOptions.cpp index 0c25c8cad..59ef25106 100644 --- a/syncd/CommandLineOptions.cpp +++ b/syncd/CommandLineOptions.cpp @@ -44,6 +44,9 @@ CommandLineOptions::CommandLineOptions() #endif // SAITHRIFT + m_supportingBulkCounterGroups = ""; + + m_enableAttrVersionCheck = false; } std::string CommandLineOptions::getCommandLineString() const @@ -67,6 +70,7 @@ std::string CommandLineOptions::getCommandLineString() const ss << " BreakConfig=" << m_breakConfig; ss << " WatchdogWarnTimeSpan=" << m_watchdogWarnTimeSpan; ss << " SupportingBulkCounters=" << m_supportingBulkCounterGroups; + ss << " EnableAttrVersionCheck=" << (m_enableAttrVersionCheck ? "YES" : "NO"); #ifdef SAITHRIFT diff --git a/syncd/CommandLineOptions.h b/syncd/CommandLineOptions.h index 99d0827d6..72982ea5e 100644 --- a/syncd/CommandLineOptions.h +++ b/syncd/CommandLineOptions.h @@ -100,5 +100,6 @@ namespace syncd std::string m_supportingBulkCounterGroups; + bool m_enableAttrVersionCheck; }; } diff --git a/syncd/CommandLineOptionsParser.cpp b/syncd/CommandLineOptionsParser.cpp index d49624336..66c13e49c 100644 --- a/syncd/CommandLineOptionsParser.cpp +++ b/syncd/CommandLineOptionsParser.cpp @@ -19,9 +19,9 @@ std::shared_ptr CommandLineOptionsParser::parseCommandLine( auto options = std::make_shared(); #ifdef SAITHRIFT - const char* const optstring = "dp:t:g:x:b:B:w:uSUCsz:lrm:h"; + const char* const optstring = "dp:t:g:x:b:B:aw:uSUCsz:lrm:h"; #else - const char* const optstring = "dp:t:g:x:b:B:w:uSUCsz:lh"; + const char* const optstring = "dp:t:g:x:b:B:aw:uSUCsz:lh"; #endif // SAITHRIFT while (true) @@ -43,6 +43,7 @@ std::shared_ptr CommandLineOptionsParser::parseCommandLine( { "breakConfig", required_argument, 0, 'b' }, { "watchdogWarnTimeSpan", optional_argument, 0, 'w' }, { "supportingBulkCounters", required_argument, 0, 'B' }, + { "enableAttrVersionCheck", no_argument, 0, 'a' }, #ifdef SAITHRIFT { "rpcserver", no_argument, 0, 'r' }, { "portmap", required_argument, 0, 'm' }, @@ -138,6 +139,10 @@ std::shared_ptr CommandLineOptionsParser::parseCommandLine( options->m_supportingBulkCounterGroups = std::string(optarg); break; + case 'a': + options->m_enableAttrVersionCheck = true; + break; + case 'h': printUsage(); exit(EXIT_SUCCESS); @@ -196,6 +201,8 @@ void CommandLineOptionsParser::printUsage() std::cout << " Watchdog time span (in microseconds) to watch for execution" << std::endl; std::cout << " -B --supportingBulkCounters" << std::endl; std::cout << " Counter groups those support bulk polling" << std::endl; + std::cout << " -a --enableAttrVersionCheck" << std::endl; + std::cout << " Enable attribute SAI version check when performing SAI discovery" << std::endl; #ifdef SAITHRIFT diff --git a/syncd/Makefile.am b/syncd/Makefile.am index 3c546fb74..8c8cddaf3 100644 --- a/syncd/Makefile.am +++ b/syncd/Makefile.am @@ -17,6 +17,7 @@ noinst_LIBRARIES = libSyncd.a libSyncdRequestShutdown.a libMdioIpcClient.a libSyncd_a_SOURCES = \ AsicOperation.cpp \ AsicView.cpp \ + AttrVersionChecker.cpp \ BestCandidateFinder.cpp \ BreakConfig.cpp \ BreakConfigParser.cpp \ diff --git a/syncd/SaiDiscovery.cpp b/syncd/SaiDiscovery.cpp index 8615526bb..3ba89bc98 100644 --- a/syncd/SaiDiscovery.cpp +++ b/syncd/SaiDiscovery.cpp @@ -1,4 +1,5 @@ #include "SaiDiscovery.h" +#include "VendorSaiOptions.h" #include "swss/logger.h" @@ -24,7 +25,31 @@ SaiDiscovery::SaiDiscovery( { SWSS_LOG_ENTER(); - // empty + sai_api_version_t version = SAI_VERSION(0,0,0); + + sai_status_t status = m_sai->queryApiVersion(&version); + + if (status == SAI_STATUS_SUCCESS) + { + auto vso = std::dynamic_pointer_cast(sai->getOptions(VendorSaiOptions::OPTIONS_KEY)); + + // TODO check vso for null + + m_attrVersionChecker.enable(vso->m_checkAttrVersion); + m_attrVersionChecker.setSaiApiVersion(version); + + SWSS_LOG_NOTICE("check attr version %s, libsai api version: %lu", + (vso->m_checkAttrVersion ? "ENABLED" : "DISABLED"), + version); + } + else + { + m_attrVersionChecker.enable(false); + m_attrVersionChecker.setSaiApiVersion(SAI_API_VERSION); + + SWSS_LOG_WARN("failed to obtain libsai api version: %s, will discover all attributes", + sai_serialize_status(status).c_str()); + } } SaiDiscovery::~SaiDiscovery() @@ -110,6 +135,11 @@ void SaiDiscovery::discover( attr.id = md->attrid; + if (!m_attrVersionChecker.isSufficientVersion(md)) + { + continue; + } + if (md->attrvaluetype == SAI_ATTR_VALUE_TYPE_OBJECT_ID) { if (md->defaultvaluetype == SAI_DEFAULT_VALUE_TYPE_CONST) @@ -259,6 +289,8 @@ std::set SaiDiscovery::discover( m_defaultOidMap.clear(); + m_attrVersionChecker.reset(); + std::set discovered_rids; { diff --git a/syncd/SaiDiscovery.h b/syncd/SaiDiscovery.h index c48372b08..b6f8cf053 100644 --- a/syncd/SaiDiscovery.h +++ b/syncd/SaiDiscovery.h @@ -2,6 +2,8 @@ #include "meta/SaiInterface.h" +#include "AttrVersionChecker.h" + #include #include #include @@ -61,5 +63,7 @@ namespace syncd std::shared_ptr m_sai; DefaultOidMap m_defaultOidMap; + + AttrVersionChecker m_attrVersionChecker; }; } diff --git a/syncd/SaiSwitch.h b/syncd/SaiSwitch.h index 523e1fe1d..d6bccfeed 100644 --- a/syncd/SaiSwitch.h +++ b/syncd/SaiSwitch.h @@ -34,7 +34,7 @@ namespace syncd _In_ std::shared_ptr client, _In_ std::shared_ptr translator, _In_ std::shared_ptr vendorSai, - _In_ bool warmBoot = false); + _In_ bool warmBoot); virtual ~SaiSwitch() = default; diff --git a/syncd/SingleReiniter.cpp b/syncd/SingleReiniter.cpp index f757ba515..6f5355736 100644 --- a/syncd/SingleReiniter.cpp +++ b/syncd/SingleReiniter.cpp @@ -317,7 +317,7 @@ void SingleReiniter::processSwitches() * object, so when doing discover we will get full default ASIC view. */ - m_sw = std::make_shared(m_switch_vid, m_switch_rid, m_client, m_translator, m_vendorSai); + m_sw = std::make_shared(m_switch_vid, m_switch_rid, m_client, m_translator, m_vendorSai, false); /* * We processed switch. We have switch vid/rid so we can process all diff --git a/syncd/Syncd.cpp b/syncd/Syncd.cpp index e7efff34e..5db10178f 100644 --- a/syncd/Syncd.cpp +++ b/syncd/Syncd.cpp @@ -12,6 +12,7 @@ #include "RedisNotificationProducer.h" #include "ZeroMQNotificationProducer.h" #include "WatchdogScope.h" +#include "VendorSaiOptions.h" #include "sairediscommon.h" @@ -109,6 +110,12 @@ Syncd::Syncd( m_enableSyncMode = true; } + auto vso = std::make_shared(); + + vso->m_checkAttrVersion = m_commandLineOptions->m_enableAttrVersionCheck; + + m_vendorSai->setOptions(VendorSaiOptions::OPTIONS_KEY, vso); + m_manager = std::make_shared(m_vendorSai, m_contextConfig->m_dbCounters, m_commandLineOptions->m_supportingBulkCounterGroups); loadProfileMap(); @@ -3194,7 +3201,7 @@ sai_status_t Syncd::processOidCreate( * constructor, like getting all queues, ports, etc. */ - m_switches[switchVid] = std::make_shared(switchVid, objectRid, m_client, m_translator, m_vendorSai); + m_switches[switchVid] = std::make_shared(switchVid, objectRid, m_client, m_translator, m_vendorSai, false); m_mdioIpcServer->setSwitchId(objectRid); @@ -4513,7 +4520,7 @@ void Syncd::onSwitchCreateInInitViewMode( // make switch initialization and get all default data - m_switches[switchVid] = std::make_shared(switchVid, switchRid, m_client, m_translator, m_vendorSai); + m_switches[switchVid] = std::make_shared(switchVid, switchRid, m_client, m_translator, m_vendorSai, false); m_mdioIpcServer->setSwitchId(switchRid); diff --git a/syncd/VendorSai.cpp b/syncd/VendorSai.cpp index 4e6ee2d8b..769396dc5 100644 --- a/syncd/VendorSai.cpp +++ b/syncd/VendorSai.cpp @@ -1084,6 +1084,33 @@ sai_status_t VendorSai::bulkCreate( object_statuses); } +sai_status_t VendorSai::bulkCreate( + _In_ uint32_t object_count, + _In_ const sai_prefix_compression_entry_t* entries, + _In_ const uint32_t *attr_count, + _In_ const sai_attribute_t **attr_list, + _In_ sai_bulk_op_error_mode_t mode, + _Out_ sai_status_t *object_statuses) +{ + MUTEX(); + SWSS_LOG_ENTER(); + VENDOR_CHECK_API_INITIALIZED(); + + if (!m_apis.prefix_compression_api->create_prefix_compression_entries) + { + SWSS_LOG_INFO("create_prefix_compression_entries is not supported"); + return SAI_STATUS_NOT_SUPPORTED; + } + + return m_apis.prefix_compression_api->create_prefix_compression_entries( + object_count, + entries, + attr_count, + attr_list, + mode, + object_statuses); +} + // BULK REMOVE sai_status_t VendorSai::bulkRemove( @@ -1432,6 +1459,29 @@ sai_status_t VendorSai::bulkRemove( object_statuses); } +sai_status_t VendorSai::bulkRemove( + _In_ uint32_t object_count, + _In_ const sai_prefix_compression_entry_t *entries, + _In_ sai_bulk_op_error_mode_t mode, + _Out_ sai_status_t *object_statuses) +{ + MUTEX(); + SWSS_LOG_ENTER(); + VENDOR_CHECK_API_INITIALIZED(); + + if (!m_apis.prefix_compression_api->remove_prefix_compression_entries) + { + SWSS_LOG_INFO("remove_prefix_compression_entries is not supported"); + return SAI_STATUS_NOT_SUPPORTED; + } + + return m_apis.prefix_compression_api->remove_prefix_compression_entries( + object_count, + entries, + mode, + object_statuses); +} + // BULK SET sai_status_t VendorSai::bulkSet( @@ -1710,6 +1760,20 @@ sai_status_t VendorSai::bulkSet( return SAI_STATUS_NOT_SUPPORTED; } +sai_status_t VendorSai::bulkSet( + _In_ uint32_t object_count, + _In_ const sai_prefix_compression_entry_t *entries, + _In_ const sai_attribute_t *attr_list, + _In_ sai_bulk_op_error_mode_t mode, + _Out_ sai_status_t *object_statuses) +{ + MUTEX(); + SWSS_LOG_ENTER(); + VENDOR_CHECK_API_INITIALIZED(); + + return SAI_STATUS_NOT_SUPPORTED; +} + // NON QUAD API sai_status_t VendorSai::flushFdbEntries( diff --git a/syncd/VendorSaiOptions.h b/syncd/VendorSaiOptions.h new file mode 100644 index 000000000..4c66e81d3 --- /dev/null +++ b/syncd/VendorSaiOptions.h @@ -0,0 +1,17 @@ +#pragma once + +#include "meta/SaiOptions.h" + +namespace syncd +{ + class VendorSaiOptions: + public sairedis::SaiOptions + { + public: + static constexpr const char *OPTIONS_KEY = "vok"; + + public: + + bool m_checkAttrVersion = false; + }; +} diff --git a/tests/aspell.en.pws b/tests/aspell.en.pws index 1908c5834..4a99b8dec 100644 --- a/tests/aspell.en.pws +++ b/tests/aspell.en.pws @@ -478,3 +478,5 @@ saiproxy submodule Enqueue deque +apiversion +vso diff --git a/tests/utils.pm b/tests/utils.pm index 489e5bb5a..34b120bf8 100644 --- a/tests/utils.pm +++ b/tests/utils.pm @@ -48,19 +48,19 @@ sub flush_redis sub start_syncd { print color('bright_blue') . "Starting syncd" . color('reset') . "\n"; - `./vssyncd -SUu -p "$DIR/vsprofile.ini" @_ >/dev/null 2>/dev/null &`; + `./vssyncd -aSUu -p "$DIR/vsprofile.ini" @_ >/dev/null 2>/dev/null &`; } sub start_syncd_bulk { print color('bright_blue') . "Starting syncd bulk" . color('reset') . "\n"; - `./vssyncd -SUul -p "$DIR/vsprofile.ini" @_ >/dev/null 2>/dev/null &`; + `./vssyncd -aSUul -p "$DIR/vsprofile.ini" @_ >/dev/null 2>/dev/null &`; } sub start_syncd_warm { print color('bright_blue') . "Starting syncd warm" . color('reset') . "\n"; - `./vssyncd -SUu -t warm -p "$DIR/vsprofile.ini" >/dev/null 2>/dev/null &`; + `./vssyncd -aSUu -t warm -p "$DIR/vsprofile.ini" >/dev/null 2>/dev/null &`; sleep 1; } @@ -68,13 +68,13 @@ sub start_syncd_warm sub sync_start_syncd { print color('bright_blue') . "Starting syncd" . color('reset') . "\n"; - `./vssyncd -s -SUu -p "$DIR/vsprofile.ini" >/dev/null 2>/dev/null &`; + `./vssyncd -s -aSUu -p "$DIR/vsprofile.ini" >/dev/null 2>/dev/null &`; } sub sync_start_syncd_warm { print color('bright_blue') . "Starting syncd warm" . color('reset') . "\n"; - `./vssyncd -s -SUu -t warm -p "$DIR/vsprofile.ini" >/dev/null 2>/dev/null &`; + `./vssyncd -s -aSUu -t warm -p "$DIR/vsprofile.ini" >/dev/null 2>/dev/null &`; sleep 1; } diff --git a/unittest/meta/TestMeta.cpp b/unittest/meta/TestMeta.cpp index 0b0d35b25..454f9ff85 100644 --- a/unittest/meta/TestMeta.cpp +++ b/unittest/meta/TestMeta.cpp @@ -7,6 +7,8 @@ #include +#include + #include #define VLAN_ID 2 @@ -1841,3 +1843,34 @@ TEST(Meta, remove_meter_bucket_entry) EXPECT_EQ(SAI_STATUS_INVALID_PARAMETER, sai.remove(e)); } + +TEST(Meta, remove_prefix_compression_entry) +{ + Meta sai(std::make_shared()); + + sai_prefix_compression_entry_t* e = nullptr; + + EXPECT_EQ(SAI_STATUS_INVALID_PARAMETER, sai.remove(e)); +} + +TEST(Meta, isPortObjectIdValid) +{ + EXPECT_EQ(Meta::isPortObjectIdValid(SAI_OBJECT_TYPE_PORT), true); + EXPECT_EQ(Meta::isPortObjectIdValid(SAI_OBJECT_TYPE_BRIDGE_PORT), true); + EXPECT_EQ(Meta::isPortObjectIdValid(SAI_OBJECT_TYPE_LAG), true); + + EXPECT_EQ(Meta::isPortObjectIdValid(SAI_OBJECT_TYPE_TUNNEL),false); + EXPECT_EQ(Meta::isPortObjectIdValid(SAI_OBJECT_TYPE_NULL), false); + EXPECT_EQ(Meta::isPortObjectIdValid(SAI_OBJECT_TYPE_VLAN), false); +} + +TEST(Meta, getValidPortObjectTypes) +{ + auto v = Meta::getValidPortObjectTypes(); + + EXPECT_EQ(v.size(), 3); + + auto s = boost::algorithm::join(v, ","); + + EXPECT_EQ(s, "PORT,LAG,BRIDGE_PORT"); +} diff --git a/unittest/meta/TestSaiInterface.cpp b/unittest/meta/TestSaiInterface.cpp index 19a095ab8..cf35fc113 100644 --- a/unittest/meta/TestSaiInterface.cpp +++ b/unittest/meta/TestSaiInterface.cpp @@ -117,3 +117,37 @@ TEST(SaiInterface, stats_meter_bucket_entry) EXPECT_EQ(SAI_STATUS_NOT_IMPLEMENTED, s->getStatsExt(m, 0, nullptr, SAI_STATS_MODE_READ, nullptr)); EXPECT_EQ(SAI_STATUS_NOT_IMPLEMENTED, s->clearStats(m, 0, nullptr)); } + +class Opt: + public SaiOptions +{ + public: + + int i; +}; + +TEST(SaiInterface, setOptions) +{ + DummySaiInterface ds; + + ds.setOptions("key", std::make_shared()); +} + +TEST(SaiInterface, getOptions) +{ + DummySaiInterface ds; + + auto opt = std::make_shared(); + + opt->i = 42; + + ds.setOptions("key", opt); + + auto o = std::dynamic_pointer_cast(ds.getOptions("key")); + + EXPECT_NE(o, nullptr); + + EXPECT_EQ(o->i, 42); + + EXPECT_EQ(ds.getOptions("foo"), nullptr); +} diff --git a/unittest/meta/TestSaiSerialize.cpp b/unittest/meta/TestSaiSerialize.cpp index b7e0c97d6..ab7521db0 100644 --- a/unittest/meta/TestSaiSerialize.cpp +++ b/unittest/meta/TestSaiSerialize.cpp @@ -1347,3 +1347,14 @@ TEST(SaiSerialize, serialize_number) EXPECT_EQ(sn, -0x12345678); EXPECT_EQ(u, 0x12345678); } + +TEST(SaiSerialize, sai_serialize_prefix_compression_entry) +{ + sai_prefix_compression_entry_t e; + + memset(&e, 0, sizeof(e)); + + auto s = sai_serialize_prefix_compression_entry(e); + + sai_deserialize_prefix_compression_entry(s, e); +} diff --git a/unittest/syncd/Makefile.am b/unittest/syncd/Makefile.am index b06a3ec08..13b0a43d2 100644 --- a/unittest/syncd/Makefile.am +++ b/unittest/syncd/Makefile.am @@ -7,6 +7,7 @@ LDADD_GTEST = -L/usr/src/gtest -lgtest -lgtest_main -lgmock tests_SOURCES = main.cpp \ MockableSaiInterface.cpp \ MockHelper.cpp \ + TestAttrVersionChecker.cpp \ TestCommandLineOptions.cpp \ TestConcurrentQueue.cpp \ TestFlexCounter.cpp \ diff --git a/unittest/syncd/TestAttrVersionChecker.cpp b/unittest/syncd/TestAttrVersionChecker.cpp new file mode 100644 index 000000000..f1bc91ab9 --- /dev/null +++ b/unittest/syncd/TestAttrVersionChecker.cpp @@ -0,0 +1,132 @@ +#include + +#include +#include +#include + +#include + +#include "AttrVersionChecker.h" +#include "swss/logger.h" + +using namespace syncd; + +TEST(AttrVersionChecker, ctr) +{ + AttrVersionChecker avc; +} + +TEST(AttrVersionChecker, enable) +{ + AttrVersionChecker avc; + + avc.enable(true); + + avc.enable(false); +} + +TEST(AttrVersionChecker, setSaiApiVersion) +{ + AttrVersionChecker avc; + + avc.setSaiApiVersion(SAI_VERSION(1,13,0)); +} + +TEST(AttrVersionChecker, reset) +{ + AttrVersionChecker avc; + + avc.reset(); +} + +#define MD(x,v,n) \ + const sai_attr_metadata_t x = {\ + .objecttype = (sai_object_type_t)SAI_OBJECT_TYPE_BRIDGE,\ + .attrid = SAI_BRIDGE_ATTR_PORT_LIST,\ + .attridname = "SAI_BRIDGE_ATTR_PORT_LIST",\ + .brief = "List of bridge ports associated to this bridge.",\ + .attrvaluetype = SAI_ATTR_VALUE_TYPE_OBJECT_LIST,\ + .flags = (sai_attr_flags_t)(SAI_ATTR_FLAGS_READ_ONLY),\ + .allowedobjecttypes = NULL,\ + .allowedobjecttypeslength = 0,\ + .allowrepetitiononlist = false,\ + .allowmixedobjecttypes = false,\ + .allowemptylist = false,\ + .allownullobjectid = false,\ + .isoidattribute = (1 > 0),\ + .defaultvaluetype = SAI_DEFAULT_VALUE_TYPE_NONE,\ + .defaultvalue = NULL,\ + .defaultvalueobjecttype = SAI_OBJECT_TYPE_NULL,\ + .defaultvalueattrid = SAI_INVALID_ATTRIBUTE_ID,\ + .storedefaultvalue = false,\ + .isenum = false,\ + .isenumlist = false,\ + .enummetadata = NULL,\ + .conditiontype = SAI_ATTR_CONDITION_TYPE_NONE,\ + .conditions = NULL,\ + .conditionslength = 0,\ + .isconditional = (0 != 0),\ + .validonlytype = SAI_ATTR_CONDITION_TYPE_NONE,\ + .validonly = NULL,\ + .validonlylength = 0,\ + .isvalidonly = (0 != 0),\ + .getsave = false,\ + .isvlan = false,\ + .isaclfield = false,\ + .isaclaction = false,\ + .isaclmask = false,\ + .ismandatoryoncreate = false,\ + .iscreateonly = false,\ + .iscreateandset = false,\ + .isreadonly = true,\ + .iskey = false,\ + .isprimitive = false,\ + .notificationtype = -1,\ + .iscallback = false,\ + .pointertype = -1,\ + .capability = NULL,\ + .capabilitylength = 0,\ + .isextensionattr = false,\ + .isresourcetype = false,\ + .isdeprecated = false,\ + .isconditionrelaxed = false,\ + .iscustom = false,\ + .apiversion = (v),\ + .nextrelease = (n),\ + };\ + + +TEST(AttrVersionChecker, isSufficientVersion) +{ + AttrVersionChecker avc; + + avc.enable(true); + EXPECT_EQ(avc.isSufficientVersion(nullptr), false); + + avc.enable(false); + avc.setSaiApiVersion(SAI_VERSION(1,10,0)); + avc.enable(true); + + MD(md,SAI_VERSION(1,9,0),false); + EXPECT_EQ(avc.isSufficientVersion(&md), true); + + MD(md1,SAI_VERSION(1,11,0),false); + EXPECT_EQ(avc.isSufficientVersion(&md1),false); + + avc.enable(false); + EXPECT_EQ(avc.isSufficientVersion(&md1),true); + + avc.enable(true); + avc.setSaiApiVersion(SAI_VERSION(1,10,0)); + EXPECT_EQ(avc.isSufficientVersion(&md1),false); + + avc.setSaiApiVersion(SAI_VERSION(1,12,0)); + EXPECT_EQ(avc.isSufficientVersion(&md1),true); + + avc.setSaiApiVersion(SAI_VERSION(1,11,0)); + EXPECT_EQ(avc.isSufficientVersion(&md1),true); + + MD(md2,SAI_VERSION(1,11,0),true); + avc.setSaiApiVersion(SAI_VERSION(1,11,0)); + EXPECT_EQ(avc.isSufficientVersion(&md2),false); +} diff --git a/unittest/syncd/TestCommandLineOptions.cpp b/unittest/syncd/TestCommandLineOptions.cpp index 7c9088bf0..0eb5aaef3 100644 --- a/unittest/syncd/TestCommandLineOptions.cpp +++ b/unittest/syncd/TestCommandLineOptions.cpp @@ -38,6 +38,8 @@ R"(Usage: syncd [-d] [-p profile] [-t type] [-u] [-S] [-U] [-C] [-s] [-z mode] [ Watchdog time span (in microseconds) to watch for execution -B --supportingBulkCounters Counter groups those support bulk polling + -a --enableAttrVersionCheck + Enable attribute SAI version check when performing SAI discovery -h --help Print out this message )"; @@ -51,7 +53,7 @@ TEST(CommandLineOptions, getCommandLineString) EXPECT_EQ(str, " EnableDiagShell=NO EnableTempView=NO DisableExitSleep=NO EnableUnittests=NO" " EnableConsistencyCheck=NO EnableSyncMode=NO RedisCommunicationMode=redis_async" " EnableSaiBulkSuport=NO StartType=cold ProfileMapFile= GlobalContext=0 ContextConfig= BreakConfig=" - " WatchdogWarnTimeSpan=30000000 SupportingBulkCounters="); + " WatchdogWarnTimeSpan=30000000 SupportingBulkCounters= EnableAttrVersionCheck=NO"); } TEST(CommandLineOptions, startTypeStringToStartType) diff --git a/unittest/syncd/TestVendorSai.cpp b/unittest/syncd/TestVendorSai.cpp index 63977e713..f14d280a4 100644 --- a/unittest/syncd/TestVendorSai.cpp +++ b/unittest/syncd/TestVendorSai.cpp @@ -1543,3 +1543,18 @@ TEST(VendorSai, bulk_meter_rules) EXPECT_EQ(SAI_STATUS_SUCCESS, sai.remove((sai_object_type_t)SAI_OBJECT_TYPE_METER_POLICY, meter_policy0)); EXPECT_EQ(SAI_STATUS_SUCCESS, sai.remove((sai_object_type_t)SAI_OBJECT_TYPE_METER_POLICY, meter_policy1)); } + +TEST_F(VendorSaiTest, bulk_prefix_compression_entry) +{ + sai_prefix_compression_entry_t *e = nullptr; + + // metadata will fail + EXPECT_EQ(SAI_STATUS_INVALID_PARAMETER, + m_vsai->bulkCreate(0, e, nullptr, nullptr, SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR, nullptr)); + + EXPECT_EQ(SAI_STATUS_INVALID_PARAMETER, + m_vsai->bulkRemove(0, e, SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR, nullptr)); + + EXPECT_EQ(SAI_STATUS_NOT_SUPPORTED, + m_vsai->bulkSet(0, e, nullptr, SAI_BULK_OP_ERROR_MODE_STOP_ON_ERROR, nullptr)); +}