bonding: T5254: Fixed changing ethernet when it is a bond member

If ethernet interface is a bond memeber:
  1. Allow for changing only specific parameters which are specified
     in EthernetIf.get_bond_member_allowed_options function.
  2. Added inheritable parameters from bond interface to ethernet
     interface which are scpecified
     in BondIf.get_inherit_bond_options.
     Users can change inheritable options under ethernet interface
     but in commit it will be copied from bond interface.
  3. All other parameters are denied for changing.

Added migration script. It deletes all denied parameters under
ethernet interface if it is a bond member.

(cherry picked from commit aa0282ceb379df1ab3cc93e4bd019134d37f0d89)

2 files changed, 239 insertions, 36 deletions

diff --git a/src/conf_mode/interfaces-bonding.py b/src/conf_mode/interfaces-bonding.py
index 0bd306ed0..1179e3e4f 100755
--- a/src/conf_mode/interfaces-bonding.py
+++ b/src/conf_mode/interfaces-bonding.py
@@ -18,7 +18,6 @@ import os
 
 from sys import exit
 from netifaces import interfaces
-
 from vyos.config import Config
 from vyos.configdict import get_interface_dict
 from vyos.configdict import is_node_changed
@@ -34,10 +33,13 @@ from vyos.configverify import verify_source_interface
 from vyos.configverify import verify_vlan_config
 from vyos.configverify import verify_vrf
 from vyos.ifconfig import BondIf
+from vyos.ifconfig.ethernet import EthernetIf
 from vyos.ifconfig import Section
 from vyos.utils.dict import dict_search
+from vyos.utils.dict import dict_to_paths_values
 from vyos.configdict import has_address_configured
 from vyos.configdict import has_vrf_configured
+from vyos.configdep import set_dependents, call_dependents
 from vyos import ConfigError
 from vyos import airbag
 airbag.enable()
@@ -90,7 +92,6 @@ def get_config(config=None):
 
     # determine which members have been removed
     interfaces_removed = leaf_node_changed(conf, base + [ifname, 'member', 'interface'])
-
     # Reset config level to interfaces
     old_level = conf.get_level()
     conf.set_level(['interfaces'])
@@ -102,6 +103,10 @@ def get_config(config=None):
 
         tmp = {}
         for interface in interfaces_removed:
+            # if member is deleted from bond, add dependencies to call
+            # ethernet commit again in apply function
+            # to apply options under ethernet section
+            set_dependents('ethernet', conf, interface)
             section = Section.section(interface) # this will be 'ethernet' for 'eth0'
             if conf.exists([section, interface, 'disable']):
                 tmp[interface] = {'disable': ''}
@@ -116,9 +121,21 @@ def get_config(config=None):
 
     if dict_search('member.interface', bond):
         for interface, interface_config in bond['member']['interface'].items():
+
+            interface_ethernet_config = conf.get_config_dict(
+                ['interfaces', 'ethernet', interface],
+                key_mangling=('-', '_'),
+                get_first_key=True,
+                no_tag_node_value_mangle=True,
+                with_defaults=False,
+                with_recursive_defaults=False)
+
+            interface_config['config_paths'] = dict_to_paths_values(interface_ethernet_config)
+
             # Check if member interface is a new member
             if not conf.exists_effective(base + [ifname, 'member', 'interface', interface]):
                 bond['shutdown_required'] = {}
+                interface_config['new_added'] = {}
 
             # Check if member interface is disabled
             conf.set_level(['interfaces'])
@@ -151,7 +168,6 @@ def get_config(config=None):
             # bond members must not have a VRF attached
             tmp = has_vrf_configured(conf, interface)
             if tmp: interface_config['has_vrf'] = {}
-
     return bond
 
 
@@ -212,6 +228,14 @@ def verify(bond):
             if 'has_vrf' in interface_config:
                 raise ConfigError(error_msg + 'it has a VRF assigned!')
 
