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')

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 0a21905786d31e759416dd335d87726f9ed46ffa Mon Sep 17 00:00:00 2001
From: Christian Poessinger <christian@poessinger.com>
Date: Sun, 20 Sep 2020 13:40:51 +0200
Subject: vif-s: ifconfig: T2903: use explicit VLAN protocol over raw numbers

In the past we had to provide the ethertype value used for the VLAN
protocol (0x88A8 -> 802.1ad or 0x8100 -> 802.1q).

This should be changed to a more user friendly CLI node (protocol over
ethertype) and 802.1ad over it's raw value 0x88A8. There is no need in
presenting RAW information from the ethernet header to the user. Also iproute2
calls it protocol which makes way more sense over the "raw" value.
---
 interface-definitions/include/vif-s.xml.i | 19 ++++++-----
 python/vyos/ifconfig/interface.py         | 18 +++-------
 src/migration-scripts/interfaces/12-to-13 | 57 +++++++++++++++++++++++++++++++
 3 files changed, 72 insertions(+), 22 deletions(-)
 create mode 100755 src/migration-scripts/interfaces/12-to-13

(limited to 'python')

diff --git a/interface-definitions/include/vif-s.xml.i b/interface-definitions/include/vif-s.xml.i
index a6d7c81ce..cd0afe742 100644
--- a/interface-definitions/include/vif-s.xml.i
+++ b/interface-definitions/include/vif-s.xml.i
@@ -13,25 +13,26 @@
     #include <include/dhcpv6-options.xml.i>
     #include <include/interface-disable-link-detect.xml.i>
     #include <include/interface-disable.xml.i>
-    <leafNode name="ethertype">
+    <leafNode name="protocol">
       <properties>
-        <help>Set Ethertype</help>
+        <help>Protocol used for service VLAN (default: 802.1ad)</help>
         <completionHelp>
-          <list>0x88A8 0x8100</list>
+          <list>802.1ad 802.1q</list>
         </completionHelp>
         <valueHelp>
-          <format>0x88A8</format>
-          <description>802.1ad</description>
+          <format>802.1ad</format>
+          <description>Provider Bridging (IEEE 802.1ad, Q-inQ), ethertype 0x88a8</description>
         </valueHelp>
         <valueHelp>
-          <format>0x8100</format>
-          <description>802.1q</description>
+          <format>802.1q</format>
+          <description>VLAN-tagged frame (IEEE 802.1q), ethertype 0x8100</description>
         </valueHelp>
         <constraint>
-          <regex>(0x88A8|0x8100)</regex>
+          <regex>(802.1q|802.1ad)</regex>
         </constraint>
-        <constraintErrorMessage>Ethertype must be 0x88A8 or 0x8100</constraintErrorMessage>
+        <constraintErrorMessage>Ethertype must be 802.1ad or 802.1q</constraintErrorMessage>
       </properties>
+      <defaultValue>802.1ad</defaultValue>
     </leafNode>
     <node name="ip">
       <children>
diff --git a/python/vyos/ifconfig/interface.py b/python/vyos/ifconfig/interface.py
index be97b411b..4e420dc1d 100644
--- a/python/vyos/ifconfig/interface.py
+++ b/python/vyos/ifconfig/interface.py
@@ -50,14 +50,6 @@ from vyos.ifconfig.vrrp import VRRP
 from vyos.ifconfig.operational import Operational
 from vyos.ifconfig import Section
 
-def get_ethertype(ethertype_val):
-    if ethertype_val == '0x88A8':
-        return '802.1ad'
-    elif ethertype_val == '0x8100':
-        return '802.1q'
-    else:
-        raise ConfigError('invalid ethertype "{}"'.format(ethertype_val))
-
 class Interface(Control):
     # This is the class which will be used to create
     # self.operational, it allows subclasses, such as
@@ -1013,7 +1005,7 @@ class Interface(Control):
         # create/update 802.1ad (Q-in-Q VLANs)
         for vif_s_id, vif_s_config in config.get('vif_s', {}).items():
             tmp = deepcopy(VLANIf.get_config())
-            tmp['ethertype'] = get_ethertype(vif_s_config.get('ethertype', '0x88A8'))
+            tmp['protocol'] = vif_s_config['protocol']
             tmp['source_interface'] = ifname
             tmp['vlan_id'] = vif_s_id
 
@@ -1061,13 +1053,13 @@ class VLANIf(Interface):
         'type': 'vlan',
         'source_interface': '',
         'vlan_id': '',
-        'ethertype': '',
+        'protocol': '',
         'ingress_qos': '',
         'egress_qos': '',
     }
 
     options = Interface.options + \
