Orchagent - reassert bridge port admin state on VLAN member re-add

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;
     }
 

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<Orch *>(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();