Not wait till kernel net_devices are created for all physical ports to (

sonic-net#1109) Apply buffer profiles when hardware physical ports are created Introduce a mini state machine to track the portConfigState transition on receiving event from APPL_DB PORT_TABLE Signed-off-by: Wenda Ni <[email protected]>
zhenggen-xu · Nov 4, 2019 · bb4e19c · bb4e19c
1 parent bab7b93
commit bb4e19c
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 8 deletions.
diff --git 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
@@ -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);
@@ -1513,14 +1524,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))
             {
@@ -1652,7 +1663,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();)
                 {
@@ -1697,9 +1708,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
@@ -57,6 +57,7 @@ class PortsOrch : public Orch, public Subject
 
     bool allPortsReady();
     bool isInitDone();
+    bool isConfigDone();
     bool isPortAdminUp(const string &alias);
 
     map<string, Port>& 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<set<int>, sai_object_id_t> m_portListLaneMap;
     map<set<int>, tuple<string, uint32_t, int, string>> m_lanesAliasSpeedMap;