Skip to content

Commit

Permalink
[syncd] Add attribute version check feature
Browse files Browse the repository at this point in the history
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
  • Loading branch information
kcudnik committed Nov 21, 2024
1 parent 29a8f86 commit b348e00
Show file tree
Hide file tree
Showing 20 changed files with 371 additions and 28 deletions.
107 changes: 107 additions & 0 deletions syncd/AttrVersionChecker.cpp
Original file line number Diff line number Diff line change
@@ -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;
}
40 changes: 40 additions & 0 deletions syncd/AttrVersionChecker.h
Original file line number Diff line number Diff line change
@@ -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;
};
}
4 changes: 4 additions & 0 deletions syncd/CommandLineOptions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@ CommandLineOptions::CommandLineOptions()

#endif // SAITHRIFT

m_supportingBulkCounterGroups = "";

m_enableAttrVersionCheck = false;
}

std::string CommandLineOptions::getCommandLineString() const
Expand All @@ -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

Expand Down
1 change: 1 addition & 0 deletions syncd/CommandLineOptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -100,5 +100,6 @@ namespace syncd

std::string m_supportingBulkCounterGroups;

bool m_enableAttrVersionCheck;
};
}
11 changes: 9 additions & 2 deletions syncd/CommandLineOptionsParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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' },
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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

Expand Down
9 changes: 6 additions & 3 deletions syncd/HardReiniter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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();

Expand Down
5 changes: 4 additions & 1 deletion syncd/HardReiniter.h
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -59,5 +60,7 @@ namespace syncd
std::shared_ptr<RedisClient> m_client;

std::shared_ptr<NotificationHandler> m_handler;

bool m_checkAttrVersion;
};
}
1 change: 1 addition & 0 deletions syncd/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand Down
30 changes: 28 additions & 2 deletions syncd/SaiDiscovery.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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;

{
Expand Down
7 changes: 6 additions & 1 deletion syncd/SaiDiscovery.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

#include "meta/SaiInterface.h"

#include "AttrVersionChecker.h"

#include <memory>
#include <set>
#include <map>
Expand All @@ -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();

Expand Down Expand Up @@ -61,5 +64,7 @@ namespace syncd
std::shared_ptr<sairedis::SaiInterface> m_sai;

DefaultOidMap m_defaultOidMap;

AttrVersionChecker m_attrVersionChecker;
};
}
10 changes: 6 additions & 4 deletions syncd/SaiSwitch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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);

Expand Down
Loading

0 comments on commit b348e00

Please sign in to comment.