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

[neighorch] Replace hasNextHop() asserts with .at() #1894

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

[neighorch] Replace hasNextHop() asserts with .at() #1894

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2f4dfa6
[neighorch] Replace hasNextHop() asserts with .at()
abanu-ms Aug 24, 2021
0c14bcc
[neighorch] Replace hasNextHop asserts with checks
abanu-ms Sep 6, 2021
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
46 changes: 38 additions & 8 deletions orchagent/neighorch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ NeighOrch::NeighOrch(DBConnector *appDb, string tableName, IntfsOrch *intfsOrch,
SWSS_LOG_ENTER();

m_fdbOrch->attach(this);

if(gMySwitchType == "voq")
{
//Add subscriber to process VOQ system neigh
Expand Down Expand Up @@ -428,7 +428,13 @@ bool NeighOrch::removeNextHop(const IpAddress &ipAddress, const string &alias)
nexthop.alias = inbp.m_alias;
}

assert(hasNextHop(nexthop));
// If the next hop does not exist, log an error as the code is wrongly trying to remove a NH and return true as it
// can be considered that the NH does not exist anymore.
if (!hasNextHop(nexthop))
{
SWSS_LOG_ERROR("Trying to remove next hop %s which does not exist", nexthop.to_string().c_str());
return true;
}

gFgNhgOrch->invalidNextHopInNextHopGroup(nexthop);

Expand Down Expand Up @@ -459,7 +465,13 @@ bool NeighOrch::removeMplsNextHop(const NextHopKey& nh)
nexthop.alias = inbp.m_alias;
}

assert(hasNextHop(nexthop));
// If the next hop does not exist, log an error as the code is wrongly trying to remove a NH and return true as it
// can be considered that the NH does not exist anymore.
if (!hasNextHop(nexthop))
{
SWSS_LOG_ERROR("Trying to remove next hop %s which does not exist", nexthop.to_string().c_str());
return true;
}

SWSS_LOG_INFO("Removing next hop %s", nexthop.to_string().c_str());

Expand Down Expand Up @@ -516,7 +528,13 @@ bool NeighOrch::removeOverlayNextHop(const NextHopKey &nexthop)
{
SWSS_LOG_ENTER();

assert(hasNextHop(nexthop));
// If the next hop does not exist, log an error as the code is wrongly trying to remove a NH and return true as it
// can be considered that the NH does not exist anymore.
if (!hasNextHop(nexthop))
{
SWSS_LOG_ERROR("Trying to remove next hop %s which does not exist", nexthop.to_string().c_str());
return true;
}

if (m_syncdNextHops[nexthop].ref_count > 0)
{
Expand All @@ -541,7 +559,13 @@ sai_object_id_t NeighOrch::getLocalNextHopId(const NextHopKey& nexthop)

sai_object_id_t NeighOrch::getNextHopId(const NextHopKey &nexthop)
{
assert(hasNextHop(nexthop));
// If the next hop does not exist, log an error as the code is wrongly trying to get the NH ID of a next hop that
// does not exist and return SAI_NULL_OBJECT_ID.
if (!hasNextHop(nexthop))
{
SWSS_LOG_ERROR("Trying to get next hop ID of %s which does not exist", nexthop.to_string().c_str());
return SAI_NULL_OBJECT_ID;
}

/*
* The nexthop id could be varying depending on the use-case
Expand All @@ -559,7 +583,13 @@ sai_object_id_t NeighOrch::getNextHopId(const NextHopKey &nexthop)

int NeighOrch::getNextHopRefCount(const NextHopKey &nexthop)
{
assert(hasNextHop(nexthop));
// If the next hop does not exist, log an error as the code is wrongly trying to get the ref count of a next hop
// that does not exist and return 0.
if (!hasNextHop(nexthop))
{
SWSS_LOG_ERROR("Trying to get next hop %s ref count which does not exist", nexthop.to_string().c_str());
return 0;
}
return m_syncdNextHops[nexthop].ref_count;
}

Expand Down Expand Up @@ -1024,7 +1054,7 @@ bool NeighOrch::removeNeighbor(const NeighborEntry &neighborEntry, bool disable)

NeighborUpdate update = { neighborEntry, MacAddress(), false };
notify(SUBJECT_TYPE_NEIGH_CHANGE, static_cast<void *>(&update));

if(gMySwitchType == "voq")
{
//Sync the neighbor to delete from the CHASSIS_APP_DB
Expand Down Expand Up @@ -1298,7 +1328,7 @@ void NeighOrch::doVoqSystemNeighTask(Consumer &consumer)
// the owner asic's mac is not readily avaiable here, the owner asic mac is derived from
// the switch id and lower 5 bytes of asic mac which is assumed to be same for all asics
// in the VS system.
// Therefore to make VOQ chassis systems work in VS platform based setups like the setups
// Therefore to make VOQ chassis systems work in VS platform based setups like the setups
// using KVMs, it is required that all asics have same base mac in the format given below
// <lower 5 bytes of mac same for all asics>:<6th byte = switch_id>

Expand Down