+            if 'new_added' in interface_config and 'config_paths' in interface_config:
+                for option_path, option_value in interface_config['config_paths'].items():
+                    if option_path in EthernetIf.get_bond_member_allowed_options() :
+                        continue
+                    if option_path in BondIf.get_inherit_bond_options():
+                        continue
+                    raise ConfigError(error_msg + f'it has a "{option_path.replace(".", " ")}" assigned!')
+
     if 'primary' in bond:
         if bond['primary'] not in bond['member']['interface']:
             raise ConfigError(f'Primary interface of bond "{bond_name}" must be a member interface')
@@ -227,13 +251,17 @@ def generate(bond):
 
 def apply(bond):
     b = BondIf(bond['ifname'])
-
     if 'deleted' in bond:
         # delete interface
         b.remove()
     else:
         b.update(bond)
-
+    if dict_search('member.interface_remove', bond):
+        try:
+            call_dependents()
+        except ConfigError:
+            raise ConfigError('Error in updating ethernet interface '
+                              'after deleting it from bond')
     return None
 
 if __name__ == '__main__':
diff --git a/src/conf_mode/interfaces-ethernet.py b/src/conf_mode/interfaces-ethernet.py
index f3e65ad5e..7374a29f7 100755
--- a/src/conf_mode/interfaces-ethernet.py
+++ b/src/conf_mode/interfaces-ethernet.py
@@ -15,6 +15,7 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 import os
+import pprint
 
 from glob import glob
 from sys import exit
@@ -35,6 +36,7 @@ from vyos.configverify import verify_vrf
 from vyos.configverify import verify_bond_bridge_member
 from vyos.ethtool import Ethtool
 from vyos.ifconfig import EthernetIf
+from vyos.ifconfig import BondIf
 from vyos.pki import find_chain
 from vyos.pki import encode_certificate
 from vyos.pki import load_certificate
@@ -42,6 +44,9 @@ from vyos.pki import wrap_private_key
 from vyos.template import render
 from vyos.utils.process import call
 from vyos.utils.dict import dict_search
+from vyos.utils.dict import dict_to_paths_values
+from vyos.utils.dict import dict_set
+from vyos.utils.dict import dict_delete
 from vyos.utils.file import write_file
 from vyos import ConfigError
 from vyos import airbag
@@ -51,6 +56,90 @@ airbag.enable()
 cfg_dir = '/run/wpa_supplicant'
 wpa_suppl_conf = '/run/wpa_supplicant/{ifname}.conf'
 
