From deb3fba81b6219a18d72eea48c644ec3c2cb724a Mon Sep 17 00:00:00 2001
From: Christian Poessinger <christian@poessinger.com>
Date: Sun, 20 Sep 2020 13:10:21 +0200
Subject: vyos.configdict: T2665: cleanup get_interface_dict() default dict
 handling

---
 python/vyos/configdict.py | 139 ++++++++++++++++++++++------------------------
 1 file changed, 66 insertions(+), 73 deletions(-)

(limited to 'python/vyos/configdict.py')

diff --git a/python/vyos/configdict.py b/python/vyos/configdict.py
index bfc70b772..ef1b452a8 100644
--- a/python/vyos/configdict.py
+++ b/python/vyos/configdict.py
@@ -18,9 +18,11 @@ A library for retrieving value dicts from VyOS configs in a declarative fashion.
 """
 import os
 
-from enum import Enum
 from copy import deepcopy
 
+from vyos.util import vyos_dict_search
+from vyos.validate import is_member
+from vyos.xml import defaults
 from vyos import ConfigError
 
 def retrieve_config(path_hash, base_path, config):
@@ -104,47 +106,6 @@ def list_diff(first, second):
     second = set(second)
     return [item for item in first if item not in second]
 
-def T2665_default_dict_cleanup(dict):
-    """ Cleanup default keys for tag nodes https://phabricator.vyos.net/T2665. """
-    # Cleanup
-    for vif in ['vif', 'vif_s']:
-        if vif in dict:
-            for key in ['ip', 'mtu', 'dhcpv6_options']:
-                if key in dict[vif]:
-                    del dict[vif][key]
-
-            # cleanup VIF-S defaults
-            if 'vif_c' in dict[vif]:
-                for key in ['ip', 'mtu', 'dhcpv6_options']:
-                    if key in dict[vif]['vif_c']:
-                        del dict[vif]['vif_c'][key]
-                # If there is no vif-c defined and we just cleaned the default
-                # keys - we can clean the entire vif-c dict as it's useless
-                if not dict[vif]['vif_c']:
-                    del dict[vif]['vif_c']
-
-            # If there is no real vif/vif-s defined and we just cleaned the default
-            # keys - we can clean the entire vif dict as it's useless
-            if not dict[vif]:
-                del dict[vif]
-
-    if 'dhcpv6_options' in dict and 'pd' in dict['dhcpv6_options']:
-        if 'length' in dict['dhcpv6_options']['pd']:
-            del dict['dhcpv6_options']['pd']['length']
-
-    # delete empty dicts
-    if 'dhcpv6_options' in dict:
-        if 'pd' in dict['dhcpv6_options']:
-            # test if 'pd' is an empty node so we can remove it
-            if not dict['dhcpv6_options']['pd']:
-                del dict['dhcpv6_options']['pd']
-
-        # test if 'dhcpv6_options' is an empty node so we can remove it
-        if not dict['dhcpv6_options']:
-            del dict['dhcpv6_options']
-
-    return dict
-
 def leaf_node_changed(conf, path):
     """
     Check if a leaf node was altered. If it has been altered - values has been
@@ -207,13 +168,21 @@ def get_removed_vlans(conf, dict):
 
     return dict
 
+def T2665_set_dhcpv6pd_defaults(config_dict):
+    """ Properly configure DHCPv6 default options in the dictionary. If there is
+    no DHCPv6 configured at all, it is safe to remove the entire configuration.
+    """
+    # As this is the same for every interface type it is safe to assume this
+    # for ethernet
+    pd_defaults = defaults(['interfaces', 'ethernet', 'dhcpv6-options', 'pd'])
 
-def dict_add_dhcpv6pd_defaults(defaults, config_dict):
     # Implant default dictionary for DHCPv6-PD instances
-    if 'dhcpv6_options' in config_dict and 'pd' in config_dict['dhcpv6_options']:
-        for pd, pd_config in config_dict['dhcpv6_options']['pd'].items():
-            config_dict['dhcpv6_options']['pd'][pd] = dict_merge(
-                defaults, pd_config)
+    if vyos_dict_search('dhcpv6_options.pd.length', config_dict):
+        del config_dict['dhcpv6_options']['pd']['length']
+
+    for pd in (vyos_dict_search('dhcpv6_options.pd', config_dict) or []):
+        config_dict['dhcpv6_options']['pd'][pd] = dict_merge(pd_defaults,
+            config_dict['dhcpv6_options']['pd'][pd])
 
     return config_dict
 
@@ -225,10 +194,6 @@ def get_interface_dict(config, base, ifname=''):
 
     Will return a dictionary with the necessary interface configuration
     """
