bridge: T3137: Let VLAN aware bridge approach the behavior of professional equipment

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/>
@@ -196,7 +202,6 @@
               <valueless/>
             </properties>
           </leafNode>
-          #include <include/vif-s.xml.i>
           #include <include/vif.xml.i>
         </children>
       </tagNode>

python/vyos/ifconfig/bridge.py

@@ -22,6 +22,7 @@
 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):
@@ -274,20 +275,36 @@ def update(self, config):
         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)
+
+        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 {vif} 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
@@ -315,63 +332,43 @@ def update(self, config):
                     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
+                    else:
+                        # VLAN 1 is the default VLAN for all unlabeled packets
+                        add_vlan.append(1)
+                        native_vlan_id = 1
+
+                    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
+                    cmd = f'bridge vlan add dev {interface} vid {native_vlan_id} pvid untagged master'
+                    self._cmd(cmd)
 
         # Enable/Disable of an interface must always be done at the end of the
         # derived class to make use of the ref-counting set_admin_state()

python/vyos/ifconfig/interface.py

@@ -900,49 +900,45 @@ def add_to_bridge(self, bridge_dict):
             if 'priority' in bridge_config:
                 self.set_path_cost(bridge_config['priority'])
 
-            vlan_filter = 0
-            vlan_add = set()
-
-            del_ifname_vlan_ids = get_vlan_ids(ifname)
             bridge_vlan_filter = Section.klass(bridge)(bridge, create=True).get_vlan_filter()
 
-            if bridge_vlan_filter:
-                if 1 in del_ifname_vlan_ids:
-                    del_ifname_vlan_ids.remove(1)
-                vlan_filter = 1
-
-            for vlan in del_ifname_vlan_ids:
-                cmd = f'bridge vlan del dev {ifname} vid {vlan}'
-                self._cmd(cmd)
-
-            if 'native_vlan' in bridge_config:
-                vlan_filter = 1
-                cmd = f'bridge vlan del dev {self.ifname} vid 1'
-                self._cmd(cmd)
-                vlan_id = bridge_config['native_vlan']
-                cmd = f'bridge vlan add dev {self.ifname} vid {vlan_id} pvid untagged master'
-                self._cmd(cmd)
-                vlan_add.add(vlan_id)
-
-            if 'allowed_vlan' in bridge_config:
-                vlan_filter = 1
-                if 'native_vlan' not in bridge_config:
-                    cmd = f'bridge vlan del dev {self.ifname} vid 1'
-                    self._cmd(cmd)
-                for vlan in bridge_config['allowed_vlan']:
-                    cmd = f'bridge vlan add dev {self.ifname} vid {vlan} master'
+            if int(bridge_vlan_filter):
+                cur_vlan_ids = get_vlan_ids(ifname)
+                add_vlan = []
+                native_vlan_id = None
+                allowed_vlan_ids= []
+
+                if 'native_vlan' in bridge_config:
+                    vlan_id = bridge_config['native_vlan']
+                    add_vlan.append(vlan_id)
+                    native_vlan_id = vlan_id
+                else:
+                    # VLAN 1 is the default VLAN for all unlabeled packets
+                    add_vlan.append(1)
+                    native_vlan_id = 1
+
+                if 'allowed_vlan' in bridge_config:
+                    for vlan in bridge_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 {ifname} vid {vlan} master'
                     self._cmd(cmd)
-                    vlan_add.add(vlan)
-
-            if vlan_filter:
-                # Setting VLAN ID for the bridge
-                for vlan in vlan_add:
-                    cmd = f'bridge vlan add dev {bridge} vid {vlan} self'
+
+                for vlan in allowed_vlan_ids:
+                    cmd = f'bridge vlan add dev {ifname} vid {vlan} master'
                     self._cmd(cmd)
-
-                # enable/disable Vlan Filter
-                # When the VLAN aware option is not detected, the setting of `bridge` should not be overwritten
-                Section.klass(bridge)(bridge, create=True).set_vlan_filter(vlan_filter)
+                # Setting native VLAN to system
+                cmd = f'bridge vlan add dev {ifname} vid {native_vlan_id} pvid untagged master'
+                self._cmd(cmd)
 
     def set_dhcp(self, enable):
         """

smoketest/scripts/cli/test_interfaces_bridge.py

@@ -25,14 +25,14 @@
 from vyos.ifconfig import Section
 from vyos.util import cmd
 from vyos.util import read_file
+from vyos.validate import is_intf_addr_assigned
 
 class BridgeInterfaceTest(BasicInterfaceTest.BaseTest):
     def setUp(self):
         self._test_ip = True
         self._test_ipv6 = True
         self._test_ipv6_pd = True
         self._test_vlan = True
-        self._test_qinq = True
         self._base_path = ['interfaces', 'bridge']
         self._mirror_interfaces = ['dum21354']
         self._members = []
@@ -52,6 +52,12 @@ def setUp(self):
         self._interfaces = list(self._options)
 
         super().setUp()
+
+    def tearDown(self):
+        for intf in self._interfaces:
+            self.session.delete(self._base_path + [intf])
+
+        super().tearDown()
 
     def test_add_remove_bridge_member(self):
         # Add member interfaces to bridge and set STP cost/priority
@@ -86,13 +92,23 @@ def test_add_remove_bridge_member(self):
             self.session.delete(self._base_path + [interface, 'member'])
 
         self.session.commit()
+
+    def test_8021q_vlan_interfaces(self):
+        for interface in self._interfaces:
+            base = self._base_path + [interface]
+            self.session.set(base + ['enable-vlan'])
+        super().test_8021q_vlan_interfaces()
 
     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.session.set(base + ['vif', '1', 'address', '192.0.2.1/24'])
-            self.session.set(base + ['vif', '2', 'address', '192.0.3.1/24'])
+            self.session.set(base + ['enable-vlan'])
+            self.session.set(base + ['address', '192.0.2.1/24'])
+            self.session.set(base + ['vif', str(vif_vlan), 'address', '192.0.3.1/24'])
+            self.session.set(base + ['vif', str(vif_vlan), 'mtu', self._mtu])
 
             vlan_id = 101
             allowed_vlan = 2
@@ -159,7 +175,7 @@ def test_bridge_vlan_filter(self):
 
             for member in self._members:
                 self.assertIn(member, bridge_members)
-
+        
         # delete all members
         for interface in self._interfaces:
             self.session.delete(self._base_path + [interface, 'member'])