From a30fa0b00685778ac147326cf53a12444a986ca2 Mon Sep 17 00:00:00 2001 From: Johanna Date: Mon, 20 Jul 2026 11:31:40 -0700 Subject: [PATCH] orchagent - reassert bridge port admin state on VLAN member re-add Signed-off-by: Johanna --- orchagent/portsorch.cpp | 27 +++++++++++++++++ tests/mock_tests/portsorch_ut.cpp | 49 +++++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+) diff --git a/orchagent/portsorch.cpp b/orchagent/portsorch.cpp index e684119d941..6c6f9732df3 100644 --- a/orchagent/portsorch.cpp +++ b/orchagent/portsorch.cpp @@ -7353,6 +7353,33 @@ bool PortsOrch::addBridgePort(Port &port) if (port.m_bridge_port_id != SAI_NULL_OBJECT_ID) { + // Re-assert ADMIN_STATE=true and hostif VLAN tag mode on reuse so an existing + // bridge port is never silently left disabled or mis-tagged from a prior removal. + sai_attribute_t admin_attr; + admin_attr.id = SAI_BRIDGE_PORT_ATTR_ADMIN_STATE; + admin_attr.value.booldata = true; + + sai_status_t admin_status = sai_bridge_api->set_bridge_port_attribute(port.m_bridge_port_id, &admin_attr); + if (admin_status != SAI_STATUS_SUCCESS) + { + SWSS_LOG_ERROR( + "Failed to re-assert bridge port %s admin status to UP on reuse, rv:%d", + port.m_alias.c_str(), admin_status); + + task_process_status handle_status = handleSaiSetStatus(SAI_API_BRIDGE, admin_status); + if (handle_status != task_success) + { + return parseHandleSaiStatusFailure(handle_status); + } + } + + if (!setHostIntfsStripTag(port, SAI_HOSTIF_VLAN_TAG_KEEP)) + { + SWSS_LOG_ERROR("Failed to set %s for hostif of port %s", + hostif_vlan_tag[SAI_HOSTIF_VLAN_TAG_KEEP], port.m_alias.c_str()); + return false; + } + return true; } diff --git a/tests/mock_tests/portsorch_ut.cpp b/tests/mock_tests/portsorch_ut.cpp index cb39122f882..0a656a42c9c 100644 --- a/tests/mock_tests/portsorch_ut.cpp +++ b/tests/mock_tests/portsorch_ut.cpp @@ -3025,6 +3025,55 @@ namespace portsorch_test _unhook_sai_bridge_api(); } + /** + * PortsOrch::addBridgePort() re-asserts ADMIN_STATE=true when + * reusing a bridge port that was left stranded in the disabled state + */ + TEST_F(PortsOrchTest, addBridgePortReassertsAdminStateOnReuse) + { + Table portTable = Table(m_app_db.get(), APP_PORT_TABLE_NAME); + + // Get SAI default ports to populate DB + auto ports = ut_helper::getInitialSaiPorts(); + + // Populate port table with SAI ports + for (const auto &it : ports) + { + portTable.set(it.first, it.second); + } + + // Set PortConfigDone, PortInitDone + portTable.set("PortConfigDone", { { "count", to_string(ports.size()) } }); + portTable.set("PortInitDone", { { "lanes", "0" } }); + + // refill consumer + gPortsOrch->addExistingData(&portTable); + // Apply configuration : create ports + static_cast(gPortsOrch)->doTask(); + + Port port; + gPortsOrch->getPort("Ethernet0", port); + + // Create the bridge port for the first time. + ASSERT_TRUE(gPortsOrch->addBridgePort(port)); + ASSERT_NE(port.m_bridge_port_id, SAI_NULL_OBJECT_ID); + + // Simulate the stranded-disabled bug: the bridge port still exists but + // was left at ADMIN_STATE=false (e.g. by a failed removeBridgePort()). + sai_attribute_t attr; + attr.id = SAI_BRIDGE_PORT_ATTR_ADMIN_STATE; + attr.value.booldata = false; + ASSERT_EQ(sai_bridge_api->set_bridge_port_attribute(port.m_bridge_port_id, &attr), SAI_STATUS_SUCCESS); + + // Re-adding the port should reuse the existing bridge port and restore + // ADMIN_STATE=true instead of silently leaving it disabled. + ASSERT_TRUE(gPortsOrch->addBridgePort(port)); + + attr.id = SAI_BRIDGE_PORT_ATTR_ADMIN_STATE; + ASSERT_EQ(sai_bridge_api->get_bridge_port_attribute(port.m_bridge_port_id, 1, &attr), SAI_STATUS_SUCCESS); + ASSERT_TRUE(attr.value.booldata); + } + TEST_F(PortsOrchTest, SupportedLinkEventDampingAlgorithmSuccess) { _hook_sai_port_api();