T3871: order bootstrap naming by PCIe distance from the root complex

compute_bootstrap_plan() sorted candidates purely by MAC address, which
is fully deterministic but has no relationship to physical topology: a
NIC wired directly to the CPU's PCIe root complex (e.g. an onboard LOM)
could end up sorted last just because its MAC happens to be numerically
higher than an add-in card several PCIe-switch hops away.

Add pcie_distance(): counts PCI domain:bus:device.function segments
(e.g. 0000:00:1f.6) in the fully-resolved sysfs path of an interface's
'device' symlink. Non-PCI hops in that path (virtioN, usbN, the
net/<ifname> tail) simply don't match the pattern and are skipped, so
virtio's device->virtioN->real-PCI-parent indirection (confirmed on a
real virtio_net host) needs no special-casing, and multi-function
siblings at the same slot aren't double-counted. Returns a sort-last
sentinel (PCIE_DISTANCE_UNKNOWN) if the device symlink is missing/
unresolvable or the path has no PCI segment at all (e.g. a USB NIC).

compute_bootstrap_plan()'s sort key becomes (pcie_distance, mac) instead
of mac alone - topology first, MAC only as a tie-break among NICs at the
same depth. Both are static hardware properties (slot wiring, permanent
address), so this stays a pure, deterministic function of the discovered
hardware - the core T3871 guarantee (same hardware, same names, every
boot) is unaffected.

Chose raw sysfs path parsing over pyudev's find_parent('pci') (both
viable; pyudev is already a project dependency used elsewhere) to keep
this boot-critical script's existing dependency-light style - stdlib
only, matching is_wireless_interface()'s sysfs-symlink-check pattern
right next to it.

Adds TestPcieDistance (shallow device, cascaded bridges, the virtio
indirection case, missing/broken device symlink, no-PCI-segment/USB
case) and extends TestComputeBootstrapPlan with distance-vs-MAC
ordering cases; existing tests default pcie_distance to a constant via
setUp() so they keep exercising pure MAC-rank ordering unmodified.
Verified the two new distance-ordering tests actually fail against the
prior MAC-only sort before confirming they pass with the fix.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Author: c-po

src/system/vyos-net-name-resolve.py

@@ -197,6 +197,38 @@ def is_wireless_interface(name: str, sys_class_net: str = '/sys/class/net') -> b
     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/<ifname> 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 = []
@@ -284,13 +316,18 @@ def compute_rename_plan(configured: dict, current: dict) -> dict:
 
 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 in ascending
-    MAC order within their type group (ethernet/wireless) instead of
-    leaving them at whatever name the racy udev-time fast path produced.
-    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, since the
-    name assigned here gets frozen into config.boot by
-    vyos-interface-rescan.py the same way a real hw-id match would.
+    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.
 
     existing_plan is the hw-id based plan already computed by
     compute_rename_plan(): its sources are excluded from bootstrap
@@ -313,7 +350,7 @@ def compute_bootstrap_plan(configured: dict, current: dict, existing_plan: dict)
         'wlan': highest_configured_index(configured, 'wlan') + 1,
     }
 
-    for mac, name in sorted(candidates):
+    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, floor[prefix])
         taken.add(new_name)

src/tests/test_net_name_resolve.py

@@ -249,11 +249,80 @@ def test_detection_independent_of_misleading_name(self):
         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/<if>/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 <tmp>/<name>/device as a symlink to a fake sysfs device
+        node at devices_root/<path_segments...>.
+        """
+        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):
-    """MAC-sorted 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 MAC rank, never
-    on whatever name the racy cosmetic fast-path happened to assign.
+    """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):
@@ -262,6 +331,11 @@ def setUp(self):
         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'}
@@ -328,6 +402,43 @@ def test_bootstrap_targets_never_collide_with_existing_plan_values(self):
                                                 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 TestSafeBulkRename(unittest.TestCase):
     """The two-phase rename must accurately report what actually happened -