From f40894e44e9887941dc6e5c8b82bcb54749a9cb8 Mon Sep 17 00:00:00 2001 From: Zixun LI Date: Mon, 14 Sep 2026 12:10:33 +0200 Subject: [PATCH 01/10] Fix and re-enable network HIL testing --- .../skills/hil-contract/scripts/hil_remote.py | 1 + test/hil/helper/hil_util.py | 2 +- test/hil/hil_test.py | 67 ++----- test/hil/net_test.py | 161 +++++++++++++++++ test/hil/requirements.txt | 5 +- test/hil/test/test_change_impact.py | 14 +- test/hil/test/test_hil_bounded.py | 2 +- test/hil/test/test_hil_net.py | 163 ++++++++++++++++++ 8 files changed, 349 insertions(+), 66 deletions(-) create mode 100644 test/hil/net_test.py create mode 100644 test/hil/test/test_hil_net.py diff --git a/.claude/skills/hil-contract/scripts/hil_remote.py b/.claude/skills/hil-contract/scripts/hil_remote.py index 8c6c70faf9..7c9af900b2 100644 --- a/.claude/skills/hil-contract/scripts/hil_remote.py +++ b/.claude/skills/hil-contract/scripts/hil_remote.py @@ -75,6 +75,7 @@ def helper(name): 'test/hil/pymtp.py', 'test/hil/mtp_test.py', 'test/hil/mtp_raw.py', + 'test/hil/net_test.py', 'test/hil/sysview_ci.py', # hil_test.py imports it; --sysview-out itself is refused here 'test/hil/helper/__init__.py', 'test/hil/helper/agentrc.py', diff --git a/test/hil/helper/hil_util.py b/test/hil/helper/hil_util.py index e46f7c06fa..55a66f1f53 100644 --- a/test/hil/helper/hil_util.py +++ b/test/hil/helper/hil_util.py @@ -37,7 +37,7 @@ 'device/midi_test', 'device/mtp', 'device/usbtest', # 0525:a4a0 (Gadget Zero, usbtest's static id); runs the Linux testusb tier-4 battery via usbtest.py - # 'device/net_lwip_webserver', # disabled for PR #3605: USB net iface enum is flaky on the CI HIL host + 'device/net_lwip_webserver', ] dual_tests = [ diff --git a/test/hil/hil_test.py b/test/hil/hil_test.py index 7fb53727eb..6cd390fbe0 100755 --- a/test/hil/hil_test.py +++ b/test/hil/hil_test.py @@ -23,12 +23,13 @@ # THE SOFTWARE. # Host setup (required: a missing tool fails its test rather than skipping it): -# - System packages: sudo apt install mtools libmtp9 libmtp-runtime alsa-utils iperf +# - System packages: sudo apt install mtools libmtp9 libmtp-runtime alsa-utils iproute2 # mtools read_disk_file (device/cdc_msc, device/msc_dual_lun) # libmtp9 pymtp ctypes load (device/mtp); Debian 13 uses libmtp9t64 # libmtp-runtime mtp-probe and the completed-device /dev/libmtp-* marker # alsa-utils arecord (device/audio_test_freertos) -# iperf throughput tests (device/net_lwip_*) +# iproute2 isolated USB network test (root or passwordless sudo) +# udev udevadm wait (systemd >= 251) for USB network initialization # openocd unified openocd from https://github.com/hathach/openocd (branch tinyusb) for wch, rp2040/rp2350, analog max32 # - device/usbtest: usbtest kernel module + testusb binary (kernel tools/usb/testusb.c) on PATH, # plus sudo for modprobe / sysfs writes @@ -1326,60 +1327,14 @@ def test_device_mtp(board): def test_device_net_lwip_webserver(board): - # MAC hard-coded in examples/device/net_lwip_webserver/src/main.c; Linux names the - # iface enx. Device IP 192.168.7.1, iperf2 TCP server on 5001 - # (INCLUDE_IPERF). - import socket - mac_no_colons = '0202846a9600' - iface = 'enx' + mac_no_colons - device_ip = '192.168.7.1' - iperf_port = 5001 - - # Wait for an IPv4 address in the device's subnet (it serves DHCP); 30s because USB - # enum + DHCP serve is slower on the CI HIL hardware than locally. - iface_timeout = 30 - deadline = time.monotonic() + iface_timeout - host_ip = None - while time.monotonic() < deadline: - ret = subprocess.run(['ip', '-o', '-4', 'addr', 'show', iface], - capture_output=True, text=True, timeout=2) - m = re.search(r'inet (192\.168\.7\.\d+)/', ret.stdout) if ret.returncode == 0 else None - if m: - host_ip = m.group(1) - break - time.sleep(0.5) - assert host_ip, f'USB net iface {iface} did not come up with 192.168.7.x within {iface_timeout}s' - - # Poll until the device accepts: the net stack and the iperf bind come up after DHCP. - deadline = time.monotonic() + enum_timeout() - last_err = None - while time.monotonic() < deadline: - try: - with socket.create_connection((device_ip, iperf_port), timeout=1): - last_err = None - break - except OSError as e: - last_err = e - time.sleep(0.3) - assert last_err is None, f'iperf TCP {device_ip}:{iperf_port} not accepting within {enum_timeout()}s: {last_err}' - - # 5-second iperf2 TCP test; -y C for stable parsing (final summary line is - # timestamp,src_ip,src_port,dst_ip,dst_port,id,interval,bytes,bps). - ret = hil_util.run_cmd(f'iperf -c {device_ip} -t 5 -y C', - timeout=30, split_stderr=True, quiet=True) - stderr = (ret.stderr or '').strip() - stdout = (ret.stdout or '').strip() - assert ret.returncode == 0, f'iperf rc={ret.returncode}: stderr={stderr!r} stdout={stdout!r}' - lines = [l for l in stdout.splitlines() if l] - assert lines, f'iperf produced no output (rc={ret.returncode}, stderr={stderr!r})' - try: - bps = int(lines[-1].split(',')[-1]) - except (ValueError, IndexError) as e: - raise AssertionError(f'could not parse iperf output: {lines[-1]!r} ({e})') - mbps = bps / 1e6 - print(f' iperf {mbps:5.1f} Mbps', end='') - - assert mbps >= 1.0, f'iperf throughput too low: {mbps:.2f} Mbps' + script = Path(__file__).resolve().parent / 'net_test.py' + cmd = [sys.executable, str(script), '--uid', board['uid']] + if os.geteuid() != 0: + cmd = ['sudo', '-n'] + cmd + ret = hil_util.run_cmd(cmd, timeout=110, split_stderr=True, quiet=True) + assert ret.returncode == 0, (f'USB network test failed (rc={ret.returncode}): ' + f'{ret.stdout} {ret.stderr}') + print(ret.stdout.strip(), end='') def test_device_msc_dual_lun(board): diff --git a/test/hil/net_test.py b/test/hil/net_test.py new file mode 100644 index 0000000000..428545cbba --- /dev/null +++ b/test/hil/net_test.py @@ -0,0 +1,161 @@ +#!/usr/bin/env python3 +# SPDX-License-Identifier: MIT +"""Exercise the webserver on one USB device, independent of host DHCP and routing. + +Invoked by hil_test.py while it holds the board lock. Only the USB network interface +whose parent has the requested serial is moved. Each board gets its own namespace: +the example deliberately shares its MAC and 192.168.7.1 with every other board. +Requires iproute2, udevadm wait, and root (or passwordless sudo). +No persistent host configuration. +""" +import argparse +from contextlib import contextmanager +import hashlib +import http.client +import json +import os +from pathlib import Path +import signal +import sys +import time +import uuid + +from helper import hil_util + +DEVICE_IP = '192.168.7.1' +# Default assets in the pinned lwIP dependency's src/apps/http/fsdata.c. Verify +# complete responses, including the binary image, rather than just TCP connect/200. +ASSETS = ( + ('/index.html', 1751, '1da17ad4638e314329a47a9176973ea59ff0b19fc498f55f01c19053c7816fcc'), + ('/img/sics.gif', 724, '3b4eb378fdefac1e7dd6dbb6c84db20e1512463457626f5d991bf7c1ccb7cf63'), +) + + +def command(argv, timeout=5): + ret = hil_util.run_cmd(argv, timeout=timeout, split_stderr=True, quiet=True) + if ret.returncode: + raise RuntimeError(f'{" ".join(argv)}: rc={ret.returncode}: {ret.stderr} {ret.stdout}') + return ret.stdout + + +def interfaces_for_uid(uid): + matches = [] + for dev in hil_util.usb_scan(vid='cafe', serial=uid): + matches.extend(p.name for p in Path(dev['dir']).glob('*:*/net/*')) + return sorted(set(matches)) + + +def wait_interface(uid, timeout=30): + deadline = time.monotonic() + timeout + matches = [] + detail = '' + while time.monotonic() < deadline: + matches = interfaces_for_uid(uid) + if len(matches) == 1: + # A sysfs node exists BEFORE udev finishes initializing/renaming it. + # Wait for this device only: a global settle would also wait for peers' + # mtp-probe jobs. Rediscover by serial if the name changed during wait. + ret = hil_util.run_cmd(['udevadm', 'wait', '--timeout=1', + '/sys/class/net/' + matches[0]], + timeout=2, split_stderr=True, quiet=True) + detail = ret.stderr or '' + if ret.returncode == 0 and interfaces_for_uid(uid) == matches: + return matches[0] + time.sleep(0.2) + raise RuntimeError(f'USB serial {uid}: expected one initialized network interface within {timeout}s; ' + f'found {matches}; {detail}') + + +@contextmanager +def network_namespace(iface): + ns = 'tusb-net-' + uuid.uuid4().hex[:12] + command(['ip', 'netns', 'add', ns]) + try: + command(['ip', 'link', 'set', 'dev', iface, 'netns', ns]) + # udev may have renamed it while it was still on the host. The new + # namespace contains only loopback and the interface we just moved. + links = json.loads(command(['ip', '-n', ns, '-j', 'link', 'show'])) + names = [link['ifname'] for link in links if link['ifname'] != 'lo'] + if len(names) != 1: + raise RuntimeError(f'{ns}: expected one network interface, found {names}') + iface = names[0] + # Remove any address a host network manager assigned before the move. + command(['ip', '-n', ns, 'addr', 'flush', 'dev', iface]) + command(['ip', '-n', ns, 'addr', 'add', '192.168.7.2/24', 'dev', iface]) + command(['ip', '-n', ns, 'link', 'set', 'dev', iface, 'up']) + yield ns + finally: + # Delete on success and failure. Once no process holds the namespace, its + # physical USB interface returns to the host (ip-netns(8)). + command(['ip', 'netns', 'delete', ns]) + + +def check_http(): + # HTTP is present on every SRAM tier; INCLUDE_IPERF is deliberately absent on + # smaller parts. Direct connections also ignore the host's HTTP proxy settings. + total = 0 + for _ in range(3): + for path, size, digest in ASSETS: + conn = http.client.HTTPConnection(DEVICE_IP, timeout=3) + try: + # Bringing the interface up triggers the class packet filter. Only + # the first connection may wait for that; data errors are failures. + deadline = time.monotonic() + (10 if total == 0 else 0) + while True: + try: + conn.connect() + break + except OSError: + conn.close() + if time.monotonic() >= deadline: + raise + time.sleep(0.5) + conn.request('GET', path, headers={'Connection': 'close'}) + response = conn.getresponse() + data = response.read(size + 1) + if response.status != 200 or len(data) != size or hashlib.sha256(data).hexdigest() != digest: + raise AssertionError(f'{path}: HTTP {response.status}, {len(data)} bytes; expected {size} bytes with SHA256 {digest}') + total += len(data) + finally: + conn.close() + print(f'HTTP: {3 * len(ASSETS)} responses verified, {total} bytes', flush=True) + + +def interrupted(signum, _frame): + raise TimeoutError(f'network test interrupted (signal {signum})') + + +def main(): + ap = argparse.ArgumentParser(description=__doc__) + mode = ap.add_mutually_exclusive_group(required=True) + mode.add_argument('--uid') + mode.add_argument('--http', action='store_true', help=argparse.SUPPRESS) + args = ap.parse_args() + # A root-owned helper must bound itself even if its unprivileged caller dies. + signal.signal(signal.SIGALRM, interrupted) + signal.signal(signal.SIGTERM, interrupted) + signal.alarm(75) + if args.http: + check_http() + else: + if os.geteuid() != 0: + raise RuntimeError('USB network setup requires root or passwordless sudo') + iface = wait_interface(args.uid) + print(f'USB serial {args.uid}: interface {iface}', flush=True) + with network_namespace(iface) as ns: + try: + output = command(['ip', 'netns', 'exec', ns, sys.executable, + str(Path(__file__).resolve()), '--http'], timeout=30) + except RuntimeError: + for view in (['-s', 'link'], ['addr'], ['neigh']): + try: + print(command(['ip', '-n', ns, *view, 'show']), file=sys.stderr) + except RuntimeError as diagnostic: + print(diagnostic, file=sys.stderr) + raise + print(output, end='', flush=True) + signal.alarm(0) + + +if __name__ == '__main__': + main() diff --git a/test/hil/requirements.txt b/test/hil/requirements.txt index e3e0bdcfb7..80a77f5c66 100644 --- a/test/hil/requirements.txt +++ b/test/hil/requirements.txt @@ -1,10 +1,11 @@ # System packages (install separately): -# sudo apt install mtools libmtp9 libmtp-runtime alsa-utils iperf +# sudo apt install mtools libmtp9 libmtp-runtime alsa-utils iproute2 # mtools - read_disk_file (device/cdc_msc, device/msc_dual_lun) # libmtp9 - pymtp ctypes load (device/mtp); Debian 13 uses libmtp9t64 # libmtp-runtime - mtp-probe and the completed-device /dev/libmtp-* marker # alsa-utils - arecord (device/audio_test_freertos) -# iperf - throughput tests (device/net_lwip_*) +# iproute2 - isolated USB network test (root or passwordless sudo) +# udev - udevadm wait (systemd >= 251) for USB network initialization hidapi pyserial esptool diff --git a/test/hil/test/test_change_impact.py b/test/hil/test/test_change_impact.py index b66ab8e8a0..a184cdfef4 100644 --- a/test/hil/test/test_change_impact.py +++ b/test/hil/test/test_change_impact.py @@ -1086,6 +1086,7 @@ def test_nothing_rig_affecting_has_moved_into_the_carve_out(self): 'test/hil/test/test_hil_flash.py', 'test/hil/test/test_hil_lock.py', 'test/hil/test/test_hil_mtp_raw.py', + 'test/hil/test/test_hil_net.py', 'test/hil/test/test_hil_pr_comment.py', 'test/hil/test/test_hil_pyocd.py', 'test/hil/test/test_hil_remote.py', @@ -1456,12 +1457,13 @@ def test_hil_lib_selects_the_using_tests(self): self.assertNotIn('stm32f407disco', s['boards']) self.assertNotIn('espressif_s3_devkitm', s['boards']) - def test_hil_lib_used_only_by_a_disabled_test_selects_nothing(self): - # device/net_lwip_webserver is commented out of hil_util.device_tests, so the - # intersection with the HIL universe is empty - s = sel(['lib/networking/dhserver.c']) - self.assertFalse(s['full']) - self.assertEqual(s['boards'], {}) + def test_hil_network_changes_select_webserver(self): + for path in ('lib/networking/dhserver.c', 'src/class/net/ncm_device.c'): + with self.subTest(path=path): + s = sel([path]) + self.assertFalse(s['full']) + self.assertEqual(set(s['boards']['stm32f407disco']), + {'device/net_lwip_webserver'}) def test_hil_lib_nobody_builds_selects_nothing(self): s = sel(['lib/SEGGER_RTT/RTT/SEGGER_RTT.c']) diff --git a/test/hil/test/test_hil_bounded.py b/test/hil/test/test_hil_bounded.py index af34a9d2f4..75ed3ed7e2 100644 --- a/test/hil/test/test_hil_bounded.py +++ b/test/hil/test/test_hil_bounded.py @@ -428,7 +428,7 @@ def imported_paths(pyfile): else: yield node.module.replace('.', '/') + '.py' - seeds = ['hil_test.py', 'usbtest.py', 'mtp_test.py'] # CLI + spawned helpers + seeds = ['hil_test.py', 'usbtest.py', 'mtp_test.py', 'net_test.py'] # CLI + spawned helpers for f in seeds: # a renamed seed must fail loudly, not fall out of the walk self.assertTrue((hil_dir / f).exists(), f'stale RemoteStaging seed: {f}') todo, seen = list(seeds), set() diff --git a/test/hil/test/test_hil_net.py b/test/hil/test/test_hil_net.py new file mode 100644 index 0000000000..d7dfc8df4e --- /dev/null +++ b/test/hil/test/test_hil_net.py @@ -0,0 +1,163 @@ +#!/usr/bin/env python3 +# SPDX-License-Identifier: MIT +"""Network regression coverage without USB hardware or privileged host changes.""" +import hashlib +import io +import json +import os +from pathlib import Path +import sys +from tempfile import TemporaryDirectory +import unittest +from unittest.mock import Mock, patch + +sys.path.insert(0, str(Path(__file__).resolve().parents[1])) +import net_test as net + + +class InterfaceSelection(unittest.TestCase): + @unittest.skipUnless(os.name == 'posix', 'sysfs interface names contain colons') + def test_interface_comes_from_usb_serial_not_shared_mac_or_name(self): + with TemporaryDirectory() as tmp: + target = Path(tmp) / '1-2' + other = Path(tmp) / '1-3' + (target / '1-2:1.0/net/usb7').mkdir(parents=True) + (other / '1-3:1.0/net/enx0202846a9600').mkdir(parents=True) + with patch.object(net.hil_util, 'usb_scan', return_value=[{'dir': str(target)}]) as scan: + self.assertEqual(net.interfaces_for_uid('ABC123'), ['usb7']) + scan.assert_called_once_with(vid='cafe', serial='ABC123') + + def test_waits_for_driver_interface(self): + with patch.object(net, 'interfaces_for_uid', side_effect=[[], ['usb7'], ['usb7']]), \ + patch.object(net.hil_util, 'run_cmd', return_value=Mock(returncode=0, stderr='')), \ + patch.object(net.time, 'sleep'): + self.assertEqual(net.wait_interface('ABC123'), 'usb7') + + def test_udev_rename_is_followed_by_serial(self): + with patch.object(net, 'interfaces_for_uid', + side_effect=[['usb0'], ['enx123'], ['enx123'], ['enx123']]), \ + patch.object(net.hil_util, 'run_cmd', return_value=Mock(returncode=0, stderr='')), \ + patch.object(net.time, 'sleep'): + self.assertEqual(net.wait_interface('ABC123'), 'enx123') + + def test_uninitialized_interface_is_not_moved(self): + with patch.object(net, 'interfaces_for_uid', return_value=['usb0']), \ + patch.object(net.hil_util, 'run_cmd', return_value=Mock(returncode=1, stderr='not ready')), \ + patch.object(net.time, 'monotonic', side_effect=[0, 0, 2]), \ + patch.object(net.time, 'sleep'): + with self.assertRaisesRegex(RuntimeError, 'not ready'): + net.wait_interface('ABC123', timeout=1) + + def test_ambiguous_serial_never_selects_arbitrary_board(self): + with patch.object(net, 'interfaces_for_uid', return_value=['usb0', 'usb1']), \ + patch.object(net.time, 'monotonic', side_effect=[0, 0, 2]), \ + patch.object(net.time, 'sleep'): + with self.assertRaisesRegex(RuntimeError, 'found.*usb0.*usb1'): + net.wait_interface('ABC123', timeout=1) + + +class NamespaceCleanup(unittest.TestCase): + def setUp(self): + self.calls = [] + self.failure = None + + def command(argv): + self.calls.append(argv) + if self.failure and self.failure in argv: + raise RuntimeError('injected failure') + if '-j' in argv: + return json.dumps([{'ifname': 'lo'}, {'ifname': 'renamed0'}]) + return '' + + p = patch.object(net, 'command', side_effect=command) + p.start() + self.addCleanup(p.stop) + + def test_namespace_deleted_after_data_failure_and_rename(self): + with self.assertRaisesRegex(AssertionError, 'bad data'): + with net.network_namespace('usb7') as ns: + self.assertIn(['ip', '-n', ns, 'addr', 'add', '192.168.7.2/24', + 'dev', 'renamed0'], self.calls) + raise AssertionError('bad data') + self.assertEqual(self.calls[-1], ['ip', 'netns', 'delete', ns]) + self.assertIn(['ip', 'link', 'set', 'dev', 'usb7', 'netns', ns], self.calls) + + def test_namespace_deleted_if_move_fails(self): + self.failure = 'usb7' + with self.assertRaisesRegex(RuntimeError, 'injected'): + with net.network_namespace('usb7'): + self.fail('move failure must abort setup') + self.assertEqual(self.calls[-1][:3], ['ip', 'netns', 'delete']) + + def test_failed_creation_does_not_delete_an_existing_namespace(self): + self.failure = 'add' + with self.assertRaises(RuntimeError): + with net.network_namespace('usb7'): + self.fail('creation failed') + self.assertEqual(len(self.calls), 1) + + def test_signal_unwinds_namespace(self): + with self.assertRaises(TimeoutError): + with net.network_namespace('usb7'): + net.interrupted(15, None) + self.assertEqual(self.calls[-1][:3], ['ip', 'netns', 'delete']) + + +class HttpVerification(unittest.TestCase): + def setUp(self): + self.payload = b'\x00\xffbinary\r\nasset' + self.response = Mock(status=200) + self.response.read.return_value = self.payload + self.conn = Mock() + self.conn.getresponse.return_value = self.response + for p in [patch.object(net, 'ASSETS', [('/fixture', len(self.payload), + hashlib.sha256(self.payload).hexdigest())]), + patch.object(net.http.client, 'HTTPConnection', return_value=self.conn), + patch('sys.stdout', new_callable=io.StringIO)]: + p.start() + self.addCleanup(p.stop) + + def test_repeated_complete_binary_responses(self): + net.check_http() + self.assertEqual(self.conn.request.call_count, 3) + self.assertEqual(self.conn.close.call_count, 3) + + def test_corruption_is_not_retried(self): + self.response.read.return_value = b'x' * len(self.payload) + with self.assertRaises(AssertionError): + net.check_http() + self.assertEqual(self.conn.request.call_count, 1) + self.conn.close.assert_called_once() + + def test_truncated_and_excess_data_fail(self): + for payload in (self.payload[:-1], self.payload + b'x'): + with self.subTest(payload=payload): + self.response.read.return_value = payload + with self.assertRaises(AssertionError): + net.check_http() + + def test_http_error_with_correct_body_fails(self): + self.response.status = 404 + with self.assertRaises(AssertionError): + net.check_http() + + def test_initial_connect_can_wait_for_link(self): + self.conn.connect.side_effect = [ConnectionRefusedError(), None, None, None] + with patch.object(net.time, 'sleep'): + net.check_http() + self.assertEqual(self.conn.request.call_count, 3) + + def test_later_connection_failure_is_not_hidden(self): + self.conn.connect.side_effect = [None, ConnectionRefusedError()] + with self.assertRaises(ConnectionRefusedError): + net.check_http() + self.assertEqual(self.conn.request.call_count, 1) + + @unittest.skipUnless(os.name == 'posix', 'process-group timeout is Linux HIL behavior') + def test_command_reaps_a_stalled_client(self): + with self.assertRaisesRegex(RuntimeError, 'rc=124'): + net.command([sys.executable, '-c', 'import time; time.sleep(60)'], timeout=0.1) + + +if __name__ == '__main__': + unittest.main() From 8f1bf51c3605d858d6575fa2a2a6885c9274da3e Mon Sep 17 00:00:00 2001 From: Zixun LI Date: Mon, 14 Sep 2026 12:20:04 +0200 Subject: [PATCH 02/10] Include udev in HIL setup commands --- test/hil/hil_test.py | 2 +- test/hil/requirements.txt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/hil/hil_test.py b/test/hil/hil_test.py index 6cd390fbe0..a0689dbd87 100755 --- a/test/hil/hil_test.py +++ b/test/hil/hil_test.py @@ -23,7 +23,7 @@ # THE SOFTWARE. # Host setup (required: a missing tool fails its test rather than skipping it): -# - System packages: sudo apt install mtools libmtp9 libmtp-runtime alsa-utils iproute2 +# - System packages: sudo apt install mtools libmtp9 libmtp-runtime alsa-utils iproute2 udev # mtools read_disk_file (device/cdc_msc, device/msc_dual_lun) # libmtp9 pymtp ctypes load (device/mtp); Debian 13 uses libmtp9t64 # libmtp-runtime mtp-probe and the completed-device /dev/libmtp-* marker diff --git a/test/hil/requirements.txt b/test/hil/requirements.txt index 80a77f5c66..db5d16e797 100644 --- a/test/hil/requirements.txt +++ b/test/hil/requirements.txt @@ -1,5 +1,5 @@ # System packages (install separately): -# sudo apt install mtools libmtp9 libmtp-runtime alsa-utils iproute2 +# sudo apt install mtools libmtp9 libmtp-runtime alsa-utils iproute2 udev # mtools - read_disk_file (device/cdc_msc, device/msc_dual_lun) # libmtp9 - pymtp ctypes load (device/mtp); Debian 13 uses libmtp9t64 # libmtp-runtime - mtp-probe and the completed-device /dev/libmtp-* marker From 6c46f5ccd759ec61b1d43a372a7ccf711c0d8821 Mon Sep 17 00:00:00 2001 From: Zixun LI Date: Mon, 14 Sep 2026 12:34:51 +0200 Subject: [PATCH 03/10] Keep network clients unprivileged and bind namespaces to their lifetime --- test/hil/hil_test.py | 5 +- test/hil/net_test.py | 103 ++++++++++++++++++++++------------ test/hil/requirements.txt | 3 +- test/hil/test/test_hil_net.py | 96 +++++++++++++++++++------------ 4 files changed, 132 insertions(+), 75 deletions(-) diff --git a/test/hil/hil_test.py b/test/hil/hil_test.py index a0689dbd87..b8622039f9 100755 --- a/test/hil/hil_test.py +++ b/test/hil/hil_test.py @@ -23,12 +23,13 @@ # THE SOFTWARE. # Host setup (required: a missing tool fails its test rather than skipping it): -# - System packages: sudo apt install mtools libmtp9 libmtp-runtime alsa-utils iproute2 udev +# - System packages: sudo apt install mtools libmtp9 libmtp-runtime alsa-utils iproute2 udev util-linux # mtools read_disk_file (device/cdc_msc, device/msc_dual_lun) # libmtp9 pymtp ctypes load (device/mtp); Debian 13 uses libmtp9t64 # libmtp-runtime mtp-probe and the completed-device /dev/libmtp-* marker # alsa-utils arecord (device/audio_test_freertos) # iproute2 isolated USB network test (root or passwordless sudo) +# util-linux unshare/nsenter for per-process network namespaces # udev udevadm wait (systemd >= 251) for USB network initialization # openocd unified openocd from https://github.com/hathach/openocd (branch tinyusb) for wch, rp2040/rp2350, analog max32 # - device/usbtest: usbtest kernel module + testusb binary (kernel tools/usb/testusb.c) on PATH, @@ -1329,8 +1330,6 @@ def test_device_mtp(board): def test_device_net_lwip_webserver(board): script = Path(__file__).resolve().parent / 'net_test.py' cmd = [sys.executable, str(script), '--uid', board['uid']] - if os.geteuid() != 0: - cmd = ['sudo', '-n'] + cmd ret = hil_util.run_cmd(cmd, timeout=110, split_stderr=True, quiet=True) assert ret.returncode == 0, (f'USB network test failed (rc={ret.returncode}): ' f'{ret.stdout} {ret.stderr}') diff --git a/test/hil/net_test.py b/test/hil/net_test.py index 428545cbba..3b0562a020 100644 --- a/test/hil/net_test.py +++ b/test/hil/net_test.py @@ -5,11 +5,10 @@ Invoked by hil_test.py while it holds the board lock. Only the USB network interface whose parent has the requested serial is moved. Each board gets its own namespace: the example deliberately shares its MAC and 192.168.7.1 with every other board. -Requires iproute2, udevadm wait, and root (or passwordless sudo). +Requires iproute2, util-linux, udevadm wait, and root (or passwordless sudo). No persistent host configuration. """ import argparse -from contextlib import contextmanager import hashlib import http.client import json @@ -18,7 +17,8 @@ import signal import sys import time -import uuid +import select +import subprocess from helper import hil_util @@ -66,28 +66,72 @@ def wait_interface(uid, timeout=30): f'found {matches}; {detail}') -@contextmanager -def network_namespace(iface): - ns = 'tusb-net-' + uuid.uuid4().hex[:12] - command(['ip', 'netns', 'add', ns]) +def privileged(argv): + return (['sudo', '-n'] if os.geteuid() != 0 else []) + argv + + +def stop_client(proc, pid): + # The unshare child runs as us, even when its sudo wrapper belongs to root. + # Terminate that child directly so sudo can reap it and return normally. + for sig in (signal.SIGTERM, signal.SIGKILL): + if pid is not None: + try: + os.kill(pid, sig) + except ProcessLookupError: + pass + try: + proc.communicate(timeout=5) + return + except subprocess.TimeoutExpired: + pass + hil_util._close_pipes(proc) + raise RuntimeError('network client did not exit after SIGKILL') + + +def check_device(uid): + iface = wait_interface(uid) + print(f'USB serial {uid}: interface {iface}', flush=True) + # Anonymous namespace: its last process exiting releases the USB interface, + # even on SIGKILL. unshare drops to the caller's UID/GID BEFORE executing any + # checkout-controlled Python. Only trusted system tools run with elevation. + argv = privileged(['unshare', '--net', '--setgid', str(os.getgid()), + '--setuid', str(os.getuid()), sys.executable, + str(Path(__file__).resolve()), '--http']) + # Keep the child in our process group: hil_test's interruption sweep can kill + # the whole group without stranding a detached namespace owner. + proc = subprocess.Popen(argv, stdin=subprocess.PIPE, stdout=subprocess.PIPE, + stderr=subprocess.PIPE, text=True) + pid = None try: - command(['ip', 'link', 'set', 'dev', iface, 'netns', ns]) - # udev may have renamed it while it was still on the host. The new - # namespace contains only loopback and the interface we just moved. - links = json.loads(command(['ip', '-n', ns, '-j', 'link', 'show'])) + if not select.select([proc.stdout], [], [], 5)[0]: + raise RuntimeError('network namespace client did not become ready') + ready = proc.stdout.readline(64).strip() + if not ready.isdecimal(): + raise RuntimeError(f'network namespace client failed to start: {ready!r}') + pid = int(ready) + ns = f'/proc/{pid}/ns/net' + command(privileged(['ip', 'link', 'set', 'dev', iface, 'netns', str(pid)])) + ip = privileged(['nsenter', '--net=' + ns, 'ip']) + links = json.loads(command(ip + ['-j', 'link', 'show'])) names = [link['ifname'] for link in links if link['ifname'] != 'lo'] if len(names) != 1: raise RuntimeError(f'{ns}: expected one network interface, found {names}') iface = names[0] - # Remove any address a host network manager assigned before the move. - command(['ip', '-n', ns, 'addr', 'flush', 'dev', iface]) - command(['ip', '-n', ns, 'addr', 'add', '192.168.7.2/24', 'dev', iface]) - command(['ip', '-n', ns, 'link', 'set', 'dev', iface, 'up']) - yield ns + command(ip + ['addr', 'flush', 'dev', iface]) + command(ip + ['addr', 'add', '192.168.7.2/24', 'dev', iface]) + command(ip + ['link', 'set', 'dev', iface, 'up']) + try: + stdout, stderr = proc.communicate(input='go\n', timeout=30) + except subprocess.TimeoutExpired as exc: + raise RuntimeError('network HTTP client timed out after 30s') from exc + if proc.returncode: + raise RuntimeError(f'network HTTP client failed (rc={proc.returncode}): {stdout} {stderr}') + print(stdout, end='', flush=True) finally: - # Delete on success and failure. Once no process holds the namespace, its - # physical USB interface returns to the host (ip-netns(8)). - command(['ip', 'netns', 'delete', ns]) + if proc.poll() is None: + stop_client(proc, pid) + else: + hil_util._close_pipes(proc) def check_http(): @@ -136,24 +180,13 @@ def main(): signal.signal(signal.SIGTERM, interrupted) signal.alarm(75) if args.http: + print(os.getpid(), flush=True) + if sys.stdin.readline().strip() != 'go': + raise RuntimeError('network setup owner exited before starting HTTP') + print(f'network client uid/gid: {os.getuid()}/{os.getgid()}', flush=True) check_http() else: - if os.geteuid() != 0: - raise RuntimeError('USB network setup requires root or passwordless sudo') - iface = wait_interface(args.uid) - print(f'USB serial {args.uid}: interface {iface}', flush=True) - with network_namespace(iface) as ns: - try: - output = command(['ip', 'netns', 'exec', ns, sys.executable, - str(Path(__file__).resolve()), '--http'], timeout=30) - except RuntimeError: - for view in (['-s', 'link'], ['addr'], ['neigh']): - try: - print(command(['ip', '-n', ns, *view, 'show']), file=sys.stderr) - except RuntimeError as diagnostic: - print(diagnostic, file=sys.stderr) - raise - print(output, end='', flush=True) + check_device(args.uid) signal.alarm(0) diff --git a/test/hil/requirements.txt b/test/hil/requirements.txt index db5d16e797..981375a442 100644 --- a/test/hil/requirements.txt +++ b/test/hil/requirements.txt @@ -1,10 +1,11 @@ # System packages (install separately): -# sudo apt install mtools libmtp9 libmtp-runtime alsa-utils iproute2 udev +# sudo apt install mtools libmtp9 libmtp-runtime alsa-utils iproute2 udev util-linux # mtools - read_disk_file (device/cdc_msc, device/msc_dual_lun) # libmtp9 - pymtp ctypes load (device/mtp); Debian 13 uses libmtp9t64 # libmtp-runtime - mtp-probe and the completed-device /dev/libmtp-* marker # alsa-utils - arecord (device/audio_test_freertos) # iproute2 - isolated USB network test (root or passwordless sudo) +# util-linux - unshare/nsenter for per-process network namespaces # udev - udevadm wait (systemd >= 251) for USB network initialization hidapi pyserial diff --git a/test/hil/test/test_hil_net.py b/test/hil/test/test_hil_net.py index d7dfc8df4e..87c1e38f8a 100644 --- a/test/hil/test/test_hil_net.py +++ b/test/hil/test/test_hil_net.py @@ -56,51 +56,75 @@ def test_ambiguous_serial_never_selects_arbitrary_board(self): net.wait_interface('ABC123', timeout=1) -class NamespaceCleanup(unittest.TestCase): +class NamespaceLifetime(unittest.TestCase): def setUp(self): self.calls = [] - self.failure = None + self.proc = Mock(returncode=0) + self.proc.stdout = io.StringIO('4321\n') + self.proc.poll.return_value = 0 + self.proc.communicate.return_value = ('HTTP verified\n', '') def command(argv): self.calls.append(argv) - if self.failure and self.failure in argv: - raise RuntimeError('injected failure') if '-j' in argv: return json.dumps([{'ifname': 'lo'}, {'ifname': 'renamed0'}]) return '' - p = patch.object(net, 'command', side_effect=command) - p.start() - self.addCleanup(p.stop) - - def test_namespace_deleted_after_data_failure_and_rename(self): - with self.assertRaisesRegex(AssertionError, 'bad data'): - with net.network_namespace('usb7') as ns: - self.assertIn(['ip', '-n', ns, 'addr', 'add', '192.168.7.2/24', - 'dev', 'renamed0'], self.calls) - raise AssertionError('bad data') - self.assertEqual(self.calls[-1], ['ip', 'netns', 'delete', ns]) - self.assertIn(['ip', 'link', 'set', 'dev', 'usb7', 'netns', ns], self.calls) - - def test_namespace_deleted_if_move_fails(self): - self.failure = 'usb7' - with self.assertRaisesRegex(RuntimeError, 'injected'): - with net.network_namespace('usb7'): - self.fail('move failure must abort setup') - self.assertEqual(self.calls[-1][:3], ['ip', 'netns', 'delete']) - - def test_failed_creation_does_not_delete_an_existing_namespace(self): - self.failure = 'add' - with self.assertRaises(RuntimeError): - with net.network_namespace('usb7'): - self.fail('creation failed') - self.assertEqual(len(self.calls), 1) - - def test_signal_unwinds_namespace(self): - with self.assertRaises(TimeoutError): - with net.network_namespace('usb7'): - net.interrupted(15, None) - self.assertEqual(self.calls[-1][:3], ['ip', 'netns', 'delete']) + for name, value in [('wait_interface', Mock(return_value='usb7')), + ('command', Mock(side_effect=command))]: + p = patch.object(net, name, value) + p.start() + self.addCleanup(p.stop) + for p in [patch.object(net.subprocess, 'Popen', return_value=self.proc), + patch.object(net.select, 'select', return_value=([self.proc.stdout], [], [])), + patch.object(net.os, 'geteuid', return_value=1000, create=True), + patch.object(net.os, 'getuid', return_value=1000, create=True), + patch.object(net.os, 'getgid', return_value=1000, create=True), + patch.object(net.signal, 'SIGKILL', 9, create=True), + patch('sys.stdout', new_callable=io.StringIO)]: + p.start() + self.addCleanup(p.stop) + + def test_python_runs_as_caller_in_anonymous_namespace(self): + net.check_device('ABC123') + argv = net.subprocess.Popen.call_args.args[0] + self.assertEqual(argv[:7], ['sudo', '-n', 'unshare', '--net', '--setgid', '1000', '--setuid']) + self.assertEqual(argv[7], '1000') + self.assertNotIn('start_new_session', net.subprocess.Popen.call_args.kwargs) + self.assertIn(['sudo', '-n', 'ip', 'link', 'set', 'dev', 'usb7', 'netns', '4321'], self.calls) + self.assertTrue(any('renamed0' in c for c in self.calls)) + self.assertFalse(any(c[:4] == ['sudo', '-n', 'ip', 'netns'] for c in self.calls)) + self.proc.communicate.assert_called_once_with(input='go\n', timeout=30) + + def test_interrupt_terminates_client_and_reaps_sudo(self): + net.command.side_effect = KeyboardInterrupt() + self.proc.poll.return_value = None + with patch.object(net.os, 'kill') as kill: + with self.assertRaises(KeyboardInterrupt): + net.check_device('ABC123') + kill.assert_called_once_with(4321, net.signal.SIGTERM) + self.proc.communicate.assert_called_once_with(timeout=5) + + def test_setup_failure_releases_namespace_owner(self): + net.command.side_effect = RuntimeError('move failed') + self.proc.poll.return_value = None + with patch.object(net.os, 'kill') as kill: + with self.assertRaisesRegex(RuntimeError, 'move failed'): + net.check_device('ABC123') + kill.assert_called_once_with(4321, net.signal.SIGTERM) + + def test_stalled_client_escalates_after_cleanup_grace(self): + self.proc.communicate.side_effect = [net.subprocess.TimeoutExpired('client', 5), ('', '')] + with patch.object(net.os, 'kill') as kill: + net.stop_client(self.proc, 4321) + self.assertEqual([c.args for c in kill.call_args_list], + [(4321, net.signal.SIGTERM), (4321, net.signal.SIGKILL)]) + + def test_http_failure_is_reported(self): + self.proc.returncode = 1 + self.proc.communicate.return_value = ('', 'corrupt data') + with self.assertRaisesRegex(RuntimeError, 'corrupt data'): + net.check_device('ABC123') class HttpVerification(unittest.TestCase): From bd6d96c9a338ea048edb294e1be923345e4b0ebb Mon Sep 17 00:00:00 2001 From: Zixun LI Date: Mon, 14 Sep 2026 14:05:39 +0200 Subject: [PATCH 04/10] Reap network clients that fail before reporting readiness --- test/hil/net_test.py | 21 +++++++++--- test/hil/test/test_hil_net.py | 61 ++++++++++++++++++++++++++++++++++- 2 files changed, 77 insertions(+), 5 deletions(-) diff --git a/test/hil/net_test.py b/test/hil/net_test.py index 3b0562a020..93c67654d5 100644 --- a/test/hil/net_test.py +++ b/test/hil/net_test.py @@ -20,7 +20,7 @@ import select import subprocess -from helper import hil_util +from helper import hil_health, hil_util DEVICE_IP = '192.168.7.1' # Default assets in the pinned lwIP dependency's src/apps/http/fsdata.c. Verify @@ -70,17 +70,30 @@ def privileged(argv): return (['sudo', '-n'] if os.geteuid() != 0 else []) + argv -def stop_client(proc, pid): +def stop_client(proc, pid, grace=5): # The unshare child runs as us, even when its sudo wrapper belongs to root. # Terminate that child directly so sudo can reap it and return normally. for sig in (signal.SIGTERM, signal.SIGKILL): - if pid is not None: + if pid is None: + # Startup can fail before Python reports its PID. Walk only this + # Popen child's descendants, leaves first, then signal the wrapper. + # Never killpg: these processes share the HIL caller's group. + children = hil_health.child_procs([proc.pid]).get(proc.pid, []) + for child, _pgid in reversed(children): + try: + os.kill(child, sig) + except (ProcessLookupError, PermissionError): + # sudo relays SIGTERM to a child still running as root before + # unshare drops privileges; the caller can signal sudo itself. + pass + proc.send_signal(sig) + else: try: os.kill(pid, sig) except ProcessLookupError: pass try: - proc.communicate(timeout=5) + proc.communicate(timeout=grace) return except subprocess.TimeoutExpired: pass diff --git a/test/hil/test/test_hil_net.py b/test/hil/test/test_hil_net.py index 87c1e38f8a..e2a82da45d 100644 --- a/test/hil/test/test_hil_net.py +++ b/test/hil/test/test_hil_net.py @@ -59,7 +59,7 @@ def test_ambiguous_serial_never_selects_arbitrary_board(self): class NamespaceLifetime(unittest.TestCase): def setUp(self): self.calls = [] - self.proc = Mock(returncode=0) + self.proc = Mock(returncode=0, pid=1234) self.proc.stdout = io.StringIO('4321\n') self.proc.poll.return_value = 0 self.proc.communicate.return_value = ('HTTP verified\n', '') @@ -126,6 +126,65 @@ def test_http_failure_is_reported(self): with self.assertRaisesRegex(RuntimeError, 'corrupt data'): net.check_device('ABC123') + def test_readiness_timeout_cleans_up_without_reported_pid(self): + net.select.select.return_value = ([], [], []) + self.proc.poll.return_value = None + with patch.object(net.hil_health, 'child_procs', return_value={1234: [(4321, 1)]}), \ + patch.object(net.os, 'kill') as kill: + with self.assertRaisesRegex(RuntimeError, 'did not become ready'): + net.check_device('ABC123') + net.command.assert_not_called() + kill.assert_called_once_with(4321, net.signal.SIGTERM) + self.proc.send_signal.assert_called_once_with(net.signal.SIGTERM) + self.proc.communicate.assert_called_once_with(timeout=5) + + def test_root_child_is_left_for_sudo_to_signal(self): + with patch.object(net.hil_health, 'child_procs', return_value={1234: [(4321, 1)]}), \ + patch.object(net.os, 'kill', side_effect=PermissionError()): + net.stop_client(self.proc, None) + self.proc.send_signal.assert_called_once_with(net.signal.SIGTERM) + self.proc.communicate.assert_called_once_with(timeout=5) + + +@unittest.skipUnless(os.name == 'posix', 'Linux process-tree cleanup') +class UnreportedClient(unittest.TestCase): + def test_live_wrapper_and_descendant_without_pid_are_reaped(self): + # Both ignore TERM so this also exercises the KILL pass. They inherit our + # process group; cleanup must not signal the test runner or its siblings. + code = '''import os, signal, subprocess, sys, time +signal.signal(signal.SIGTERM, signal.SIG_IGN) +child = subprocess.Popen([sys.executable, '-c', + 'import signal,time; signal.signal(signal.SIGTERM,signal.SIG_IGN); print("ready",flush=True); time.sleep(60)'], + stdout=subprocess.PIPE, text=True) +child.stdout.readline() +print(child.pid, flush=True) +time.sleep(60) +''' + proc = net.subprocess.Popen([sys.executable, '-c', code], stdout=net.subprocess.PIPE, + stderr=net.subprocess.PIPE, text=True) + child = None + try: + self.assertTrue(net.select.select([proc.stdout], [], [], 5)[0]) + # The test knows the descendant PID; stop_client deliberately does not. + child = int(proc.stdout.readline()) + with patch.object(net.os, 'killpg') as killpg: + net.stop_client(proc, None, grace=0.1) + killpg.assert_not_called() + self.assertIsNotNone(proc.poll()) + stat = Path('/proc') / str(child) / 'stat' + if stat.exists(): + self.assertEqual(stat.read_text().rsplit(')', 1)[1].split()[0], 'Z') + finally: + if proc.poll() is None: + proc.kill() + proc.wait(timeout=5) + if child is not None: + try: + os.kill(child, net.signal.SIGKILL) + except ProcessLookupError: + pass + net.hil_util._close_pipes(proc) + class HttpVerification(unittest.TestCase): def setUp(self): From 689ab537a1e3c96c8beeafd546cb97071e80e23c Mon Sep 17 00:00:00 2001 From: HiFiPhile Date: Sun, 8 Nov 2026 12:20:30 +0100 Subject: [PATCH 05/10] net_test: follow master's HIL helper moves hil_health (#4018) and hil_util._close_pipes (#4255) are gone, and usb_scan now lives in agentrc's usb_observe (#4245). Walk /proc for the unreported client's descendants and close its pipes locally; scan through usb_observe as hil_test.py does. --- test/hil/net_test.py | 42 ++++++++++++++++++++++++++++++----- test/hil/test/test_hil_net.py | 8 +++---- 2 files changed, 40 insertions(+), 10 deletions(-) diff --git a/test/hil/net_test.py b/test/hil/net_test.py index 93c67654d5..facf4c85df 100644 --- a/test/hil/net_test.py +++ b/test/hil/net_test.py @@ -20,7 +20,7 @@ import select import subprocess -from helper import hil_health, hil_util +from helper import hil_util, usb_observe DEVICE_IP = '192.168.7.1' # Default assets in the pinned lwIP dependency's src/apps/http/fsdata.c. Verify @@ -40,7 +40,7 @@ def command(argv, timeout=5): def interfaces_for_uid(uid): matches = [] - for dev in hil_util.usb_scan(vid='cafe', serial=uid): + for dev in usb_observe.usb_scan(vid='cafe', serial=uid): matches.extend(p.name for p in Path(dev['dir']).glob('*:*/net/*')) return sorted(set(matches)) @@ -66,6 +66,37 @@ def wait_interface(uid, timeout=30): f'found {matches}; {detail}') +def descendants(root): + """[pid, ...] of every descendant of `root`, parents before children, from one /proc walk.""" + by_parent = {} + for entry in Path('/proc').iterdir(): + if not entry.name.isdigit(): + continue + try: + stat = (entry / 'stat').read_bytes() + # comm (field 2) may contain spaces and ')': split only after the last ')' + ppid = int(stat[stat.rindex(b')') + 2:].split()[1]) + except (OSError, ValueError, IndexError): + continue # exited mid-walk, unreadable or truncated + by_parent.setdefault(ppid, []).append(int(entry.name)) + out, todo = [], list(by_parent.get(root, [])) + while todo: + pid = todo.pop(0) + out.append(pid) + todo += by_parent.get(pid, []) + return out + + +def close_pipes(proc): + """Close our ends of a child's pipes. Never raises.""" + for pipe in (proc.stdout, proc.stderr, proc.stdin): + try: + if pipe is not None: + pipe.close() + except OSError: + pass + + def privileged(argv): return (['sudo', '-n'] if os.geteuid() != 0 else []) + argv @@ -78,8 +109,7 @@ def stop_client(proc, pid, grace=5): # Startup can fail before Python reports its PID. Walk only this # Popen child's descendants, leaves first, then signal the wrapper. # Never killpg: these processes share the HIL caller's group. - children = hil_health.child_procs([proc.pid]).get(proc.pid, []) - for child, _pgid in reversed(children): + for child in reversed(descendants(proc.pid)): try: os.kill(child, sig) except (ProcessLookupError, PermissionError): @@ -97,7 +127,7 @@ def stop_client(proc, pid, grace=5): return except subprocess.TimeoutExpired: pass - hil_util._close_pipes(proc) + close_pipes(proc) raise RuntimeError('network client did not exit after SIGKILL') @@ -144,7 +174,7 @@ def check_device(uid): if proc.poll() is None: stop_client(proc, pid) else: - hil_util._close_pipes(proc) + close_pipes(proc) def check_http(): diff --git a/test/hil/test/test_hil_net.py b/test/hil/test/test_hil_net.py index e2a82da45d..ec1575babc 100644 --- a/test/hil/test/test_hil_net.py +++ b/test/hil/test/test_hil_net.py @@ -23,7 +23,7 @@ def test_interface_comes_from_usb_serial_not_shared_mac_or_name(self): other = Path(tmp) / '1-3' (target / '1-2:1.0/net/usb7').mkdir(parents=True) (other / '1-3:1.0/net/enx0202846a9600').mkdir(parents=True) - with patch.object(net.hil_util, 'usb_scan', return_value=[{'dir': str(target)}]) as scan: + with patch.object(net.usb_observe, 'usb_scan', return_value=[{'dir': str(target)}]) as scan: self.assertEqual(net.interfaces_for_uid('ABC123'), ['usb7']) scan.assert_called_once_with(vid='cafe', serial='ABC123') @@ -129,7 +129,7 @@ def test_http_failure_is_reported(self): def test_readiness_timeout_cleans_up_without_reported_pid(self): net.select.select.return_value = ([], [], []) self.proc.poll.return_value = None - with patch.object(net.hil_health, 'child_procs', return_value={1234: [(4321, 1)]}), \ + with patch.object(net, 'descendants', return_value=[4321]), \ patch.object(net.os, 'kill') as kill: with self.assertRaisesRegex(RuntimeError, 'did not become ready'): net.check_device('ABC123') @@ -139,7 +139,7 @@ def test_readiness_timeout_cleans_up_without_reported_pid(self): self.proc.communicate.assert_called_once_with(timeout=5) def test_root_child_is_left_for_sudo_to_signal(self): - with patch.object(net.hil_health, 'child_procs', return_value={1234: [(4321, 1)]}), \ + with patch.object(net, 'descendants', return_value=[4321]), \ patch.object(net.os, 'kill', side_effect=PermissionError()): net.stop_client(self.proc, None) self.proc.send_signal.assert_called_once_with(net.signal.SIGTERM) @@ -183,7 +183,7 @@ def test_live_wrapper_and_descendant_without_pid_are_reaped(self): os.kill(child, net.signal.SIGKILL) except ProcessLookupError: pass - net.hil_util._close_pipes(proc) + net.close_pipes(proc) class HttpVerification(unittest.TestCase): From 3cd57b44d53dc893293fc0e633e3cef35a67f56f Mon Sep 17 00:00:00 2001 From: HiFiPhile Date: Sun, 8 Nov 2026 12:30:49 +0100 Subject: [PATCH 06/10] test_hil_net: accept a grandchild already reaped or being reaped Once the wrapper dies, init may reap the killed grandchild between the exists() check and the stat read (ProcessLookupError), or the read may see state X. Both mean it is dead; under 8-way parallel load 13/40 runs failed, 0/240 after. --- test/hil/test/test_hil_net.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/test/hil/test/test_hil_net.py b/test/hil/test/test_hil_net.py index ec1575babc..17d576f4b0 100644 --- a/test/hil/test/test_hil_net.py +++ b/test/hil/test/test_hil_net.py @@ -171,9 +171,14 @@ def test_live_wrapper_and_descendant_without_pid_are_reaped(self): net.stop_client(proc, None, grace=0.1) killpg.assert_not_called() self.assertIsNotNone(proc.poll()) - stat = Path('/proc') / str(child) / 'stat' - if stat.exists(): - self.assertEqual(stat.read_text().rsplit(')', 1)[1].split()[0], 'Z') + # Dead: a zombie (Z), being reaped (X), or already gone once init reaped + # it after its parent died; read once and treat absence as reaped. + try: + stat = (Path('/proc') / str(child) / 'stat').read_text() + except (FileNotFoundError, ProcessLookupError): + stat = None + if stat is not None: + self.assertIn(stat.rsplit(')', 1)[1].split()[0], ('Z', 'X')) finally: if proc.poll() is None: proc.kill() From 74232d31f51b50dbb122c4f07a07fe10ce21e0e0 Mon Sep 17 00:00:00 2001 From: HiFiPhile Date: Sun, 8 Nov 2026 15:04:31 +0100 Subject: [PATCH 07/10] net_test: keep agentrc out of the namespace client, bind the move to ifindex The --http child runs under sudo + unshare, which drop AGENTRC_DIR/CI and replace HOME; importing usb_observe at module top loaded agentrc there, so the client died before reporting its PID ("namespace client failed to start: ''" on every hfp board). Import usb_observe only in the --uid parent, and put the script's own folder on sys.path for PYTHONSAFEPATH. Read the interface's ifindex before the move and require the one link in the namespace to carry it, so a peer that took over the name cannot pass. Gate the /proc reap test on Linux, and cover every asset per round. --- test/hil/net_test.py | 29 +++++++++--- test/hil/test/test_hil_net.py | 88 +++++++++++++++++++++++++++++++++-- 2 files changed, 107 insertions(+), 10 deletions(-) diff --git a/test/hil/net_test.py b/test/hil/net_test.py index facf4c85df..f50acd3723 100644 --- a/test/hil/net_test.py +++ b/test/hil/net_test.py @@ -20,7 +20,10 @@ import select import subprocess -from helper import hil_util, usb_observe +sys.path.append(os.path.dirname(os.path.abspath(__file__))) # PYTHONSAFEPATH drops it +# hil_util is stdlib-only at import. usb_observe loads agentrc, which the --http child +# must not need: sudo may drop AGENTRC_DIR/CI and replace HOME before it reports its PID. +from helper import hil_util # noqa: E402 DEVICE_IP = '192.168.7.1' # Default assets in the pinned lwIP dependency's src/apps/http/fsdata.c. Verify @@ -39,6 +42,7 @@ def command(argv, timeout=5): def interfaces_for_uid(uid): + from helper import usb_observe # the --uid parent only; see the import note above matches = [] for dev in usb_observe.usb_scan(vid='cafe', serial=uid): matches.extend(p.name for p in Path(dev['dir']).glob('*:*/net/*')) @@ -66,6 +70,11 @@ def wait_interface(uid, timeout=30): f'found {matches}; {detail}') +def interface_index(iface): + """The kernel ifindex of `iface` in our namespace; it survives a move into a fresh one.""" + return int(Path('/sys/class/net', iface, 'ifindex').read_text()) + + def descendants(root): """[pid, ...] of every descendant of `root`, parents before children, from one /proc walk.""" by_parent = {} @@ -133,7 +142,12 @@ def stop_client(proc, pid, grace=5): def check_device(uid): iface = wait_interface(uid) - print(f'USB serial {uid}: interface {iface}', flush=True) + # Bind the move to this device, not to a name a peer could take over: read the + # index, then confirm the serial still owns that name, and check it after the move. + ifindex = interface_index(iface) + if interfaces_for_uid(uid) != [iface]: + raise RuntimeError(f'USB serial {uid}: interface {iface} changed before the namespace move') + print(f'USB serial {uid}: interface {iface} (ifindex {ifindex})', flush=True) # Anonymous namespace: its last process exiting releases the USB interface, # even on SIGKILL. unshare drops to the caller's UID/GID BEFORE executing any # checkout-controlled Python. Only trusted system tools run with elevation. @@ -155,11 +169,12 @@ def check_device(uid): ns = f'/proc/{pid}/ns/net' command(privileged(['ip', 'link', 'set', 'dev', iface, 'netns', str(pid)])) ip = privileged(['nsenter', '--net=' + ns, 'ip']) - links = json.loads(command(ip + ['-j', 'link', 'show'])) - names = [link['ifname'] for link in links if link['ifname'] != 'lo'] - if len(names) != 1: - raise RuntimeError(f'{ns}: expected one network interface, found {names}') - iface = names[0] + links = [link for link in json.loads(command(ip + ['-j', 'link', 'show'])) + if link['ifname'] != 'lo'] + if len(links) != 1 or links[0].get('ifindex') != ifindex: + found = [(link['ifname'], link.get('ifindex')) for link in links] + raise RuntimeError(f'{ns}: expected one network interface with ifindex {ifindex}, found {found}') + iface = links[0]['ifname'] command(ip + ['addr', 'flush', 'dev', iface]) command(ip + ['addr', 'add', '192.168.7.2/24', 'dev', iface]) command(ip + ['link', 'set', 'dev', iface, 'up']) diff --git a/test/hil/test/test_hil_net.py b/test/hil/test/test_hil_net.py index 17d576f4b0..d6762d8b8b 100644 --- a/test/hil/test/test_hil_net.py +++ b/test/hil/test/test_hil_net.py @@ -6,6 +6,7 @@ import json import os from pathlib import Path +import subprocess import sys from tempfile import TemporaryDirectory import unittest @@ -14,6 +15,47 @@ sys.path.insert(0, str(Path(__file__).resolve().parents[1])) import net_test as net +SCRIPT = str(Path(net.__file__).resolve()) + + +def clean_env(tmp): + """A fresh interpreter's env as the sudo'd client may get it: no agentrc to find.""" + env = {k: v for k, v in os.environ.items() + if k not in ('GITHUB_ACTIONS', 'CI') and not k.startswith('PYTHON')} + env.update(PYTHONSAFEPATH='1', AGENTRC_DIR=tmp, HOME=tmp, USERPROFILE=tmp) + return env + + +class Startup(unittest.TestCase): + # hil_test.py starts net_test.py in a fresh interpreter, and the --http child under + # sudo; the in-process tests above add test/hil to sys.path and would hide both. + def test_safe_path_startup_finds_helper(self): + with TemporaryDirectory() as tmp: + ret = subprocess.run([sys.executable, SCRIPT, '--help'], cwd=tmp, env=clean_env(tmp), + capture_output=True, text=True, timeout=30) + self.assertNotIn('ModuleNotFoundError', ret.stderr) + self.assertEqual(ret.returncode, 0, ret.stderr) + + @unittest.skipUnless(os.name == 'posix', 'the --http client arms SIGALRM') + def test_http_client_does_not_load_agentrc(self): + # runpy does not put the script's folder on sys.path: its own append must. + code = '''import io, runpy, signal, sys +sys.argv = [sys.argv[1], '--http'] +sys.stdin = io.StringIO('') +try: + runpy.run_path(sys.argv[0], run_name='__main__') +except RuntimeError as e: + print('stopped:', e) +signal.alarm(0) +print('loaded:', sorted(m for m in sys.modules if 'agentrc' in m or 'usb_observe' in m)) +''' + with TemporaryDirectory() as tmp: + ret = subprocess.run([sys.executable, '-c', code, SCRIPT], cwd=tmp, env=clean_env(tmp), + capture_output=True, text=True, timeout=30) + self.assertEqual(ret.returncode, 0, ret.stderr) + self.assertIn('stopped: network setup owner exited before starting HTTP', ret.stdout) + self.assertIn('loaded: []', ret.stdout) + class InterfaceSelection(unittest.TestCase): @unittest.skipUnless(os.name == 'posix', 'sysfs interface names contain colons') @@ -23,7 +65,7 @@ def test_interface_comes_from_usb_serial_not_shared_mac_or_name(self): other = Path(tmp) / '1-3' (target / '1-2:1.0/net/usb7').mkdir(parents=True) (other / '1-3:1.0/net/enx0202846a9600').mkdir(parents=True) - with patch.object(net.usb_observe, 'usb_scan', return_value=[{'dir': str(target)}]) as scan: + with patch('helper.usb_observe.usb_scan', return_value=[{'dir': str(target)}]) as scan: self.assertEqual(net.interfaces_for_uid('ABC123'), ['usb7']) scan.assert_called_once_with(vid='cafe', serial='ABC123') @@ -64,13 +106,18 @@ def setUp(self): self.proc.poll.return_value = 0 self.proc.communicate.return_value = ('HTTP verified\n', '') + self.moved_ifindex = 7 + def command(argv): self.calls.append(argv) if '-j' in argv: - return json.dumps([{'ifname': 'lo'}, {'ifname': 'renamed0'}]) + return json.dumps([{'ifname': 'lo', 'ifindex': 1}, + {'ifname': 'renamed0', 'ifindex': self.moved_ifindex}]) return '' for name, value in [('wait_interface', Mock(return_value='usb7')), + ('interface_index', Mock(return_value=7)), + ('interfaces_for_uid', Mock(return_value=['usb7'])), ('command', Mock(side_effect=command))]: p = patch.object(net, name, value) p.start() @@ -96,6 +143,23 @@ def test_python_runs_as_caller_in_anonymous_namespace(self): self.assertFalse(any(c[:4] == ['sudo', '-n', 'ip', 'netns'] for c in self.calls)) self.proc.communicate.assert_called_once_with(input='go\n', timeout=30) + def test_peer_interface_moved_under_reused_name_fails(self): + # The requested board left and a peer took its name before the move. + self.moved_ifindex = 8 + self.proc.poll.return_value = None + with patch.object(net.os, 'kill') as kill: + with self.assertRaisesRegex(RuntimeError, 'ifindex 7.*renamed0.*8'): + net.check_device('ABC123') + self.assertFalse(any('addr' in c for c in self.calls)) + kill.assert_called_once_with(4321, net.signal.SIGTERM) + + def test_name_lost_before_move_is_not_moved(self): + net.interfaces_for_uid.return_value = [] + with self.assertRaisesRegex(RuntimeError, 'changed before the namespace move'): + net.check_device('ABC123') + net.subprocess.Popen.assert_not_called() + net.command.assert_not_called() + def test_interrupt_terminates_client_and_reaps_sudo(self): net.command.side_effect = KeyboardInterrupt() self.proc.poll.return_value = None @@ -146,7 +210,7 @@ def test_root_child_is_left_for_sudo_to_signal(self): self.proc.communicate.assert_called_once_with(timeout=5) -@unittest.skipUnless(os.name == 'posix', 'Linux process-tree cleanup') +@unittest.skipUnless(sys.platform == 'linux', 'Linux process-tree cleanup') class UnreportedClient(unittest.TestCase): def test_live_wrapper_and_descendant_without_pid_are_reaped(self): # Both ignore TERM so this also exercises the KILL pass. They inherit our @@ -210,6 +274,24 @@ def test_repeated_complete_binary_responses(self): self.assertEqual(self.conn.request.call_count, 3) self.assertEqual(self.conn.close.call_count, 3) + def test_every_asset_is_requested_each_round(self): + # Each asset is its own fixture, one of them binary: a loop over ASSETS[:1] + # would skip the image on hardware while a single-asset test still passed. + payloads = {'/page.html': b'ok', '/img/bin.gif': b'GIF89a\x00\xff\r\n\x01'} + requested = [] + + def request(method, path, headers=None): + requested.append(path) + self.response.read.return_value = payloads[path] + + self.conn.request.side_effect = request + with patch.object(net, 'ASSETS', [(path, len(data), hashlib.sha256(data).hexdigest()) + for path, data in payloads.items()]): + net.check_http() + self.assertEqual({path: requested.count(path) for path in payloads}, + {path: 3 for path in payloads}) + self.assertEqual(len(requested), 6) + def test_corruption_is_not_retried(self): self.response.read.return_value = b'x' * len(self.payload) with self.assertRaises(AssertionError): From 2b0c13bf0fb50f87cdfb3e704ed0728540892a08 Mon Sep 17 00:00:00 2001 From: hathach Date: Tue, 10 Nov 2026 02:27:11 +0700 Subject: [PATCH 08/10] net_test: move the interface with hil_root netns, the client unprivileged --- test/hil/helper/agentrc.py | 1 + test/hil/hil_test.py | 5 +- test/hil/net_test.py | 104 +++++++++---------------- test/hil/requirements.txt | 5 +- test/hil/test/test_hil_net.py | 142 ++++++++++++---------------------- 5 files changed, 92 insertions(+), 165 deletions(-) diff --git a/test/hil/helper/agentrc.py b/test/hil/helper/agentrc.py index 372dd52e14..bae28b70d6 100644 --- a/test/hil/helper/agentrc.py +++ b/test/hil/helper/agentrc.py @@ -44,6 +44,7 @@ 'skills/hil/scripts/hil_remote_core.py', 'skills/hil/scripts/hil_report_core.py', 'skills/hil/scripts/hil_report_notice.py', + 'skills/hil/scripts/hil_root.py', 'skills/hil/scripts/hil_tt.py', 'skills/hil/scripts/hil_usbtest.py', 'skills/hil/scripts/usb_observe.py', diff --git a/test/hil/hil_test.py b/test/hil/hil_test.py index b8622039f9..618e02b634 100755 --- a/test/hil/hil_test.py +++ b/test/hil/hil_test.py @@ -28,8 +28,9 @@ # libmtp9 pymtp ctypes load (device/mtp); Debian 13 uses libmtp9t64 # libmtp-runtime mtp-probe and the completed-device /dev/libmtp-* marker # alsa-utils arecord (device/audio_test_freertos) -# iproute2 isolated USB network test (root or passwordless sudo) -# util-linux unshare/nsenter for per-process network namespaces +# iproute2 isolated USB network test, run inside its own namespace +# util-linux unshare/nsenter for per-process user and network namespaces; the one root +# step, moving the interface in, is agentrc's hil_root netns (sudo) # udev udevadm wait (systemd >= 251) for USB network initialization # openocd unified openocd from https://github.com/hathach/openocd (branch tinyusb) for wch, rp2040/rp2350, analog max32 # - device/usbtest: usbtest kernel module + testusb binary (kernel tools/usb/testusb.c) on PATH, diff --git a/test/hil/net_test.py b/test/hil/net_test.py index f50acd3723..77332c1a9b 100644 --- a/test/hil/net_test.py +++ b/test/hil/net_test.py @@ -5,7 +5,8 @@ Invoked by hil_test.py while it holds the board lock. Only the USB network interface whose parent has the requested serial is moved. Each board gets its own namespace: the example deliberately shares its MAC and 192.168.7.1 with every other board. -Requires iproute2, util-linux, udevadm wait, and root (or passwordless sudo). +Requires iproute2, util-linux, udevadm wait, unprivileged user namespaces and sudo on +hil_root (agentrc), whose netns verb is the one root step; run it as a user other than root. No persistent host configuration. """ import argparse @@ -21,8 +22,8 @@ import subprocess sys.path.append(os.path.dirname(os.path.abspath(__file__))) # PYTHONSAFEPATH drops it -# hil_util is stdlib-only at import. usb_observe loads agentrc, which the --http child -# must not need: sudo may drop AGENTRC_DIR/CI and replace HOME before it reports its PID. +# hil_util is stdlib-only at import. usb_observe and hil_root load agentrc, which only the +# --uid parent uses: the --http client stays stdlib-only. from helper import hil_util # noqa: E402 DEVICE_IP = '192.168.7.1' @@ -49,6 +50,19 @@ def interfaces_for_uid(uid): return sorted(set(matches)) +def busport_for_uid(uid): + from helper import usb_observe # the --uid parent only; see the import note above + devs = usb_observe.usb_scan(vid='cafe', serial=uid) + if len(devs) != 1: + raise RuntimeError(f'USB serial {uid}: expected one device, found {len(devs)}') + return Path(devs[0]['dir']).name + + +def hil_root(): + from helper import agentrc # the --uid parent only; see the import note above + return agentrc.load_file('skills/hil/scripts/hil_root.py') + + def wait_interface(uid, timeout=30): deadline = time.monotonic() + timeout matches = [] @@ -75,27 +89,6 @@ def interface_index(iface): return int(Path('/sys/class/net', iface, 'ifindex').read_text()) -def descendants(root): - """[pid, ...] of every descendant of `root`, parents before children, from one /proc walk.""" - by_parent = {} - for entry in Path('/proc').iterdir(): - if not entry.name.isdigit(): - continue - try: - stat = (entry / 'stat').read_bytes() - # comm (field 2) may contain spaces and ')': split only after the last ')' - ppid = int(stat[stat.rindex(b')') + 2:].split()[1]) - except (OSError, ValueError, IndexError): - continue # exited mid-walk, unreadable or truncated - by_parent.setdefault(ppid, []).append(int(entry.name)) - out, todo = [], list(by_parent.get(root, [])) - while todo: - pid = todo.pop(0) - out.append(pid) - todo += by_parent.get(pid, []) - return out - - def close_pipes(proc): """Close our ends of a child's pipes. Never raises.""" for pipe in (proc.stdout, proc.stderr, proc.stdin): @@ -106,31 +99,11 @@ def close_pipes(proc): pass -def privileged(argv): - return (['sudo', '-n'] if os.geteuid() != 0 else []) + argv - - -def stop_client(proc, pid, grace=5): - # The unshare child runs as us, even when its sudo wrapper belongs to root. - # Terminate that child directly so sudo can reap it and return normally. +def stop_client(proc, grace=5): + # unshare execs the client, so proc is the client itself. Never killpg: it shares + # the HIL caller's group. for sig in (signal.SIGTERM, signal.SIGKILL): - if pid is None: - # Startup can fail before Python reports its PID. Walk only this - # Popen child's descendants, leaves first, then signal the wrapper. - # Never killpg: these processes share the HIL caller's group. - for child in reversed(descendants(proc.pid)): - try: - os.kill(child, sig) - except (ProcessLookupError, PermissionError): - # sudo relays SIGTERM to a child still running as root before - # unshare drops privileges; the caller can signal sudo itself. - pass - proc.send_signal(sig) - else: - try: - os.kill(pid, sig) - except ProcessLookupError: - pass + proc.send_signal(sig) try: proc.communicate(timeout=grace) return @@ -142,23 +115,20 @@ def stop_client(proc, pid, grace=5): def check_device(uid): iface = wait_interface(uid) - # Bind the move to this device, not to a name a peer could take over: read the - # index, then confirm the serial still owns that name, and check it after the move. + # Bind the move to this device: hil_root moves the one interface at its busport and + # prints that interface's index, which must be the one read here. ifindex = interface_index(iface) - if interfaces_for_uid(uid) != [iface]: - raise RuntimeError(f'USB serial {uid}: interface {iface} changed before the namespace move') - print(f'USB serial {uid}: interface {iface} (ifindex {ifindex})', flush=True) - # Anonymous namespace: its last process exiting releases the USB interface, - # even on SIGKILL. unshare drops to the caller's UID/GID BEFORE executing any - # checkout-controlled Python. Only trusted system tools run with elevation. - argv = privileged(['unshare', '--net', '--setgid', str(os.getgid()), - '--setuid', str(os.getuid()), sys.executable, - str(Path(__file__).resolve()), '--http']) + busport = busport_for_uid(uid) + print(f'USB serial {uid}: interface {iface} (ifindex {ifindex}) at {busport}', flush=True) + # Anonymous user and network namespace, ours: its last process exiting releases the + # USB interface, even on SIGKILL. hil_root moves the interface in only for a namespace + # a user other than root (the sudo caller) created. + argv = ['unshare', '--user', '--map-root-user', '--net', sys.executable, + str(Path(__file__).resolve()), '--http'] # Keep the child in our process group: hil_test's interruption sweep can kill # the whole group without stranding a detached namespace owner. proc = subprocess.Popen(argv, stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True) - pid = None try: if not select.select([proc.stdout], [], [], 5)[0]: raise RuntimeError('network namespace client did not become ready') @@ -166,14 +136,15 @@ def check_device(uid): if not ready.isdecimal(): raise RuntimeError(f'network namespace client failed to start: {ready!r}') pid = int(ready) - ns = f'/proc/{pid}/ns/net' - command(privileged(['ip', 'link', 'set', 'dev', iface, 'netns', str(pid)])) - ip = privileged(['nsenter', '--net=' + ns, 'ip']) + moved = command(hil_root().argv('netns', busport, str(pid)), timeout=30).split() + if moved[:1] != [str(ifindex)]: + raise RuntimeError(f'hil_root netns {busport}: moved {moved}, expected ifindex {ifindex}') + ip = ['nsenter', '--preserve-credentials', f'--target={pid}', '--user', '--net', 'ip'] links = [link for link in json.loads(command(ip + ['-j', 'link', 'show'])) if link['ifname'] != 'lo'] if len(links) != 1 or links[0].get('ifindex') != ifindex: found = [(link['ifname'], link.get('ifindex')) for link in links] - raise RuntimeError(f'{ns}: expected one network interface with ifindex {ifindex}, found {found}') + raise RuntimeError(f'pid {pid}: expected one network interface with ifindex {ifindex}, found {found}') iface = links[0]['ifname'] command(ip + ['addr', 'flush', 'dev', iface]) command(ip + ['addr', 'add', '192.168.7.2/24', 'dev', iface]) @@ -187,7 +158,7 @@ def check_device(uid): print(stdout, end='', flush=True) finally: if proc.poll() is None: - stop_client(proc, pid) + stop_client(proc) else: close_pipes(proc) @@ -233,7 +204,7 @@ def main(): mode.add_argument('--uid') mode.add_argument('--http', action='store_true', help=argparse.SUPPRESS) args = ap.parse_args() - # A root-owned helper must bound itself even if its unprivileged caller dies. + # Each side bounds itself even if the other, or its caller, dies. signal.signal(signal.SIGALRM, interrupted) signal.signal(signal.SIGTERM, interrupted) signal.alarm(75) @@ -241,7 +212,6 @@ def main(): print(os.getpid(), flush=True) if sys.stdin.readline().strip() != 'go': raise RuntimeError('network setup owner exited before starting HTTP') - print(f'network client uid/gid: {os.getuid()}/{os.getgid()}', flush=True) check_http() else: check_device(args.uid) diff --git a/test/hil/requirements.txt b/test/hil/requirements.txt index 981375a442..c6d666303d 100644 --- a/test/hil/requirements.txt +++ b/test/hil/requirements.txt @@ -4,8 +4,9 @@ # libmtp9 - pymtp ctypes load (device/mtp); Debian 13 uses libmtp9t64 # libmtp-runtime - mtp-probe and the completed-device /dev/libmtp-* marker # alsa-utils - arecord (device/audio_test_freertos) -# iproute2 - isolated USB network test (root or passwordless sudo) -# util-linux - unshare/nsenter for per-process network namespaces +# iproute2 - isolated USB network test, run inside its own namespace +# util-linux - unshare/nsenter for per-process user and network namespaces; the one root +# step, moving the interface in, is agentrc's hil_root netns (sudo) # udev - udevadm wait (systemd >= 251) for USB network initialization hidapi pyserial diff --git a/test/hil/test/test_hil_net.py b/test/hil/test/test_hil_net.py index d6762d8b8b..5b52aa0848 100644 --- a/test/hil/test/test_hil_net.py +++ b/test/hil/test/test_hil_net.py @@ -19,7 +19,7 @@ def clean_env(tmp): - """A fresh interpreter's env as the sudo'd client may get it: no agentrc to find.""" + """A fresh interpreter's env with no agentrc to find: the --http client needs none.""" env = {k: v for k, v in os.environ.items() if k not in ('GITHUB_ACTIONS', 'CI') and not k.startswith('PYTHON')} env.update(PYTHONSAFEPATH='1', AGENTRC_DIR=tmp, HOME=tmp, USERPROFILE=tmp) @@ -28,7 +28,7 @@ def clean_env(tmp): class Startup(unittest.TestCase): # hil_test.py starts net_test.py in a fresh interpreter, and the --http child under - # sudo; the in-process tests above add test/hil to sys.path and would hide both. + # unshare; the in-process tests above add test/hil to sys.path and would hide both. def test_safe_path_startup_finds_helper(self): with TemporaryDirectory() as tmp: ret = subprocess.run([sys.executable, SCRIPT, '--help'], cwd=tmp, env=clean_env(tmp), @@ -69,6 +69,15 @@ def test_interface_comes_from_usb_serial_not_shared_mac_or_name(self): self.assertEqual(net.interfaces_for_uid('ABC123'), ['usb7']) scan.assert_called_once_with(vid='cafe', serial='ABC123') + def test_busport_is_the_one_device_with_the_serial(self): + with patch('helper.usb_observe.usb_scan', return_value=[{'dir': '/sys/bus/usb/devices/1-2.3'}]) as scan: + self.assertEqual(net.busport_for_uid('ABC123'), '1-2.3') + scan.assert_called_once_with(vid='cafe', serial='ABC123') + for found in ([], [{'dir': '/x/1-2'}, {'dir': '/x/1-3'}]): + with self.subTest(found=len(found)), patch('helper.usb_observe.usb_scan', return_value=found): + with self.assertRaisesRegex(RuntimeError, f'expected one device, found {len(found)}'): + net.busport_for_uid('ABC123') + def test_waits_for_driver_interface(self): with patch.object(net, 'interfaces_for_uid', side_effect=[[], ['usb7'], ['usb7']]), \ patch.object(net.hil_util, 'run_cmd', return_value=Mock(returncode=0, stderr='')), \ @@ -107,82 +116,82 @@ def setUp(self): self.proc.communicate.return_value = ('HTTP verified\n', '') self.moved_ifindex = 7 + self.printed_ifindex = 7 - def command(argv): + def command(argv, timeout=5): self.calls.append(argv) + if argv[:2] == ['hil_root', 'netns']: + return f'{self.printed_ifindex} usb7\n' if '-j' in argv: return json.dumps([{'ifname': 'lo', 'ifindex': 1}, {'ifname': 'renamed0', 'ifindex': self.moved_ifindex}]) return '' + hil_root = Mock() + hil_root.argv.side_effect = lambda verb, *args: ['hil_root', verb, *args] for name, value in [('wait_interface', Mock(return_value='usb7')), ('interface_index', Mock(return_value=7)), - ('interfaces_for_uid', Mock(return_value=['usb7'])), + ('busport_for_uid', Mock(return_value='1-2')), + ('hil_root', Mock(return_value=hil_root)), ('command', Mock(side_effect=command))]: p = patch.object(net, name, value) p.start() self.addCleanup(p.stop) for p in [patch.object(net.subprocess, 'Popen', return_value=self.proc), patch.object(net.select, 'select', return_value=([self.proc.stdout], [], [])), - patch.object(net.os, 'geteuid', return_value=1000, create=True), - patch.object(net.os, 'getuid', return_value=1000, create=True), - patch.object(net.os, 'getgid', return_value=1000, create=True), patch.object(net.signal, 'SIGKILL', 9, create=True), patch('sys.stdout', new_callable=io.StringIO)]: p.start() self.addCleanup(p.stop) - def test_python_runs_as_caller_in_anonymous_namespace(self): + def test_client_runs_unprivileged_and_hil_root_moves_the_interface(self): net.check_device('ABC123') argv = net.subprocess.Popen.call_args.args[0] - self.assertEqual(argv[:7], ['sudo', '-n', 'unshare', '--net', '--setgid', '1000', '--setuid']) - self.assertEqual(argv[7], '1000') + self.assertEqual(argv, ['unshare', '--user', '--map-root-user', '--net', sys.executable, SCRIPT, '--http']) self.assertNotIn('start_new_session', net.subprocess.Popen.call_args.kwargs) - self.assertIn(['sudo', '-n', 'ip', 'link', 'set', 'dev', 'usb7', 'netns', '4321'], self.calls) + self.assertEqual(self.calls[0], ['hil_root', 'netns', '1-2', '4321']) + self.assertFalse(any('sudo' in c for c in self.calls[1:])) + nsenter = ['nsenter', '--preserve-credentials', '--target=4321', '--user', '--net', 'ip'] + self.assertTrue(all(c[:6] == nsenter for c in self.calls[1:])) self.assertTrue(any('renamed0' in c for c in self.calls)) - self.assertFalse(any(c[:4] == ['sudo', '-n', 'ip', 'netns'] for c in self.calls)) self.proc.communicate.assert_called_once_with(input='go\n', timeout=30) - def test_peer_interface_moved_under_reused_name_fails(self): - # The requested board left and a peer took its name before the move. + def test_namespace_holding_another_ifindex_fails(self): self.moved_ifindex = 8 self.proc.poll.return_value = None - with patch.object(net.os, 'kill') as kill: - with self.assertRaisesRegex(RuntimeError, 'ifindex 7.*renamed0.*8'): - net.check_device('ABC123') + with self.assertRaisesRegex(RuntimeError, 'ifindex 7.*renamed0.*8'): + net.check_device('ABC123') self.assertFalse(any('addr' in c for c in self.calls)) - kill.assert_called_once_with(4321, net.signal.SIGTERM) + self.proc.send_signal.assert_called_once_with(net.signal.SIGTERM) - def test_name_lost_before_move_is_not_moved(self): - net.interfaces_for_uid.return_value = [] - with self.assertRaisesRegex(RuntimeError, 'changed before the namespace move'): + def test_hil_root_moving_another_index_fails_before_setup(self): + self.printed_ifindex = 9 + self.proc.poll.return_value = None + with self.assertRaisesRegex(RuntimeError, r"moved \['9', 'usb7'\], expected ifindex 7"): net.check_device('ABC123') - net.subprocess.Popen.assert_not_called() - net.command.assert_not_called() + self.assertEqual(len(self.calls), 1) + self.proc.send_signal.assert_called_once_with(net.signal.SIGTERM) - def test_interrupt_terminates_client_and_reaps_sudo(self): + def test_interrupt_terminates_client(self): net.command.side_effect = KeyboardInterrupt() self.proc.poll.return_value = None - with patch.object(net.os, 'kill') as kill: - with self.assertRaises(KeyboardInterrupt): - net.check_device('ABC123') - kill.assert_called_once_with(4321, net.signal.SIGTERM) + with self.assertRaises(KeyboardInterrupt): + net.check_device('ABC123') + self.proc.send_signal.assert_called_once_with(net.signal.SIGTERM) self.proc.communicate.assert_called_once_with(timeout=5) def test_setup_failure_releases_namespace_owner(self): net.command.side_effect = RuntimeError('move failed') self.proc.poll.return_value = None - with patch.object(net.os, 'kill') as kill: - with self.assertRaisesRegex(RuntimeError, 'move failed'): - net.check_device('ABC123') - kill.assert_called_once_with(4321, net.signal.SIGTERM) + with self.assertRaisesRegex(RuntimeError, 'move failed'): + net.check_device('ABC123') + self.proc.send_signal.assert_called_once_with(net.signal.SIGTERM) def test_stalled_client_escalates_after_cleanup_grace(self): self.proc.communicate.side_effect = [net.subprocess.TimeoutExpired('client', 5), ('', '')] - with patch.object(net.os, 'kill') as kill: - net.stop_client(self.proc, 4321) - self.assertEqual([c.args for c in kill.call_args_list], - [(4321, net.signal.SIGTERM), (4321, net.signal.SIGKILL)]) + net.stop_client(self.proc) + self.assertEqual([c.args for c in self.proc.send_signal.call_args_list], + [(net.signal.SIGTERM,), (net.signal.SIGKILL,)]) def test_http_failure_is_reported(self): self.proc.returncode = 1 @@ -190,71 +199,16 @@ def test_http_failure_is_reported(self): with self.assertRaisesRegex(RuntimeError, 'corrupt data'): net.check_device('ABC123') - def test_readiness_timeout_cleans_up_without_reported_pid(self): + def test_readiness_timeout_stops_the_client(self): net.select.select.return_value = ([], [], []) self.proc.poll.return_value = None - with patch.object(net, 'descendants', return_value=[4321]), \ - patch.object(net.os, 'kill') as kill: - with self.assertRaisesRegex(RuntimeError, 'did not become ready'): - net.check_device('ABC123') + with self.assertRaisesRegex(RuntimeError, 'did not become ready'): + net.check_device('ABC123') net.command.assert_not_called() - kill.assert_called_once_with(4321, net.signal.SIGTERM) - self.proc.send_signal.assert_called_once_with(net.signal.SIGTERM) - self.proc.communicate.assert_called_once_with(timeout=5) - - def test_root_child_is_left_for_sudo_to_signal(self): - with patch.object(net, 'descendants', return_value=[4321]), \ - patch.object(net.os, 'kill', side_effect=PermissionError()): - net.stop_client(self.proc, None) self.proc.send_signal.assert_called_once_with(net.signal.SIGTERM) self.proc.communicate.assert_called_once_with(timeout=5) -@unittest.skipUnless(sys.platform == 'linux', 'Linux process-tree cleanup') -class UnreportedClient(unittest.TestCase): - def test_live_wrapper_and_descendant_without_pid_are_reaped(self): - # Both ignore TERM so this also exercises the KILL pass. They inherit our - # process group; cleanup must not signal the test runner or its siblings. - code = '''import os, signal, subprocess, sys, time -signal.signal(signal.SIGTERM, signal.SIG_IGN) -child = subprocess.Popen([sys.executable, '-c', - 'import signal,time; signal.signal(signal.SIGTERM,signal.SIG_IGN); print("ready",flush=True); time.sleep(60)'], - stdout=subprocess.PIPE, text=True) -child.stdout.readline() -print(child.pid, flush=True) -time.sleep(60) -''' - proc = net.subprocess.Popen([sys.executable, '-c', code], stdout=net.subprocess.PIPE, - stderr=net.subprocess.PIPE, text=True) - child = None - try: - self.assertTrue(net.select.select([proc.stdout], [], [], 5)[0]) - # The test knows the descendant PID; stop_client deliberately does not. - child = int(proc.stdout.readline()) - with patch.object(net.os, 'killpg') as killpg: - net.stop_client(proc, None, grace=0.1) - killpg.assert_not_called() - self.assertIsNotNone(proc.poll()) - # Dead: a zombie (Z), being reaped (X), or already gone once init reaped - # it after its parent died; read once and treat absence as reaped. - try: - stat = (Path('/proc') / str(child) / 'stat').read_text() - except (FileNotFoundError, ProcessLookupError): - stat = None - if stat is not None: - self.assertIn(stat.rsplit(')', 1)[1].split()[0], ('Z', 'X')) - finally: - if proc.poll() is None: - proc.kill() - proc.wait(timeout=5) - if child is not None: - try: - os.kill(child, net.signal.SIGKILL) - except ProcessLookupError: - pass - net.close_pipes(proc) - - class HttpVerification(unittest.TestCase): def setUp(self): self.payload = b'\x00\xffbinary\r\nasset' From aaec6181d1db8214850d5c166762003d613e2c69 Mon Sep 17 00:00:00 2001 From: hathach Date: Tue, 10 Nov 2026 09:34:08 +0700 Subject: [PATCH 09/10] net_test: pick the moved interface by ifindex, ignoring fallback tunnels --- test/hil/net_test.py | 9 +++++---- test/hil/test/test_hil_net.py | 10 +++++++++- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/test/hil/net_test.py b/test/hil/net_test.py index 77332c1a9b..ae41e10754 100644 --- a/test/hil/net_test.py +++ b/test/hil/net_test.py @@ -140,12 +140,13 @@ def check_device(uid): if moved[:1] != [str(ifindex)]: raise RuntimeError(f'hil_root netns {busport}: moved {moved}, expected ifindex {ifindex}') ip = ['nsenter', '--preserve-credentials', f'--target={pid}', '--user', '--net', 'ip'] - links = [link for link in json.loads(command(ip + ['-j', 'link', 'show'])) - if link['ifname'] != 'lo'] - if len(links) != 1 or links[0].get('ifindex') != ifindex: + # A fresh netns may also hold fallback tunnels (tunl0, sit0, ...) of loaded modules. + links = json.loads(command(ip + ['-j', 'link', 'show'])) + moved_links = [link for link in links if link.get('ifindex') == ifindex] + if len(moved_links) != 1: found = [(link['ifname'], link.get('ifindex')) for link in links] raise RuntimeError(f'pid {pid}: expected one network interface with ifindex {ifindex}, found {found}') - iface = links[0]['ifname'] + iface = moved_links[0]['ifname'] command(ip + ['addr', 'flush', 'dev', iface]) command(ip + ['addr', 'add', '192.168.7.2/24', 'dev', iface]) command(ip + ['link', 'set', 'dev', iface, 'up']) diff --git a/test/hil/test/test_hil_net.py b/test/hil/test/test_hil_net.py index 5b52aa0848..4fc95501ad 100644 --- a/test/hil/test/test_hil_net.py +++ b/test/hil/test/test_hil_net.py @@ -117,13 +117,14 @@ def setUp(self): self.moved_ifindex = 7 self.printed_ifindex = 7 + self.extra_links = [] def command(argv, timeout=5): self.calls.append(argv) if argv[:2] == ['hil_root', 'netns']: return f'{self.printed_ifindex} usb7\n' if '-j' in argv: - return json.dumps([{'ifname': 'lo', 'ifindex': 1}, + return json.dumps([{'ifname': 'lo', 'ifindex': 1}, *self.extra_links, {'ifname': 'renamed0', 'ifindex': self.moved_ifindex}]) return '' @@ -156,6 +157,13 @@ def test_client_runs_unprivileged_and_hil_root_moves_the_interface(self): self.assertTrue(any('renamed0' in c for c in self.calls)) self.proc.communicate.assert_called_once_with(input='go\n', timeout=30) + def test_fallback_tunnels_next_to_the_moved_interface_are_ignored(self): + self.extra_links = [{'ifname': 'tunl0', 'ifindex': 2}, {'ifname': 'sit0', 'ifindex': 3}] + net.check_device('ABC123') + self.assertIn(['set', 'dev', 'renamed0', 'up'], [c[-4:] for c in self.calls]) + self.assertFalse(any('tunl0' in c or 'sit0' in c for c in self.calls)) + self.proc.communicate.assert_called_once_with(input='go\n', timeout=30) + def test_namespace_holding_another_ifindex_fails(self): self.moved_ifindex = 8 self.proc.poll.return_value = None From f51f4d4e5bb128e1210debdc2be32d95a7e4f7d2 Mon Sep 17 00:00:00 2001 From: hathach Date: Tue, 10 Nov 2026 14:52:36 +0700 Subject: [PATCH 10/10] net_test: report the host-to-device iperf rate --- test/hil/hil_test.py | 5 ++- test/hil/net_test.py | 48 +++++++++++++++++---- test/hil/requirements.txt | 3 +- test/hil/test/test_hil_net.py | 81 ++++++++++++++++++++++++++++++++++- 4 files changed, 124 insertions(+), 13 deletions(-) diff --git a/test/hil/hil_test.py b/test/hil/hil_test.py index 618e02b634..517db8b9ca 100755 --- a/test/hil/hil_test.py +++ b/test/hil/hil_test.py @@ -23,12 +23,13 @@ # THE SOFTWARE. # Host setup (required: a missing tool fails its test rather than skipping it): -# - System packages: sudo apt install mtools libmtp9 libmtp-runtime alsa-utils iproute2 udev util-linux +# - System packages: sudo apt install mtools libmtp9 libmtp-runtime alsa-utils iproute2 iperf udev util-linux # mtools read_disk_file (device/cdc_msc, device/msc_dual_lun) # libmtp9 pymtp ctypes load (device/mtp); Debian 13 uses libmtp9t64 # libmtp-runtime mtp-probe and the completed-device /dev/libmtp-* marker # alsa-utils arecord (device/audio_test_freertos) # iproute2 isolated USB network test, run inside its own namespace +# iperf its host-to-device TCP rate, on builds with the lwiperf server # util-linux unshare/nsenter for per-process user and network namespaces; the one root # step, moving the interface in, is agentrc's hil_root netns (sudo) # udev udevadm wait (systemd >= 251) for USB network initialization @@ -1335,6 +1336,8 @@ def test_device_net_lwip_webserver(board): assert ret.returncode == 0, (f'USB network test failed (rc={ret.returncode}): ' f'{ret.stdout} {ret.stderr}') print(ret.stdout.strip(), end='') + rate = re.search(r'iperf: ([\d.]+) Mbit/s', ret.stdout) + return f'{hil_report.REPORT_CELL["pass"]} {rate.group(1)} Mbit/s' if rate else None def test_device_msc_dual_lun(board): diff --git a/test/hil/net_test.py b/test/hil/net_test.py index ae41e10754..4377f0b8f7 100644 --- a/test/hil/net_test.py +++ b/test/hil/net_test.py @@ -1,12 +1,12 @@ #!/usr/bin/env python3 # SPDX-License-Identifier: MIT -"""Exercise the webserver on one USB device, independent of host DHCP and routing. +"""Exercise the webserver and iperf server on one USB device, independent of host DHCP and routing. Invoked by hil_test.py while it holds the board lock. Only the USB network interface whose parent has the requested serial is moved. Each board gets its own namespace: the example deliberately shares its MAC and 192.168.7.1 with every other board. -Requires iproute2, util-linux, udevadm wait, unprivileged user namespaces and sudo on -hil_root (agentrc), whose netns verb is the one root step; run it as a user other than root. +Requires iproute2, util-linux, iperf (2.x), udevadm wait, unprivileged user namespaces and sudo +on hil_root (agentrc), whose netns verb is the one root step; run it as a user other than root. No persistent host configuration. """ import argparse @@ -16,6 +16,7 @@ import os from pathlib import Path import signal +import socket import sys import time import select @@ -27,6 +28,8 @@ from helper import hil_util # noqa: E402 DEVICE_IP = '192.168.7.1' +IPERF_PORT = 5001 # lwiperf's LWIPERF_TCP_PORT_DEFAULT +IPERF_SECONDS = 3 # Default assets in the pinned lwIP dependency's src/apps/http/fsdata.c. Verify # complete responses, including the binary image, rather than just TCP connect/200. ASSETS = ( @@ -151,11 +154,11 @@ def check_device(uid): command(ip + ['addr', 'add', '192.168.7.2/24', 'dev', iface]) command(ip + ['link', 'set', 'dev', iface, 'up']) try: - stdout, stderr = proc.communicate(input='go\n', timeout=30) + stdout, stderr = proc.communicate(input='go\n', timeout=45) except subprocess.TimeoutExpired as exc: - raise RuntimeError('network HTTP client timed out after 30s') from exc + raise RuntimeError('network client timed out after 45s') from exc if proc.returncode: - raise RuntimeError(f'network HTTP client failed (rc={proc.returncode}): {stdout} {stderr}') + raise RuntimeError(f'network client failed (rc={proc.returncode}): {stdout} {stderr}') print(stdout, end='', flush=True) finally: if proc.poll() is None: @@ -165,8 +168,7 @@ def check_device(uid): def check_http(): - # HTTP is present on every SRAM tier; INCLUDE_IPERF is deliberately absent on - # smaller parts. Direct connections also ignore the host's HTTP proxy settings. + # Direct connections also ignore the host's HTTP proxy settings. total = 0 for _ in range(3): for path, size, digest in ASSETS: @@ -195,6 +197,33 @@ def check_http(): print(f'HTTP: {3 * len(ASSETS)} responses verified, {total} bytes', flush=True) +def check_iperf(): + # lwiperf runs only on the high-throughput SRAM tier (INCLUDE_IPERF), so a refused port is + # reported, not failed: it cannot tell a build without lwiperf from one whose server failed + # to start. Its tradeoff (-r) and dual (-d) tests fail against iperf 2.2, so only host to + # device is measured. + try: + socket.create_connection((DEVICE_IP, IPERF_PORT), timeout=3).close() + except ConnectionRefusedError: + print(f'iperf: port {IPERF_PORT} refused, no server', flush=True) + return + try: + ret = subprocess.run(['iperf', '-c', DEVICE_IP, '-p', str(IPERF_PORT), '-t', str(IPERF_SECONDS), '-y', 'C'], + capture_output=True, text=True, timeout=IPERF_SECONDS + 15) + except subprocess.TimeoutExpired as exc: + raise RuntimeError(f'iperf timed out after {exc.timeout}s') from exc + # iperf 2 exits 0 even on a refused or broken connection: only one whole CSV report counts. + lines = ret.stdout.splitlines() + fields = lines[0].split(',') if len(lines) == 1 else [] + if ret.returncode or ret.stderr.strip() or len(fields) != 9: + raise RuntimeError(f'iperf failed (rc={ret.returncode}): {ret.stdout} {ret.stderr}') + *_, interval, nbytes, bps = fields + start, end = map(float, interval.split('-')) + if end - start < IPERF_SECONDS - 0.5 or int(nbytes) <= 0: + raise RuntimeError(f'iperf ended early: {lines[0]}') + print(f'iperf: {int(bps) / 1e6:.1f} Mbit/s host to device', flush=True) + + def interrupted(signum, _frame): raise TimeoutError(f'network test interrupted (signal {signum})') @@ -208,12 +237,13 @@ def main(): # Each side bounds itself even if the other, or its caller, dies. signal.signal(signal.SIGALRM, interrupted) signal.signal(signal.SIGTERM, interrupted) - signal.alarm(75) + signal.alarm(90) if args.http: print(os.getpid(), flush=True) if sys.stdin.readline().strip() != 'go': raise RuntimeError('network setup owner exited before starting HTTP') check_http() + check_iperf() else: check_device(args.uid) signal.alarm(0) diff --git a/test/hil/requirements.txt b/test/hil/requirements.txt index c6d666303d..2cfadf902b 100644 --- a/test/hil/requirements.txt +++ b/test/hil/requirements.txt @@ -1,10 +1,11 @@ # System packages (install separately): -# sudo apt install mtools libmtp9 libmtp-runtime alsa-utils iproute2 udev util-linux +# sudo apt install mtools libmtp9 libmtp-runtime alsa-utils iproute2 iperf udev util-linux # mtools - read_disk_file (device/cdc_msc, device/msc_dual_lun) # libmtp9 - pymtp ctypes load (device/mtp); Debian 13 uses libmtp9t64 # libmtp-runtime - mtp-probe and the completed-device /dev/libmtp-* marker # alsa-utils - arecord (device/audio_test_freertos) # iproute2 - isolated USB network test, run inside its own namespace +# iperf - its host-to-device TCP rate, on builds with the lwiperf server # util-linux - unshare/nsenter for per-process user and network namespaces; the one root # step, moving the interface in, is agentrc's hil_root netns (sudo) # udev - udevadm wait (systemd >= 251) for USB network initialization diff --git a/test/hil/test/test_hil_net.py b/test/hil/test/test_hil_net.py index 4fc95501ad..88503324c8 100644 --- a/test/hil/test/test_hil_net.py +++ b/test/hil/test/test_hil_net.py @@ -155,14 +155,14 @@ def test_client_runs_unprivileged_and_hil_root_moves_the_interface(self): nsenter = ['nsenter', '--preserve-credentials', '--target=4321', '--user', '--net', 'ip'] self.assertTrue(all(c[:6] == nsenter for c in self.calls[1:])) self.assertTrue(any('renamed0' in c for c in self.calls)) - self.proc.communicate.assert_called_once_with(input='go\n', timeout=30) + self.proc.communicate.assert_called_once_with(input='go\n', timeout=45) def test_fallback_tunnels_next_to_the_moved_interface_are_ignored(self): self.extra_links = [{'ifname': 'tunl0', 'ifindex': 2}, {'ifname': 'sit0', 'ifindex': 3}] net.check_device('ABC123') self.assertIn(['set', 'dev', 'renamed0', 'up'], [c[-4:] for c in self.calls]) self.assertFalse(any('tunl0' in c or 'sit0' in c for c in self.calls)) - self.proc.communicate.assert_called_once_with(input='go\n', timeout=30) + self.proc.communicate.assert_called_once_with(input='go\n', timeout=45) def test_namespace_holding_another_ifindex_fails(self): self.moved_ifindex = 8 @@ -291,5 +291,82 @@ def test_command_reaps_a_stalled_client(self): net.command([sys.executable, '-c', 'import time; time.sleep(60)'], timeout=0.1) +class IperfVerification(unittest.TestCase): + # Observed iperf 2.2.1 output against lwiperf: a 3 s host-to-device run, and a tradeoff + # run whose reverse leg failed yet exited 0. + GOOD = '20261110074851,192.168.7.2,38608,192.168.7.1,5001,1,0.0-3.3,1441856,3538660\n' + BROKEN = ('[ 1] tcp write failed\n[ 1] shutdown failed: Transport endpoint is not connected\n' + '20261110074852,192.168.7.2,38616,192.168.7.1,5001,1,0.0-0.0,75920,47917948\n' + 'ERROR: expected reverse connect did not occur\n') + + def setUp(self): + self.run = Mock(return_value=subprocess.CompletedProcess([], 0, self.GOOD, '')) + self.connect = Mock() + for p in [patch.object(net.subprocess, 'run', self.run), + patch.object(net.socket, 'create_connection', self.connect), + patch('sys.stdout', new_callable=io.StringIO)]: + p.start() + self.addCleanup(p.stop) + + def test_rate_from_one_whole_report(self): + net.check_iperf() + self.assertIn('iperf: 3.5 Mbit/s host to device', sys.stdout.getvalue()) + argv = self.run.call_args.args[0] + self.assertEqual(argv[:3], ['iperf', '-c', net.DEVICE_IP]) + self.assertIn('-y', argv) + + def test_refused_port_is_reported_not_failed(self): + self.connect.side_effect = ConnectionRefusedError() + net.check_iperf() + self.assertIn('iperf: port 5001 refused, no server', sys.stdout.getvalue()) + self.run.assert_not_called() + + def test_broken_or_refused_run_exiting_zero_fails(self): + refused = '[ 1] tcp connect to 192.168.7.1 port 5001 failed (Connection refused)\n' + for out, err in ((self.BROKEN, ''), (refused, ''), ('', ''), (self.GOOD, 'warning\n')): + with self.subTest(out=out, err=err): + self.run.return_value = subprocess.CompletedProcess([], 0, out, err) + with self.assertRaisesRegex(RuntimeError, 'iperf failed'): + net.check_iperf() + + def test_short_or_empty_run_fails(self): + for old, new in (('0.0-3.3', '0.0-0.4'), (',1441856,', ',0,')): + with self.subTest(new=new): + self.run.return_value = subprocess.CompletedProcess([], 0, self.GOOD.replace(old, new), '') + with self.assertRaisesRegex(RuntimeError, 'ended early'): + net.check_iperf() + + def test_hung_run_fails_with_its_bound(self): + self.run.side_effect = subprocess.TimeoutExpired(['iperf'], 18) + with self.assertRaisesRegex(RuntimeError, 'iperf timed out after 18s'): + net.check_iperf() + + def test_unreachable_port_is_not_read_as_missing_server(self): + self.connect.side_effect = TimeoutError() + with self.assertRaises(TimeoutError): + net.check_iperf() + self.run.assert_not_called() + + +class ReportCell(unittest.TestCase): + def setUp(self): + import usbtest_harness # noqa: F401 - stubs serial before hil_test is imported + import hil_test + self.hil_test = hil_test + + def cell(self, stdout): + ret = subprocess.CompletedProcess([], 0, stdout, '') + with patch.object(self.hil_test.hil_util, 'run_cmd', return_value=ret), \ + patch('sys.stdout', new_callable=io.StringIO): + return self.hil_test.test_device_net_lwip_webserver({'uid': 'ABC123'}) + + def test_iperf_rate_is_the_cell(self): + self.assertEqual(self.cell('HTTP: 6 responses verified, 7425 bytes\niperf: 3.5 Mbit/s host to device\n'), + '\u2705 3.5 Mbit/s') + + def test_no_iperf_server_keeps_the_plain_cell(self): + self.assertIsNone(self.cell('HTTP: 6 responses verified, 7425 bytes\niperf: port 5001 refused, no server\n')) + + if __name__ == '__main__': unittest.main()