From c4048b2047f693436b38196632cddf65beb67a86 Mon Sep 17 00:00:00 2001 From: jack9603301 Date: Thu, 10 Dec 2020 23:23:44 +0800 Subject: interfaces: T3114: Fix VLAN-aware bridge setting failure --- python/vyos/ifconfig/bridge.py | 37 +++++++++++++--- python/vyos/ifconfig/interface.py | 59 +++++++++++++++++++++++++ smoketest/scripts/cli/test_interfaces_bridge.py | 16 +++++++ 3 files changed, 106 insertions(+), 6 deletions(-) diff --git a/python/vyos/ifconfig/bridge.py b/python/vyos/ifconfig/bridge.py index d0d5da881..959345f2f 100644 --- a/python/vyos/ifconfig/bridge.py +++ b/python/vyos/ifconfig/bridge.py @@ -14,6 +14,7 @@ # License along with this library. If not, see . from netifaces import interfaces +import json from vyos.ifconfig.interface import Interface from vyos.validate import assert_boolean @@ -208,6 +209,35 @@ 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 @@ -309,7 +339,7 @@ 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: vlan_del.add(1) cmd = f'bridge vlan add dev {interface} vid {vlan_id} pvid untagged master' self._cmd(cmd) @@ -329,16 +359,11 @@ class BridgeIf(Interface): self._cmd(cmd) vlan_add.add(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) diff --git a/python/vyos/ifconfig/interface.py b/python/vyos/ifconfig/interface.py index 6e6a83f36..eda3fadd6 100644 --- a/python/vyos/ifconfig/interface.py +++ b/python/vyos/ifconfig/interface.py @@ -731,6 +731,7 @@ class Interface(Control): >>> Interface('eth0').set_proxy_arp_pvlan(1) """ self.set_interface('proxy_arp_pvlan', enable) + def get_addr(self): """ @@ -898,6 +899,64 @@ class Interface(Control): # set bridge port path priority if 'priority' in bridge_config: self.set_path_cost(bridge_config['priority']) + + vlan_filter = 0 + + vlan_del = set() + vlan_add = set() + + 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) + + if 'allowed_vlan' in bridge_config: + vlan_filter = 1 + + if vlan_filter: + if 'native_vlan' not in bridge_config: + cmd = f'bridge vlan del dev {self.ifname} vid 1' + self._cmd(cmd) + + if 'allowed_vlan' in bridge_config: + 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) + + vlan_bridge_ids = Section.klass(bridge)(bridge, create=True).get_vlan_ids() + + # 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: + if isinstance(vlan,str) and vlan.isnumeric(): + if int(vlan) not in vlan_bridge_ids: + cmd = f'bridge vlan add dev {bridge} vid {vlan} self' + self._cmd(cmd) + elif isinstance(vlan,str) and not vlan.isnumeric(): + vlan_range = vlan.split('-') + for vlan_id in range(int(vlan_range[0]),int(vlan_range[1]) + 1): + if int(vlan_id) not in vlan_bridge_ids: + cmd = f'bridge vlan add dev {bridge} vid {vlan_id} self' + self._cmd(cmd) + else: + if vlan not in vlan_bridge_ids: + cmd = f'bridge vlan add dev {bridge} vid {vlan} self' + self._cmd(cmd) + + # enable/disable Vlan Filter + 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..0072a4d3b 100755 --- a/smoketest/scripts/cli/test_interfaces_bridge.py +++ b/smoketest/scripts/cli/test_interfaces_bridge.py @@ -45,10 +45,25 @@ 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.commit() + self.session.set(['interfaces','bonding','bond1','member','interface','dum0']) + self.session.set(['interfaces','bonding','bond1','member','interface','dum1']) + self.session.commit() + for tmp in Section.interfaces("bonding"): + if not '.' in tmp: + self._members.append(tmp) 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 """ @@ -56,6 +71,7 @@ class BridgeInterfaceTest(BasicInterfaceTest.BaseTest): base = self._base_path + [interface] self.session.set(base + ['stp']) self.session.set(base + ['address', '192.0.2.1/24']) + self.session.commit() cost = 1000 priority = 10 -- cgit v1.2.3 From 934392a7f30b6a043a3de54c882453ddbea117ae Mon Sep 17 00:00:00 2001 From: jack9603301 Date: Thu, 10 Dec 2020 23:52:17 +0800 Subject: interfaces: T3114: Remove some redundant code --- python/vyos/ifconfig/interface.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/python/vyos/ifconfig/interface.py b/python/vyos/ifconfig/interface.py index eda3fadd6..e11c04209 100644 --- a/python/vyos/ifconfig/interface.py +++ b/python/vyos/ifconfig/interface.py @@ -950,10 +950,6 @@ class Interface(Control): if int(vlan_id) not in vlan_bridge_ids: cmd = f'bridge vlan add dev {bridge} vid {vlan_id} self' self._cmd(cmd) - else: - if vlan not in vlan_bridge_ids: - cmd = f'bridge vlan add dev {bridge} vid {vlan} self' - self._cmd(cmd) # enable/disable Vlan Filter Section.klass(bridge)(bridge, create=True).set_vlan_filter(vlan_filter) -- cgit v1.2.3 From 780573690fad3f5ff8e40e33ecf47f1264d9eb9d Mon Sep 17 00:00:00 2001 From: jack9603301 Date: Fri, 11 Dec 2020 23:32:38 +0800 Subject: smoketest: T3114: Completely remove `commit` in `setUp` --- smoketest/scripts/cli/test_interfaces_bridge.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/smoketest/scripts/cli/test_interfaces_bridge.py b/smoketest/scripts/cli/test_interfaces_bridge.py index 0072a4d3b..6c1efaf75 100755 --- a/smoketest/scripts/cli/test_interfaces_bridge.py +++ b/smoketest/scripts/cli/test_interfaces_bridge.py @@ -48,13 +48,9 @@ class BridgeInterfaceTest(BasicInterfaceTest.BaseTest): self.session.set(['interfaces','dummy','dum0']) self.session.set(['interfaces','dummy','dum1']) - self.session.commit() self.session.set(['interfaces','bonding','bond1','member','interface','dum0']) self.session.set(['interfaces','bonding','bond1','member','interface','dum1']) - self.session.commit() - for tmp in Section.interfaces("bonding"): - if not '.' in tmp: - self._members.append(tmp) + self._members.append('bond1') self._options['br0'] = [] for member in self._members: @@ -71,7 +67,6 @@ class BridgeInterfaceTest(BasicInterfaceTest.BaseTest): base = self._base_path + [interface] self.session.set(base + ['stp']) self.session.set(base + ['address', '192.0.2.1/24']) - self.session.commit() cost = 1000 priority = 10 -- cgit v1.2.3 From 7f2b4a8ba9b963399c0b94c6504d29168b79a1ab Mon Sep 17 00:00:00 2001 From: jack9603301 Date: Sat, 12 Dec 2020 00:36:57 +0800 Subject: interfaces: T3114: Improve the processing of enabling logic for `vlan_filter` to avoid redundant paths --- python/vyos/ifconfig/bridge.py | 4 ---- python/vyos/ifconfig/interface.py | 4 ---- 2 files changed, 8 deletions(-) diff --git a/python/vyos/ifconfig/bridge.py b/python/vyos/ifconfig/bridge.py index 959345f2f..3cbb23b8e 100644 --- a/python/vyos/ifconfig/bridge.py +++ b/python/vyos/ifconfig/bridge.py @@ -347,13 +347,9 @@ class BridgeIf(Interface): 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: for vlan in interface_config['allowed_vlan']: cmd = f'bridge vlan add dev {interface} vid {vlan} master' self._cmd(cmd) diff --git a/python/vyos/ifconfig/interface.py b/python/vyos/ifconfig/interface.py index e11c04209..699896cb2 100644 --- a/python/vyos/ifconfig/interface.py +++ b/python/vyos/ifconfig/interface.py @@ -918,13 +918,9 @@ class Interface(Control): if 'allowed_vlan' in bridge_config: vlan_filter = 1 - - if vlan_filter: if 'native_vlan' not in bridge_config: cmd = f'bridge vlan del dev {self.ifname} vid 1' self._cmd(cmd) - - if 'allowed_vlan' in bridge_config: for vlan in bridge_config['allowed_vlan']: cmd = f'bridge vlan add dev {self.ifname} vid {vlan} master' self._cmd(cmd) -- cgit v1.2.3 From 078671898eacb0ef4ccec7c13db77c1359597c1c Mon Sep 17 00:00:00 2001 From: jack9603301 Date: Sat, 12 Dec 2020 00:42:08 +0800 Subject: interfaces: T3114: When the VLAN aware option is not detected, the setting of `bridge` should not be overwritten --- python/vyos/ifconfig/interface.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/python/vyos/ifconfig/interface.py b/python/vyos/ifconfig/interface.py index 699896cb2..eba3f3b99 100644 --- a/python/vyos/ifconfig/interface.py +++ b/python/vyos/ifconfig/interface.py @@ -948,7 +948,9 @@ class Interface(Control): self._cmd(cmd) # enable/disable Vlan Filter - Section.klass(bridge)(bridge, create=True).set_vlan_filter(vlan_filter) + # When the VLAN aware option is not detected, the setting of `bridge` should not be overwritten + if vlan_filter: + Section.klass(bridge)(bridge, create=True).set_vlan_filter(vlan_filter) def set_dhcp(self, enable): """ -- cgit v1.2.3 From 9863c5773e87a0a40eabdd135f3defb953a364de Mon Sep 17 00:00:00 2001 From: jack9603301 Date: Sat, 12 Dec 2020 21:07:40 +0800 Subject: interfaces: T3114: Improve VLAN ID setting logic in `bridge` --- python/vyos/ifconfig/interface.py | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/python/vyos/ifconfig/interface.py b/python/vyos/ifconfig/interface.py index eba3f3b99..495e96600 100644 --- a/python/vyos/ifconfig/interface.py +++ b/python/vyos/ifconfig/interface.py @@ -928,6 +928,8 @@ class Interface(Control): 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: @@ -936,16 +938,16 @@ class Interface(Control): # Setting VLAN ID for the bridge for vlan in vlan_add: - if isinstance(vlan,str) and vlan.isnumeric(): - if int(vlan) not in vlan_bridge_ids: - cmd = f'bridge vlan add dev {bridge} vid {vlan} self' - self._cmd(cmd) - elif isinstance(vlan,str) and not vlan.isnumeric(): - vlan_range = vlan.split('-') - for vlan_id in range(int(vlan_range[0]),int(vlan_range[1]) + 1): + 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: - cmd = f'bridge vlan add dev {bridge} vid {vlan_id} self' - self._cmd(cmd) + 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 -- cgit v1.2.3 From d03176d70e6390f42c4e46451770dc5f86864bac Mon Sep 17 00:00:00 2001 From: jack9603301 Date: Sun, 13 Dec 2020 04:23:50 +0800 Subject: interfaces: T3114: Modify the logic of the second addition to complete the setting and streamline the code --- python/vyos/configdict.py | 22 ++++++++++++ python/vyos/ifconfig/bridge.py | 72 ++++++++++++++++++--------------------- python/vyos/ifconfig/interface.py | 51 ++++++++++++--------------- 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): -- cgit v1.2.3