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.

Sign up for GitHub

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

Fix for credo gbsyncd warm-reboot #1499

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Fix for credo gbsyncd warm-reboot #1499

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
39763c5
Add more attributes the gbsyncd warm-boot
byu343 Dec 10, 2024
b7c87c9
Fix determining first run on gbsyncd warm-reboot
byu343 Dec 10, 2024
c5cb821
Skip the steps related to SAI_SWITCH_ATTR_DEFAULT_TRAP_GROUP when not…
byu343 Dec 10, 2024
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
23 changes: 20 additions & 3 deletions syncd/ComparisonLogic.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,26 @@ ComparisonLogic::ComparisonLogic(

// TODO needs to be removed and done in generic

m_current->m_defaultTrapGroupRid = m_switch->getSwitchDefaultAttrOid(SAI_SWITCH_ATTR_DEFAULT_TRAP_GROUP);
m_temp->m_defaultTrapGroupRid = m_switch->getSwitchDefaultAttrOid(SAI_SWITCH_ATTR_DEFAULT_TRAP_GROUP);
try
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why tis is modified?

{
m_current->m_defaultTrapGroupRid = m_switch->getSwitchDefaultAttrOid(SAI_SWITCH_ATTR_DEFAULT_TRAP_GROUP);
}
catch(const std::exception &e)
{
SWSS_LOG_WARN("Failed to to obtain SAI_SWITCH_ATTR_DEFAULT_TRAP_GROUP in current view");
m_current->m_defaultTrapGroupRid = SAI_NULL_OBJECT_ID;
}

try
{
m_temp->m_defaultTrapGroupRid = m_switch->getSwitchDefaultAttrOid(SAI_SWITCH_ATTR_DEFAULT_TRAP_GROUP);
}
catch(const std::exception &e)
{
SWSS_LOG_WARN("Failed to to obtain SAI_SWITCH_ATTR_DEFAULT_TRAP_GROUP in temp view");
m_temp->m_defaultTrapGroupRid = SAI_NULL_OBJECT_ID;

}
auto seed = (unsigned int)std::time(0);

SWSS_LOG_NOTICE("srand seed for switch %s: %u", sai_serialize_object_id(m_switch->getVid()).c_str(), seed);
Expand Down Expand Up @@ -2272,7 +2289,7 @@ void ComparisonLogic::bringDefaultTrapGroupToFinalState(

sai_object_id_t rid = currentView.m_defaultTrapGroupRid;

if (temporaryView.hasRid(rid))
if (rid == SAI_NULL_OBJECT_ID || temporaryView.hasRid(rid))
{
/*
* Default trap group is defined inside temporary view
Expand Down
9 changes: 9 additions & 0 deletions syncd/RedisClient.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -691,6 +691,15 @@ bool RedisClient::hasNoHiddenKeysDefined() const
return keys.size() == 0;
}

bool RedisClient::hasNoSwitchDefined() const
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why there would be no switch? What is purpose of this? You have flag isveryfirstrun

{
SWSS_LOG_ENTER();

auto keys = m_dbAsic->keys("ASIC_STATE:SAI_OBJECT_TYPE_SWITCH:*");

return keys.size() == 0;
}

void RedisClient::removeVidAndRid(
_In_ sai_object_id_t vid,
_In_ sai_object_id_t rid)
Expand Down
1 change: 1 addition & 0 deletions syncd/RedisClient.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ namespace syncd
_In_ const std::string& key) const;

bool hasNoHiddenKeysDefined() const;
bool hasNoSwitchDefined() const;

void removeVidAndRid(
_In_ sai_object_id_t vid,
Expand Down
15 changes: 12 additions & 3 deletions syncd/Syncd.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4645,7 +4645,16 @@ void Syncd::performWarmRestartSingleSwitch(
sai_attribute_t *attrList = list.get_attr_list();

uint32_t attrCount = list.get_attr_count();

bool isPhy = false;
for (uint32_t idx = 0; idx < attrCount; idx++)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a function for that in sai utils

{
auto id = attrList[idx].id;
if (id == SAI_SWITCH_ATTR_TYPE && attrList[idx].value.s32 == SAI_SWITCH_TYPE_PHY)
{
isPhy = true;
break;
}
}
for (uint32_t idx = 0; idx < attrCount; idx++)
{
auto id = attrList[idx].id;
Expand All @@ -4662,7 +4671,7 @@ void Syncd::performWarmRestartSingleSwitch(
* new process could be loaded at different address space.
*/

if (id == SAI_SWITCH_ATTR_SWITCH_HARDWARE_INFO || meta->attrvaluetype == SAI_ATTR_VALUE_TYPE_POINTER)
if (isPhy || id == SAI_SWITCH_ATTR_SWITCH_HARDWARE_INFO || meta->attrvaluetype == SAI_ATTR_VALUE_TYPE_POINTER)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can pass all attributes, look at my comment in main thread

{
attrs.push_back(attrList[idx]);
continue;
Expand Down Expand Up @@ -5103,7 +5112,7 @@ bool Syncd::isVeryFirstRun()
* this is first run, let's query HIDDEN ?
*/

bool firstRun = m_client->hasNoHiddenKeysDefined();
bool firstRun = m_client->hasNoHiddenKeysDefined() && m_client->hasNoSwitchDefined();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You already have flag isveryfirst run, why this is modified?


SWSS_LOG_NOTICE("First Run: %s", firstRun ? "True" : "False");

Expand Down
Loading