[syncd] Introduce VendorSaiOptions class (sonic-net#1472)

And SaiOptions interface, for easy passing parameters over Sai objects

This will simplify passing parameters over different objects that have SaiInterface.

meta/SaiInterface.cpp

@@ -378,3 +378,20 @@ sai_status_t SaiInterface::clearStats(
 
     return SAI_STATUS_NOT_IMPLEMENTED;
 }
+
+std::shared_ptr<SaiOptions> 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<SaiOptions> options)
+{
+    SWSS_LOG_ENTER();
+
+    m_optionsMap[key] = options;
+}

meta/SaiInterface.h

@@ -5,6 +5,12 @@ extern "C" {
 #include "saimetadata.h"
 }
 
+#include "SaiOptions.h"
+
+#include <map>
+#include <memory>
+#include <string>
+
 #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<SaiOptions> getOptions(
+                    _In_ const std::string& key);
+
+            void setOptions(
+                    _In_ const std::string& key,
+                    _In_ std::shared_ptr<SaiOptions> options);
+
+        private:
+
+            std::map<std::string, std::shared_ptr<SaiOptions>> m_optionsMap;
     };
 }

meta/SaiOptions.h

@@ -0,0 +1,11 @@
+#pragma once
+
+namespace sairedis
+{
+    class SaiOptions
+    {
+        public:
+
+            virtual ~SaiOptions() = default;
+    };
+}

syncd/HardReiniter.cpp

@@ -13,13 +13,11 @@ HardReiniter::HardReiniter(
         _In_ std::shared_ptr<RedisClient> client,
         _In_ std::shared_ptr<VirtualOidTranslator> translator,
         _In_ std::shared_ptr<sairedis::SaiInterface> sai,
-        _In_ std::shared_ptr<NotificationHandler> handler,
-        _In_ bool checkAttrVersion):
+        _In_ std::shared_ptr<NotificationHandler> 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<sai_object_id_t, std::shared_ptr<syncd::SaiSwitch>> HardReiniter::hardR
                 m_handler,
                 m_switchVidToRid.at(kvp.first),
                 m_switchRidToVid.at(kvp.first),
-                kvp.second,
-                m_checkAttrVersion);
+                kvp.second);
 
         sr->hardReinit();
 

syncd/HardReiniter.h

@@ -27,8 +27,7 @@ namespace syncd
                     _In_ std::shared_ptr<RedisClient> client,
                     _In_ std::shared_ptr<VirtualOidTranslator> translator,
                     _In_ std::shared_ptr<sairedis::SaiInterface> sai,
-                    _In_ std::shared_ptr<NotificationHandler> handler,
-                    _In_ bool checkAttrVersion);
+                    _In_ std::shared_ptr<NotificationHandler> handler);
 
             virtual ~HardReiniter();
 
@@ -60,7 +59,5 @@ namespace syncd
             std::shared_ptr<RedisClient> m_client;
 
             std::shared_ptr<NotificationHandler> m_handler;
-
-            bool m_checkAttrVersion;
     };
 }

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<sairedis::SaiInterface> sai,
-        _In_ bool checkAttrVersion):
+        _In_ std::shared_ptr<sairedis::SaiInterface> 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<VendorSaiOptions>(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
     {

syncd/SaiDiscovery.h

@@ -20,8 +20,7 @@ namespace syncd
         public:
 
             SaiDiscovery(
-                    _In_ std::shared_ptr<sairedis::SaiInterface> sai,
-                    _In_ bool checkAttrVersion);
+                    _In_ std::shared_ptr<sairedis::SaiInterface> sai);
 
             virtual ~SaiDiscovery();
 

syncd/SaiSwitch.cpp

@@ -26,14 +26,12 @@ SaiSwitch::SaiSwitch(
         _In_ std::shared_ptr<RedisClient> client,
         _In_ std::shared_ptr<VirtualOidTranslator> translator,
         _In_ std::shared_ptr<sairedis::SaiInterface> 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);
 

syncd/SaiSwitch.h

@@ -34,8 +34,7 @@ namespace syncd
                     _In_ std::shared_ptr<RedisClient> client,
                     _In_ std::shared_ptr<VirtualOidTranslator> translator,
                     _In_ std::shared_ptr<sairedis::SaiInterface> vendorSai,
-                    _In_ bool warmBoot,
-                    _In_ bool checkAttrVersion);
+                    _In_ bool warmBoot);
 
             virtual ~SaiSwitch() = default;
 
