Do basic mutual exclusion check on vlan member and router interface c…

orchagent/intfsorch.cpp

@@ -319,6 +319,13 @@ void IntfsOrch::doTask(Consumer &consumer)
                 vnet_name = m_vnetInfses.at(alias);
             }
 
+            if (port.m_bridge_port_id != SAI_NULL_OBJECT_ID)
+            {
+                SWSS_LOG_ERROR("Router Intfs config on vlan member %s is not allowed", alias.c_str());
+                it = consumer.m_toSync.erase(it);
+                continue;
+            }
+
             if (!vnet_name.empty())
             {
                 VNetOrch* vnet_orch = gDirectory.get<VNetOrch*>();

orchagent/portsorch.cpp

@@ -2030,6 +2030,13 @@ void PortsOrch::doVlanMemberTask(Consumer &consumer)
             continue;
         }
 
+        if (port.m_rif_id)
+        {
+            SWSS_LOG_ERROR("Vlan config on router interface %s is not allowed", port.m_alias.c_str());
+            it = consumer.m_toSync.erase(it);
+            continue;
+        }
+
         if (op == SET_COMMAND)
         {
             string tagging_mode = "untagged";
@@ -2056,10 +2063,15 @@ void PortsOrch::doVlanMemberTask(Consumer &consumer)
                 continue;
             }
 
