summaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
authorChristian Breunig <christian@breunig.cc>2026-08-13 06:11:11 +0000
committerChristian Breunig <christian@breunig.cc>2026-08-15 18:10:24 +0200
commit4b92c2fe6f9928a1ff126c92e89e993de17fb0f5 (patch)
tree3b96e0d04f927f870651c89405f09acdd38b4b16 /src
parent32f3ce2d951e30335dc6216e3759d7c1fcef4832 (diff)
downloadvyos-1x-4b92c2fe6f9928a1ff126c92e89e993de17fb0f5.tar.gz
vyos-1x-4b92c2fe6f9928a1ff126c92e89e993de17fb0f5.zip
T3871: never guess a pending node's hardware when ambiguous
Reported against the previous PCIe/MAC-sorted replacement fill: on a real, already-provisioned box (hw-id from historical probe-order rescan, unrelated to any PCIe/MAC sort), a configured node's address ended up silently applied to a different physical NIC. Deleting one interface's hw-id (keeping its settings) while a different, unrelated interface's config was fully removed in the same boot was enough - the two freed candidates' MAC order didn't match their old name order, so the deterministic sort swapped them. Once written back by the rescan helper, the wrong binding became permanent and self-reinforcing on every later boot. There's no way to verify, from MAC and PCIe position alone, which of several unconfigured candidates is genuinely a given node's own hardware once its hw-id is gone. Restore strict matching: a node only recovers its hw-id automatically when it's the sole pending node of its type this boot and exactly one candidate exists - any other count leaves it pending and reported rather than guessed. A candidate that isn't matched this way is not otherwise held back - it still gets an ordinary, settings-free bootstrap name instead of being lost, just never inherits another node's configuration. This necessarily changes what a combined "delete one interface fully, clear a different one's hw-id" reboot can auto-resolve; the accompanying test-harness change acknowledges that trade-off explicitly.
Diffstat (limited to 'src')
-rwxr-xr-xsrc/system/vyos-net-name-resolve.py171
-rw-r--r--src/tests/test_net_name_resolve.py308
2 files changed, 311 insertions, 168 deletions
diff --git a/src/system/vyos-net-name-resolve.py b/src/system/vyos-net-name-resolve.py
index 7934e8442..ca8ce02ea 100755
--- a/src/system/vyos-net-name-resolve.py
+++ b/src/system/vyos-net-name-resolve.py
@@ -365,7 +365,60 @@ def unmatched_candidates(configured: dict, current: dict, existing_plan: dict) -
if mac not in configured]
-def compute_bootstrap_plan(configured: dict, current: dict, existing_plan: dict) -> dict:
+def match_pending_nodes(pending: dict, candidates: list) -> dict:
+ """Match this boot's unconfigured candidates to pending (hw-id-less)
+ config nodes, one type (ethernet/wireless) at a time. Conservative by
+ design: only matches when there is EXACTLY ONE pending node and
+ EXACTLY ONE candidate of that type this boot - any other cardinality
+ is left alone rather than guessed.
+
+ A pending node still carries its OTHER settings (address,
+ description, ...) - a wrong guess would silently apply one physical
+ port's configuration to a DIFFERENT port. Confirmed in the field: on
+ a real, already-provisioned box (whose hw-id came from historical
+ probe-order rescan, not from any PCIe/MAC sort), a second, completely
+ unrelated candidate freed in the same boot - e.g. a different
+ interface's config being fully deleted - is enough for a naive
+ ascending-sort fill to swap the two, binding a configured node's
+ address to the wrong wire. There is no way to tell, from MAC and
+ PCIe position alone, which candidate is genuinely the pending node's
+ own hardware once its hw-id is gone; leaving the node unresolved and
+ reported is safer than guessing. A candidate that isn't reclaimed
+ here is not otherwise held back - it still proceeds to ordinary
+ bootstrap naming (see compute_bootstrap_plan()) and gets its own
+ fresh, settings-free name instead.
+
+ Returns {mac: node_name} for every unambiguous match this boot.
+ """
+ grouped = {'ethernet': [], 'wireless': []}
+ for mac, name in candidates:
+ group = 'wireless' if is_wireless_interface(name) else 'ethernet'
+ grouped[group].append((mac, name))
+
+ matched = {}
+ for intf_type, nodes in pending.items():
+ if not nodes:
+ continue
+ cands = grouped.get(intf_type, [])
+ if len(nodes) == 1 and len(cands) == 1:
+ (mac, _name) = cands[0]
+ (target,) = nodes
+ matched[mac] = target
+ else:
+ cand_desc = ', '.join(f"'{name}' ({mac})" for mac, name in cands) or 'none'
+ logger.warning(
+ f'{len(nodes)} pending {intf_type} node(s) '
+ f"({', '.join(sorted(nodes))}) and {len(cands)} unconfigured "
+ f'{intf_type} candidate(s) this boot ({cand_desc}) - not '
+ 'unambiguous, leaving pending rather than guessing'
+ )
+
+ return matched
+
+
+def compute_bootstrap_plan(configured: dict, current: dict, existing_plan: dict,
+ pending: dict = None,
+ reclaimed_macs: frozenset = frozenset()) -> 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
@@ -380,23 +433,6 @@ def compute_bootstrap_plan(configured: dict, current: dict, existing_plan: dict)
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
@@ -405,19 +441,29 @@ def compute_bootstrap_plan(configured: dict, current: dict, existing_plan: dict)
(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.
+ needlessly block a lower slot this function might otherwise give it,
+ or another candidate, once real).
+
+ pending node names are reserved the same way a real hw-id target is -
+ a candidate main() could not unambiguously match to one via
+ match_pending_nodes() must never squat there instead, since that name
+ still carries the node's other settings (address, description, ...).
+ reclaimed_macs are candidates main() already matched to a pending
+ node - excluded here so this function never reassigns them elsewhere.
+
+ A numeric slot is only ever off-limits here because something still
+ occupies or reserves it - a real hw-id target or a pending node. A gap
+ left by fully deleting an interface's config (hw-id and node both
+ gone, nothing in `pending` either) carries no such reservation and is
+ freely backfilled, exactly as bootstrap naming would treat it on a
+ system that never had any config for it at all - there is no
+ remaining signal in config.boot to tell those two cases apart, and
+ deleting the whole node is the admin's explicit way of saying so.
"""
plan = {}
candidates = unmatched_candidates(configured, current, existing_plan)
+ candidates = [(mac, name) for mac, name in candidates
+ if mac not in reclaimed_macs]
if not candidates:
return plan
@@ -431,12 +477,12 @@ def compute_bootstrap_plan(configured: dict, current: dict, existing_plan: dict)
# 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.
+ # as taken here either.
+ reserved_pending = set()
+ if pending:
+ reserved_pending = pending.get('ethernet', set()) | pending.get('wireless', set())
taken = (set(current) - candidate_names - set(rightful_movers)) \
- | set(rightful_movers.values())
+ | set(rightful_movers.values()) | reserved_pending
for mac, name in sorted(candidates, key=lambda c: (pcie_distance(c[1]), c[0])):
prefix = 'wlan' if is_wireless_interface(name) else 'eth'
@@ -572,17 +618,9 @@ def main():
all_pending = pending.get('ethernet', set()) | pending.get('wireless', set())
+ reclaimed = {}
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
@@ -592,25 +630,44 @@ def main():
# 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)
+ if configured:
+ # a configured hw-id whose driver was too slow to show up
+ # within wait_for_hardware()'s timeout can still appear during
+ # the extra time wait_for_settle() just spent waiting for
+ # unconfigured hardware to stabilize - recompute against the
+ # settled snapshot so it still gets renamed (and drops out of
+ # `missing`) this boot instead of being silently skipped.
+ missing = set(configured) - set(current.values())
+ plan = compute_rename_plan(configured, current, pending)
+
+ # let a NIC that just lost its hw-id (the documented "delete
+ # hw-id to force regeneration" remediation) reclaim the exact
+ # node its other settings (address, description, ...) still live
+ # under, rather than bootstrap-naming it to a new bare node and
+ # orphaning that config - only when unambiguous, see
+ # match_pending_nodes()
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
+ reclaimed = match_pending_nodes(pending, candidates)
+ candidate_by_mac = dict(candidates)
+ for mac, target in reclaimed.items():
+ name = candidate_by_mac[mac]
+ if name != target:
+ plan[name] = target
logger.info(
- f"reclaiming pending node '{final_name}' for hw-id '{mac}' "
+ f"reclaiming pending node '{target}' for hw-id '{mac}' "
'this boot'
)
+
+ # bootstrap-name whatever is left, 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. Pending node names stay reserved here
+ # (see compute_bootstrap_plan()) so an ambiguous leftover
+ # candidate can never squat on one and inherit its settings.
+ plan.update(compute_bootstrap_plan(
+ configured, current, plan, pending=pending,
+ reclaimed_macs=set(reclaimed)))
+
for name in sorted(all_pending - set(reclaimed.values())):
logger.warning(
f"pending node '{name}' still has no hw-id after this boot's "
diff --git a/src/tests/test_net_name_resolve.py b/src/tests/test_net_name_resolve.py
index cc401ddb3..26d2e174e 100644
--- a/src/tests/test_net_name_resolve.py
+++ b/src/tests/test_net_name_resolve.py
@@ -183,6 +183,63 @@ class TestUnmatchedCandidates(unittest.TestCase):
self.assertEqual(candidates, [])
+class TestMatchPendingNodes(unittest.TestCase):
+ """A pending node's hw-id is unknown by construction - there's no MAC
+ to check a candidate against - so matching must only ever happen in
+ the unambiguous exactly-one-pending/exactly-one-candidate case per
+ type. Confirmed in the field: a naive deterministic fill (sorting
+ pending nodes and candidates together like numeric gaps) silently
+ bound a configured node's address to a different physical NIC than
+ its own, because an already-provisioned box's existing names have no
+ relationship to PCIe/MAC sort order. Guessing in any other
+ cardinality risks the same mistake; leaving the node unresolved and
+ reported is safer.
+ """
+
+ def setUp(self):
+ patcher = mock.patch.object(resolver, 'is_wireless_interface',
+ return_value=False)
+ self.is_wireless = patcher.start()
+ self.addCleanup(patcher.stop)
+
+ def test_unambiguous_single_pending_single_candidate_matches(self):
+ pending = {'ethernet': {'eth1'}, 'wireless': set()}
+ candidates = [('m1', 'eth9')]
+ matched = resolver.match_pending_nodes(pending, candidates)
+ self.assertEqual(matched, {'m1': 'eth1'})
+
+ def test_two_pending_one_candidate_no_match(self):
+ # can't tell which of the two pending nodes this one candidate
+ # belongs to - neither is matched.
+ pending = {'ethernet': {'eth1', 'eth4'}, 'wireless': set()}
+ candidates = [('m1', 'eth9')]
+ matched = resolver.match_pending_nodes(pending, candidates)
+ self.assertEqual(matched, {})
+
+ def test_one_pending_two_candidates_no_match(self):
+ # the field-reported shape: an unrelated second candidate (e.g.
+ # freed by a different interface's config being fully deleted in
+ # the same boot) is enough to make this genuinely ambiguous, even
+ # though it isn't itself pending anything.
+ pending = {'ethernet': {'eth1'}, 'wireless': set()}
+ candidates = [('m1', 'eth9'), ('m2', 'eth8')]
+ matched = resolver.match_pending_nodes(pending, candidates)
+ self.assertEqual(matched, {})
+
+ def test_ethernet_and_wireless_matched_independently(self):
+ self.is_wireless.side_effect = lambda name: name == 'radio0'
+ pending = {'ethernet': {'eth1'}, 'wireless': {'wlan2'}}
+ candidates = [('m1', 'ifaceB'), ('m2', 'radio0')]
+ matched = resolver.match_pending_nodes(pending, candidates)
+ self.assertEqual(matched, {'m1': 'eth1', 'm2': 'wlan2'})
+
+ def test_no_pending_returns_empty(self):
+ pending = {'ethernet': set(), 'wireless': set()}
+ candidates = [('m1', 'eth9')]
+ matched = resolver.match_pending_nodes(pending, candidates)
+ self.assertEqual(matched, {})
+
+
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
@@ -558,13 +615,13 @@ class TestComputeBootstrapPlan(unittest.TestCase):
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()).
+class TestComputeBootstrapPlanGapFill(unittest.TestCase):
+ """compute_bootstrap_plan() fills ordinary numeric gaps (no config
+ trace at all) deterministically by PCIe distance then MAC. A pending
+ node (hw-id deleted, node kept - see get_pending_hwid_nodes()) is a
+ separate concern here: passing `pending` reserves its name so an
+ unrelated candidate can never squat on it and inherit its settings -
+ only match_pending_nodes() may fill it, and only when unambiguous.
"""
def setUp(self):
@@ -579,9 +636,8 @@ class TestComputeBootstrapPlanPendingSlots(unittest.TestCase):
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.
+ # 'eth1' has nothing configured for it and no pending reservation
+ # - it is simply the lowest open name.
configured = {'m0': 'eth0'}
current = {'eth0': 'm0', 'eth9': 'new-mac'}
plan = resolver.compute_bootstrap_plan(configured, current, {})
@@ -598,15 +654,10 @@ class TestComputeBootstrapPlanPendingSlots(unittest.TestCase):
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.
+ def test_two_plain_gaps_fill_in_ascending_order(self):
+ # two fully-deleted interfaces (no hw-id, no pending node either)
+ # - both ordinary gaps, filled by the same ascending PCIe/MAC
+ # sort as any other unconfigured hardware.
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',
@@ -623,15 +674,14 @@ class TestComputeBootstrapPlanPendingSlots(unittest.TestCase):
# 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.
+ # target name (a full rotation), including the two plain gaps
+ # (eth1, eth6) 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',
@@ -644,14 +694,38 @@ class TestComputeBootstrapPlanPendingSlots(unittest.TestCase):
'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()})
+ existing_plan = resolver.compute_rename_plan(configured, current)
plan = resolver.compute_bootstrap_plan(configured, current,
existing_plan)
self.assertEqual(plan.get('eth5'), 'eth1')
self.assertEqual(plan.get('eth0'), 'eth6')
+ def test_pending_name_reserved_against_unrelated_bootstrap_candidate(self):
+ # 'eth1' is pending (no hw-id) - two unrelated new NICs discovered
+ # this boot must not land on it just because sequential numbering
+ # would otherwise reach it as the second assignment.
+ configured = {}
+ pending = {'ethernet': {'eth1'}, 'wireless': set()}
+ current = {'eth9': 'aa', 'eth8': 'bb'}
+ plan = resolver.compute_bootstrap_plan(configured, current, {},
+ pending=pending)
+ self.assertEqual(plan.get('eth9'), 'eth0')
+ self.assertEqual(plan.get('eth8'), 'eth2')
+ self.assertNotEqual(plan.get('eth9'), 'eth1')
+ self.assertNotEqual(plan.get('eth8'), 'eth1')
+
+ def test_reclaimed_candidate_excluded_from_ordinary_bootstrap(self):
+ # main() already matched this candidate to a pending node via
+ # match_pending_nodes() - it must not also be assigned a fresh
+ # bootstrap name here, even if its current name differs from the
+ # reclaimed target (main() handles the actual rename itself).
+ configured = {}
+ current = {'eth9': 'm1', 'eth8': 'm2'}
+ plan = resolver.compute_bootstrap_plan(configured, current, {},
+ reclaimed_macs={'m1'})
+ self.assertNotIn('eth9', plan)
+ self.assertIn('eth8', plan)
+
class TestSafeBulkRename(unittest.TestCase):
"""The two-phase rename must accurately report what actually happened -
@@ -1035,26 +1109,24 @@ class TestMainFirstBootBootstrap(unittest.TestCase):
self.assertNotIn('eth11', state)
self.assertNotIn('eth12', state)
- def test_stray_leftover_interface_still_fills_both_open_names(self):
+ def test_stray_leftover_interface_makes_reclaim_ambiguous(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.
+ # was also present, making this a 1 pending/2 candidate case.
+ # There is no way to tell which candidate was "really" eth7's own
+ # past hardware once its hw-id is gone, so eth7 stays unresolved;
+ # both candidates still get a real, settings-free hw-id via
+ # ordinary bootstrap naming instead of being lost.
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
+ state.update({'eth9': 'aa:bb:cc:dd:ee:07', # eth7's own hardware
+ 'vyeth13': 'ff:ff:ff:ff:ff:99'}) # unrelated leftover
def fake_discover(*_a, **_kw):
return dict(state)
@@ -1081,25 +1153,25 @@ class TestMainFirstBootBootstrap(unittest.TestCase):
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
+ # 'eth7' stays pending rather than being guessed at; both
+ # candidates still land on real, settings-free names (the lone
+ # ordinary gap, then the next fresh slot after that)
+ self.assertNotIn('eth7', state)
self.assertEqual(state.get('eth3'), 'aa:bb:cc:dd:ee:07')
- self.assertEqual(state.get('eth7'), 'ff:ff:ff:ff:ff:99')
+ self.assertEqual(state.get('eth8'), 'ff:ff:ff:ff:ff:99')
hints = set(os.listdir(self.udev_dir))
- self.assertEqual(hints, {'eth3', 'eth7'})
+ self.assertEqual(hints, {'eth3', 'eth8'})
status = json.loads(resolver.status_file.read_text())
- self.assertEqual(status['pending_unresolved'], [])
- self.assertEqual(status['reclaimed'], {'ff:ff:ff:ff:ff:99': 'eth7'})
+ self.assertEqual(status['pending_unresolved'], ['eth7'])
+ self.assertEqual(status['reclaimed'], {})
- def test_second_pending_node_fills_lowest_numbered_first_when_hardware_is_short(self):
+ def test_second_pending_node_makes_reclaim_ambiguous(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.
+ # up this boot - which one it belongs to can't be told, so
+ # neither is matched; the candidate still gets a real,
+ # settings-free name via ordinary bootstrap naming.
configured = {'m0': 'eth0'}
pending = {'ethernet': {'eth1', 'eth7'}, 'wireless': set()}
state = {'eth0': 'm0', 'eth9': 'aa:bb:cc:dd:ee:07'}
@@ -1129,30 +1201,28 @@ class TestMainFirstBootBootstrap(unittest.TestCase):
mock.patch('time.sleep'):
resolver.main()
- self.assertEqual(state.get('eth1'), 'aa:bb:cc:dd:ee:07')
+ self.assertNotIn('eth1', state)
self.assertNotIn('eth7', state)
+ self.assertEqual(state.get('eth2'), 'aa:bb:cc:dd:ee:07')
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'})
+ self.assertEqual(status['pending_unresolved'], ['eth1', 'eth7'])
+ self.assertEqual(status['reclaimed'], {})
- def test_two_candidates_for_one_pending_node_fill_deterministically(self):
+ def test_ambiguous_candidates_never_get_a_permanent_wrong_binding(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.
+ # EARLIER ambiguous boot had already permanently bound the wrong
+ # candidate elsewhere. One pending node with two candidates this
+ # boot must not be guessed at either way - it stays pending, and
+ # both candidates still get a real, settings-free hw-id via
+ # ordinary bootstrap naming (the one open gap, then a fresh slot).
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
+ 'racyA': 'aa:bb:cc:dd:ee:02', # eth2's own hardware
+ 'racyB': 'ff:ff:ff:ff:ff:99', # unrelated leftover candidate
}
def fake_discover(*_a, **_kw):
@@ -1180,24 +1250,32 @@ class TestMainFirstBootBootstrap(unittest.TestCase):
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
+ # 'eth2' stays pending; both candidates land on real, settings-
+ # free names instead (the one open gap, then a fresh slot)
+ self.assertNotIn('eth2', state)
self.assertEqual(state.get('eth1'), 'aa:bb:cc:dd:ee:02')
- self.assertEqual(state.get('eth2'), 'ff:ff:ff:ff:ff:99')
+ self.assertEqual(state.get('eth3'), 'ff:ff:ff:ff:ff:99')
hints = set(os.listdir(self.udev_dir))
- self.assertEqual(hints, {'eth1', 'eth2'})
+ self.assertEqual(hints, {'eth1', 'eth3'})
status = json.loads(resolver.status_file.read_text())
- self.assertEqual(status['pending_unresolved'], [])
- self.assertEqual(status['reclaimed'], {'ff:ff:ff:ff:ff:99': 'eth2'})
+ self.assertEqual(status['pending_unresolved'], ['eth2'])
+ self.assertEqual(status['reclaimed'], {})
- def test_two_pending_nodes_two_candidates_both_fill_deterministically(self):
+ def test_simultaneous_ambiguity_holds_back_every_candidate(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.
+ # Each candidate's MAC happens to obviously "belong" to one
+ # specific pending node (racyA's mac ends in :00, matching what
+ # eth0 used to be; racyB's mac ends in :02, matching eth2) - but
+ # there is no way to actually verify that pairing, so with 2
+ # pending nodes and 2 candidates present at once, neither may be
+ # auto-matched: guessing the "obvious" pairing risks silently
+ # binding a configured node's settings to the wrong physical NIC
+ # if the guess is ever wrong. Both stay pending and reported;
+ # both candidates still get a real, settings-free hw-id via
+ # ordinary bootstrap naming.
configured = {
'm1': 'eth1', 'm3': 'eth3', 'm4': 'eth4',
'm5': 'eth5', 'm6': 'eth6', 'm7': 'eth7',
@@ -1205,8 +1283,8 @@ class TestMainFirstBootBootstrap(unittest.TestCase):
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',
+ 'racyA': 'aa:bb:cc:dd:ee:00', # looks like eth0's old hardware
+ 'racyB': 'aa:bb:cc:dd:ee:02', # looks like eth2's old hardware
})
def fake_discover(*_a, **_kw):
@@ -1234,18 +1312,20 @@ class TestMainFirstBootBootstrap(unittest.TestCase):
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')
+ # neither pending node is guessed at - both candidates land on
+ # fresh, settings-free bootstrap names instead (no ordinary gaps
+ # exist here, so both go beyond the highest configured index)
+ self.assertNotIn('eth0', state)
+ self.assertNotIn('eth2', state)
+ self.assertEqual(state.get('eth8'), 'aa:bb:cc:dd:ee:00')
+ self.assertEqual(state.get('eth9'), 'aa:bb:cc:dd:ee:02')
hints = set(os.listdir(self.udev_dir))
- self.assertEqual(hints, {'eth0', 'eth2'})
+ self.assertEqual(hints, {'eth8', 'eth9'})
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',
- })
+ self.assertEqual(status['pending_unresolved'], ['eth0', 'eth2'])
+ self.assertEqual(status['reclaimed'], {})
def test_squatting_pending_candidate_still_reclaims_correctly(self):
# field report: deleting eth7's hw-id (node left in place) produced
@@ -1310,15 +1390,14 @@ class TestMainFirstBootBootstrap(unittest.TestCase):
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):
+ def test_squatting_candidate_and_extra_candidate_makes_reclaim_ambiguous(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.
+ # second, unrelated candidate also present this boot - genuinely
+ # ambiguous, exactly like a non-squatting extra candidate would
+ # be. The pending node stays unresolved; the squatter still gets
+ # evicted off of the configured slot it's sitting on (so the
+ # rightful owner isn't blocked) and, like the stray, lands on a
+ # real, settings-free bootstrap name instead of the pending one.
configured = {
'm0': 'eth0', 'm1': 'eth1', 'm2': 'eth2',
'm3': 'eth3', 'm4': 'eth4', 'm5': 'eth5',
@@ -1356,29 +1435,35 @@ class TestMainFirstBootBootstrap(unittest.TestCase):
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
+ # m3 still lands on its own configured slot; 'eth7' stays pending
+ # rather than being guessed at; the squatter and the stray both
+ # land on real, settings-free names instead
self.assertEqual(state.get('eth3'), 'm3')
+ self.assertNotIn('eth7', state)
self.assertEqual(state.get('eth6'), 'aa:bb:cc:dd:ee:07')
- self.assertEqual(state.get('eth7'), 'ff:ff:ff:ff:ff:99')
+ self.assertEqual(state.get('eth8'), 'ff:ff:ff:ff:ff:99')
hints = set(os.listdir(self.udev_dir))
- self.assertEqual(hints, {'eth6', 'eth7'})
+ self.assertEqual(hints, {'eth6', 'eth8'})
status = json.loads(resolver.status_file.read_text())
- self.assertEqual(status['pending_unresolved'], [])
- self.assertEqual(status['reclaimed'], {'ff:ff:ff:ff:ff:99': 'eth7'})
+ self.assertEqual(status['pending_unresolved'], ['eth7'])
+ self.assertEqual(status['reclaimed'], {})
- def test_gap_backfill_and_pending_reclaim_coexist_in_the_same_boot(self):
+ def test_gap_backfill_and_pending_node_coexisting_stays_ambiguous(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.
+ # (del_idx, hwid_idx = random.sample(range(8), 2)). This is
+ # genuinely ambiguous for the pending node (eth2): its own
+ # hardware and the gap's freed hardware are two indistinguishable
+ # candidates of the same type, and guessing risks silently
+ # binding eth2's settings to the wrong physical NIC - confirmed
+ # in the field. eth2 stays pending; both candidates still get a
+ # real, settings-free hw-id via ordinary bootstrap naming (the
+ # actual gap, then a fresh slot) - nothing is lost, just not
+ # auto-attributed to the right name.
configured = {
'm0': 'eth0', 'm1': 'eth1', 'm3': 'eth3', 'm4': 'eth4',
'm6': 'eth6', 'm8': 'eth8', 'm9': 'eth9',
@@ -1415,12 +1500,13 @@ class TestMainFirstBootBootstrap(unittest.TestCase):
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')
+ self.assertNotIn('eth2', state)
+ self.assertEqual(state.get('eth5'), 'aa:bb:cc:dd:ee:02')
+ self.assertEqual(state.get('eth7'), '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'})
+ self.assertEqual(status['pending_unresolved'], ['eth2'])
+ self.assertEqual(status['reclaimed'], {})
class TestWriteStatus(unittest.TestCase):