[portsorch] dont set serdes attributes when no change

Signed-off-by: Stepan Blyschak <[email protected]>
stepanblyschak · Jan 24, 2024 · bfedda8 · bfedda8
1 parent 41330ab
commit bfedda8
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 14 deletions.
diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp
@@ -3757,12 +3757,6 @@ void PortsOrch::doPortTask(Consumer &consumer)
                         m_portList[p.m_alias] = p;
                         updatePortStatePoll(p, PORT_STATE_POLL_LT, pCfg.link_training.value);
 
-                        // Restore pre-emphasis when LT is transitioned from ON to OFF
-                        if (!p.m_link_training && serdes_attr.empty())
-                        {
-                            serdes_attr = p.m_preemphasis;
-                        }
-
                         SWSS_LOG_NOTICE(
                             "Set port %s link training to %s",
                             p.m_alias.c_str(), m_portHlpr.getLinkTrainingStr(pCfg).c_str()
@@ -4170,15 +4164,9 @@ void PortsOrch::doPortTask(Consumer &consumer)
                     }
                 }
 
-                if (!serdes_attr.empty())
+                if (!serdes_attrs.empty() && p.m_preemphasis != serdes_attr)
                 {
-                    if (p.m_link_training)
-                    {
-                        SWSS_LOG_NOTICE("Save port %s preemphasis for LT", p.m_alias.c_str());
-                        p.m_preemphasis = serdes_attr;
-                        m_portList[p.m_alias] = p;
-                    }
-                    else
+                    if (!p.m_link_training)
                     {
                         if (p.m_admin_state_up)
                         {

diff --git a/tests/mock_tests/portsorch_ut.cpp b/tests/mock_tests/portsorch_ut.cpp
@@ -886,6 +886,35 @@ namespace portsorch_test
         ASSERT_EQ(_sai_set_admin_state_down_count, ++current_sai_api_call_count);
         ASSERT_EQ(_sai_set_admin_state_up_count, current_sai_api_call_count);
 
+        // Configure non-serdes attribute that does not trigger admin state change
+        std::deque<KeyOpFieldsValuesTuple> kfvMtu = {{
+            "Ethernet0",
+            SET_COMMAND, {
+                { "mtu", "1234" },
+            }
+        }};
+
+        // Refill consumer
+        consumer->addToSync(kfvMtu);
+
+        _hook_sai_port_api();
+        current_sai_api_call_count = _sai_set_admin_state_down_count;
+
+        // Apply configuration
+        static_cast<Orch*>(gPortsOrch)->doTask();
+
+        _unhook_sai_port_api();
+
+        ASSERT_TRUE(gPortsOrch->getPort("Ethernet0", p));
+        ASSERT_TRUE(p.m_admin_state_up);
+
+        // Verify mtu is set
+        ASSERT_EQ(p.m_mtu, 1234);
+
+        // Verify no admin-disable then admin-enable
+        ASSERT_EQ(_sai_set_admin_state_down_count, current_sai_api_call_count);
+        ASSERT_EQ(_sai_set_admin_state_up_count, current_sai_api_call_count);
+
         // Dump pending tasks
         std::vector<std::string> taskList;
         gPortsOrch->dumpPendingTasks(taskList);