Merge pull request #4911 from c-po/bond-ena

bond: T8084: disallow bond members that do not support MAC changes

3 files changed, 52 insertions, 34 deletions

diff --git a/python/vyos/ethtool.py b/python/vyos/ethtool.py
index 22e066def..d0495df43 100644
--- a/python/vyos/ethtool.py
+++ b/python/vyos/ethtool.py
@@ -25,6 +25,11 @@ _drivers_without_speed_duplex_flow = ['vmxnet3', 'virtio_net', 'xen_netfront',
                                       'iavf', 'ice', 'i40e', 'hv_netvsc', 'veth', 'ixgbevf',
                                       'tun', 'vif']
 
+_drivers_without_mac_change = ['ena']
+# enable interface bonding will change the interface MAC address, thus all drivers
+# not supporting MAC address change, also do not support bonding
+_drivers_without_bonding_support = _drivers_without_mac_change + []
+
 class Ethtool:
     """
     Class is used to retrive and cache information about an ethernet adapter
@@ -222,3 +227,11 @@ class Ethtool:
         matches = re.findall(rf'{rx_tx_comb}:\s+(\d+)', self._channels)
 
         return [int(value) for value in matches]
+
+    def check_mac_change(self) -> bool:
+        """ Check if ethernet drivers supports changing MAC address """
+        return bool(self.get_driver_name() not in _drivers_without_mac_change)
+
+    def check_bonding(self) -> bool:
+        """ Check if ethernet drivers supports bonding """
+        return bool(self.get_driver_name() not in _drivers_without_bonding_support)
diff --git a/src/conf_mode/interfaces_bonding.py b/src/conf_mode/interfaces_bonding.py
index f844d0a21..e6aaa3440 100755
--- a/src/conf_mode/interfaces_bonding.py
+++ b/src/conf_mode/interfaces_bonding.py
@@ -30,11 +30,11 @@ from vyos.configverify import verify_mirror_redirect
 from vyos.configverify import verify_mtu_ipv6
 from vyos.configverify import verify_vlan_config
 from vyos.configverify import verify_vrf
+from vyos.ethtool import Ethtool
 from vyos.frrender import FRRender
 from vyos.frrender import get_frrender_dict
 from vyos.ifconfig import BondIf
 from vyos.ifconfig.ethernet import EthernetIf
-from vyos.ifconfig import Section
 from vyos.utils.assertion import assert_mac
 from vyos.utils.dict import dict_search
 from vyos.utils.dict import dict_to_paths_values
@@ -114,8 +114,7 @@ def get_config(config=None):
             # 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']):
+            if conf.exists(['ethernet', interface, 'disable']):
                 tmp[interface] = {'disable': ''}
             else:
                 tmp[interface] = {}
@@ -144,14 +143,8 @@ def get_config(config=None):
                 bond['shutdown_required'] = {}
                 bond['member']['interface'][interface].update({'new_added' : {}})
 
-            # Check if member interface is disabled
-            conf.set_level(['interfaces'])
-
-            section = Section.section(interface) # this will be 'ethernet' for 'eth0'
-            if conf.exists([section, interface, 'disable']):
-                if tmp: bond['member']['interface'][interface].update({'disable': ''})
-
-            conf.set_level(old_level)
+            if 'disable' in interface_ethernet_config:
+                bond['member']['interface'][interface].update({'disable': ''})
 
             # Check if member interface is already member of another bridge
             tmp = is_member(conf, interface, 'bridge')
@@ -259,6 +252,10 @@ def verify(bond):
                     raise ConfigError('Configured MTU is less then member '\
                                       f'interface "{interface}" minimum of {min_mtu}!')
 
+            # not all ethernet drivers support interface bonding
+            if not Ethtool(interface).check_bonding():
+                raise ConfigError(error_msg + 'driver is not supported!')
+
     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')
diff --git a/src/conf_mode/interfaces_ethernet.py b/src/conf_mode/interfaces_ethernet.py
index edcc6563c..d926c458c 100755
--- a/src/conf_mode/interfaces_ethernet.py
+++ b/src/conf_mode/interfaces_ethernet.py
@@ -273,6 +273,20 @@ def verify_offload(ethernet: dict, ethtool: Ethtool):
             raise ConfigError('Xen netback drivers requires scatter-gatter offloading '\
                               'for MTU size larger then 1500 bytes')
 
+def verify_mac_change(ethernet: dict, ethtool: Ethtool):
+    """
+     Verify if ethernet card driver supports changing the interface MAC address.
+     AWS ENA driver has no support for MAC address changes.
+
+    :param ethernet: dictionary which is received from get_interface_dict
+    :type ethernet: dict
+    :param ethtool: Ethernet object
+    :type ethtool: Ethtool
+    """
+    if 'mac' not in ethernet:
+        return None
+    if not ethtool.check_mac_change():
+        raise ConfigError(f'Driver does not suport changing MAC address!')
 
 def verify_allowedbond_changes(ethernet: dict):
     """
@@ -371,51 +385,45 @@ def verify(ethernet):
 
     if 'deleted' in ethernet:
         return None
+
+    ethtool = Ethtool(ethernet['ifname'])
+    verify_interface_exists(ethernet, ethernet['ifname'])
+    verify_eapol(ethernet)
+    verify_mirror_redirect(ethernet)
+    # 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)
+    verify_mac_change(ethernet, ethtool)
+
     if 'is_bond_member' in ethernet:
-        verify_bond_member(ethernet)
+        verify_bond_member(ethernet, ethtool)
     else:
-        verify_ethernet(ethernet)
+        verify_ethernet(ethernet, ethtool)
 
 
-def verify_bond_member(ethernet):
+def verify_bond_member(ethernet: dict, ethtool: Ethtool) -> None:
     """
      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(ethernet, 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)
+    return None
 
-def verify_ethernet(ethernet):
+def verify_ethernet(ethernet: dict, ethtool: Ethtool) -> None:
     """
      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(ethernet, 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