interfaces: T3114: Modify the logic of the second addition to complete the setting and streamline the code

3 files changed, 77 insertions, 68 deletions

diff --git a/python/vyos/configdict.py b/python/vyos/configdict.py
index cdcd3f9ea..99c1ae2e4 100644
--- a/python/vyos/configdict.py
+++ b/python/vyos/configdict.py
@@ -17,10 +17,12 @@
 A library for retrieving value dicts from VyOS configs in a declarative fashion.
 """
 import os
+import json
 
 from vyos.util import dict_search
 from vyos.xml import defaults
 from vyos import ConfigError
+from vyos.util import cmd
 
 def retrieve_config(path_hash, base_path, config):
     """
@@ -420,6 +422,26 @@ def get_interface_dict(config, base, ifname=''):
     dict = get_removed_vlans(config, dict)
     return dict
 
+def get_vlan_ids(interface):
+    """
+    Get the VLAN ID of the interface bound to the bridge
+    """
+    vlan_ids = set()
+        
+    bridge_status = cmd('bridge -j vlan show', shell=True)
+    vlan_filter_status = json.loads(bridge_status)
+
+    if vlan_filter_status is not None:
+        for interface_status in vlan_filter_status:
+            ifname = interface_status['ifname']
+            if interface == ifname:
+                vlans_status = interface_status['vlans']
+                for vlan_status in vlans_status:
+                    vlan_id = vlan_status['vlan']
+                    vlan_ids.add(vlan_id)
+        
+    return vlan_ids
+
 
 def get_accel_dict(config, base, chap_secrets):
     """
diff --git a/python/vyos/ifconfig/bridge.py b/python/vyos/ifconfig/bridge.py
index 3cbb23b8e..9bd4a22e7 100644
--- a/python/vyos/ifconfig/bridge.py
+++ b/python/vyos/ifconfig/bridge.py
@@ -21,6 +21,7 @@ from vyos.validate import assert_boolean
 from vyos.validate import assert_positive
 from vyos.util import cmd
 from vyos.util import dict_search
+from vyos.configdict import get_vlan_ids
 
 @Interface.register
 class BridgeIf(Interface):
@@ -45,6 +46,14 @@ class BridgeIf(Interface):
             'vlan': True,
         },
     }
+    
+    _sysfs_get = {
+        **Interface._sysfs_get,**{
+            'vlan_filter': {
+                'location': '/sys/class/net/{ifname}/bridge/vlan_filtering'
+            }
+        }
+    }
 
     _sysfs_set = {**Interface._sysfs_set, **{
         'ageing_time': {
@@ -93,6 +102,13 @@ class BridgeIf(Interface):
             'shellcmd': 'ip link set dev {value} nomaster',
         },
     }}
+    
+    def get_vlan_filter(self):
+        """
+        Get the status of the bridge VLAN filter
+        """
+        
+        return self.get_interface('vlan_filter')
 
 
     def set_ageing_time(self, time):
@@ -209,35 +225,6 @@ class BridgeIf(Interface):
         """
         return self.set_interface('del_port', interface)
 