-        ['source_interface', 'vlan_id', 'ethertype', 'ingress_qos', 'egress_qos']
+        ['source_interface', 'vlan_id', 'protocol', 'ingress_qos', 'egress_qos']
 
     def remove(self):
         """
@@ -1096,8 +1088,8 @@ class VLANIf(Interface):
             return
 
         cmd = 'ip link add link {source_interface} name {ifname} type vlan id {vlan_id}'
-        if self.config['ethertype']:
-            cmd += ' proto {ethertype}'
+        if self.config['protocol']:
+            cmd += ' protocol {protocol}'
         if self.config['ingress_qos']:
             cmd += ' ingress-qos-map {ingress_qos}'
         if self.config['egress_qos']:
diff --git a/src/migration-scripts/interfaces/12-to-13 b/src/migration-scripts/interfaces/12-to-13
new file mode 100755
index 000000000..17d1d0b0a
--- /dev/null
+++ b/src/migration-scripts/interfaces/12-to-13
@@ -0,0 +1,57 @@
+#!/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 <http://www.gnu.org/licenses/>.
+
+# - T2903: Change vif-s ethertype from numeric number to literal
+#   - 0x88a8 -> 802.1ad
+#   - 0x8100 -> 802.1q
+
+from sys import exit, argv
+from vyos.configtree import ConfigTree
+
+if __name__ == '__main__':
+    if (len(argv) < 1):
+        print("Must specify file name!")
+        exit(1)
+
+    file_name = argv[1]
+    with open(file_name, 'r') as f:
+        config_file = f.read()
+
+    config = ConfigTree(config_file)
+
+    for type in config.list_nodes(['interfaces']):
+        for interface in config.list_nodes(['interfaces', type]):
+            if not config.exists(['interfaces', type, interface, 'vif-s']):
+                continue
+
+            for vif_s in config.list_nodes(['interfaces', type, interface, 'vif-s']):
+                base_path = ['interfaces', type, interface, 'vif-s', vif_s]
+                if config.exists(base_path + ['ethertype']):
+                    protocol = '802.1ad'
+                    tmp = config.return_value(base_path + ['ethertype'])
+                    if tmp == '0x8100':
+                        protocol = '802.1q'
+
+                    config.set(base_path + ['protocol'], value=protocol)
+                    config.delete(base_path + ['ethertype'])
+
+    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))
+        exit(1)
+
-- 
cgit v1.2.3


From 103e8404cdea70dad486940f209b9683f1c7b936 Mon Sep 17 00:00:00 2001
From: Christian Poessinger <christian@poessinger.com>
Date: Sun, 20 Sep 2020 15:18:50 +0200
Subject: ifconfig: T2653: remove duplicates of get_config()

A lot of derived classes from Interface implemented their own get_config()
method which more or less was the same everywhere. We also hat different
qualifiers like @staticmethod or @classmethod.

This is now changed to only have the @classmethod in Interface base class which
will return the necessary dictionary keys for the required interfaces. This
change is a mid reduction in lines of code which is always a very nice thing!
---
 python/vyos/ifconfig/geneve.py              | 13 -----------
 python/vyos/ifconfig/interface.py           | 25 +++++++++------------
 python/vyos/ifconfig/l2tpv3.py              | 34 +++++++++--------------------
 python/vyos/ifconfig/macsec.py              | 16 --------------
 python/vyos/ifconfig/macvlan.py             | 15 +------------
 python/vyos/ifconfig/vxlan.py               | 14 ------------
 python/vyos/ifconfig/wireless.py            | 16 --------------
 src/conf_mode/interfaces-geneve.py          | 11 +++++-----
 src/conf_mode/interfaces-l2tpv3.py          | 10 ++++-----
 src/conf_mode/interfaces-macsec.py          | 12 +++++-----
 src/conf_mode/interfaces-pseudo-ethernet.py | 11 +++++-----
 src/conf_mode/interfaces-vxlan.py           | 10 ++++-----
 src/conf_mode/interfaces-wireless.py        | 10 ++++-----
 13 files changed, 51 insertions(+), 146 deletions(-)

(limited to 'python')

diff --git a/python/vyos/ifconfig/geneve.py b/python/vyos/ifconfig/geneve.py
index 0a13043cc..5c4597be8 100644
--- a/python/vyos/ifconfig/geneve.py
+++ b/python/vyos/ifconfig/geneve.py
@@ -13,7 +13,6 @@
 # You should have received a copy of the GNU Lesser General Public
 # License along with this library.  If not, see <http://www.gnu.org/licenses/>.
 
-from copy import deepcopy
 from vyos.ifconfig.interface import Interface
 
 @Interface.register
@@ -51,18 +50,6 @@ class GeneveIf(Interface):
         # interface is always A/D down. It needs to be enabled explicitly
         self.set_admin_state('down')
 
-    @classmethod
-    def get_config(cls):
-        """
-        GENEVE interfaces require a configuration when they are added using
-        iproute2. This static method will provide the configuration dictionary
-        used by this class.
-
-        Example:
-        >> dict = GeneveIf().get_config()
-        """
-        return deepcopy(cls.default)
-
     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
diff --git a/python/vyos/ifconfig/interface.py b/python/vyos/ifconfig/interface.py
index 4e420dc1d..807191b3d 100644
--- a/python/vyos/ifconfig/interface.py
+++ b/python/vyos/ifconfig/interface.py
@@ -174,6 +174,15 @@ class Interface(Control):
     def exists(cls, ifname):
         return os.path.exists(f'/sys/class/net/{ifname}')
 
+    @classmethod
+    def get_config(cls):
+        """
+        Some but not all interfaces require a configuration when they are added
+        using iproute2. This method will provide the configuration dictionary
+        used by this class.
+        """
+        return deepcopy(cls.default)
+
     def __init__(self, ifname, **kargs):
         """
         This is the base interface class which supports basic IP/MAC address
@@ -1084,7 +1093,7 @@ class VLANIf(Interface):
 
     def _create(self):
         # bail out early if interface already exists
-        if os.path.exists(f'/sys/class/net/{self.ifname}'):
+        if self.exists(f'{self.ifname}'):
             return
 
         cmd = 'ip link add link {source_interface} name {ifname} type vlan id {vlan_id}'
@@ -1100,20 +1109,6 @@ class VLANIf(Interface):
         # interface is always A/D down. It needs to be enabled explicitly
         self.set_admin_state('down')
 
-    @staticmethod
-    def get_config():
-        """
-        MACsec interfaces require a configuration when they are added using
-        iproute2. This static method will provide the configuration dictionary
-        used by this class.
-
-        Example:
-        >> dict = VLANIf().get_config()
-        """
-        config = deepcopy(__class__.default)
-        del config['type']
-        return config
-
     def set_admin_state(self, state):
         """
         Set interface administrative state to be 'up' or 'down'
diff --git a/python/vyos/ifconfig/l2tpv3.py b/python/vyos/ifconfig/l2tpv3.py
index 33740921e..5fd90f9cf 100644
--- a/python/vyos/ifconfig/l2tpv3.py
+++ b/python/vyos/ifconfig/l2tpv3.py
@@ -13,7 +13,6 @@
 # You should have received a copy of the GNU Lesser General Public
 # License along with this library.  If not, see <http://www.gnu.org/licenses/>.
 
-import os
 from vyos.ifconfig.interface import Interface
 
 @Interface.register
@@ -28,6 +27,15 @@ class L2TPv3If(Interface):
 
     default = {
         'type': 'l2tp',
+        'peer_tunnel_id': '',
+        'local_port': 0,
+        'remote_port': 0,
+        'encapsulation': 'udp',
+        'local_address': '',
+        'remote_address': '',
+        'session_id': '',
+        'tunnel_id': '',
+        'peer_session_id': ''
     }
     definition = {
         **Interface.definition,
@@ -73,7 +81,7 @@ class L2TPv3If(Interface):
         >>> i.remove()
         """
 
-        if os.path.exists('/sys/class/net/{}'.format(self.config['ifname'])):
+        if self.exists(self.config['ifname']):
             # interface is always A/D down. It needs to be enabled explicitly
             self.set_admin_state('down')
 
@@ -86,25 +94,3 @@ class L2TPv3If(Interface):
                 cmd = 'ip l2tp del tunnel tunnel_id {tunnel_id}'
                 self._cmd(cmd.format(**self.config))
 
