bridge: T3137: backport vlan features from 1.4 current

5 files changed, 172 insertions, 136 deletions

diff --git a/interface-definitions/interfaces-bridge.xml.in b/interface-definitions/interfaces-bridge.xml.in
index 91ce00ba6..ddfc5ade4 100644
--- a/interface-definitions/interfaces-bridge.xml.in
+++ b/interface-definitions/interfaces-bridge.xml.in
@@ -86,6 +86,12 @@
           #include <include/interface/ipv6-options.xml.i>
           #include <include/interface/mac.xml.i>
           #include <include/interface/mirror.xml.i>
+          <leafNode name="enable-vlan">
+            <properties>
+              <help>Enable VLAN aware bridge</help>
+              <valueless/>
+            </properties>
+          </leafNode>
           <leafNode name="max-age">
             <properties>
               <help>Interval at which neighbor bridges are removed</help>
@@ -138,7 +144,7 @@
                         <description>VLAN id range allowed on this interface (use '-' as delimiter)</description>
                       </valueHelp>
                       <constraint>
-                        <regex>^([0-9]{1,4}-[0-9]{1,4})|([0-9]{1,4})$</regex>
+                        <validator name="allowed-vlan"/>
                       </constraint>
                       <constraintErrorMessage>not a valid VLAN ID value or range</constraintErrorMessage>
                       <multi/>
@@ -172,6 +178,12 @@
                     </properties>
                     <defaultValue>32</defaultValue>
                   </leafNode>
+                  <leafNode name="isolated">
+                    <properties>
+                      <help>Port is isolated (also known as Private-VLAN)</help>
+                      <valueless/>
+                    </properties>
+                  </leafNode>
                 </children>
               </tagNode>
             </children>
@@ -196,7 +208,6 @@
               <valueless/>
             </properties>
           </leafNode>
-          #include <include/interface/vif-s.xml.i>
           #include <include/interface/vif.xml.i>
         </children>
       </tagNode>
diff --git a/python/vyos/ifconfig/bridge.py b/python/vyos/ifconfig/bridge.py
index aadef0c09..27073b266 100644
--- a/python/vyos/ifconfig/bridge.py
+++ b/python/vyos/ifconfig/bridge.py
@@ -1,4 +1,4 @@
-# Copyright 2019 VyOS maintainers and contributors <maintainers@vyos.io>
+# Copyright 2019-2021 VyOS maintainers and contributors <maintainers@vyos.io>
 #
 # This library is free software; you can redistribute it and/or
 # modify it under the terms of the GNU Lesser General Public
@@ -22,6 +22,7 @@ from vyos.validate import assert_positive
 from vyos.util import cmd
 from vyos.util import dict_search
 from vyos.configdict import get_vlan_ids
+from vyos.configdict import list_diff
 
 @Interface.register
 class BridgeIf(Interface):
