From b8ab027dd321446e6428342cff30f0b10290d823 Mon Sep 17 00:00:00 2001 From: Christian Breunig Date: Wed, 7 Oct 2026 19:56:19 +0200 Subject: Revert: T3871: never guess a pending node's hardware when ambiguous This reverts commit 4b92c2fe6f9928a1ff126c92e89e993de17fb0f5. --- src/tests/test_net_name_resolve.py | 308 +++++++++++++------------------------ 1 file changed, 111 insertions(+), 197 deletions(-) (limited to 'src/tests/test_net_name_resolve.py') diff --git a/src/tests/test_net_name_resolve.py b/src/tests/test_net_name_resolve.py index e1530eba7..ea8d77427 100644 --- a/src/tests/test_net_name_resolve.py +++ b/src/tests/test_net_name_resolve.py @@ -200,63 +200,6 @@ 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 @@ -632,13 +575,13 @@ class TestComputeBootstrapPlan(unittest.TestCase): self.assertEqual(plan.get('ifaceB'), 'eth0') -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. +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): @@ -653,8 +596,9 @@ class TestComputeBootstrapPlanGapFill(unittest.TestCase): self.addCleanup(distance_patcher.stop) def test_open_name_filled_like_an_ordinary_gap(self): - # 'eth1' has nothing configured for it and no pending reservation - # - it is simply the lowest open name. + # '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, {}) @@ -671,10 +615,15 @@ class TestComputeBootstrapPlanGapFill(unittest.TestCase): self.assertEqual(plan.get('eth8'), 'eth1') self.assertEqual(plan.get('eth9'), 'eth4') - 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. + 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', @@ -691,14 +640,15 @@ class TestComputeBootstrapPlanGapFill(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 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. + # 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', @@ -711,38 +661,14 @@ class TestComputeBootstrapPlanGapFill(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) + 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') - 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 - @@ -1126,24 +1052,26 @@ class TestMainFirstBootBootstrap(unittest.TestCase): self.assertNotIn('eth11', state) self.assertNotIn('eth12', state) - def test_stray_leftover_interface_makes_reclaim_ambiguous(self): + 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. - # 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. + # 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', # eth7's own hardware - 'vyeth13': 'ff:ff:ff:ff:ff:99'}) # unrelated leftover + 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) @@ -1170,25 +1098,25 @@ class TestMainFirstBootBootstrap(unittest.TestCase): mock.patch('time.sleep'): resolver.main() - # '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) + # 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('eth8'), 'ff:ff:ff:ff:ff:99') + self.assertEqual(state.get('eth7'), 'ff:ff:ff:ff:ff:99') hints = set(os.listdir(self.udev_dir)) - self.assertEqual(hints, {'eth3', 'eth8'}) + self.assertEqual(hints, {'eth3', 'eth7'}) status = json.loads(resolver.status_file.read_text()) - self.assertEqual(status['pending_unresolved'], ['eth7']) - self.assertEqual(status['reclaimed'], {}) + self.assertEqual(status['pending_unresolved'], []) + self.assertEqual(status['reclaimed'], {'ff:ff:ff:ff:ff:99': 'eth7'}) - def test_second_pending_node_makes_reclaim_ambiguous(self): + 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 - 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. + # 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'} @@ -1218,28 +1146,30 @@ class TestMainFirstBootBootstrap(unittest.TestCase): mock.patch('time.sleep'): resolver.main() - self.assertNotIn('eth1', state) + self.assertEqual(state.get('eth1'), 'aa:bb:cc:dd:ee:07') 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'], ['eth1', 'eth7']) - self.assertEqual(status['reclaimed'], {}) + self.assertEqual(status['pending_unresolved'], ['eth7']) + self.assertEqual(status['reclaimed'], {'aa:bb:cc:dd:ee:07': 'eth1'}) - def test_ambiguous_candidates_never_get_a_permanent_wrong_binding(self): + 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 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). + # 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', # eth2's own hardware - 'racyB': 'ff:ff:ff:ff:ff:99', # unrelated leftover candidate + '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): @@ -1267,32 +1197,24 @@ class TestMainFirstBootBootstrap(unittest.TestCase): mock.patch('time.sleep'): resolver.main() - # 'eth2' stays pending; both candidates land on real, settings- - # free names instead (the one open gap, then a fresh slot) - self.assertNotIn('eth2', state) + # 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('eth3'), 'ff:ff:ff:ff:ff:99') + self.assertEqual(state.get('eth2'), 'ff:ff:ff:ff:ff:99') hints = set(os.listdir(self.udev_dir)) - self.assertEqual(hints, {'eth1', 'eth3'}) + self.assertEqual(hints, {'eth1', 'eth2'}) status = json.loads(resolver.status_file.read_text()) - self.assertEqual(status['pending_unresolved'], ['eth2']) - self.assertEqual(status['reclaimed'], {}) + self.assertEqual(status['pending_unresolved'], []) + self.assertEqual(status['reclaimed'], {'ff:ff:ff:ff:ff:99': 'eth2'}) - def test_simultaneous_ambiguity_holds_back_every_candidate(self): + 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. - # 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. + # 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', @@ -1300,8 +1222,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', # looks like eth0's old hardware - 'racyB': 'aa:bb:cc:dd:ee:02', # looks like eth2's old hardware + 'racyA': 'aa:bb:cc:dd:ee:00', + 'racyB': 'aa:bb:cc:dd:ee:02', }) def fake_discover(*_a, **_kw): @@ -1329,20 +1251,18 @@ class TestMainFirstBootBootstrap(unittest.TestCase): mock.patch('time.sleep'): resolver.main() - # 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') + 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, {'eth8', 'eth9'}) + self.assertEqual(hints, {'eth0', 'eth2'}) status = json.loads(resolver.status_file.read_text()) - self.assertEqual(status['pending_unresolved'], ['eth0', 'eth2']) - self.assertEqual(status['reclaimed'], {}) + 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 @@ -1407,14 +1327,15 @@ 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_makes_reclaim_ambiguous(self): + 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 - 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. + # 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', @@ -1452,35 +1373,29 @@ class TestMainFirstBootBootstrap(unittest.TestCase): mock.patch('time.sleep'): resolver.main() - # 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 + # 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.assertNotIn('eth7', state) self.assertEqual(state.get('eth6'), 'aa:bb:cc:dd:ee:07') - self.assertEqual(state.get('eth8'), 'ff:ff:ff:ff:ff:99') + self.assertEqual(state.get('eth7'), 'ff:ff:ff:ff:ff:99') hints = set(os.listdir(self.udev_dir)) - self.assertEqual(hints, {'eth6', 'eth8'}) + self.assertEqual(hints, {'eth6', 'eth7'}) status = json.loads(resolver.status_file.read_text()) - self.assertEqual(status['pending_unresolved'], ['eth7']) - self.assertEqual(status['reclaimed'], {}) + self.assertEqual(status['pending_unresolved'], []) + self.assertEqual(status['reclaimed'], {'ff:ff:ff:ff:ff:99': 'eth7'}) - def test_gap_backfill_and_pending_node_coexisting_stays_ambiguous(self): + 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)). 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. + # (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', @@ -1517,13 +1432,12 @@ class TestMainFirstBootBootstrap(unittest.TestCase): mock.patch('time.sleep'): resolver.main() - 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') + 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'], ['eth2']) - self.assertEqual(status['reclaimed'], {}) + self.assertEqual(status['pending_unresolved'], []) + self.assertEqual(status['reclaimed'], {'aa:bb:cc:dd:ee:02': 'eth2'}) class TestMainInContainer(unittest.TestCase): -- cgit v1.2.3 From 552ade702fc033f68cda998055bd5b74f195c7ac Mon Sep 17 00:00:00 2001 From: Christian Breunig Date: Wed, 7 Oct 2026 19:56:33 +0200 Subject: Revert: T3871: make boot-time interface naming deterministic and hw-id aware This reverts commit 22cfee116c9935f4a33da4923eff189f2f646371. --- src/helpers/vyos-interface-rescan.py | 3 +- src/init/vyos-router | 27 - src/system/vyos-net-name-resolve.py | 662 ------------ src/systemd/vyos-net-name-resolve.service | 17 - src/tests/helper.py | 11 - src/tests/test_net_name_resolve.py | 1591 ----------------------------- src/udev/vyos_net_name | 142 ++- 7 files changed, 105 insertions(+), 2348 deletions(-) delete mode 100755 src/system/vyos-net-name-resolve.py delete mode 100644 src/systemd/vyos-net-name-resolve.service delete mode 100644 src/tests/test_net_name_resolve.py (limited to 'src/tests/test_net_name_resolve.py') diff --git a/src/helpers/vyos-interface-rescan.py b/src/helpers/vyos-interface-rescan.py index 0bc3ac7d0..fea9bca1c 100755 --- a/src/helpers/vyos-interface-rescan.py +++ b/src/helpers/vyos-interface-rescan.py @@ -97,8 +97,7 @@ 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-resolve.py for interfaces without a configured hw-id + """ Read any new interface data left in /run/udev/vyos by vyos_net_name """ interfaces = {} diff --git a/src/init/vyos-router b/src/init/vyos-router index f109b95da..c0d12e2a7 100755 --- a/src/init/vyos-router +++ b/src/init/vyos-router @@ -362,26 +362,6 @@ 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", @@ -626,12 +606,6 @@ 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_bootup_script pre run_preconfig_script @@ -639,7 +613,6 @@ 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 deleted file mode 100755 index 29cfe6681..000000000 --- a/src/system/vyos-net-name-resolve.py +++ /dev/null @@ -1,662 +0,0 @@ -#!/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.system.image import is_running_as_container -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. - - Also excludes interfaces enslaved to another netdev (master symlink - present), which covers Azure VF datapath interfaces bound under their - synthetic parent. Those are acceleration children, not independent - primary interfaces, and must not be candidates for hw-id naming. - - 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(): - logger.debug( - f"skipping '{entry.name}': no backing device in sysfs" - ) - continue - if (entry / 'master').exists(): - logger.debug( - f"skipping '{entry.name}': interface is enslaved via master link" - ) - 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(): - if is_running_as_container(): - # A container has no NIC of its own - its interfaces are veth pairs - # created by the container runtime. They have no backing bus device in - # sysfs and their MAC is assigned by the host and regenerated on every - # start, so there is nothing to wait for and no hw-id worth binding. - logger.info('running inside a container - skipping hw-id naming pass') - return - - 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 deleted file mode 100644 index ec7c45636..000000000 --- a/src/systemd/vyos-net-name-resolve.service +++ /dev/null @@ -1,17 +0,0 @@ -[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 5a74525b7..2c4421d0a 100644 --- a/src/tests/helper.py +++ b/src/tests/helper.py @@ -14,20 +14,9 @@ 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 deleted file mode 100644 index ea8d77427..000000000 --- a/src/tests/test_net_name_resolve.py +++ /dev/null @@ -1,1591 +0,0 @@ -# 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') - -_container_patcher = None - - -def setUpModule(): - # main() deliberately short-circuits inside a container, and these tests - # are routinely run from one (the VyOS build container). Pin the detection - # off for the whole module so the naming pass under test actually runs - - # TestMainInContainer patches it back on locally where that is the point. - global _container_patcher - _container_patcher = mock.patch.object( - resolver, 'is_running_as_container', return_value=False) - _container_patcher.start() - - -def tearDownModule(): - _container_patcher.stop() - - -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 TestMainInContainer(unittest.TestCase): - """A container owns no NIC: its interfaces are runtime-created veth - pairs with no backing bus device in sysfs and a host-assigned MAC that - changes on every start. The naming pass can therefore never resolve a - pending node there - it only spent both bounded hardware waits and then - told the user to bind an hw-id that would not survive a restart. - """ - - 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_naming_pass_is_skipped_entirely(self): - with mock.patch.object(resolver, 'is_running_as_container', - return_value=True), \ - mock.patch.object(resolver, 'get_configfile_interfaces') as configured, \ - mock.patch.object(resolver, 'get_pending_hwid_nodes') as pending, \ - mock.patch.object(resolver, 'discover_physical_interfaces') as discover, \ - mock.patch.object(resolver, 'run') as run: - resolver.main() - - configured.assert_not_called() - pending.assert_not_called() - discover.assert_not_called() - run.assert_not_called() - # no status file means vyos-router's warn_missing_interface_hardware() - # stays silent instead of warning about an unbindable pending node - self.assertFalse(resolver.status_file.exists()) - self.assertEqual(os.listdir(self.udev_dir), []) - - -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 388cc78be..f5c3b9f1b 100755 --- a/src/udev/vyos_net_name +++ b/src/udev/vyos_net_name @@ -19,28 +19,20 @@ 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'] - -# 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. +config_path = '/opt/vyatta/etc/config/config.boot' def is_available(intfs: dict, intf_name: str) -> bool: @@ -52,24 +44,16 @@ 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 = [] - 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 = [ + int(x.replace(prefix, '')) for x in list(intfs.values()) if prefix in x + ] 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}' + return f'{prefix}{len(index_list)}' def mod_ifname(ifname: str) -> str: @@ -105,10 +89,7 @@ def get_biosdevname(ifname: str) -> str: try: biosname = cmdl(['/sbin/biosdevname', '--policy', 'all_ethN', '-i', ifname]) except Exception as 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}') + logger.error(f'biosdevname error: {e}') biosname = '' return intf if biosname == '' else biosname @@ -136,6 +117,81 @@ 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): @@ -153,25 +209,30 @@ 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 + """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] - 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'{intf_name} ({hwid}): already assigned this boot: {interfaces}') + logger.debug(f'adding assigned interfaces: {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"{intf_name} ({hwid}) -> '{newname}'") + logger.info(f"new name for '{intf_name}' is '{newname}'") leave_rescan_hint(newname, hwid) @@ -191,9 +252,9 @@ if __name__ == '__main__': logger = logging.getLogger() logger.addHandler(syslog_handler) - logger.setLevel(logging.INFO) + logger.setLevel(logging.DEBUG) - logger.debug(f'started with arguments: {argv}') + logger.debug(f'Started with arguments: {argv}') if len(argv) > 3: predef_name = argv[3] @@ -205,6 +266,11 @@ if __name__ == '__main__': lock.acquire(60) if not boot_configuration_complete(): - print(on_boot_event(argv[1], argv[2], predefined=predef_name)) + 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') lock.release() + logger.debug('Finished') -- cgit v1.2.3 From c73faac7f95ec837da6f964d15cc05ff9ffd2de6 Mon Sep 17 00:00:00 2001 From: Christian Breunig Date: Wed, 7 Oct 2026 19:56:37 +0200 Subject: interfaces: T3871: name interfaces from a slot-anchored mapping store Names used to be derived from 'hw-id' in config.boot. A node carrying an address but no hw-id had to be matched to some NIC, and nothing available at boot could say which one - every heuristic could bind a configured address to the wrong physical wire. Naming authority moves to a mapping store binding a name to the slot the hardware occupies. The slot is the identity, so a replacement card in that slot is that interface again. The store renders systemd .link files, leaving udev to apply names as devices appear. --- debian/control | 1 - python/vyos/defaults.py | 1 - python/vyos/ifconfig/ifname_store.py | 373 ++++++++++++++ .../rules.d/62-temporary-interface-rename.rules | 1 - src/etc/udev/rules.d/63-hyperv-vf-net.rules | 16 + src/etc/udev/rules.d/64-vyos-vmware-net.rules | 14 - src/etc/udev/rules.d/65-vyos-net.rules | 41 -- src/helpers/vyos-interface-rescan.py | 155 ++---- src/init/vyos-router | 37 +- src/system/vyos-net-name-resolve.py | 221 ++++++++ src/systemd/vyos-net-name-resolve.service | 15 + src/tests/helper.py | 11 + src/tests/test_ifname_store.py | 572 +++++++++++++++++++++ src/tests/test_net_name_resolve.py | 227 ++++++++ src/udev/vyos_net_name | 276 ---------- 15 files changed, 1518 insertions(+), 443 deletions(-) create mode 100644 python/vyos/ifconfig/ifname_store.py delete mode 100644 src/etc/udev/rules.d/62-temporary-interface-rename.rules delete mode 100644 src/etc/udev/rules.d/64-vyos-vmware-net.rules delete mode 100644 src/etc/udev/rules.d/65-vyos-net.rules create mode 100755 src/system/vyos-net-name-resolve.py create mode 100644 src/systemd/vyos-net-name-resolve.service create mode 100644 src/tests/test_ifname_store.py create mode 100644 src/tests/test_net_name_resolve.py delete mode 100755 src/udev/vyos_net_name (limited to 'src/tests/test_net_name_resolve.py') diff --git a/debian/control b/debian/control index 0c7a6f505..43ebe43e8 100644 --- a/debian/control +++ b/debian/control @@ -68,7 +68,6 @@ Depends: at, rsync, vyatta-bash, - vyatta-biosdevname, vyatta-cfg, vyos-http-api-tools, ## End of Fundamentals diff --git a/python/vyos/defaults.py b/python/vyos/defaults.py index 84244d5c1..20d13df7b 100644 --- a/python/vyos/defaults.py +++ b/python/vyos/defaults.py @@ -32,7 +32,6 @@ directories = { 'api_schema': f'{base_dir}/services/api/graphql/graphql/schema/', 'api_client_op': f'{base_dir}/services/api/graphql/graphql/client_op/', 'api_templates': f'{base_dir}/services/api/graphql/session/templates/', - 'vyos_udev_dir' : '/run/udev/vyos', 'isc_dhclient_dir' : '/run/dhclient', 'dhcp6_client_dir' : '/run/dhcp6c', 'vyos_configdir' : '/opt/vyatta/config', diff --git a/python/vyos/ifconfig/ifname_store.py b/python/vyos/ifconfig/ifname_store.py new file mode 100644 index 000000000..0d10fd1ef --- /dev/null +++ b/python/vyos/ifconfig/ifname_store.py @@ -0,0 +1,373 @@ +# Copyright VyOS maintainers and contributors +# +# This library is free software; you can redistribute it and/or +# modify it under the terms of the GNU Lesser General Public +# License as published by the Free Software Foundation; either +# version 2.1 of the License, or (at your option) any later version. +# +# This library 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 +# Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public +# License along with this library. If not, see . +# +# Persistent interface name store. +# +# The kernel command line carries net.ifnames=0, so VyOS owns the +# 'ethN'/'wlanN' namespace and has to make its choice survive a reboot. A name +# is mapped to the slot the hardware sits in rather than to a MAC in +# config.boot: the slot is the identity, so a replacement card in that slot is +# simply that interface again. +# +# The key is a single udev property picked from a fallback chain, most stable +# first. udev still computes these under net.ifnames=0 - only the renaming +# policy is disabled. + +import json +import os +import re +import tempfile +from pathlib import Path + +from vyos.defaults import directories +from vyos.utils.process import rc_cmd +from vyos.utils.permission import get_cfg_group_id + +STORE_VERSION = 1 + +# The real path, not '/config': naming runs before the bind mount is up. +store_path = Path(directories['config']) / 'interface-mapping.json' + +# Most stable first: a firmware slot index survives PCI renumbering, a path +# does not. The MAC-derived entry is a last resort for paravirtual buses. +KEY_PROPERTIES = ( + 'ID_NET_NAME_ONBOARD', + 'ID_NET_NAME_SLOT', + 'ID_NET_NAME_PATH', + 'ID_PATH', + 'ID_NET_NAME_MAC', +) + +_BUS_RANK = {'pci': 0, 'vmbus': 1, 'xen': 2, 'usb': 3} +_BUS_RANK_UNKNOWN = 90 + +_PCI_BDF = re.compile(r'([0-9a-f]{4}):([0-9a-f]{2}):([0-9a-f]{2})\.([0-9a-f]+)', re.I) +_MAC_RE = re.compile(r'([0-9a-fA-F]{2}(:[0-9a-fA-F]{2}){5})') + + +def ambiguous_keys(devices: list) -> set: + """Property values carried by more than one device.""" + seen = {} + for properties in devices: + for prop in KEY_PROPERTIES: + value = properties.get(prop, '') + if value: + tmp = f'{prop}={value}' + seen[tmp] = seen.get(tmp, 0) + 1 + return {key for key, count in seen.items() if count > 1} + + +def hardware_key(properties: dict, ambiguous: set = frozenset()) -> str: + """The most stable identifier this device offers, as 'PROPERTY=value'. + + A value more than one device carries identifies neither of them, so it is + passed over for the next one. + """ + for prop in KEY_PROPERTIES: + value = properties.get(prop, '') + if value and f'{prop}={value}' not in ambiguous: + return f'{prop}={value}' + return '' + + +def device_matches(properties: dict, key: str) -> bool: + """Does this device carry the property a stored key names?""" + if not key or '=' not in key: + return False + prop, _, value = key.partition('=') + return properties.get(prop, '') == value + + +def canonical_sort_key(device: dict) -> tuple: + """Total order by bus class, physical position then MAC, so it never + depends on probe order. Only used for hardware not yet in the store. + """ + properties = device.get('properties') or {} + path = properties.get('ID_PATH', '') + rank = _BUS_RANK.get(properties.get('ID_BUS', ''), _BUS_RANK_UNKNOWN) + + # numeric, so 0000:09:00.0 sorts before 0000:10:00.0 + segments = [tuple(int(part, 16) for part in m.groups()) + for m in _PCI_BDF.finditer(path)] + + return (rank, segments, path, device.get('mac') or '') + + +def permanent_mac(ifname: str, sys_class_net: str = '/sys/class/net') -> str: + """Permanent hardware address, falling back to the current one when the + driver cannot report it separately. + """ + code, out = rc_cmd(f'ethtool -P {ifname}') + if code == 0: + m = _MAC_RE.search(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 is_wireless(ifname: str, sys_class_net: str = '/sys/class/net') -> bool: + """Checked via sysfs, not via the - possibly still probe-order - name.""" + return (Path(sys_class_net) / ifname / 'phy80211').exists() + + +def read_properties(ifname: str) -> dict: + """udev properties for an interface.""" + try: + import pyudev + + context = pyudev.Context() + device = pyudev.Devices.from_path(context, f'/sys/class/net/{ifname}') + return dict(device.properties) + except Exception: + return {} + + +def discover_devices(sys_class_net: str = '/sys/class/net') -> list: + """Every interface backed by independently-addressable hardware. + + Skips virtual interfaces, enslaved ones, SR-IOV virtual functions and + hypervisor VF datapaths - the latter share another interface's MAC. + """ + devices = [] + net_dir = Path(sys_class_net) + if not net_dir.is_dir(): + return devices + + for entry in sorted(net_dir.iterdir()): + if not (entry / 'device').exists(): + continue + if (entry / 'master').exists(): + continue + if (entry / 'device' / 'physfn').exists(): + continue + if entry.name.startswith('vf_'): + continue + + mac = permanent_mac(entry.name, sys_class_net) + if not mac: + continue + + devices.append({ + 'name': entry.name, + 'mac': mac, + 'wireless': is_wireless(entry.name, sys_class_net), + 'properties': read_properties(entry.name), + }) + + ambiguous = ambiguous_keys([device['properties'] for device in devices]) + for device in devices: + device['key'] = hardware_key(device['properties'], ambiguous) + + return devices + + +def empty_store() -> dict: + return {'version': STORE_VERSION, 'interfaces': {}, 'hardware': {}} + + +def load_store(path: Path = None) -> dict: + """A missing, unreadable or outdated store yields an empty one, so + everything is named from scratch. + """ + path = path or store_path + try: + data = json.loads(path.read_text()) + except (FileNotFoundError, OSError, ValueError): + return empty_store() + + if not isinstance(data, dict) or data.get('version') != STORE_VERSION: + return empty_store() + if not isinstance(data.get('interfaces'), dict): + return empty_store() + + interfaces = {name: key for name, key in data['interfaces'].items() + if isinstance(key, str) and key} + + # the card each name last sat on. Absent from a store written before this + # was recorded, which just means the first pass has nothing to compare + # against and reports no replacement - not that nothing changed. + seen = data.get('hardware') + hardware = {name: mac for name, mac in seen.items() + if isinstance(mac, str) and mac and name in interfaces + } if isinstance(seen, dict) else {} + + return {'version': STORE_VERSION, 'interfaces': interfaces, + 'hardware': hardware} + + +def config_dir_is_mounted(path: Path = None) -> bool: + """Is the config directory the real one, or a bare rootfs overlay? + + A failed TPM/LUKS unlock does not stop the boot. Writing to the overlay + underneath would be masked once the real volume mounts, leaving two + stores which silently diverge. + """ + path = path or store_path + return (path.parent / '.vyatta_config').exists() + + +def save_store(store: dict, path: Path = None) -> bool: + """Write atomically, so an interrupted boot leaves no half-written file. + + Returns False without writing on a degraded boot: names are still + resolved for this boot, they just must not be persisted. + """ + path = path or store_path + if not config_dir_is_mounted(path): + return False + + path.parent.mkdir(parents=True, exist_ok=True) + + fd, tmp = tempfile.mkstemp(dir=path.parent, prefix=f'.{path.name}.') + try: + with os.fdopen(fd, 'w') as f: + json.dump(store, f, indent=2, sort_keys=True) + f.write('\n') + os.chmod(tmp, 0o664) + try: + os.chown(tmp, -1, get_cfg_group_id()) + except (OSError, KeyError): + # not fatal, the store is read by root at boot + pass + os.replace(tmp, path) + except Exception: + Path(tmp).unlink(missing_ok=True) + raise + + return True + + +def _prefix_for(device: dict) -> str: + return 'wlan' if device.get('wireless') else 'eth' + + +def _next_free(taken: set, prefix: str) -> str: + index = 0 + while f'{prefix}{index}' in taken: + index += 1 + return f'{prefix}{index}' + + +def resolve(devices: list, store: dict) -> tuple: + """Name every physical interface present this boot. + + Returns (plan, new_store, report); plan is {current: new} for the + interfaces which have to be renamed. The slot decides the name - nothing + here looks at the configuration, so there is nothing to guess. + """ + entries = store.get('interfaces', {}) + seen = store.get('hardware', {}) + + assigned = {} # final name -> device + report = {'matched': {}, 'bootstrapped': {}} + + # A recorded name stays off-limits even while its slot is empty, so new + # hardware cannot inherit an absent interface's name and configuration. + reserved = set(entries) + remaining = list(devices) + + def claim(device, name, bucket): + assigned[name] = device + report[bucket][name] = device['mac'] + remaining.remove(device) + + # whatever card occupies the recorded slot is this interface + for name, key in entries.items(): + for device in remaining: + if device_matches(device['properties'], key): + claim(device, name, 'matched') + break + + # unknown hardware: lowest free name of its type, in hardware order + for device in sorted(remaining, key=canonical_sort_key): + name = _next_free(reserved | set(assigned), _prefix_for(device)) + claim(device, name, 'bootstrapped') + + # keep entries for empty slots - an unplugged card keeps its name + new_interfaces = dict(entries) + for name, device in assigned.items(): + if device['key']: + new_interfaces[name] = device['key'] + + # remember which card each name sat on, keeping the last record for an + # absent one so its eventual replacement is still noticed + new_hardware = dict(seen) + for name, device in assigned.items(): + if device['mac']: + new_hardware[name] = device['mac'] + + # a slot which kept its name but changed card hands that interface's + # addresses to different hardware - never silently + report['replaced'] = {name: mac for name, mac in report['matched'].items() + if seen.get(name) and seen[name] != mac} + + plan = {device['name']: name for name, device in assigned.items() + if device['name'] != name} + + return plan, {'version': STORE_VERSION, 'interfaces': new_interfaces, + 'hardware': new_hardware}, report + + +LINK_DIR = Path('/etc/systemd/network') +LINK_PREFIX = '10-vyos-' + + +def render_link(name: str, key: str) -> str: + """Render a systemd .link file body, matching on the stored property. + + The device type has to match as well, and a radio is 'wlan' - a .link + naming a type the device is not simply never applies, leaving the + interface on its probe-order name. resolve() already names by type (see + _prefix_for()), so the recorded name is what says which. + """ + return ( + '# Generated by VyOS - do not edit.\n' + "# Interface names are managed via 'interface-mapping.json'.\n" + '[Match]\n' + f"Type={'wlan' if name.startswith('wlan') else 'ether'}\n" + f'Property={key}\n' + '\n' + '[Link]\n' + f'Name={name}\n' + ) + + +def sync_link_files(store: dict, link_dir: Path = None) -> list: + """One .link file per recorded name, so udev applies it as the device is + added - before anything can observe a probe-order name. + """ + link_dir = link_dir or LINK_DIR + link_dir.mkdir(parents=True, exist_ok=True) + + written = {} + for name, key in store.get('interfaces', {}).items(): + if key: + written[link_dir / f'{LINK_PREFIX}{name}.link'] = render_link(name, key) + + for path, body in written.items(): + if path.exists() and path.read_text() == body: + continue + path.write_text(body) + + # drop stale files, but only our own generated ones + for existing in link_dir.glob(f'{LINK_PREFIX}*.link'): + if existing not in written: + existing.unlink(missing_ok=True) + + return sorted(written) diff --git a/src/etc/udev/rules.d/62-temporary-interface-rename.rules b/src/etc/udev/rules.d/62-temporary-interface-rename.rules deleted file mode 100644 index 4a579dcab..000000000 --- a/src/etc/udev/rules.d/62-temporary-interface-rename.rules +++ /dev/null @@ -1 +0,0 @@ -SUBSYSTEM=="net", ACTION=="add", KERNEL=="eth*", DRIVERS=="?*", NAME="e$env{IFINDEX}" diff --git a/src/etc/udev/rules.d/63-hyperv-vf-net.rules b/src/etc/udev/rules.d/63-hyperv-vf-net.rules index e5ecb4ede..224af12ab 100644 --- a/src/etc/udev/rules.d/63-hyperv-vf-net.rules +++ b/src/etc/udev/rules.d/63-hyperv-vf-net.rules @@ -1,3 +1,12 @@ +# Azure/Hyper-V VF interfaces must be kept out of the ethN namespace that +# VyOS manages. A VF is the acceleration datapath of an existing synthetic +# NIC, not an interface in its own right, and it shares that NIC's MAC - so +# left alone it would be taken for a second, independent interface. +# +# Renaming it to vf_ethN here, at device-add, is what keeps it out of the +# way; vyos-net-name-resolve.py then skips anything named vf_* (as well as +# anything with an SR-IOV physfn link, which is the general case). + ATTR{[dmi/id]sys_vendor}!="Microsoft Corporation", GOTO="end_hyperv_nic" # Primary rename path for Azure VF. @@ -6,4 +15,11 @@ ATTR{[dmi/id]sys_vendor}!="Microsoft Corporation", GOTO="end_hyperv_nic" # the interfaces already renamed to vf_ethN ACTION=="add", SUBSYSTEM=="net", DRIVERS=="hv_pci", KERNEL=="eth*", PROGRAM="vyos_vf_name %k", NAME="%c" +# Fallback path: a VF that still appears as a plain ethN, which happens +# when it is seen in an intermediate state before the hv_pci match above +# applies. Scoped to Azure on purpose - bare-metal Mellanox NICs are +# ordinary interfaces and must not be caught here. +ACTION=="add", SUBSYSTEM=="net", DRIVERS=="mlx*_core", KERNEL=="eth*", PROGRAM="vyos_vf_name %k", NAME="%c" +ACTION=="add", SUBSYSTEM=="net", DRIVERS=="mana", KERNEL=="eth*", PROGRAM="vyos_vf_name %k", NAME="%c" + LABEL="end_hyperv_nic" diff --git a/src/etc/udev/rules.d/64-vyos-vmware-net.rules b/src/etc/udev/rules.d/64-vyos-vmware-net.rules deleted file mode 100644 index 66a4a069b..000000000 --- a/src/etc/udev/rules.d/64-vyos-vmware-net.rules +++ /dev/null @@ -1,14 +0,0 @@ -ATTR{[dmi/id]sys_vendor}!="VMware, Inc.", GOTO="end_vmware_nic" - -ACTION=="add", SUBSYSTEM=="net", ATTRS{label}=="Ethernet0", ENV{VYOS_IFNAME}="eth0" -ACTION=="add", SUBSYSTEM=="net", ATTRS{label}=="Ethernet1", ENV{VYOS_IFNAME}="eth1" -ACTION=="add", SUBSYSTEM=="net", ATTRS{label}=="Ethernet2", ENV{VYOS_IFNAME}="eth2" -ACTION=="add", SUBSYSTEM=="net", ATTRS{label}=="Ethernet3", ENV{VYOS_IFNAME}="eth3" -ACTION=="add", SUBSYSTEM=="net", ATTRS{label}=="Ethernet4", ENV{VYOS_IFNAME}="eth4" -ACTION=="add", SUBSYSTEM=="net", ATTRS{label}=="Ethernet5", ENV{VYOS_IFNAME}="eth5" -ACTION=="add", SUBSYSTEM=="net", ATTRS{label}=="Ethernet6", ENV{VYOS_IFNAME}="eth6" -ACTION=="add", SUBSYSTEM=="net", ATTRS{label}=="Ethernet7", ENV{VYOS_IFNAME}="eth7" -ACTION=="add", SUBSYSTEM=="net", ATTRS{label}=="Ethernet8", ENV{VYOS_IFNAME}="eth8" -ACTION=="add", SUBSYSTEM=="net", ATTRS{label}=="Ethernet9", ENV{VYOS_IFNAME}="eth9" - -LABEL="end_vmware_nic" diff --git a/src/etc/udev/rules.d/65-vyos-net.rules b/src/etc/udev/rules.d/65-vyos-net.rules deleted file mode 100644 index 0e7567cf8..000000000 --- a/src/etc/udev/rules.d/65-vyos-net.rules +++ /dev/null @@ -1,41 +0,0 @@ -# These rules use vyos_net_name to persistently name network interfaces -# per "hwid" association in the VyOS configuration file. - -ACTION!="add", GOTO="vyos_net_end" -SUBSYSTEM!="net", GOTO="vyos_net_end" - -# Fallback path for Azure VF interfaces: if a VF still appears as plain ethN, -# rename it to vf_ethX before persistent synthetic interface naming runs to -# prevent it from incorrectly being considered as a synthetic interface. -ATTR{[dmi/id]sys_vendor}=="Microsoft Corporation", DRIVERS=="mlx*_core", KERNEL=="eth*", PROGRAM="vyos_vf_name %k", NAME="%c", GOTO="vyos_net_end" -ATTR{[dmi/id]sys_vendor}=="Microsoft Corporation", DRIVERS=="mana", KERNEL=="eth*", PROGRAM="vyos_vf_name %k", NAME="%c", GOTO="vyos_net_end" - -# VF interfaces are not handled by vyos_net_name, so skip them. -# KERNEL=="vf_*" catches already-renamed Mellanox and MANA VFs. -# DRIVERS=="mlx*_core" and DRIVERS=="mana" on Azure catch VF interfaces -# in an unexpected intermediate state (still named ethN/eN) that must not -# be remapped by vyos_net_name. -# This guard is intentionally scoped to Microsoft Azure to -# avoid excluding bare-metal Mellanox NICs on physical hardware, which do -# need persistent naming via vyos_net_name. -KERNEL=="vf_*", GOTO="vyos_net_end" -ATTR{[dmi/id]sys_vendor}=="Microsoft Corporation", DRIVERS=="mlx*_core", GOTO="vyos_net_end" -ATTR{[dmi/id]sys_vendor}=="Microsoft Corporation", DRIVERS=="mana", GOTO="vyos_net_end" - -# Do name change for ethernet and wireless devices only -KERNEL!="eth*|wlan*|e*", GOTO="vyos_net_end" - -# ignore "secondary" monitor interfaces of mac80211 drivers -KERNEL=="wlan*", ATTRS{type}=="803", GOTO="vyos_net_end" - -# If using VyOS predefined names -ENV{VYOS_IFNAME}!="eth*", GOTO="end_vyos_predef_names" - -DRIVERS=="?*", PROGRAM="vyos_net_name %k $attr{address} $env{VYOS_IFNAME}", NAME="%c", GOTO="vyos_net_end" - -LABEL="end_vyos_predef_names" - -# ignore interfaces without a driver link like bridges and VLANs -DRIVERS=="?*", PROGRAM="vyos_net_name %k $attr{address}", NAME="%c" - -LABEL="vyos_net_end" diff --git a/src/helpers/vyos-interface-rescan.py b/src/helpers/vyos-interface-rescan.py index fea9bca1c..fd2bf907e 100755 --- a/src/helpers/vyos-interface-rescan.py +++ b/src/helpers/vyos-interface-rescan.py @@ -14,21 +14,25 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . # +# Make discovered interfaces visible in the configuration, so a newly +# installed NIC shows up without the operator adding it by hand. # +# T3871: the node is created bare. Writing a 'hw-id' back into config.boot is +# what used to make a bad guess unrecoverable - naming belongs to the store. -import os -import stat import argparse import logging -import netaddr +import os +import stat +from pathlib import Path from vyos.configtree import ConfigTree from vyos.defaults import directories +from vyos.ifconfig.ifname_store import load_store from vyos.utils.permission import get_cfg_group_id debug = False -vyos_udev_dir = directories['vyos_udev_dir'] vyos_log_dir = directories['log'] log_file = os.path.splitext(os.path.basename(__file__))[0] vyos_log_file = os.path.join(vyos_log_dir, log_file) @@ -39,139 +43,73 @@ formatter = logging.Formatter('%(levelname)s: %(message)s') handler.setFormatter(formatter) logger.addHandler(handler) -passlist = { - '02:07:01' : 'Interlan', - '02:60:60' : '3Com', - '02:60:8c' : '3Com', - '02:a0:c9' : 'Intel', - '02:aa:3c' : 'Olivetti', - '02:cf:1f' : 'CMC', - '02:e0:3b' : 'Prominet', - '02:e6:d3' : 'BTI', - '52:54:00' : 'Realtek', - '52:54:4c' : 'Novell 2000', - '52:54:ab' : 'Realtec', - 'e2:0c:0f' : 'Kingston Technologies' -} - -def is_multicast(addr: netaddr.eui.EUI) -> bool: - return bool(addr.words[0] & 0b1) - -def is_locally_administered(addr: netaddr.eui.EUI) -> bool: - return bool(addr.words[0] & 0b10) - -def is_on_passlist(hwid: str) -> bool: - top = hwid.rsplit(':', 3)[0] - if top in list(passlist): - return True - return False - -def is_persistent(hwid: str) -> bool: - addr = netaddr.EUI(hwid) - if is_multicast(addr): - return False - if is_locally_administered(addr) and not is_on_passlist(hwid): - return False - return True + +def get_interface_type(intf: str) -> str: + if intf.startswith('eth'): + return 'ethernet' + if intf.startswith('wlan'): + return 'wireless' + logger.critical(f"Unrecognized interface type for '{intf}'") + return '' + def get_wireless_physical_device(intf: str) -> str: - if 'wlan' not in intf: - return '' + """The phy a wireless interface belongs to.""" try: - tmp = os.readlink(f'/sys/class/net/{intf}/phy80211') + return os.path.basename( + os.readlink(f'/sys/class/net/{intf}/phy80211')) except OSError: - logger.critical(f"Failed to read '/sys/class/net/{intf}/phy80211'") return '' - phy = os.path.basename(tmp) - logger.info(f"wireless phy is {phy}") - return phy - -def get_interface_type(intf: str) -> str: - if 'eth' in intf: - intf_type = 'ethernet' - elif 'wlan' in intf: - intf_type = 'wireless' - else: - logger.critical('Unrecognized interface type!') - intf_type = '' - return intf_type - -def get_new_interfaces() -> dict: - """ Read any new interface data left in /run/udev/vyos by vyos_net_name - """ - interfaces = {} - - for intf in os.listdir(vyos_udev_dir): - path = os.path.join(vyos_udev_dir, intf) - try: - with open(path) as f: - hwid = f.read().rstrip() - except OSError as e: - logger.error(f"OSError {e}") - continue - interfaces[intf] = hwid - # reverse sort to simplify insertion in config - interfaces = {key: value for key, value in sorted(interfaces.items(), - reverse=True)} - return interfaces - -def filter_interfaces(intfs: dict) -> dict: - """ Ignore no longer existing interfaces or non-persistent mac addresses - """ - filtered = {} - - for intf, hwid in intfs.items(): - if not os.path.isdir(os.path.join('/sys/class/net', intf)): - continue - if not is_persistent(hwid): - continue - filtered[intf] = hwid - - return filtered def interface_rescan(config_path: str): - """ Read new data and update config file - """ - interfaces = get_new_interfaces() - - logger.debug(f"interfaces from udev: {interfaces}") + """Add a node for every named interface which does not have one yet.""" + store = load_store() - interfaces = filter_interfaces(interfaces) + # a reserved name whose hardware is absent has nothing to create a node for + names = [name for name in sorted(store.get('interfaces', {})) + if Path(f'/sys/class/net/{name}').is_dir()] - logger.debug(f"filtered interfaces: {interfaces}") + logger.debug(f'named interfaces present: {names}') try: - with open(config_path) as f: - config_file = f.read() + config_file = Path(config_path).read_text() except OSError as e: - logger.critical(f"OSError {e}") + logger.critical(f'OSError {e}') exit(1) config = ConfigTree(config_file) + changed = False - for intf, hwid in interfaces.items(): - logger.info(f"Writing '{intf}' '{hwid}' to config file") + for intf in names: intf_type = get_interface_type(intf) if not intf_type: continue + if config.exists(['interfaces', intf_type, intf]): + continue + + logger.info(f"Adding '{intf}' to the configuration") if not config.exists(['interfaces', intf_type]): config.set(['interfaces', intf_type]) config.set_tag(['interfaces', intf_type]) - config.set(['interfaces', intf_type, intf, 'hw-id'], value=hwid) + config.set(['interfaces', intf_type, intf]) + changed = True if intf_type == 'wireless': phy = get_wireless_physical_device(intf) - if not phy: - continue - config.set(['interfaces', intf_type, intf, 'physical-device'], - value=phy) + if phy: + config.set(['interfaces', intf_type, intf, 'physical-device'], + value=phy) + + if not changed: + return try: - with open(config_path, 'w') as f: - f.write(config.to_string()) + Path(config_path).write_text(config.to_string()) except OSError as e: - logger.critical(f"OSError {e}") + logger.critical(f'OSError {e}') + exit(1) + def main(): global debug @@ -202,5 +140,6 @@ def main(): interface_rescan(configfile) + if __name__ == '__main__': main() diff --git a/src/init/vyos-router b/src/init/vyos-router index c0d12e2a7..ff1de0584 100755 --- a/src/init/vyos-router +++ b/src/init/vyos-router @@ -357,11 +357,39 @@ clear_or_override_config_files () update_interface_config () { - if [ -d /run/udev/vyos ]; then + # add a configuration node for any interface the name store knows about + # that does not have one yet + if [ -f /opt/vyatta/etc/config/interface-mapping.json ]; then $vyos_libexec_dir/vyos-interface-rescan.py $BOOTFILE 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[] | "\(.key) \(.value)"' "${status_file}" \ + 2>/dev/null | while read -r intf hwid; do + [ -n "${intf}" ] || continue + log_failure_msg "interface '${intf}' (${hwid}) did not appear during boot - check cabling, hardware and driver/firmware. Its name stays reserved." + done + + # A replacement NIC inheriting a slot's name also inherits that + # interface's addresses, so say so rather than doing it silently. + jq -r '.matched_by_path // {} | to_entries[] | "\(.key) \(.value)"' "${status_file}" \ + 2>/dev/null | while read -r intf hwid; do + [ -n "${intf}" ] || continue + log_warning_msg "interface '${intf}' was matched to new hardware ${hwid} by slot - verify it is the port you expect before relying on its configuration" + done + + if [ "$(jq -r '.persisted // true' "${status_file}" 2>/dev/null)" = "false" ]; then + log_failure_msg "interface names could not be persisted - the configuration volume is not mounted" + fi +} + cleanup_post_commit_hooks () { # Remove links from the post-commit hooks directory. # note that this approach only supports hooks that are "configured", @@ -606,6 +634,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_bootup_script pre run_preconfig_script @@ -613,6 +647,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 000000000..b7fe052e6 --- /dev/null +++ b/src/system/vyos-net-name-resolve.py @@ -0,0 +1,221 @@ +#!/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 . +# +# Boot-time maintenance of the interface mapping store (T3871). +# +# The store is rendered into systemd .link files, so udev does the actual +# naming as each device is added. What is left here is what udev cannot do: +# name hardware the store has never seen, record it, and re-render. +# +# Nothing here reads the configuration - a name is a property of the +# hardware, not of what happens to be configured on it. + +import json +import logging +import logging.handlers +import time +from pathlib import Path + +from vyos.ifconfig.ifname_store import device_matches +from vyos.ifconfig.ifname_store import discover_devices +from vyos.ifconfig.ifname_store import load_store +from vyos.ifconfig.ifname_store import resolve +from vyos.ifconfig.ifname_store import save_store +from vyos.ifconfig.ifname_store import sync_link_files +from vyos.system.image import is_running_as_container +from vyos.utils.process import run + +# bounded wait for known hardware to appear +HARDWARE_WAIT_TIMEOUT = 10 +HARDWARE_WAIT_POLL = 0.25 + +# unknown hardware has no specific device to wait for, so N identical +# snapshots stand in for "probing has finished" +HARDWARE_SETTLE_TIMEOUT = 10 +HARDWARE_SETTLE_STABLE_POLLS = 3 + +status_file = Path('/run/vyos-net-name-resolve.json') + +logger = logging.getLogger() + + +def _snapshot(devices: list) -> set: + return {(d['name'], d['mac']) for d in devices} + + +def _unmatched(keys: set, devices: list) -> set: + """Recorded slots that no device present this boot occupies.""" + return {key for key in keys + if not any(device_matches(d['properties'], key) for d in devices)} + + +def wait_for_hardware(known_keys: set, timeout: float = HARDWARE_WAIT_TIMEOUT, + poll: float = HARDWARE_WAIT_POLL) -> tuple: + """Poll until every slot the store knows is filled, or until timeout.""" + deadline = time.monotonic() + timeout + devices = discover_devices() + while True: + missing = _unmatched(known_keys, devices) + if not missing or time.monotonic() >= deadline: + return devices, missing + time.sleep(poll) + devices = discover_devices() + + +def wait_for_settle(initial: list, timeout: float = HARDWARE_SETTLE_TIMEOUT, + poll: float = HARDWARE_WAIT_POLL, + stable_polls: int = HARDWARE_SETTLE_STABLE_POLLS) -> list: + """Poll until the set of interfaces stops changing, or until timeout. + Used before naming hardware the store has never seen, where there is no + specific MAC to wait for. + """ + deadline = time.monotonic() + timeout + devices = initial + stable = 1 if devices else 0 + while stable < stable_polls and time.monotonic() < deadline: + time.sleep(poll) + seen = discover_devices() + stable = stable + 1 if _snapshot(seen) == _snapshot(devices) else 1 + devices = seen + return devices + + +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: + """Stage every interface under a scratch name derived from its ifindex + before assigning the final ones, so the batch cannot collide on a + permutation or a cycle. Only new hardware reaches this - udev has already + named everything the store knows. + """ + 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 under the scratch name - do not report it as renamed + del applied[old] + run(f'ip link set dev {tmp} up') + + return applied + + +def write_status(store: dict, devices: list, applied: dict, report: dict, + persisted: bool) -> None: + """Publish what this pass decided, to warn about and to inspect later.""" + absent = _unmatched(set(store['interfaces'].values()), devices) + status = { + 'interfaces': dict(sorted(store['interfaces'].items())), + 'missing': { + name: key for name, key in sorted(store['interfaces'].items()) + if key in absent + }, + 'renamed': applied, + 'matched': report.get('matched', {}), + 'matched_by_path': report.get('replaced', {}), + 'new_hardware': report.get('bootstrapped', {}), + 'persisted': persisted, + } + 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(): + if is_running_as_container(): + # a container owns no NIC - its veth pairs have no backing bus device + logger.info('running inside a container - skipping interface naming') + return + + store = load_store() + known = set(store['interfaces'].values()) + + devices, _missing = wait_for_hardware(known) + + if not store['interfaces'] or any( + not any(device_matches(d['properties'], k) for k in known) + for d in devices): + # unknown hardware present - let slower drivers finish registering + # so a merely late NIC is not treated as new + devices = wait_for_settle(devices) + + absent = _unmatched(known, devices) + for name, key in sorted(store['interfaces'].items()): + if key in absent: + logger.warning( + f"'{name}' ({key}) did not appear within " + f'{HARDWARE_WAIT_TIMEOUT}s - hardware may be missing, still ' + 'initializing, or its driver failed to load. Its name stays ' + 'reserved.' + ) + + plan, store, report = resolve(devices, store) + + for name, mac in sorted(report.get('bootstrapped', {}).items()): + logger.info(f"new hardware {mac} named '{name}'") + applied = safe_bulk_rename(plan) + + persisted = save_store(store) + if persisted: + # render so udev applies the names on the next boot + try: + sync_link_files(store) + except OSError as e: + logger.error(f'could not write .link files: {e}') + else: + logger.warning( + 'the configuration volume is not mounted - interface names were ' + 'resolved for this boot but could not be persisted' + ) + + write_status(store, devices, applied, report, persisted) + + +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 000000000..c3563dd84 --- /dev/null +++ b/src/systemd/vyos-net-name-resolve.service @@ -0,0 +1,15 @@ +[Unit] +Description=VyOS persistent interface naming +Documentation=man:vyos-net-name-resolve.py(8) + +# Started by vyos-router once the config directory is mounted - for an +# encrypted volume that is only after it has been unlocked - so this can not +# be ordered independently. +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 2c4421d0a..5a74525b7 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_ifname_store.py b/src/tests/test_ifname_store.py new file mode 100644 index 000000000..3f12594d4 --- /dev/null +++ b/src/tests/test_ifname_store.py @@ -0,0 +1,572 @@ +# 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: slot-anchored persistent interface naming. +# +# The regression classes at the bottom replay, verbatim, the field +# reproductions from the review of vyos-1x PR #5417 - the cases where the +# config-tree-driven resolver bound a configured address to the wrong +# physical wire. Under the store-based design none of them have a code path +# to travel, and these tests exist to keep it that way. + +import os +import shutil +import tempfile +import unittest +from pathlib import Path +from unittest import mock + +from vyos.ifconfig.ifname_store import STORE_VERSION +from vyos.ifconfig.ifname_store import canonical_sort_key +from vyos.ifconfig.ifname_store import device_matches +from vyos.ifconfig.ifname_store import ambiguous_keys +from vyos.ifconfig.ifname_store import hardware_key +from vyos.ifconfig.ifname_store import discover_devices +from vyos.ifconfig.ifname_store import empty_store +from vyos.ifconfig.ifname_store import load_store +from vyos.ifconfig.ifname_store import permanent_mac +from vyos.ifconfig.ifname_store import render_link +from vyos.ifconfig.ifname_store import resolve +from vyos.ifconfig.ifname_store import sync_link_files +from vyos.ifconfig.ifname_store import save_store + + +def dev(name, path, mac, wireless=False, bus='pci'): + """A device sitting at 'path'. Its key is what udev would report for it - + here the raw ID_PATH, which is what a plain virtio or e1000e NIC with no + firmware slot index ends up keyed on.""" + properties = {'ID_PATH': path, 'ID_BUS': bus} if path else {} + return {'name': name, 'mac': mac, 'wireless': wireless, + 'properties': properties, + 'key': hardware_key(properties)} + + +def store_of(mapping, hardware=None): + """mapping: {name: path} - the slot each name is recorded against""" + return { + 'version': STORE_VERSION, + 'interfaces': {n: f'ID_PATH={p}' for n, p in mapping.items()}, + 'hardware': dict(hardware or {}), + } + + +class TestCanonicalOrder(unittest.TestCase): + def test_pci_sorted_numerically_not_lexically(self): + # 0000:09:00.0 must precede 0000:10:00.0 - string order gets this wrong + a = dev('x', 'pci-0000:00:09.0', 'aa:00:00:00:00:01') + b = dev('y', 'pci-0000:00:10.0', 'aa:00:00:00:00:02') + self.assertLess(canonical_sort_key(a), canonical_sort_key(b)) + + def test_pci_before_usb(self): + pci = dev('x', 'pci-0000:00:1f.6', 'aa:00:00:00:00:01') + usb = dev('y', 'usb-0:1:1.0', 'aa:00:00:00:00:00', bus='usb') + self.assertLess(canonical_sort_key(pci), canonical_sort_key(usb)) + + def test_order_is_independent_of_mac(self): + # the whole point: a numerically higher MAC in a lower slot still wins + low_slot = dev('x', 'pci-0000:00:02.0', 'ff:ff:ff:ff:ff:ff') + high_slot = dev('y', 'pci-0000:00:05.0', '00:00:00:00:00:01') + self.assertLess(canonical_sort_key(low_slot), canonical_sort_key(high_slot)) + + +class TestReplacedHardware(unittest.TestCase): + """A slot keeps its name when its card is swapped - which hands that + interface's addresses to different hardware. resolve() reports the change + so vyos-router can warn instead of letting it happen silently. + """ + + PATH = 'pci-0000:00:12.0' + + def _device(self, mac): + return dev('eth0', self.PATH, mac) + + def test_same_card_reports_nothing(self): + store = store_of({'eth0': self.PATH}, {'eth0': 'aa:aa:aa:aa:aa:01'}) + _, _, report = resolve([self._device('aa:aa:aa:aa:aa:01')], store) + self.assertEqual(report['replaced'], {}) + + def test_swapped_card_is_reported_with_its_new_mac(self): + store = store_of({'eth0': self.PATH}, {'eth0': 'aa:aa:aa:aa:aa:01'}) + _, new_store, report = resolve([self._device('bb:bb:bb:bb:bb:02')], store) + self.assertEqual(report['replaced'], {'eth0': 'bb:bb:bb:bb:bb:02'}) + # the name is still the slot's, and the new card is now what is on record + self.assertEqual(new_store['interfaces']['eth0'], f'ID_PATH={self.PATH}') + self.assertEqual(new_store['hardware']['eth0'], 'bb:bb:bb:bb:bb:02') + + def test_reported_once_not_on_every_later_boot(self): + store = store_of({'eth0': self.PATH}, {'eth0': 'aa:aa:aa:aa:aa:01'}) + _, store, _ = resolve([self._device('bb:bb:bb:bb:bb:02')], store) + _, _, report = resolve([self._device('bb:bb:bb:bb:bb:02')], store) + self.assertEqual(report['replaced'], {}) + + def test_absent_card_keeps_its_record_so_a_later_swap_is_caught(self): + store = store_of({'eth0': self.PATH}, {'eth0': 'aa:aa:aa:aa:aa:01'}) + _, store, _ = resolve([], store) + self.assertEqual(store['hardware']['eth0'], 'aa:aa:aa:aa:aa:01') + _, _, report = resolve([self._device('bb:bb:bb:bb:bb:02')], store) + self.assertEqual(report['replaced'], {'eth0': 'bb:bb:bb:bb:bb:02'}) + + def test_new_hardware_is_not_a_replacement(self): + _, _, report = resolve([self._device('aa:aa:aa:aa:aa:01')], empty_store()) + self.assertEqual(report['replaced'], {}) + self.assertIn('eth0', report['bootstrapped']) + + def test_store_written_before_this_field_reports_nothing(self): + # nothing to compare against yet - and no interface may be renamed + legacy = {'version': STORE_VERSION, + 'interfaces': {'eth0': f'ID_PATH={self.PATH}'}} + plan, new_store, report = resolve([self._device('aa:aa:aa:aa:aa:01')], + legacy) + self.assertEqual(report['replaced'], {}) + self.assertEqual(plan, {}) + self.assertEqual(new_store['interfaces']['eth0'], f'ID_PATH={self.PATH}') + + +class TestStoreIO(unittest.TestCase): + @staticmethod + def _config_dir(d): + """A directory that looks like a properly mounted config volume.""" + (Path(d) / '.vyatta_config').touch() + return Path(d) / 'interface-mapping.json' + + def test_roundtrip(self): + with tempfile.TemporaryDirectory() as d: + p = self._config_dir(d) + s = store_of({'eth0': 'pci-0000:00:12.0'}) + self.assertTrue(save_store(s, p)) + self.assertEqual(load_store(p), s) + + def test_refuses_to_write_when_config_volume_is_not_mounted(self): + # TPM/LUKS unlock failed: vyos-router continues, but persisting here + # would write a shadow store that the real volume masks later + with tempfile.TemporaryDirectory() as d: + p = Path(d) / 'interface-mapping.json' # no .vyatta_config marker + self.assertFalse(save_store(store_of({'eth0': 'x'}), p)) + self.assertFalse(p.exists()) + + def test_missing_file_is_empty_store(self): + with tempfile.TemporaryDirectory() as d: + self.assertEqual(load_store(Path(d) / 'nope.json'), empty_store()) + + def test_corrupt_file_is_empty_store(self): + with tempfile.TemporaryDirectory() as d: + p = self._config_dir(d) + p.write_text('{ this is not json') + self.assertEqual(load_store(p), empty_store()) + + def test_write_is_atomic_no_leftovers(self): + with tempfile.TemporaryDirectory() as d: + p = self._config_dir(d) + save_store(store_of({'eth0': 'pci-0000:00:12.0'}), p) + self.assertEqual(sorted(f.name for f in Path(d).iterdir()), + ['.vyatta_config', 'interface-mapping.json']) + + +def final_names(report): + """Which name each MAC ended up with, read back off the resolve report.""" + out = {} + for bucket in report.values(): + for name, mac in bucket.items(): + out[mac] = name + return out + + +class TestDeviceMatches(unittest.TestCase): + """A stored key names the property it was taken from, so matching a + device is comparing that one property - never a guess about which + identifier was meant.""" + + def test_matches_the_named_property(self): + self.assertTrue(device_matches({'ID_NET_NAME_SLOT': 'ens18'}, + 'ID_NET_NAME_SLOT=ens18')) + + def test_same_value_under_a_different_property_does_not_match(self): + self.assertFalse(device_matches({'ID_NET_NAME_PATH': 'ens18'}, + 'ID_NET_NAME_SLOT=ens18')) + + def test_device_without_the_property_does_not_match(self): + self.assertFalse(device_matches({'ID_PATH': 'pci-0000:00:12.0'}, + 'ID_NET_NAME_SLOT=ens18')) + + def test_empty_or_malformed_key_never_matches(self): + self.assertFalse(device_matches({'ID_NET_NAME_SLOT': 'ens18'}, '')) + self.assertFalse(device_matches({'ID_NET_NAME_SLOT': 'ens18'}, 'ens18')) + + +class TestResolveBasics(unittest.TestCase): + def test_first_boot_bootstraps_in_slot_order(self): + devices = [ + dev('eth0', 'pci-0000:00:15.0', '00:00:5e:00:53:03'), + dev('eth1', 'pci-0000:00:12.0', '00:00:5e:00:53:00'), + dev('eth2', 'pci-0000:00:13.0', '00:00:5e:00:53:01'), + ] + _, new_store, _ = resolve(devices, empty_store()) + self.assertEqual(new_store['interfaces']['eth0'], 'ID_PATH=pci-0000:00:12.0') + self.assertEqual(new_store['interfaces']['eth1'], 'ID_PATH=pci-0000:00:13.0') + self.assertEqual(new_store['interfaces']['eth2'], 'ID_PATH=pci-0000:00:15.0') + + def test_established_names_are_stable_regardless_of_probe_order(self): + store = store_of({'eth0': 'pci-0000:00:12.0', 'eth1': 'pci-0000:00:13.0'}) + # drivers finished in the opposite order this boot + devices = [ + dev('eth0', 'pci-0000:00:13.0', '00:00:5e:00:53:01'), + dev('eth1', 'pci-0000:00:12.0', '00:00:5e:00:53:00'), + ] + plan, _, _ = resolve(devices, store) + self.assertEqual(plan, {'eth0': 'eth1', 'eth1': 'eth0'}) + + def test_replacement_nic_in_same_slot_inherits_name(self): + # the slot is the identity, so this needs no special rule: whatever + # card is in eth1's slot is eth1, along with its configuration + store = store_of({'eth1': 'pci-0000:00:13.0'}) + devices = [dev('eth0', 'pci-0000:00:13.0', 'bb:bb:bb:bb:bb:bb')] + plan, new_store, report = resolve(devices, store) + self.assertEqual(plan, {'eth0': 'eth1'}) + self.assertEqual(report['matched'], {'eth1': 'bb:bb:bb:bb:bb:bb'}) + self.assertEqual(new_store['interfaces']['eth1'], 'ID_PATH=pci-0000:00:13.0') + + def test_a_card_moved_to_another_slot_takes_that_slot_s_name(self): + # the consequence of slot identity, stated plainly: move a card and it + # becomes whatever the new slot is called, not what it used to be + store = store_of({'eth1': 'pci-0000:00:13.0', 'eth2': 'pci-0000:00:14.0'}) + devices = [dev('eth9', 'pci-0000:00:14.0', 'aa:aa:aa:aa:aa:aa')] + plan, _, report = resolve(devices, store) + self.assertEqual(plan, {'eth9': 'eth2'}) + self.assertEqual(final_names(report)['aa:aa:aa:aa:aa:aa'], 'eth2') + + def test_absent_hardware_keeps_its_name_reserved(self): + store = store_of({'eth0': 'pci-0000:00:12.0', 'eth1': 'pci-0000:00:13.0'}) + # eth0's slot is empty; a brand new card appeared elsewhere + devices = [ + dev('eth0', 'pci-0000:00:13.0', '00:00:5e:00:53:01'), + dev('eth1', 'pci-0000:00:1c.0', 'cc:cc:cc:cc:cc:cc'), + ] + _, new_store, report = resolve(devices, store) + # the new card must not inherit eth0's name, and with it eth0's config + self.assertNotEqual(final_names(report)['cc:cc:cc:cc:cc:cc'], 'eth0') + self.assertEqual(new_store['interfaces']['eth0'], 'ID_PATH=pci-0000:00:12.0') + + def test_hardware_with_no_usable_path_falls_back_to_its_mac(self): + # some paravirtual buses give udev nothing to build a path from; the + # MAC-derived name is the last step of the key chain so the interface + # is still stable instead of being renamed every boot + properties = {'ID_NET_NAME_MAC': 'enxaabbccddeeff'} + device = {'name': 'eth0', 'mac': 'aa:bb:cc:dd:ee:ff', 'wireless': False, + 'properties': properties, 'key': hardware_key(properties)} + _, new_store, _ = resolve([device], empty_store()) + self.assertEqual(new_store['interfaces']['eth0'], + 'ID_NET_NAME_MAC=enxaabbccddeeff') + + def test_a_path_two_devices_share_is_not_used_to_identify_either(self): + # a modem can put several interfaces on one parent device, and they + # then report the same path. Keying both on it would give them one + # name between them, so fall through to something which tells them + # apart. + first = {'ID_PATH': 'pci-0000:00:12.0', + 'ID_NET_NAME_MAC': 'enxaabbccddee01'} + second = {'ID_PATH': 'pci-0000:00:12.0', + 'ID_NET_NAME_MAC': 'enxaabbccddee02'} + ambiguous = ambiguous_keys([first, second]) + self.assertEqual(hardware_key(first, ambiguous), + 'ID_NET_NAME_MAC=enxaabbccddee01') + self.assertEqual(hardware_key(second, ambiguous), + 'ID_NET_NAME_MAC=enxaabbccddee02') + + def test_a_path_only_one_device_has_is_still_used(self): + first = {'ID_PATH': 'pci-0000:00:12.0'} + second = {'ID_PATH': 'pci-0000:00:13.0'} + ambiguous = ambiguous_keys([first, second]) + self.assertEqual(hardware_key(first, ambiguous), + 'ID_PATH=pci-0000:00:12.0') + + def test_firmware_slot_is_preferred_over_a_bare_pci_path(self): + # a firmware slot index survives PCI renumbering, a bus address does not + properties = {'ID_NET_NAME_SLOT': 'ens18', + 'ID_NET_NAME_PATH': 'enp0s18', + 'ID_PATH': 'pci-0000:00:12.0'} + self.assertEqual(hardware_key(properties), 'ID_NET_NAME_SLOT=ens18') + + def test_wireless_uses_its_own_namespace(self): + devices = [dev('eth0', 'pci-0000:00:12.0', 'aa:aa:aa:aa:aa:aa'), + dev('wlan5', 'pci-0000:00:14.0', 'cc:cc:cc:cc:cc:cc', + wireless=True)] + _, new_store, _ = resolve(devices, empty_store()) + self.assertIn('eth0', new_store['interfaces']) + self.assertIn('wlan0', new_store['interfaces']) + + +class TestPR5417Regressions(unittest.TestCase): + """Each test replays a reproduction from the review of PR #5417, where the + config-tree-driven resolver put a configured address on the wrong wire. + The assertions are about physical correctness - which MAC ends up behind + which name - not about log output. + """ + + def test_multi_nic_cloud_init_first_boot(self): + """2026-08-25 #1: three NICs, cloud-init synthesized a hw-id for eth0 + only. eth1/eth2 were pushed to eth3/eth4 and both data addresses were + unreachable, persisting across reboot. + """ + devices = [ + dev('eth0', 'pci-0000:00:03.0', '52:54:00:00:00:10'), + dev('eth1', 'pci-0000:00:04.0', '52:54:00:ff:00:21'), + dev('eth2', 'pci-0000:00:05.0', '52:54:00:00:00:22'), + ] + _, new_store, report = resolve(devices, empty_store()) + names = final_names(report) + self.assertEqual(names['52:54:00:00:00:10'], 'eth0') + self.assertEqual(names['52:54:00:ff:00:21'], 'eth1') + self.assertEqual(names['52:54:00:00:00:22'], 'eth2') + self.assertEqual(set(new_store['interfaces']), {'eth0', 'eth1', 'eth2'}) + + def test_names_follow_slots_not_mac_magnitude(self): + """2026-08-25 #2: the resolver swapped eth1/eth2 by MAC order, so each + address landed on the other's physical network, and the swap was + persisted with no warning. eth1 sits in the lower slot but has the + numerically HIGHER MAC. + """ + devices = [ + dev('eth1', 'pci-0000:00:04.0', '52:54:00:ff:00:21'), + dev('eth2', 'pci-0000:00:05.0', '52:54:00:00:00:22'), + ] + _, _, report = resolve(devices, empty_store()) + names = final_names(report) + self.assertEqual(names['52:54:00:ff:00:21'], 'eth0') + self.assertEqual(names['52:54:00:00:00:22'], 'eth1') + + def test_box_named_out_of_slot_order_is_not_reordered(self): + """2026-09-02 / 2026-09-09: a box first named by an earlier release in + MAC order - eth1 on the slot-5 NIC, eth4 on the slot-2 NIC. Deleting + hw-id entries made the resolver re-derive names in PCI slot order and + put eth1 and eth4 on each other's cable, silently and permanently. + + The store records the slot each name was actually on, so nothing is + ever re-derived and the box keeps the names it had. + """ + store = store_of({ + 'eth0': 'pci-0000:00:01.0', + 'eth1': 'pci-0000:00:05.0', + 'eth2': 'pci-0000:00:03.0', + 'eth3': 'pci-0000:00:04.0', + 'eth4': 'pci-0000:00:02.0', + }) + devices = [ + dev('eth0', 'pci-0000:00:01.0', '00:00:5e:00:54:01'), + dev('eth1', 'pci-0000:00:05.0', '00:00:5e:00:54:12'), + dev('eth2', 'pci-0000:00:03.0', '00:00:5e:00:54:13'), + dev('eth3', 'pci-0000:00:04.0', '00:00:5e:00:54:14'), + dev('eth4', 'pci-0000:00:02.0', '00:00:5e:00:54:19'), + ] + plan, _, report = resolve(devices, store) + self.assertEqual(plan, {}, 'nothing may be renamed') + names = final_names(report) + self.assertEqual(names['00:00:5e:00:54:12'], 'eth1') + self.assertEqual(names['00:00:5e:00:54:19'], 'eth4') + + def test_spare_nic_does_not_displace_configured_interfaces(self): + """2026-09-02 #2: a fourth, unconfigured NIC was enough to make the + three configured interfaces fail to get their hardware - eth1/eth2 + stayed bare while eth3/eth4/eth5 appeared holding the NICs. + """ + devices = [ + dev('eth0', 'pci-0000:00:03.0', '00:00:5e:00:54:31'), + dev('eth1', 'pci-0000:00:04.0', '00:00:5e:00:54:39'), + dev('eth2', 'pci-0000:00:05.0', '00:00:5e:00:54:32'), + dev('eth3', 'pci-0000:00:06.0', '00:00:5e:00:54:3c'), + ] + _, new_store, report = resolve(devices, empty_store()) + self.assertEqual(set(new_store['interfaces']), + {'eth0', 'eth1', 'eth2', 'eth3'}) + names = final_names(report) + self.assertEqual(names['00:00:5e:00:54:31'], 'eth0') + self.assertEqual(names['00:00:5e:00:54:39'], 'eth1') + self.assertEqual(names['00:00:5e:00:54:32'], 'eth2') + + def test_config_node_without_hardware_does_not_block_the_others(self): + """2026-09-09: eth0/eth1/eth2 configured with no hw-id on a two-NIC + VM. Every interface was left pending, both NICs parked on eth3/eth4, + and the box was unreachable from the network. + """ + devices = [ + dev('eth0', 'pci-0000:00:03.0', '00:00:5e:00:54:a1'), + dev('eth1', 'pci-0000:00:04.0', '00:00:5e:00:54:a9'), + ] + plan, new_store, report = resolve(devices, empty_store()) + # the two NICs present take eth0 and eth1; a configured eth2 node + # simply has no hardware, which is a reporting matter not a naming one + self.assertEqual(plan, {}) + names = final_names(report) + self.assertEqual(names['00:00:5e:00:54:a1'], 'eth0') + self.assertEqual(names['00:00:5e:00:54:a9'], 'eth1') + self.assertNotIn('eth3', new_store['interfaces']) + + +class TestLinkFiles(unittest.TestCase): + """The .link layer is what removes the rename window: udev applies the + name as the device is added, so no probe-order name is ever observable. + """ + + def test_matches_the_same_property_the_store_keyed_on(self): + # udev must apply exactly the mapping recorded in the store - if the + # .link matched on anything else the two could disagree about what + # identifies an interface + body = render_link('eth1', 'ID_NET_NAME_SLOT=ens18') + self.assertIn('Property=ID_NET_NAME_SLOT=ens18', body) + self.assertIn('Name=eth1', body) + self.assertIn('Type=ether', body) + + def test_link_for_a_radio_matches_the_wireless_type(self): + # a .link naming a type the device is not never applies, so a radio + # rendered as 'ether' would silently keep its probe-order name + body = render_link('wlan0', 'ID_PATH=pci-0000:00:14.0') + self.assertIn('Type=wlan', body) + self.assertNotIn('Type=ether', body) + self.assertIn('Name=wlan0', body) + + def test_radio_named_by_resolve_gets_a_wireless_link(self): + # end to end: resolve() names by type, and the file written for that + # name has to agree with it + devices = [dev('wlan5', 'pci-0000:00:14.0', 'aa:aa:aa:aa:aa:01', + wireless=True), + dev('eth9', 'pci-0000:00:12.0', 'bb:bb:bb:bb:bb:02')] + _, store, _ = resolve(devices, empty_store()) + self.assertIn('wlan0', store['interfaces']) + with tempfile.TemporaryDirectory() as d: + written = sync_link_files(store, Path(d)) + bodies = {p.name: p.read_text() for p in written} + self.assertIn('Type=wlan', bodies['10-vyos-wlan0.link']) + self.assertIn('Type=ether', bodies['10-vyos-eth0.link']) + + def test_one_file_per_interface(self): + with tempfile.TemporaryDirectory() as d: + written = sync_link_files( + store_of({'eth1': 'pci-0000:00:13.0'}), Path(d)) + self.assertEqual([p.name for p in written], ['10-vyos-eth1.link']) + + def test_stale_files_are_removed(self): + with tempfile.TemporaryDirectory() as d: + link_dir = Path(d) + sync_link_files(store_of({'eth1': 'p1', + 'eth2': 'p2'}), link_dir) + sync_link_files(store_of({'eth1': 'p1'}), link_dir) + remaining = sorted(p.name for p in link_dir.glob('*.link')) + self.assertEqual(remaining, ['10-vyos-eth1.link']) + + def test_operator_files_are_left_alone(self): + with tempfile.TemporaryDirectory() as d: + link_dir = Path(d) + custom = link_dir / '99-my-tuning.link' + custom.write_text('[Match]\nType=ether\n\n[Link]\nMTUBytes=9000\n') + sync_link_files(store_of({'eth1': 'p1'}), link_dir) + self.assertTrue(custom.exists()) + + +class TestPermanentMac(unittest.TestCase): + """Reading the hardware's own address matters wherever a custom MAC may + be in effect: the current address is then the custom one. This used to + come from the hw-id node, which could also simply be stale. + """ + + def test_ethtool_permanent_address(self): + with mock.patch('vyos.ifconfig.ifname_store.rc_cmd', + return_value=(0, 'Permanent address: aa:bb:cc:dd:ee:ff\n')): + self.assertEqual(permanent_mac('eth0'), 'aa:bb:cc:dd:ee:ff') + + def test_zero_address_falls_back_to_sysfs(self): + # some drivers report an all-zero "unsupported" permanent address + # instead of failing outright + with mock.patch('vyos.ifconfig.ifname_store.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(permanent_mac('eth0'), '11:22:33:44:55:66') + + def test_ethtool_unsupported_falls_back_to_sysfs(self): + with mock.patch('vyos.ifconfig.ifname_store.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(permanent_mac('eth0'), '11:22:33:44:55:66') + + def test_no_mac_available_returns_empty(self): + with mock.patch('vyos.ifconfig.ifname_store.rc_cmd', return_value=(1, '')), \ + mock.patch('pathlib.Path.read_text', side_effect=OSError): + self.assertEqual(permanent_mac('eth0'), '') + + +class TestDiscoverDevices(unittest.TestCase): + """Only real, independently-addressable hardware is a naming candidate. + Virtual interfaces have no bus device; enslaved interfaces, SR-IOV VFs + and hypervisor VF datapaths are acceleration children that share another + interface's MAC, so naming them would invent a second interface for one + physical port. + """ + + def setUp(self): + self.tmp = tempfile.mkdtemp() + self.addCleanup(shutil.rmtree, self.tmp, ignore_errors=True) + + for patch in (mock.patch('vyos.ifconfig.ifname_store.read_properties', + return_value={}), + mock.patch('vyos.ifconfig.ifname_store.rc_cmd', + return_value=(1, ''))): + patch.start() + self.addCleanup(patch.stop) + + def make(name, mac=None, has_device=True, enslaved=False, is_vf=False): + path = os.path.join(self.tmp, name) + os.mkdir(path) + if has_device: + os.mkdir(os.path.join(path, 'device')) + if is_vf: + pf = os.path.join(self.tmp, f'{name}_pf') + os.mkdir(pf) + os.symlink(pf, os.path.join(path, 'device', 'physfn')) + if enslaved: + master = os.path.join(self.tmp, f'{name}_master') + os.mkdir(master) + os.symlink(master, os.path.join(path, 'master')) + if mac: + with open(os.path.join(path, 'address'), 'w') as f: + f.write(mac + '\n') + + make('eth0', mac='aa:aa:aa:aa:aa:00') + make('eth1', mac='aa:aa:aa:aa:aa:01') + make('br0', mac='aa:aa:aa:aa:aa:99', has_device=False) + make('lo', has_device=False) + make('eth8', mac='aa:aa:aa:aa:aa:08', enslaved=True) + make('eth9', mac='aa:aa:aa:aa:aa:09', is_vf=True) + make('vf_eth0', mac='aa:aa:aa:aa:aa:00') + + def _names(self): + return {d['name'] for d in discover_devices(self.tmp)} + + def test_only_physical_independent_interfaces(self): + self.assertEqual(self._names(), {'eth0', 'eth1'}) + + def test_virtual_interfaces_excluded(self): + self.assertNotIn('br0', self._names()) + self.assertNotIn('lo', self._names()) + + def test_enslaved_interface_excluded(self): + self.assertNotIn('eth8', self._names()) + + def test_sriov_virtual_function_excluded(self): + # the general marker, independent of hypervisor - the previous + # exclusion relied on udev rules scoped to a Microsoft DMI vendor + # string and would miss a bare-metal Mellanox VF + self.assertNotIn('eth9', self._names()) + + def test_hypervisor_vf_datapath_excluded(self): + # parked as vf_* by 63-hyperv-vf-net.rules; shares its synthetic + # parent's MAC and must not be taken for a second interface + self.assertNotIn('vf_eth0', self._names()) diff --git a/src/tests/test_net_name_resolve.py b/src/tests/test_net_name_resolve.py new file mode 100644 index 000000000..e4473f49b --- /dev/null +++ b/src/tests/test_net_name_resolve.py @@ -0,0 +1,227 @@ +# 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: the boot-time half of persistent interface naming - +# settling, renaming and boot ordering. +# +# The naming decision and hardware discovery both live in +# vyos.ifconfig.ifname_store and are covered by test_ifname_store.py. + +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') + +_container_patcher = None + + +def setUpModule(): + # main() deliberately short-circuits inside a container, and these tests + # are routinely run from one (the VyOS build container). Pin the detection + # off for the whole module so the naming pass under test actually runs - + # TestMainInContainer patches it back on locally where that is the point. + global _container_patcher + _container_patcher = mock.patch.object( + resolver, 'is_running_as_container', return_value=False) + _container_patcher.start() + + +def tearDownModule(): + _container_patcher.stop() + + +def device(name, mac, path='', wireless=False): + return {'name': name, 'mac': mac, 'path': path, 'wireless': wireless, + 'properties': {}} + + +class TestWaitForSettle(unittest.TestCase): + """There is no specific MAC to wait for when naming hardware the store + has never seen, so settling is detected as N consecutive identical + discover_physical_interfaces() snapshots instead. + """ + + def test_returns_after_n_consecutive_stable_polls(self): + one = [device('eth0', 'm0')] + two = [device('eth0', 'm0'), device('eth1', 'm1')] + snapshots = [one, two, two, two] + with mock.patch.object(resolver, 'discover_devices', + side_effect=snapshots): + result = resolver.wait_for_settle([], timeout=5, poll=0, + stable_polls=3) + self.assertEqual(resolver._snapshot(result), + {('eth0', 'm0'), ('eth1', 'm1')}) + + def test_bounded_by_timeout_when_never_stable(self): + counter = iter(range(1000)) + + def ever_changing(*_a, **_kw): + return [device('eth0', f'm{next(counter)}')] + + clock = iter([0, 0.1, 20, 20]) + with mock.patch.object(resolver, 'discover_devices', + 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.assertEqual(len(result), 1) + + def test_stable_polls_of_one_returns_on_initial_snapshot(self): + initial = [device('eth0', 'm0')] + with mock.patch.object(resolver, 'discover_devices') as m: + result = resolver.wait_for_settle(initial, timeout=5, poll=0, + stable_polls=1) + m.assert_not_called() + self.assertEqual(result, initial) + + +class TestSafeBulkRename(unittest.TestCase): + """Only hardware the store has never seen should reach this on a settled + system, but when it does the two-phase rename must report accurately: 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): + 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) + + 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 TestMainInContainer(unittest.TestCase): + """A container owns no NIC: its interfaces are runtime-created veth pairs + with no backing bus device in sysfs and a host-assigned MAC that changes + on every start. There is nothing to name and nothing worth persisting. + """ + + 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 test_naming_pass_is_skipped_entirely(self): + with mock.patch.object(resolver, 'is_running_as_container', + return_value=True), \ + mock.patch.object(resolver, 'load_store') as load, \ + mock.patch.object(resolver, 'discover_devices') as discover, \ + mock.patch.object(resolver, 'save_store') as save, \ + mock.patch.object(resolver, 'run') as run: + resolver.main() + + load.assert_not_called() + discover.assert_not_called() + save.assert_not_called() + run.assert_not_called() + self.assertFalse(resolver.status_file.exists()) + + +class TestBootOrdering(unittest.TestCase): + """The store lives in the config directory, which does not exist until + vyos-router mounts (and, for an encrypted config volume, decrypts) it. So + the pass must be started explicitly from inside vyos-router after that + point, and must not be reintroduced as an independently-scheduled unit + ordered before it. + """ + + 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 current') + self.assertLess(resolve, update_iface, + 'resolver must write the store before the rescan reads it') + 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 deleted file mode 100755 index f5c3b9f1b..000000000 --- a/src/udev/vyos_net_name +++ /dev/null @@ -1,276 +0,0 @@ -#!/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 . - -import os -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' - - -def is_available(intfs: dict, intf_name: str) -> bool: - """Check if interface name is already assigned""" - if intf_name in list(intfs.values()): - return False - return True - - -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.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)}' - - -def mod_ifname(ifname: str) -> str: - """Check interface with names eX and return ifname on the next format eth{ifindex} - 2""" - if re.match('^e[0-9]+$', ifname): - intf = ifname.split('e') - if intf[1]: - if int(intf[1]) >= 2: - return 'eth' + str(int(intf[1]) - 2) - else: - return 'eth' + str(intf[1]) - - return ifname - - -def get_biosdevname(ifname: str) -> str: - """Use legacy vyatta-biosdevname to query for name - - This is carried over for compatibility only, and will likely be dropped - going forward. - XXX: This throws an error, and likely has for a long time, unnoticed - since vyatta_net_name redirected stderr to /dev/null. - """ - intf = mod_ifname(ifname) - - if 'eth' not in intf: - return intf - if os.path.isdir('/proc/xen'): - return intf - - time.sleep(1) - - try: - biosname = cmdl(['/sbin/biosdevname', '--policy', 'all_ethN', '-i', ifname]) - except Exception as e: - logger.error(f'biosdevname error: {e}') - biosname = '' - - return intf if biosname == '' else biosname - - -def leave_rescan_hint(intf_name: str, hwid: str): - """Write interface information reported by udev - - This script is called while the root mount is still read-only. Leave - information in /run/udev: file name, the interface; contents, the - hardware id. - """ - try: - os.mkdir(vyos_udev_dir) - except FileExistsError: - pass - except Exception as e: - logger.critical(f'Error creating rescan hint directory: {e}') - exit(1) - - try: - with open(os.path.join(vyos_udev_dir, intf_name), 'w') as f: - f.write(hwid) - except OSError as e: - 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): - return - - for intf in os.listdir(vyos_udev_dir): - path = os.path.join(vyos_udev_dir, intf) - try: - with open(path) as f: - hwid = f.read().rstrip() - except OSError as e: - logger.error(f'OSError {e}') - continue - intfs[hwid] = intf - - -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] - - add_assigned_interfaces(interfaces) - logger.debug(f'adding assigned interfaces: {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}'") - - leave_rescan_hint(newname, hwid) - - return newname - - -def hotplug_event(): - # Not yet implemented, since interface-rescan will only be run on boot. - pass - - -if __name__ == '__main__': - # Set up logging to syslog - syslog_handler = logging.handlers.SysLogHandler(address='/dev/log') - formatter = logging.Formatter(f'{Path(__file__).name}: %(message)s') - syslog_handler.setFormatter(formatter) - - logger = logging.getLogger() - logger.addHandler(syslog_handler) - logger.setLevel(logging.DEBUG) - - logger.debug(f'Started with arguments: {argv}') - - if len(argv) > 3: - predef_name = argv[3] - else: - predef_name = '' - - lock = Lock('vyos_net_name') - # Wait 60 seconds for other running scripts to finish - 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') - - lock.release() - logger.debug('Finished') -- cgit v1.2.3 From 5fdfcca2a97dabf3c6410a1362c0726142d2acc8 Mon Sep 17 00:00:00 2001 From: Christian Breunig Date: Wed, 7 Oct 2026 19:57:10 +0200 Subject: interfaces: T3871: never rename an interface which is in use Editing the map and then plugging any card in applied the edit to every interface at once, because the pass re-resolves the whole system whatever woke it. A running port was renamed out from under the configuration still naming it, a bridge member included since members became discoverable. Leave a name alone while the system is relying on it - the port is up, or it has a master. At boot that is true of nothing, as the pass runs before the configuration is applied, so naming is unaffected; afterwards it is true of everything configured, and a card which has just arrived is still down. Say in the log and the status file when a rename was refused, as the map and the running system then disagree until the next boot. --- src/system/vyos-net-name-resolve.py | 42 ++++++++++++++++++++++++++++-- src/tests/test_net_name_resolve.py | 51 +++++++++++++++++++++++++++++++++++++ 2 files changed, 91 insertions(+), 2 deletions(-) (limited to 'src/tests/test_net_name_resolve.py') diff --git a/src/system/vyos-net-name-resolve.py b/src/system/vyos-net-name-resolve.py index 43d2ed8d2..58966e291 100755 --- a/src/system/vyos-net-name-resolve.py +++ b/src/system/vyos-net-name-resolve.py @@ -100,6 +100,36 @@ def get_ifindex(name: str) -> str: return name +def interface_in_use(name: str, sys_class_net: str = '/sys/class/net') -> bool: + """Is the running system relying on this name? Renaming it then leaves + the configuration pointing at a name nothing answers to. + + At boot this is false for everything - the pass runs before the + configuration is applied, so the ports are still down. Afterwards, which + is the only time the hot-plug unit runs, it is true for every port the + configuration touched and for a card which just arrived it is not. + """ + try: + flags = int((Path(sys_class_net) / name / 'flags').read_text(), 16) + except (OSError, ValueError): + return False + IFF_UP = 0x1 + return (bool(flags & IFF_UP) + or (Path(sys_class_net) / name / 'master').exists()) + + +def refuse_renames_in_use(plan: dict, + sys_class_net: str = '/sys/class/net') -> tuple: + """Split a rename plan into what may be applied and what must not.""" + allowed, refused = {}, {} + for old, target in plan.items(): + if interface_in_use(old, sys_class_net): + refused[old] = target + else: + allowed[old] = target + return allowed, refused + + 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}') @@ -139,7 +169,7 @@ def safe_bulk_rename(plan: dict) -> dict: def write_status(store: dict, devices: list, applied: dict, report: dict, - persisted: bool) -> None: + persisted: bool, refused: dict = None) -> None: """Publish what this pass decided, to warn about and to inspect later.""" absent = _unmatched(set(store['interfaces'].values()), devices) status = { @@ -149,6 +179,8 @@ def write_status(store: dict, devices: list, applied: dict, report: dict, if key in absent }, 'renamed': applied, + # in use, so left alone until the next boot + 'rename_refused': refused or {}, # everything recognised from the store, however it was recognised 'matched': {**report.get('matched', {}), **report.get('reassigned', {}), @@ -210,6 +242,12 @@ def main(): f"'{name}' was matched to new hardware {mac} by slot - verify it " 'is the port you expect before relying on its configuration' ) + plan, refused = refuse_renames_in_use(plan) + for old, target in sorted(refused.items()): + logger.warning( + f"'{old}' is in use and was not renamed to '{target}' - the map " + 'and the running system disagree until the next boot' + ) applied = safe_bulk_rename(plan) persisted = save_store(store) @@ -225,7 +263,7 @@ def main(): 'resolved for this boot but could not be persisted' ) - write_status(store, devices, applied, report, persisted) + write_status(store, devices, applied, report, persisted, refused) if __name__ == '__main__': diff --git a/src/tests/test_net_name_resolve.py b/src/tests/test_net_name_resolve.py index e4473f49b..62236219a 100644 --- a/src/tests/test_net_name_resolve.py +++ b/src/tests/test_net_name_resolve.py @@ -149,6 +149,57 @@ class TestSafeBulkRename(unittest.TestCase): self.assertEqual(applied.get('eth9'), 'eth1') +class TestRefuseRenamesInUse(unittest.TestCase): + """A name the running system answers to must not move. The map is only + free to change a name while nothing is using it, which at boot is every + interface and afterwards is only a card which has just arrived. + """ + + def setUp(self): + self.tmp = tempfile.mkdtemp() + self.addCleanup(shutil.rmtree, self.tmp, ignore_errors=True) + + def _make(self, name, up=False, enslaved=False): + path = os.path.join(self.tmp, name) + os.mkdir(path) + # IFF_UP is bit 0; 0x1002 is a down interface with IFF_BROADCAST + with open(os.path.join(path, 'flags'), 'w') as f: + f.write('0x1003\n' if up else '0x1002\n') + if enslaved: + master = os.path.join(self.tmp, f'{name}_master') + os.mkdir(master) + os.symlink(master, os.path.join(path, 'master')) + + def _split(self, plan): + return resolver.refuse_renames_in_use(plan, self.tmp) + + def test_a_card_which_just_arrived_is_renamed(self): + self._make('eth8') + allowed, refused = self._split({'eth8': 'eth2'}) + self.assertEqual(allowed, {'eth8': 'eth2'}) + self.assertEqual(refused, {}) + + def test_an_interface_carrying_traffic_is_left_alone(self): + # what an edited map plus an unrelated hot-plug used to do: rename a + # running port out from under the configuration still naming it + self._make('eth1', up=True) + allowed, refused = self._split({'eth1': 'eth23'}) + self.assertEqual(allowed, {}) + self.assertEqual(refused, {'eth1': 'eth23'}) + + def test_a_bridge_member_is_left_alone(self): + self._make('eth1', enslaved=True) + allowed, refused = self._split({'eth1': 'eth23'}) + self.assertEqual(refused, {'eth1': 'eth23'}) + + def test_one_refusal_does_not_hold_up_the_rest(self): + self._make('eth1', up=True) + self._make('eth8') + allowed, refused = self._split({'eth1': 'eth23', 'eth8': 'eth2'}) + self.assertEqual(allowed, {'eth8': 'eth2'}) + self.assertEqual(refused, {'eth1': 'eth23'}) + + class TestMainInContainer(unittest.TestCase): """A container owns no NIC: its interfaces are runtime-created veth pairs with no backing bus device in sysfs and a host-assigned MAC that changes -- cgit v1.2.3 From 153dd688b327c922ec4b371db1fdd11b8ee8cf9e Mon Sep 17 00:00:00 2001 From: Alex Kudentsov <43482574+alexk37@users.noreply.github.com> Date: Wed, 14 Oct 2026 20:46:08 +0700 Subject: interfaces: T3871: give a re-created Hyper-V NIC the name of the NIC it replaced A Hyper-V NIC is known by its MAC alone. One deleted and created again on the same VM - an Azure NIC re-created, or a Hyper-V adapter removed and added - comes back with a new MAC, so nothing on it matched the store: its name stayed reserved, the new NIC got a new name, and its configuration was left on a name with no NIC. cloud-init does not run again on the same instance, so nothing else repairs it. At boot, a vacant name now goes to a new NIC only when the pairing is not in doubt: - exactly one recorded name has no NIC, and exactly one new Hyper-V NIC appeared. With more of either, the order the NICs come up in does not say which replaced which, so nothing is handed over and the names stay reserved, as before; - the name went missing at this pass. The store records which names were missing at the last pass, so a NIC removed for good does not hand its name to a NIC added at a later boot; - the name's own NIC was a Hyper-V one. The store records which names sat on a Hyper-V NIC; a MAC-keyed name can belong to other hardware, and a store written before this was recorded hands nothing over (the boot pass logs that when it holds a name back); - only at boot. On a running system the configuration under a name is not applied to the NIC that takes it, so the hotplug unit passes --hotplug, which names the new NIC like any other and records the vacant name as missing, so the next boot does not hand it over either. An adoption is logged as a warning, reported under 'adopted' in the status file (kept there across hotplug passes while it holds), and repeated by vyos-router, as a slot match is: verify the port before relying on its configuration. --- python/vyos/ifconfig/ifname_store.py | 89 +++++++++- src/init/vyos-router | 6 + src/system/vyos-net-name-resolve.py | 56 ++++++- src/systemd/vyos-net-name-resolve-hotplug.service | 2 +- src/systemd/vyos-net-name-resolve.service | 4 +- src/tests/test_ifname_store.py | 191 +++++++++++++++++++++- src/tests/test_net_name_resolve.py | 157 ++++++++++++++++++ 7 files changed, 487 insertions(+), 18 deletions(-) (limited to 'src/tests/test_net_name_resolve.py') diff --git a/python/vyos/ifconfig/ifname_store.py b/python/vyos/ifconfig/ifname_store.py index 20731aed1..961a81d15 100644 --- a/python/vyos/ifconfig/ifname_store.py +++ b/python/vyos/ifconfig/ifname_store.py @@ -302,8 +302,25 @@ def load_store(path: Path = None) -> dict: if isinstance(mac, str) and mac and name in interfaces } if isinstance(seen, dict) else {} - return {'version': STORE_VERSION, 'interfaces': interfaces, - 'hardware': hardware} + store = {'version': STORE_VERSION, 'interfaces': interfaces, 'hardware': hardware} + + # names whose hardware was missing at the last pass - kept only while + # there are any, so a store without them reads as it always did + absent = data.get('absent') + if isinstance(absent, list): + absent = sorted({n for n in absent if n in interfaces}) + if absent: + store['absent'] = absent + + # names whose NIC was a Hyper-V one when last seen - the only names a new + # Hyper-V NIC may take over. A store without them (written before they + # were recorded) has none. + hyperv = data.get('hyperv') + if isinstance(hyperv, list): + hyperv = sorted({n for n in hyperv if n in interfaces}) + if hyperv: + store['hyperv'] = hyperv + return store def encrypted_config_volume() -> bool: @@ -411,12 +428,15 @@ def _next_free(taken: set, prefix: str) -> str: return f'{prefix}{index}' -def resolve(devices: list, store: dict) -> tuple: +def resolve(devices: list, store: dict, adopt: bool = True) -> tuple: """Name every physical interface present this boot. Returns (plan, new_store, report); plan is {current: new} for the interfaces which have to be renamed. The slot decides the name - nothing here looks at the configuration, so there is nothing to guess. + + adopt=False keeps a vacant Hyper-V name from going to a new NIC, for a + pass on a running system (see the adoption block below). """ entries = store.get('interfaces', {}) seen = store.get('hardware', {}) @@ -436,13 +456,23 @@ def resolve(devices: list, store: dict) -> tuple: seen = {n: m for n, m in seen.items() if n not in misfiled} assigned = {} # final name -> device - report = {'matched': {}, 'reassigned': {}, 'moved': {}, - 'replaced': {}, 'bootstrapped': {}} + report = { + 'matched': {}, + 'reassigned': {}, + 'moved': {}, + 'replaced': {}, + 'adopted': {}, + 'bootstrapped': {}, + } # A recorded name stays off-limits even while its slot is empty, so new # hardware cannot inherit an absent interface's name and configuration. reserved = set(entries) remaining = list(devices) + # what the last pass recorded: names already missing then, and names + # whose NIC was a Hyper-V one + missing_before = set(store.get('absent', [])) + hyperv_before = set(store.get('hyperv', [])) def claim(device, name, bucket): assigned[name] = device @@ -500,6 +530,36 @@ def resolve(devices: list, store: dict) -> tuple: claim(device, name, 'replaced') break + # a Hyper-V NIC is known by its MAC alone, and one deleted and created + # again comes back with a new MAC, so nothing on it could match. When one + # recorded NIC is gone and one new NIC took its place the pairing is not in + # doubt, and the name goes to it. With more of either it is: the order the + # NICs come up in here does not say which replaced which, so nothing is + # handed over and the names stay reserved. + # Only on the pass the NIC first went missing: a replacement is one change + # to the VM. A name already missing at the last pass belongs to a NIC that + # was removed, and a NIC added later is not its replacement. + # Only at boot: on a running system the configuration under the name is + # not applied to the NIC that takes it, so the name would point at a port + # that is down and unaddressed until the next boot. The new NIC gets a new + # name there, and the vacant one is recorded as missing, so the next boot + # does not hand it over either. + # Only a name whose own NIC was a Hyper-V one: a MAC-keyed name can + # belong to other hardware too, and a store that does not say (written + # before this was recorded) hands nothing over. + vacant = [ + name + for name, key in entries.items() + if name not in assigned + and name not in missing_before + and name in hyperv_before + and key.startswith('ID_NET_NAME_MAC=') + ] + adoptable = [device for device in remaining if is_hyperv_nic(device)] + if (adopt and len(vacant) == 1 and len(adoptable) == 1 + and _name_fits(vacant[0], adoptable[0])): + claim(adoptable[0], vacant[0], 'adopted') + # unknown hardware: lowest free name of its type, in hardware order for device in sorted(remaining, key=canonical_sort_key): name = _next_free(reserved | set(assigned), _prefix_for(device)) @@ -521,8 +581,23 @@ def resolve(devices: list, store: dict) -> tuple: plan = {device['name']: name for name, device in assigned.items() if device['name'] != name} - return plan, {'version': STORE_VERSION, 'interfaces': new_interfaces, - 'hardware': new_hardware}, report + new_store = { + 'version': STORE_VERSION, + 'interfaces': new_interfaces, + 'hardware': new_hardware, + } + absent = sorted(name for name in new_interfaces if name not in assigned) + if absent: + new_store['absent'] = absent + # what each name's NIC was, kept for an absent one like its address + hyperv = sorted( + {name for name, device in assigned.items() if is_hyperv_nic(device)} + | (hyperv_before & set(absent)) + ) + if hyperv: + new_store['hyperv'] = hyperv + + return plan, new_store, report LINK_DIR = Path('/etc/systemd/network') diff --git a/src/init/vyos-router b/src/init/vyos-router index ff1de0584..8978d4157 100755 --- a/src/init/vyos-router +++ b/src/init/vyos-router @@ -385,6 +385,12 @@ warn_missing_interface_hardware () log_warning_msg "interface '${intf}' was matched to new hardware ${hwid} by slot - verify it is the port you expect before relying on its configuration" done + jq -r '.adopted // {} | to_entries[] | "\(.key) \(.value)"' "${status_file}" \ + 2>/dev/null | while read -r intf hwid; do + [ -n "${intf}" ] || continue + log_warning_msg "interface '${intf}' had no NIC this boot and was given to the new Hyper-V NIC ${hwid} - verify it is the port you expect before relying on its configuration" + done + if [ "$(jq -r '.persisted // true' "${status_file}" 2>/dev/null)" = "false" ]; then log_failure_msg "interface names could not be persisted - the configuration volume is not mounted" fi diff --git a/src/system/vyos-net-name-resolve.py b/src/system/vyos-net-name-resolve.py index d74663707..f3e4e62fb 100755 --- a/src/system/vyos-net-name-resolve.py +++ b/src/system/vyos-net-name-resolve.py @@ -26,6 +26,7 @@ import json import logging import logging.handlers +import sys import time from pathlib import Path @@ -182,16 +183,21 @@ def write_status(store: dict, devices: list, applied: dict, report: dict, # in use, so left alone until the next boot 'rename_refused': refused or {}, # everything recognised from the store, however it was recognised - 'matched': {**report.get('matched', {}), - **report.get('reassigned', {}), - **report.get('moved', {}), - **report.get('replaced', {})}, + 'matched': { + **report.get('matched', {}), + **report.get('reassigned', {}), + **report.get('moved', {}), + **report.get('replaced', {}), + **report.get('adopted', {}), + }, # the name changed hands between cards the store already knows 'reassigned': report.get('reassigned', {}), # the card was found in a different slot - certain, it is the same card 'moved': report.get('moved', {}), # a different card took the slot - the one case that needs verifying 'matched_by_path': report.get('replaced', {}), + # a recorded Hyper-V NIC was gone and its one replacement took its name + 'adopted': report.get('adopted', {}), 'new_hardware': report.get('bootstrapped', {}), 'persisted': persisted, } @@ -201,7 +207,25 @@ def write_status(store: dict, devices: list, applied: dict, report: dict, logger.error(f'could not write status file: {e}') -def main(): +def boot_adoptions(store: dict) -> dict: + """The adoptions the boot pass reported that still hold. A pass for + hot-plugged hardware adopts nothing, and must not wipe the record of + what this boot did.""" + try: + adopted = json.loads(status_file.read_text()).get('adopted') or {} + except (OSError, ValueError, AttributeError): + return {} + hardware = store.get('hardware', {}) + return {name: mac for name, mac in adopted.items() + if hardware.get(name) == mac} + + +def hotplug_requested(argv: list) -> bool: + """--hotplug: started for hardware added to a running system""" + return '--hotplug' in argv + + +def main(hotplug: bool = False): if is_running_as_container(): # a container owns no NIC - its veth pairs have no backing bus device logger.info('running inside a container - skipping interface naming') @@ -229,7 +253,16 @@ def main(): 'reserved.' ) - plan, store, report = resolve(devices, store) + if not hotplug and 'hyperv' not in store and any( + key.startswith('ID_NET_NAME_MAC=') and key in absent + for key in store['interfaces'].values()): + # a name is vacant, and the map was written by code that did not + # record which NICs were Hyper-V ones: the name is not handed over. + # Each pass records the Hyper-V NICs it finds, but never a vacant name + logger.info('the interface map does not say which interfaces were ' + 'Hyper-V NICs - no vacant name is given to a new NIC ' + 'this boot') + plan, store, report = resolve(devices, store, adopt=not hotplug) for name, mac in sorted(report.get('bootstrapped', {}).items()): logger.info(f"new hardware {mac} named '{name}'") @@ -242,6 +275,12 @@ def main(): f"'{name}' was matched to new hardware {mac} by slot - verify it " 'is the port you expect before relying on its configuration' ) + for name, mac in sorted(report.get('adopted', {}).items()): + logger.warning( + f"'{name}' had no NIC this boot and was given to the new Hyper-V " + f'NIC {mac}, which took its place - verify it is the port you ' + 'expect before relying on its configuration' + ) plan, refused = refuse_renames_in_use(plan) for old, target in sorted(refused.items()): logger.warning( @@ -263,6 +302,9 @@ def main(): 'resolved for this boot but could not be persisted' ) + if hotplug: + # reported once, by the boot pass; kept on record until the next boot + report = {**report, 'adopted': boot_adoptions(store)} write_status(store, devices, applied, report, persisted, refused) @@ -274,4 +316,4 @@ if __name__ == '__main__': logger.addHandler(syslog_handler) logger.setLevel(logging.DEBUG) - main() + main(hotplug=hotplug_requested(sys.argv[1:])) diff --git a/src/systemd/vyos-net-name-resolve-hotplug.service b/src/systemd/vyos-net-name-resolve-hotplug.service index c798feb99..d52fc6061 100644 --- a/src/systemd/vyos-net-name-resolve-hotplug.service +++ b/src/systemd/vyos-net-name-resolve-hotplug.service @@ -8,6 +8,6 @@ ConditionPathExists=/tmp/vyos-config-status [Service] Type=oneshot -ExecStart=/usr/libexec/vyos/system/vyos-net-name-resolve.py +ExecStart=/usr/libexec/vyos/system/vyos-net-name-resolve.py --hotplug TimeoutStartSec=90 StandardOutput=journal diff --git a/src/systemd/vyos-net-name-resolve.service b/src/systemd/vyos-net-name-resolve.service index b02d22cf7..8ad1c3418 100644 --- a/src/systemd/vyos-net-name-resolve.service +++ b/src/systemd/vyos-net-name-resolve.service @@ -4,8 +4,8 @@ Documentation=man:vyos-net-name-resolve.py(8) # Started by vyos-router once the config directory is mounted - for an # encrypted volume that is only after it has been unlocked - so this can not -# be ordered independently. Started again for hot-plugged hardware, hence no -# RemainAfterExit, which would make every later start a no-op. +# be ordered independently. This is the boot pass; hot-plugged hardware is +# named by vyos-net-name-resolve-hotplug.service, which passes --hotplug. DefaultDependencies=no [Service] diff --git a/src/tests/test_ifname_store.py b/src/tests/test_ifname_store.py index ff0f916ec..635033fff 100644 --- a/src/tests/test_ifname_store.py +++ b/src/tests/test_ifname_store.py @@ -199,7 +199,9 @@ def keyed(devices): class TestHyperV(unittest.TestCase): """Azure, four NICs, measured on a fresh deployment of this series: every Hyper-V NIC shares one ID_PATH and has no slot, so its MAC is its key. - The order Azure lists the NICs in is cloud-init's to record. + The order Azure lists the NICs in is cloud-init's to record; what is left + here is a NIC deleted and created again on the same VM, where cloud-init + does not run again. """ AZURE = [ @@ -231,6 +233,162 @@ class TestHyperV(unittest.TestCase): ), ) + def test_a_recreated_nic_takes_the_name_of_the_one_it_replaced(self): + # LAN3's NIC deleted and created again: new MAC. It came up as eth4, + # eth3 stayed reserved and the boot commit failed on it every boot. + store = self.stored() + devices = self.nics() + devices[3] = keyed([hv('eth3', '00:0d:3a:47:aa:01')] + devices[:3])[0] + plan, store, report = resolve(devices, store) + self.assertEqual(report.get('adopted', {}), {'eth3': '00:0d:3a:47:aa:01'}) + self.assertEqual(report['bootstrapped'], {}) + self.assertEqual(plan, {}) + self.assertEqual(store['interfaces']['eth3'], 'ID_NET_NAME_MAC=enx000d3a47aa01') + + def test_two_recreated_nics_are_not_paired_by_guess(self): + # nothing here says which new NIC replaced which, and pairing them by + # the order they came up in can swap two subnets' configuration + store = self.stored() + devices = self.nics()[:2] + keyed( + [hv('eth3', '00:0d:3a:99:00:11'), hv('eth2', '00:0d:3a:99:00:33')] + ) + _, store, report = resolve(devices, store) + self.assertEqual(report.get('adopted', {}), {}) + self.assertEqual(set(report['bootstrapped']), {'eth4', 'eth5'}) + self.assertIn('eth2', store['interfaces']) + self.assertIn('eth3', store['interfaces']) + + def test_two_vacancies_and_one_new_nic_are_not_paired(self): + # which of two gone NICs the one new NIC replaced is not known + store = self.stored() + devices = self.nics()[:2] + keyed( + [hv('eth3', '00:0d:3a:99:00:11')] + self.nics()[:2])[:1] + _, store, report = resolve(devices, store) + self.assertEqual(report.get('adopted', {}), {}) + self.assertEqual(report['bootstrapped'], {'eth4': '00:0d:3a:99:00:11'}) + self.assertEqual(store.get('absent'), ['eth2', 'eth3']) + + def test_a_new_nic_with_no_vacancy_takes_a_new_name(self): + store = self.stored() + devices = self.nics() + keyed([hv('eth4', '00:0d:3a:48:00:09')]) + _, _, report = resolve(devices, store) + self.assertEqual(report.get('adopted', {}), {}) + self.assertEqual(report['bootstrapped'], {'eth4': '00:0d:3a:48:00:09'}) + + def test_one_vacancy_and_two_new_nics_are_not_paired(self): + store = self.stored() + devices = self.nics()[:3] + keyed( + [hv('eth3', '00:0d:3a:48:00:01'), hv('eth4', '00:0d:3a:48:00:02')] + ) + _, _, report = resolve(devices, store) + self.assertEqual(report.get('adopted', {}), {}) + + def test_a_disk_on_all_new_nics_is_not_handed_over_here(self): + # every recorded NIC gone at once: the platform's order is cloud-init's + # to supply (it runs again on a new instance); guessing it here could + # put the WAN's configuration on a LAN + store = self.stored() + new = keyed([hv(f'eth{i}', f'00:0d:3a:48:00:0{i}') for i in range(4)]) + _, _, report = resolve(new, store) + self.assertEqual(report.get('adopted', {}), {}) + + def test_a_slot_keyed_name_is_never_adopted(self): + # a PCI card has a slot to come back to; its name stays reserved + devices = [dev('eth0', 'pci-0000:00:03.0', 'aa:00')] + _, store, _ = resolve(devices, empty_store()) + other = keyed([hv('eth1', '00:0d:3a:48:00:09')]) + _, store, report = resolve(other, store) + self.assertEqual(report.get('adopted', {}), {}) + self.assertEqual(report['bootstrapped'], {'eth1': '00:0d:3a:48:00:09'}) + + def test_a_nic_added_after_one_was_removed_is_not_its_replacement(self): + # P8, measured on Azure: LAN3's NIC deleted for good (one boot), a NIC + # on another subnet added later (the next boot). Taking the vacant + # name put LAN3's configuration on the other subnet. + store = self.stored() + devices = self.nics()[:3] + _, store, report = resolve(devices, store) + self.assertEqual(report.get('adopted', {}), {}) + later = devices + keyed([hv('eth4', '38:33:c5:cc:a3:3e')]) + _, store, report = resolve(later, store) + self.assertEqual(report.get('adopted', {}), {}) + self.assertEqual(report['bootstrapped'], {'eth4': '38:33:c5:cc:a3:3e'}) + self.assertEqual(store.get('absent'), ['eth3']) + + def test_a_nic_replaced_on_a_running_system_is_not_adopted(self): + # measured on Hyper-V: a NIC hot-added after another was hot-removed + # took its name at once, while the configuration under that name + # stayed unapplied until the next boot + store = self.stored() + devices = self.nics() + devices[3] = keyed([hv('eth3', '00:0d:3a:47:aa:01')] + devices[:3])[0] + _, store, report = resolve(devices, store, adopt=False) + self.assertEqual(report.get('adopted', {}), {}) + self.assertEqual(report['bootstrapped'], {'eth4': '00:0d:3a:47:aa:01'}) + self.assertEqual(store.get('absent'), ['eth3']) + # nor at the next boot: the name was already missing at the last pass + _, store, report = resolve(devices, store) + self.assertEqual(report.get('adopted', {}), {}) + self.assertEqual(report['matched'].get('eth4'), '00:0d:3a:47:aa:01') + + def test_a_name_whose_nic_was_not_hyperv_is_not_handed_over(self): + # a MAC-keyed name can belong to other hardware sharing the VMBus + # path; only a Hyper-V NIC's name goes to a Hyper-V replacement + other = hv('eth1', '00:0d:3a:47:3b:29') + other['properties']['ID_NET_DRIVER'] = 'mlx5_core' + devices = keyed([hv('eth0', '00:0d:3a:47:38:1d'), other]) + _, store, _ = resolve(devices, empty_store()) + self.assertEqual(store.get('hyperv'), ['eth0']) + later = keyed([hv('eth0', '00:0d:3a:47:38:1d'), + hv('eth1', '00:0d:3a:48:00:09')]) + _, _, report = resolve(later, store) + self.assertEqual(report.get('adopted', {}), {}) + self.assertEqual(report['bootstrapped'], {'eth2': '00:0d:3a:48:00:09'}) + + def test_only_a_hyperv_nic_takes_a_vacant_name(self): + store = self.stored() + new = hv('eth3', '00:0d:3a:48:00:09') + new['properties']['ID_NET_DRIVER'] = 'virtio_net' + devices = self.nics()[:3] + keyed([new] + self.nics()[:3])[:1] + _, _, report = resolve(devices, store) + self.assertEqual(report.get('adopted', {}), {}) + self.assertEqual(report['bootstrapped'], {'eth4': '00:0d:3a:48:00:09'}) + + def test_a_new_nic_takes_no_name_of_another_namespace(self): + store = self.stored() + new = keyed([hv('wlan0', '00:0d:3a:48:00:09')] + self.nics()[:3])[0] + new['wireless'] = True + _, _, report = resolve(self.nics()[:3] + [new], store) + self.assertEqual(report.get('adopted', {}), {}) + self.assertEqual(report['bootstrapped'], {'wlan0': '00:0d:3a:48:00:09'}) + + def test_a_store_that_does_not_say_which_nics_were_hyperv_hands_nothing_over(self): + # written by code that did not record it: whether the vacant name + # was a Hyper-V NIC's, or how long it has been vacant, is unknown + store = self.stored() + store.pop('hyperv') + devices = self.nics() + devices[3] = keyed([hv('eth3', '00:0d:3a:47:aa:01')] + devices[:3])[0] + _, store, report = resolve(devices, store) + self.assertEqual(report.get('adopted', {}), {}) + self.assertEqual(report['bootstrapped'], {'eth4': '00:0d:3a:47:aa:01'}) + self.assertEqual(store.get('absent'), ['eth3']) + + def test_the_hyperv_record_survives_a_load_and_follows_the_nics(self): + store = self.stored() + self.assertEqual(store['hyperv'], ['eth0', 'eth1', 'eth2', 'eth3']) + with tempfile.TemporaryDirectory() as d: + path = Path(d) / 'map.json' + with mock.patch('vyos.ifconfig.ifname_store.config_dir_is_mounted', + return_value=True), \ + mock.patch('vyos.ifconfig.ifname_store.get_cfg_group_id', + return_value=0): + save_store(store, path) + self.assertEqual(load_store(path)['hyperv'], store['hyperv']) + # an absent NIC keeps its record, so its replacement is recognised + _, store, _ = resolve(self.nics()[:3], store) + self.assertEqual(store['hyperv'], ['eth0', 'eth1', 'eth2', 'eth3']) + def test_a_lone_hyperv_nic_keyed_on_its_path_still_pins_its_driver(self): # one NIC has the VMBus path to itself, so it is keyed on it; its VF # must not match its file any more than with a MAC key @@ -242,6 +400,37 @@ class TestHyperV(unittest.TestCase): body = (Path(d) / '10-vyos-eth0.link').read_text() self.assertIn('Driver=hv_netvsc\n', body) + def test_a_store_without_the_record_still_adopts(self): + # written before names missing at a pass were recorded: nothing was + # missing then, as far as it can say, so a one-step replacement holds + store = self.stored() + store.pop('absent', None) + devices = self.nics() + devices[3] = keyed([hv('eth3', '00:0d:3a:47:aa:01')] + devices[:3])[0] + _, _, report = resolve(devices, store) + self.assertEqual(report.get('adopted', {}), {'eth3': '00:0d:3a:47:aa:01'}) + + def test_the_record_clears_when_the_nic_is_back(self): + store = self.stored() + _, store, _ = resolve(self.nics()[:3], store) + self.assertEqual(store.get('absent'), ['eth3']) + _, store, report = resolve(self.nics(), store) + self.assertNotIn('absent', store) + self.assertEqual(report['matched']['eth3'], self.AZURE[3][1]) + + def test_the_record_survives_a_load(self): + tmp = Path(tempfile.mkdtemp()) + self.addCleanup(shutil.rmtree, tmp) + path = tmp / 'interface-mapping.json' + store = self.stored() + _, store, _ = resolve(self.nics()[:3], store) + path.write_text(json.dumps(store)) + self.assertEqual(load_store(path).get('absent'), ['eth3']) + # a name the map no longer has is dropped from it + store['absent'] = ['eth3', 'eth9'] + path.write_text(json.dumps(store)) + self.assertEqual(load_store(path).get('absent'), ['eth3']) + def test_link_file_pins_the_driver_for_a_mac_key(self): # the accelerated-networking VF carries its NIC's MAC body = render_link('eth0', 'ID_NET_NAME_MAC=enx000d3a47381d', 'hv_netvsc') diff --git a/src/tests/test_net_name_resolve.py b/src/tests/test_net_name_resolve.py index 62236219a..b529c7164 100644 --- a/src/tests/test_net_name_resolve.py +++ b/src/tests/test_net_name_resolve.py @@ -18,10 +18,12 @@ # The naming decision and hardware discovery both live in # vyos.ifconfig.ifname_store and are covered by test_ifname_store.py. +import json import os import shutil import tempfile import unittest +from pathlib import Path from unittest import mock from helper import prepare_module @@ -200,6 +202,33 @@ class TestRefuseRenamesInUse(unittest.TestCase): self.assertEqual(refused, {'eth1': 'eth23'}) +class TestStatusAdopted(unittest.TestCase): + """A Hyper-V NIC re-created with a new MAC takes the vacant name. That is + neither a card found in its recorded slot nor anything the slot says, so + it is reported under its own key - vyos-router warns about it from there. + """ + + def test_an_adoption_is_reported_apart_from_slot_matches(self): + tmp = tempfile.mkdtemp() + self.addCleanup(shutil.rmtree, tmp) + status = os.path.join(tmp, 'status.json') + store = { + 'version': 1, + 'hardware': {}, + 'interfaces': {'eth3': 'ID_NET_NAME_MAC=enx000d3a47aa01'}, + } + report = {'adopted': {'eth3': '00:0d:3a:47:aa:01'}} + devices = [device('eth3', '00:0d:3a:47:aa:01')] + devices[0]['properties'] = {'ID_NET_NAME_MAC': 'enx000d3a47aa01'} + with mock.patch.object(resolver, 'status_file', Path(status)): + resolver.write_status(store, devices, {}, report, True) + with open(status) as f: + data = json.load(f) + self.assertEqual(data['adopted'], {'eth3': '00:0d:3a:47:aa:01'}) + self.assertEqual(data['matched_by_path'], {}) + self.assertIn('eth3', data['matched']) + + class TestMainInContainer(unittest.TestCase): """A container owns no NIC: its interfaces are runtime-created veth pairs with no backing bus device in sysfs and a host-assigned MAC that changes @@ -230,6 +259,134 @@ class TestMainInContainer(unittest.TestCase): self.assertFalse(resolver.status_file.exists()) +class TestHotplugPass(unittest.TestCase): + """A pass for hardware added to a running system hands no vacant name + over: the configuration under it would not be applied until the next boot. + """ + + def _unit(self, name): + with open(os.path.join(_here, '../systemd', name)) as f: + return f.read() + + def test_only_the_hotplug_unit_says_so(self): + self.assertIn('vyos-net-name-resolve.py --hotplug\n', + self._unit('vyos-net-name-resolve-hotplug.service')) + exec_start = [line for line in + self._unit('vyos-net-name-resolve.service').splitlines() + if line.startswith('ExecStart=')] + self.assertEqual(len(exec_start), 1) + self.assertNotIn('--hotplug', exec_start[0]) + + def _main(self, hotplug): + status_dir = tempfile.mkdtemp() + self.addCleanup(shutil.rmtree, status_dir, ignore_errors=True) + store = {'interfaces': {}, 'hardware': {}} + with mock.patch.object(resolver, 'is_running_as_container', + return_value=False), \ + mock.patch.object(resolver, 'status_file', + resolver.Path(status_dir) / 'status.json'), \ + mock.patch.object(resolver, 'load_store', return_value=store), \ + mock.patch.object(resolver, 'wait_for_hardware', + return_value=([], set())), \ + mock.patch.object(resolver, 'wait_for_settle', return_value=[]), \ + mock.patch.object(resolver, 'resolve', + return_value=({}, store, {})) as resolve, \ + mock.patch.object(resolver, 'save_store', return_value=False): + resolver.main(hotplug=hotplug) + return resolve.call_args + + def test_the_flag_is_read_from_the_command_line(self): + self.assertTrue(resolver.hotplug_requested(['--hotplug'])) + self.assertFalse(resolver.hotplug_requested([])) + + def test_a_hotplug_pass_keeps_the_boot_adoption_on_record(self): + # the boot pass gave eth2 to a new NIC; a later VF re-add starts a + # hotplug pass, which must neither forget nor repeat that + status_dir = tempfile.mkdtemp() + self.addCleanup(shutil.rmtree, status_dir, ignore_errors=True) + status = resolver.Path(status_dir) / 'status.json' + status.write_text(json.dumps({'adopted': {'eth2': '60:45:bd:00:00:01', + 'eth3': '60:45:bd:00:00:02'}})) + store = {'interfaces': {'eth2': 'ID_NET_NAME_MAC=enx6045bd000001', + 'eth3': 'ID_NET_NAME_MAC=enx6045bd000002'}, + 'hardware': {'eth2': '60:45:bd:00:00:01', + 'eth3': '60:45:bd:00:00:09'}} + with mock.patch.object(resolver, 'is_running_as_container', + return_value=False), \ + mock.patch.object(resolver, 'status_file', status), \ + mock.patch.object(resolver, 'load_store', return_value=store), \ + mock.patch.object(resolver, 'wait_for_hardware', + return_value=([], set())), \ + mock.patch.object(resolver, 'wait_for_settle', return_value=[]), \ + mock.patch.object(resolver, 'resolve', + return_value=({}, store, {})), \ + mock.patch.object(resolver, 'save_store', return_value=False), \ + mock.patch.object(resolver.logger, 'warning') as warning: + resolver.main(hotplug=True) + # eth3's NIC changed since: that record no longer holds + self.assertEqual(json.loads(status.read_text())['adopted'], + {'eth2': '60:45:bd:00:00:01'}) + self.assertFalse(any('had no NIC this boot' in str(c) + for c in warning.call_args_list)) + + def test_the_boot_pass_reports_only_its_own_adoptions(self): + # a status left from an earlier pass is not carried by a boot pass + status_dir = tempfile.mkdtemp() + self.addCleanup(shutil.rmtree, status_dir, ignore_errors=True) + status = resolver.Path(status_dir) / 'status.json' + status.write_text(json.dumps({'adopted': {'eth2': '60:45:bd:00:00:01'}})) + store = {'interfaces': {'eth2': 'ID_NET_NAME_MAC=enx6045bd000001'}, + 'hardware': {'eth2': '60:45:bd:00:00:01'}, 'hyperv': ['eth2']} + with mock.patch.object(resolver, 'is_running_as_container', + return_value=False), \ + mock.patch.object(resolver, 'status_file', status), \ + mock.patch.object(resolver, 'load_store', return_value=store), \ + mock.patch.object(resolver, 'wait_for_hardware', + return_value=([], set())), \ + mock.patch.object(resolver, 'wait_for_settle', return_value=[]), \ + mock.patch.object(resolver, 'resolve', + return_value=({}, store, {})), \ + mock.patch.object(resolver, 'save_store', return_value=False): + resolver.main(hotplug=False) + self.assertEqual(json.loads(status.read_text())['adopted'], {}) + + def _unrecorded_map_logs(self, present): + store = {'interfaces': {'eth0': 'ID_NET_NAME_MAC=enx000d3a000001'}, + 'hardware': {'eth0': '00:0d:3a:00:00:01'}} + devices = [{'properties': {'ID_NET_NAME_MAC': 'enx000d3a000001'}, + 'mac': '00:0d:3a:00:00:01', 'name': 'eth0'}] if present else [] + status_dir = tempfile.mkdtemp() + self.addCleanup(shutil.rmtree, status_dir, ignore_errors=True) + with mock.patch.object(resolver, 'is_running_as_container', + return_value=False), \ + mock.patch.object(resolver, 'status_file', + resolver.Path(status_dir) / 'status.json'), \ + mock.patch.object(resolver, 'load_store', return_value=store), \ + mock.patch.object(resolver, 'wait_for_hardware', + return_value=(devices, set())), \ + mock.patch.object(resolver, 'wait_for_settle', + return_value=devices), \ + mock.patch.object(resolver, 'resolve', + return_value=({}, store, {})), \ + mock.patch.object(resolver, 'save_store', return_value=False), \ + mock.patch.object(resolver.logger, 'info') as info: + resolver.main(hotplug=False) + return any('does not say which interfaces' in str(c) + for c in info.call_args_list) + + def test_an_unrecorded_map_is_mentioned_only_when_a_name_is_vacant(self): + # a fresh Azure deployment: cloud-init wrote the map, every NIC is + # there - nothing is held back, so nothing is said (5479.0.9 G1) + self.assertFalse(self._unrecorded_map_logs(present=True)) + self.assertTrue(self._unrecorded_map_logs(present=False)) + + def test_the_boot_pass_may_adopt(self): + self.assertTrue(self._main(False).kwargs['adopt']) + + def test_the_hotplug_pass_does_not(self): + self.assertFalse(self._main(True).kwargs['adopt']) + + class TestBootOrdering(unittest.TestCase): """The store lives in the config directory, which does not exist until vyos-router mounts (and, for an encrypted config volume, decrypts) it. So -- cgit v1.2.3