-    from vyos.util import vyos_dict_search
-    from vyos.validate import is_member
-    from vyos.xml import defaults
-
     if not ifname:
         # determine tagNode instance
         if 'VYOS_TAGNODE_VALUE' not in os.environ:
@@ -238,6 +203,12 @@ def get_interface_dict(config, base, ifname=''):
     # retrieve interface default values
     default_values = defaults(base)
 
+    # We take care about VLAN (vif, vif-s, vif-c) default values later on when
+    # parsing vlans in default dict and merge the "proper" values in correctly,
+    # see T2665.
+    for vif in ['vif', 'vif_s']:
+        if vif in default_values: del default_values[vif]
+
     # setup config level which is extracted in get_removed_vlans()
     config.set_level(base + [ifname])
     dict = config.get_config_dict([], key_mangling=('-', '_'), get_first_key=True)
@@ -249,11 +220,20 @@ def get_interface_dict(config, base, ifname=''):
     # Add interface instance name into dictionary
     dict.update({'ifname': ifname})
 
+    # XXX: T2665: When there is no DHCPv6-PD configuration given, we can safely
+    # remove the default values from the dict.
+    if 'dhcpv6_options' not in dict:
+        if 'dhcpv6_options' in default_values:
+            del default_values['dhcpv6_options']
+
     # We have gathered the dict representation of the CLI, but there are
     # default options which we need to update into the dictionary
     # retrived.
     dict = dict_merge(default_values, dict)
 
+    # XXX: T2665: blend in proper DHCPv6-PD default values
+    dict = T2665_set_dhcpv6pd_defaults(dict)
+
     # Check if we are a member of a bridge device
     bridge = is_member(config, ifname, 'bridge')
     if bridge:
@@ -276,36 +256,49 @@ def get_interface_dict(config, base, ifname=''):
         else:
             dict['ipv6']['address'].update({'eui64_old': eui64})
 
-    # remove wrongly inserted values
-    dict = T2665_default_dict_cleanup(dict)
-
-    # Implant default dictionary for DHCPv6-PD instances
-    default_pd_values = defaults(base + ['dhcpv6-options', 'pd'])
-    dict = dict_add_dhcpv6pd_defaults(default_pd_values, dict)
-
     # Implant default dictionary in vif/vif-s VLAN interfaces. Values are
     # identical for all types of VLAN interfaces as they all include the same
     # XML definitions which hold the defaults.
-    default_vif_values = defaults(base + ['vif'])
     for vif, vif_config in dict.get('vif', {}).items():
-        dict['vif'][vif] = dict_add_dhcpv6pd_defaults(
-                default_pd_values, vif_config)
-        dict['vif'][vif] = T2665_default_dict_cleanup(
-                dict_merge(default_vif_values, vif_config))
+        default_vif_values = defaults(base + ['vif'])
+        # XXX: T2665: When there is no DHCPv6-PD configuration given, we can safely
+        # remove the default values from the dict.
+        if not 'dhcpv6_options' in vif_config:
+            del default_vif_values['dhcpv6_options']
+
+        dict['vif'][vif] = dict_merge(default_vif_values, vif_config)
+        # XXX: T2665: blend in proper DHCPv6-PD default values
+        dict['vif'][vif] = T2665_set_dhcpv6pd_defaults(dict['vif'][vif])
 
     for vif_s, vif_s_config in dict.get('vif_s', {}).items():
-        dict['vif_s'][vif_s] = dict_add_dhcpv6pd_defaults(
-                default_pd_values, vif_s_config)
-        dict['vif_s'][vif_s] = T2665_default_dict_cleanup(
-                dict_merge(default_vif_values, vif_s_config))
+        default_vif_s_values = defaults(base + ['vif-s'])
+        # XXX: T2665: we only wan't the vif-s defaults - do not care about vif-c
+        if 'vif_c' in default_vif_s_values: del default_vif_s_values['vif_c']
+
+        # XXX: T2665: When there is no DHCPv6-PD configuration given, we can safely
+        # remove the default values from the dict.
+        if not 'dhcpv6_options' in vif_s_config:
+            del default_vif_s_values['dhcpv6_options']
+
+        dict['vif_s'][vif_s] = dict_merge(default_vif_s_values, vif_s_config)
+        # XXX: T2665: blend in proper DHCPv6-PD default values
+        dict['vif_s'][vif_s] = T2665_set_dhcpv6pd_defaults(
+            dict['vif_s'][vif_s])
+
         for vif_c, vif_c_config in vif_s_config.get('vif_c', {}).items():