@@ -33,7 +34,6 @@ class BridgeIf(Interface):
 
     The Linux bridge code implements a subset of the ANSI/IEEE 802.1d standard.
     """
-
     iftype = 'bridge'
     definition = {
         **Interface.definition,
@@ -267,21 +267,37 @@ class BridgeIf(Interface):
         for member in (tmp or []):
             if member in interfaces():
                 self.del_port(member)
-        vlan_filter = 0
 
-        vlan_del = set()
-        vlan_add = set()
+        # enable/disable Vlan Filter
+        vlan_filter = '1' if 'enable_vlan' in config else '0'
+        self.set_vlan_filter(vlan_filter)
 
         ifname = config['ifname']
+        if int(vlan_filter):
+            add_vlan = []
+            cur_vlan_ids = get_vlan_ids(ifname)
+
+            tmp = dict_search('vif', config)
+            if tmp:
+                for vif, vif_config in tmp.items():
+                    add_vlan.append(vif)
+
+            # Remove redundant VLANs from the system
+            for vlan in list_diff(cur_vlan_ids, add_vlan):
+                cmd = f'bridge vlan del dev {ifname} vid {vlan} self'
+                self._cmd(cmd)
+
+            for vlan in add_vlan:
+                cmd = f'bridge vlan add dev {ifname} vid {vlan} self'
+                self._cmd(cmd)
+
+            # VLAN of bridge parent interface is always 1
+            # VLAN 1 is the default VLAN for all unlabeled packets
+            cmd = f'bridge vlan add dev {ifname} vid 1 pvid untagged self'
+            self._cmd(cmd)
+
         tmp = dict_search('member.interface', config)
         if tmp:
-            if self.get_vlan_filter():
-                bridge_vlan_ids = get_vlan_ids(ifname)
-                # Delete VLAN ID for the bridge
-                if 1 in bridge_vlan_ids:
-                    bridge_vlan_ids.remove(1)
-                for vlan in bridge_vlan_ids:
-                    vlan_del.add(str(vlan))
 
             for interface, interface_config in tmp.items():
                 # if interface does yet not exist bail out early and
@@ -296,9 +312,15 @@ class BridgeIf(Interface):
                 # not have any addresses configured by CLI so just flush any
                 # remaining ones
                 lower.flush_addrs()
+
                 # enslave interface port to bridge
                 self.add_port(interface)
 
+                # always set private-vlan/port isolation
+                tmp = dict_search('isolated', interface_config)
+                value = 'on' if (tmp != None) else 'off'
+                lower.set_port_isolation(value)
+
                 # set bridge port path cost
                 if 'cost' in interface_config:
                     value = interface_config.get('cost')
@@ -309,61 +331,39 @@ class BridgeIf(Interface):
                     value = interface_config.get('priority')
                     lower.set_path_priority(value)
 
-                tmp = dict_search('native_vlan_removed', interface_config)
-
-                for vlan_id in (tmp or []):
-                    cmd = f'bridge vlan del dev {interface} vid {vlan_id}'
-                    self._cmd(cmd)
-                    cmd = f'bridge vlan add dev {interface} vid 1 pvid untagged master'
-                    self._cmd(cmd)
-                    vlan_del.add(vlan_id)
-                    vlan_add.add(1)
-
-                tmp = dict_search('allowed_vlan_removed', interface_config)
-
-                for vlan_id in (tmp or []):
-                    cmd = f'bridge vlan del dev {interface} vid {vlan_id}'
-                    self._cmd(cmd)
-                    vlan_del.add(vlan_id)
-
-                if 'native_vlan' in interface_config:
-                    vlan_filter = 1
-                    cmd = f'bridge vlan del dev {interface} vid 1'
-                    self._cmd(cmd)
-                    vlan_id = interface_config['native_vlan']
-                    if int(vlan_id) != 1:
-                        if 1 in vlan_add:
-                            vlan_add.remove(1)
-                        vlan_del.add(1)
-                    cmd = f'bridge vlan add dev {interface} vid {vlan_id} pvid untagged master'
-                    self._cmd(cmd)
-                    vlan_add.add(vlan_id)
-                    if vlan_id in vlan_del:
-                        vlan_del.remove(vlan_id)
-
-                if 'allowed_vlan' in interface_config:
-                    vlan_filter = 1
-                    if 'native_vlan' not in interface_config:
-                        cmd = f'bridge vlan del dev {interface} vid 1'
+                if int(vlan_filter):
+                    add_vlan = []
+                    native_vlan_id = None
+                    allowed_vlan_ids= []
+                    cur_vlan_ids = get_vlan_ids(interface)
+
+                    if 'native_vlan' in interface_config:
+                        vlan_id = interface_config['native_vlan']
+                        add_vlan.append(vlan_id)
+                        native_vlan_id = vlan_id
+
+                    if 'allowed_vlan' in interface_config:
+                        for vlan in interface_config['allowed_vlan']:
+                            vlan_range = vlan.split('-')
+                            if len(vlan_range) == 2:
+                                for vlan_add in range(int(vlan_range[0]),int(vlan_range[1]) + 1):
+                                    add_vlan.append(str(vlan_add))
+                                    allowed_vlan_ids.append(str(vlan_add))
+                            else:
+                                add_vlan.append(vlan)
+                                allowed_vlan_ids.append(vlan)
+
+                    # Remove redundant VLANs from the system
+                    for vlan in list_diff(cur_vlan_ids, add_vlan):
+                        cmd = f'bridge vlan del dev {interface} vid {vlan} master'
                         self._cmd(cmd)
-                        vlan_del.add(1)
-                    for vlan in interface_config['allowed_vlan']:
+
+                    for vlan in allowed_vlan_ids:
                         cmd = f'bridge vlan add dev {interface} vid {vlan} master'
                         self._cmd(cmd)
-                        vlan_add.add(vlan)
-                        if vlan in vlan_del:
-                            vlan_del.remove(vlan)
-
-        for vlan in vlan_del:
-            cmd = f'bridge vlan del dev {ifname} vid {vlan} self'
-            self._cmd(cmd)
-
-        for vlan in vlan_add:
-            cmd = f'bridge vlan add dev {ifname} vid {vlan} self'
-            self._cmd(cmd)
-
-        # enable/disable Vlan Filter
-        self.set_vlan_filter(vlan_filter)
-
+                    # Setting native VLAN to system
+                    if native_vlan_id:
+                        cmd = f'bridge vlan add dev {interface} vid {native_vlan_id} pvid untagged master'
+                        self._cmd(cmd)
 
         super().update(config)
diff --git a/smoketest/scripts/cli/test_interfaces_bridge.py b/smoketest/scripts/cli/test_interfaces_bridge.py
index 4014c1a4c..2152dba72 100755
--- a/smoketest/scripts/cli/test_interfaces_bridge.py
+++ b/smoketest/scripts/cli/test_interfaces_bridge.py
@@ -63,6 +63,32 @@ class BridgeInterfaceTest(BasicInterfaceTest.TestCase):
 
         super().tearDown()
 
+    def test_isolated_interfaces(self):
+        # Add member interfaces to bridge and set STP cost/priority
+        for interface in self._interfaces:
+            base = self._base_path + [interface]
+            self.cli_set(base + ['stp'])
+
+            # assign members to bridge interface
+            for member in self._members:
+                base_member = base + ['member', 'interface', member]
+                self.cli_set(base_member + ['isolated'])
+
+        # commit config
+        self.cli_commit()
+
+        for interface in self._interfaces:
+            tmp = get_interface_config(interface)
+            # STP must be enabled as configured above
+            self.assertEqual(1, tmp['linkinfo']['info_data']['stp_state'])
+
+            # validate member interface configuration
+            for member in self._members:
+                tmp = get_interface_config(member)
+                # Isolated must be enabled as configured above
+                self.assertTrue(tmp['linkinfo']['info_slave_data']['isolated'])
+
+
     def test_add_remove_bridge_member(self):
         # Add member interfaces to bridge and set STP cost/priority
         for interface in self._interfaces:
@@ -97,12 +123,34 @@ class BridgeInterfaceTest(BasicInterfaceTest.TestCase):
                 cost += 1
                 priority += 1
 
+
+    def test_vif_8021q_interfaces(self):
+        for interface in self._interfaces:
+            base = self._base_path + [interface]
+            self.cli_set(base + ['enable-vlan'])
+        super().test_vif_8021q_interfaces()
+
+    def test_vif_8021q_lower_up_down(self):
+        for interface in self._interfaces:
+            base = self._base_path + [interface]
+            self.cli_set(base + ['enable-vlan'])
+        super().test_vif_8021q_lower_up_down()
+
+    def test_vif_8021q_mtu_limits(self):
+        for interface in self._interfaces:
+            base = self._base_path + [interface]
+            self.cli_set(base + ['enable-vlan'])
+        super().test_vif_8021q_mtu_limits()
+
     def test_bridge_vlan_filter(self):
+        vif_vlan = 2
         # Add member interface to bridge and set VLAN filter
         for interface in self._interfaces:
             base = self._base_path + [interface]
-            self.cli_set(base + ['vif', '1', 'address', '192.0.2.1/24'])
-            self.cli_set(base + ['vif', '2', 'address', '192.0.3.1/24'])
+            self.cli_set(base + ['enable-vlan'])
+            self.cli_set(base + ['address', '192.0.2.1/24'])
+            self.cli_set(base + ['vif', str(vif_vlan), 'address', '192.0.3.1/24'])
+            self.cli_set(base + ['vif', str(vif_vlan), 'mtu', self._mtu])
 
             vlan_id = 101
             allowed_vlan = 2
@@ -174,6 +222,7 @@ class BridgeInterfaceTest(BasicInterfaceTest.TestCase):
         for interface in self._interfaces:
             self.cli_delete(self._base_path + [interface, 'member'])
 
+
     def test_bridge_vlan_members(self):
         # T2945: ensure that VIFs are not dropped from bridge
         vifs = ['300', '400']
diff --git a/src/conf_mode/interfaces-bridge.py b/src/conf_mode/interfaces-bridge.py
index 5b0046a72..4d3ebc587 100755
--- a/src/conf_mode/interfaces-bridge.py
+++ b/src/conf_mode/interfaces-bridge.py
@@ -18,7 +18,6 @@ import os
 
 from sys import exit
 from netifaces import interfaces
-import re
 
 from vyos.config import Config
 from vyos.configdict import get_interface_dict
@@ -41,26 +40,6 @@ from vyos import ConfigError
 from vyos import airbag
 airbag.enable()
 
-def helper_check_removed_vlan(conf,bridge,key,key_mangling):
-    key_update = re.sub(key_mangling[0], key_mangling[1], key)
-    if dict_search('member.interface', bridge):
-        for interface in bridge['member']['interface']:
-            tmp = leaf_node_changed(conf, ['member', 'interface',interface,key])
-            if tmp:
-                if 'member' in bridge:
-                    if 'interface' in bridge['member']:
-                        if interface in bridge['member']['interface']:
-                            bridge['member']['interface'][interface].update({f'{key_update}_removed': tmp })
-                        else:
-                            bridge['member']['interface'].update({interface: {f'{key_update}_removed': tmp }})
-                    else:
-                        bridge['member'].update({ 'interface': {interface: {f'{key_update}_removed': tmp }}})
-                else:
-                    bridge.update({'member': { 'interface': {interface: {f'{key_update}_removed': tmp }}}})
-                    
-    return bridge
-    
-
 def get_config(config=None):
     """
     Retrive CLI config as dictionary. Dictionary can never be empty, as at least the
