Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[syncd] Add attribute version check feature #1470

Merged
merged 1 commit into from
Nov 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading