Skip to content

Commit d624e47

Browse files
stepanblyschaklukasstockner
authored andcommitted
[portsorch] remove port OID from saiOidToAlias map on port deletion (sonic-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]>
1 parent cf7cf50 commit d624e47

File tree

2 files changed

+11
-7
lines changed

2 files changed

+11
-7
lines changed

orchagent/portsorch.cpp

+6-2
Original file line numberDiff line numberDiff line change
@@ -753,7 +753,10 @@ bool PortsOrch::getPort(sai_object_id_t id, Port &port)
753753
}
754754
else
755755
{
756-
getPort(itr->second, port);
756+
if (!getPort(itr->second, port))
757+
{
758+
SWSS_LOG_THROW("Inconsistent saiOidToAlias map and m_portList map: oid=%" PRIx64, id);
759+
}
757760
return true;
758761
}
759762

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

33313334
/* Delete port from port list */
33323335
m_portList.erase(alias);
3336+
saiOidToAlias.erase(port_id);
33333337
}
33343338
else
33353339
{
@@ -5630,7 +5634,7 @@ void PortsOrch::doTask(NotificationConsumer &consumer)
56305634

56315635
if (!getPort(id, port))
56325636
{
5633-
SWSS_LOG_ERROR("Failed to get port object for port id 0x%" PRIx64, id);
5637+
SWSS_LOG_NOTICE("Got port state change for port id 0x%" PRIx64 " which does not exist, possibly outdated event", id);
56345638
continue;
56355639
}
56365640

tests/mock_tests/portsorch_ut.cpp

+5-5
Original file line numberDiff line numberDiff line change
@@ -862,7 +862,7 @@ namespace portsorch_test
862862
* updated to DB.
863863
*/
864864
TEST_F(PortsOrchTest, PortOperStatusIsUpAndOperSpeedIsZero)
865-
{
865+
{
866866
Table portTable = Table(m_app_db.get(), APP_PORT_TABLE_NAME);
867867

868868
// Get SAI default ports to populate DB
@@ -887,7 +887,7 @@ namespace portsorch_test
887887
Port port;
888888
gPortsOrch->getPort("Ethernet0", port);
889889
ASSERT_TRUE(port.m_oper_status != SAI_PORT_OPER_STATUS_UP);
890-
890+
891891
// save original api since we will spy
892892
auto orig_port_api = sai_port_api;
893893
sai_port_api = new sai_port_api_t();
@@ -905,14 +905,14 @@ namespace portsorch_test
905905
// Return 0 for port operational speed
906906
attrs[0].value.u32 = 0;
907907
}
908-
908+
909909
return (sai_status_t)SAI_STATUS_SUCCESS;
910910
}
911911
);
912912

913913
auto exec = static_cast<Notifier *>(gPortsOrch->getExecutor("PORT_STATUS_NOTIFICATIONS"));
914914
auto consumer = exec->getNotificationConsumer();
915-
915+
916916
// mock a redis reply for notification, it notifies that Ehernet0 is going to up
917917
mockReply = (redisReply *)calloc(sizeof(redisReply), 1);
918918
mockReply->type = REDIS_REPLY_ARRAY;
@@ -934,7 +934,7 @@ namespace portsorch_test
934934
// trigger the notification
935935
consumer->readData();
936936
gPortsOrch->doTask(*consumer);
937-
mockReply = nullptr;
937+
mockReply = nullptr;
938938

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

0 commit comments

Comments
 (0)