@@ -80,12 +59,6 @@ def get_config(config=None):
             bridge['member'].update({'interface_remove': tmp })
         else:
             bridge.update({'member': {'interface_remove': tmp }})
-        
-            
-    # determine which members vlan have been removed
-    
-    bridge = helper_check_removed_vlan(conf,bridge,'native-vlan',('-', '_'))
-    bridge = helper_check_removed_vlan(conf,bridge,'allowed-vlan',('-', '_'))
 
     if dict_search('member.interface', bridge):
         # XXX: T2665: we need a copy of the dict keys for iteration, else we will get:
@@ -99,7 +72,6 @@ def get_config(config=None):
         # the default dictionary is not properly paged into the dict (see T2665)
         # thus we will ammend it ourself
         default_member_values = defaults(base + ['member', 'interface'])
-        vlan_aware = False 
         for interface,interface_config in bridge['member']['interface'].items():
             bridge['member']['interface'][interface] = dict_merge(
                     default_member_values, bridge['member']['interface'][interface])
@@ -120,19 +92,11 @@ def get_config(config=None):
             # Bridge members must not have an assigned address
             tmp = has_address_configured(conf, interface)
             if tmp: bridge['member']['interface'][interface].update({'has_address' : ''})
-            
+
             # VLAN-aware bridge members must not have VLAN interface configuration