-            dict['vif_s'][vif_s]['vif_c'][vif_c] = dict_add_dhcpv6pd_defaults(
-                    default_pd_values, vif_c_config)
-            dict['vif_s'][vif_s]['vif_c'][vif_c] = T2665_default_dict_cleanup(
-                    dict_merge(default_vif_values, vif_c_config))
+            default_vif_c_values = defaults(base + ['vif-s', 'vif-c'])
+
+            # XXX: T2665: When there is no DHCPv6-PD configuration given, we can safely
+            # remove the default values from the dict.
+            if not 'dhcpv6_options' in vif_c_config:
+                del default_vif_c_values['dhcpv6_options']
+
+            dict['vif_s'][vif_s]['vif_c'][vif_c] = dict_merge(
+                    default_vif_c_values, vif_c_config)
+            # XXX: T2665: blend in proper DHCPv6-PD default values
+            dict['vif_s'][vif_s]['vif_c'][vif_c] = T2665_set_dhcpv6pd_defaults(
+                dict['vif_s'][vif_s]['vif_c'][vif_c])
 
     # Check vif, vif-s/vif-c VLAN interfaces for removal
     dict = get_removed_vlans(config, dict)
-
     return dict
-
-- 
cgit v1.2.3


From d28a6a516d449ede788816574c35061fbf7d6485 Mon Sep 17 00:00:00 2001
From: Christian Poessinger <christian@poessinger.com>
Date: Tue, 22 Sep 2020 18:35:44 +0200
Subject: ifconfig: T2653: move is_member() from vyos.vylidate to
 vyos.configdict

---
 python/vyos/configdict.py                   | 54 +++++++++++++++++++++++++----
 python/vyos/validate.py                     | 41 ----------------------
 src/conf_mode/interfaces-bonding.py         | 19 +++++-----
 src/conf_mode/interfaces-bridge.py          | 27 +++++++--------
 src/conf_mode/interfaces-macsec.py          | 10 ------
 src/conf_mode/interfaces-openvpn.py         |  3 +-
 src/conf_mode/interfaces-pseudo-ethernet.py | 25 +------------
 src/conf_mode/interfaces-tunnel.py          |  9 ++---
 8 files changed, 77 insertions(+), 111 deletions(-)

(limited to 'python/vyos/configdict.py')

diff --git a/python/vyos/configdict.py b/python/vyos/configdict.py
index ef1b452a8..4a4a767f3 100644
--- a/python/vyos/configdict.py
+++ b/python/vyos/configdict.py
@@ -21,8 +21,9 @@ import os
 from copy import deepcopy
 
 from vyos.util import vyos_dict_search
-from vyos.validate import is_member
 from vyos.xml import defaults
+from vyos.xml import is_tag
+from vyos.xml import is_leaf
 from vyos import ConfigError
 
 def retrieve_config(path_hash, base_path, config):
@@ -186,6 +187,47 @@ def T2665_set_dhcpv6pd_defaults(config_dict):
 
     return config_dict
 
+def is_member(conf, interface, intftype=None):
+    """
+    Checks if passed interface is member of other interface of specified type.
+    intftype is optional, if not passed it will search all known types
+    (currently bridge and bonding)
+
+    Returns:
+    None -> Interface is not a member
+    interface name -> Interface is a member of this interface
+    False -> interface type cannot have members
+    """
+    ret_val = None
+    intftypes = ['bonding', 'bridge']
+    if intftype not in intftypes + [None]:
+        raise ValueError((
+            f'unknown interface type "{intftype}" or it cannot '
+            f'have member interfaces'))
+
+    intftype = intftypes if intftype == None else [intftype]
+
+    # set config level to root
+    old_level = conf.get_level()
+    conf.set_level([])
+
+    for it in intftype:
+        base = ['interfaces', it]
+        for intf in conf.list_nodes(base):
+            memberintf = base + [intf, 'member', 'interface']
+            if is_tag(memberintf):
+                if interface in conf.list_nodes(memberintf):
+                    ret_val = intf
+                    break
+            elif is_leaf(memberintf):
+                if ( conf.exists(memberintf) and
+                        interface in conf.return_values(memberintf) ):
+                    ret_val = intf
+                    break
+
+    old_level = conf.set_level(old_level)
+    return ret_val
+
 def get_interface_dict(config, base, ifname=''):
     """
     Common utility function to retrieve and mandgle the interfaces available
@@ -236,17 +278,15 @@ def get_interface_dict(config, base, ifname=''):
 
     # Check if we are a member of a bridge device
     bridge = is_member(config, ifname, 'bridge')
-    if bridge:
-        dict.update({'is_bridge_member' : bridge})
+    if bridge: dict.update({'is_bridge_member' : bridge})
 
     # Check if we are a member of a bond device
     bond = is_member(config, ifname, 'bonding')
-    if bond:
-        dict.update({'is_bond_member' : bond})
+    if bond: dict.update({'is_bond_member' : bond})
+
 
     mac = leaf_node_changed(config, ['mac'])
-    if mac:
-        dict.update({'mac_old' : mac})
+    if mac: dict.update({'mac_old' : mac})
 
     eui64 = leaf_node_changed(config, ['ipv6', 'address', 'eui64'])
     if eui64:
diff --git a/python/vyos/validate.py b/python/vyos/validate.py
index ceeb6888a..691cf3c8e 100644
--- a/python/vyos/validate.py
+++ b/python/vyos/validate.py
@@ -19,7 +19,6 @@ import netifaces
 import ipaddress
 
 from vyos.util import cmd
-from vyos import xml
 
 # Important note when you are adding new validation functions:
 #
@@ -267,46 +266,6 @@ def assert_mac(m):
         raise ValueError(f'{m} is a VRRP MAC address')
 
 
-def is_member(conf, interface, intftype=None):
-    """
-    Checks if passed interface is member of other interface of specified type.
-    intftype is optional, if not passed it will search all known types
-    (currently bridge and bonding)
-
-    Returns:
-    None -> Interface is not a member
-    interface name -> Interface is a member of this interface
-    False -> interface type cannot have members
-    """
-    ret_val = None
-    if intftype not in ['bonding', 'bridge', None]:
-        raise ValueError((
-            f'unknown interface type "{intftype}" or it cannot '
-            f'have member interfaces'))
-
-    intftype = ['bonding', 'bridge'] if intftype == None else [intftype]
-
-    # set config level to root
-    old_level = conf.get_level()
-    conf.set_level([])
-
-    for it in intftype:
-        base = ['interfaces', it]
-        for intf in conf.list_nodes(base):
-            memberintf = base + [intf, 'member', 'interface']
-            if xml.is_tag(memberintf):
-                if interface in conf.list_nodes(memberintf):
-                    ret_val = intf
-                    break
-            elif xml.is_leaf(memberintf):
-                if ( conf.exists(memberintf) and
-                        interface in conf.return_values(memberintf) ):
-                    ret_val = intf
-                    break
-
-    old_level = conf.set_level(old_level)
-    return ret_val
-
 def has_address_configured(conf, intf):
     """
     Checks if interface has an address configured.
diff --git a/src/conf_mode/interfaces-bonding.py b/src/conf_mode/interfaces-bonding.py
index a9679b47c..5ac4feb77 100755
--- a/src/conf_mode/interfaces-bonding.py
+++ b/src/conf_mode/interfaces-bonding.py
@@ -22,6 +22,7 @@ from netifaces import interfaces
 from vyos.config import Config
 from vyos.configdict import get_interface_dict
 from vyos.configdict import leaf_node_changed
+from vyos.configdict import is_member
 from vyos.configverify import verify_address
 from vyos.configverify import verify_bridge_delete
 from vyos.configverify import verify_dhcpv6
@@ -30,7 +31,7 @@ from vyos.configverify import verify_vlan_config
 from vyos.configverify import verify_vrf
 from vyos.ifconfig import BondIf
 from vyos.ifconfig import Section
-from vyos.validate import is_member
+from vyos.util import vyos_dict_search
 from vyos.validate import has_address_configured
 from vyos import ConfigError
 from vyos import airbag
@@ -98,14 +99,13 @@ def get_config(config=None):
         # also present the interfaces to be removed from the bond as dictionary
         bond['member'].update({'interface_remove': tmp})
 
-    if 'member' in bond and 'interface' in bond['member']:
+    if vyos_dict_search('member.interface', bond):
         for interface, interface_config in bond['member']['interface'].items():
-            # Check if we are a member of another bond device
+            # Check if member interface is already member of another bridge
             tmp = is_member(conf, interface, 'bridge')
-            if tmp:
-                interface_config.update({'is_bridge_member' : tmp})
+            if tmp: interface_config.update({'is_bridge_member' : tmp})
 
