From 900e75e387939a1d1d4d5b0b79809b8bb2305b91 Mon Sep 17 00:00:00 2001 From: Jernej Jakob Date: Sun, 3 May 2020 13:46:47 +0200 Subject: validate: T2241: rewrite is_bridge_member to generic is_member - rewrite the function to support both bridge and bonding interface types, if the type is passed it searches only that type, otherwise it searches both - move is_member check out of the deleted condition - move is_member check to intf_from_dict for interfaces that use it --- src/conf_mode/interfaces-tunnel.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src/conf_mode/interfaces-tunnel.py') diff --git a/src/conf_mode/interfaces-tunnel.py b/src/conf_mode/interfaces-tunnel.py index fc084814a..1916d2de2 100755 --- a/src/conf_mode/interfaces-tunnel.py +++ b/src/conf_mode/interfaces-tunnel.py @@ -25,7 +25,7 @@ from vyos.config import Config from vyos.ifconfig import Interface, GREIf, GRETapIf, IPIPIf, IP6GREIf, IPIP6If, IP6IP6If, SitIf, Sit6RDIf from vyos.ifconfig.afi import IP4, IP6 from vyos.configdict import list_diff -from vyos.validate import is_ipv4, is_ipv6, is_bridge_member +from vyos.validate import is_ipv4, is_ipv6, is_member from vyos import ConfigError from vyos.dicts import FixedDict @@ -410,7 +410,7 @@ def get_config(): options['tunnel'] = {} # check for bridges - options['bridge'] = is_bridge_member(conf, ifname) + options['bridge'] = is_member(conf, ifname, 'bridge') options['interfaces'] = interfaces() for name in ct: -- cgit v1.2.3 From f5ebaaa3ea3091f3228c509991c06300269883ef Mon Sep 17 00:00:00 2001 From: Jernej Jakob Date: Fri, 1 May 2020 19:50:21 +0200 Subject: tunnel: T2241: make VRF and bridge membership mutually exclusive --- src/conf_mode/interfaces-tunnel.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) (limited to 'src/conf_mode/interfaces-tunnel.py') diff --git a/src/conf_mode/interfaces-tunnel.py b/src/conf_mode/interfaces-tunnel.py index 1916d2de2..363daaf4f 100755 --- a/src/conf_mode/interfaces-tunnel.py +++ b/src/conf_mode/interfaces-tunnel.py @@ -525,10 +525,15 @@ def verify(conf): print(f'Should not use IPv6 addresses on tunnel {iftype} {ifname}') # vrf check - - vrf = options['vrf'] - if vrf and vrf not in options['interfaces']: - raise ConfigError(f'VRF "{vrf}" does not exist') + if options['vrf']: + if options['vrf'] not in options['interfaces']: + raise ConfigError(f'VRF "{options["vrf"]}" does not exist') + + if options['bridge']: + raise ConfigError(( + f'Interface "{options["ifname"]}" cannot be member of VRF ' + f'"{options["vrf"]}" and bridge {options["bridge"]} ' + f'at the same time!')) # source-interface check -- cgit v1.2.3 From c6fba800f088ca389bd73386793b5444ef185c22 Mon Sep 17 00:00:00 2001 From: Jernej Jakob Date: Fri, 1 May 2020 19:51:09 +0200 Subject: tunnel: T2241: make address and bridge membership mutually exclusive Bridge members should not have any addresses assigned. --- src/conf_mode/interfaces-tunnel.py | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'src/conf_mode/interfaces-tunnel.py') diff --git a/src/conf_mode/interfaces-tunnel.py b/src/conf_mode/interfaces-tunnel.py index 363daaf4f..2ef1017c9 100755 --- a/src/conf_mode/interfaces-tunnel.py +++ b/src/conf_mode/interfaces-tunnel.py @@ -535,6 +535,14 @@ def verify(conf): f'"{options["vrf"]}" and bridge {options["bridge"]} ' f'at the same time!')) + # bridge and address check + if ( options['bridge'] + and ( options['addresses-add'] + or options['ipv6_autoconf'] ) ): + raise ConfigError(( + f'Cannot assign address to interface "{options["name"]}" ' + f'as it is a member of bridge "{options["bridge"]}"!')) + # source-interface check if tun_dev and tun_dev not in options['interfaces']: -- cgit v1.2.3 From 6866efd1081c39b50a473b58aea61188f4eb6d6e Mon Sep 17 00:00:00 2001 From: Jernej Jakob Date: Fri, 1 May 2020 19:51:56 +0200 Subject: tunnel: T2241: fix falling out of bridge when changing settings Previously, set_vrf was always called, which uses the same master and nomaster commands as bridge, so it removed the interface from the bridge. - add checks to make VRF and bridge membership mutually exclusive --- src/conf_mode/interfaces-tunnel.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'src/conf_mode/interfaces-tunnel.py') diff --git a/src/conf_mode/interfaces-tunnel.py b/src/conf_mode/interfaces-tunnel.py index 2ef1017c9..57bc3b8e0 100755 --- a/src/conf_mode/interfaces-tunnel.py +++ b/src/conf_mode/interfaces-tunnel.py @@ -633,12 +633,17 @@ def apply(conf): # set other interface properties for option in ('alias', 'mtu', 'link_detect', 'multicast', 'allmulticast', - 'vrf', 'ipv6_autoconf', 'ipv6_forwarding', 'ipv6_dad_transmits'): + 'ipv6_autoconf', 'ipv6_forwarding', 'ipv6_dad_transmits'): if not options[option]: # should never happen but better safe continue tunnel.set_interface(option, options[option]) + # assign/remove VRF (ONLY when not a member of a bridge, + # otherwise 'nomaster' removes it from it) + if not options['bridge']: + tunnel.set_vrf(options['vrf']) + # Configure interface address(es) for addr in options['addresses-del']: tunnel.del_addr(addr) -- cgit v1.2.3 From dd50da5f22d18745adafb482ae19ac2455f86cba Mon Sep 17 00:00:00 2001 From: Jernej Jakob Date: Fri, 1 May 2020 19:53:41 +0200 Subject: tunnel: T2241: cleanup verify section - make error output more user friendly - replace .format with f-strings - split into lines less than ~80 characters long --- src/conf_mode/interfaces-tunnel.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) (limited to 'src/conf_mode/interfaces-tunnel.py') diff --git a/src/conf_mode/interfaces-tunnel.py b/src/conf_mode/interfaces-tunnel.py index 57bc3b8e0..f4cd53981 100755 --- a/src/conf_mode/interfaces-tunnel.py +++ b/src/conf_mode/interfaces-tunnel.py @@ -436,11 +436,14 @@ def verify(conf): if changes['section'] == 'delete': if ifname in options['nhrp']: - raise ConfigError(f'Can not delete interface tunnel {iftype} {ifname}, it is used by nhrp') + raise ConfigError(( + f'Cannot delete interface tunnel {iftype} {ifname}, ' + 'it is used by NHRP')) - bridge = options['bridge'] - if bridge: - raise ConfigError(f'Interface "{ifname}" can not be deleted as it belongs to bridge "{bridge}"!') + if options['bridge']: + raise ConfigError(( + f'Cannot delete interface "{options["ifname"]}" as it is a ' + f'member of bridge "{options["bridge"]}"!')) # done, bail out early return None -- cgit v1.2.3