[portsorch] process only updated APP_DB fields when port is already

created Signed-off-by: Stepan Blyschak <stepanb@nvidia.com>
sonic-net · Feb 14, 2024 · bd5e554 · bd5e554
1 parent 41330ab
commit bd5e554
Show file tree

Hide file tree

Showing 4 changed files with 60 additions and 19 deletions.
diff --git a/orchagent/port/porthlpr.cpp b/orchagent/port/porthlpr.cpp
@@ -1001,7 +1001,7 @@ bool PortHelper::parsePortConfig(PortConfig &port) const
         }
     }
 
-    return this->validatePortConfig(port);
+    return true;
 }
 
 bool PortHelper::validatePortConfig(PortConfig &port) const

diff --git a/orchagent/port/porthlpr.h b/orchagent/port/porthlpr.h
@@ -28,6 +28,7 @@ class PortHelper final
     std::string getAdminStatusStr(const PortConfig &port) const;
 
     bool parsePortConfig(PortConfig &port) const;
+    bool validatePortConfig(PortConfig &port) const;
 
 private:
     std::string getFieldValueStr(const PortConfig &port, const std::string &field) const;
@@ -52,6 +53,4 @@ class PortHelper final
     bool parsePortRole(PortConfig &port, const std::string &field, const std::string &value) const;
     bool parsePortAdminStatus(PortConfig &port, const std::string &field, const std::string &value) const;
     bool parsePortDescription(PortConfig &port, const std::string &field, const std::string &value) const;
-
-    bool validatePortConfig(PortConfig &port) const;
 };
diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp
@@ -3515,28 +3515,61 @@ void PortsOrch::doPortTask(Consumer &consumer)
 
         if (op == SET_COMMAND)
         {
-            auto &fvMap = m_portConfigMap[key];
-
-            for (const auto &cit : kfvFieldsValues(keyOpFieldsValues))
+            if (m_portList.find(key) == m_portList.end())
             {
-                auto fieldName = fvField(cit);
-                auto fieldValue = fvValue(cit);
+                // Aggregate configuration while the port is not created.
+                auto &fvMap = m_portConfigMap[key];
+
+                for (const auto &cit : kfvFieldsValues(keyOpFieldsValues))
+                {
+                    auto fieldName = fvField(cit);
+                    auto fieldValue = fvValue(cit);
 
-                SWSS_LOG_INFO("FIELD: %s, VALUE: %s", fieldName.c_str(), fieldValue.c_str());
+                    SWSS_LOG_INFO("FIELD: %s, VALUE: %s", fieldName.c_str(), fieldValue.c_str());
 
-                fvMap[fieldName] = fieldValue;
-            }
+                    fvMap[fieldName] = fieldValue;
+                }
 
-            pCfg.fieldValueMap = fvMap;
+                pCfg.fieldValueMap = fvMap;
 
-            if (!m_portHlpr.parsePortConfig(pCfg))
-            {
-                it = taskMap.erase(it);
-                continue;
+                if (!m_portHlpr.parsePortConfig(pCfg))
+                {
+                    it = taskMap.erase(it);
+                    continue;
+                }
+
+                if (!m_portHlpr.validatePortConfig(pCfg))
+                {
+                    it = taskMap.erase(it);
+                    continue;
+                }
+
+                /* Collect information about all received ports */
+                m_lanesAliasSpeedMap[pCfg.lanes.value] = pCfg;
             }
+            else
+            {
+                // Port is already created, gather updated field-values.
+                std::unordered_map<std::string, std::string> fvMap;
+
+                for (const auto &cit : kfvFieldsValues(keyOpFieldsValues))
+                {
+                    auto fieldName = fvField(cit);
+                    auto fieldValue = fvValue(cit);
+
+                    SWSS_LOG_INFO("FIELD: %s, VALUE: %s", fieldName.c_str(), fieldValue.c_str());
+
+                    fvMap[fieldName] = fieldValue;
+                }
+
+                pCfg.fieldValueMap = fvMap;
 
-            /* Collect information about all received ports */
-            m_lanesAliasSpeedMap[pCfg.lanes.value] = pCfg;
+                if (!m_portHlpr.parsePortConfig(pCfg))
+                {
+                    it = taskMap.erase(it);
+                    continue;
+                }
+            }
 
             // TODO:
             // Fix the issue below
@@ -3652,6 +3685,9 @@ void PortsOrch::doPortTask(Consumer &consumer)
                 PortSerdesAttrMap_t serdes_attr;
                 getPortSerdesAttr(serdes_attr, pCfg);
 
+                // Saved configured admin status
+                bool admin_status = p.m_admin_state_up;
+
                 if (pCfg.autoneg.is_set)
                 {
                     if (!p.m_an_cfg || p.m_autoneg != pCfg.autoneg.value)
@@ -4212,6 +4248,13 @@ void PortsOrch::doPortTask(Consumer &consumer)
                 /* create host_tx_ready field in state-db */
                 initHostTxReadyState(p);
 
+                // Restore admin status if the port was brought down
+                if (admin_status != p.m_admin_state_up)
+                {
+                    pCfg.admin_status.is_set = true;
+                    pCfg.admin_status.value = admin_status;
+                }
+
                 /* Last step set port admin status */
                 if (pCfg.admin_status.is_set)
                 {

diff --git a/tests/mock_tests/portsorch_ut.cpp b/tests/mock_tests/portsorch_ut.cpp
@@ -859,7 +859,6 @@ namespace portsorch_test
         std::deque<KeyOpFieldsValuesTuple> kfvSerdes = {{
             "Ethernet0",
             SET_COMMAND, {
-                { "admin_status", "up"              },
                 { "idriver"     , "0x6,0x6,0x6,0x6" }
             }
         }};
-Original file line number
+Diff line change
@@ Expand Up / @@ -1001,7 +1001,7 @@ bool PortHelper::parsePortConfig(PortConfig &port) const @@
             }
         }
-        return this->validatePortConfig(port);
+        return true;
     }
     bool PortHelper::validatePortConfig(PortConfig &port) const
@@ Expand Down @@