From bd5e554169bee6986bf5a97456f29998f83bc9d8 Mon Sep 17 00:00:00 2001 From: Stepan Blyschak Date: Tue, 23 Jan 2024 13:51:23 +0200 Subject: [PATCH] [portsorch] process only updated APP_DB fields when port is already created Signed-off-by: Stepan Blyschak --- orchagent/port/porthlpr.cpp | 2 +- orchagent/port/porthlpr.h | 3 +- orchagent/portsorch.cpp | 73 ++++++++++++++++++++++++------- tests/mock_tests/portsorch_ut.cpp | 1 - 4 files changed, 60 insertions(+), 19 deletions(-) diff --git a/orchagent/port/porthlpr.cpp b/orchagent/port/porthlpr.cpp index 64c05b2aec0..f6463b0af5b 100644 --- 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 index 4bcae7fca5b..6729a83a4da 100644 --- 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 index abfa9d5ee9e..91dd2c767b4 100755 --- 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 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 index fca4f34bebf..f0643750a55 100644 --- a/tests/mock_tests/portsorch_ut.cpp +++ b/tests/mock_tests/portsorch_ut.cpp @@ -859,7 +859,6 @@ namespace portsorch_test std::deque kfvSerdes = {{ "Ethernet0", SET_COMMAND, { - { "admin_status", "up" }, { "idriver" , "0x6,0x6,0x6,0x6" } } }};