-            # Check if we are a member of a bond device
+            # Check if member interface is already member of a bond
             tmp = is_member(conf, interface, 'bonding')
             if tmp and tmp != bond['ifname']:
                 interface_config.update({'is_bond_member' : tmp})
@@ -144,10 +144,9 @@ def verify(bond):
     verify_vlan_config(bond)
 
     bond_name = bond['ifname']
-    if 'member' in bond:
-        member = bond.get('member')
-        for interface, interface_config in member.get('interface', {}).items():
-            error_msg = f'Can not add interface "{interface}" to bond "{bond_name}", '
+    if vyos_dict_search('member.interface', bond):
+        for interface, interface_config in bond['member']['interface'].items():
+            error_msg = f'Can not add interface "{interface}" to bond, '
 
             if interface == 'lo':
                 raise ConfigError('Loopback interface "lo" can not be added to a bond')
diff --git a/src/conf_mode/interfaces-bridge.py b/src/conf_mode/interfaces-bridge.py
index 47c8c05f9..3bddac023 100755
--- a/src/conf_mode/interfaces-bridge.py
+++ b/src/conf_mode/interfaces-bridge.py
@@ -22,13 +22,15 @@ from netifaces import interfaces
 from vyos.config import Config
 from vyos.configdict import get_interface_dict
 from vyos.configdict import node_changed
+from vyos.configdict import is_member
 from vyos.configverify import verify_dhcpv6
 from vyos.configverify import verify_vrf
 from vyos.ifconfig import BridgeIf
-from vyos.validate import is_member, has_address_configured
+from vyos.validate import has_address_configured
 from vyos.xml import defaults
 
 from vyos.util import cmd
+from vyos.util import vyos_dict_search
 from vyos import ConfigError
 
 from vyos import airbag
@@ -54,8 +56,8 @@ def get_config(config=None):
         else:
             bridge.update({'member': {'interface_remove': tmp }})
 
-    if 'member' in bridge and 'interface' in bridge['member']:
-        # XXX TT2665 we need a copy of the dict keys for iteration, else we will get:
+    if vyos_dict_search('member.interface', bridge):
+        # XXX: T2665: we need a copy of the dict keys for iteration, else we will get:
         # RuntimeError: dictionary changed size during iteration
         for interface in list(bridge['member']['interface']):
             for key in ['cost', 'priority']:
@@ -69,20 +71,19 @@ def get_config(config=None):
         for interface, interface_config in bridge['member']['interface'].items():
             interface_config.update(default_member_values)
 
-            # Check if we are a member of another bridge device
+            # Check if member interface is already member of another bridge
             tmp = is_member(conf, interface, 'bridge')
             if tmp and tmp != bridge['ifname']:
                 interface_config.update({'is_bridge_member' : tmp})
 
-            # Check if we are a member of a bond device
+            # Check if member interface is already member of a bond
             tmp = is_member(conf, interface, 'bonding')
-            if tmp:
-                interface_config.update({'is_bond_member' : tmp})
+            if tmp: interface_config.update({'is_bond_member' : tmp})
+
 
             # Bridge members must not have an assigned address
             tmp = has_address_configured(conf, interface)
-            if tmp:
-                interface_config.update({'has_address' : ''})
+            if tmp: interface_config.update({'has_address' : ''})
 
     return bridge
 
@@ -93,11 +94,9 @@ def verify(bridge):
     verify_dhcpv6(bridge)
     verify_vrf(bridge)
 
-    if 'member' in bridge:
-        member = bridge.get('member')
-        bridge_name = bridge['ifname']
-        for interface, interface_config in member.get('interface', {}).items():
-            error_msg = f'Can not add interface "{interface}" to bridge "{bridge_name}", '
+    if vyos_dict_search('member.interface', bridge):
+        for interface, interface_config in bridge['member']['interface'].items():
+            error_msg = f'Can not add interface "{interface}" to bridge, '
 
             if interface == 'lo':
                 raise ConfigError('Loopback interface "lo" can not be added to a bridge')
diff --git a/src/conf_mode/interfaces-macsec.py b/src/conf_mode/interfaces-macsec.py
index 73b62dcf1..abf8b05c3 100755
--- a/src/conf_mode/interfaces-macsec.py
+++ b/src/conf_mode/interfaces-macsec.py
@@ -28,7 +28,6 @@ from vyos.configverify import verify_vrf
 from vyos.configverify import verify_address
 from vyos.configverify import verify_bridge_delete
 from vyos.configverify import verify_source_interface