+def update_bond_options(conf: Config, eth_conf: dict) -> list:
+    """
+    Return list of blocked options if interface is a bond member
+    :param conf: Config object
+    :type conf: Config
+    :param eth_conf: Ethernet config dictionary
+    :type eth_conf: dict
+    :return: List of blocked options
+    :rtype: list
+    """
+    blocked_list = []
+    bond_name = list(eth_conf['is_bond_member'].keys())[0]
+    config_without_defaults = conf.get_config_dict(
+        ['interfaces', 'ethernet', eth_conf['ifname']],
+        key_mangling=('-', '_'),
+        get_first_key=True,
+        no_tag_node_value_mangle=True,
+        with_defaults=False,
+        with_recursive_defaults=False)
+    config_with_defaults = conf.get_config_dict(
+        ['interfaces', 'ethernet', eth_conf['ifname']],
+        key_mangling=('-', '_'),
+        get_first_key=True,
+        no_tag_node_value_mangle=True,
+        with_defaults=True,
+        with_recursive_defaults=True)
+    bond_config_with_defaults = conf.get_config_dict(
+        ['interfaces', 'bonding', bond_name],
+        key_mangling=('-', '_'),
+        get_first_key=True,
+        no_tag_node_value_mangle=True,
+        with_defaults=True,
+        with_recursive_defaults=True)
+    eth_dict_paths = dict_to_paths_values(config_without_defaults)
+    eth_path_base = ['interfaces', 'ethernet', eth_conf['ifname']]
+
+    #if option is configured under ethernet section
+    for option_path, option_value in eth_dict_paths.items():
+        bond_option_value = dict_search(option_path, bond_config_with_defaults)
+
+        #If option is allowed for changing then continue
+        if option_path in EthernetIf.get_bond_member_allowed_options():
+            continue
+        # if option is inherited from bond then set valued from bond interface
+        if option_path in BondIf.get_inherit_bond_options():
+            # If option equals to bond option then do nothing
+            if option_value == bond_option_value:
+                continue
+            else:
+                # if ethernet has option and bond interface has
+                # then copy it from bond
+                if bond_option_value is not None:
+                    if is_node_changed(conf, eth_path_base + option_path.split('.')):
+                        Warning(
+                            f'Cannot apply "{option_path.replace(".", " ")}" to "{option_value}".' \
+                            f' Interface "{eth_conf["ifname"]}" is a bond member.' \
+                            f' Option is inherited from bond "{bond_name}"')
+                    dict_set(option_path, bond_option_value, eth_conf)
+                    continue
+                # if ethernet has option and bond interface does not have
+                # then delete it form dict and do not apply it
+                else:
+                    if is_node_changed(conf, eth_path_base + option_path.split('.')):
+                        Warning(
+                            f'Cannot apply "{option_path.replace(".", " ")}".' \
+                            f' Interface "{eth_conf["ifname"]}" is a bond member.' \
+                            f' Option is inherited from bond "{bond_name}"')
+                    dict_delete(option_path, eth_conf)
+        blocked_list.append(option_path)
+
+    # if inherited option is not configured under ethernet section but configured under bond section
+    for option_path in BondIf.get_inherit_bond_options():
+        bond_option_value = dict_search(option_path, bond_config_with_defaults)
+        if bond_option_value is not None:
+            if option_path not in eth_dict_paths:
+                if is_node_changed(conf, eth_path_base + option_path.split('.')):
+                    Warning(
+                        f'Cannot apply "{option_path.replace(".", " ")}" to "{dict_search(option_path, config_with_defaults)}".' \
+                        f' Interface "{eth_conf["ifname"]}" is a bond member. ' \
+                        f'Option is inherited from bond "{bond_name}"')
+                dict_set(option_path, bond_option_value, eth_conf)
+    eth_conf['bond_blocked_changes'] = blocked_list
+    return None
+
 def get_config(config=None):
     """
     Retrive CLI config as dictionary. Dictionary can never be empty, as at least the
@@ -68,6 +157,8 @@ def get_config(config=None):
 
     base = ['interfaces', 'ethernet']
     ifname, ethernet = get_interface_dict(conf, base)
+    if 'is_bond_member' in ethernet:
+        update_bond_options(conf, ethernet)
 
     if 'deleted' not in ethernet:
        if pki: ethernet['pki'] = pki
@@ -80,26 +171,20 @@ def get_config(config=None):
 
     return ethernet
 
-def verify(ethernet):
-    if 'deleted' in ethernet:
-        return None
 
-    ifname = ethernet['ifname']
-    verify_interface_exists(ifname)
-    verify_mtu(ethernet)
-    verify_mtu_ipv6(ethernet)
-    verify_dhcpv6(ethernet)
-    verify_address(ethernet)
-    verify_vrf(ethernet)
-    verify_bond_bridge_member(ethernet)
-    verify_eapol(ethernet)
-    verify_mirror_redirect(ethernet)
 
-    ethtool = Ethtool(ifname)
-    # No need to check speed and duplex keys as both have default values.
+def verify_speed_duplex(ethernet: dict, ethtool: Ethtool):
+    """
+     Verify speed and duplex
+    :param ethernet: dictionary which is received from get_interface_dict
+    :type ethernet: dict
+    :param ethtool: Ethernet object
+    :type ethtool: Ethtool
+    """
     if ((ethernet['speed'] == 'auto' and ethernet['duplex'] != 'auto') or
-        (ethernet['speed'] != 'auto' and ethernet['duplex'] == 'auto')):
-            raise ConfigError('Speed/Duplex missmatch. Must be both auto or manually configured')
+            (ethernet['speed'] != 'auto' and ethernet['duplex'] == 'auto')):
+        raise ConfigError(
+            'Speed/Duplex missmatch. Must be both auto or manually configured')
 
     if ethernet['speed'] != 'auto' and ethernet['duplex'] != 'auto':
         # We need to verify if the requested speed and duplex setting is
@@ -107,37 +192,66 @@ def verify(ethernet):
         speed = ethernet['speed']
         duplex = ethernet['duplex']
         if not ethtool.check_speed_duplex(speed, duplex):
-            raise ConfigError(f'Adapter does not support changing speed and duplex '\
-                              f'settings to: {speed}/{duplex}!')
+            raise ConfigError(
+                f'Adapter does not support changing speed ' \
+                f'and duplex settings to: {speed}/{duplex}!')
 
+
+def verify_flow_control(ethernet: dict, ethtool: Ethtool):
+    """
+     Verify flow control
+    :param ethernet: dictionary which is received from get_interface_dict
+    :type ethernet: dict
+    :param ethtool: Ethernet object
+    :type ethtool: Ethtool
+    """
     if 'disable_flow_control' in ethernet:
         if not ethtool.check_flow_control():
-            raise ConfigError('Adapter does not support changing flow-control settings!')
+            raise ConfigError(
+                'Adapter does not support changing flow-control settings!')
+
 
+def verify_ring_buffer(ethernet: dict, ethtool: Ethtool):
+    """
+     Verify ring buffer
+    :param ethernet: dictionary which is received from get_interface_dict
+    :type ethernet: dict
+    :param ethtool: Ethernet object
+    :type ethtool: Ethtool
+    """
     if 'ring_buffer' in ethernet:
         max_rx = ethtool.get_ring_buffer_max('rx')
         if not max_rx:
-            raise ConfigError('Driver does not support RX ring-buffer configuration!')
+            raise ConfigError(
+                'Driver does not support RX ring-buffer configuration!')
 
         max_tx = ethtool.get_ring_buffer_max('tx')
         if not max_tx:
-            raise ConfigError('Driver does not support TX ring-buffer configuration!')
+            raise ConfigError(
+                'Driver does not support TX ring-buffer configuration!')
 
         rx = dict_search('ring_buffer.rx', ethernet)
         if rx and int(rx) > int(max_rx):
-            raise ConfigError(f'Driver only supports a maximum RX ring-buffer '\
+            raise ConfigError(f'Driver only supports a maximum RX ring-buffer ' \
                               f'size of "{max_rx}" bytes!')
 
         tx = dict_search('ring_buffer.tx', ethernet)
         if tx and int(tx) > int(max_tx):
-            raise ConfigError(f'Driver only supports a maximum TX ring-buffer '\
+            raise ConfigError(f'Driver only supports a maximum TX ring-buffer ' \
                               f'size of "{max_tx}" bytes!')
 
-    # verify offloading capabilities
+
+def verify_offload(ethernet: dict, ethtool: Ethtool):
+    """
+     Verify offloading capabilities
+    :param ethernet: dictionary which is received from get_interface_dict
+    :type ethernet: dict
+    :param ethtool: Ethernet object
+    :type ethtool: Ethtool
+    """
     if dict_search('offload.rps', ethernet) != None:
-        if not os.path.exists(f'/sys/class/net/{ifname}/queues/rx-0/rps_cpus'):
+        if not os.path.exists(f'/sys/class/net/{ethernet["ifname"]}/queues/rx-0/rps_cpus'):
             raise ConfigError('Interface does not suport RPS!')
-
     driver = ethtool.get_driver_name()
     # T3342 - Xen driver requires special treatment
     if driver == 'vif':
@@ -145,14 +259,73 @@ def verify(ethernet):
             raise ConfigError('Xen netback drivers requires scatter-gatter offloading '\
                               'for MTU size larger then 1500 bytes')
 
-    if {'is_bond_member', 'mac'} <= set(ethernet):
-        Warning(f'changing mac address "{mac}" will be ignored as "{ifname}" ' \
-                f'is a member of bond "{is_bond_member}"'.format(**ethernet))
 
+def verify_allowedbond_changes(ethernet: dict):
+    """
+     Verify changed options if interface is in bonding
+    :param ethernet: dictionary which is received from get_interface_dict
+    :type ethernet: dict
+    """
+    if 'bond_blocked_changes' in ethernet:
+        for option in ethernet['bond_blocked_changes']:
+            raise ConfigError(f'Cannot configure "{option.replace(".", " ")}"' \
+                              f' on interface "{ethernet["ifname"]}".' \
+                              f' Interface is a bond member')
+
+
+def verify(ethernet):
+    if 'deleted' in ethernet:
+        return None
+    if 'is_bond_member' in ethernet:
+        verify_bond_member(ethernet)
+    else:
+        verify_ethernet(ethernet)
+
+
+def verify_bond_member(ethernet):
+    """
+     Verification function for ethernet interface which is in bonding
+    :param ethernet: dictionary which is received from get_interface_dict
+    :type ethernet: dict
+    """
+    ifname = ethernet['ifname']
+    verify_interface_exists(ifname)
+    verify_eapol(ethernet)
+    verify_mirror_redirect(ethernet)
+    ethtool = Ethtool(ifname)
+    verify_speed_duplex(ethernet, ethtool)
+    verify_flow_control(ethernet, ethtool)
+    verify_ring_buffer(ethernet, ethtool)
+    verify_offload(ethernet, ethtool)
+    verify_allowedbond_changes(ethernet)
+
+def verify_ethernet(ethernet):
+    """
+     Verification function for simple ethernet interface
+    :param ethernet: dictionary which is received from get_interface_dict
+    :type ethernet: dict
+    """
+    ifname = ethernet['ifname']
+    verify_interface_exists(ifname)
+    verify_mtu(ethernet)
+    verify_mtu_ipv6(ethernet)
+    verify_dhcpv6(ethernet)
+    verify_address(ethernet)
+    verify_vrf(ethernet)
+    verify_bond_bridge_member(ethernet)
+    verify_eapol(ethernet)
+    verify_mirror_redirect(ethernet)
+    ethtool = Ethtool(ifname)
+    # No need to check speed and duplex keys as both have default values.
+    verify_speed_duplex(ethernet, ethtool)
+    verify_flow_control(ethernet, ethtool)
+    verify_ring_buffer(ethernet, ethtool)
+    verify_offload(ethernet, ethtool)
     # use common function to verify VLAN configuration
     verify_vlan_config(ethernet)
     return None
 
+
 def generate(ethernet):
     # render real configuration file once
     wpa_supplicant_conf = wpa_suppl_conf.format(**ethernet)
@@ -192,7 +365,8 @@ def generate(ethernet):
                 pki_ca_cert = ethernet['pki']['ca'][ca_cert_name]
                 loaded_ca_cert = load_certificate(pki_ca_cert['certificate'])
                 ca_full_chain = find_chain(loaded_ca_cert, loaded_ca_certs)
-                ca_chains.append('\n'.join(encode_certificate(c) for c in ca_full_chain))
+                ca_chains.append(
+                    '\n'.join(encode_certificate(c) for c in ca_full_chain))
 
             write_file(ca_cert_file_path, '\n'.join(ca_chains))
 
@@ -219,6 +393,7 @@ if __name__ == '__main__':
         c = get_config()
         verify(c)
         generate(c)
+
         apply(c)
     except ConfigError as e:
         print(e)