-    @staticmethod
-    def get_config():
-        """
-        L2TPv3 interfaces require a configuration when they are added using
-        iproute2. This static method will provide the configuration dictionary
-        used by this class.
-
-        Example:
-        >> dict = L2TPv3If().get_config()
-        """
-        config = {
-            'peer_tunnel_id': '',
-            'local_port': 0,
-            'remote_port': 0,
-            'encapsulation': 'udp',
-            'local_address': '',
-            'remote_address': '',
-            'session_id': '',
-            'tunnel_id': '',
-            'peer_session_id': ''
-        }
-        return config
diff --git a/python/vyos/ifconfig/macsec.py b/python/vyos/ifconfig/macsec.py
index 6f570d162..456686ea6 100644
--- a/python/vyos/ifconfig/macsec.py
+++ b/python/vyos/ifconfig/macsec.py
@@ -56,22 +56,6 @@ class MACsecIf(Interface):
         # interface is always A/D down. It needs to be enabled explicitly
         self.set_admin_state('down')
 
-    @staticmethod
-    def get_config():
-        """
-        MACsec interfaces require a configuration when they are added using
-        iproute2. This static method will provide the configuration dictionary
-        used by this class.
-
-        Example:
-        >> dict = MACsecIf().get_config()
-        """
-        config = {
-            'security_cipher': '',
-            'source_interface': '',
-        }
-        return config
-
     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
diff --git a/python/vyos/ifconfig/macvlan.py b/python/vyos/ifconfig/macvlan.py
index 9c1d09c1c..2447fec77 100644
--- a/python/vyos/ifconfig/macvlan.py
+++ b/python/vyos/ifconfig/macvlan.py
@@ -1,4 +1,4 @@
-# Copyright 2019 VyOS maintainers and contributors <maintainers@vyos.io>
+# Copyright 2019-2020 VyOS maintainers and contributors <maintainers@vyos.io>
 #
 # This library is free software; you can redistribute it and/or
 # modify it under the terms of the GNU Lesser General Public
@@ -13,7 +13,6 @@
 # You should have received a copy of the GNU Lesser General Public
 # License along with this library.  If not, see <http://www.gnu.org/licenses/>.
 
-from copy import deepcopy
 from vyos.ifconfig.interface import Interface
 
 @Interface.register
@@ -53,18 +52,6 @@ class MACVLANIf(Interface):
         cmd = f'ip link set dev {ifname} type macvlan mode {mode}'
         return self._cmd(cmd)
 
-    @classmethod
-    def get_config(cls):
-        """
-        MACVLAN interfaces require a configuration when they are added using
-        iproute2. This method will provide the configuration dictionary used
-        by this class.
-
-        Example:
-        >> dict = MACVLANIf().get_config()
-        """
-        return deepcopy(cls.default)
-
     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
diff --git a/python/vyos/ifconfig/vxlan.py b/python/vyos/ifconfig/vxlan.py
index dba62b61a..ad1f605ed 100644
--- a/python/vyos/ifconfig/vxlan.py
+++ b/python/vyos/ifconfig/vxlan.py
@@ -13,8 +13,6 @@
 # You should have received a copy of the GNU Lesser General Public
 # License along with this library.  If not, see <http://www.gnu.org/licenses/>.
 
-from copy import deepcopy
-
 from vyos import ConfigError
 from vyos.ifconfig.interface import Interface
 
@@ -97,18 +95,6 @@ class VXLANIf(Interface):
 
         self._cmd(cmd)
 
-    @classmethod
-    def get_config(cls):
-        """
-        VXLAN interfaces require a configuration when they are added using
-        iproute2. This static method will provide the configuration dictionary
-        used by this class.
-
-        Example:
-        >> dict = VXLANIf().get_config()
-        """
-        return deepcopy(cls.default)
-
     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
diff --git a/python/vyos/ifconfig/wireless.py b/python/vyos/ifconfig/wireless.py
index 346577119..37703d242 100644
--- a/python/vyos/ifconfig/wireless.py
+++ b/python/vyos/ifconfig/wireless.py
@@ -50,22 +50,6 @@ class WiFiIf(Interface):
             .format(**self.config)
         self._cmd(cmd)
 
-    @staticmethod
-    def get_config():
-        """
-        WiFi interfaces require a configuration when they are added using
-        iw (type/phy). This static method will provide the configuration
-        ictionary used by this class.
-
-        Example:
-        >> conf = WiFiIf().get_config()
-        """
-        config = {
-            'phy': 'phy0'
-        }
-        return config
-
-
     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
diff --git a/src/conf_mode/interfaces-geneve.py b/src/conf_mode/interfaces-geneve.py
index cc2cf025a..af7c121f4 100755
--- a/src/conf_mode/interfaces-geneve.py
+++ b/src/conf_mode/interfaces-geneve.py
@@ -17,7 +17,6 @@
 import os
 
 from sys import exit
-from copy import deepcopy
 from netifaces import interfaces
 
 from vyos.config import Config
@@ -62,7 +61,6 @@ def verify(geneve):
 def generate(geneve):
     return None
 
-
 def apply(geneve):
     # Check if GENEVE interface already exists
     if geneve['ifname'] in interfaces():
@@ -72,10 +70,11 @@ def apply(geneve):
         g.remove()
 
     if 'deleted' not in geneve:
-        # GENEVE interface needs to be created on-block
-        # instead of passing a ton of arguments, I just use a dict
-        # that is managed by vyos.ifconfig
-        conf = deepcopy(GeneveIf.get_config())
+        # This is a special type of interface which needs additional parameters
+        # when created using iproute2. Instead of passing a ton of arguments,
+        # use a dictionary provided by the interface class which holds all the
+        # options necessary.
+        conf = GeneveIf.get_config()
 
         # Assign GENEVE instance configuration parameters to config dict
         conf['vni'] = geneve['vni']
diff --git a/src/conf_mode/interfaces-l2tpv3.py b/src/conf_mode/interfaces-l2tpv3.py
index 144cee5fe..2653ff19c 100755
--- a/src/conf_mode/interfaces-l2tpv3.py
+++ b/src/conf_mode/interfaces-l2tpv3.py
@@ -17,7 +17,6 @@
 import os
 
 from sys import exit
-from copy import deepcopy
 from netifaces import interfaces
 
 from vyos.config import Config
@@ -88,10 +87,11 @@ def generate(l2tpv3):
     return None
 
 def apply(l2tpv3):
-    # L2TPv3 interface needs to be created/deleted on-block, instead of
-    # passing a ton of arguments, I just use a dict that is managed by
-    # vyos.ifconfig
-    conf = deepcopy(L2TPv3If.get_config())
+    # This is a special type of interface which needs additional parameters
+    # when created using iproute2. Instead of passing a ton of arguments,
+    # use a dictionary provided by the interface class which holds all the
+    # options necessary.
+    conf = L2TPv3If.get_config()
 
     # Check if L2TPv3 interface already exists
     if l2tpv3['ifname'] in interfaces():