-            if 'native_vlan' in interface_config:
-                vlan_aware = True
-            
-            if 'allowed_vlan' in interface_config:
-                vlan_aware = True
-            
-            
-            if vlan_aware:
-                tmp = has_vlan_subinterface_configured(conf,interface)
-                if tmp:
-                    if tmp: bridge['member']['interface'][interface].update({'has_vlan' : ''})
+            tmp = has_vlan_subinterface_configured(conf,interface)
+            if 'enable_vlan' in bridge and tmp:
+                bridge['member']['interface'][interface].update({'has_vlan' : ''})
 
     return bridge
 
@@ -142,8 +106,8 @@ def verify(bridge):
 
     verify_dhcpv6(bridge)
     verify_vrf(bridge)
-    
-    vlan_aware = False 
+
+    ifname = bridge['ifname']
 
     if dict_search('member.interface', bridge):
         for interface, interface_config in bridge['member']['interface'].items():
@@ -166,31 +130,24 @@ def verify(bridge):
 
             if 'has_address' in interface_config:
                 raise ConfigError(error_msg + 'it has an address assigned!')
-            
-            if 'has_vlan' in interface_config:
-                raise ConfigError(error_msg + 'it has an VLAN subinterface assigned!')
-            
-            # VLAN-aware bridge members must not have VLAN interface configuration
-            if 'native_vlan' in interface_config:
-                vlan_aware = True
-            
-            if 'allowed_vlan' in interface_config:
-                vlan_aware = True
-            
-            if vlan_aware and 'wlan' in interface:
-                raise ConfigError(error_msg + 'VLAN aware cannot be set!')
-            
-            if 'allowed_vlan' in interface_config:
-                for vlan in interface_config['allowed_vlan']:
-                    if re.search('[0-9]{1,4}-[0-9]{1,4}', vlan):
-                        vlan_range = vlan.split('-')
-                        if int(vlan_range[0]) <1 and int(vlan_range[0])>4094:
-                            raise ConfigError('VLAN ID must be between 1 and 4094')
-                        if int(vlan_range[1]) <1 and int(vlan_range[1])>4094:
-                            raise ConfigError('VLAN ID must be between 1 and 4094')
-                    else:
-                        if int(vlan) <1 and int(vlan)>4094:
-                            raise ConfigError('VLAN ID must be between 1 and 4094')
+
+            if 'enable_vlan' in bridge:
+                if 'has_vlan' in interface_config:
+                    raise ConfigError(error_msg + 'it has an VLAN subinterface assigned!')
+
+                if 'wlan' in interface:
+                    raise ConfigError(error_msg + 'VLAN aware cannot be set!')
+            else:
+                for option in ['allowed_vlan', 'native_vlan']:
+                    if option in interface_config:
+                        raise ConfigError('Can not use VLAN options on non VLAN aware bridge')
+
+    if 'enable_vlan' in bridge:
+        if dict_search('vif.1', bridge):
+            raise ConfigError(f'VLAN 1 sub interface cannot be set for VLAN aware bridge {ifname}, and VLAN 1 is always the parent interface')
+    else:
+        if dict_search('vif', bridge):
+            raise ConfigError(f'You must first activate "enable-vlan" of {ifname} bridge to use "vif"')
 
     return None
 
diff --git a/src/validators/allowed-vlan b/src/validators/allowed-vlan
new file mode 100755
index 000000000..11389390b
--- /dev/null
+++ b/src/validators/allowed-vlan
@@ -0,0 +1,19 @@
+#! /usr/bin/python3
+
+import sys
+import re
+
+if __name__ == '__main__':
+    if len(sys.argv)>1:
+        allowed_vlan = sys.argv[1]
+        if re.search('[0-9]{1,4}-[0-9]{1,4}', allowed_vlan):
+            for tmp in allowed_vlan.split('-'):
+                if int(tmp) not in range(1, 4095):
+                    sys.exit(1)
+        else:
+            if int(allowed_vlan) not in range(1, 4095):
+                sys.exit(1)
+    else:
+        sys.exit(2)
+    
+    sys.exit(0)