-from vyos.validate import is_member
 from vyos import ConfigError
 from vyos import airbag
 airbag.enable()
@@ -62,11 +61,6 @@ def get_config(config=None):
                 base + ['source-interface'])
         macsec.update({'source_interface': source_interface})
 
-    if 'source_interface' in macsec:
-        # Check if source interface is used by another bridge
-        tmp = is_member(conf, macsec['source_interface'], 'bridge')
-        if tmp: macsec.update({'is_bridge_member_source_interface' : tmp})
-
     return macsec
 
 
@@ -94,10 +88,6 @@ def verify(macsec):
             raise ConfigError('Missing mandatory MACsec security '
                               'keys as encryption is enabled!')
 
-    if 'is_bridge_member_source_interface' in macsec:
-        raise ConfigError('source-interface is already member of bridge ' \
-                          '{is_bridge_member_source_interface}!'.format(**macsec))
-
     if 'source_interface' in macsec:
         # MACsec adds a 40 byte overhead (32 byte MACsec + 8 bytes VLAN 802.1ad
         # and 802.1q) - we need to check the underlaying MTU if our configured
diff --git a/src/conf_mode/interfaces-openvpn.py b/src/conf_mode/interfaces-openvpn.py
index 958b305dd..f83590209 100755
--- a/src/conf_mode/interfaces-openvpn.py
+++ b/src/conf_mode/interfaces-openvpn.py
@@ -26,10 +26,11 @@ from shutil import rmtree
 
 from vyos.config import Config
 from vyos.configdict import list_diff
+from vyos.configdict import is_member
 from vyos.ifconfig import VTunIf
 from vyos.template import render
 from vyos.util import call, chown, chmod_600, chmod_755
-from vyos.validate import is_addr_assigned, is_member, is_ipv4
+from vyos.validate import is_addr_assigned, is_ipv4
 from vyos import ConfigError
 
 from vyos import airbag
diff --git a/src/conf_mode/interfaces-pseudo-ethernet.py b/src/conf_mode/interfaces-pseudo-ethernet.py
index 98397b28f..ddbef56ac 100755
--- a/src/conf_mode/interfaces-pseudo-ethernet.py
+++ b/src/conf_mode/interfaces-pseudo-ethernet.py
@@ -25,7 +25,6 @@ from vyos.configverify import verify_bridge_delete
 from vyos.configverify import verify_source_interface
 from vyos.configverify import verify_vlan_config
 from vyos.ifconfig import MACVLANIf
-from vyos.validate import is_member
 from vyos import ConfigError
 
 from vyos import airbag
@@ -44,19 +43,7 @@ def get_config(config=None):
     peth = get_interface_dict(conf, base)
 
     mode = leaf_node_changed(conf, ['mode'])
-    if mode:
-        peth.update({'mode_old' : mode})
-
-    # Check if source-interface is member of a bridge device
-    if 'source_interface' in peth:
-        bridge = is_member(conf, peth['source_interface'], 'bridge')
-        if bridge:
-            peth.update({'source_interface_is_bridge_member' : bridge})
-
-        # Check if we are a member of a bond device
-        bond = is_member(conf, peth['source_interface'], 'bonding')
-        if bond:
-            peth.update({'source_interface_is_bond_member' : bond})
+    if mode: peth.update({'mode_old' : mode})
 
     return peth
 
@@ -69,16 +56,6 @@ def verify(peth):
     verify_vrf(peth)
     verify_address(peth)
 
-    if 'source_interface_is_bridge_member' in peth:
-        raise ConfigError(
-            'Source interface "{source_interface}" can not be used as it is already a '
-            'member of bridge "{source_interface_is_bridge_member}"!'.format(**peth))
-
-    if 'source_interface_is_bond_member' in peth:
-        raise ConfigError(
-            'Source interface "{source_interface}" can not be used as it is already a '
-            'member of bond "{source_interface_is_bond_member}"!'.format(**peth))
-
     # use common function to verify VLAN configuration
     verify_vlan_config(peth)
     return None
diff --git a/src/conf_mode/interfaces-tunnel.py b/src/conf_mode/interfaces-tunnel.py
index 11d8d6edc..f1d885b15 100755
--- a/src/conf_mode/interfaces-tunnel.py
+++ b/src/conf_mode/interfaces-tunnel.py
@@ -22,10 +22,11 @@ from copy import deepcopy
 from netifaces import interfaces
 
 from vyos.config import Config
+from vyos.configdict import is_member
 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_member
