Do not allow to add port to .1Q bridge while router port deletion is not completed (sonic-net#2669)

* Do not set port to be a bridge port while it is still a router port

Author: liorghub

orchagent/portsorch.cpp

@@ -4886,6 +4886,12 @@ bool PortsOrch::addBridgePort(Port &port)
         return true;
     }
 
+    if (port.m_rif_id != 0)
+    {
+        SWSS_LOG_NOTICE("Cannot create bridge port, interface %s is a router port", port.m_alias.c_str());
+        return false;
+    }
+
     sai_attribute_t attr;
     vector<sai_attribute_t> attrs;
 

tests/mock_tests/mock_sai_bridge.h

@@ -0,0 +1,34 @@
+// Define classes and functions to mock SAI bridge functions.
+#pragma once
+
+#include <gmock/gmock.h>
+
+extern "C"
+{
+#include "sai.h"
+}
+
+// Mock class including mock functions mapping to SAI bridge functions.
+class MockSaiBridge
+{
+  public:
+    MOCK_METHOD4(create_bridge_port, sai_status_t(sai_object_id_t *bridge_port_id,
+                                                  sai_object_id_t switch_id,
+                                                  uint32_t attr_count,
+                                                  const sai_attribute_t *attr_list));
+};
+
+// Note that before mock functions below are used, mock_sai_bridge must be
+// initialized to point to an instance of MockSaiBridge.
+MockSaiBridge *mock_sai_bridge;
+
+sai_status_t mock_create_bridge_port(sai_object_id_t *bridge_port_id,
+                                     sai_object_id_t switch_id,
+                                     uint32_t attr_count,
+                                     const sai_attribute_t *attr_list)
+{
+    return mock_sai_bridge->create_bridge_port(bridge_port_id, switch_id, attr_count, attr_list);
+}
+
+
+

tests/mock_tests/portsorch_ut.cpp

@@ -7,6 +7,7 @@
 #include "mock_orchagent_main.h"
 #include "mock_table.h"
 #include "notifier.h"
+#include "mock_sai_bridge.h"
 #define private public
 #include "pfcactionhandler.h"
 #include <sys/mman.h>
@@ -15,6 +16,8 @@
 #include <sstream>
 
 extern redisReply *mockReply;
+using ::testing::_;
+using ::testing::StrictMock;
 
 namespace portsorch_test
 {
@@ -151,6 +154,21 @@ namespace portsorch_test
         sai_queue_api = pold_sai_queue_api;
     }
 
+    sai_bridge_api_t ut_sai_bridge_api;
+    sai_bridge_api_t *org_sai_bridge_api;
+
+    void _hook_sai_bridge_api()
+    {
+        ut_sai_bridge_api = *sai_bridge_api;
+        org_sai_bridge_api = sai_bridge_api;
+        sai_bridge_api = &ut_sai_bridge_api;
+    }
+
+    void _unhook_sai_bridge_api()
+    {
+        sai_bridge_api = org_sai_bridge_api;
+    }
+
     struct PortsOrchTest : public ::testing::Test
     {
         shared_ptr<swss::DBConnector> m_app_db;
@@ -345,6 +363,48 @@ namespace portsorch_test
         ASSERT_FALSE(gPortsOrch->getPort(port.m_port_id, port));
     }
 
+    /**
+     * Test case: PortsOrch::addBridgePort() does not add router port to .1Q bridge
+     */
+    TEST_F(PortsOrchTest, addBridgePortOnRouterPort)
+    {
+        _hook_sai_bridge_api();
+
+        StrictMock<MockSaiBridge> mock_sai_bridge_;
+        mock_sai_bridge = &mock_sai_bridge_;
+        sai_bridge_api->create_bridge_port = mock_create_bridge_port;
+
+        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();
+
+        // Get first port and set its rif id to simulate it is router port
+        Port port;
+        gPortsOrch->getPort("Ethernet0", port);
+        port.m_rif_id = 1;
+
+        ASSERT_FALSE(gPortsOrch->addBridgePort(port));
+        EXPECT_CALL(mock_sai_bridge_, create_bridge_port(_, _, _, _)).Times(0);
+
+        _unhook_sai_bridge_api();
+    }
+
     TEST_F(PortsOrchTest, PortSupportedFecModes)
     {
         _hook_sai_port_api();