Skip to content

Commit 47df807

Browse files
[portsorch] Fixed port comparison logic executed every doTask for every port (#3821)
* [portsorch] Fixed port comparison logic executed every doTask for every port What I did: - Removed TODO comment about N² loop issue that was causing initPort() to be called multiple times - Fixed state checking logic to use explicit PORT_CONFIG_RECEIVED instead of != PORT_CONFIG_MISSING - Added proper handling for PORT_CONFIG_DONE state to prevent duplicate port initialization - Added error handling for port addition failures in the new state handling block Why I did it: initPort is beeing called 36138312 times on a system with 514 ports during cold boot and shows up in profilers. This is because the port comparison logic is invoked every port update and loops for every port. For every port that hasn't changed its lanes initPort is called. This is an N^2 loop and causes inefficient PORT table handling in cold boot. Saved around ~6 sec in cold boot downtime test with this change. Testing: - Verified port initialization works correctly in both initial and runtime port split cases. - Confirmed no duplicate port initialization occurs
1 parent 0c60bdb commit 47df807

File tree

1 file changed

+21
-9
lines changed

1 file changed

+21
-9
lines changed

orchagent/portsorch.cpp

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4314,7 +4314,8 @@ void PortsOrch::doPortTask(Consumer &consumer)
43144314
return true;
43154315
};
43164316

4317-
if (m_portList.find(key) == m_portList.end())
4317+
const bool portExists = m_portList.count(key) > 0;
4318+
if (!portExists)
43184319
{
43194320
// Aggregate configuration while the port is not created.
43204321
auto &fvMap = m_portConfigMap[key];
@@ -4346,18 +4347,12 @@ void PortsOrch::doPortTask(Consumer &consumer)
43464347
}
43474348
}
43484349

4349-
// TODO:
4350-
// Fix the issue below
4351-
// After PortConfigDone, while waiting for "PortInitDone" and the first gBufferOrch->isPortReady(alias),
4352-
// the complete m_lanesAliasSpeedMap may be populated again, so initExistingPort() will be called more than once
4353-
// for the same port.
4354-
43554350
/* Once all ports received, go through the each port and perform appropriate actions:
43564351
* 1. Remove ports which don't exist anymore
43574352
* 2. Create new ports
43584353
* 3. Initialize all ports
43594354
*/
4360-
if (getPortConfigState() != PORT_CONFIG_MISSING)
4355+
if (getPortConfigState() == PORT_CONFIG_RECEIVED)
43614356
{
43624357
std::vector<PortConfig> portsToAddList;
43634358
std::vector<sai_object_id_t> portsToRemoveList;
@@ -4418,8 +4413,25 @@ void PortsOrch::doPortTask(Consumer &consumer)
44184413

44194414
setPortConfigState(PORT_CONFIG_DONE);
44204415
}
4416+
else if (getPortConfigState() == PORT_CONFIG_DONE)
4417+
{
4418+
// Add and initialize the port
4419+
if (!portExists)
4420+
{
4421+
std::vector<PortConfig> portsToAddList { pCfg };
4422+
std::vector<Port> addedPorts;
44214423

4422-
if (getPortConfigState() != PORT_CONFIG_DONE)
4424+
if (!addPortBulk(portsToAddList, addedPorts))
4425+
{
4426+
SWSS_LOG_ERROR("Failed to add port %s", pCfg.key.c_str());
4427+
it++;
4428+
continue;
4429+
}
4430+
4431+
initPortsBulk(addedPorts);
4432+
}
4433+
}
4434+
else
44234435
{
44244436
// Not yet receive PortConfigDone. Save it for future retry
44254437
it++;

0 commit comments

Comments
 (0)