+from vyos.validate import is_ipv4, is_ipv6
 from vyos import ConfigError
 from vyos.dicts import FixedDict
 
@@ -170,8 +171,8 @@ class ConfigurationState(object):
         """
         >>> conf.get_values('addresses', 'address')
         will place a list of the new IP present in 'interface dummy dum1 address'
-        into the dictionnary entry "-add" (here 'addresses-add') using 
-        Config.return_values and will add the the one which were removed in into 
+        into the dictionnary entry "-add" (here 'addresses-add') using
+        Config.return_values and will add the the one which were removed in into
         the entry "-del" (here addresses-del')
         """
         add_name = f'{name}-add'
@@ -263,7 +264,7 @@ class ConfigurationState(object):
             d = d[lpath[-1]]
         # XXX: it should have provided me the content and not the key
         self._conf.set_level(l)
-        return d    
+        return d
 
     def to_api(self):
         """
-- 
cgit v1.2.3


From 83a9ce7991195c709736eec234fea3d60cde7582 Mon Sep 17 00:00:00 2001
From: Christian Poessinger <christian@poessinger.com>
Date: Tue, 22 Sep 2020 18:37:00 +0200
Subject: ifconfig: T2653: bond: bridge: ensure member interface is not a
 source-interface

As we already check that a bond/bridge member interface is not a member of any
other bridge or bond, the check must be extended. We also need to ensure that
the bond member interface is not used as a source-interface to pppoe, macsec,
tunnel, pseudo-ethernet, vxlan interfaces.
---
 python/vyos/configdict.py           | 46 +++++++++++++++++++++++++++++++++++++
 python/vyos/configverify.py         | 15 ++++++++++--
 src/conf_mode/interfaces-bonding.py |  9 ++++++++
 src/conf_mode/interfaces-bridge.py  |  8 +++++++
 4 files changed, 76 insertions(+), 2 deletions(-)

(limited to 'python/vyos/configdict.py')

diff --git a/python/vyos/configdict.py b/python/vyos/configdict.py
index 4a4a767f3..58ecd3f17 100644
--- a/python/vyos/configdict.py
+++ b/python/vyos/configdict.py
@@ -228,6 +228,41 @@ def is_member(conf, interface, intftype=None):
     old_level = conf.set_level(old_level)
     return ret_val
 
+def is_source_interface(conf, interface, intftype=None):
+    """
+    Checks if passed interface is configured as source-interface of other
+    interfaces of specified type. intftype is optional, if not passed it will
+    search all known types (currently pppoe, macsec, pseudo-ethernet, tunnel
+    and vxlan)
+
+    Returns:
+    None -> Interface is not a member
+    interface name -> Interface is a member of this interface
+    False -> interface type cannot have members
+    """
+    ret_val = None
+    intftypes = ['macsec', 'pppoe', 'pseudo-ethernet', 'tunnel', 'vxlan']
+    if intftype not in intftypes + [None]:
+        raise ValueError(f'unknown interface type "{intftype}" or it can not '
+            'have a source-interface')
+
+    intftype = intftypes if intftype == None else [intftype]
+
+    # set config level to root
+    old_level = conf.get_level()
+    conf.set_level([])
+
+    for it in intftype:
+        base = ['interfaces', it]
+        for intf in conf.list_nodes(base):
+            lower_intf = base + [intf, 'source-interface']
+            if conf.exists(lower_intf) and interface in conf.return_values(lower_intf):
+                ret_val = intf
+                break
+
+    old_level = conf.set_level(old_level)
+    return ret_val
+
 def get_interface_dict(config, base, ifname=''):
     """
     Common utility function to retrieve and mandgle the interfaces available
@@ -284,6 +319,17 @@ def get_interface_dict(config, base, ifname=''):
     bond = is_member(config, ifname, 'bonding')
     if bond: dict.update({'is_bond_member' : bond})
 
+    # Some interfaces come with a source_interface which must also not be part
+    # of any other bond or bridge interface as it is exclusivly assigned as the
+    # Kernels "lower" interface to this new "virtual/upper" interface.
+    if 'source_interface' in dict:
+        # Check if source interface is member of another bridge
+        tmp = is_member(config, dict['source_interface'], 'bridge')
+        if tmp: dict.update({'source_interface_is_bridge_member' : tmp})
+
+        # Check if source interface is member of another bridge
+        tmp = is_member(config, dict['source_interface'], 'bonding')
+        if tmp: dict.update({'source_interface_is_bond_member' : tmp})
 
     mac = leaf_node_changed(config, ['mac'])
     if mac: dict.update({'mac_old' : mac})