diff --git a/src/conf_mode/interfaces-macsec.py b/src/conf_mode/interfaces-macsec.py
index 2866ccc0a..73c80866a 100755
--- a/src/conf_mode/interfaces-macsec.py
+++ b/src/conf_mode/interfaces-macsec.py
@@ -16,7 +16,6 @@
 
 import os
 
-from copy import deepcopy
 from sys import exit
 
 from vyos.config import Config
@@ -102,12 +101,11 @@ def apply(macsec):
             os.unlink(wpa_suppl_conf.format(**macsec))
 
     else:
-        # MACsec interfaces require a configuration when they are added using
-        # iproute2. This static method will provide the configuration
-        # dictionary used by this class.
-
-        # XXX: subject of removal after completing T2653
-        conf = deepcopy(MACsecIf.get_config())
+        # This is a special type of interface which needs additional parameters
+        # when created using iproute2. Instead of passing a ton of arguments,
+        # use a dictionary provided by the interface class which holds all the
+        # options necessary.
+        conf = MACsecIf.get_config()
         conf['source_interface'] = macsec['source_interface']
         conf['security_cipher'] = macsec['security']['cipher']
 
diff --git a/src/conf_mode/interfaces-pseudo-ethernet.py b/src/conf_mode/interfaces-pseudo-ethernet.py
index 59edca1cc..98397b28f 100755
--- a/src/conf_mode/interfaces-pseudo-ethernet.py
+++ b/src/conf_mode/interfaces-pseudo-ethernet.py
@@ -14,9 +14,6 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-import os
-
-from copy import deepcopy
 from sys import exit
 
 from vyos.config import Config
@@ -101,9 +98,11 @@ def apply(peth):
     if 'mode_old' in peth:
         MACVLANIf(peth['ifname']).remove()
 
-    # MACVLAN interface needs to be created on-block instead of passing a ton
-    # of arguments, I just use a dict that is managed by vyos.ifconfig
-    conf = deepcopy(MACVLANIf.get_config())
+    # This is a special type of interface which needs additional parameters
+    # when created using iproute2. Instead of passing a ton of arguments,
+    # use a dictionary provided by the interface class which holds all the
+    # options necessary.
+    conf = MACVLANIf.get_config()
 
     # Assign MACVLAN instance configuration parameters to config dict
     conf['source_interface'] = peth['source_interface']
diff --git a/src/conf_mode/interfaces-vxlan.py b/src/conf_mode/interfaces-vxlan.py
index bea3aa25b..a00c58608 100755
--- a/src/conf_mode/interfaces-vxlan.py
+++ b/src/conf_mode/interfaces-vxlan.py
@@ -17,7 +17,6 @@
 import os
 
 from sys import exit
-from copy import deepcopy
 from netifaces import interfaces
 
 from vyos.config import Config
@@ -95,10 +94,11 @@ def apply(vxlan):
         v.remove()
 
     if 'deleted' not in vxlan:
-        # VXLAN interface needs to be created on-block
-        # instead of passing a ton of arguments, I just use a dict
-        # that is managed by vyos.ifconfig
-        conf = deepcopy(VXLANIf.get_config())
+        # This is a special type of interface which needs additional parameters
+        # when created using iproute2. Instead of passing a ton of arguments,
+        # use a dictionary provided by the interface class which holds all the
+        # options necessary.
+        conf = VXLANIf.get_config()
 
         # Assign VXLAN instance configuration parameters to config dict
         for tmp in ['vni', 'group', 'source_address', 'source_interface', 'remote', 'port']:
diff --git a/src/conf_mode/interfaces-wireless.py b/src/conf_mode/interfaces-wireless.py
index c6c843e7b..be59b72b5 100755
--- a/src/conf_mode/interfaces-wireless.py
+++ b/src/conf_mode/interfaces-wireless.py
@@ -18,7 +18,6 @@ import os
 
 from sys import exit
 from re import findall
-from copy import deepcopy
 from netaddr import EUI, mac_unix_expanded
 
 from vyos.config import Config
@@ -233,10 +232,11 @@ def apply(wifi):
     if 'deleted' in wifi:
         WiFiIf(interface).remove()
     else:
-        # WiFi interface needs to be created on-block (e.g. mode or physical
-        # interface) instead of passing a ton of arguments, I just use a dict
-        # that is managed by vyos.ifconfig
-        conf = deepcopy(WiFiIf.get_config())
+        # This is a special type of interface which needs additional parameters
+        # when created using iproute2. Instead of passing a ton of arguments,
+        # use a dictionary provided by the interface class which holds all the
+        # options necessary.
+        conf = WiFiIf.get_config()
 
         # Assign WiFi instance configuration parameters to config dict
         conf['phy'] = wifi['physical_device']
-- 
cgit v1.2.3


From 79b1ab8dc67c9011a3d5e5397ad4d73a6c537d80 Mon Sep 17 00:00:00 2001
From: Christian Poessinger <christian@poessinger.com>
Date: Mon, 21 Sep 2020 22:21:12 +0200
Subject: bridge: ifconfig: T2653: only delete member interfaces which still
 exist

When removing e.g. a macsec interface and also its associated member interface
from the bridge, it will happen that the macsec interface instance is long gone
before we reach the code in the bridge interface which will remove it from the
bridge itself.

When this is the case, we can not call BridgeIf.del_port() as it will throw an
exception that the interface does not exist. We now only remove a bridge member
if the interface in question is still available in the kernel.
---
 python/vyos/ifconfig/bridge.py | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

(limited to 'python')

diff --git a/python/vyos/ifconfig/bridge.py b/python/vyos/ifconfig/bridge.py
index 4c76fe996..c133a56fc 100644
--- a/python/vyos/ifconfig/bridge.py
+++ b/python/vyos/ifconfig/bridge.py
@@ -13,6 +13,8 @@
 # You should have received a copy of the GNU Lesser General Public
 # License along with this library.  If not, see <http://www.gnu.org/licenses/>.
 
+from netifaces import interfaces
+
 from vyos.ifconfig.interface import Interface
 from vyos.ifconfig.stp import STP
 from vyos.validate import assert_boolean
@@ -228,8 +230,8 @@ class BridgeIf(Interface):
 
         # remove interface from bridge
         tmp = vyos_dict_search('member.interface_remove', config)
-        if tmp:
-            for member in tmp:
+        for member in (tmp or []):
+            if member in interfaces():
                 self.del_port(member)
 
         STPBridgeIf = STP.enable(BridgeIf)
