diff --git a/orchagent/bufferorch.cpp b/orchagent/bufferorch.cpp index 2257e927bd4..20944304b80 100644 --- a/orchagent/bufferorch.cpp +++ b/orchagent/bufferorch.cpp @@ -830,7 +830,7 @@ void BufferOrch::doTask(Consumer &consumer) { SWSS_LOG_ENTER(); - if (!gPortsOrch->isInitDone()) + if (!gPortsOrch->isConfigDone()) { return; } diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index ae876467fb2..03893214a4b 100644 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -394,12 +394,23 @@ bool PortsOrch::allPortsReady() return m_initDone && m_pendingPortSet.empty(); } -/* Upon receiving PortInitDone, all the configured ports have been created*/ +/* Upon receiving PortInitDone, all the configured ports have been created in both hardware and kernel*/ bool PortsOrch::isInitDone() { return m_initDone; } +// Upon m_portConfigState transiting to PORT_CONFIG_DONE state, all physical ports have been "created" in hardware. +// Because of the asynchronous nature of sairedis calls, "create" in the strict sense means that the SAI create_port() +// function is called and the create port event has been pushed to the sairedis pipeline. Because sairedis pipeline +// preserves the order of the events received, any event that depends on the physical port being created first, e.g., +// buffer profile apply, will be popped in the FIFO fashion, processed in the right order after the physical port is +// physically created in the ASIC, and thus can be issued safely when this function call returns true. +bool PortsOrch::isConfigDone() +{ + return m_portConfigState == PORT_CONFIG_DONE; +} + bool PortsOrch::isPortAdminUp(const string &alias) { auto it = m_portList.find(alias); @@ -1503,14 +1514,14 @@ void PortsOrch::doPortTask(Consumer &consumer) if (alias == "PortConfigDone") { - if (m_portConfigDone) + if (m_portConfigState != PORT_CONFIG_MISSING) { - // Already done, ignore this task + // Already received, ignore this task it = consumer.m_toSync.erase(it); continue; } - m_portConfigDone = true; + m_portConfigState = PORT_CONFIG_RECEIVED; for (auto i : kfvFieldsValues(t)) { @@ -1642,7 +1653,7 @@ void PortsOrch::doPortTask(Consumer &consumer) * 2. Create new ports * 3. Initialize all ports */ - if (m_portConfigDone && (m_lanesAliasSpeedMap.size() == m_portCount)) + if (m_portConfigState == PORT_CONFIG_RECEIVED && (m_lanesAliasSpeedMap.size() == m_portCount)) { for (auto it = m_portListLaneMap.begin(); it != m_portListLaneMap.end();) { @@ -1687,9 +1698,11 @@ void PortsOrch::doPortTask(Consumer &consumer) it = m_lanesAliasSpeedMap.erase(it); } + + m_portConfigState = PORT_CONFIG_DONE; } - if (!m_portConfigDone) + if (m_portConfigState != PORT_CONFIG_DONE) { // Not yet receive PortConfigDone. Save it for future retry it++; diff --git a/orchagent/portsorch.h b/orchagent/portsorch.h index 6b12dd24fb0..75831fdaf9e 100644 --- a/orchagent/portsorch.h +++ b/orchagent/portsorch.h @@ -57,6 +57,7 @@ class PortsOrch : public Orch, public Subject bool allPortsReady(); bool isInitDone(); + bool isConfigDone(); bool isPortAdminUp(const string &alias); map& getAllPorts(); @@ -116,7 +117,14 @@ class PortsOrch : public Orch, public Subject sai_object_id_t m_default1QBridge; sai_object_id_t m_defaultVlan; - bool m_portConfigDone = false; + typedef enum + { + PORT_CONFIG_MISSING, + PORT_CONFIG_RECEIVED, + PORT_CONFIG_DONE, + } port_config_state_t; + + port_config_state_t m_portConfigState = PORT_CONFIG_MISSING; sai_uint32_t m_portCount; map, sai_object_id_t> m_portListLaneMap; map, tuple> m_lanesAliasSpeedMap;