bridge: T3137: Better implementation of VLAN aware Bridge

3 files changed, 81 insertions, 45 deletions

diff --git a/python/vyos/ifconfig/bridge.py b/python/vyos/ifconfig/bridge.py
index 87af0c9df..eed6df0e5 100644
--- a/python/vyos/ifconfig/bridge.py
+++ b/python/vyos/ifconfig/bridge.py
@@ -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):
@@ -276,21 +277,26 @@ class BridgeIf(Interface):
                 self.del_port(member)
 
         # enable/disable Vlan Filter
-        vlan_filter = 1 if 'enable_vlan' in config else 0
-        self.set_vlan_filter(str(vlan_filter))
+        vlan_filter = '1' if 'enable_vlan' in config else '0'
+        self.set_vlan_filter(vlan_filter)
 
-        if vlan_filter:
-            del_ifname_vlan_ids = get_vlan_ids(ifname)
-            # Delete all VLAN IDS configured in the interface first
-            for vlan in del_ifname_vlan_ids:
-                cmd = f'bridge vlan del dev {ifname} vid {vlan} self'
-                self._cmd(cmd)
-                
+        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():
-                    cmd = f'bridge vlan add dev {ifname} vid {vif} self'
-                    self._cmd(cmd)
+                    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
@@ -326,28 +332,44 @@ class BridgeIf(Interface):
                     value = interface_config.get('priority')
                     lower.set_path_priority(value)
 
-                if vlan_filter:
-                    
-                    del_member_vlan_ids = get_vlan_ids(interface)
-                    # Delete all VLAN IDS configured in the interface first
-                    for vlan in del_member_vlan_ids:
-                        cmd = f'bridge vlan del dev {interface} vid {vlan} master'
-                        self._cmd(cmd)
+                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']
-                        cmd = f'bridge vlan add dev {interface} vid {vlan_id} pvid untagged master'
-                        self._cmd(cmd)
+                        add_vlan.append(vlan_id)
+                        native_vlan_id = vlan_id
                     else:
                         # VLAN 1 is the default VLAN for all unlabeled packets
-                        cmd = f'bridge vlan add dev {interface} vid 1 pvid untagged master'
-                        self._cmd(cmd)
+                        add_vlan.append(1)
+                        native_vlan_id = 1
 
                     if 'allowed_vlan' in interface_config:
                         for vlan in interface_config['allowed_vlan']:
-                            cmd = f'bridge vlan add dev {interface} vid {vlan} master'
-                            self._cmd(cmd)
-
+                            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)
+                    
+                    for vlan in allowed_vlan_ids:
+                        cmd = f'bridge vlan add dev {interface} vid {vlan} master'
+                        self._cmd(cmd)
+                    
+                    # 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()
diff --git a/python/vyos/ifconfig/interface.py b/python/vyos/ifconfig/interface.py
index ffe55648c..433d19dbf 100644
--- a/python/vyos/ifconfig/interface.py
+++ b/python/vyos/ifconfig/interface.py
@@ -900,29 +900,46 @@ class Interface(Control):
             if 'priority' in bridge_config:
                 self.set_path_cost(bridge_config['priority'])
 
-            del_ifname_vlan_ids = get_vlan_ids(ifname)
-            bridge_vlan_filter = int(Section.klass(bridge)(bridge, create=True).get_vlan_filter())
+            bridge_vlan_filter = Section.klass(bridge)(bridge, create=True).get_vlan_filter()
 
-            if bridge_vlan_filter:
-
-                # Delete all VLAN IDS configured in the interface first
-                for vlan in del_ifname_vlan_ids:
-                    cmd = f'bridge vlan del dev {ifname} vid {vlan} master'
-                    self._cmd(cmd)
+            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']
-                    cmd = f'bridge vlan add dev {ifname} vid {vlan_id} pvid untagged master'
-                    self._cmd(cmd)
+                    add_vlan.append(vlan_id)
+                    native_vlan_id = vlan_id
                 else:
                     # VLAN 1 is the default VLAN for all unlabeled packets
-                    cmd = f'bridge vlan add dev {ifname} vid 1 pvid untagged master'
-                    self._cmd(cmd)
+                    add_vlan.append(1)
+                    native_vlan_id = 1
 
                 if 'allowed_vlan' in bridge_config:
                     for vlan in bridge_config['allowed_vlan']:
-                        cmd = f'bridge vlan add dev {ifname} vid {vlan} master'
-                        self._cmd(cmd)
+                        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)
+                
+                for vlan in allowed_vlan_ids:
+                    cmd = f'bridge vlan add dev {ifname} vid {vlan} master'
+                    self._cmd(cmd)
+                
+                # 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):
         """
diff --git a/src/conf_mode/interfaces-bridge.py b/src/conf_mode/interfaces-bridge.py
index d5bcfec4f..4817947eb 100755
--- a/src/conf_mode/interfaces-bridge.py
+++ b/src/conf_mode/interfaces-bridge.py
@@ -73,7 +73,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 = True if 'enable_vlan' in bridge else False
         for interface,interface_config in bridge['member']['interface'].items():
             bridge['member']['interface'][interface] = dict_merge(
                     default_member_values, bridge['member']['interface'][interface])
@@ -97,7 +96,7 @@ def get_config(config=None):
 
             # VLAN-aware bridge members must not have VLAN interface configuration
             tmp = has_vlan_subinterface_configured(conf,interface)
-            if vlan_aware and tmp:
+            if 'enable_vlan' in bridge and tmp:
                 bridge['member']['interface'][interface].update({'has_vlan' : ''})
 
     return bridge
@@ -108,8 +107,6 @@ def verify(bridge):
 
     verify_dhcpv6(bridge)
     verify_vrf(bridge)
-
-    vlan_aware = True if 'enable_vlan' in bridge else False
     
     ifname = bridge['ifname']
 
@@ -135,7 +132,7 @@ def verify(bridge):
             if 'has_address' in interface_config:
                 raise ConfigError(error_msg + 'it has an address assigned!')
 
-            if vlan_aware:
+            if 'enable_vlan' in bridge:
                 if 'has_vlan' in interface_config:
                     raise ConfigError(error_msg + 'it has an VLAN subinterface assigned!')
 
@@ -159,7 +156,7 @@ def verify(bridge):
                 if 'native_vlan' in interface_config:
                     raise ConfigError(f'You must first activate "enable-vlan" of {ifname} bridge to use "native-vlan"')
     
-    if vlan_aware:
+    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: