Merge pull request #4095 from vyos/mergify/bp/circinus/pr-4086

bridge: T6675: VXLAN Interface configuration lost due to improper bridge detachment (backport #4086)

smoketest/scripts/cli/test_interfaces_bridge.py

@@ -22,6 +22,7 @@
 from copy import deepcopy
 from glob import glob
 
+from vyos.configsession import ConfigSessionError
 from vyos.ifconfig import Section
 from vyos.template import ip_from_cidr
 from vyos.utils.process import cmd
@@ -460,5 +461,38 @@ def test_bridge_vlan_protocol(self):
             tmp = get_interface_config(interface)
             self.assertEqual(protocol, tmp['linkinfo']['info_data']['vlan_protocol'])
 
+    def test_bridge_delete_with_vxlan_heighbor_suppress(self):
+        vxlan_if = 'vxlan0'
+        vni = '123'
+        br_if = 'br0'
+        eth0_addr = '192.0.2.2/30'
+
+        self.cli_set(['interfaces', 'ethernet', 'eth0', 'address', eth0_addr])
+        self.cli_set(['interfaces', 'vxlan', vxlan_if, 'parameters', 'neighbor-suppress'])
+        self.cli_set(['interfaces', 'vxlan', vxlan_if, 'mtu', '1426'])
+        self.cli_set(['interfaces', 'vxlan', vxlan_if, 'source-address', ip_from_cidr(eth0_addr)])
+        self.cli_set(['interfaces', 'vxlan', vxlan_if, 'vni', vni])
+
+        self.cli_set(['interfaces', 'bridge', br_if, 'member', 'interface', vxlan_if])
+
+        self.cli_commit()
+
+        self.assertTrue(interface_exists(vxlan_if))
+        self.assertTrue(interface_exists(br_if))
+
+        # cannot delete bridge interface if "neighbor-suppress" parameter is configured for VXLAN interface
+        self.cli_delete(['interfaces', 'bridge', br_if])
+        with self.assertRaises(ConfigSessionError):
+            self.cli_commit()
+        self.cli_delete(['interfaces', 'vxlan', vxlan_if, 'parameters', 'neighbor-suppress'])
+
+        self.cli_commit()
+
+        self.assertFalse(interface_exists(br_if))
+
+        self.cli_delete(['interfaces', 'vxlan', vxlan_if])
+        self.cli_delete(['interfaces', 'ethernet', 'eth0', 'address', eth0_addr])
+
+
 if __name__ == '__main__':
     unittest.main(verbosity=2)

src/conf_mode/interfaces_bridge.py

@@ -53,20 +53,22 @@ def get_config(config=None):
     tmp = node_changed(conf, base + [ifname, 'member', 'interface'])
     if tmp:
         if 'member' in bridge:
-            bridge['member'].update({'interface_remove' : tmp })
+            bridge['member'].update({'interface_remove': {t: {} for t in tmp}})
         else:
-            bridge.update({'member' : {'interface_remove' : tmp }})
-            for interface in tmp:
-                # When using VXLAN member interfaces that are configured for Single
-                # VXLAN Device (SVD) we need to call the VXLAN conf-mode script to
-                # re-create VLAN to VNI mappings if required, but only if the interface
-                # is already live on the system - this must not be done on first commit
-                if interface.startswith('vxlan') and interface_exists(interface):
-                    set_dependents('vxlan', conf, interface)
-                # When using Wireless member interfaces we need to inform hostapd
-                # to properly set-up the bridge
-                elif interface.startswith('wlan') and interface_exists(interface):
-                    set_dependents('wlan', conf, interface)
+            bridge.update({'member': {'interface_remove': {t: {} for t in tmp}}})
+        for interface in tmp:
+            # When using VXLAN member interfaces that are configured for Single
+            # VXLAN Device (SVD) we need to call the VXLAN conf-mode script to
+            # re-create VLAN to VNI mappings if required, but only if the interface
+            # is already live on the system - this must not be done on first commit
+            if interface.startswith('vxlan') and interface_exists(interface):
+                set_dependents('vxlan', conf, interface)
+                _, vxlan = get_interface_dict(conf, ['interfaces', 'vxlan'], ifname=interface)
+                bridge['member']['interface_remove'].update({interface: vxlan})
+            # When using Wireless member interfaces we need to inform hostapd
+            # to properly set-up the bridge
+            elif interface.startswith('wlan') and interface_exists(interface):
+                set_dependents('wlan', conf, interface)
 
     if dict_search('member.interface', bridge) is not None:
         for interface in list(bridge['member']['interface']):
@@ -118,6 +120,16 @@ def get_config(config=None):
     return bridge
 
 def verify(bridge):
+    # to delete interface or remove a member interface VXLAN first need to check if
+    # VXLAN does not require to be a member of a bridge interface
+    if dict_search('member.interface_remove', bridge):
+        for iface, iface_config in bridge['member']['interface_remove'].items():
+            if iface.startswith('vxlan') and dict_search('parameters.neighbor_suppress', iface_config) != None:
+                raise ConfigError(
+                    f'To detach interface {iface} from bridge you must first '
+                    f'disable "neighbor-suppress" parameter in the VXLAN interface {iface}'
+                )
+
     if 'deleted' in bridge:
         return None
 
@@ -192,7 +204,7 @@ def apply(bridge):
             try:
                 call_dependents()
             except ConfigError:
-                raise ConfigError('Error updating member interface configuration after changing bridge!')
+                raise ConfigError(f'Error updating member interface {interface} configuration after changing bridge!')
 
     return None