diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index cdbe77d4dd8..56632be0769 100644 --- a/orchagent/portsorch.cpp +++ b/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,27 +5360,8 @@ 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; } @@ -5349,27 +5369,8 @@ void PortsOrch::doPortTask(Consumer &consumer) 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; } diff --git a/orchagent/portsorch.h b/orchagent/portsorch.h index f2cb495d826..f8e52bddcde 100644 --- a/orchagent/portsorch.h +++ b/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 &serdes_attr); + bool getSaiAclBindPointType(Port::Type type, sai_acl_bind_point_type_t &sai_acl_bind_type); diff --git a/tests/mock_tests/portsorch_ut.cpp b/tests/mock_tests/portsorch_ut.cpp index d8cb36bb724..0047d82cdd3 100644 --- a/tests/mock_tests/portsorch_ut.cpp +++ b/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; }