Extract serdes programming logic into dedicated function

orchagent/portsorch.cpp

@@ -4395,6 +4395,64 @@ void PortsOrch::doSendToIngressPortTask(Consumer &consumer)
     }
 }
 
+// Helper function to program serdes with admin state management
+bool PortsOrch::programSerdes(
+    Port &port,
+    sai_object_id_t port_id,
+    sai_object_id_t switch_id,
+    PortSerdesAttrMap_t &serdes_attr)
+{
+    // Validate port_id and determine serdes type
+    const char* serdes_type_name;
+    if (port_id == port.m_port_id)
+    {
+        serdes_type_name = "ASIC";
+    }
+    else if (port_id == port.m_line_side_id)
+    {
+        serdes_type_name = "gearbox line-side";
+    }
+    else if (port_id == port.m_system_side_id)
+    {
+        serdes_type_name = "gearbox system-side";
+    }
+    else
+    {
+        SWSS_LOG_ERROR("Invalid port_id 0x%" PRIx64 " does not belong to port %s "
+                      "(port_id: 0x%" PRIx64 ", line_side_id: 0x%" PRIx64 ", system_side_id: 0x%" PRIx64 ")",
+                      port_id, port.m_alias.c_str(),
+                      port.m_port_id, port.m_line_side_id, port.m_system_side_id);
+        return false;
+    }
+
+    if (port.m_admin_state_up)
+    {
+        /* Bring port down before applying serdes attribute */
+        if (!setPortAdminStatus(port, false))
+        {
+            SWSS_LOG_ERROR("Failed to set port %s admin status DOWN to set %s serdes attr",
+                          port.m_alias.c_str(), serdes_type_name);
+            return false;
+        }
+
+        port.m_admin_state_up = false;
+        m_portList[port.m_alias] = port;
+    }
+
+    if (setPortSerdesAttribute(port_id, switch_id, serdes_attr))
+    {
+        SWSS_LOG_NOTICE("Successfully set %s serdes tunings for port %s",
+                       serdes_type_name, port.m_alias.c_str());
+        return true;
+    }
+    else
+    {
+        SWSS_LOG_ERROR("Failed to set %s serdes tunings for port %s",
+                      serdes_type_name, port.m_alias.c_str());
+        return false;
+    }
+}
+
 void PortsOrch::doPortTask(Consumer &consumer)
 {
     SWSS_LOG_ENTER();
@@ -5275,32 +5333,13 @@ void PortsOrch::doPortTask(Consumer &consumer)
                     }
                     else
                     {
-                        if (p.m_admin_state_up)
-                        {
-                                /* Bring port down before applying serdes attribute*/
-                                if (!setPortAdminStatus(p, false))
-                                {
-                                    SWSS_LOG_ERROR("Failed to set port %s admin status DOWN to set serdes attr", p.m_alias.c_str());
-                                    it++;
-                                    continue;
-                                }
-
-                                p.m_admin_state_up = false;
-                                m_portList[p.m_alias] = p;
-                        }
-
-                        if (setPortSerdesAttribute(p.m_port_id, gSwitchId, serdes_attr))
+                        if (!programSerdes(p, p.m_port_id, gSwitchId, serdes_attr))
                         {
-                            SWSS_LOG_NOTICE("Set port %s SI settings is successful", p.m_alias.c_str());
-                            p.m_serdes_attrs = serdes_attr;
-                            m_portList[p.m_alias] = p;
-                        }
-                        else
-                        {
-                            SWSS_LOG_ERROR("Failed to set port %s SI settings", p.m_alias.c_str());
                             it++;
                             continue;
                         }
+                        p.m_serdes_attrs = serdes_attr;
+                        m_portList[p.m_alias] = p;
                     }
                 }
                 if (pCfg.media_type.is_set)
@@ -5321,55 +5360,17 @@ void PortsOrch::doPortTask(Consumer &consumer)
 
                 if (p.m_line_side_id && !line_serdes_attr.empty())
                 {
-                    if (p.m_admin_state_up)
-                    {
-                            /* Bring port down before applying serdes attribute*/
-                            if (!setPortAdminStatus(p, false))
-                            {
-                                SWSS_LOG_ERROR("Failed to set port %s admin status DOWN to set gb line serdes attr", p.m_alias.c_str());
-                                it++;
-                                continue;
-                            }
-
-                            p.m_admin_state_up = false;
-                            m_portList[p.m_alias] = p;
-                    }
-
-                    if (setPortSerdesAttribute(p.m_line_side_id, p.m_switch_id, line_serdes_attr))
+                    if (!programSerdes(p, p.m_line_side_id, p.m_switch_id, line_serdes_attr))
                     {
-                        SWSS_LOG_NOTICE("Successfully set line-side gearbox tunings for port %s", p.m_alias.c_str());
-                    }
-                    else
-                    {
-                        SWSS_LOG_ERROR("Failed to set line-side gearbox tunings for port %s", p.m_alias.c_str());
                         it++;
                         continue;
                     }
                 }
 
                 if (p.m_system_side_id && !system_serdes_attr.empty())
                 {
-                    if (p.m_admin_state_up)
-                    {
-                            /* Bring port down before applying serdes attribute*/
-                            if (!setPortAdminStatus(p, false))
-                            {
-                                SWSS_LOG_ERROR("Failed to set port %s admin status DOWN to set gb system serdes attr", p.m_alias.c_str());
-                                it++;
-                                continue;
-                            }
-
-                            p.m_admin_state_up = false;
-                            m_portList[p.m_alias] = p;
-                    }
-
-                    if (setPortSerdesAttribute(p.m_system_side_id, p.m_switch_id, system_serdes_attr))
-                    {
-                        SWSS_LOG_NOTICE("Successfully set system-side gearbox tunings for port %s", p.m_alias.c_str());
-                    }
-                    else
+                    if (!programSerdes(p, p.m_system_side_id, p.m_switch_id, system_serdes_attr))
                     {
-                        SWSS_LOG_ERROR("Failed to set system-side gearbox tunings for port %s", p.m_alias.c_str());
                         it++;
                         continue;
                     }

orchagent/portsorch.h

@@ -560,6 +560,9 @@ class PortsOrch : public Orch, public Subject
 
     void removePortSerdesAttribute(sai_object_id_t port_id);
 
+    bool programSerdes(Port &port, sai_object_id_t port_id, sai_object_id_t switch_id,
+                       std::map<sai_port_serdes_attr_t, SerdesValue> &serdes_attr);
+
     bool getSaiAclBindPointType(Port::Type                type,
                                 sai_acl_bind_point_type_t &sai_acl_bind_type);
 

tests/mock_tests/portsorch_ut.cpp

@@ -344,6 +344,21 @@ namespace portsorch_test
         _In_ sai_object_id_t port_serdes_id)
     {
         _sai_remove_port_serdes_calls.push_back(port_serdes_id);
+
+        // Erase any port-to-serdes mappings that reference this serdes ID,
+        // so subsequent SAI_PORT_ATTR_PORT_SERDES_ID queries reflect removal.
+        for (auto it = _port_to_serdes_map.begin(); it != _port_to_serdes_map.end(); )
+        {
+            if (it->second == port_serdes_id)
+            {
+                it = _port_to_serdes_map.erase(it);
+            }
+            else
+            {
+                ++it;
+            }
+        }
+
         return SAI_STATUS_SUCCESS;
     }