From 0c4d53f2b756c8ecc3c17e3403ba66ebb5ce1560 Mon Sep 17 00:00:00 2001 From: Chris Patterson Date: Wed, 10 May 2023 12:05:15 -0700 Subject: net: purge blacklist_drivers across net and azure (#2160) It was only used by Hyper-V which now has a filtering mechanism that does not require the use of a denylist. This exposed some issues with tests misspelling "hv_netvsc" and using unmatched mac addresses. This fixes those to work with the current filter that does not rely on the driver name. Signed-off-by: Chris Patterson --- cloudinit/distros/networking.py | 20 ++----- cloudinit/net/__init__.py | 104 ++++++++++------------------------ cloudinit/net/network_state.py | 2 +- cloudinit/sources/DataSourceAzure.py | 29 +--------- tests/unittests/net/test_init.py | 24 ++++++-- tests/unittests/sources/test_azure.py | 9 +-- tests/unittests/test_net.py | 39 ++++++------- tests/unittests/test_upgrade.py | 4 -- 8 files changed, 77 insertions(+), 154 deletions(-) diff --git a/cloudinit/distros/networking.py b/cloudinit/distros/networking.py index 28ee1b43..c645c748 100644 --- a/cloudinit/distros/networking.py +++ b/cloudinit/distros/networking.py @@ -1,7 +1,6 @@ import abc import logging import os -from typing import List, Optional from cloudinit import net, subp, util from cloudinit.distros.parsers import ifconfig @@ -23,9 +22,6 @@ class Networking(metaclass=abc.ABCMeta): Hierarchy" in CONTRIBUTING.rst for full details. """ - def __init__(self): - self.blacklist_drivers: Optional[List[str]] = None - def _get_current_rename_info(self) -> dict: return net._get_current_rename_info() @@ -45,15 +41,11 @@ class Networking(metaclass=abc.ABCMeta): def extract_physdevs(self, netcfg: NetworkConfig) -> list: return net.extract_physdevs(netcfg) - def find_fallback_nic(self, *, blacklist_drivers=None): - return net.find_fallback_nic(blacklist_drivers=blacklist_drivers) + def find_fallback_nic(self): + return net.find_fallback_nic() - def generate_fallback_config( - self, *, blacklist_drivers=None, config_driver: bool = False - ): - return net.generate_fallback_config( - blacklist_drivers=blacklist_drivers, config_driver=config_driver - ) + def generate_fallback_config(self, *, config_driver: bool = False): + return net.generate_fallback_config(config_driver=config_driver) def get_devicelist(self) -> list: return net.get_devicelist() @@ -73,9 +65,7 @@ class Networking(metaclass=abc.ABCMeta): return net.get_interfaces() def get_interfaces_by_mac(self) -> dict: - return net.get_interfaces_by_mac( - blacklist_drivers=self.blacklist_drivers - ) + return net.get_interfaces_by_mac() def get_master(self, devname: DeviceName): return net.get_master(devname) diff --git a/cloudinit/net/__init__.py b/cloudinit/net/__init__.py index 0d7ed8c9..bf21633b 100644 --- a/cloudinit/net/__init__.py +++ b/cloudinit/net/__init__.py @@ -399,65 +399,52 @@ def is_disabled_cfg(cfg): return cfg.get("config") == "disabled" -def find_candidate_nics( - blacklist_drivers: Optional[List[str]] = None, -) -> List[str]: +def find_candidate_nics() -> List[str]: """Get the list of network interfaces viable for networking. @return List of interfaces, sorted naturally. """ if util.is_FreeBSD() or util.is_DragonFlyBSD(): - return find_candidate_nics_on_freebsd(blacklist_drivers) + return find_candidate_nics_on_freebsd() elif util.is_NetBSD() or util.is_OpenBSD(): - return find_candidate_nics_on_netbsd_or_openbsd(blacklist_drivers) + return find_candidate_nics_on_netbsd_or_openbsd() else: - return find_candidate_nics_on_linux(blacklist_drivers) + return find_candidate_nics_on_linux() -def find_fallback_nic( - blacklist_drivers: Optional[List[str]] = None, -) -> Optional[str]: +def find_fallback_nic() -> Optional[str]: """Get the name of the 'fallback' network device.""" if util.is_FreeBSD() or util.is_DragonFlyBSD(): - return find_fallback_nic_on_freebsd(blacklist_drivers) + return find_fallback_nic_on_freebsd() elif util.is_NetBSD() or util.is_OpenBSD(): - return find_fallback_nic_on_netbsd_or_openbsd(blacklist_drivers) + return find_fallback_nic_on_netbsd_or_openbsd() else: - return find_fallback_nic_on_linux(blacklist_drivers) + return find_fallback_nic_on_linux() -def find_candidate_nics_on_netbsd_or_openbsd( - blacklist_drivers: Optional[List[str]] = None, -) -> List[str]: +def find_candidate_nics_on_netbsd_or_openbsd() -> List[str]: """Get the names of the candidate network devices on NetBSD/OpenBSD. - @param blacklist_drivers: currently ignored @return list of sorted interfaces """ return sorted(get_interfaces_by_mac().values(), key=natural_sort_key) -def find_fallback_nic_on_netbsd_or_openbsd( - blacklist_drivers: Optional[List[str]] = None, -) -> Optional[str]: +def find_fallback_nic_on_netbsd_or_openbsd() -> Optional[str]: """Get the 'fallback' network device name on NetBSD/OpenBSD. - @param blacklist_drivers: currently ignored @return default interface, or None """ - names = find_candidate_nics_on_netbsd_or_openbsd(blacklist_drivers) + names = find_candidate_nics_on_netbsd_or_openbsd() if names: return names[0] return None -def find_candidate_nics_on_freebsd( - blacklist_drivers: Optional[List[str]] = None, -) -> List[str]: +def find_candidate_nics_on_freebsd() -> List[str]: """Get the names of the candidate network devices on FreeBSD. - @param blacklist_drivers: Currently ignored. @return List of sorted interfaces. """ stdout, _stderr = subp.subp(["ifconfig", "-l", "-u", "ether"]) @@ -470,32 +457,23 @@ def find_candidate_nics_on_freebsd( return sorted(get_interfaces_by_mac().values(), key=natural_sort_key) -def find_fallback_nic_on_freebsd( - blacklist_drivers: Optional[List[str]] = None, -) -> Optional[str]: +def find_fallback_nic_on_freebsd() -> Optional[str]: """Get the 'fallback' network device name on FreeBSD. - @param blacklist_drivers: Currently ignored. @return List of sorted interfaces. """ - names = find_candidate_nics_on_freebsd(blacklist_drivers) + names = find_candidate_nics_on_freebsd() if names: return names[0] return None -def find_candidate_nics_on_linux( - blacklist_drivers: Optional[List[str]] = None, -) -> List[str]: +def find_candidate_nics_on_linux() -> List[str]: """Get the names of the candidate network devices on Linux. - @param blacklist_drivers: Filter out NICs with these drivers. @return List of sorted interfaces. """ - if not blacklist_drivers: - blacklist_drivers = [] - if "net.ifnames=0" in util.get_cmdline(): LOG.debug("Stable ifnames disabled by net.ifnames=0 in /proc/cmdline") else: @@ -517,7 +495,6 @@ def find_candidate_nics_on_linux( connected = [] possibly_connected = [] for interface, _, _, _ in get_interfaces( - blacklist_drivers=blacklist_drivers, filter_openvswitch_internal=False, filter_slave_if_master_not_bridge_bond_openvswitch=False, filter_vlan=False, @@ -565,27 +542,24 @@ def find_candidate_nics_on_linux( return sorted_interfaces -def find_fallback_nic_on_linux( - blacklist_drivers: Optional[List[str]] = None, -) -> Optional[str]: +def find_fallback_nic_on_linux() -> Optional[str]: """Get the 'fallback' network device name on Linux. - @param blacklist_drivers: Ignore devices with these drivers. @return List of sorted interfaces. """ - names = find_candidate_nics_on_linux(blacklist_drivers) + names = find_candidate_nics_on_linux() if names: return names[0] return None -def generate_fallback_config(blacklist_drivers=None, config_driver=None): +def generate_fallback_config(config_driver=None): """Generate network cfg v2 for dhcp on the NIC most likely connected.""" if not config_driver: config_driver = False - target_name = find_fallback_nic(blacklist_drivers=blacklist_drivers) + target_name = find_fallback_nic() if not target_name: # can't read any interfaces addresses (or there are none); give up return None @@ -903,23 +877,15 @@ def get_ib_interface_hwaddr(ifname, ethernet_format): return mac -def get_interfaces_by_mac(blacklist_drivers=None) -> dict: +def get_interfaces_by_mac() -> dict: if util.is_FreeBSD() or util.is_DragonFlyBSD(): - return get_interfaces_by_mac_on_freebsd( - blacklist_drivers=blacklist_drivers - ) + return get_interfaces_by_mac_on_freebsd() elif util.is_NetBSD(): - return get_interfaces_by_mac_on_netbsd( - blacklist_drivers=blacklist_drivers - ) + return get_interfaces_by_mac_on_netbsd() elif util.is_OpenBSD(): - return get_interfaces_by_mac_on_openbsd( - blacklist_drivers=blacklist_drivers - ) + return get_interfaces_by_mac_on_openbsd() else: - return get_interfaces_by_mac_on_linux( - blacklist_drivers=blacklist_drivers - ) + return get_interfaces_by_mac_on_linux() def find_interface_name_from_mac(mac: str) -> Optional[str]: @@ -929,7 +895,7 @@ def find_interface_name_from_mac(mac: str) -> Optional[str]: return None -def get_interfaces_by_mac_on_freebsd(blacklist_drivers=None) -> dict: +def get_interfaces_by_mac_on_freebsd() -> dict: (out, _) = subp.subp(["ifconfig", "-a", "ether"]) # flatten each interface block in a single line @@ -957,7 +923,7 @@ def get_interfaces_by_mac_on_freebsd(blacklist_drivers=None) -> dict: return results -def get_interfaces_by_mac_on_netbsd(blacklist_drivers=None) -> dict: +def get_interfaces_by_mac_on_netbsd() -> dict: ret = {} re_field_match = ( r"(?P\w+).*address:\s" @@ -973,7 +939,7 @@ def get_interfaces_by_mac_on_netbsd(blacklist_drivers=None) -> dict: return ret -def get_interfaces_by_mac_on_openbsd(blacklist_drivers=None) -> dict: +def get_interfaces_by_mac_on_openbsd() -> dict: ret = {} re_field_match = ( r"(?P\w+).*lladdr\s" @@ -989,15 +955,13 @@ def get_interfaces_by_mac_on_openbsd(blacklist_drivers=None) -> dict: return ret -def get_interfaces_by_mac_on_linux(blacklist_drivers=None) -> dict: +def get_interfaces_by_mac_on_linux() -> dict: """Build a dictionary of tuples {mac: name}. Bridges and any devices that have a 'stolen' mac are excluded.""" ret: dict = {} - for name, mac, driver, _devid in get_interfaces( - blacklist_drivers=blacklist_drivers - ): + for name, mac, driver, _devid in get_interfaces(): if mac in ret: # This is intended to be a short-term fix of LP: #1997922 # Long term, we should better handle configuration of virtual @@ -1063,7 +1027,6 @@ def get_interfaces_by_mac_on_linux(blacklist_drivers=None) -> dict: def get_interfaces( - blacklist_drivers=None, filter_hyperv_vf_with_synthetic: bool = True, filter_openvswitch_internal: bool = True, filter_slave_if_master_not_bridge_bond_openvswitch: bool = True, @@ -1077,8 +1040,6 @@ def get_interfaces( Bridges and any devices that have a 'stolen' mac are excluded.""" filtered_logger = LOG.debug if log_filtered_reasons else lambda *args: None ret = [] - if blacklist_drivers is None: - blacklist_drivers = [] devs = get_devicelist() # 16 somewhat arbitrarily chosen. Normally a mac is 6 '00:' tokens. zero_mac = ":".join(("00",) * 16) @@ -1115,14 +1076,7 @@ def get_interfaces( name ): continue - # skip nics that have drivers blacklisted driver = device_driver(name) - if driver in blacklist_drivers: - filtered_logger( - "Ignoring interface with %s driver: %s", driver, name - ) - continue - ret.append((name, mac, driver, device_devid(name))) # Last-pass filter(s) which need the full device list to perform properly. diff --git a/cloudinit/net/network_state.py b/cloudinit/net/network_state.py index 158a2951..40b1ee0f 100644 --- a/cloudinit/net/network_state.py +++ b/cloudinit/net/network_state.py @@ -675,7 +675,7 @@ class NetworkStateInterpreter(metaclass=CommandHandlerMeta): eno1: match: macaddress: 00:11:22:33:44:55 - driver: hv_netsvc + driver: hv_netvsc wakeonlan: true dhcp4: true dhcp6: false diff --git a/cloudinit/sources/DataSourceAzure.py b/cloudinit/sources/DataSourceAzure.py index 863a06b5..77b53aaa 100644 --- a/cloudinit/sources/DataSourceAzure.py +++ b/cloudinit/sources/DataSourceAzure.py @@ -79,25 +79,6 @@ UBUNTU_EXTENDED_NETWORK_SCRIPTS = [ "/run/network/interfaces.ephemeral.d", ] -# This list is used to blacklist devices that will be considered -# for renaming or fallback interfaces. -# -# On Azure network devices using these drivers are automatically -# configured by the platform and should not be configured by -# cloud-init's network configuration. -# -# Note: -# Azure Dv4 and Ev4 series VMs always have mlx5 hardware. -# https://docs.microsoft.com/en-us/azure/virtual-machines/dv4-dsv4-series -# https://docs.microsoft.com/en-us/azure/virtual-machines/ev4-esv4-series -# Earlier D and E series VMs (such as Dv2, Dv3, and Ev3 series VMs) -# can have either mlx4 or mlx5 hardware, with the older series VMs -# having a higher chance of coming with mlx4 hardware. -# https://docs.microsoft.com/en-us/azure/virtual-machines/dv2-dsv2-series -# https://docs.microsoft.com/en-us/azure/virtual-machines/dv3-dsv3-series -# https://docs.microsoft.com/en-us/azure/virtual-machines/ev3-esv3-series -BLACKLIST_DRIVERS = ["mlx4_core", "mlx5_core"] - def find_storvscid_from_sysctl_pnpinfo(sysctl_out, deviceid): # extract the 'X' from dev.storvsc.X. if deviceid matches @@ -182,7 +163,7 @@ def determine_device_driver_for_mac(mac: str) -> Optional[str]: """Determine the device driver to match on, if any.""" drivers = [ i[2] - for i in net.get_interfaces(blacklist_drivers=BLACKLIST_DRIVERS) + for i in net.get_interfaces() if mac == normalize_mac_address(i[1]) ] if "hv_netvsc" in drivers: @@ -723,8 +704,6 @@ class DataSourceAzure(sources.DataSource): except Exception as e: LOG.warning("Failed to get system information: %s", e) - self.distro.networking.blacklist_drivers = BLACKLIST_DRIVERS - try: crawled_data = util.log_time( logfunc=LOG.debug, @@ -1888,13 +1867,11 @@ def generate_network_config_from_instance_network_metadata( @azure_ds_telemetry_reporter def _generate_network_config_from_fallback_config() -> dict: - """Generate fallback network config excluding blacklisted devices. + """Generate fallback network config. @return: Dictionary containing network version 2 standard configuration. """ - cfg = net.generate_fallback_config( - blacklist_drivers=BLACKLIST_DRIVERS, config_driver=True - ) + cfg = net.generate_fallback_config(config_driver=True) if cfg is None: return {} return cfg diff --git a/tests/unittests/net/test_init.py b/tests/unittests/net/test_init.py index 7cf5db27..a57cbdff 100644 --- a/tests/unittests/net/test_init.py +++ b/tests/unittests/net/test_init.py @@ -482,13 +482,27 @@ class TestNetFindCandidateNics: operstate="testing", ) self.create_fake_interface( - name="blacklistedDriverIgnored", - driver="bad", + name="hv", + driver="hv_netvsc", + address="00:11:22:00:00:f0", ) - - assert ( - net.find_candidate_nics_on_linux(blacklist_drivers=["bad"]) == [] + self.create_fake_interface( + name="hv_vf_mlx4", + driver="mlx4_core", + address="00:11:22:00:00:f0", + ) + self.create_fake_interface( + name="hv_vf_mlx5", + driver="mlx5_core", + address="00:11:22:00:00:f0", ) + self.create_fake_interface( + name="hv_vf_mana", + driver="mana", + address="00:11:22:00:00:f0", + ) + + assert net.find_candidate_nics_on_linux() == ["hv"] def test_carrier_preferred(self): self.create_fake_interface(name="eth0", carrier=False, dormant=True) diff --git a/tests/unittests/sources/test_azure.py b/tests/unittests/sources/test_azure.py index f1964fde..677c250d 100644 --- a/tests/unittests/sources/test_azure.py +++ b/tests/unittests/sources/test_azure.py @@ -804,7 +804,7 @@ class TestNetworkConfig: "type": "physical", "name": "eth0", "mac_address": "00:11:22:33:44:55", - "params": {"driver": "hv_netsvc"}, + "params": {"driver": "hv_netvsc"}, "subnets": [{"type": "dhcp"}], } ], @@ -2004,14 +2004,9 @@ scbus-1 on xpt0 bus 0 distro = distro_cls("ubuntu", {}, self.paths) dsrc = self._get_ds(data, distro=distro) dsrc.get_data() - self.assertEqual( - distro.networking.blacklist_drivers, dsaz.BLACKLIST_DRIVERS - ) distro.networking.get_interfaces_by_mac() - m_net_get_interfaces.assert_called_with( - blacklist_drivers=dsaz.BLACKLIST_DRIVERS - ) + m_net_get_interfaces.assert_called_with() @mock.patch( "cloudinit.sources.helpers.azure.OpenSSLManager.parse_certificates" diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py index a8b18ee2..dafe9d06 100644 --- a/tests/unittests/test_net.py +++ b/tests/unittests/test_net.py @@ -4045,7 +4045,7 @@ class TestGenerateFallbackConfig(CiTestCase): "dormant": False, "operstate": "down", "address": "00:11:22:33:44:55", - "device/driver": "hv_netsvc", + "device/driver": "hv_netvsc", "device/device": "0x3", "name_assign_type": "4", }, @@ -4078,7 +4078,7 @@ class TestGenerateFallbackConfig(CiTestCase): "set-name": "eth0", "match": { "macaddress": "00:11:22:33:44:55", - "driver": "hv_netsvc", + "driver": "hv_netvsc", }, } }, @@ -4099,7 +4099,7 @@ class TestGenerateFallbackConfig(CiTestCase): "dormant": False, "operstate": "down", "address": "00:11:22:33:44:55", - "device/driver": "hv_netsvc", + "device/driver": "hv_netvsc", "device/device": "0x3", "name_assign_type": "4", "addr_assign_type": "0", @@ -4164,7 +4164,7 @@ iface eth0 inet dhcp expected_rule = [ 'SUBSYSTEM=="net"', 'ACTION=="add"', - 'DRIVERS=="hv_netsvc"', + 'DRIVERS=="hv_netvsc"', 'ATTR{address}=="00:11:22:33:44:55"', 'NAME="eth0"', ] @@ -4173,7 +4173,7 @@ iface eth0 inet dhcp @mock.patch("cloudinit.net.sys_dev_path") @mock.patch("cloudinit.net.read_sys_net") @mock.patch("cloudinit.net.get_devicelist") - def test_device_driver_blacklist( + def test_hv_netvsc_vf_filter( self, mock_get_devicelist, mock_read_sys_net, mock_sys_dev_path ): devices = { @@ -4183,7 +4183,7 @@ iface eth0 inet dhcp "dormant": False, "operstate": "down", "address": "00:11:22:33:44:55", - "device/driver": "hv_netsvc", + "device/driver": "hv_netvsc", "device/device": "0x3", "name_assign_type": "4", "addr_assign_type": "0", @@ -4195,7 +4195,7 @@ iface eth0 inet dhcp "carrier": False, "dormant": False, "operstate": "down", - "address": "00:11:22:33:44:56", + "address": "00:11:22:33:44:55", "device/driver": "mlx4_core", "device/device": "0x7", "name_assign_type": "4", @@ -4214,10 +4214,7 @@ iface eth0 inet dhcp dev_attrs=devices, ) - blacklist = ["mlx4_core"] - network_cfg = net.generate_fallback_config( - blacklist_drivers=blacklist, config_driver=True - ) + network_cfg = net.generate_fallback_config(config_driver=True) ns = network_state.parse_net_config_data( network_cfg, skip_broken=False ) @@ -4251,7 +4248,7 @@ iface eth1 inet dhcp expected_rule = [ 'SUBSYSTEM=="net"', 'ACTION=="add"', - 'DRIVERS=="hv_netsvc"', + 'DRIVERS=="hv_netvsc"', 'ATTR{address}=="00:11:22:33:44:55"', 'NAME="eth1"', ] @@ -4278,7 +4275,7 @@ iface eth1 inet dhcp "dormant": False, "operstate": "down", "address": "00:11:22:33:44:55", - "device/driver": "hv_netsvc", + "device/driver": "hv_netvsc", "device/device": "0x3", "name_assign_type": False, }, @@ -4327,7 +4324,7 @@ iface eth1 inet dhcp "dormant": False, "operstate": "down", "address": "00:11:22:33:44:55", - "device/driver": "hv_netsvc", + "device/driver": "hv_netvsc", "device/device": "0x3", "name_assign_type": False, }, @@ -5252,7 +5249,7 @@ USERCTL=no "dormant": False, "operstate": "down", "address": "CF:D6:AF:48:E8:80", - "device/driver": "hv_netsvc", + "device/driver": "hv_netvsc", "device/device": "0x3", "name_assign_type": "4", "addr_assign_type": "0", @@ -8490,14 +8487,14 @@ class TestRenameInterfaces(CiTestCase): @mock.patch("cloudinit.subp.subp") def test_rename_duplicate_macs(self, mock_subp): renames = [ - ("00:11:22:33:44:55", "eth0", "hv_netsvc", "0x3"), + ("00:11:22:33:44:55", "eth0", "hv_netvsc", "0x3"), ("00:11:22:33:44:55", "vf1", "mlx4_core", "0x5"), ] current_info = { "eth0": { "downable": True, "device_id": "0x3", - "driver": "hv_netsvc", + "driver": "hv_netvsc", "mac": "00:11:22:33:44:55", "name": "eth0", "up": False, @@ -8524,14 +8521,14 @@ class TestRenameInterfaces(CiTestCase): @mock.patch("cloudinit.subp.subp") def test_rename_duplicate_macs_driver_no_devid(self, mock_subp): renames = [ - ("00:11:22:33:44:55", "eth0", "hv_netsvc", None), + ("00:11:22:33:44:55", "eth0", "hv_netvsc", None), ("00:11:22:33:44:55", "vf1", "mlx4_core", None), ] current_info = { "eth0": { "downable": True, "device_id": "0x3", - "driver": "hv_netsvc", + "driver": "hv_netvsc", "mac": "00:11:22:33:44:55", "name": "eth0", "up": False, @@ -8558,7 +8555,7 @@ class TestRenameInterfaces(CiTestCase): @mock.patch("cloudinit.subp.subp") def test_rename_multi_mac_dups(self, mock_subp): renames = [ - ("00:11:22:33:44:55", "eth0", "hv_netsvc", "0x3"), + ("00:11:22:33:44:55", "eth0", "hv_netvsc", "0x3"), ("00:11:22:33:44:55", "vf1", "mlx4_core", "0x5"), ("00:11:22:33:44:55", "vf2", "mlx4_core", "0x7"), ] @@ -8566,7 +8563,7 @@ class TestRenameInterfaces(CiTestCase): "eth0": { "downable": True, "device_id": "0x3", - "driver": "hv_netsvc", + "driver": "hv_netvsc", "mac": "00:11:22:33:44:55", "name": "eth0", "up": False, diff --git a/tests/unittests/test_upgrade.py b/tests/unittests/test_upgrade.py index 218915ed..531ed3cf 100644 --- a/tests/unittests/test_upgrade.py +++ b/tests/unittests/test_upgrade.py @@ -64,10 +64,6 @@ class TestUpgrade: """We always expect to have ``.networking`` on ``Distro`` objects.""" assert previous_obj_pkl.distro.networking is not None - def test_blacklist_drivers_set_on_networking(self, previous_obj_pkl): - """We always expect Networking.blacklist_drivers to be initialised.""" - assert previous_obj_pkl.distro.networking.blacklist_drivers is None - def test_paths_has_run_dir_attribute(self, previous_obj_pkl): assert previous_obj_pkl.paths.run_dir is not None -- cgit v1.2.1