@@ -354,7 +353,5 @@ namespace syncd
             std::shared_ptr<VirtualOidTranslator> m_translator;
 
             std::shared_ptr<RedisClient> m_client;
-
-            bool m_checkAttrVersion;
     };
 }

syncd/SingleReiniter.cpp

@@ -22,16 +22,14 @@ SingleReiniter::SingleReiniter(
         _In_ std::shared_ptr<NotificationHandler> handler,
         _In_ const ObjectIdMap& vidToRidMap,
         _In_ const ObjectIdMap& ridToVidMap,
-        _In_ const std::vector<std::string>& asicKeys,
-        _In_ bool checkAttrVersion):
+        _In_ const std::vector<std::string>& 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<SaiSwitch>(m_switch_vid, m_switch_rid, m_client, m_translator, m_vendorSai, false, m_checkAttrVersion);
+        m_sw = std::make_shared<SaiSwitch>(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

syncd/SingleReiniter.h

@@ -32,8 +32,7 @@ namespace syncd
                     _In_ std::shared_ptr<NotificationHandler> handler,
                     _In_ const ObjectIdMap& vidToRidMap,
                     _In_ const ObjectIdMap& ridToVidMap,
-                    _In_ const std::vector<std::string>& asicKeys,
-                    _In_ bool checkAttrVersion);
+                    _In_ const std::vector<std::string>& asicKeys);
 
             virtual ~SingleReiniter();
 
@@ -137,7 +136,5 @@ namespace syncd
             std::shared_ptr<RedisClient> m_client;
 
             std::shared_ptr<NotificationHandler> m_handler;
-
-            bool m_checkAttrVersion;
     };
 }

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<VendorSaiOptions>();
+
+    vso->m_checkAttrVersion = m_commandLineOptions->m_enableAttrVersionCheck;
+
+    m_vendorSai->setOptions(VendorSaiOptions::OPTIONS_KEY, vso);
+
     m_manager = std::make_shared<FlexCounterManager>(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<SaiSwitch>(switchVid, objectRid, m_client, m_translator, m_vendorSai, false, m_commandLineOptions->m_enableAttrVersionCheck);
+            m_switches[switchVid] = std::make_shared<SaiSwitch>(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<SaiSwitch>(switchVid, switchRid, m_client, m_translator, m_vendorSai, false, m_commandLineOptions->m_enableAttrVersionCheck);
+        m_switches[switchVid] = std::make_shared<SaiSwitch>(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<SaiSwitch>(switchVid, switchRid, m_client, m_translator, m_vendorSai, true, m_commandLineOptions->m_enableAttrVersionCheck);
+    auto sw = m_switches[switchVid] = std::make_shared<SaiSwitch>(switchVid, switchRid, m_client, m_translator, m_vendorSai, true);
 
     startDiagShell(switchRid);
 }

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;
+    };
+}

tests/aspell.en.pws

@@ -479,3 +479,4 @@ submodule
 Enqueue
 deque
 apiversion
+vso

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<Opt>());
+}
+
+TEST(SaiInterface, getOptions)
+{
+    DummySaiInterface ds;
+
+    auto opt = std::make_shared<Opt>();
+
+    opt->i = 42;
+
+    ds.setOptions("key", opt);
+
+    auto o = std::dynamic_pointer_cast<Opt>(ds.getOptions("key"));
+
+    EXPECT_NE(o, nullptr);
+
+    EXPECT_EQ(o->i, 42);
+
+    EXPECT_EQ(ds.getOptions("foo"), nullptr);
+}