diff --git a/python/vyos/configverify.py b/python/vyos/configverify.py
index 7e1930878..bf4e26fa7 100644
--- a/python/vyos/configverify.py
+++ b/python/vyos/configverify.py
@@ -82,9 +82,20 @@ def verify_source_interface(config):
     if 'source_interface' not in config:
         raise ConfigError('Physical source-interface required for '
                           'interface "{ifname}"'.format(**config))
+
     if config['source_interface'] not in interfaces():
-        raise ConfigError('Source interface {source_interface} does not '
-                          'exist'.format(**config))
+        raise ConfigError('Specified source-interface {source_interface} does '
+                          'not exist'.format(**config))
+
+    if 'source_interface_is_bridge_member' in config:
+        raise ConfigError('Invalid source-interface {source_interface}. Interface '
+                          'is already a member of bridge '
+                          '{source_interface_is_bridge_member}'.format(**config))
+
+    if 'source_interface_is_bond_member' in config:
+        raise ConfigError('Invalid source-interface {source_interface}. Interface '
+                          'is already a member of bond '
+                          '{source_interface_is_bond_member}'.format(**config))
 
 def verify_dhcpv6(config):
     """
diff --git a/src/conf_mode/interfaces-bonding.py b/src/conf_mode/interfaces-bonding.py
index 5ac4feb77..aece2a04b 100755
--- a/src/conf_mode/interfaces-bonding.py
+++ b/src/conf_mode/interfaces-bonding.py
@@ -23,6 +23,7 @@ from vyos.config import Config
 from vyos.configdict import get_interface_dict
 from vyos.configdict import leaf_node_changed
 from vyos.configdict import is_member
+from vyos.configdict import is_source_interface
 from vyos.configverify import verify_address
 from vyos.configverify import verify_bridge_delete
 from vyos.configverify import verify_dhcpv6
@@ -110,6 +111,10 @@ def get_config(config=None):
             if tmp and tmp != bond['ifname']:
                 interface_config.update({'is_bond_member' : tmp})
 
+            # Check if member interface is used as source-interface on another interface
+            tmp = is_source_interface(conf, interface)
+            if tmp: interface_config.update({'is_source_interface' : tmp})
+
             # bond members must not have an assigned address
             tmp = has_address_configured(conf, interface)
             if tmp: interface_config.update({'has_address' : ''})
@@ -162,6 +167,10 @@ def verify(bond):
                 tmp = interface_config['is_bond_member']
                 raise ConfigError(error_msg + f'it is already a member of bond "{tmp}"!')
 
+            if 'is_source_interface' in interface_config:
+                tmp = interface_config['is_source_interface']
+                raise ConfigError(error_msg + f'it is the source-interface of "{tmp}"!')
+
             if 'has_address' in interface_config:
                 raise ConfigError(error_msg + 'it has an address assigned!')
 
diff --git a/src/conf_mode/interfaces-bridge.py b/src/conf_mode/interfaces-bridge.py
index 3bddac023..485decb17 100755
--- a/src/conf_mode/interfaces-bridge.py
+++ b/src/conf_mode/interfaces-bridge.py
@@ -23,6 +23,7 @@ from vyos.config import Config
 from vyos.configdict import get_interface_dict
 from vyos.configdict import node_changed
 from vyos.configdict import is_member
+from vyos.configdict import is_source_interface
 from vyos.configverify import verify_dhcpv6
 from vyos.configverify import verify_vrf
 from vyos.ifconfig import BridgeIf
@@ -80,6 +81,9 @@ def get_config(config=None):
             tmp = is_member(conf, interface, 'bonding')
             if tmp: interface_config.update({'is_bond_member' : tmp})
 
+            # Check if member interface is used as source-interface on another interface
+            tmp = is_source_interface(conf, interface)
+            if tmp: interface_config.update({'is_source_interface' : tmp})
 
             # Bridge members must not have an assigned address
             tmp = has_address_configured(conf, interface)
@@ -112,6 +116,10 @@ def verify(bridge):
                 tmp = interface_config['is_bond_member']
                 raise ConfigError(error_msg + f'it is already a member of bond "{tmp}"!')
 
+            if 'is_source_interface' in interface_config:
+                tmp = interface_config['is_source_interface']
+                raise ConfigError(error_msg + f'it is the source-interface of "{tmp}"!')
+
             if 'has_address' in interface_config:
                 raise ConfigError(error_msg + 'it has an address assigned!')
 
-- 
cgit v1.2.3