From 38566b8fbdec60b1601ed127fd759c85802909e9 Mon Sep 17 00:00:00 2001 From: jack9603301 Date: Sat, 9 Jan 2021 21:32:19 +0800 Subject: bridge: T3137: Let VLAN aware bridge approach the behavior of professional equipment According to the consensus, the specific behavior of a VLAN aware bridge should conform to the behavior of professional equipment. This commit makes a significant change to the behavior of VLAN aware bridge, and has the following behaviors: 1. Disable `vif 1` configuration 2. When the VLAN aware bridge is enabled, the parent interface is always VLAN 1 3. When `native-vlan` is not configured, the default behavior of the device is `native-vlan 1` 4. The VLAN ids forwarded by the bridge are determined by `vif` 5. It has an `enable-vlan` node to enable VLAN awareness 6. VLAN configuration is allowed only when VLAN aware bridge is activated --- interface-definitions/interfaces-bridge.xml.in | 7 +- python/vyos/ifconfig/bridge.py | 102 +++++++++-------------- python/vyos/ifconfig/interface.py | 52 ++++-------- smoketest/scripts/cli/test_interfaces_bridge.py | 25 ++++-- src/conf_mode/interfaces-bridge.py | 104 +++++++++--------------- src/migration-scripts/interfaces/18-to-19 | 84 +++++++++++++++++++ 6 files changed, 204 insertions(+), 170 deletions(-) create mode 100755 src/migration-scripts/interfaces/18-to-19 diff --git a/interface-definitions/interfaces-bridge.xml.in b/interface-definitions/interfaces-bridge.xml.in index c32c0ca32..e940e6685 100644 --- a/interface-definitions/interfaces-bridge.xml.in +++ b/interface-definitions/interfaces-bridge.xml.in @@ -86,6 +86,12 @@ #include #include #include + + + Enable VLAN aware bridge + + + Interval at which neighbor bridges are removed @@ -196,7 +202,6 @@ - #include #include diff --git a/python/vyos/ifconfig/bridge.py b/python/vyos/ifconfig/bridge.py index 76520f2ba..87af0c9df 100644 --- a/python/vyos/ifconfig/bridge.py +++ b/python/vyos/ifconfig/bridge.py @@ -274,20 +274,31 @@ class BridgeIf(Interface): 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(str(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) + + 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) + + # 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,62 +326,27 @@ class BridgeIf(Interface): 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 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) - vlan_del.add(1) - for vlan in interface_config['allowed_vlan']: - cmd = f'bridge vlan add dev {interface} vid {vlan} master' + + 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) + 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) - 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) + 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) # Enable/Disable of an interface must always be done at the end of the diff --git a/python/vyos/ifconfig/interface.py b/python/vyos/ifconfig/interface.py index 1561d340e..ffe55648c 100644 --- a/python/vyos/ifconfig/interface.py +++ b/python/vyos/ifconfig/interface.py @@ -900,49 +900,29 @@ class Interface(Control): 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() + bridge_vlan_filter = int(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' + + # 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) - 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' + 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) + 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) - # 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) + 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) def set_dhcp(self, enable): """ diff --git a/smoketest/scripts/cli/test_interfaces_bridge.py b/smoketest/scripts/cli/test_interfaces_bridge.py index 464226b6f..c1e98cc67 100755 --- a/smoketest/scripts/cli/test_interfaces_bridge.py +++ b/smoketest/scripts/cli/test_interfaces_bridge.py @@ -25,14 +25,13 @@ from netifaces import interfaces 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 +51,12 @@ class BridgeInterfaceTest(BasicInterfaceTest.BaseTest): 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 @@ -88,11 +93,15 @@ class BridgeInterfaceTest(BasicInterfaceTest.BaseTest): self.session.commit() 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 +168,13 @@ class BridgeInterfaceTest(BasicInterfaceTest.BaseTest): for member in self._members: self.assertIn(member, bridge_members) - + + # Check VLAN sub interface + for intf in self._interfaces: + vif = f'{intf}.{vif_vlan}' + tmp = read_file(f'/sys/class/net/{vif}/mtu') + self.assertEqual(tmp, self._mtu) + # delete all members for interface in self._interfaces: self.session.delete(self._base_path + [interface, 'member']) diff --git a/src/conf_mode/interfaces-bridge.py b/src/conf_mode/interfaces-bridge.py index 7af3e3d7c..d5bcfec4f 100755 --- a/src/conf_mode/interfaces-bridge.py +++ b/src/conf_mode/interfaces-bridge.py @@ -41,26 +41,6 @@ from vyos import ConfigError from vyos import airbag airbag.enable() -def helper_check_removed_vlan(conf,bridge,key,key_mangling): - key_update = re.sub(key_mangling[0], key_mangling[1], key) - if dict_search('member.interface', bridge): - for interface in bridge['member']['interface']: - tmp = leaf_node_changed(conf, ['member', 'interface',interface,key]) - if tmp: - if 'member' in bridge: - if 'interface' in bridge['member']: - if interface in bridge['member']['interface']: - bridge['member']['interface'][interface].update({f'{key_update}_removed': tmp }) - else: - bridge['member']['interface'].update({interface: {f'{key_update}_removed': tmp }}) - else: - bridge['member'].update({ 'interface': {interface: {f'{key_update}_removed': tmp }}}) - else: - bridge.update({'member': { 'interface': {interface: {f'{key_update}_removed': tmp }}}}) - - return bridge - - def get_config(config=None): """ Retrive CLI config as dictionary. Dictionary can never be empty, as at least the @@ -80,12 +60,6 @@ def get_config(config=None): bridge['member'].update({'interface_remove': tmp }) else: bridge.update({'member': {'interface_remove': tmp }}) - - - # determine which members vlan have been removed - - bridge = helper_check_removed_vlan(conf,bridge,'native-vlan',('-', '_')) - bridge = helper_check_removed_vlan(conf,bridge,'allowed-vlan',('-', '_')) if dict_search('member.interface', bridge): # XXX: T2665: we need a copy of the dict keys for iteration, else we will get: @@ -99,7 +73,7 @@ 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 = False + 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]) @@ -120,19 +94,11 @@ def get_config(config=None): # Bridge members must not have an assigned address tmp = has_address_configured(conf, interface) if tmp: bridge['member']['interface'][interface].update({'has_address' : ''}) - + # VLAN-aware bridge members must not have VLAN interface configuration - if 'native_vlan' in interface_config: - vlan_aware = True - - if 'allowed_vlan' in interface_config: - vlan_aware = True - - - if vlan_aware: - tmp = has_vlan_subinterface_configured(conf,interface) - if tmp: - if tmp: bridge['member']['interface'][interface].update({'has_vlan' : ''}) + tmp = has_vlan_subinterface_configured(conf,interface) + if vlan_aware and tmp: + bridge['member']['interface'][interface].update({'has_vlan' : ''}) return bridge @@ -142,8 +108,10 @@ def verify(bridge): verify_dhcpv6(bridge) verify_vrf(bridge) + + vlan_aware = True if 'enable_vlan' in bridge else False - vlan_aware = False + ifname = bridge['ifname'] if dict_search('member.interface', bridge): for interface, interface_config in bridge['member']['interface'].items(): @@ -166,31 +134,37 @@ def verify(bridge): if 'has_address' in interface_config: raise ConfigError(error_msg + 'it has an address assigned!') - - if 'has_vlan' in interface_config: - raise ConfigError(error_msg + 'it has an VLAN subinterface assigned!') - - # VLAN-aware bridge members must not have VLAN interface configuration - if 'native_vlan' in interface_config: - vlan_aware = True - - if 'allowed_vlan' in interface_config: - vlan_aware = True - - if vlan_aware and 'wlan' in interface: - raise ConfigError(error_msg + 'VLAN aware cannot be set!') - - if 'allowed_vlan' in interface_config: - for vlan in interface_config['allowed_vlan']: - if re.search('[0-9]{1,4}-[0-9]{1,4}', vlan): - vlan_range = vlan.split('-') - if int(vlan_range[0]) <1 and int(vlan_range[0])>4094: - raise ConfigError('VLAN ID must be between 1 and 4094') - if int(vlan_range[1]) <1 and int(vlan_range[1])>4094: - raise ConfigError('VLAN ID must be between 1 and 4094') - else: - if int(vlan) <1 and int(vlan)>4094: - raise ConfigError('VLAN ID must be between 1 and 4094') + + if vlan_aware: + if 'has_vlan' in interface_config: + raise ConfigError(error_msg + 'it has an VLAN subinterface assigned!') + + if 'wlan' in interface: + raise ConfigError(error_msg + 'VLAN aware cannot be set!') + + if 'allowed_vlan' in interface_config: + for vlan in interface_config['allowed_vlan']: + if re.search('[0-9]{1,4}-[0-9]{1,4}', vlan): + vlan_range = vlan.split('-') + if int(vlan_range[0]) <1 and int(vlan_range[0])>4094: + raise ConfigError('VLAN ID must be between 1 and 4094') + if int(vlan_range[1]) <1 and int(vlan_range[1])>4094: + raise ConfigError('VLAN ID must be between 1 and 4094') + else: + if int(vlan) <1 and int(vlan)>4094: + raise ConfigError('VLAN ID must be between 1 and 4094') + else: + if 'allowed_vlan' in interface_config: + raise ConfigError(f'You must first activate "enable-vlan" of {ifname} bridge to use "allowed-vlan"') + 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 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: + if dict_search('vif', bridge): + raise ConfigError(f'You must first activate "enable-vlan" of {ifname} bridge to use "vif"') return None diff --git a/src/migration-scripts/interfaces/18-to-19 b/src/migration-scripts/interfaces/18-to-19 new file mode 100755 index 000000000..86b2343b9 --- /dev/null +++ b/src/migration-scripts/interfaces/18-to-19 @@ -0,0 +1,84 @@ +#!/usr/bin/env python3 +# +# Copyright (C) 2020 VyOS maintainers and contributors +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License version 2 or later as +# published by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +# VLAN 1 command line migration for bridge aware +# https://phabricator.vyos.net/T3137 + +import sys +from vyos.configtree import ConfigTree + +if __name__ == '__main__': + if (len(sys.argv) < 1): + print("Must specify file name!") + sys.exit(1) + + file_name = sys.argv[1] + + with open(file_name, 'r') as f: + config_file = f.read() + + config = ConfigTree(config_file) + base = ['interfaces', 'bridge'] + if not config.exists(base): + # Nothing to do + sys.exit(0) + + vlan_add = set() + + for interface in config.list_nodes(base): + vif_1_old_base = base + [interface, 'vif', '1'] + if config.exists(vif_1_old_base): + address_base = vif_1_old_base + ['address'] + if config.exists(address_base): + address = config.return_values(address_base) + for addr in address: + config.set(base + [interface, 'address'],addr,False) + config.delete(vif_1_old_base) + + # Get all VLANs + member_base = base + [interface, 'member', 'interface'] + if config.exists(member_base): + for mem_intf in config.list_nodes(member_base): + native_vlan_base = member_base + [mem_intf,'native-vlan'] + allowed_vlan_base = member_base + [mem_intf,'allowed-vlan'] + if config.exists(native_vlan_base): + vlan = config.return_values(native_vlan_base)[0] + vlan_add.add(vlan) + + if config.exists(allowed_vlan_base): + vlan_ranges = config.return_values(allowed_vlan_base) + for vlan_range in vlan_ranges: + vlan_data = vlan_range.split('-') + for vlan in range(int(vlan_data[0]),int(vlan_data[1])+1): + vlan_add.add(vlan) + + # Start configuration + if len(vlan_add): + for vlan in vlan_add: + if int(vlan) != 1: + config.set(base + [interface, 'vif', vlan]) + config.set(base + [interface, 'enable-vlan']) + + + config.set_tag(base + [interface]) + + + try: + with open(file_name, 'w') as f: + f.write(config.to_string()) + except OSError as e: + print("Failed to save the modified config: {}".format(e)) + sys.exit(1) -- cgit v1.2.3 From fddf7daf9f5177c11f3bac911f477e3063f69786 Mon Sep 17 00:00:00 2001 From: jack9603301 Date: Wed, 13 Jan 2021 13:33:29 +0800 Subject: bridge: T3137: Better implementation of VLAN aware Bridge --- python/vyos/ifconfig/bridge.py | 72 +++++++++++++++++++++++++------------- python/vyos/ifconfig/interface.py | 45 ++++++++++++++++-------- src/conf_mode/interfaces-bridge.py | 9 ++--- 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: -- cgit v1.2.3 From 6ad6908327c62b97d046da5d83b5ffced2594ee5 Mon Sep 17 00:00:00 2001 From: jack9603301 Date: Wed, 13 Jan 2021 16:03:59 +0800 Subject: bridge: op: T3137: Operation mode output VLAN uses color output --- op-mode-definitions/show-interfaces-bridge.xml.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/op-mode-definitions/show-interfaces-bridge.xml.in b/op-mode-definitions/show-interfaces-bridge.xml.in index cc4b248b6..5dbc8cf85 100644 --- a/op-mode-definitions/show-interfaces-bridge.xml.in +++ b/op-mode-definitions/show-interfaces-bridge.xml.in @@ -37,7 +37,7 @@ View the VLAN filter settings of the bridge - /usr/sbin/bridge -c vlan show + /usr/sbin/bridge -c --color vlan show -- cgit v1.2.3 From ac86489e1fb3b25f7a7f728a34ad5568b3979877 Mon Sep 17 00:00:00 2001 From: jack9603301 Date: Thu, 14 Jan 2021 13:38:48 +0800 Subject: bridge: T3137: Delete blank lines --- python/vyos/ifconfig/bridge.py | 1 - python/vyos/ifconfig/interface.py | 1 - 2 files changed, 2 deletions(-) diff --git a/python/vyos/ifconfig/bridge.py b/python/vyos/ifconfig/bridge.py index eed6df0e5..85b5335de 100644 --- a/python/vyos/ifconfig/bridge.py +++ b/python/vyos/ifconfig/bridge.py @@ -366,7 +366,6 @@ class BridgeIf(Interface): 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) diff --git a/python/vyos/ifconfig/interface.py b/python/vyos/ifconfig/interface.py index 433d19dbf..f7578fe2d 100644 --- a/python/vyos/ifconfig/interface.py +++ b/python/vyos/ifconfig/interface.py @@ -936,7 +936,6 @@ class Interface(Control): 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) -- cgit v1.2.3 From 8623a8bf6d394050a671c00ce400d13f4a6b4043 Mon Sep 17 00:00:00 2001 From: jack9603301 Date: Fri, 15 Jan 2021 12:02:12 +0800 Subject: Revert "bridge: op: T3137: Operation mode output VLAN uses color output" This reverts commit 0f9f2753bc0d7d7884a2c57a4c8ed201c4a8347a. --- op-mode-definitions/show-interfaces-bridge.xml.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/op-mode-definitions/show-interfaces-bridge.xml.in b/op-mode-definitions/show-interfaces-bridge.xml.in index 5dbc8cf85..cc4b248b6 100644 --- a/op-mode-definitions/show-interfaces-bridge.xml.in +++ b/op-mode-definitions/show-interfaces-bridge.xml.in @@ -37,7 +37,7 @@ View the VLAN filter settings of the bridge - /usr/sbin/bridge -c --color vlan show + /usr/sbin/bridge -c vlan show -- cgit v1.2.3 From ac41073bffb8a9080beeb6f164307a8cce5ee1d8 Mon Sep 17 00:00:00 2001 From: jack9603301 Date: Fri, 15 Jan 2021 12:05:56 +0800 Subject: smoketest: bridge: T3137: Optimize smoketest --- smoketest/scripts/cli/test_interfaces_bridge.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/smoketest/scripts/cli/test_interfaces_bridge.py b/smoketest/scripts/cli/test_interfaces_bridge.py index c1e98cc67..3367cca5c 100755 --- a/smoketest/scripts/cli/test_interfaces_bridge.py +++ b/smoketest/scripts/cli/test_interfaces_bridge.py @@ -32,6 +32,7 @@ class BridgeInterfaceTest(BasicInterfaceTest.BaseTest): self._test_ip = True self._test_ipv6 = True self._test_ipv6_pd = True + self._test_vlan = True self._base_path = ['interfaces', 'bridge'] self._mirror_interfaces = ['dum21354'] self._members = [] @@ -91,6 +92,12 @@ class BridgeInterfaceTest(BasicInterfaceTest.BaseTest): 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): @@ -169,12 +176,6 @@ class BridgeInterfaceTest(BasicInterfaceTest.BaseTest): for member in self._members: self.assertIn(member, bridge_members) - # Check VLAN sub interface - for intf in self._interfaces: - vif = f'{intf}.{vif_vlan}' - tmp = read_file(f'/sys/class/net/{vif}/mtu') - self.assertEqual(tmp, self._mtu) - # delete all members for interface in self._interfaces: self.session.delete(self._base_path + [interface, 'member']) -- cgit v1.2.3 From c759e4030441ddb891657a6ed03cae8a9bfb980a Mon Sep 17 00:00:00 2001 From: jack9603301 Date: Fri, 15 Jan 2021 12:07:38 +0800 Subject: bridge: T3137: Remove migrators --- src/migration-scripts/interfaces/18-to-19 | 84 ------------------------------- 1 file changed, 84 deletions(-) delete mode 100755 src/migration-scripts/interfaces/18-to-19 diff --git a/src/migration-scripts/interfaces/18-to-19 b/src/migration-scripts/interfaces/18-to-19 deleted file mode 100755 index 86b2343b9..000000000 --- a/src/migration-scripts/interfaces/18-to-19 +++ /dev/null @@ -1,84 +0,0 @@ -#!/usr/bin/env python3 -# -# Copyright (C) 2020 VyOS maintainers and contributors -# -# This program is free software; you can redistribute it and/or modify -# it under the terms of the GNU General Public License version 2 or later as -# published by the Free Software Foundation. -# -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with this program. If not, see . - -# VLAN 1 command line migration for bridge aware -# https://phabricator.vyos.net/T3137 - -import sys -from vyos.configtree import ConfigTree - -if __name__ == '__main__': - if (len(sys.argv) < 1): - print("Must specify file name!") - sys.exit(1) - - file_name = sys.argv[1] - - with open(file_name, 'r') as f: - config_file = f.read() - - config = ConfigTree(config_file) - base = ['interfaces', 'bridge'] - if not config.exists(base): - # Nothing to do - sys.exit(0) - - vlan_add = set() - - for interface in config.list_nodes(base): - vif_1_old_base = base + [interface, 'vif', '1'] - if config.exists(vif_1_old_base): - address_base = vif_1_old_base + ['address'] - if config.exists(address_base): - address = config.return_values(address_base) - for addr in address: - config.set(base + [interface, 'address'],addr,False) - config.delete(vif_1_old_base) - - # Get all VLANs - member_base = base + [interface, 'member', 'interface'] - if config.exists(member_base): - for mem_intf in config.list_nodes(member_base): - native_vlan_base = member_base + [mem_intf,'native-vlan'] - allowed_vlan_base = member_base + [mem_intf,'allowed-vlan'] - if config.exists(native_vlan_base): - vlan = config.return_values(native_vlan_base)[0] - vlan_add.add(vlan) - - if config.exists(allowed_vlan_base): - vlan_ranges = config.return_values(allowed_vlan_base) - for vlan_range in vlan_ranges: - vlan_data = vlan_range.split('-') - for vlan in range(int(vlan_data[0]),int(vlan_data[1])+1): - vlan_add.add(vlan) - - # Start configuration - if len(vlan_add): - for vlan in vlan_add: - if int(vlan) != 1: - config.set(base + [interface, 'vif', vlan]) - config.set(base + [interface, 'enable-vlan']) - - - config.set_tag(base + [interface]) - - - try: - with open(file_name, 'w') as f: - f.write(config.to_string()) - except OSError as e: - print("Failed to save the modified config: {}".format(e)) - sys.exit(1) -- cgit v1.2.3 From 2d1e8a2fdba707aaae46b9f136aa8dd171ff8f3d Mon Sep 17 00:00:00 2001 From: jack9603301 Date: Sat, 16 Jan 2021 13:32:50 +0800 Subject: bridge: T3137: Improved verification logic --- interface-definitions/interfaces-bridge.xml.in | 2 +- src/conf_mode/interfaces-bridge.py | 19 +++---------------- src/validators/allowed-vlan | 19 +++++++++++++++++++ 3 files changed, 23 insertions(+), 17 deletions(-) create mode 100755 src/validators/allowed-vlan diff --git a/interface-definitions/interfaces-bridge.xml.in b/interface-definitions/interfaces-bridge.xml.in index e940e6685..63c543f33 100644 --- a/interface-definitions/interfaces-bridge.xml.in +++ b/interface-definitions/interfaces-bridge.xml.in @@ -144,7 +144,7 @@ VLAN id range allowed on this interface (use '-' as delimiter) - ^([0-9]{1,4}-[0-9]{1,4})|([0-9]{1,4})$ + not a valid VLAN ID value or range diff --git a/src/conf_mode/interfaces-bridge.py b/src/conf_mode/interfaces-bridge.py index 4817947eb..ca2718423 100755 --- a/src/conf_mode/interfaces-bridge.py +++ b/src/conf_mode/interfaces-bridge.py @@ -138,23 +138,10 @@ def verify(bridge): if 'wlan' in interface: raise ConfigError(error_msg + 'VLAN aware cannot be set!') - - if 'allowed_vlan' in interface_config: - for vlan in interface_config['allowed_vlan']: - if re.search('[0-9]{1,4}-[0-9]{1,4}', vlan): - vlan_range = vlan.split('-') - if int(vlan_range[0]) <1 and int(vlan_range[0])>4094: - raise ConfigError('VLAN ID must be between 1 and 4094') - if int(vlan_range[1]) <1 and int(vlan_range[1])>4094: - raise ConfigError('VLAN ID must be between 1 and 4094') - else: - if int(vlan) <1 and int(vlan)>4094: - raise ConfigError('VLAN ID must be between 1 and 4094') else: - if 'allowed_vlan' in interface_config: - raise ConfigError(f'You must first activate "enable-vlan" of {ifname} bridge to use "allowed-vlan"') - if 'native_vlan' in interface_config: - raise ConfigError(f'You must first activate "enable-vlan" of {ifname} bridge to use "native-vlan"') + for option in ['allowed_vlan', 'native_vlan']: + if option in interface_config: + raise ConfigError('Can not use VLAN options on non VLAN aware bridge') if 'enable_vlan' in bridge: if dict_search('vif.1', bridge): diff --git a/src/validators/allowed-vlan b/src/validators/allowed-vlan new file mode 100755 index 000000000..11389390b --- /dev/null +++ b/src/validators/allowed-vlan @@ -0,0 +1,19 @@ +#! /usr/bin/python3 + +import sys +import re + +if __name__ == '__main__': + if len(sys.argv)>1: + allowed_vlan = sys.argv[1] + if re.search('[0-9]{1,4}-[0-9]{1,4}', allowed_vlan): + for tmp in allowed_vlan.split('-'): + if int(tmp) not in range(1, 4095): + sys.exit(1) + else: + if int(allowed_vlan) not in range(1, 4095): + sys.exit(1) + else: + sys.exit(2) + + sys.exit(0) -- cgit v1.2.3