From ffd7c28109197bc9fc18e75dd79c2ff375a6f1c3 Mon Sep 17 00:00:00 2001 From: kcudnik Date: Fri, 22 Nov 2024 17:41:07 +0100 Subject: [PATCH 1/2] [syncd] Introduce VendorSaiOptions class And SaiOptions interface, for easy passing parameters over Sai objects --- meta/SaiInterface.cpp | 17 +++++++++++++++ meta/SaiInterface.h | 19 +++++++++++++++++ meta/SaiOptions.h | 11 ++++++++++ syncd/HardReiniter.cpp | 9 +++----- syncd/HardReiniter.h | 5 +---- syncd/SaiDiscovery.cpp | 14 ++++++++---- syncd/SaiDiscovery.h | 3 +-- syncd/SaiSwitch.cpp | 10 ++++----- syncd/SaiSwitch.h | 5 +---- syncd/SingleReiniter.cpp | 8 +++---- syncd/SingleReiniter.h | 5 +---- syncd/Syncd.cpp | 15 +++++++++---- syncd/VendorSaiOptions.h | 17 +++++++++++++++ unittest/meta/TestSaiInterface.cpp | 34 ++++++++++++++++++++++++++++++ 14 files changed, 133 insertions(+), 39 deletions(-) create mode 100644 meta/SaiOptions.h create mode 100644 syncd/VendorSaiOptions.h 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/syncd/HardReiniter.cpp b/syncd/HardReiniter.cpp index 9b621f8b0..8c714eb23 100644 --- a/syncd/HardReiniter.cpp +++ b/syncd/HardReiniter.cpp @@ -13,13 +13,11 @@ HardReiniter::HardReiniter( _In_ std::shared_ptr client, _In_ std::shared_ptr translator, _In_ std::shared_ptr sai, - _In_ std::shared_ptr handler, - _In_ bool checkAttrVersion): + _In_ std::shared_ptr handler): m_vendorSai(sai), m_translator(translator), m_client(client), - m_handler(handler), - m_checkAttrVersion(checkAttrVersion) + m_handler(handler) { SWSS_LOG_ENTER(); @@ -101,8 +99,7 @@ std::map> HardReiniter::hardR m_handler, m_switchVidToRid.at(kvp.first), m_switchRidToVid.at(kvp.first), - kvp.second, - m_checkAttrVersion); + kvp.second); sr->hardReinit(); diff --git a/syncd/HardReiniter.h b/syncd/HardReiniter.h index 435f38f16..9a032d16e 100644 --- a/syncd/HardReiniter.h +++ b/syncd/HardReiniter.h @@ -27,8 +27,7 @@ namespace syncd _In_ std::shared_ptr client, _In_ std::shared_ptr translator, _In_ std::shared_ptr sai, - _In_ std::shared_ptr handler, - _In_ bool checkAttrVersion); + _In_ std::shared_ptr handler); virtual ~HardReiniter(); @@ -60,7 +59,5 @@ namespace syncd std::shared_ptr m_client; std::shared_ptr m_handler; - - bool m_checkAttrVersion; }; } diff --git a/syncd/SaiDiscovery.cpp b/syncd/SaiDiscovery.cpp index 43e01016a..3ba89bc98 100644 --- a/syncd/SaiDiscovery.cpp +++ b/syncd/SaiDiscovery.cpp @@ -1,4 +1,5 @@ #include "SaiDiscovery.h" +#include "VendorSaiOptions.h" #include "swss/logger.h" @@ -19,8 +20,7 @@ using namespace syncd; #define SAI_DISCOVERY_LIST_MAX_ELEMENTS 1024 SaiDiscovery::SaiDiscovery( - _In_ std::shared_ptr sai, - _In_ bool checkAttrVersion): + _In_ std::shared_ptr sai): m_sai(sai) { SWSS_LOG_ENTER(); @@ -31,10 +31,16 @@ SaiDiscovery::SaiDiscovery( if (status == SAI_STATUS_SUCCESS) { - m_attrVersionChecker.enable(checkAttrVersion); + 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 ENABLED, libsai api version: %lu", version); + SWSS_LOG_NOTICE("check attr version %s, libsai api version: %lu", + (vso->m_checkAttrVersion ? "ENABLED" : "DISABLED"), + version); } else { diff --git a/syncd/SaiDiscovery.h b/syncd/SaiDiscovery.h index c677a5b87..b6f8cf053 100644 --- a/syncd/SaiDiscovery.h +++ b/syncd/SaiDiscovery.h @@ -20,8 +20,7 @@ namespace syncd public: SaiDiscovery( - _In_ std::shared_ptr sai, - _In_ bool checkAttrVersion); + _In_ std::shared_ptr sai); virtual ~SaiDiscovery(); diff --git a/syncd/SaiSwitch.cpp b/syncd/SaiSwitch.cpp index 4f11dba4c..fb711db4d 100644 --- a/syncd/SaiSwitch.cpp +++ b/syncd/SaiSwitch.cpp @@ -26,14 +26,12 @@ SaiSwitch::SaiSwitch( _In_ std::shared_ptr client, _In_ std::shared_ptr translator, _In_ std::shared_ptr vendorSai, - _In_ bool warmBoot, - _In_ bool checkAttrVersion): + _In_ bool warmBoot): SaiSwitchInterface(switch_vid, switch_rid), m_vendorSai(vendorSai), m_warmBoot(warmBoot), m_translator(translator), - m_client(client), - m_checkAttrVersion(checkAttrVersion) + m_client(client) { SWSS_LOG_ENTER(); @@ -663,7 +661,7 @@ void SaiSwitch::helperDiscover() { SWSS_LOG_ENTER(); - SaiDiscovery sd(m_vendorSai, m_checkAttrVersion); + SaiDiscovery sd(m_vendorSai); m_discovered_rids = sd.discover(m_switch_rid); @@ -954,7 +952,7 @@ void SaiSwitch::onPostPortCreate( { SWSS_LOG_ENTER(); - SaiDiscovery sd(m_vendorSai, m_checkAttrVersion); + SaiDiscovery sd(m_vendorSai); auto discovered = sd.discover(port_rid); diff --git a/syncd/SaiSwitch.h b/syncd/SaiSwitch.h index 4c317678a..d6bccfeed 100644 --- a/syncd/SaiSwitch.h +++ b/syncd/SaiSwitch.h @@ -34,8 +34,7 @@ namespace syncd _In_ std::shared_ptr client, _In_ std::shared_ptr translator, _In_ std::shared_ptr vendorSai, - _In_ bool warmBoot, - _In_ bool checkAttrVersion); + _In_ bool warmBoot); virtual ~SaiSwitch() = default; @@ -354,7 +353,5 @@ namespace syncd std::shared_ptr m_translator; std::shared_ptr m_client; - - bool m_checkAttrVersion; }; } diff --git a/syncd/SingleReiniter.cpp b/syncd/SingleReiniter.cpp index 6844d3cf7..6f5355736 100644 --- a/syncd/SingleReiniter.cpp +++ b/syncd/SingleReiniter.cpp @@ -22,16 +22,14 @@ SingleReiniter::SingleReiniter( _In_ std::shared_ptr handler, _In_ const ObjectIdMap& vidToRidMap, _In_ const ObjectIdMap& ridToVidMap, - _In_ const std::vector& asicKeys, - _In_ bool checkAttrVersion): + _In_ const std::vector& asicKeys): m_vendorSai(sai), m_vidToRidMap(vidToRidMap), m_ridToVidMap(ridToVidMap), m_asicKeys(asicKeys), m_translator(translator), m_client(client), - m_handler(handler), - m_checkAttrVersion(checkAttrVersion) + m_handler(handler) { SWSS_LOG_ENTER(); @@ -319,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, false, m_checkAttrVersion); + 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/SingleReiniter.h b/syncd/SingleReiniter.h index b6a27eaab..e33b26af8 100644 --- a/syncd/SingleReiniter.h +++ b/syncd/SingleReiniter.h @@ -32,8 +32,7 @@ namespace syncd _In_ std::shared_ptr handler, _In_ const ObjectIdMap& vidToRidMap, _In_ const ObjectIdMap& ridToVidMap, - _In_ const std::vector& asicKeys, - _In_ bool checkAttrVersion); + _In_ const std::vector& asicKeys); virtual ~SingleReiniter(); @@ -137,7 +136,5 @@ namespace syncd std::shared_ptr m_client; std::shared_ptr m_handler; - - bool m_checkAttrVersion; }; } diff --git a/syncd/Syncd.cpp b/syncd/Syncd.cpp index 83f6b18ba..6c06a4fd5 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(); @@ -3136,7 +3143,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, false, m_commandLineOptions->m_enableAttrVersionCheck); + m_switches[switchVid] = std::make_shared(switchVid, objectRid, m_client, m_translator, m_vendorSai, false); m_mdioIpcServer->setSwitchId(objectRid); @@ -4353,7 +4360,7 @@ void Syncd::onSyncdStart( SWSS_LOG_THROW("performing hard reinit, but there are %zu switches defined, bug!", m_switches.size()); } - HardReiniter hr(m_client, m_translator, m_vendorSai, m_handler, m_commandLineOptions->m_enableAttrVersionCheck); + HardReiniter hr(m_client, m_translator, m_vendorSai, m_handler); m_switches = hr.hardReinit(); @@ -4455,7 +4462,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, false, m_commandLineOptions->m_enableAttrVersionCheck); + m_switches[switchVid] = std::make_shared(switchVid, switchRid, m_client, m_translator, m_vendorSai, false); m_mdioIpcServer->setSwitchId(switchRid); @@ -4639,7 +4646,7 @@ void Syncd::performWarmRestartSingleSwitch( // perform all get operations on existing switch - auto sw = m_switches[switchVid] = std::make_shared(switchVid, switchRid, m_client, m_translator, m_vendorSai, true, m_commandLineOptions->m_enableAttrVersionCheck); + auto sw = m_switches[switchVid] = std::make_shared(switchVid, switchRid, m_client, m_translator, m_vendorSai, true); startDiagShell(switchRid); } 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/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); +} From 79d6ae3bbfd5465d4975e74d6029f29cb2c86e33 Mon Sep 17 00:00:00 2001 From: kcudnik Date: Fri, 22 Nov 2024 19:04:26 +0100 Subject: [PATCH 2/2] Update aspell --- tests/aspell.en.pws | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/aspell.en.pws b/tests/aspell.en.pws index a51e8f4c6..4a99b8dec 100644 --- a/tests/aspell.en.pws +++ b/tests/aspell.en.pws @@ -479,3 +479,4 @@ submodule Enqueue deque apiversion +vso