-- 
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')

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')

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


From 806f35b5856c3f8dae634718a6a9e82cc90bb63a Mon Sep 17 00:00:00 2001
From: Christian Poessinger <christian@poessinger.com>
Date: Thu, 24 Sep 2020 19:55:54 +0200
Subject: wireless: T2241: add "wds" CLI option

---
 data/templates/wifi/hostapd.conf.tmpl            |  8 +++++++
 interface-definitions/interfaces-wireless.xml.in |  6 +++++
 python/vyos/ifconfig/wireless.py                 | 29 ++++++++++++------------
 src/conf_mode/interfaces-wireless.py             |  1 +
 4 files changed, 30 insertions(+), 14 deletions(-)

(limited to 'python')

diff --git a/data/templates/wifi/hostapd.conf.tmpl b/data/templates/wifi/hostapd.conf.tmpl
index 3980fb896..c5e4240d1 100644
--- a/data/templates/wifi/hostapd.conf.tmpl
+++ b/data/templates/wifi/hostapd.conf.tmpl
@@ -448,6 +448,14 @@ macaddr_acl=0
 max_num_sta={{ max_stations }}
 {% endif %}
 
+{% if wds is defined %}
+# WDS (4-address frame) mode with per-station virtual interfaces
+# (only supported with driver=nl80211)
+# This mode allows associated stations to use 4-address frames to allow layer 2
+# bridging to be used.
+wds_sta=1
+{% endif %}
+
 {% if isolate_stations is defined %}
 # Client isolation can be used to prevent low-level bridging of frames between
 # associated stations in the BSS. By default, this bridging is allowed.
diff --git a/interface-definitions/interfaces-wireless.xml.in b/interface-definitions/interfaces-wireless.xml.in
index a0caf810f..8c594e758 100644
--- a/interface-definitions/interfaces-wireless.xml.in
+++ b/interface-definitions/interfaces-wireless.xml.in
@@ -770,6 +770,12 @@
           </leafNode>
           #include <include/vif.xml.i>
           #include <include/vif-s.xml.i>
+          <leafNode name="wds">
+            <properties>
+              <help>Enable WDS (Wireless Distribution System)</help>
+              <valueless/>
+            </properties>
+          </leafNode>
         </children>
       </tagNode>
     </children>
diff --git a/python/vyos/ifconfig/wireless.py b/python/vyos/ifconfig/wireless.py
index 37703d242..deca68bf0 100644
--- a/python/vyos/ifconfig/wireless.py
+++ b/python/vyos/ifconfig/wireless.py
@@ -23,8 +23,10 @@ class WiFiIf(Interface):
 
     default = {
         'type': 'wifi',
-        'phy': 'phy0'
+        'phy': '',
+        'wds': 'off',
     }
+
     definition = {
         **Interface.definition,
         **{
@@ -33,12 +35,19 @@ class WiFiIf(Interface):
             'bridgeable': True,
         }
     }
+
     options = Interface.options + \
         ['phy', 'op_mode']
 
+    _command_set = {**Interface._command_set, **{
+        '4addr': {
+            'shellcmd': 'iw dev {ifname} set 4addr {value}',
+        },
+    }}
+
     def _create(self):
         # all interfaces will be added in monitor mode
-        cmd = 'iw phy {phy} interface add {ifname} type monitor' \
+        cmd = 'iw phy {phy} interface add {ifname} type monitor 4addr {wds}' \
             .format(**self.config)
         self._cmd(cmd)
 
@@ -50,28 +59,20 @@ class WiFiIf(Interface):
             .format(**self.config)
         self._cmd(cmd)
 
+    def set_4aadr_mode(self, state):
+        return self.set_interface('4addr', state)
+
     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
         interface setup code and provide a single point of entry when workin
         on any interface. """
 
-        # We can not call add_to_bridge() until wpa_supplicant is running, thus
-        # we will remove the key from the config dict and react to this specal
-        # case in thie derived class.
-        # re-add ourselves to any bridge we might have fallen out of
-        bridge_member = ''
-        if 'is_bridge_member' in config:
-            bridge_member = config['is_bridge_member']
-            del config['is_bridge_member']
+        self.set_4aadr_mode('on' if 'wds' in config else 'off')
 
         # call base class first
         super().update(config)
 
-        # re-add ourselves to any bridge we might have fallen out of
-        if bridge_member:
-            self.add_to_bridge(bridge_member)
-
         # 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()
         # function. We will only enable the interface if 'up' was called as
diff --git a/src/conf_mode/interfaces-wireless.py b/src/conf_mode/interfaces-wireless.py
index be59b72b5..f8520aecf 100755
--- a/src/conf_mode/interfaces-wireless.py
+++ b/src/conf_mode/interfaces-wireless.py
@@ -240,6 +240,7 @@ def apply(wifi):
 
         # Assign WiFi instance configuration parameters to config dict
         conf['phy'] = wifi['physical_device']
+        conf['wds'] = 'on' if 'wds' in wifi else 'off'
 
         # Finally create the new interface
         w = WiFiIf(interface, **conf)
-- 
cgit v1.2.3


From 7f09beeac924b1bc9bf61f8153870bd4cb939b96 Mon Sep 17 00:00:00 2001
From: Christian Poessinger <christian@poessinger.com>
Date: Thu, 24 Sep 2020 21:31:47 +0200
Subject: dhcpv6-pd: verify: T2923: interface is required where the prefix is
 assigned

When configuring DHCPv6-PD it is mandatory to also specify at least one
interface where the newly delegated prefix will be used. Without this setting
DHCPv6-PD makes no sense at all.
---
 python/vyos/configverify.py | 5 +++++
 1 file changed, 5 insertions(+)

(limited to 'python')

diff --git a/python/vyos/configverify.py b/python/vyos/configverify.py
index bf4e26fa7..afa6c7f06 100644
--- a/python/vyos/configverify.py
+++ b/python/vyos/configverify.py
@@ -113,6 +113,11 @@ def verify_dhcpv6(config):
         # assigned IPv6 subnet from a delegated prefix
         for pd in vyos_dict_search('dhcpv6_options.pd', config):
             sla_ids = []
+
+            if not vyos_dict_search(f'dhcpv6_options.pd.{pd}.interface', config):
+                raise ConfigError('DHCPv6-PD requires an interface where to assign '
+                                  'the delegated prefix!')
+
             for interface in vyos_dict_search(f'dhcpv6_options.pd.{pd}.interface', config):
                 sla_id = vyos_dict_search(
                     f'dhcpv6_options.pd.{pd}.interface.{interface}.sla_id', config)
-- 
cgit v1.2.3


From 49a79954373eb3f70eddb444e855ed744a322e58 Mon Sep 17 00:00:00 2001
From: Christian Poessinger <christian@poessinger.com>
Date: Fri, 25 Sep 2020 20:32:28 +0200
Subject: ifconfig: T2912: add helper to retrieve interface min/max supported
 MTU

  >>> from vyos.ifconfig import Interface
  >>> tmp=Interface('eth0')
  >>> tmp.get_min_mtu()
  60
  >>> tmp.get_max_mtu()
  9000
---
 python/vyos/ifconfig/interface.py | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

(limited to 'python')

diff --git a/python/vyos/ifconfig/interface.py b/python/vyos/ifconfig/interface.py
index 807191b3d..c8ba05edd 100644
--- a/python/vyos/ifconfig/interface.py
+++ b/python/vyos/ifconfig/interface.py
@@ -78,6 +78,14 @@ class Interface(Control):
             'shellcmd': 'ip -json link show dev {ifname}',
             'format': lambda j: 'up' if 'UP' in jmespath.search('[*].flags | [0]', json.loads(j)) else 'down',
         },
+        'min_mtu': {
+            'shellcmd': 'ip -json -detail link list dev {ifname}',
+            'format': lambda j: jmespath.search('[*].min_mtu | [0]', json.loads(j)),
+        },
+        'max_mtu': {
+            'shellcmd': 'ip -json -detail link list dev {ifname}',
+            'format': lambda j: jmespath.search('[*].max_mtu | [0]', json.loads(j)),
+        },
     }
 
     _command_set = {
@@ -282,6 +290,28 @@ class Interface(Control):
         cmd = 'ip link del dev {ifname}'.format(**self.config)
         return self._cmd(cmd)
 
+    def get_min_mtu(self):
+        """
+        Get hardware minimum supported MTU
+
+        Example:
+        >>> from vyos.ifconfig import Interface
+        >>> Interface('eth0').get_min_mtu()
+        '60'
+        """
+        return self.get_interface('min_mtu')
+
+    def get_max_mtu(self):
+        """
+        Get hardware maximum supported MTU
+
+        Example:
+        >>> from vyos.ifconfig import Interface
+        >>> Interface('eth0').get_max_mtu()
+        '9000'
+        """
+        return self.get_interface('max_mtu')
+
     def get_mtu(self):
         """
         Get/set interface mtu in bytes.