-    def get_vlan_ids(self):
-        """
-        Get the VLAN ID of the interface bound to the bridge
-        
-        is_trunk is 1 means to obtain the VLAN ID of Trunk mode, otherwise obtain the VLAN ID of Access mode
-
-        Example:
-        >>> from vyos.ifconfig import BridgeIf
-        >>> Interface('br0').get_vlan_id()
-        None
-        """
-        interface = self.config['ifname']
-        
-        vlan_ids = []
-        
-        bridge_status = cmd('bridge -j vlan show', shell=True)
-        vlan_filter_status = json.loads(bridge_status)
-        
-        if vlan_filter_status is not None:
-            for interface_status in vlan_filter_status:
-                ifname = interface_status['ifname']
-                if interface == ifname:
-                    vlans_status = interface_status['vlans']
-                    for vlan_status in vlans_status:
-                        vlan_id = vlan_status['vlan']
-                        vlan_ids.append(vlan_id)
-        
-        return vlan_ids
-
     def update(self, config):
         """ General helper function which works on a dictionary retrived by
         get_config_dict(). It's main intention is to consolidate the scattered
@@ -290,6 +277,14 @@ class BridgeIf(Interface):
 
         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
                 # add it later
@@ -340,32 +335,31 @@ class BridgeIf(Interface):
                     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'
                         self._cmd(cmd)
+                        vlan_del.add(1)
                     for vlan in interface_config['allowed_vlan']:
                         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:
-            if isinstance(vlan,str) and vlan.isnumeric():
-                if int(vlan) == 1:
-                    cmd = f'bridge vlan del dev {ifname} vid {vlan} self'
-                    self._cmd(cmd)
-                else:
-                    cmd = f'bridge vlan del dev {ifname} vid {vlan} self'
-                    self._cmd(cmd)
-            else:
-                cmd = f'bridge vlan del dev {ifname} vid {vlan} self'
-                self._cmd(cmd)
+            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'
diff --git a/python/vyos/ifconfig/interface.py b/python/vyos/ifconfig/interface.py
index 495e96600..e3c6beb8f 100644
--- a/python/vyos/ifconfig/interface.py
+++ b/python/vyos/ifconfig/interface.py
@@ -32,6 +32,7 @@ from netifaces import AF_INET6
 from vyos import ConfigError
 from vyos.configdict import list_diff
 from vyos.configdict import dict_merge
+from vyos.configdict import get_vlan_ids
 from vyos.template import render
 from vyos.util import mac2eui64
 from vyos.util import dict_search
@@ -887,6 +888,8 @@ class Interface(Control):
 
         # drop all interface addresses first
         self.flush_addrs()
+        
+        ifname = self.ifname
 
         for bridge, bridge_config in bridge_dict.items():
             # add interface to bridge - use Section.klass to get BridgeIf class
@@ -901,17 +904,25 @@ class Interface(Control):
                 self.set_path_cost(bridge_config['priority'])
             
             vlan_filter = 0
-            
-            vlan_del = set()
             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']
-                if int(vlan_id) != 1:
-                    vlan_del.add(1)
                 cmd = f'bridge vlan add dev {self.ifname} vid {vlan_id} pvid untagged master'
                 self._cmd(cmd)
                 vlan_add.add(vlan_id)
@@ -926,32 +937,14 @@ class Interface(Control):
                     self._cmd(cmd)
                     vlan_add.add(vlan)
             
-            vlan_bridge_ids = Section.klass(bridge)(bridge, create=True).get_vlan_ids()
-            
-            apply_vlan_ids = set()
-            
-            # Delete VLAN ID for the bridge
-            for vlan in vlan_del:
-                if int(vlan) == 1:
-                    cmd = f'bridge vlan del dev {bridge} vid {vlan} self'
-                    self._cmd(cmd)
-            
-            # Setting VLAN ID for the bridge
-            for vlan in vlan_add:
-                vlan_range = vlan.split('-')
-                apply_vlan_ids.add(int(vlan_range[0]))
-                if(len(vlan_range) == 2):
-                    for vlan_id in range(int(vlan_range[0])+1,int(vlan_range[1]) + 1):
-                        if int(vlan_id) not in vlan_bridge_ids:
-                            apply_vlan_ids.add(int(vlan_id))
-            
-            for vlan in apply_vlan_ids:
-                cmd = f'bridge vlan add dev {bridge} vid {vlan} self'
-                self._cmd(cmd)
-            
-            # enable/disable Vlan Filter
-            # When the VLAN aware option is not detected, the setting of `bridge` should not be overwritten
             if vlan_filter:
+                # Setting VLAN ID for the bridge
+                for vlan in vlan_add:
+                    cmd = f'bridge vlan add dev {bridge} vid {vlan} self'
+                    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)
 
     def set_dhcp(self, enable):