Merge pull request #638 from jack9603301/T3114

interfaces: T3114: Fix VLAN-aware bridge setting failure

4 files changed, 116 insertions, 20 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 d0d5da881..9bd4a22e7 100644
--- a/python/vyos/ifconfig/bridge.py
+++ b/python/vyos/ifconfig/bridge.py
@@ -14,12 +14,14 @@
 # License along with this library.  If not, see <http://www.gnu.org/licenses/>.
 
 from netifaces import interfaces
+import json
 
 from vyos.ifconfig.interface import Interface
 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):
@@ -44,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': {
@@ -92,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):
@@ -260,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
@@ -309,42 +334,32 @@ class BridgeIf(Interface):
                     cmd = f'bridge vlan del dev {interface} vid 1'
                     self._cmd(cmd)
                     vlan_id = interface_config['native_vlan']
-                    if vlan_id != 1:
+                    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 vlan_filter:
                     if 'native_vlan' not in interface_config:
                         cmd = f'bridge vlan del dev {interface} vid 1'
                         self._cmd(cmd)
-
-                if 'allowed_vlan' in interface_config:
+                        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)
-                    cmd = f'bridge vlan add dev {ifname} vid {vlan} pvid untagged 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 6e6a83f36..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
@@ -731,6 +732,7 @@ class Interface(Control):
         >>> Interface('eth0').set_proxy_arp_pvlan(1)
         """
         self.set_interface('proxy_arp_pvlan', enable)
+    
 
     def get_addr(self):
         """
@@ -886,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
@@ -898,6 +902,50 @@ class Interface(Control):
             # set bridge port path priority
             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'
+                    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'
+                    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):
         """
diff --git a/smoketest/scripts/cli/test_interfaces_bridge.py b/smoketest/scripts/cli/test_interfaces_bridge.py
index 9bddede31..6c1efaf75 100755
--- a/smoketest/scripts/cli/test_interfaces_bridge.py
+++ b/smoketest/scripts/cli/test_interfaces_bridge.py
@@ -45,10 +45,21 @@ class BridgeInterfaceTest(BasicInterfaceTest.BaseTest):
             for tmp in Section.interfaces("ethernet"):
                 if not '.' in tmp:
                     self._members.append(tmp)
+            
+            self.session.set(['interfaces','dummy','dum0'])
+            self.session.set(['interfaces','dummy','dum1'])
+            self.session.set(['interfaces','bonding','bond1','member','interface','dum0'])
+            self.session.set(['interfaces','bonding','bond1','member','interface','dum1'])
+            self._members.append('bond1')
 
         self._options['br0'] = []
         for member in self._members:
             self._options['br0'].append(f'member interface {member}')
+    
+    def tearDown(self):
+        self.session.delete(['interfaces','bonding'])
+        self.session.delete(['interfaces','dummy'])
+        super().tearDown()
 
     def test_add_remove_member(self):
         """ Add member interfaces to bridge and set STP cost/priority """