-            if (addBridgePort(port) && addVlanMember(vlan, port, tagging_mode))
-                it = consumer.m_toSync.erase(it);
-            else
-                it++;
+            if (addBridgePort(port))
+            {
+                addVlanMember(vlan, port, tagging_mode);
+            }
+            /*
+             * Failure in addBridgePort and addVlanMember is non-recoverable.
+             * Erase the vlan member task in both success and failure scenarios.
+             */
+            it = consumer.m_toSync.erase(it);
         }
         else if (op == DEL_COMMAND)
         {

tests/test_interface.py

@@ -256,6 +256,17 @@ def set_mtu(self, interface, mtu):
         tbl.set(interface, fvs)
         time.sleep(1)
 
+    # function to check vlan member and router interface mutual exclusion validation written in syslog
+    def check_syslog_for_rif_vlan_mutex_error(self, dvs, marker, alias, mutextype):
+        if mutextype == "vlanMember_on_rif":
+            (exitcode, num) = dvs.runcmd(['sh', '-c', "awk \'/%s/,ENDFILE {print;}\' /var/log/syslog | grep orchagent | grep 'router interface %s is not allowed' | wc -l" % (marker, alias)])
+            assert num.strip() == "1"
+        elif mutextype == "rif_on_vlanMember":
+            (exitcode, num) = dvs.runcmd(['sh', '-c', "awk \'/%s/,ENDFILE {print;}\' /var/log/syslog | grep orchagent | grep 'vlan member %s is not allowed' | wc -l" % (marker, alias)])
+            assert num.strip() == "1"
+        else:
+            assert "mutextype is unknown" == ""
+
     def test_InterfaceAddRemoveIpv4Address(self, dvs, testlog):
         self.setup_db(dvs)
 
@@ -388,6 +399,80 @@ def test_InterfaceSetMtu(self, dvs, testlog):
         # remove port channel
         self.remove_port_channel(dvs, "PortChannel002")
 
+
+    def test_InterfaceAndVlanMutexCheck(self, dvs, testlog):
+        self.setup_db(dvs)
+        dvs.setup_db()
+
+        marker = dvs.add_log_marker()
+
+        # create port channel
+        self.create_port_channel(dvs, "PortChannel002")
+
+        # assign IP to interface
+        self.add_ip_address("PortChannel002", "30.0.0.4/31")
+
+        # check ASIC route database
+        tbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_ROUTE_ENTRY")
+        for key in tbl.getKeys():
+            route = json.loads(key)
+            if route["dest"] == "30.0.0.4/31":
+                subnet_found = True
+            if route["dest"] == "30.0.0.4/32":
+                ip2me_found = True
+
+        assert subnet_found and ip2me_found
+
+        dvs.create_vlan("2")
+        # create vlan member.
+        # Note: schema validation above configDB should have prevented the operation of
+        # adding PortChannel002 to VLAN
+        # from being pushed to configDB, the mechanism is not available yet.
+        dvs.create_vlan_member("2", "PortChannel002")
+
+        # orchagent should have stopped the wrong configuration.
+        self.check_syslog_for_rif_vlan_mutex_error(dvs, marker, "PortChannel002", "vlanMember_on_rif")
+
+        # Have to remove vlan member to clean up configDB and appDB
+        dvs.remove_vlan_member("2", "PortChannel002")
+
+        # remove IP from interface
+        self.remove_ip_address("PortChannel002", "30.0.0.4/31")
+
+        #time.sleep(2)
+        dvs.create_vlan_member("2", "PortChannel002")
+
+        # assigning IP to interface on PortChannel002 which is a vlan member now is invalid operation.
+        self.add_ip_address("PortChannel002", "30.0.0.4/31")
+
+        # orchagent should have stopped the wrong configuration.
+        self.check_syslog_for_rif_vlan_mutex_error(dvs, marker, "PortChannel002", "rif_on_vlanMember")
+
+        # Unfortunately, the ip configuration went down to appDB
+        tbl = swsscommon.Table(self.pdb, "INTF_TABLE:PortChannel002")
+        intf_entries = tbl.getKeys()
+        assert len(intf_entries) == 1
+
+        # check ASIC database, no subnet or ip2me router entry. Orchagent did its part correctly.
+        tbl = swsscommon.Table(self.adb, "ASIC_STATE:SAI_OBJECT_TYPE_ROUTE_ENTRY")
+        for key in tbl.getKeys():
+            route = json.loads(key)
+            if route["dest"] == "30.0.0.4/31":
+                assert False
+            if route["dest"] == "30.0.0.4/32":
+                assert False
+
+        # Have to remove ip config to clean up configDB and appDB
+        self.remove_ip_address("PortChannel002", "30.0.0.4/31")
+
+        # remove vlan member
+        dvs.remove_vlan_member("2", "PortChannel002")
+
+        # remvoe vlan
+        dvs.remove_vlan("2")
+        # remove port channel
+        self.remove_port_channel(dvs, "PortChannel002")
+
 class TestLoopbackRouterInterface(object):
     def setup_db(self, dvs):
         self.pdb = swsscommon.DBConnector(0, dvs.redis_sock, 0)

tests/test_vnet.py

@@ -568,7 +568,6 @@ def test_vnet_orch_1(self, dvs, testlog):
         tunnel_name = 'tunnel_1'
 
         vnet_obj.fetch_exist_entries(dvs)
-
         create_vxlan_tunnel(dvs, tunnel_name, '10.10.10.10')
         create_vnet_entry(dvs, 'Vnet_2000', tunnel_name, '2000', "")
 
@@ -599,13 +598,14 @@ def test_vnet_orch_1(self, dvs, testlog):
         vnet_obj.check_vnet_entry(dvs, 'Vnet_2001')
         vnet_obj.check_vxlan_tunnel_entry(dvs, tunnel_name, 'Vnet_2001', '2001')
 
-        create_phy_interface(dvs, "Ethernet4", "Vnet_2001", "100.102.1.1/24")
+        # use Ethernet32 so it doesn't interfere with the following test cases.
+        create_phy_interface(dvs, "Ethernet32", "Vnet_2001", "100.102.1.1/24")
         vnet_obj.check_router_interface(dvs, 'Vnet_2001')
 
         create_vnet_routes(dvs, "100.100.2.1/32", 'Vnet_2001', '10.10.10.2', "00:12:34:56:78:9A")
         vnet_obj.check_vnet_routes(dvs, 'Vnet_2001', '10.10.10.2', tunnel_name, "00:12:34:56:78:9A")
 
-        create_vnet_local_routes(dvs, "100.102.1.0/24", 'Vnet_2001', 'Ethernet4')
+        create_vnet_local_routes(dvs, "100.102.1.0/24", 'Vnet_2001', 'Ethernet32')
         vnet_obj.check_vnet_local_routes(dvs, 'Vnet_2001')
 
     '''