From 347347c68687849822912e5e26febc5f92955b8b Mon Sep 17 00:00:00 2001 From: Jernej Jakob Date: Sun, 3 May 2020 15:43:16 +0200 Subject: bridge: T2241: use vyos.util get_bridge_member_config function Was previously moved out of this script. --- src/conf_mode/interfaces-bridge.py | 28 +++++++++------------------- 1 file changed, 9 insertions(+), 19 deletions(-) (limited to 'src/conf_mode/interfaces-bridge.py') diff --git a/src/conf_mode/interfaces-bridge.py b/src/conf_mode/interfaces-bridge.py index 9d638653c..d46b625d8 100755 --- a/src/conf_mode/interfaces-bridge.py +++ b/src/conf_mode/interfaces-bridge.py @@ -24,7 +24,7 @@ from vyos.ifconfig import BridgeIf, Section from vyos.ifconfig.stp import STP from vyos.configdict import list_diff from vyos.config import Config -from vyos.util import cmd +from vyos.util import cmd, get_bridge_member_config from vyos import ConfigError default_config_data = { @@ -202,22 +202,12 @@ def get_config(): # Determine bridge member interface (currently configured) for intf in conf.list_nodes('member interface'): - # cost and priority initialized with linux defaults - # by reading /sys/devices/virtual/net/br0/brif/eth2/{path_cost,priority} - # after adding interface to bridge after reboot - iface = { - 'name': intf, - 'cost': 100, - 'priority': 32 - } - - if conf.exists('member interface {} cost'.format(intf)): - iface['cost'] = int(conf.return_value('member interface {} cost'.format(intf))) - - if conf.exists('member interface {} priority'.format(intf)): - iface['priority'] = int(conf.return_value('member interface {} priority'.format(intf))) - - bridge['member'].append(iface) + # defaults are stored in util.py (they can't be here as all interface + # scripts use the function) + memberconf = get_bridge_member_config(conf, bridge['intf'], intf) + if memberconf: + memberconf['name'] = intf + bridge['member'].append(memberconf) # Determine bridge member interface (currently effective) - to determine which # interfaces is no longer assigend to the bridge and thus can be removed @@ -382,9 +372,9 @@ def apply(bridge): for member in bridge['member']: i = STPBridgeIf(member['name']) # configure ARP cache timeout - i.set_arp_cache_tmo(bridge['arp_cache_tmo']) + i.set_arp_cache_tmo(member['arp_cache_tmo']) # ignore link state changes - i.set_link_detect(bridge['disable_link_detect']) + i.set_link_detect(member['disable_link_detect']) # set bridge port path cost i.set_path_cost(member['cost']) # set bridge port path priority -- cgit v1.2.3 From 6369d56ed5bd5a36c4ae3de21e0e316989f0da8e Mon Sep 17 00:00:00 2001 From: Jernej Jakob Date: Fri, 1 May 2020 16:37:59 +0200 Subject: bridge: T2241: cleanup verify section - use is_member function instead of checking config directly - rearrange to join 2 for loops into one - make error output more user friendly - replace .format with f-strings - split into lines less than ~80 characters long --- src/conf_mode/interfaces-bridge.py | 41 +++++++++++++++++++++----------------- 1 file changed, 23 insertions(+), 18 deletions(-) (limited to 'src/conf_mode/interfaces-bridge.py') diff --git a/src/conf_mode/interfaces-bridge.py b/src/conf_mode/interfaces-bridge.py index d46b625d8..3f54e0dc3 100755 --- a/src/conf_mode/interfaces-bridge.py +++ b/src/conf_mode/interfaces-bridge.py @@ -23,6 +23,7 @@ from netifaces import interfaces from vyos.ifconfig import BridgeIf, Section from vyos.ifconfig.stp import STP from vyos.configdict import list_diff +from vyos.validate import is_member from vyos.config import Config from vyos.util import cmd, get_bridge_member_config from vyos import ConfigError @@ -238,30 +239,34 @@ def verify(bridge): raise ConfigError(f'VRF "{vrf_name}" does not exist') conf = Config() - for br in conf.list_nodes('interfaces bridge'): - # it makes no sense to verify ourself in this case - if br == bridge['intf']: - continue - - for intf in bridge['member']: - tmp = conf.list_nodes('interfaces bridge {} member interface'.format(br)) - if intf['name'] in tmp: - raise ConfigError('Interface "{}" belongs to bridge "{}" and can not be enslaved.'.format(intf['name'], bridge['intf'])) - - # the interface must exist prior adding it to a bridge for intf in bridge['member']: + # the interface must exist prior adding it to a bridge if intf['name'] not in interfaces(): - raise ConfigError('Can not add non existing interface "{}" to bridge "{}"'.format(intf['name'], bridge['intf'])) + raise ConfigError(( + f'Cannot add nonexistent interface "{intf["name"]}" ' + f'to bridge "{bridge["intf"]}"')) if intf['name'] == 'lo': raise ConfigError('Loopback interface "lo" can not be added to a bridge') - # bridge members are not allowed to be bond members, too - for intf in bridge['member']: - for bond in conf.list_nodes('interfaces bonding'): - if conf.exists('interfaces bonding ' + bond + ' member interface'): - if intf['name'] in conf.return_values('interfaces bonding ' + bond + ' member interface'): - raise ConfigError('Interface {} belongs to bond {}, can not add it to {}'.format(intf['name'], bond, bridge['intf'])) + # bridge members aren't allowed to be members of another bridge + for br in conf.list_nodes('interfaces bridge'): + # it makes no sense to verify ourself in this case + if br == bridge['intf']: + continue + + tmp = conf.list_nodes(f'interfaces bridge {br} member interface') + if intf['name'] in tmp: + raise ConfigError(( + f'Cannot add interface "{intf["name"]}" to bridge ' + f'"{bridge["intf"]}", it is already a member of bridge "{br}"!')) + + # bridge members are not allowed to be bond members + tmp = is_member(conf, intf['name'], 'bonding') + if tmp: + raise ConfigError(( + f'Cannot add interface "{intf["name"]}" to bridge ' + f'"{bridge["intf"]}", it is already a member of bond "{tmp}"!')) return None -- cgit v1.2.3 From f81ca22c1988495943531cba11add7cfc9441033 Mon Sep 17 00:00:00 2001 From: Jernej Jakob Date: Fri, 1 May 2020 17:45:20 +0200 Subject: bridge: T2241: disallow adding interfaces with addresses to bridge --- src/conf_mode/interfaces-bridge.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'src/conf_mode/interfaces-bridge.py') diff --git a/src/conf_mode/interfaces-bridge.py b/src/conf_mode/interfaces-bridge.py index 3f54e0dc3..fe3675190 100755 --- a/src/conf_mode/interfaces-bridge.py +++ b/src/conf_mode/interfaces-bridge.py @@ -23,7 +23,7 @@ from netifaces import interfaces from vyos.ifconfig import BridgeIf, Section from vyos.ifconfig.stp import STP from vyos.configdict import list_diff -from vyos.validate import is_member +from vyos.validate import is_member, has_address_configured from vyos.config import Config from vyos.util import cmd, get_bridge_member_config from vyos import ConfigError @@ -268,6 +268,12 @@ def verify(bridge): f'Cannot add interface "{intf["name"]}" to bridge ' f'"{bridge["intf"]}", it is already a member of bond "{tmp}"!')) + # bridge members must not have an assigned address + if has_address_configured(conf, intf['name']): + raise ConfigError(( + f'Cannot add interface "{intf["name"]}" to bridge ' + f'"{bridge["intf"]}", it has an address assigned!')) + return None def generate(bridge): -- cgit v1.2.3 From b16dc5044eb9735c51ea211ea00fa35297d921f3 Mon Sep 17 00:00:00 2001 From: Jernej Jakob Date: Sat, 2 May 2020 15:54:42 +0200 Subject: bridge: T2367: use simple 'ip addr flush' to flush member addresses We've already verified that all member interfaces don't have any addresses configured, so it should be safe to simply call 'ip addr flush' on them to flush the remaining addresses (e.g. IPv6 link-local) --- src/conf_mode/interfaces-bridge.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) (limited to 'src/conf_mode/interfaces-bridge.py') diff --git a/src/conf_mode/interfaces-bridge.py b/src/conf_mode/interfaces-bridge.py index fe3675190..c43fae78b 100755 --- a/src/conf_mode/interfaces-bridge.py +++ b/src/conf_mode/interfaces-bridge.py @@ -348,12 +348,8 @@ def apply(bridge): # add interfaces to bridge for member in bridge['member']: - # flushes address of only children of Interfaces class - # (e.g. vlan are not) - if member['name'] in Section.interfaces(): - klass = Section.klass(member['name'], vlan=False) - klass(member['name'], create=False).flush_addrs() - # flushes all interfaces + # if we've come here we already verified the interface doesn't + # have addresses configured so just flush any remaining ones cmd(f'ip addr flush dev "{member["name"]}"') br.add_port(member['name']) -- cgit v1.2.3