Skip to content

Commit

Permalink
[portsorch] remove port OID from saiOidToAlias map on port deletion (s…
Browse files Browse the repository at this point in the history
…onic-net#2483)

- What I did

I fixed an issue that on port deletion (port breakout scenario), the port OID is not removed from saiOidToAlias map, resulting in getPort returns true when querying non-existing port OID but the Port structure is not filled with correct values. Also lowered the log level on receiving non-existing port operational status update

- Why I did it

To fix errors in the log during breakout:

Oct  4 13:15:45.654396 r-bulldog-04 NOTICE swss#orchagent: :- updatePortOperStatus: Port  oper state set from unknown to down
Oct  4 13:15:45.654773 r-bulldog-04 ERR swss#orchagent: :- set: switch id oid:0x0 doesn't exist
Oct  4 13:15:45.654773 r-bulldog-04 WARNING swss#orchagent: :- setHostIntfsOperStatus: Failed to set operation status DOWN to host interface
Oct  4 13:15:45.654773 r-bulldog-04 ERR swss#orchagent: :- updatePortOperStatus: Failed to set host interface  operational status down
Oct  4 13:15:45.654773 r-bulldog-04 WARNING swss#orchagent: :- flushFDBEntries: Couldn't flush FDB. Bridge port OID: 0x0 bvid:0,

- How I verified it
Run UT, run manual port breakout tests.

Signed-off-by: Stepan Blyschak <[email protected]>
  • Loading branch information
stepanblyschak authored and lukasstockner committed Mar 2, 2023
1 parent cf7cf50 commit d624e47
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 7 deletions.
8 changes: 6 additions & 2 deletions orchagent/portsorch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -753,7 +753,10 @@ bool PortsOrch::getPort(sai_object_id_t id, Port &port)
}
else
{
getPort(itr->second, port);
if (!getPort(itr->second, port))
{
SWSS_LOG_THROW("Inconsistent saiOidToAlias map and m_portList map: oid=%" PRIx64, id);
}
return true;
}

Expand Down Expand Up @@ -3330,6 +3333,7 @@ void PortsOrch::doPortTask(Consumer &consumer)

/* Delete port from port list */
m_portList.erase(alias);
saiOidToAlias.erase(port_id);
}
else
{
Expand Down Expand Up @@ -5630,7 +5634,7 @@ void PortsOrch::doTask(NotificationConsumer &consumer)

if (!getPort(id, port))
{
SWSS_LOG_ERROR("Failed to get port object for port id 0x%" PRIx64, id);
SWSS_LOG_NOTICE("Got port state change for port id 0x%" PRIx64 " which does not exist, possibly outdated event", id);
continue;
}

Expand Down
10 changes: 5 additions & 5 deletions tests/mock_tests/portsorch_ut.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -862,7 +862,7 @@ namespace portsorch_test
* updated to DB.
*/
TEST_F(PortsOrchTest, PortOperStatusIsUpAndOperSpeedIsZero)
{
{
Table portTable = Table(m_app_db.get(), APP_PORT_TABLE_NAME);

// Get SAI default ports to populate DB
Expand All @@ -887,7 +887,7 @@ namespace portsorch_test
Port port;
gPortsOrch->getPort("Ethernet0", port);
ASSERT_TRUE(port.m_oper_status != SAI_PORT_OPER_STATUS_UP);

// save original api since we will spy
auto orig_port_api = sai_port_api;
sai_port_api = new sai_port_api_t();
Expand All @@ -905,14 +905,14 @@ namespace portsorch_test
// Return 0 for port operational speed
attrs[0].value.u32 = 0;
}

return (sai_status_t)SAI_STATUS_SUCCESS;
}
);

auto exec = static_cast<Notifier *>(gPortsOrch->getExecutor("PORT_STATUS_NOTIFICATIONS"));
auto consumer = exec->getNotificationConsumer();

// mock a redis reply for notification, it notifies that Ehernet0 is going to up
mockReply = (redisReply *)calloc(sizeof(redisReply), 1);
mockReply->type = REDIS_REPLY_ARRAY;
Expand All @@ -934,7 +934,7 @@ namespace portsorch_test
// trigger the notification
consumer->readData();
gPortsOrch->doTask(*consumer);
mockReply = nullptr;
mockReply = nullptr;

gPortsOrch->getPort("Ethernet0", port);
ASSERT_TRUE(port.m_oper_status == SAI_PORT_OPER_STATUS_UP);
Expand Down

0 comments on commit d624e47

Please sign in to comment.