diff --git a/python/vyos/ethtool.py b/python/vyos/ethtool.py index 85e228ede63..688fa324909 100644 --- a/python/vyos/ethtool.py +++ b/python/vyos/ethtool.py @@ -217,6 +217,8 @@ def check_speed_duplex(self, speed, duplex): def check_flow_control(self): """ Check if the NIC supports flow-control """ + if self.get_driver_name() in _drivers_without_speed_duplex_flow: + return False return bool(self._flow_control) def get_flow_control(self): diff --git a/smoketest/scripts/cli/test_interfaces_ethernet.py b/smoketest/scripts/cli/test_interfaces_ethernet.py index 6f290c7feab..086eed33802 100755 --- a/smoketest/scripts/cli/test_interfaces_ethernet.py +++ b/smoketest/scripts/cli/test_interfaces_ethernet.py @@ -229,6 +229,10 @@ def test_ethtool_coalesce(self): msg = 'Driver does not fully support coalesce configuration' with self.assertRaisesRegex(ConfigSessionError, msg): self.cli_commit() + # the failed commit leaves rx-usecs/tx-usecs staged in + # the candidate config (commit() does not auto-rollback) - + # discard it so it doesn't leak into the next interface + self.cli_discard() continue # To find out the supported features @@ -287,17 +291,25 @@ def test_ethtool_coalesce(self): def test_ethtool_flow_control(self): for interface in self._interfaces: + ethtool = Ethtool(interface) # Disable flow-control self.cli_set(self._base_path + [interface, 'disable-flow-control']) - # Check current flow-control state on ethernet interface - out, err = popen(f'sudo ethtool --json --show-pause {interface}') - # Flow-control not supported - test if it bails out with a proper - # this is a dynamic path where err = 1 on VMware, but err = 0 on - # a physical box. - if bool(err): + + # Ask the same capability check the CLI commit itself uses, + # rather than a raw ethtool --show-pause probe: some drivers + # (virtio_net, vmxnet3, xen_netfront, ...) support querying + # pause parameters but not changing them, so a bare --show-pause + # exit code is not a reliable predictor of whether the commit + # will succeed. + if not ethtool.check_flow_control(): with self.assertRaises(ConfigSessionError): self.cli_commit() + # the failed commit leaves disable-flow-control staged in + # the candidate config (commit() does not auto-rollback) - + # discard it so it doesn't leak into the next interface + self.cli_discard() else: + out, err = popen(f'sudo ethtool --json --show-pause {interface}') out = loads(out) # Flow control is on self.assertTrue(out[0]['autonegotiate']) diff --git a/src/helpers/vyos-interface-rescan.py b/src/helpers/vyos-interface-rescan.py index fea9bca1c4b..0bc3ac7d084 100755 --- a/src/helpers/vyos-interface-rescan.py +++ b/src/helpers/vyos-interface-rescan.py @@ -97,7 +97,8 @@ def get_interface_type(intf: str) -> str: return intf_type def get_new_interfaces() -> dict: - """ Read any new interface data left in /run/udev/vyos by vyos_net_name + """ Read any new interface data left in /run/udev/vyos by + vyos-net-name-resolve.py for interfaces without a configured hw-id """ interfaces = {} diff --git a/src/init/vyos-router b/src/init/vyos-router index 06ec89d722c..8e0a671a66a 100755 --- a/src/init/vyos-router +++ b/src/init/vyos-router @@ -367,6 +367,26 @@ update_interface_config () fi } +# warn (but do not fail boot) about configured hw-id interfaces whose +# hardware was not found by vyos-net-name-resolve.service +warn_missing_interface_hardware () +{ + local status_file="/run/vyos-net-name-resolve.json" + [ -f "${status_file}" ] || return 0 + + jq -r '.missing // {} | to_entries[] | "\(.value) \(.key)"' "${status_file}" \ + 2>/dev/null | while read -r intf hwid; do + [ -n "${intf}" ] || continue + log_failure_msg "hw-id ${hwid} configured as '${intf}' was not found during boot - check cabling, hardware and driver/firmware" + done + + jq -r '.pending_unresolved // [] | .[]' "${status_file}" 2>/dev/null | \ + while read -r intf; do + [ -n "${intf}" ] || continue + log_failure_msg "interface '${intf}' still has no hw-id configured after this boot's naming pass - bind it manually, e.g. 'set interfaces ethernet ${intf} hw-id ', or remove the node if it is no longer needed" + done +} + cleanup_post_commit_hooks () { # Remove links from the post-commit hooks directory. # note that this approach only supports hooks that are "configured", @@ -610,6 +630,12 @@ start () disabled migrate || migrate_bootfile || overall_status=1 + # config.boot is only guaranteed to exist/be current from this point on + # (tmpfs mount + encrypted config unlock + syntax migration have all + # already happened above) - hw-id based interface naming must be + # resolved against it before anything below applies interface config. + systemctl start vyos-net-name-resolve.service || overall_status=1 + restore_if_missing_preconfig_script run_preconfig_script @@ -617,6 +643,7 @@ start () run_postupgrade_script update_interface_config || overall_status=1 + warn_missing_interface_hardware disabled system_activate || system_activate || overall_status=1 diff --git a/src/system/vyos-net-name-resolve.py b/src/system/vyos-net-name-resolve.py new file mode 100755 index 00000000000..7934e844221 --- /dev/null +++ b/src/system/vyos-net-name-resolve.py @@ -0,0 +1,640 @@ +#!/usr/bin/env python3 +# +# Copyright 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 . +# +# Single authoritative pass that enforces "interfaces/{ethernet,wireless}/ +# */hw-id" from config.boot onto the live system. It runs once, after udev +# has settled, and decides names from a fully-formed view of the hardware +# instead of reacting to individual "add" uevents. +# +# The old udev-time approach (src/udev/vyos_net_name, invoked straight from +# a udev rule) decided the final, hw-id based name from a single sysfs +# snapshot taken at the moment one interface's "add" event was evaluated. +# On systems with several different NIC vendors/drivers that snapshot is +# unreliable: the DRIVERS attribute walk-up and the interface's permanent +# MAC address are not guaranteed to be populated yet, and neither gets a +# second chance since each uevent is only evaluated once. This script +# sidesteps that by re-reading live state after hardware has had time to +# settle, and by verifying/repairing the outcome rather than guessing once. + +import json +import logging +import logging.handlers +import re +import tempfile +import time +from pathlib import Path +from sys import exit + +from vyos.configtree import ConfigTree +from vyos.defaults import directories +from vyos.migrate import ConfigMigrate +from vyos.utils.process import rc_cmd +from vyos.utils.process import run + +# Bounded wait for all configured hw-id hardware to appear. NICs needing a +# longer firmware/link-training window are rare enough that this has not +# been exposed as a CLI knob yet. +HARDWARE_WAIT_TIMEOUT = 10 +HARDWARE_WAIT_POLL = 0.25 + +# Bounded wait for hardware without a configured hw-id to settle before +# bootstrap-naming it. There is no specific MAC to wait for here, so +# "N consecutive identical snapshots" stands in for "hardware has stopped +# registering/renaming interfaces". +HARDWARE_SETTLE_TIMEOUT = 10 +HARDWARE_SETTLE_STABLE_POLLS = 3 + +config_path = '/opt/vyatta/etc/config/config.boot' +vyos_udev_dir = directories['vyos_udev_dir'] +status_file = Path('/run/vyos-net-name-resolve.json') + +logger = logging.getLogger() + + +def _load_config_boot() -> ConfigTree: + """Parse config.boot into a ConfigTree, migrating stale syntax if + needed. Returns None if there is no persisted config yet (livecd/ISO). + """ + if not Path(config_path).is_file(): + return None + + try: + config_file = Path(config_path).read_text() + except OSError as e: + logger.critical(f'OSError {e}') + exit(1) + + try: + return ConfigTree(config_file) + except Exception: + try: + logger.debug('updating component version string syntax') + # this will update the component version string syntax, + # required for updates 1.2 --> 1.3/1.4 + with tempfile.NamedTemporaryFile() as fp: + Path(fp.name).write_text(config_file) + config_migrate = ConfigMigrate(fp.name) + if config_migrate.syntax_update_needed(): + config_migrate.update_syntax() + config_migrate.write_config() + config_file = Path(fp.name).read_text() + + return ConfigTree(config_file) + except Exception as e: + logger.critical(f'ConfigTree error: {e}') + exit(1) + + +def get_configfile_interfaces() -> dict: + """Read hw-id -> name mapping for ethernet/wireless from config.boot""" + interfaces: dict = {} + + config = _load_config_boot() + if config is None: + return interfaces + + for base in (['interfaces', 'ethernet'], ['interfaces', 'wireless']): + if not config.exists(base): + continue + for intf in config.list_nodes(base): + path = base + [intf, 'hw-id'] + if not config.exists(path): + logger.warning(f"no 'hw-id' entry for {intf}") + continue + hwid = config.return_value(path).lower() + if hwid in interfaces: + logger.warning( + f'multiple entries for {hwid}: {interfaces[hwid]}, {intf}' + ) + continue + interfaces[hwid] = intf + + logger.debug(f'config file entries: {interfaces}') + return interfaces + + +def get_pending_hwid_nodes() -> dict: + """Interface nodes that exist under interfaces/{ethernet,wireless} in + config.boot but have no 'hw-id' leaf - e.g. the documented "NIC + replaced" remediation (`delete interfaces ethernet eth1 hw-id`) + intentionally leaves the node (and its other settings, like address) + in place, expecting the SAME node to receive a fresh hw-id. These + names have no known MAC yet - that is the point - so they cannot be + folded into get_configfile_interfaces()'s MAC-keyed dict; they are + surfaced here, grouped by type, so main() can try to match them to + this boot's unconfigured hardware (see match_pending_nodes()). + """ + pending = {'ethernet': set(), 'wireless': set()} + + config = _load_config_boot() + if config is None: + return pending + + for intf_type, base in (('ethernet', ['interfaces', 'ethernet']), + ('wireless', ['interfaces', 'wireless'])): + if not config.exists(base): + continue + for intf in config.list_nodes(base): + if not config.exists(base + [intf, 'hw-id']): + pending[intf_type].add(intf) + + return pending + + +def get_permanent_mac(ifname: str, sys_class_net: str = '/sys/class/net') -> str: + """Best-effort permanent hardware address of an interface. + + Falls back to the interface's current address if the driver does not + support reporting a permanent address separately (ethtool -P). + + sys_class_net is overridable for testing against a fake sysfs tree. + """ + code, out = rc_cmd(f'ethtool -P {ifname}') + if code == 0: + m = re.search(r'([0-9a-fA-F]{2}(:[0-9a-fA-F]{2}){5})', out) + if m and m.group(1) != '00:00:00:00:00:00': + return m.group(1).lower() + + try: + return Path(f'{sys_class_net}/{ifname}/address').read_text().strip().lower() + except OSError: + return '' + + +def discover_physical_interfaces(sys_class_net: str = '/sys/class/net') -> dict: + """Return {kernel_name: mac} for every interface backed by a real bus + device - excludes lo, bridges, bonds, VLANs, veth, tunnels, etc. + + sys_class_net is overridable for testing against a fake sysfs tree. + """ + interfaces = {} + net_dir = Path(sys_class_net) + if not net_dir.is_dir(): + return interfaces + + for entry in net_dir.iterdir(): + if not (entry / 'device').exists(): + continue + mac = get_permanent_mac(entry.name, sys_class_net) + if mac: + interfaces[entry.name] = mac + + return interfaces + + +def wait_for_hardware(configured_macs: set, timeout: float = HARDWARE_WAIT_TIMEOUT, + poll: float = HARDWARE_WAIT_POLL) -> tuple: + """Poll until every configured hw-id has shown up, or until timeout""" + deadline = time.monotonic() + timeout + current = discover_physical_interfaces() + while True: + missing = configured_macs - set(current.values()) + if not missing or time.monotonic() >= deadline: + return current, missing + time.sleep(poll) + current = discover_physical_interfaces() + + +def wait_for_settle(initial: dict, timeout: float = HARDWARE_SETTLE_TIMEOUT, + poll: float = HARDWARE_WAIT_POLL, + stable_polls: int = HARDWARE_SETTLE_STABLE_POLLS) -> dict: + """Poll until discover_physical_interfaces() returns the same snapshot + stable_polls times in a row, or until timeout. Used before bootstrap- + naming hardware that has no configured hw-id to wait for by MAC. + """ + deadline = time.monotonic() + timeout + current = initial + stable = 1 if current else 0 + while stable < stable_polls and time.monotonic() < deadline: + time.sleep(poll) + seen = discover_physical_interfaces() + stable = stable + 1 if seen == current else 1 + current = seen + return current + + +def is_wireless_interface(name: str, sys_class_net: str = '/sys/class/net') -> bool: + """Race-free wireless check: existence of the phy80211 symlink, not the + interface's current (possibly still cosmetic/pre-bootstrap) name. + """ + return (Path(sys_class_net) / name / 'phy80211').exists() + + +PCI_BDF_RE = re.compile(r'^[0-9a-f]{4}:[0-9a-f]{2}:[0-9a-f]{2}\.[0-9a-f]$') + +# No real hop-count can reach this - reserves it as a sort-last sentinel +# for interfaces whose bus topology can't be determined (USB NICs, or a +# device symlink that doesn't exist/resolve). +PCIE_DISTANCE_UNKNOWN = 999 + + +def pcie_distance(name: str, sys_class_net: str = '/sys/class/net') -> int: + """Approximate PCIe bus distance from the root complex - counts PCI + domain:bus:device.function segments (e.g. 0000:00:1f.6) in the fully + resolved sysfs path of the interface's 'device' symlink. Non-PCI hops + (virtioN, usbN, the net/ tail, ...) simply don't match and are + skipped, so virtio's device->virtioN->real-PCI-parent indirection + needs no special-casing. Multi-function siblings at the same slot + contribute exactly one matching segment each, so they are not + double-counted relative to their shared depth. + + Returns PCIE_DISTANCE_UNKNOWN (sorts after every real hop-count) if + the 'device' symlink is missing/unresolvable, or the resolved path + has no PCI BDF segment at all (e.g. a USB NIC). + """ + device_link = Path(sys_class_net) / name / 'device' + try: + resolved = device_link.resolve(strict=True) + except (OSError, RuntimeError): + return PCIE_DISTANCE_UNKNOWN + + hops = sum(1 for part in resolved.parts if PCI_BDF_RE.match(part)) + return hops if hops > 0 else PCIE_DISTANCE_UNKNOWN + + +def find_available(names: set, prefix: str) -> str: + """Find the lowest free index for a given interface name prefix""" + index_list = [] + for name in names: + if not name.startswith(prefix): + continue + suffix = name[len(prefix):] + if suffix.isdigit(): + index_list.append(int(suffix)) + + if not index_list: + return f'{prefix}0' + + index_list.sort() + # find 'holes' in list, if any + missing = sorted(set(range(index_list[0], index_list[-1])) - set(index_list)) + if missing: + return f'{prefix}{missing[0]}' + + return f'{prefix}{index_list[-1] + 1}' + + +def find_next_available(names: set, prefix: str, floor: int = 0) -> str: + """Find the lowest free index for a prefix, scanning up from floor. + + Unlike find_available(), this always starts the scan at floor (0 by + default) rather than at the lowest index already present in `names` - + used for bootstrap naming, where a gap must be backfillable even when + it sits below every other index currently in `names` (e.g. `names` + contains only 'eth5' and 'eth9', but 'eth0'..'eth4' and 'eth6'..'eth8' + are genuinely free). A slot only stays unavailable here because it is + explicitly present in `names` - the caller decides what belongs there + (configured hw-id targets, pending nodes, ...), not this function. + """ + n = floor + while f'{prefix}{n}' in names: + n += 1 + return f'{prefix}{n}' + + +def compute_rename_plan(configured: dict, current: dict, pending: dict = None) -> dict: + """Build {from_name: to_name} for every interface that needs to move to + its CLI hw-id name. Also relocates any interface that currently squats + on a name owned by a different hw-id, so the rightful owner can take it. + """ + plan = {} + current_by_mac = {mac: name for name, mac in current.items()} + target_owner_mac = {name: mac for mac, name in configured.items()} + + for mac, target in configured.items(): + source = current_by_mac.get(mac) + if source and source != target: + plan[source] = target + + unchanged = set(current) - set(plan) + # every configured target is reserved, even one whose hw-id hasn't + # shown up yet this boot (missing/faulty/slow driver) - a squatter must + # never be relocated onto a name that hardware still owns, or it would + # just need relocating again the moment that hardware actually appears. + # A pending (hw-id-less but still-configured) node name is reserved the + # same way: it may be about to be reclaimed by the exact NIC that just + # vacated it (see match_pending_nodes()), and even when it isn't, its + # other settings (address, description, ...) are still live in + # config.boot and must not be handed to an unrelated squatter. + reserved = set(configured.values()) + if pending: + reserved |= pending.get('ethernet', set()) | pending.get('wireless', set()) + + for name, mac in current.items(): + if name in plan: + continue + owner_mac = target_owner_mac.get(name) + if owner_mac is not None and owner_mac != mac: + prefix = re.sub(r'\d+$', '', name) or name + taken = unchanged | set(plan.values()) | set(plan.keys()) | reserved + new_name = find_available(taken, prefix) + plan[name] = new_name + unchanged.discard(name) + + return plan + + +def unmatched_candidates(configured: dict, current: dict, existing_plan: dict) -> list: + """Physical interfaces this boot with no configured hw-id - the pool + both ordinary bootstrap naming and pending-node reclaim matching draw + from. This deliberately INCLUDES squatters compute_rename_plan() is + already evicting from a configured target name: excluding them here + let their mac skip match_pending_nodes() entirely, so an unconfigured + NIC that happened to be squatting on someone else's hw-id slot could + silently get a permanent, possibly-ambiguous hw-id via ordinary + bootstrap naming instead of being reclaimed or held back like any + other unconfigured candidate. Their eviction destination from + existing_plan still stands as the fallback - match_pending_nodes() + (via main()'s reclaim loop) or compute_bootstrap_plan() only override + it, they never leave a squatter un-evicted. + """ + return [(mac, name) for name, mac in current.items() + if mac not in configured] + + +def compute_bootstrap_plan(configured: dict, current: dict, existing_plan: dict) -> dict: + """Build {from_name: to_name} for physical interfaces that have no + configured hw-id at all, assigning them a canonical name within their + type group (ethernet/wireless) ordered by PCIe distance from the root + complex first and MAC address as a tie-break, instead of leaving them + at whatever name the racy udev-time fast path produced. Ordering by + topology rather than raw MAC magnitude means an onboard/directly + CPU-attached NIC isn't sorted after add-in cards just because its MAC + happens to be numerically higher. This is what makes a box's very + first boot - before any hw-id exists - just as deterministic as every + boot after hw-id is written (PCIe wiring and MAC are both static + hardware properties), since the name assigned here gets frozen into + config.boot by vyos-interface-rescan.py the same way a real hw-id + match would. + + A pending node's name (hw-id deleted, node kept - see + get_pending_hwid_nodes()) is treated as just another available slot + here, exactly like a numeric gap - not reserved for it specifically. + That is what lets a pending node recover its OWN original hardware + whenever it and some other now-unclaimed NIC become free in the same + boot (e.g. a different interface's config was fully deleted at the + same time): PCIe distance and MAC are static per-NIC properties, so + the relative sort order among any subset of NICs is identical on + every boot. Removing whichever NICs stay configured from that fixed + order leaves the rest in the same relative order they always had - + which, since a box's very first boot assigns names by this exact + sort, is precisely each one's original slot. No pending-node-specific + matching logic is needed for this to hold; it falls out of sorting + the same way every time. main() attributes a resulting name back to + a "reclaim" after the fact by checking it against `pending`'s node + names - see there. + + existing_plan is the hw-id based plan already computed by + compute_rename_plan(): its RIGHTFUL-OWNER targets (an interface moving + to its own configured hw-id name) are reserved so a bootstrap name can + never collide with a configured one. A squatter compute_rename_plan() + is evicting from a configured target IS still bootstrap candidacy here + (see unmatched_candidates()) - its OWN eviction destination is only a + provisional fallback that this function is about to recompute for it + from scratch, so that value must not also count as "taken" (it would + needlessly block a lower slot the unified ascending fill might + otherwise give it, or another candidate, once real). + + A numeric slot is only ever off-limits here because a real, currently + configured hw-id target still occupies or reserves it. A gap left by + fully deleting an interface's config, or a pending node whose hw-id + alone was deleted, carry no such reservation and are freely (and + identically) backfilled - there is no remaining signal in config.boot + to treat those two cases differently, and either way the admin's own + action is what freed the slot. + """ + plan = {} + candidates = unmatched_candidates(configured, current, existing_plan) + if not candidates: + return plan + + candidate_names = {name for _, name in candidates} + # a plan entry whose source IS one of this function's own candidates + # is a squatter's provisional eviction fallback, about to be + # recomputed below - only a rightful-owner move's target is a real, + # authoritative reservation + rightful_movers = {src: target for src, target in existing_plan.items() + if src not in candidate_names} + # a rightful mover's CURRENT (source) name looks occupied right now, + # but safe_bulk_rename()'s two-phase scratch-name staging vacates it + # before any target name is actually claimed - so it must not count + # as taken here either, or a candidate that belongs there (e.g. a + # pending node whose name happens to be some other configured mac's + # racy cosmetic position this boot) gets pushed to a fresh slot + # instead for no reason. + taken = (set(current) - candidate_names - set(rightful_movers)) \ + | set(rightful_movers.values()) + + for mac, name in sorted(candidates, key=lambda c: (pcie_distance(c[1]), c[0])): + prefix = 'wlan' if is_wireless_interface(name) else 'eth' + new_name = find_next_available(taken, prefix) + taken.add(new_name) + if new_name != name: + plan[name] = new_name + + return plan + + +def get_ifindex(name: str) -> str: + try: + return Path(f'/sys/class/net/{name}/ifindex').read_text().strip() + except OSError: + return name + + +def rename_interface(old: str, new: str) -> bool: + run(f'ip link set dev {old} down') + code = run(f'ip link set dev {old} name {new}') + if code != 0: + logger.error(f"failed to rename '{old}' -> '{new}' (exit {code})") + return False + logger.info(f"renamed '{old}' -> '{new}'") + return True + + +def safe_bulk_rename(plan: dict) -> dict: + """Two-phase rename: stage every interface via a unique scratch name + (derived from its ifindex, which is always unique) before assigning + final names. This makes the whole batch collision-proof regardless of + permutations/cycles between current and target names - a straight + from->to rename can fail if the target name is still held by another + interface earlier/later in the same plan. + """ + if not plan: + return {} + + applied = {} + scratch = {} + for old, target in plan.items(): + tmp = f'vyeth{get_ifindex(old)}' + if rename_interface(old, tmp): + scratch[tmp] = (old, target) + applied[old] = target + + for tmp, (old, target) in scratch.items(): + if rename_interface(tmp, target): + run(f'ip link set dev {target} up') + else: + # still sitting under the scratch name, not the target - don't + # report it as renamed, and don't leave it down indefinitely + del applied[old] + run(f'ip link set dev {tmp} up') + + return applied + + +def sync_rescan_hints(current_state: dict, configured: dict, + stale_names: set = frozenset()) -> None: + """Leave a hint under vyos_udev_dir for every interface that has no + hw-id configured yet, so vyos-interface-rescan.py can auto-populate + config.boot. Remove stale/no-longer-hw-id-needed hints. + """ + try: + Path(vyos_udev_dir).mkdir(parents=True, exist_ok=True) + except OSError as e: + logger.critical(f'error creating rescan hint directory: {e}') + return + + for name, mac in current_state.items(): + hint = Path(vyos_udev_dir) / name + if mac in configured: + hint.unlink(missing_ok=True) + else: + try: + hint.write_text(mac) + except OSError as e: + logger.error(f'could not write rescan hint for {name}: {e}') + + for old_name in stale_names: + if old_name not in current_state: + (Path(vyos_udev_dir) / old_name).unlink(missing_ok=True) + + +def write_status(configured: dict, found: dict, missing: set, plan: dict, + pending: dict = None, reclaimed: dict = None, + candidates: list = None) -> None: + """candidates is the full unmatched_candidates() list evaluated this + boot (before reclaim/bootstrap assignment) - surfaced here so a + pending node left unresolved for lack of hardware is self-diagnosable + from this one file: 'pending_unresolved' names what needed a match, + 'unconfigured_candidates' names every physical interface that was in + the running for one, without needing a separate `ip -br link show` or + `show configuration commands` to reconstruct the same picture by hand. + """ + pending = pending or {} + reclaimed = reclaimed or {} + candidates = candidates or [] + all_pending = pending.get('ethernet', set()) | pending.get('wireless', set()) + status = { + 'configured': configured, + 'found': sorted(found.values()), + 'missing': {mac: configured[mac] for mac in sorted(missing)}, + 'renamed': plan, + 'pending_unresolved': sorted(all_pending - set(reclaimed.values())), + 'reclaimed': reclaimed, + 'unconfigured_candidates': {name: mac for mac, name in candidates}, + } + try: + status_file.write_text(json.dumps(status, indent=2)) + except OSError as e: + logger.error(f'could not write status file: {e}') + + +def main(): + configured = get_configfile_interfaces() + pending = get_pending_hwid_nodes() + + if configured: + current, missing = wait_for_hardware(set(configured)) + for mac in sorted(missing): + logger.warning( + f"hw-id '{mac}' configured as '{configured[mac]}' was not found " + f'within {HARDWARE_WAIT_TIMEOUT}s - hardware may be missing, ' + 'still initializing, or its driver failed to load' + ) + plan = compute_rename_plan(configured, current, pending) + else: + logger.info('no hw-id configured yet') + current, missing, plan = discover_physical_interfaces(), set(), {} + + all_pending = pending.get('ethernet', set()) | pending.get('wireless', set()) + + candidates = [] + if all_pending or any(mac not in configured for mac in current.values()): + # bootstrap-name whatever has no hw-id match, deterministically by + # PCIe distance and MAC, instead of leaving it at its racy cosmetic + # udev-time name - this is what vyos-interface-rescan.py will + # freeze into config.boot. A NIC that just lost its hw-id (the + # documented "delete hw-id to force regeneration" remediation) + # competes for this same ascending fill exactly like a numeric + # gap does - see compute_bootstrap_plan() for why that recovers + # its own original hardware rather than an arbitrary one. + # + # The `all_pending` half of this condition matters even when + # `current` (from wait_for_hardware() above, bounded only on + # already-CONFIGURED macs) shows nothing unconfigured yet: on a + # system with several different NIC vendors/drivers, the exact + # hardware a pending node needs can simply not have finished + # probing at this snapshot. Without this, wait_for_settle() below + # would never even run, giving that slower hardware zero extra + # time to appear before this boot gives up on the pending node. + current = wait_for_settle(current) + candidates = unmatched_candidates(configured, current, plan) + plan.update(compute_bootstrap_plan(configured, current, plan)) + + # attribute any candidate that landed on a pending node's name back to + # that node - so its rescan hint lands there and vyos-interface- + # rescan.py can write the real hw-id into the node's existing settings + # (address, description, ...) - and warn about any pending node still + # without a hw-id after this pass (a candidate may have existed and + # landed on a lower-numbered unrelated slot instead - this is not + # necessarily a hardware shortage). + reclaimed = {} + for mac, name in candidates: + final_name = plan.get(name, name) + if final_name in all_pending: + reclaimed[mac] = final_name + logger.info( + f"reclaiming pending node '{final_name}' for hw-id '{mac}' " + 'this boot' + ) + for name in sorted(all_pending - set(reclaimed.values())): + logger.warning( + f"pending node '{name}' still has no hw-id after this boot's " + 'naming pass' + ) + + applied = safe_bulk_rename(plan) + + final_current = discover_physical_interfaces() + # configured is passed unmutated (not merged with `reclaimed`): the + # reclaimed mac still has no real hw-id in config.boot yet, so it must + # still get a rescan hint under its (now reclaimed) name, letting + # vyos-interface-rescan.py write the hw-id into the existing node. + sync_rescan_hints(final_current, configured, set(applied.keys())) + write_status(configured, current, missing, applied, + pending=pending, reclaimed=reclaimed, candidates=candidates) + + +if __name__ == '__main__': + syslog_handler = logging.handlers.SysLogHandler(address='/dev/log') + formatter = logging.Formatter(f'{Path(__file__).name}: %(message)s') + syslog_handler.setFormatter(formatter) + + logger.addHandler(syslog_handler) + logger.setLevel(logging.DEBUG) + + main() diff --git a/src/systemd/vyos-net-name-resolve.service b/src/systemd/vyos-net-name-resolve.service new file mode 100644 index 00000000000..ec7c456364d --- /dev/null +++ b/src/systemd/vyos-net-name-resolve.service @@ -0,0 +1,17 @@ +[Unit] +Description=VyOS deterministic hw-id based interface naming +Documentation=man:vyos-net-name-resolve.py(8) + +# Started explicitly by vyos-router (like frr.service/vyos-netlinkd.service) +# once /opt/vyatta/etc/config is mounted and config.boot exists - which, for +# an encrypted config volume, only happens after vyos-router has unlocked +# and mounted it. It cannot be ordered independently before vyos-router: +# there is no config.boot to read before that point. +DefaultDependencies=no + +[Service] +Type=oneshot +RemainAfterExit=yes +ExecStart=/usr/libexec/vyos/system/vyos-net-name-resolve.py +TimeoutStartSec=90 +StandardOutput=journal+console diff --git a/src/tests/helper.py b/src/tests/helper.py index 2c4421d0a0d..5a74525b794 100644 --- a/src/tests/helper.py +++ b/src/tests/helper.py @@ -14,9 +14,20 @@ import sys import importlib.util +from importlib.machinery import SourceFileLoader def prepare_module(file_path='', module_name=''): + """Load file_path as module_name, returning the loaded module. + + file_path may be an extensionless script (e.g. udev helpers under + src/udev/) - spec_from_file_location() cannot infer a loader for those + from the suffix alone, so fall back to an explicit SourceFileLoader. + """ spec = importlib.util.spec_from_file_location(module_name, file_path) + if spec is None: + loader = SourceFileLoader(module_name, file_path) + spec = importlib.util.spec_from_loader(module_name, loader) module = importlib.util.module_from_spec(spec) spec.loader.exec_module(module) sys.modules[module_name] = module + return module diff --git a/src/tests/test_net_name_resolve.py b/src/tests/test_net_name_resolve.py new file mode 100644 index 00000000000..cc401ddb39e --- /dev/null +++ b/src/tests/test_net_name_resolve.py @@ -0,0 +1,1534 @@ +# Copyright 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 . +# +# Coverage for T3871 (boot race on hw-id based interface naming): the +# MAC-driven, collision-safe rename plan that replaces the old per-uevent +# udev decision, the find_available() crash it used to hit, the +# ethtool/sysfs permanent-MAC fallback, and the config.boot-availability +# ordering corner case (vyos-net-name-resolve must run from inside +# vyos-router, after the config directory is mounted/decrypted, not as an +# independently-scheduled early-boot unit). + +import json +import os +import shutil +import tempfile +import unittest +from unittest import mock + +from helper import prepare_module + +_here = os.path.dirname(__file__) + +resolver = prepare_module( + os.path.join(_here, '../system/vyos-net-name-resolve.py'), + 'vyos_net_name_resolve') +vyos_net_name = prepare_module( + os.path.join(_here, '../udev/vyos_net_name'), + 'vyos_net_name') + + +class TestGetPendingHwidNodes(unittest.TestCase): + """A node that exists under interfaces/{ethernet,wireless} but has no + hw-id (e.g. after `delete interfaces ethernet eth1 hw-id`, the + documented "NIC replaced" remediation) must be surfaced separately + from the MAC-keyed configured dict, since it has no MAC to key by yet + - this is what lets main() try to reclaim it for the exact NIC that + vacated it, instead of silently orphaning its other settings. + """ + + def setUp(self): + self.tmp = tempfile.mkdtemp() + self.addCleanup(shutil.rmtree, self.tmp, ignore_errors=True) + self.config_path = os.path.join(self.tmp, 'config.boot') + self._orig_config_path = resolver.config_path + resolver.config_path = self.config_path + self.addCleanup(setattr, resolver, 'config_path', self._orig_config_path) + + def _write_config(self, text): + with open(self.config_path, 'w') as f: + f.write(text) + + def test_node_with_hwid_not_pending(self): + self._write_config( + 'interfaces {\n' + ' ethernet eth0 {\n' + ' hw-id 00:11:22:33:44:55\n' + ' }\n' + '}\n' + ) + self.assertEqual(resolver.get_pending_hwid_nodes(), + {'ethernet': set(), 'wireless': set()}) + + def test_node_without_hwid_is_pending(self): + self._write_config( + 'interfaces {\n' + ' ethernet eth1 {\n' + ' address 10.99.1.1/24\n' + ' }\n' + '}\n' + ) + self.assertEqual(resolver.get_pending_hwid_nodes(), + {'ethernet': {'eth1'}, 'wireless': set()}) + + def test_mixed_ethernet_and_wireless(self): + self._write_config( + 'interfaces {\n' + ' ethernet eth0 {\n' + ' hw-id 00:11:22:33:44:55\n' + ' }\n' + ' ethernet eth1 {\n' + ' address 10.99.1.1/24\n' + ' }\n' + ' wireless wlan0 {\n' + ' ssid example\n' + ' }\n' + '}\n' + ) + self.assertEqual(resolver.get_pending_hwid_nodes(), + {'ethernet': {'eth1'}, 'wireless': {'wlan0'}}) + + def test_no_interfaces_section(self): + self._write_config('system {\n host-name vyos\n}\n') + self.assertEqual(resolver.get_pending_hwid_nodes(), + {'ethernet': set(), 'wireless': set()}) + + def test_no_config_boot_at_all(self): + # livecd/ISO case - config_path doesn't exist + self.assertEqual(resolver.get_pending_hwid_nodes(), + {'ethernet': set(), 'wireless': set()}) + + +class TestFindAvailableCrashFix(unittest.TestCase): + """find_available() used to raise IndexError on an empty candidate set + (index_list[0]) and ValueError on a name with no trailing digit + (int('')) - both are realistic inputs on a box's first boot, or when a + biosdevname/predefined name has no numeric suffix. + """ + + def test_resolver_empty_set(self): + self.assertEqual(resolver.find_available(set(), 'eth'), 'eth0') + + def test_resolver_no_prefix_match(self): + self.assertEqual(resolver.find_available({'wlan3'}, 'eth'), 'eth0') + + def test_resolver_contiguous(self): + names = {'eth0', 'eth1', 'eth2'} + self.assertEqual(resolver.find_available(names, 'eth'), 'eth3') + + def test_resolver_fills_hole(self): + self.assertEqual(resolver.find_available({'eth2', 'eth5'}, 'eth'), 'eth3') + + def test_resolver_no_hole_appends(self): + self.assertEqual(resolver.find_available({'eth2', 'eth3'}, 'eth'), 'eth4') + + def test_vyos_net_name_empty_dict(self): + self.assertEqual(vyos_net_name.find_available({}, 'eth'), 'eth0') + + def test_vyos_net_name_no_digit_suffix(self): + self.assertEqual( + vyos_net_name.find_available({'m': 'wlan3'}, 'eth'), 'eth0') + + def test_vyos_net_name_contiguous(self): + intfs = {'a': 'eth0', 'b': 'eth1'} + self.assertEqual(vyos_net_name.find_available(intfs, 'eth'), 'eth2') + + def test_vyos_net_name_is_available(self): + self.assertTrue(vyos_net_name.is_available({'a': 'eth0'}, 'eth1')) + self.assertFalse(vyos_net_name.is_available({'a': 'eth0'}, 'eth0')) + + def test_vyos_net_name_mod_ifname(self): + self.assertEqual(vyos_net_name.mod_ifname('e5'), 'eth3') + self.assertEqual(vyos_net_name.mod_ifname('e2'), 'eth0') + self.assertEqual(vyos_net_name.mod_ifname('e1'), 'eth1') + self.assertEqual(vyos_net_name.mod_ifname('wlan0'), 'wlan0') + + +class TestUnmatchedCandidates(unittest.TestCase): + """Field report: an unconfigured NIC that happens to be squatting on a + DIFFERENT interface's configured hw-id slot this boot (e.g. due to + probe-order scrambling) was excluded here just because + compute_rename_plan() already scheduled it for eviction. That skipped + it past match_pending_nodes() entirely, so a squatter that was really + the exact NIC a pending node should reclaim - or one that made a + reclaim genuinely ambiguous - fell through to ordinary bootstrap + naming and got a permanent hw-id with no ambiguity check at all. + Squatters must stay in this pool; only their eviction destination + (still computed by compute_rename_plan()) is a fallback, not a reason + to hide them from reclaim/ambiguity handling. + """ + + def test_squatter_is_still_a_candidate(self): + configured = {'m0': 'eth1'} + current = {'eth1': 'squatter-mac', 'eth9': 'm0'} + existing_plan = {'eth1': 'eth3', 'eth9': 'eth1'} + candidates = resolver.unmatched_candidates(configured, current, existing_plan) + self.assertEqual(candidates, [('squatter-mac', 'eth1')]) + + def test_rightful_owner_still_excluded(self): + configured = {'m0': 'eth0'} + current = {'eth0': 'm0'} + candidates = resolver.unmatched_candidates(configured, current, {}) + self.assertEqual(candidates, []) + + +class TestComputeRenamePlan(unittest.TestCase): + """The authoritative rename plan is what fixes the multi-vendor NIC + boot race: it is driven purely by MAC address, independent of + ifindex/probe order, and must never produce a colliding assignment. + """ + + def test_noop_when_already_correct(self): + configured = {'m0': 'eth0', 'm1': 'eth1'} + current = {'eth0': 'm0', 'eth1': 'm1'} + self.assertEqual(resolver.compute_rename_plan(configured, current), {}) + + def test_rename_independent_of_probe_order(self): + # m0/m1 landed on unrelated names this boot (e.g. a different + # vendor driver won the probe race) - the plan must still point + # them at their configured targets regardless. + configured = {'m0': 'eth0', 'm1': 'eth1'} + current = {'eth5': 'm0', 'eth3': 'm1'} + plan = resolver.compute_rename_plan(configured, current) + self.assertEqual(plan, {'eth5': 'eth0', 'eth3': 'eth1'}) + + def test_full_swap_cycle(self): + # eth0 and eth1 need to trade names - a naive from->to rename + # would collide; the two-phase scratch-name staging must not. + configured = {'m0': 'eth1', 'm1': 'eth0'} + current = {'eth0': 'm0', 'eth1': 'm1'} + plan = resolver.compute_rename_plan(configured, current) + self.assertEqual(plan, {'eth0': 'eth1', 'eth1': 'eth0'}) + + def test_squatter_relocated(self): + # an unconfigured interface currently sits on a name a configured + # hw-id needs - it must be moved out of the way, not left there. + configured = {'m0': 'eth0'} + current = {'eth0': 'unconfigured-mac', 'eth5': 'm0'} + plan = resolver.compute_rename_plan(configured, current) + self.assertEqual(plan['eth5'], 'eth0') + self.assertNotEqual(plan['eth0'], 'eth0') + self.assertTrue(plan['eth0'].startswith('eth')) + + def test_squatter_never_relocated_onto_a_missing_hwid_target(self): + # macMissing's hardware hasn't shown up this boot (removed/faulty/ + # slow driver), so its reserved target 'eth1' never appears in + # `current` and the main hw-id loop skips it entirely (source is + # None) - the squatter being relocated off of 'eth0' must still not + # be handed 'eth1', or it would need relocating again the moment + # that hardware actually appears. + configured = {'macA': 'eth0', 'macMissing': 'eth1'} + current = {'eth0': 'squatterMac', 'eth2': 'macA'} + plan = resolver.compute_rename_plan(configured, current) + self.assertEqual(plan.get('eth2'), 'eth0') + self.assertNotEqual(plan.get('eth0'), 'eth1') + + def test_squatter_never_relocated_onto_a_pending_hwid_less_node(self): + # 'eth1' has no hw-id at all (e.g. `delete interfaces ethernet + # eth1 hw-id`, the documented remediation for a replaced NIC) but + # its config node - and settings like address - still exist. A + # squatter being relocated off of 'eth0' must not be handed 'eth1' + # either, or it would permanently steal the exact slot the + # original NIC needs to reclaim. + configured = {'macA': 'eth0'} + pending = {'ethernet': {'eth1'}, 'wireless': set()} + current = {'eth0': 'squatterMac', 'eth2': 'macA'} + plan = resolver.compute_rename_plan(configured, current, pending) + self.assertEqual(plan.get('eth2'), 'eth0') + self.assertNotEqual(plan.get('eth0'), 'eth1') + + def test_missing_hardware_no_crash_no_entry(self): + # hw-id configured but the NIC never showed up (missing/faulty) - + # must be skipped, not crash or incorrectly assign another interface. + configured = {'m0': 'eth0', 'missing-mac': 'eth9'} + current = {'eth5': 'm0'} + plan = resolver.compute_rename_plan(configured, current) + self.assertEqual(plan, {'eth5': 'eth0'}) + + def test_rename_introducing_a_gap(self): + # user edits the CLI so m2's node is renamed eth2 -> eth10 while + # keeping the same hw-id, leaving eth5..eth9 unused. + configured = { + 'm0': 'eth0', 'm1': 'eth1', 'm2': 'eth10', + 'm3': 'eth3', 'm4': 'eth4', + } + current = { + 'eth0': 'm0', 'eth1': 'm1', 'eth2': 'm2', + 'eth3': 'm3', 'eth4': 'm4', + } + plan = resolver.compute_rename_plan(configured, current) + self.assertEqual(plan, {'eth2': 'eth10'}) + + def test_rename_introducing_a_gap_with_scrambled_fastpath(self): + # same as above, but this boot's cosmetic udev fast-path lands m2 + # on an unrelated name and coincidentally puts a different NIC on + # the now-meaningless old 'eth2'. + configured = { + 'm0': 'eth0', 'm1': 'eth1', 'm2': 'eth10', + 'm3': 'eth3', 'm4': 'eth4', + } + current = { + 'eth0': 'm0', 'eth1': 'm1', 'eth7': 'm2', + 'eth2': 'm3', 'eth4': 'm4', + } + plan = resolver.compute_rename_plan(configured, current) + self.assertEqual(plan.get('eth7'), 'eth10') + self.assertEqual(plan.get('eth2'), 'eth3') + self.assertNotIn('eth2', configured.values()) + + +class TestWaitForSettle(unittest.TestCase): + """There is no specific MAC to wait for when bootstrap-naming hardware + without a configured hw-id, so settling is detected as N consecutive + identical discover_physical_interfaces() snapshots instead. + """ + + def test_returns_after_n_consecutive_stable_polls(self): + snapshots = [ + {'eth0': 'm0'}, # still changing + {'eth0': 'm0', 'eth1': 'm1'}, # still changing + {'eth0': 'm0', 'eth1': 'm1'}, # 1st stable repeat + {'eth0': 'm0', 'eth1': 'm1'}, # 2nd stable repeat -> settled + ] + with mock.patch.object(resolver, 'discover_physical_interfaces', + side_effect=snapshots): + result = resolver.wait_for_settle( + {}, timeout=5, poll=0, stable_polls=3) + self.assertEqual(result, {'eth0': 'm0', 'eth1': 'm1'}) + + def test_bounded_by_timeout_when_never_stable(self): + counter = iter(range(1000)) + + def ever_changing(*_a, **_kw): + return {'eth0': f'm{next(counter)}'} + + clock = iter([0, 0.1, 20, 20]) + with mock.patch.object(resolver, 'discover_physical_interfaces', + side_effect=ever_changing), \ + mock.patch('time.monotonic', side_effect=lambda: next(clock)): + result = resolver.wait_for_settle( + {}, timeout=10, poll=0, stable_polls=3) + # must return SOMETHING (the last snapshot seen), not hang or crash + self.assertIn('eth0', result) + + def test_stable_polls_of_one_returns_on_initial_snapshot(self): + with mock.patch.object(resolver, 'discover_physical_interfaces') as m: + result = resolver.wait_for_settle( + {'eth0': 'm0'}, timeout=5, poll=0, stable_polls=1) + m.assert_not_called() + self.assertEqual(result, {'eth0': 'm0'}) + + +class TestIsWirelessInterface(unittest.TestCase): + """Wireless detection must be race-free - it has to be correct even + when the interface's current name is a leftover from the cosmetic + udev-time fast path and doesn't look like a wlan name at all. + """ + + def setUp(self): + self.tmp = tempfile.mkdtemp() + self.addCleanup(shutil.rmtree, self.tmp, ignore_errors=True) + # phy80211 is always a symlink to a real sysfs node in practice - + # exists() follows symlinks, so the test target must actually exist + self.phy_target = os.path.join(self.tmp, 'phy0') + os.mkdir(self.phy_target) + + def test_phy80211_present_is_wireless(self): + path = os.path.join(self.tmp, 'wlan0') + os.mkdir(path) + os.symlink(self.phy_target, os.path.join(path, 'phy80211')) + self.assertTrue(resolver.is_wireless_interface('wlan0', self.tmp)) + + def test_no_phy80211_is_not_wireless(self): + os.mkdir(os.path.join(self.tmp, 'eth0')) + self.assertFalse(resolver.is_wireless_interface('eth0', self.tmp)) + + def test_detection_independent_of_misleading_name(self): + # named like a leftover cosmetic ethernet guess, but it's a real + # wifi card - must still be classified wireless + path = os.path.join(self.tmp, 'eth7') + os.mkdir(path) + os.symlink(self.phy_target, os.path.join(path, 'phy80211')) + self.assertTrue(resolver.is_wireless_interface('eth7', self.tmp)) + + +class TestPcieDistance(unittest.TestCase): + """PCIe hop-depth is what lets bootstrap naming reflect physical slot + position instead of raw MAC magnitude - a NIC wired straight to the + root complex must not sort after an add-in card just because its MAC + happens to be numerically higher. + """ + + def setUp(self): + self.tmp = tempfile.mkdtemp() + self.addCleanup(shutil.rmtree, self.tmp, ignore_errors=True) + # fake /sys/devices tree, separate from the fake /sys/class/net + # entries, so symlink targets can point at realistic PCI-bus-shaped + # paths the way a real /sys/class/net//device symlink does + self.devices_root = os.path.join(self.tmp, 'devices') + os.makedirs(self.devices_root) + + def _make_iface_pointing_at(self, name, *path_segments): + """Create //device as a symlink to a fake sysfs device + node at devices_root/. + """ + target = os.path.join(self.devices_root, *path_segments) + os.makedirs(target, exist_ok=True) + iface_path = os.path.join(self.tmp, name) + os.mkdir(iface_path) + os.symlink(target, os.path.join(iface_path, 'device')) + + def test_shallow_device_one_pci_segment(self): + # e.g. a NIC directly under the root complex: pci0000:00/0000:00:1f.6 + self._make_iface_pointing_at('eth0', 'pci0000:00', '0000:00:1f.6') + self.assertEqual(resolver.pcie_distance('eth0', self.tmp), 1) + + def test_deep_device_behind_bridges(self): + # e.g. an add-in card behind two cascaded PCIe bridges + self._make_iface_pointing_at( + 'eth1', 'pci0000:00', '0000:00:1c.0', + '0000:01:00.0', '0000:02:04.0') + self.assertEqual(resolver.pcie_distance('eth1', self.tmp), 3) + + def test_virtio_indirection_not_miscounted(self): + # verified-real virtio_net layout: device -> virtioN node, whose + # OWN parent is the true PCI BDF - the virtioN segment itself must + # be skipped, not counted as a hop. + self._make_iface_pointing_at( + 'eth2', 'pci0000:00', '0000:00:12.0', 'virtio2', 'net') + self.assertEqual(resolver.pcie_distance('eth2', self.tmp), 1) + + def test_missing_device_symlink_returns_sentinel(self): + os.mkdir(os.path.join(self.tmp, 'eth3')) # no 'device' entry at all + self.assertEqual(resolver.pcie_distance('eth3', self.tmp), + resolver.PCIE_DISTANCE_UNKNOWN) + + def test_broken_device_symlink_returns_sentinel(self): + iface_path = os.path.join(self.tmp, 'eth4') + os.mkdir(iface_path) + os.symlink(os.path.join(self.devices_root, 'does-not-exist'), + os.path.join(iface_path, 'device')) + self.assertEqual(resolver.pcie_distance('eth4', self.tmp), + resolver.PCIE_DISTANCE_UNKNOWN) + + def test_no_pci_segment_at_all_returns_sentinel(self): + # simulated USB NIC: resolved path has no PCI BDF component + self._make_iface_pointing_at('eth5', 'usb1', '1-1', '1-1:1.0') + self.assertEqual(resolver.pcie_distance('eth5', self.tmp), + resolver.PCIE_DISTANCE_UNKNOWN) + + +class TestComputeBootstrapPlan(unittest.TestCase): + """Bootstrap naming is what makes a box's very first boot (before any + hw-id exists) deterministic, the same way hw-id makes every boot after + that deterministic - it must depend only on PCIe topology and MAC + rank, never on whatever name the racy cosmetic fast-path happened to + assign. pcie_distance is held constant by default in these tests, so + they exercise pure MAC-rank ordering (see TestPcieDistance for the + topology-ordering cases specifically). + """ + + def setUp(self): + patcher = mock.patch.object(resolver, 'is_wireless_interface', + return_value=False) + self.is_wireless = patcher.start() + self.addCleanup(patcher.stop) + + distance_patcher = mock.patch.object(resolver, 'pcie_distance', + return_value=0) + self.pcie_distance = distance_patcher.start() + self.addCleanup(distance_patcher.stop) + + def test_sorted_by_mac_independent_of_current_names(self): + # names are in the OPPOSITE order of their MACs + current = {'eth5': 'bb', 'eth2': 'aa'} + plan = resolver.compute_bootstrap_plan({}, current, {}) + self.assertEqual(plan, {'eth2': 'eth0', 'eth5': 'eth1'}) + + def test_noop_when_already_in_canonical_rank_order(self): + current = {'eth0': 'aa', 'eth1': 'bb'} + plan = resolver.compute_bootstrap_plan({}, current, {}) + self.assertEqual(plan, {}) + + def test_configured_targets_are_never_overwritten(self): + configured = {'m0': 'eth0'} + current = {'eth0': 'm0', 'eth9': 'm2', 'eth8': 'm1'} + plan = resolver.compute_bootstrap_plan(configured, current, {}) + self.assertNotIn('eth0', plan.values()) + self.assertEqual(plan.get('eth8'), 'eth1') + self.assertEqual(plan.get('eth9'), 'eth2') + + def test_new_interface_backfills_a_gap_with_no_pending_node(self): + # eth1 is a gap between two hw-id'd interfaces, but nothing in + # config.boot reserves it (no hw-id, no pending node either - the + # interface was fully deleted at some point, or never existed) - + # a new, unrelated NIC discovered this boot is free to take it, + # the same as it would on a system with no config for it at all. + configured = {'m0': 'eth0', 'm2': 'eth2'} + current = {'eth0': 'm0', 'eth2': 'm2', 'eth9': 'new-mac'} + plan = resolver.compute_bootstrap_plan(configured, current, {}) + self.assertEqual(plan, {'eth9': 'eth1'}) + + def test_multiple_new_interfaces_backfill_gaps_in_order(self): + configured = {'m0': 'eth0', 'm2': 'eth2'} + current = { + 'eth0': 'm0', 'eth2': 'm2', + 'ethB': 'bb', 'ethA': 'aa', + } + plan = resolver.compute_bootstrap_plan(configured, current, {}) + # lower mac 'aa' takes the gap at eth1 first, 'bb' continues to + # the next free slot, eth3 - not "skip past the gap to eth3/eth4" + self.assertEqual(plan, {'ethA': 'eth1', 'ethB': 'eth3'}) + + def test_ethernet_and_wireless_numbered_independently(self): + self.is_wireless.side_effect = lambda name: name == 'radio0' + current = {'ifaceB': 'bb', 'radio0': 'aa'} + plan = resolver.compute_bootstrap_plan({}, current, {}) + self.assertEqual(plan.get('radio0'), 'wlan0') + self.assertEqual(plan.get('ifaceB'), 'eth0') + + def test_squatter_already_in_existing_plan_excluded(self): + # 'eth0' is a squatter the hw-id plan already decided to move away + # (existing_plan key) - it IS still bootstrap candidacy (see + # unmatched_candidates()), but must never be handed a second, + # conflicting assignment on top of its existing_plan eviction. + existing_plan = {'eth0': 'eth3'} + current = {'eth0': 'unconfigured-mac', 'eth7': 'm0'} + plan = resolver.compute_bootstrap_plan({'m0': 'eth1'}, current, + existing_plan) + self.assertNotIn('eth0', plan) + + def test_bootstrap_targets_never_collide_with_existing_plan_values(self): + # existing_plan says an interface currently named 'oldname' is + # moving to 'eth0' - nothing else in `current` is named 'eth0' or + # 'eth*' at all, so if that target isn't reserved, find_available() + # would freely hand 'eth0' to the bootstrap candidate too. + existing_plan = {'oldname': 'eth0'} + current = {'oldname': 'm0', 'newnic': 'unconfigured-mac'} + plan = resolver.compute_bootstrap_plan({'m0': 'eth0'}, current, + existing_plan) + self.assertNotEqual(plan.get('newnic'), 'eth0') + + def test_smaller_pcie_distance_wins_over_higher_mac(self): + # 'bb' has the numerically higher MAC but sits closer to the root + # complex - it must be named first despite losing on MAC alone. + current = {'eth5': 'bb', 'eth2': 'aa'} + self.pcie_distance.side_effect = lambda name: {'eth5': 0, 'eth2': 3}[name] + plan = resolver.compute_bootstrap_plan({}, current, {}) + self.assertEqual(plan, {'eth5': 'eth0', 'eth2': 'eth1'}) + + def test_same_pcie_distance_falls_back_to_mac_order(self): + current = {'eth5': 'bb', 'eth2': 'aa'} + self.pcie_distance.side_effect = lambda name: 2 # tie for both + plan = resolver.compute_bootstrap_plan({}, current, {}) + self.assertEqual(plan, {'eth2': 'eth0', 'eth5': 'eth1'}) + + def test_unknown_pcie_distance_sorts_last(self): + # 'aa' has the numerically lowest MAC but an undeterminable bus + # position (e.g. a USB NIC) - it must not jump the queue. + current = {'eth9': 'cc', 'eth5': 'bb', 'ethX': 'aa'} + self.pcie_distance.side_effect = lambda name: { + 'eth9': 1, 'eth5': 2, 'ethX': resolver.PCIE_DISTANCE_UNKNOWN, + }[name] + plan = resolver.compute_bootstrap_plan({}, current, {}) + self.assertEqual(plan, {'eth9': 'eth0', 'eth5': 'eth1', 'ethX': 'eth2'}) + + def test_pcie_distance_ordering_independent_of_ethernet_wireless_split(self): + # distance-based ordering applies within each type group separately, + # same as MAC does today - a wlan candidate's distance must not + # affect eth numbering or vice versa. + self.is_wireless.side_effect = lambda name: name == 'radio0' + current = {'ifaceB': 'bb', 'radio0': 'aa'} + self.pcie_distance.side_effect = lambda name: { + 'ifaceB': 5, 'radio0': 0, + }[name] + plan = resolver.compute_bootstrap_plan({}, current, {}) + self.assertEqual(plan.get('radio0'), 'wlan0') + self.assertEqual(plan.get('ifaceB'), 'eth0') + + +class TestComputeBootstrapPlanPendingSlots(unittest.TestCase): + """A pending node's name (see get_pending_hwid_nodes()) carries no + special reservation in compute_bootstrap_plan() at all - it is just + another name with nothing configured for it, exactly like a numeric + gap. main() is what attributes a candidate landing on such a name + back to a "reclaim" afterward, by cross-checking it against + get_pending_hwid_nodes() - not this function's job (see main()). + """ + + def setUp(self): + patcher = mock.patch.object(resolver, 'is_wireless_interface', + return_value=False) + self.is_wireless = patcher.start() + self.addCleanup(patcher.stop) + + distance_patcher = mock.patch.object(resolver, 'pcie_distance', + return_value=0) + self.pcie_distance = distance_patcher.start() + self.addCleanup(distance_patcher.stop) + + def test_open_name_filled_like_an_ordinary_gap(self): + # 'eth1' has nothing configured for it - whether that is because + # its whole node was deleted or just its hw-id makes no + # difference here, it is simply the lowest open name. + configured = {'m0': 'eth0'} + current = {'eth0': 'm0', 'eth9': 'new-mac'} + plan = resolver.compute_bootstrap_plan(configured, current, {}) + self.assertEqual(plan.get('eth9'), 'eth1') + + def test_ascending_fill_has_no_preference_between_open_names(self): + # two open names (eth1, eth4 - the reason either is open makes no + # difference) and two candidates - the lower-sorted candidate + # takes the lower-numbered open name, unconditionally. + configured = {'m0': 'eth0', 'm2': 'eth2', 'm3': 'eth3'} + current = {'eth0': 'm0', 'eth2': 'm2', 'eth3': 'm3', + 'eth9': 'bb', 'eth8': 'aa'} + plan = resolver.compute_bootstrap_plan(configured, current, {}) + self.assertEqual(plan.get('eth8'), 'eth1') + self.assertEqual(plan.get('eth9'), 'eth4') + + def test_closed_subset_recovers_original_slot_assignment(self): + # the vyos-build check-qemu-install --ifnametest shape, at the + # function level: two NICs' original slots opened up in the same + # boot (one via full node deletion, one via hw-id-only deletion). + # PCIe distance and MAC are static per-NIC properties, so the two + # freed candidates' relative sort order is identical to their + # original first-boot assignment order - re-running the same + # ascending sort over just the two open names recovers each one's + # own original slot, with no pending-specific logic involved. + configured = { + '00:00:5e:00:53:00': 'eth0', '00:00:5e:00:53:01': 'eth1', + '00:00:5e:00:53:03': 'eth3', '00:00:5e:00:53:05': 'eth5', + '00:00:5e:00:53:06': 'eth6', '00:00:5e:00:53:07': 'eth7', + } + current = {name: mac for mac, name in configured.items()} + current.update({'eth9': '00:00:5e:00:53:02', + 'eth8': '00:00:5e:00:53:04'}) + plan = resolver.compute_bootstrap_plan(configured, current, {}) + self.assertEqual(plan.get('eth9'), 'eth2') + self.assertEqual(plan.get('eth8'), 'eth4') + + def test_open_names_currently_squatted_by_a_rightful_mover_are_not_taken(self): + # real boot reproduction (via the resolver's own status file, + # captured mid-failure): the cosmetic fast-path this boot had + # every configured mac sitting on some OTHER configured mac's + # target name (a full rotation), including the two open names + # (eth1 - pending, eth6 - fully deleted) both currently squatted + # by macs that are about to move elsewhere via a rightful-owner + # move. existing_plan's SOURCE names looked "taken" at the exact + # moment this function ran, but safe_bulk_rename()'s two-phase + # staging vacates every source before any target is claimed - so + # they must not count as taken, or the two real candidates get + # needlessly pushed to fresh eth8/eth9-style slots instead of + # their own open names. + configured = { + '00:00:5e:00:53:00': 'eth0', '00:00:5e:00:53:02': 'eth2', + '00:00:5e:00:53:03': 'eth3', '00:00:5e:00:53:04': 'eth4', + '00:00:5e:00:53:05': 'eth5', '00:00:5e:00:53:07': 'eth7', + } + current = { + 'eth4': '00:00:5e:00:53:00', 'eth6': '00:00:5e:00:53:02', + 'eth7': '00:00:5e:00:53:03', 'eth2': '00:00:5e:00:53:04', + 'eth3': '00:00:5e:00:53:05', 'eth1': '00:00:5e:00:53:07', + 'eth0': '00:00:5e:00:53:06', # unconfigured - squats on eth0 + 'eth5': '00:00:5e:00:53:01', # unconfigured - squats on eth5 + } + existing_plan = resolver.compute_rename_plan(configured, current, + {'ethernet': {'eth1'}, + 'wireless': set()}) + plan = resolver.compute_bootstrap_plan(configured, current, + existing_plan) + self.assertEqual(plan.get('eth5'), 'eth1') + self.assertEqual(plan.get('eth0'), 'eth6') + + +class TestSafeBulkRename(unittest.TestCase): + """The two-phase rename must accurately report what actually happened - + a phase-2 (scratch -> final target) failure must not be reported as a + successful rename, and must not leave the interface down indefinitely + under a name nothing else knows about. + """ + + def setUp(self): + patcher = mock.patch.object(resolver, 'get_ifindex', + side_effect=lambda name: name) + patcher.start() + self.addCleanup(patcher.stop) + + def test_all_renames_succeed(self): + with mock.patch.object(resolver, 'run', return_value=0): + applied = resolver.safe_bulk_rename({'eth5': 'eth0'}) + self.assertEqual(applied, {'eth5': 'eth0'}) + + def test_phase_two_failure_is_not_reported_as_applied(self): + # 'ip link set dev vyetheth5 name eth0' fails (exit 1); everything + # else (down, the phase-1 rename to scratch) succeeds + def fake_run(command, *_a, **_kw): + return 1 if command == 'ip link set dev vyetheth5 name eth0' else 0 + + with mock.patch.object(resolver, 'run', side_effect=fake_run): + applied = resolver.safe_bulk_rename({'eth5': 'eth0'}) + + self.assertNotIn('eth5', applied) + self.assertNotEqual(applied.get('eth5'), 'eth0') + + def test_phase_two_failure_brings_scratch_name_back_up(self): + calls = [] + + def fake_run(command, *_a, **_kw): + calls.append(command) + return 1 if command == 'ip link set dev vyetheth5 name eth0' else 0 + + with mock.patch.object(resolver, 'run', side_effect=fake_run): + resolver.safe_bulk_rename({'eth5': 'eth0'}) + + self.assertIn('ip link set dev vyetheth5 up', calls) + self.assertNotIn('ip link set dev eth0 up', calls) + + def test_one_failure_does_not_affect_other_renames_in_the_batch(self): + def fake_run(command, *_a, **_kw): + return 1 if command == 'ip link set dev vyetheth5 name eth0' else 0 + + with mock.patch.object(resolver, 'run', side_effect=fake_run): + applied = resolver.safe_bulk_rename({'eth5': 'eth0', 'eth9': 'eth1'}) + + self.assertNotIn('eth5', applied) + self.assertEqual(applied.get('eth9'), 'eth1') + + +class TestGetPermanentMac(unittest.TestCase): + """Part of the original race is reading a MAC before the driver has + programmed the permanent address. Verify the ethtool-first, + sysfs-fallback behaviour, including drivers that report an all-zero + 'unsupported' permanent address instead of failing outright. + """ + + def test_ethtool_permanent_address(self): + with mock.patch.object( + resolver, 'rc_cmd', + return_value=(0, 'Permanent address: aa:bb:cc:dd:ee:ff\n')): + self.assertEqual(resolver.get_permanent_mac('eth0'), + 'aa:bb:cc:dd:ee:ff') + + def test_ethtool_zero_address_falls_back_to_sysfs(self): + with mock.patch.object( + resolver, 'rc_cmd', + return_value=(0, 'Permanent address: 00:00:00:00:00:00\n')), \ + mock.patch('pathlib.Path.read_text', + return_value='11:22:33:44:55:66\n'): + self.assertEqual(resolver.get_permanent_mac('eth0'), + '11:22:33:44:55:66') + + def test_ethtool_unsupported_falls_back_to_sysfs(self): + with mock.patch.object( + resolver, 'rc_cmd', + return_value=(1, 'Operation not supported')), \ + mock.patch('pathlib.Path.read_text', + return_value='11:22:33:44:55:66\n'): + self.assertEqual(resolver.get_permanent_mac('eth0'), + '11:22:33:44:55:66') + + def test_no_mac_available_returns_empty(self): + with mock.patch.object(resolver, 'rc_cmd', return_value=(1, '')), \ + mock.patch('pathlib.Path.read_text', side_effect=OSError): + self.assertEqual(resolver.get_permanent_mac('eth0'), '') + + +class TestDiscoverPhysicalInterfaces(unittest.TestCase): + """Only interfaces with a real bus 'device' link are candidates - + bridges/bonds/VLANs/veth must be excluded regardless of name, since + this replaces the racy udev DRIVERS=="?*" check with a stable, + post-settle sysfs read. + """ + + def setUp(self): + self.tmp = tempfile.mkdtemp() + self.addCleanup(shutil.rmtree, self.tmp, ignore_errors=True) + + def make_iface(name, mac=None, has_device=True): + path = os.path.join(self.tmp, name) + os.mkdir(path) + if has_device: + os.mkdir(os.path.join(path, 'device')) + if mac: + with open(os.path.join(path, 'address'), 'w') as f: + f.write(mac + '\n') + + make_iface('eth0', mac='aa:aa:aa:aa:aa:00') + make_iface('eth1', mac='aa:aa:aa:aa:aa:01') + make_iface('br0', mac='aa:aa:aa:aa:aa:99', has_device=False) + make_iface('lo', has_device=False) + + def test_only_physical_interfaces_with_mac(self): + with mock.patch.object(resolver, 'rc_cmd', return_value=(1, '')): + found = resolver.discover_physical_interfaces(self.tmp) + self.assertEqual(found, { + 'eth0': 'aa:aa:aa:aa:aa:00', + 'eth1': 'aa:aa:aa:aa:aa:01', + }) + self.assertNotIn('br0', found) + self.assertNotIn('lo', found) + + +class TestSyncRescanHints(unittest.TestCase): + """New/unconfigured hardware must still be surfaced to + vyos-interface-rescan.py, already-configured interfaces must not, and + hints left under a pre-rename name must not linger once the resolver + has moved that interface to its hw-id target. + """ + + def setUp(self): + self.tmp = tempfile.mkdtemp() + self.addCleanup(shutil.rmtree, self.tmp, ignore_errors=True) + self._orig_dir = resolver.vyos_udev_dir + resolver.vyos_udev_dir = self.tmp + self.addCleanup(setattr, resolver, 'vyos_udev_dir', self._orig_dir) + + def test_writes_hint_for_unconfigured_interface(self): + resolver.sync_rescan_hints({'eth5': 'new-mac'}, configured={}) + hint = os.path.join(self.tmp, 'eth5') + self.assertTrue(os.path.isfile(hint)) + with open(hint) as f: + self.assertEqual(f.read(), 'new-mac') + + def test_no_hint_for_configured_interface(self): + resolver.sync_rescan_hints({'eth0': 'm0'}, configured={'m0': 'eth0'}) + self.assertFalse(os.path.exists(os.path.join(self.tmp, 'eth0'))) + + def test_stale_hint_removed_after_rename(self): + # udev's cosmetic fast-path wrote a hint under the pre-rename name; + # the resolver then moved that interface to its hw-id target. + stale = os.path.join(self.tmp, 'eth7') + with open(stale, 'w') as f: + f.write('m2') + resolver.sync_rescan_hints({'eth10': 'm2'}, configured={'m2': 'eth10'}, + stale_names={'eth7'}) + self.assertFalse(os.path.exists(stale)) + + + +class TestMainFirstBootBootstrap(unittest.TestCase): + """Direct regression guard for the gap this feature closes: main() + used to leave a fully-unconfigured system's interfaces at whatever + racy cosmetic name they already had, with no settle wait at all, and + freeze that into the rescan hints vyos-interface-rescan.py persists. + """ + + def setUp(self): + self.udev_dir = tempfile.mkdtemp() + self.addCleanup(shutil.rmtree, self.udev_dir, ignore_errors=True) + self._orig_udev_dir = resolver.vyos_udev_dir + resolver.vyos_udev_dir = self.udev_dir + self.addCleanup(setattr, resolver, 'vyos_udev_dir', self._orig_udev_dir) + + status_dir = tempfile.mkdtemp() + self.addCleanup(shutil.rmtree, status_dir, ignore_errors=True) + self._orig_status_file = resolver.status_file + resolver.status_file = resolver.Path(status_dir) / 'status.json' + self.addCleanup(setattr, resolver, 'status_file', self._orig_status_file) + + def test_empty_configfile_bootstrap_renames_and_hints_new_names(self): + # a tiny fake kernel: name -> mac, mutated by simulated 'ip link' + # calls so discover_physical_interfaces() reflects renames for real + state = {'eth5': 'bb', 'eth2': 'aa'} # cosmetic fast-path leftover + + def fake_discover(*_a, **_kw): + return dict(state) + + def fake_run(command, *_a, **_kw): + parts = command.split() + if 'name' in parts: + old = parts[parts.index('dev') + 1] + new = parts[parts.index('name') + 1] + if old in state: + state[new] = state.pop(old) + return 0 + + with mock.patch.object(resolver, 'get_configfile_interfaces', + return_value={}), \ + mock.patch.object(resolver, 'get_pending_hwid_nodes', + return_value={'ethernet': set(), 'wireless': set()}), \ + mock.patch.object(resolver, 'discover_physical_interfaces', + side_effect=fake_discover), \ + mock.patch.object(resolver, 'is_wireless_interface', + return_value=False), \ + mock.patch.object(resolver, 'run', side_effect=fake_run), \ + mock.patch('time.sleep'): + resolver.main() + + hints = set(os.listdir(self.udev_dir)) + # renamed into MAC-rank order: lower mac 'aa' -> eth0, higher 'bb' -> eth1 + self.assertEqual(hints, {'eth0', 'eth1'}) + with open(os.path.join(self.udev_dir, 'eth0')) as f: + self.assertEqual(f.read(), 'aa') + with open(os.path.join(self.udev_dir, 'eth1')) as f: + self.assertEqual(f.read(), 'bb') + + def test_pending_node_still_settles_when_its_hardware_is_slow_to_appear(self): + # field report: a pending node ended up permanently bound to a + # fresh eth8/eth9-style name despite its own hardware genuinely + # being present - because that hardware (a slower driver, exactly + # the multi-vendor-NIC race this whole feature addresses) simply + # had not shown up in the FIRST, unsettled discover_physical_ + # interfaces() snapshot wait_for_hardware() returns as soon as all + # already-CONFIGURED macs are found. Since that snapshot showed + # nothing unconfigured yet, main()'s gate into wait_for_settle() + # never even ran, giving the slow hardware zero extra time to + # appear before the boot gave up. A pending node in play must + # force wait_for_settle() to run regardless of what the early + # snapshot shows, so slower hardware still gets its chance. + configured = {'m0': 'eth0'} + pending = {'ethernet': {'eth1'}, 'wireless': set()} + + calls = {'n': 0} + + def fake_discover(*_a, **_kw): + calls['n'] += 1 + if calls['n'] <= 2: + return {'eth0': 'm0'} # m1's hardware not visible yet + return {'eth0': 'm0', 'eth9': 'm1'} # now it has appeared + + state = {'eth0': 'm0', 'eth9': 'm1'} + + def fake_run(command, *_a, **_kw): + parts = command.split() + if 'name' in parts: + old = parts[parts.index('dev') + 1] + new = parts[parts.index('name') + 1] + if old in state: + state[new] = state.pop(old) + return 0 + + with mock.patch.object(resolver, 'get_configfile_interfaces', + return_value=configured), \ + mock.patch.object(resolver, 'get_pending_hwid_nodes', + return_value=pending), \ + mock.patch.object(resolver, 'discover_physical_interfaces', + side_effect=fake_discover), \ + mock.patch.object(resolver, 'is_wireless_interface', + return_value=False), \ + mock.patch.object(resolver, 'pcie_distance', return_value=0), \ + mock.patch.object(resolver, 'run', side_effect=fake_run), \ + mock.patch('time.sleep'): + resolver.main() + + self.assertEqual(state.get('eth1'), 'm1') + + status = json.loads(resolver.status_file.read_text()) + self.assertEqual(status['reclaimed'], {'m1': 'eth1'}) + self.assertEqual(status['pending_unresolved'], []) + + def test_reported_reproduction_end_to_end(self): + # exact reported scenario: eth0/eth2 keep their hw-id, eth1's hw-id + # was deleted (the documented "NIC replaced" remediation) but its + # node - and an address setting - stay in config.boot. The same + # physical NIC (mac 'm1') must come back as 'eth1', not bootstrap + # past the floor to 'eth3', and must still get a rescan hint under + # 'eth1' so vyos-interface-rescan.py can refill its hw-id. + configured = {'m0': 'eth0', 'm2': 'eth2'} + pending = {'ethernet': {'eth1'}, 'wireless': set()} + # cosmetic fast-path already placed everything at its final + # hw-id'd name except the replaced NIC, which landed on a racy name + state = {'eth0': 'm0', 'eth2': 'm2', 'eth9': 'm1'} + + def fake_discover(*_a, **_kw): + return dict(state) + + def fake_run(command, *_a, **_kw): + parts = command.split() + if 'name' in parts: + old = parts[parts.index('dev') + 1] + new = parts[parts.index('name') + 1] + if old in state: + state[new] = state.pop(old) + return 0 + + with mock.patch.object(resolver, 'get_configfile_interfaces', + return_value=configured), \ + mock.patch.object(resolver, 'get_pending_hwid_nodes', + return_value=pending), \ + mock.patch.object(resolver, 'discover_physical_interfaces', + side_effect=fake_discover), \ + mock.patch.object(resolver, 'is_wireless_interface', + return_value=False), \ + mock.patch.object(resolver, 'run', side_effect=fake_run), \ + mock.patch('time.sleep'): + resolver.main() + + # the NIC is back at 'eth1', not bootstrapped past the floor to 'eth3' + self.assertEqual(state.get('eth1'), 'm1') + self.assertNotIn('eth3', state) + + # still gets a rescan hint under its reclaimed name, so + # vyos-interface-rescan.py can write the real hw-id into the + # existing 'eth1' node (preserving its other settings, e.g. address) + hints = set(os.listdir(self.udev_dir)) + self.assertEqual(hints, {'eth1'}) + with open(os.path.join(self.udev_dir, 'eth1')) as f: + self.assertEqual(f.read(), 'm1') + + status = json.loads(resolver.status_file.read_text()) + self.assertEqual(status['reclaimed'], {'m1': 'eth1'}) + self.assertEqual(status['pending_unresolved'], []) + + def test_fully_deleted_interfaces_backfill_their_gaps_end_to_end(self): + # reported scenario: eth3, eth6 and eth7 were fully deleted (not + # just their hw-id - the whole node, so get_pending_hwid_nodes() + # has nothing for them either) while eth0,1,2,4,5,8,9 stay + # configured. The three now-unconfigured NICs must backfill the + # gaps at 3/6/7 in MAC order, not bootstrap past the highest + # configured index (9) to 10/11/12. + configured = { + 'm0': 'eth0', 'm1': 'eth1', 'm2': 'eth2', 'm4': 'eth4', + 'm5': 'eth5', 'm8': 'eth8', 'm9': 'eth9', + } + pending = {'ethernet': set(), 'wireless': set()} + state = {name: mac for mac, name in configured.items()} + # realistic macs, deliberately NOT alphabetically matching their + # old slot, so the test can't pass by coincidence + state.update({'ethX': 'ff:ff:ff:ff:ff:07', + 'ethY': 'aa:aa:aa:aa:aa:03', + 'ethZ': 'cc:cc:cc:cc:cc:06'}) + + def fake_discover(*_a, **_kw): + return dict(state) + + def fake_run(command, *_a, **_kw): + parts = command.split() + if 'name' in parts: + old = parts[parts.index('dev') + 1] + new = parts[parts.index('name') + 1] + if old in state: + state[new] = state.pop(old) + return 0 + + with mock.patch.object(resolver, 'get_configfile_interfaces', + return_value=configured), \ + mock.patch.object(resolver, 'get_pending_hwid_nodes', + return_value=pending), \ + mock.patch.object(resolver, 'discover_physical_interfaces', + side_effect=fake_discover), \ + mock.patch.object(resolver, 'is_wireless_interface', + return_value=False), \ + mock.patch.object(resolver, 'pcie_distance', return_value=0), \ + mock.patch.object(resolver, 'run', side_effect=fake_run), \ + mock.patch('time.sleep'): + resolver.main() + + # the three gaps got backfilled in ascending MAC order, not pushed + # past the highest configured index to eth10/eth11/eth12 + self.assertEqual(state.get('eth3'), 'aa:aa:aa:aa:aa:03') + self.assertEqual(state.get('eth6'), 'cc:cc:cc:cc:cc:06') + self.assertEqual(state.get('eth7'), 'ff:ff:ff:ff:ff:07') + self.assertNotIn('eth10', state) + self.assertNotIn('eth11', state) + self.assertNotIn('eth12', state) + + def test_stray_leftover_interface_still_fills_both_open_names(self): + # field report: deleting only eth7's hw-id and rebooting produced + # "could not be safely auto-matched" instead of a clean reclaim, + # because a stray interface left over from an earlier, unrelated + # boot (e.g. a scratch vyethN name stuck after a failed rename) + # was also present, making this a 1 pending/2 candidate case. Both + # names now reliably get a real hw-id either way - the lower- + # sorted candidate takes the lower-numbered open name (the + # ordinary gap at eth3), the higher-sorted one takes the pending + # node - nothing is left unresolved, and there is no way to know + # (or need to know) which candidate was "really" eth7's own past + # hardware once its hw-id is gone. + configured = { + 'm0': 'eth0', 'm1': 'eth1', 'm2': 'eth2', 'm4': 'eth4', + 'm5': 'eth5', 'm6': 'eth6', + } + pending = {'ethernet': {'eth7'}, 'wireless': set()} + state = {name: mac for mac, name in configured.items()} + state.update({'eth9': 'aa:bb:cc:dd:ee:07', # lower-sorted candidate + 'vyeth13': 'ff:ff:ff:ff:ff:99'}) # higher-sorted candidate + + def fake_discover(*_a, **_kw): + return dict(state) + + def fake_run(command, *_a, **_kw): + parts = command.split() + if 'name' in parts: + old = parts[parts.index('dev') + 1] + new = parts[parts.index('name') + 1] + if old in state: + state[new] = state.pop(old) + return 0 + + with mock.patch.object(resolver, 'get_configfile_interfaces', + return_value=configured), \ + mock.patch.object(resolver, 'get_pending_hwid_nodes', + return_value=pending), \ + mock.patch.object(resolver, 'discover_physical_interfaces', + side_effect=fake_discover), \ + mock.patch.object(resolver, 'is_wireless_interface', + return_value=False), \ + mock.patch.object(resolver, 'pcie_distance', return_value=0), \ + mock.patch.object(resolver, 'run', side_effect=fake_run), \ + mock.patch('time.sleep'): + resolver.main() + + # the lower-sorted candidate fills the ordinary gap (eth3), the + # higher-sorted one fills the pending node (eth7) - both real, + # both deterministic, nothing left unresolved + self.assertEqual(state.get('eth3'), 'aa:bb:cc:dd:ee:07') + self.assertEqual(state.get('eth7'), 'ff:ff:ff:ff:ff:99') + + hints = set(os.listdir(self.udev_dir)) + self.assertEqual(hints, {'eth3', 'eth7'}) + + status = json.loads(resolver.status_file.read_text()) + self.assertEqual(status['pending_unresolved'], []) + self.assertEqual(status['reclaimed'], {'ff:ff:ff:ff:ff:99': 'eth7'}) + + def test_second_pending_node_fills_lowest_numbered_first_when_hardware_is_short(self): + # two pending nodes (eth1 and eth7), but only one candidate showed + # up this boot - a hardware shortage, not ambiguity. The lower- + # numbered node fills; the higher-numbered one stays genuinely + # unresolved and reported, since there simply isn't enough + # hardware to satisfy both. + configured = {'m0': 'eth0'} + pending = {'ethernet': {'eth1', 'eth7'}, 'wireless': set()} + state = {'eth0': 'm0', 'eth9': 'aa:bb:cc:dd:ee:07'} + + def fake_discover(*_a, **_kw): + return dict(state) + + def fake_run(command, *_a, **_kw): + parts = command.split() + if 'name' in parts: + old = parts[parts.index('dev') + 1] + new = parts[parts.index('name') + 1] + if old in state: + state[new] = state.pop(old) + return 0 + + with mock.patch.object(resolver, 'get_configfile_interfaces', + return_value=configured), \ + mock.patch.object(resolver, 'get_pending_hwid_nodes', + return_value=pending), \ + mock.patch.object(resolver, 'discover_physical_interfaces', + side_effect=fake_discover), \ + mock.patch.object(resolver, 'is_wireless_interface', + return_value=False), \ + mock.patch.object(resolver, 'pcie_distance', return_value=0), \ + mock.patch.object(resolver, 'run', side_effect=fake_run), \ + mock.patch('time.sleep'): + resolver.main() + + self.assertEqual(state.get('eth1'), 'aa:bb:cc:dd:ee:07') + self.assertNotIn('eth7', state) + + status = json.loads(resolver.status_file.read_text()) + self.assertEqual(status['pending_unresolved'], ['eth7']) + self.assertEqual(status['reclaimed'], {'aa:bb:cc:dd:ee:07': 'eth1'}) + + def test_two_candidates_for_one_pending_node_fill_deterministically(self): + # field report: deleting eth2's hw-id (leaving its node in place) + # produced "0 unconfigured candidates" on a later boot, because an + # EARLIER boot with 2 candidates present had left the pending node + # unresolved and its rightful hardware un-hinted, orphaning it. + # Per the deterministic-fill policy, one pending node with two + # candidates now reliably resolves: with only 'eth0' configured, + # 'eth1' is also a genuinely open name here, so the lower-sorted + # candidate fills it first and the higher-sorted one fills the + # pending node ('eth2') - nothing is left unresolved or un-hinted + # either way. + configured = {'m0': 'eth0'} + pending = {'ethernet': {'eth2'}, 'wireless': set()} + state = { + 'eth0': 'm0', + 'racyA': 'aa:bb:cc:dd:ee:02', # lower-sorted candidate + 'racyB': 'ff:ff:ff:ff:ff:99', # higher-sorted candidate + } + + def fake_discover(*_a, **_kw): + return dict(state) + + def fake_run(command, *_a, **_kw): + parts = command.split() + if 'name' in parts: + old = parts[parts.index('dev') + 1] + new = parts[parts.index('name') + 1] + if old in state: + state[new] = state.pop(old) + return 0 + + with mock.patch.object(resolver, 'get_configfile_interfaces', + return_value=configured), \ + mock.patch.object(resolver, 'get_pending_hwid_nodes', + return_value=pending), \ + mock.patch.object(resolver, 'discover_physical_interfaces', + side_effect=fake_discover), \ + mock.patch.object(resolver, 'is_wireless_interface', + return_value=False), \ + mock.patch.object(resolver, 'pcie_distance', return_value=0), \ + mock.patch.object(resolver, 'run', side_effect=fake_run), \ + mock.patch('time.sleep'): + resolver.main() + + # the lower-sorted candidate (racyA) fills the lower open name + # (eth1); the higher-sorted one (racyB) reclaims the pending node + self.assertEqual(state.get('eth1'), 'aa:bb:cc:dd:ee:02') + self.assertEqual(state.get('eth2'), 'ff:ff:ff:ff:ff:99') + + hints = set(os.listdir(self.udev_dir)) + self.assertEqual(hints, {'eth1', 'eth2'}) + + status = json.loads(resolver.status_file.read_text()) + self.assertEqual(status['pending_unresolved'], []) + self.assertEqual(status['reclaimed'], {'ff:ff:ff:ff:ff:99': 'eth2'}) + + def test_two_pending_nodes_two_candidates_both_fill_deterministically(self): + # two NICs' hw-id were deleted before the same reboot (eth0 and + # eth2, nodes left in place), and both came back racily named. + # Sorted pending names (eth0 < eth2) pair with sorted candidates + # (by MAC here, since pcie_distance ties) - both fill in one boot, + # with no unresolved node left behind. + configured = { + 'm1': 'eth1', 'm3': 'eth3', 'm4': 'eth4', + 'm5': 'eth5', 'm6': 'eth6', 'm7': 'eth7', + } + pending = {'ethernet': {'eth0', 'eth2'}, 'wireless': set()} + state = {name: mac for mac, name in configured.items()} + state.update({ + 'racyA': 'aa:bb:cc:dd:ee:00', + 'racyB': 'aa:bb:cc:dd:ee:02', + }) + + def fake_discover(*_a, **_kw): + return dict(state) + + def fake_run(command, *_a, **_kw): + parts = command.split() + if 'name' in parts: + old = parts[parts.index('dev') + 1] + new = parts[parts.index('name') + 1] + if old in state: + state[new] = state.pop(old) + return 0 + + with mock.patch.object(resolver, 'get_configfile_interfaces', + return_value=configured), \ + mock.patch.object(resolver, 'get_pending_hwid_nodes', + return_value=pending), \ + mock.patch.object(resolver, 'discover_physical_interfaces', + side_effect=fake_discover), \ + mock.patch.object(resolver, 'is_wireless_interface', + return_value=False), \ + mock.patch.object(resolver, 'pcie_distance', return_value=0), \ + mock.patch.object(resolver, 'run', side_effect=fake_run), \ + mock.patch('time.sleep'): + resolver.main() + + self.assertEqual(state.get('eth0'), 'aa:bb:cc:dd:ee:00') + self.assertEqual(state.get('eth2'), 'aa:bb:cc:dd:ee:02') + + hints = set(os.listdir(self.udev_dir)) + self.assertEqual(hints, {'eth0', 'eth2'}) + + status = json.loads(resolver.status_file.read_text()) + self.assertEqual(status['pending_unresolved'], []) + self.assertEqual(status['reclaimed'], { + 'aa:bb:cc:dd:ee:00': 'eth0', + 'aa:bb:cc:dd:ee:02': 'eth2', + }) + + def test_squatting_pending_candidate_still_reclaims_correctly(self): + # field report: deleting eth7's hw-id (node left in place) produced + # a "0 unconfigured candidates" boot even though eth7's real + # hardware was present, and it later turned up permanently bound + # to a fresh eth8/eth9-style name. Root cause: eth7's hardware + # (mac07) happened to be squatting on eth2's configured hw-id slot + # this boot (probe-order scrambling) - compute_rename_plan() + # already scheduled it for eviction, which used to make + # unmatched_candidates() skip it entirely, so it never reached the + # ascending fill and fell through to a stale, unrelated fallback + # destination instead. A squatting candidate must still reclaim + # its pending node exactly like a non-squatting one would, when it + # is the only open name and the only candidate this boot. + configured = { + 'm0': 'eth0', 'm1': 'eth1', 'm2': 'eth2', 'm3': 'eth3', + 'm4': 'eth4', 'm5': 'eth5', 'm6': 'eth6', + } + pending = {'ethernet': {'eth7'}, 'wireless': set()} + state = {name: mac for mac, name in configured.items()} + state.update({ + 'eth2': 'aa:bb:cc:dd:ee:07', # eth7's hardware squats on eth2 + 'eth9': 'm2', # m2's rightful NIC, racy this boot + }) + + def fake_discover(*_a, **_kw): + return dict(state) + + def fake_run(command, *_a, **_kw): + parts = command.split() + if 'name' in parts: + old = parts[parts.index('dev') + 1] + new = parts[parts.index('name') + 1] + if old in state: + state[new] = state.pop(old) + return 0 + + with mock.patch.object(resolver, 'get_configfile_interfaces', + return_value=configured), \ + mock.patch.object(resolver, 'get_pending_hwid_nodes', + return_value=pending), \ + mock.patch.object(resolver, 'discover_physical_interfaces', + side_effect=fake_discover), \ + mock.patch.object(resolver, 'is_wireless_interface', + return_value=False), \ + mock.patch.object(resolver, 'pcie_distance', return_value=0), \ + mock.patch.object(resolver, 'run', side_effect=fake_run), \ + mock.patch('time.sleep'): + resolver.main() + + # m2 still lands on its own configured slot, and the squatter + # reclaims 'eth7' - not some unrelated fresh bootstrap slot + self.assertEqual(state.get('eth2'), 'm2') + self.assertEqual(state.get('eth7'), 'aa:bb:cc:dd:ee:07') + self.assertNotIn('eth8', state) + self.assertNotIn('eth9', state) + + hints = set(os.listdir(self.udev_dir)) + self.assertEqual(hints, {'eth7'}) + + status = json.loads(resolver.status_file.read_text()) + self.assertEqual(status['reclaimed'], {'aa:bb:cc:dd:ee:07': 'eth7'}) + self.assertEqual(status['pending_unresolved'], []) + + def test_squatting_candidate_and_extra_candidate_fill_ascending(self): + # same squatter-eviction shape as the test above, but with a + # second, unrelated candidate also present this boot, and exactly + # two open names (one ordinary gap, one pending node) for the two + # candidates - a closed system. The lower-sorted candidate (the + # squatter, evicted off of a configured slot) takes the lower- + # numbered open name (the gap); the higher-sorted one takes the + # higher-numbered one (the pending node) - both get a real, + # deterministic hw-id, one of them attributed as a reclaim. + configured = { + 'm0': 'eth0', 'm1': 'eth1', 'm2': 'eth2', + 'm3': 'eth3', 'm4': 'eth4', 'm5': 'eth5', + } + pending = {'ethernet': {'eth7'}, 'wireless': set()} + state = {name: mac for mac, name in configured.items()} + state.update({ + 'eth3': 'aa:bb:cc:dd:ee:07', # squats on m3's configured slot + 'eth9': 'm3', # m3's rightful NIC, racy this boot + 'racyC': 'ff:ff:ff:ff:ff:99', # unrelated leftover candidate + }) + + def fake_discover(*_a, **_kw): + return dict(state) + + def fake_run(command, *_a, **_kw): + parts = command.split() + if 'name' in parts: + old = parts[parts.index('dev') + 1] + new = parts[parts.index('name') + 1] + if old in state: + state[new] = state.pop(old) + return 0 + + with mock.patch.object(resolver, 'get_configfile_interfaces', + return_value=configured), \ + mock.patch.object(resolver, 'get_pending_hwid_nodes', + return_value=pending), \ + mock.patch.object(resolver, 'discover_physical_interfaces', + side_effect=fake_discover), \ + mock.patch.object(resolver, 'is_wireless_interface', + return_value=False), \ + mock.patch.object(resolver, 'pcie_distance', return_value=0), \ + mock.patch.object(resolver, 'run', side_effect=fake_run), \ + mock.patch('time.sleep'): + resolver.main() + + # m3 still lands on its own configured slot; the squatter takes + # the lower-numbered open gap, the stray takes the pending node + self.assertEqual(state.get('eth3'), 'm3') + self.assertEqual(state.get('eth6'), 'aa:bb:cc:dd:ee:07') + self.assertEqual(state.get('eth7'), 'ff:ff:ff:ff:ff:99') + + hints = set(os.listdir(self.udev_dir)) + self.assertEqual(hints, {'eth6', 'eth7'}) + + status = json.loads(resolver.status_file.read_text()) + self.assertEqual(status['pending_unresolved'], []) + self.assertEqual(status['reclaimed'], {'ff:ff:ff:ff:ff:99': 'eth7'}) + + def test_gap_backfill_and_pending_reclaim_coexist_in_the_same_boot(self): + # the exact vyos-build check-qemu-install --ifnametest shape: one + # interface's whole config node is fully deleted (a free numeric + # gap) while a DIFFERENT interface's hw-id alone is deleted (a + # pending node) in the very same reboot + # (del_idx, hwid_idx = random.sample(range(8), 2)). Both must + # resolve independently in one boot: the pending node (eth2) + # reliably gets a real hw-id, and the fully-deleted gap (eth5) + # backfills with whatever hardware is left - no manual step + # needed for either. + configured = { + 'm0': 'eth0', 'm1': 'eth1', 'm3': 'eth3', 'm4': 'eth4', + 'm6': 'eth6', 'm8': 'eth8', 'm9': 'eth9', + } + pending = {'ethernet': {'eth2'}, 'wireless': set()} + state = {name: mac for mac, name in configured.items()} + state.update({ + 'eth7': 'aa:bb:cc:dd:ee:02', # eth2's real hardware, racy this boot + 'eth5': 'ff:ff:ff:ff:ff:05', # eth5 fully deleted, its own hardware freed + }) + + def fake_discover(*_a, **_kw): + return dict(state) + + def fake_run(command, *_a, **_kw): + parts = command.split() + if 'name' in parts: + old = parts[parts.index('dev') + 1] + new = parts[parts.index('name') + 1] + if old in state: + state[new] = state.pop(old) + return 0 + + with mock.patch.object(resolver, 'get_configfile_interfaces', + return_value=configured), \ + mock.patch.object(resolver, 'get_pending_hwid_nodes', + return_value=pending), \ + mock.patch.object(resolver, 'discover_physical_interfaces', + side_effect=fake_discover), \ + mock.patch.object(resolver, 'is_wireless_interface', + return_value=False), \ + mock.patch.object(resolver, 'pcie_distance', return_value=0), \ + mock.patch.object(resolver, 'run', side_effect=fake_run), \ + mock.patch('time.sleep'): + resolver.main() + + self.assertEqual(state.get('eth2'), 'aa:bb:cc:dd:ee:02') + self.assertEqual(state.get('eth5'), 'ff:ff:ff:ff:ff:05') + + status = json.loads(resolver.status_file.read_text()) + self.assertEqual(status['pending_unresolved'], []) + self.assertEqual(status['reclaimed'], {'aa:bb:cc:dd:ee:02': 'eth2'}) + + +class TestWriteStatus(unittest.TestCase): + """A pending node that couldn't be safely auto-matched must be + reported so vyos-router can surface a boot-time warning - it must + never regress into another silent, buried-syslog-only failure like + the one this whole fix closes. + """ + + def setUp(self): + status_dir = tempfile.mkdtemp() + self.addCleanup(shutil.rmtree, status_dir, ignore_errors=True) + self._orig_status_file = resolver.status_file + resolver.status_file = resolver.Path(status_dir) / 'status.json' + self.addCleanup(setattr, resolver, 'status_file', self._orig_status_file) + + def _read_status(self): + return json.loads(resolver.status_file.read_text()) + + def test_pending_unresolved_reported_when_no_match(self): + pending = {'ethernet': {'eth1', 'eth4'}, 'wireless': set()} + resolver.write_status({}, {}, set(), {}, pending=pending, reclaimed={}) + status = self._read_status() + self.assertEqual(status['pending_unresolved'], ['eth1', 'eth4']) + + def test_reclaimed_not_reported_as_unresolved(self): + pending = {'ethernet': {'eth1'}, 'wireless': set()} + reclaimed = {'m1': 'eth1'} + resolver.write_status({}, {}, set(), {}, pending=pending, + reclaimed=reclaimed) + status = self._read_status() + self.assertEqual(status['pending_unresolved'], []) + self.assertEqual(status['reclaimed'], {'m1': 'eth1'}) + + def test_no_pending_no_crash(self): + resolver.write_status({}, {}, set(), {}) + status = self._read_status() + self.assertEqual(status['pending_unresolved'], []) + self.assertEqual(status['reclaimed'], {}) + + def test_unconfigured_candidates_reported_for_diagnosis(self): + # the full unmatched_candidates() list from main(), so an + # ambiguous reclaim (zero or multiple candidates for a pending + # node) is diagnosable from this one file alone - which physical + # interfaces were actually competing - without needing a separate + # `ip -br link show` or `show configuration commands` to + # reconstruct the same picture by hand. + pending = {'ethernet': {'eth7'}, 'wireless': set()} + candidates = [('aa:bb:cc:dd:ee:07', 'eth9'), ('aa:bb:cc:dd:ee:99', 'vyeth13')] + resolver.write_status({}, {}, set(), {}, pending=pending, + reclaimed={}, candidates=candidates) + status = self._read_status() + self.assertEqual(status['pending_unresolved'], ['eth7']) + self.assertEqual(status['unconfigured_candidates'], { + 'eth9': 'aa:bb:cc:dd:ee:07', + 'vyeth13': 'aa:bb:cc:dd:ee:99', + }) + + def test_unconfigured_candidates_defaults_empty(self): + resolver.write_status({}, {}, set(), {}) + status = self._read_status() + self.assertEqual(status['unconfigured_candidates'], {}) + + +class TestBootOrdering(unittest.TestCase): + """Regression guard for the config.boot availability corner case: + /opt/vyatta/etc/config/config.boot does not exist until vyos-router + mounts (and, for an encrypted config volume, decrypts) the config + directory itself. vyos-net-name-resolve must be started explicitly + from inside vyos-router after that point, and must not be + reintroduced as an independently-scheduled unit ordered before + vyos-router/network-pre.target/cloud-init. + """ + + def setUp(self): + path = os.path.join(_here, '../init/vyos-router') + with open(path) as f: + self.lines = f.readlines() + + def _first_index(self, needle): + for i, line in enumerate(self.lines): + if needle in line: + return i + self.fail(f"'{needle}' not found in src/init/vyos-router") + + def test_resolver_runs_after_migrate_and_before_config_apply(self): + # search for call SITES, not the function definitions further up + migrate = self._first_index('migrate_bootfile || overall_status=1') + resolve = self._first_index( + 'systemctl start vyos-net-name-resolve.service') + update_iface = self._first_index('update_interface_config || overall_status=1') + load_boot = self._first_index('disabled configure || load_bootfile') + + self.assertLess(migrate, resolve, + 'resolver must run after config.boot is migrated/current') + self.assertLess(resolve, update_iface, + 'resolver must run before new-hardware rescan consumes its hints') + self.assertLess(resolve, load_boot, + 'resolver must run before the CLI config is applied') + + def test_service_unit_not_independently_ordered_before_vyos_router(self): + path = os.path.join(_here, '../systemd/vyos-net-name-resolve.service') + with open(path) as f: + unit = f.read() + self.assertNotIn('Before=vyos-router.service', unit) + self.assertNotIn('Before=network-pre.target', unit) + self.assertNotIn('WantedBy=', unit) + + +if __name__ == '__main__': + unittest.main() diff --git a/src/udev/vyos_net_name b/src/udev/vyos_net_name index f5c3b9f1b91..388cc78be5c 100755 --- a/src/udev/vyos_net_name +++ b/src/udev/vyos_net_name @@ -19,20 +19,28 @@ import re import time import logging import logging.handlers -import tempfile from pathlib import Path from sys import argv -from vyos.configtree import ConfigTree from vyos.defaults import directories from vyos.utils.process import cmdl from vyos.utils.boot import boot_configuration_complete from vyos.utils.locking import Lock -from vyos.migrate import ConfigMigrate # Define variables vyos_udev_dir = directories['vyos_udev_dir'] -config_path = '/opt/vyatta/etc/config/config.boot' + +# NOTE: this script only picks a provisional name for a physical interface +# as it is discovered by udev - it is a best-effort placeholder and does +# NOT look at 'hw-id' in config.boot. The authoritative, hw-id based name +# (if one is configured) is applied afterwards, once, by +# vyos-net-name-resolve.py (src/system/vyos-net-name-resolve.py), which +# runs after all hardware has settled. Deciding hw-id names here, from a +# single per-uevent sysfs snapshot, is what caused interface names to be +# unreliable on systems with multiple NIC vendors/drivers: the DRIVERS +# attribute and the interface's permanent MAC address are not guaranteed +# to be populated yet at the moment a single "add" uevent is evaluated, +# and there is no second chance to re-evaluate it. def is_available(intfs: dict, intf_name: str) -> bool: @@ -44,16 +52,24 @@ def is_available(intfs: dict, intf_name: str) -> bool: def find_available(intfs: dict, prefix: str) -> str: """Find lowest indexed interface name that is not assigned""" - index_list = [ - int(x.replace(prefix, '')) for x in list(intfs.values()) if prefix in x - ] + index_list = [] + for x in intfs.values(): + if not x.startswith(prefix): + continue + suffix = x[len(prefix):] + if suffix.isdigit(): + index_list.append(int(suffix)) + + if not index_list: + return f'{prefix}0' + index_list.sort() # find 'holes' in list, if any missing = sorted(set(range(index_list[0], index_list[-1])) - set(index_list)) if missing: return f'{prefix}{missing[0]}' - return f'{prefix}{len(index_list)}' + return f'{prefix}{index_list[-1] + 1}' def mod_ifname(ifname: str) -> str: @@ -89,7 +105,10 @@ def get_biosdevname(ifname: str) -> str: try: biosname = cmdl(['/sbin/biosdevname', '--policy', 'all_ethN', '-i', ifname]) except Exception as e: - logger.error(f'biosdevname error: {e}') + # expected/harmless on most virtualized and some bare-metal + # systems - see the XXX above; the caller already falls back to + # mod_ifname()'s guess, so this isn't worth more than a debug note + logger.debug(f'biosdevname unavailable for {ifname}: {e}') biosname = '' return intf if biosname == '' else biosname @@ -117,81 +136,6 @@ def leave_rescan_hint(intf_name: str, hwid: str): logger.critical(f'OSError {e}') -def get_configfile_interfaces() -> dict: - """Read existing interfaces from config file""" - interfaces: dict = {} - - if not os.path.isfile(config_path): - # If the case, then we are running off of livecd; return empty - return interfaces - - try: - with open(config_path) as f: - config_file = f.read() - except OSError as e: - logger.critical(f'OSError {e}') - exit(1) - - try: - config = ConfigTree(config_file) - except Exception: - try: - logger.debug('updating component version string syntax') - # this will update the component version string syntax, - # required for updates 1.2 --> 1.3/1.4 - with tempfile.NamedTemporaryFile() as fp: - with open(fp.name, 'w') as fd: - fd.write(config_file) - config_migrate = ConfigMigrate(fp.name) - if config_migrate.syntax_update_needed(): - config_migrate.update_syntax() - config_migrate.write_config() - with open(fp.name) as fd: - config_file = fd.read() - - config = ConfigTree(config_file) - - except Exception as e: - logger.critical(f'ConfigTree error: {e}') - exit(1) - - base = ['interfaces', 'ethernet'] - if config.exists(base): - eth_intfs = config.list_nodes(base) - for intf in eth_intfs: - path = base + [intf, 'hw-id'] - if not config.exists(path): - logger.warning(f"no 'hw-id' entry for {intf}") - continue - hwid = config.return_value(path) - if hwid in list(interfaces): - logger.warning( - f'multiple entries for {hwid}: {interfaces[hwid]}, {intf}' - ) - continue - interfaces[hwid] = intf - - base = ['interfaces', 'wireless'] - if config.exists(base): - wlan_intfs = config.list_nodes(base) - for intf in wlan_intfs: - path = base + [intf, 'hw-id'] - if not config.exists(path): - logger.warning(f"no 'hw-id' entry for {intf}") - continue - hwid = config.return_value(path) - if hwid in list(interfaces): - logger.warning( - f'multiple entries for {hwid}: {interfaces[hwid]}, {intf}' - ) - continue - interfaces[hwid] = intf - - logger.debug(f'config file entries: {interfaces}') - - return interfaces - - def add_assigned_interfaces(intfs: dict): """Add interfaces found by previous invocation of udev rule""" if not os.path.isdir(vyos_udev_dir): @@ -209,30 +153,25 @@ def add_assigned_interfaces(intfs: dict): def on_boot_event(intf_name: str, hwid: str, predefined: str = '') -> str: - """Called on boot by vyos-router: 'coldplug' in vyatta_net_name""" - logger.info(f'lookup {intf_name}, {hwid}') - interfaces = get_configfile_interfaces() - logger.debug(f'config file interfaces are {interfaces}') - - if hwid in list(interfaces): - logger.info(f"use mapping from config file: '{hwid}' -> '{interfaces[hwid]}'") - return interfaces[hwid] + """Called on boot by vyos-router: 'coldplug' in vyatta_net_name + Picks a provisional name only - see the module docstring. hw-id from + config.boot is deliberately NOT consulted here. + """ + interfaces: dict = {} add_assigned_interfaces(interfaces) - logger.debug(f'adding assigned interfaces: {interfaces}') + logger.debug(f'{intf_name} ({hwid}): already assigned this boot: {interfaces}') if predefined: newname = predefined - logger.info(f"predefined interface name for '{intf_name}' is '{newname}'") else: newname = get_biosdevname(intf_name) - logger.info(f"biosdevname returned '{newname}' for '{intf_name}'") if not is_available(interfaces, newname): prefix = re.sub(r'\d+$', '', newname) newname = find_available(interfaces, prefix) - logger.info(f"new name for '{intf_name}' is '{newname}'") + logger.info(f"{intf_name} ({hwid}) -> '{newname}'") leave_rescan_hint(newname, hwid) @@ -252,9 +191,9 @@ if __name__ == '__main__': logger = logging.getLogger() logger.addHandler(syslog_handler) - logger.setLevel(logging.DEBUG) + logger.setLevel(logging.INFO) - logger.debug(f'Started with arguments: {argv}') + logger.debug(f'started with arguments: {argv}') if len(argv) > 3: predef_name = argv[3] @@ -266,11 +205,6 @@ if __name__ == '__main__': lock.acquire(60) if not boot_configuration_complete(): - res = on_boot_event(argv[1], argv[2], predefined=predef_name) - logger.debug(f'on boot, returned name is {res}') - print(res) - else: - logger.debug('boot configuration complete') + print(on_boot_event(argv[1], argv[2], predefined=predef_name)) lock.release() - logger.debug('Finished')