-- 
cgit v1.2.3


From 818a75c024e4b4c0403ccfe782fb55517f390bef Mon Sep 17 00:00:00 2001
From: Christian Poessinger <christian@poessinger.com>
Date: Fri, 25 Sep 2020 20:48:15 +0200
Subject: ifconfig: T2653: get_mtu() should return int() for easier comparison

---
 python/vyos/ifconfig/interface.py  | 2 +-
 src/conf_mode/interfaces-macsec.py | 4 ++--
 src/conf_mode/interfaces-vxlan.py  | 4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)

(limited to 'python')

diff --git a/python/vyos/ifconfig/interface.py b/python/vyos/ifconfig/interface.py
index c8ba05edd..a0f0ffe04 100644
--- a/python/vyos/ifconfig/interface.py
+++ b/python/vyos/ifconfig/interface.py
@@ -321,7 +321,7 @@ class Interface(Control):
         >>> Interface('eth0').get_mtu()
         '1500'
         """
-        return self.get_interface('mtu')
+        return int(self.get_interface('mtu'))
 
     def set_mtu(self, mtu):
         """
diff --git a/src/conf_mode/interfaces-macsec.py b/src/conf_mode/interfaces-macsec.py
index abf8b05c3..a224c540e 100755
--- a/src/conf_mode/interfaces-macsec.py
+++ b/src/conf_mode/interfaces-macsec.py
@@ -92,8 +92,8 @@ def verify(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
         # MTU is at least 40 bytes less then the MTU of our physical interface.
-        underlay_mtu = int(Interface(macsec['source_interface']).get_mtu())
-        if underlay_mtu < (int(macsec['mtu']) + 40):
+        lower_mtu = Interface(macsec['source_interface']).get_mtu()
+        if lower_mtu < (int(macsec['mtu']) + 40):
             raise ConfigError('MACsec overhead does not fit into underlaying device MTU,\n' \
                               f'{underlay_mtu} bytes is too small!')
 
diff --git a/src/conf_mode/interfaces-vxlan.py b/src/conf_mode/interfaces-vxlan.py
index a00c58608..850ea28d7 100755
--- a/src/conf_mode/interfaces-vxlan.py
+++ b/src/conf_mode/interfaces-vxlan.py
@@ -72,8 +72,8 @@ def verify(vxlan):
     if 'source_interface' in vxlan:
         # VXLAN adds a 50 byte overhead - we need to check the underlaying MTU
         # if our configured MTU is at least 50 bytes less
-        underlay_mtu = int(Interface(vxlan['source_interface']).get_mtu())
-        if underlay_mtu < (int(vxlan['mtu']) + 50):
+        lower_mtu = Interface(vxlan['source_interface']).get_mtu()
+        if lower_mtu < (int(vxlan['mtu']) + 50):
             raise ConfigError('VXLAN has a 50 byte overhead, underlaying device ' \
                               f'MTU is to small ({underlay_mtu} bytes)')
 
-- 
cgit v1.2.3


From 9b268012252a4fe6f253177830abde950e2773b5 Mon Sep 17 00:00:00 2001
From: Christian Poessinger <christian@poessinger.com>
Date: Fri, 25 Sep 2020 20:57:37 +0200
Subject: ifconfig: T2912: add helper to verify interface min/max supported MTU

Currently the MTU size of an interface is only checked when entered via CLI but
if the interface supportes the configured MTU at all is not verified at all.

New helper functions get_min_mtu(), get_max_mtu() and verify_mtu() have been
added to provide a central API for validation.
---
 python/vyos/configverify.py       | 21 +++++++++++++++++++++
 python/vyos/ifconfig/interface.py |  4 ++--
 2 files changed, 23 insertions(+), 2 deletions(-)

(limited to 'python')

diff --git a/python/vyos/configverify.py b/python/vyos/configverify.py
index afa6c7f06..6e5ba1df0 100644
--- a/python/vyos/configverify.py
+++ b/python/vyos/configverify.py
@@ -23,6 +23,27 @@
 
 from vyos import ConfigError
 
+def verify_mtu(config):
+    """
+    Common helper function used by interface implementations to perform
+    recurring validation if the specified MTU can be used by the underlaying
+    hardware.
+    """
+    from vyos.ifconfig import Interface
+    if 'mtu' in config:
+        mtu = int(config['mtu'])
+
+        tmp = Interface(config['ifname'])
+        min_mtu = tmp.get_min_mtu()
+        max_mtu = tmp.get_max_mtu()
+
+        if mtu < min_mtu:
+            raise ConfigError(f'Interface MTU too low, ' \
+                              f'minimum supported MTU is {min_mtu}!')
+        if mtu > max_mtu:
+            raise ConfigError(f'Interface MTU too high, ' \
+                              f'maximum supported MTU is {max_mtu}!')
+
 def verify_vrf(config):
     """
     Common helper function used by interface implementations to perform
diff --git a/python/vyos/ifconfig/interface.py b/python/vyos/ifconfig/interface.py
index a0f0ffe04..d200fc7a8 100644
--- a/python/vyos/ifconfig/interface.py
+++ b/python/vyos/ifconfig/interface.py
@@ -299,7 +299,7 @@ class Interface(Control):
         >>> Interface('eth0').get_min_mtu()
         '60'
         """
-        return self.get_interface('min_mtu')
+        return int(self.get_interface('min_mtu'))
 
     def get_max_mtu(self):
         """
@@ -310,7 +310,7 @@ class Interface(Control):
         >>> Interface('eth0').get_max_mtu()
         '9000'
         """
-        return self.get_interface('max_mtu')
+        return int(self.get_interface('max_mtu'))
 
     def get_mtu(self):
         """
-- 
cgit v1.2.3


From 5db3d63160670c796ed74a170862c367048d89bb Mon Sep 17 00:00:00 2001
From: Christian Poessinger <christian@poessinger.com>
Date: Sat, 26 Sep 2020 12:00:06 +0200
Subject: ifconfig: mtu: disallow MTU < 1280 bytes when IPv6 is enabled on the
 interface

Using an MTU less then the required 1280 bytes (as per RFC) on an interface
where IPv6 is not explicitly disabled by:

- set interfaces ethernet eth1 ipv6 address no-default-link-local
- not having any other IPv6 address configured

Will now trigger a commit error via verify() instead of raising
FileNotFoundError!
---
 python/vyos/configverify.py           | 30 ++++++++++++++++++++++++++++++
 src/conf_mode/interfaces-bonding.py   |  2 ++
 src/conf_mode/interfaces-bridge.py    |  2 ++
 src/conf_mode/interfaces-ethernet.py  |  3 +++
 src/conf_mode/interfaces-geneve.py    |  2 ++
 src/conf_mode/interfaces-l2tpv3.py    |  2 ++
 src/conf_mode/interfaces-macsec.py    |  2 ++
 src/conf_mode/interfaces-pppoe.py     |  2 ++
 src/conf_mode/interfaces-vxlan.py     |  2 ++
 src/conf_mode/interfaces-wireguard.py |  2 ++
 10 files changed, 49 insertions(+)

(limited to 'python')

diff --git a/python/vyos/configverify.py b/python/vyos/configverify.py
index 6e5ba1df0..944fc4294 100644
--- a/python/vyos/configverify.py
+++ b/python/vyos/configverify.py
@@ -44,6 +44,36 @@ def verify_mtu(config):
             raise ConfigError(f'Interface MTU too high, ' \
                               f'maximum supported MTU is {max_mtu}!')
 
+def verify_mtu_ipv6(config):
+    """
+    Common helper function used by interface implementations to perform
+    recurring validation if the specified MTU can be used when IPv6 is
+    configured on the interface. IPv6 requires a 1280 bytes MTU.
+    """
+    from vyos.validate import is_ipv6
+    from vyos.util import vyos_dict_search
+    # IPv6 minimum required link mtu
+    min_mtu = 1280
+
+    if int(config['mtu']) < min_mtu:
+        interface = config['ifname']
+        error_msg = f'IPv6 address will be configured on interface "{interface}" ' \
+                    f'thus the minimum MTU requirement is {min_mtu}!'
+
+        if not vyos_dict_search('ipv6.address.no_default_link_local', config):
+            raise ConfigError('link-local ' + error_msg)
+
+        for address in (vyos_dict_search('address', config) or []):
+            if address in ['dhcpv6'] or is_ipv6(address):
+                raise ConfigError(error_msg)
+
+        if vyos_dict_search('ipv6.address.autoconf', config):
+            raise ConfigError(error_msg)
+
+        if vyos_dict_search('ipv6.address.eui64', config):
+            raise ConfigError(error_msg)
+
+
 def verify_vrf(config):
     """
     Common helper function used by interface implementations to perform
diff --git a/src/conf_mode/interfaces-bonding.py b/src/conf_mode/interfaces-bonding.py
index aece2a04b..9763620ac 100755
--- a/src/conf_mode/interfaces-bonding.py
+++ b/src/conf_mode/interfaces-bonding.py
@@ -28,6 +28,7 @@ from vyos.configverify import verify_address
 from vyos.configverify import verify_bridge_delete
 from vyos.configverify import verify_dhcpv6
 from vyos.configverify import verify_source_interface
+from vyos.configverify import verify_mtu_ipv6
 from vyos.configverify import verify_vlan_config
 from vyos.configverify import verify_vrf
 from vyos.ifconfig import BondIf
@@ -141,6 +142,7 @@ def verify(bond):
             raise ConfigError('Option primary - mode dependency failed, not'
                               'supported in mode {mode}!'.format(**bond))
 
+    verify_mtu_ipv6(bond)
     verify_address(bond)
     verify_dhcpv6(bond)
     verify_vrf(bond)
diff --git a/src/conf_mode/interfaces-bridge.py b/src/conf_mode/interfaces-bridge.py
index 485decb17..4ac9c8963 100755
--- a/src/conf_mode/interfaces-bridge.py
+++ b/src/conf_mode/interfaces-bridge.py
@@ -25,6 +25,7 @@ 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_mtu_ipv6
 from vyos.configverify import verify_vrf
 from vyos.ifconfig import BridgeIf
 from vyos.validate import has_address_configured
@@ -95,6 +96,7 @@ def verify(bridge):
     if 'deleted' in bridge:
         return None
 
+    verify_mtu_ipv6(bridge)
     verify_dhcpv6(bridge)
     verify_vrf(bridge)
 
diff --git a/src/conf_mode/interfaces-ethernet.py b/src/conf_mode/interfaces-ethernet.py
index 5468c7bda..1f622c003 100755
--- a/src/conf_mode/interfaces-ethernet.py
+++ b/src/conf_mode/interfaces-ethernet.py
@@ -24,6 +24,7 @@ from vyos.configverify import verify_address
 from vyos.configverify import verify_dhcpv6
 from vyos.configverify import verify_interface_exists
 from vyos.configverify import verify_mtu
+from vyos.configverify import verify_mtu_ipv6
 from vyos.configverify import verify_vlan_config
 from vyos.configverify import verify_vrf
 from vyos.ifconfig import EthernetIf
@@ -42,6 +43,7 @@ def get_config(config=None):
         conf = Config()
     base = ['interfaces', 'ethernet']
     ethernet = get_interface_dict(conf, base)
+
     return ethernet
 
 def verify(ethernet):
@@ -59,6 +61,7 @@ def verify(ethernet):
             raise ConfigError('If duplex is hardcoded, speed must be hardcoded, too')
 
     verify_mtu(ethernet)
+    verify_mtu_ipv6(ethernet)
     verify_dhcpv6(ethernet)
     verify_address(ethernet)
     verify_vrf(ethernet)
diff --git a/src/conf_mode/interfaces-geneve.py b/src/conf_mode/interfaces-geneve.py
index af7c121f4..979a5612e 100755
--- a/src/conf_mode/interfaces-geneve.py
+++ b/src/conf_mode/interfaces-geneve.py
@@ -22,6 +22,7 @@ from netifaces import interfaces
 from vyos.config import Config
 from vyos.configdict import get_interface_dict
 from vyos.configverify import verify_address
+from vyos.configverify import verify_mtu_ipv6
 from vyos.configverify import verify_bridge_delete
 from vyos.ifconfig import GeneveIf
 from vyos import ConfigError
@@ -47,6 +48,7 @@ def verify(geneve):
         verify_bridge_delete(geneve)
         return None
 
+    verify_mtu_ipv6(geneve)
     verify_address(geneve)
 
     if 'remote' not in geneve:
diff --git a/src/conf_mode/interfaces-l2tpv3.py b/src/conf_mode/interfaces-l2tpv3.py
index 2653ff19c..1118143e4 100755
--- a/src/conf_mode/interfaces-l2tpv3.py
+++ b/src/conf_mode/interfaces-l2tpv3.py
@@ -24,6 +24,7 @@ from vyos.configdict import get_interface_dict
 from vyos.configdict import leaf_node_changed
 from vyos.configverify import verify_address
 from vyos.configverify import verify_bridge_delete
+from vyos.configverify import verify_mtu_ipv6
 from vyos.ifconfig import L2TPv3If
 from vyos.util import check_kmod
 from vyos.validate import is_addr_assigned
@@ -80,6 +81,7 @@ def verify(l2tpv3):
         raise ConfigError('L2TPv3 local-ip address '
                           '"{local_ip}" is not configured!'.format(**l2tpv3))
 
+    verify_mtu_ipv6(l2tpv3)
     verify_address(l2tpv3)
     return None
 
diff --git a/src/conf_mode/interfaces-macsec.py b/src/conf_mode/interfaces-macsec.py
index a224c540e..0a20a121b 100755
--- a/src/conf_mode/interfaces-macsec.py
+++ b/src/conf_mode/interfaces-macsec.py
@@ -27,6 +27,7 @@ from vyos.util import call
 from vyos.configverify import verify_vrf
 from vyos.configverify import verify_address
 from vyos.configverify import verify_bridge_delete
+from vyos.configverify import verify_mtu_ipv6
 from vyos.configverify import verify_source_interface
 from vyos import ConfigError
 from vyos import airbag
@@ -71,6 +72,7 @@ def verify(macsec):
 
     verify_source_interface(macsec)
     verify_vrf(macsec)
+    verify_mtu_ipv6(macsec)
     verify_address(macsec)
 
     if not (('security' in macsec) and
diff --git a/src/conf_mode/interfaces-pppoe.py b/src/conf_mode/interfaces-pppoe.py
index 1b4b9e4ee..ee3b142c8 100755
--- a/src/conf_mode/interfaces-pppoe.py
+++ b/src/conf_mode/interfaces-pppoe.py
@@ -24,6 +24,7 @@ from vyos.config import Config
 from vyos.configdict import get_interface_dict
 from vyos.configverify import verify_source_interface
 from vyos.configverify import verify_vrf
+from vyos.configverify import verify_mtu_ipv6
 from vyos.template import render
 from vyos.util import call
 from vyos import ConfigError
@@ -57,6 +58,7 @@ def verify(pppoe):
 
     verify_source_interface(pppoe)
     verify_vrf(pppoe)
+    verify_mtu_ipv6(pppoe)
 
     if {'connect_on_demand', 'vrf'} <= set(pppoe):
         raise ConfigError('On-demand dialing and VRF can not be used at the same time')
diff --git a/src/conf_mode/interfaces-vxlan.py b/src/conf_mode/interfaces-vxlan.py
index 850ea28d7..002f40aef 100755
--- a/src/conf_mode/interfaces-vxlan.py
+++ b/src/conf_mode/interfaces-vxlan.py
@@ -23,6 +23,7 @@ from vyos.config import Config
 from vyos.configdict import get_interface_dict
 from vyos.configverify import verify_address
 from vyos.configverify import verify_bridge_delete
+from vyos.configverify import verify_mtu_ipv6
 from vyos.configverify import verify_source_interface
 from vyos.ifconfig import VXLANIf, Interface
 from vyos import ConfigError
@@ -77,6 +78,7 @@ def verify(vxlan):
             raise ConfigError('VXLAN has a 50 byte overhead, underlaying device ' \
                               f'MTU is to small ({underlay_mtu} bytes)')
 
+    verify_mtu_ipv6(vxlan)
     verify_address(vxlan)
     return None
 
diff --git a/src/conf_mode/interfaces-wireguard.py b/src/conf_mode/interfaces-wireguard.py
index e7c22da1a..d5800264f 100755
--- a/src/conf_mode/interfaces-wireguard.py
+++ b/src/conf_mode/interfaces-wireguard.py
@@ -27,6 +27,7 @@ from vyos.configdict import leaf_node_changed
 from vyos.configverify import verify_vrf
 from vyos.configverify import verify_address
 from vyos.configverify import verify_bridge_delete
+from vyos.configverify import verify_mtu_ipv6
 from vyos.ifconfig import WireGuardIf
 from vyos.util import check_kmod
 from vyos import ConfigError
@@ -71,6 +72,7 @@ def verify(wireguard):
         verify_bridge_delete(wireguard)
         return None
 
+    verify_mtu_ipv6(wireguard)
     verify_address(wireguard)
     verify_vrf(wireguard)
 
-- 
cgit v1.2.3