diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index db1e45d499e..610a443daf2 100755 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -3976,17 +3976,34 @@ void PortsOrch::doPortTask(Consumer &consumer) p.m_preemphasis = serdes_attr; m_portList[p.m_alias] = p; } - else if (setPortSerdesAttribute(p.m_port_id, gSwitchId, serdes_attr)) - { - SWSS_LOG_NOTICE("Set port %s preemphasis is success", p.m_alias.c_str()); - p.m_preemphasis = serdes_attr; - m_portList[p.m_alias] = p; - } else { - SWSS_LOG_ERROR("Failed to set port %s pre-emphasis", p.m_alias.c_str()); - it++; - continue; + 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)) + { + SWSS_LOG_NOTICE("Set port %s SI settings is successful", p.m_alias.c_str()); + p.m_preemphasis = 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; + } } } diff --git a/tests/mock_tests/portsorch_ut.cpp b/tests/mock_tests/portsorch_ut.cpp index 85c97ed78a9..5e253e7115f 100644 --- a/tests/mock_tests/portsorch_ut.cpp +++ b/tests/mock_tests/portsorch_ut.cpp @@ -71,6 +71,8 @@ namespace portsorch_test uint32_t _sai_set_port_fec_count; int32_t _sai_port_fec_mode; uint32_t _sai_set_pfc_mode_count; + uint32_t _sai_set_admin_state_up_count; + uint32_t _sai_set_admin_state_down_count; sai_status_t _ut_stub_sai_set_port_attribute( _In_ sai_object_id_t port_id, _In_ const sai_attribute_t *attr) @@ -89,6 +91,14 @@ namespace portsorch_test { _sai_set_pfc_mode_count++; } + else if (attr[0].id == SAI_PORT_ATTR_ADMIN_STATE) + { + if (attr[0].value.booldata) { + _sai_set_admin_state_up_count++; + } else { + _sai_set_admin_state_down_count++; + } + } return pold_sai_port_api->set_port_attribute(port_id, attr); } @@ -760,6 +770,95 @@ namespace portsorch_test cleanupPorts(gPortsOrch); } + /** + * Test that verifies admin-disable then admin-enable during setPortSerdesAttribute() + */ + TEST_F(PortsOrchTest, PortSerdesConfig) + { + auto portTable = Table(m_app_db.get(), APP_PORT_TABLE_NAME); + + // Get SAI default ports + auto &ports = defaultPortList; + ASSERT_TRUE(!ports.empty()); + + // Generate port config + for (const auto &cit : ports) + { + portTable.set(cit.first, cit.second); + } + + // Set PortConfigDone + portTable.set("PortConfigDone", { { "count", std::to_string(ports.size()) } }); + + // Refill consumer + gPortsOrch->addExistingData(&portTable); + + // Apply configuration + static_cast(gPortsOrch)->doTask(); + + // Generate basic port config + std::deque kfvBasic = {{ + "Ethernet0", + SET_COMMAND, { + { "speed", "100000" }, + { "fec", "rs" }, + { "mtu", "9100" }, + { "admin_status", "up" } + } + }}; + + // Refill consumer + auto consumer = dynamic_cast(gPortsOrch->getExecutor(APP_PORT_TABLE_NAME)); + consumer->addToSync(kfvBasic); + + // Apply configuration + static_cast(gPortsOrch)->doTask(); + + // Get port and verify admin status + Port p; + ASSERT_TRUE(gPortsOrch->getPort("Ethernet0", p)); + ASSERT_TRUE(p.m_admin_state_up); + + // Generate port serdes config + std::deque kfvSerdes = {{ + "Ethernet0", + SET_COMMAND, { + { "admin_status", "up" }, + { "idriver" , "0x6,0x6,0x6,0x6" } + } + }}; + + // Refill consumer + consumer->addToSync(kfvSerdes); + + _hook_sai_port_api(); + uint32_t current_sai_api_call_count = _sai_set_admin_state_down_count; + + // Apply configuration + static_cast(gPortsOrch)->doTask(); + + _unhook_sai_port_api(); + + ASSERT_TRUE(gPortsOrch->getPort("Ethernet0", p)); + ASSERT_TRUE(p.m_admin_state_up); + + // Verify idriver + std::vector idriver = { 0x6, 0x6, 0x6, 0x6 }; + ASSERT_EQ(p.m_preemphasis.at(SAI_PORT_SERDES_ATTR_IDRIVER), idriver); + + // Verify 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 taskList; + gPortsOrch->dumpPendingTasks(taskList); + ASSERT_TRUE(taskList.empty()); + + // Cleanup ports + cleanupPorts(gPortsOrch); + } + /** * Test that verifies PortsOrch::getPort() on a port that has been deleted */