[syncd] Add attribute version check feature (sonic-net#1470)

By default disabled. If enabled, it will use SAI metadata and libsai
version returned by sai_api_version_query to verify if given attribute
during SAI discovery process was introduced in the current libsai
version or below that version.

This feature is partial solution to this problem:
sonic-net/sonic-buildimage#20725,
more here: opencomputeproject/SAI#2099

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

syncd/AttrVersionChecker.h

@@ -0,0 +1,40 @@
+#pragma once
+
+extern "C" {
+#include "sai.h"
+#include "saimetadata.h"
+}
+
+#include <set>
+#include <string>
+
+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<std::string> m_visitedAttributes;
+    };
+}

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
 

syncd/CommandLineOptions.h

@@ -100,5 +100,6 @@ namespace syncd
 
             std::string m_supportingBulkCounterGroups;
 
+            bool m_enableAttrVersionCheck;
     };
 }

syncd/CommandLineOptionsParser.cpp

@@ -19,9 +19,9 @@ std::shared_ptr<CommandLineOptions> CommandLineOptionsParser::parseCommandLine(
     auto options = std::make_shared<CommandLineOptions>();
 
 #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<CommandLineOptions> 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<CommandLineOptions> 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
 

syncd/HardReiniter.cpp

@@ -13,11 +13,13 @@ 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_ std::shared_ptr<NotificationHandler> handler,
+        _In_ bool checkAttrVersion):
     m_vendorSai(sai),
     m_translator(translator),
     m_client(client),
-    m_handler(handler)
+    m_handler(handler),
+    m_checkAttrVersion(checkAttrVersion)
 {
     SWSS_LOG_ENTER();
 
@@ -99,7 +101,8 @@ 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);
+                kvp.second,
+                m_checkAttrVersion);
 
         sr->hardReinit();
 

syncd/HardReiniter.h

@@ -27,7 +27,8 @@ 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_ std::shared_ptr<NotificationHandler> handler,
+                    _In_ bool checkAttrVersion);
 
             virtual ~HardReiniter();
 
@@ -59,5 +60,7 @@ namespace syncd
             std::shared_ptr<RedisClient> m_client;
 
             std::shared_ptr<NotificationHandler> m_handler;
+
+            bool m_checkAttrVersion;
     };
 }

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 \

syncd/SaiDiscovery.cpp

@@ -19,12 +19,31 @@ using namespace syncd;
 #define SAI_DISCOVERY_LIST_MAX_ELEMENTS 1024
 
 SaiDiscovery::SaiDiscovery(
-        _In_ std::shared_ptr<sairedis::SaiInterface> sai):
+        _In_ std::shared_ptr<sairedis::SaiInterface> sai,
+        _In_ bool checkAttrVersion):
     m_sai(sai)
 {
     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)
+    {
+        m_attrVersionChecker.enable(checkAttrVersion);
+        m_attrVersionChecker.setSaiApiVersion(version);
+
+        SWSS_LOG_NOTICE("check attr version ENABLED, libsai api version: %lu", 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 +129,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 +283,8 @@ std::set<sai_object_id_t> SaiDiscovery::discover(
 
     m_defaultOidMap.clear();
 
+    m_attrVersionChecker.reset();
+
     std::set<sai_object_id_t> discovered_rids;
 
     {

syncd/SaiDiscovery.h

@@ -2,6 +2,8 @@
 
 #include "meta/SaiInterface.h"
 
+#include "AttrVersionChecker.h"
+
 #include <memory>
 #include <set>
 #include <map>
@@ -18,7 +20,8 @@ namespace syncd
         public:
 
             SaiDiscovery(
-                    _In_ std::shared_ptr<sairedis::SaiInterface> sai);
+                    _In_ std::shared_ptr<sairedis::SaiInterface> sai,
+                    _In_ bool checkAttrVersion);
 
             virtual ~SaiDiscovery();
 
@@ -61,5 +64,7 @@ namespace syncd
             std::shared_ptr<sairedis::SaiInterface> m_sai;
 
             DefaultOidMap m_defaultOidMap;
+
+            AttrVersionChecker m_attrVersionChecker;
     };
 }

syncd/SaiSwitch.cpp

@@ -26,12 +26,14 @@ 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 warmBoot,
+        _In_ bool checkAttrVersion):
     SaiSwitchInterface(switch_vid, switch_rid),
     m_vendorSai(vendorSai),
     m_warmBoot(warmBoot),
     m_translator(translator),
-    m_client(client)
+    m_client(client),
+    m_checkAttrVersion(checkAttrVersion)
 {
     SWSS_LOG_ENTER();
 
@@ -661,7 +663,7 @@ void SaiSwitch::helperDiscover()
 {
     SWSS_LOG_ENTER();
 
-    SaiDiscovery sd(m_vendorSai);
+    SaiDiscovery sd(m_vendorSai, m_checkAttrVersion);
 
     m_discovered_rids = sd.discover(m_switch_rid);
 
@@ -952,7 +954,7 @@ void SaiSwitch::onPostPortCreate(
 {
     SWSS_LOG_ENTER();
 
-    SaiDiscovery sd(m_vendorSai);
+    SaiDiscovery sd(m_vendorSai, m_checkAttrVersion);
 
     auto discovered = sd.discover(port_rid);