From 3307309aeb37eb223155d10fbc0323300737b0e9 Mon Sep 17 00:00:00 2001 From: "Wilczynski, Andrzej" Date: Mon, 6 Jul 2026 11:49:30 +0000 Subject: [PATCH 1/4] Fix: Wait for ptp4l/phc2sys death before PF rebind in conftest _reap_ptp_daemons used a blind 0.3s sleep between SIGTERM and SIGKILL, which does not guarantee the kernel has released the PF netdev/PHC fd ptp4l held. nicctl's VF/PF rebind checks (bind_kernel/disable_vf) can still see the PF busy, time out, and fall back to a PCI remove+rescan that force-reprobes the PF (ice_probe) -- observed on mtl-runner-2 to hit an ice driver GPF (RSS flow-profile UAF in ice_add_prof), hanging the host for the rest of the night. _wait_daemon_dead() now polls for actual process death after both SIGTERM and SIGKILL instead of sleeping a fixed duration, closing the race before callers touch the PF again. Signed-off-by: Wilczynski, Andrzej --- tests/validation/conftest.py | 44 +++++++++++++++++++++++++++++++++--- 1 file changed, 41 insertions(+), 3 deletions(-) diff --git a/tests/validation/conftest.py b/tests/validation/conftest.py index e78ce5f81..45ae8812a 100755 --- a/tests/validation/conftest.py +++ b/tests/validation/conftest.py @@ -376,13 +376,39 @@ def _select_capture_host(hosts: dict): return hosts["client"] if "client" in hosts else list(hosts.values())[0] -_REAP_GRACE_SEC = 0.3 # Grace period between SIGTERM and SIGKILL for ptp daemons +_REAP_POLL_TIMEOUT_SEC = 3 # Max time to wait for a killed daemon to actually exit _PHC_SYNC_THRESHOLD_NS = 2000 # Capture PHC must track TAI this tightly _PHC_SYNC_TIMEOUT_SEC = 30 # Max wait for phc2sys to converge before capturing +def _wait_daemon_dead(host, name: str, timeout_s: float) -> bool: + """Poll (best-effort) until no process named *name* remains, up to *timeout_s*. + + A bare ``sleep()`` after ``pkill`` does not guarantee the kernel has + finished tearing down the process -- and releasing any PF netdev/PHC fd + it held (e.g. ptp4l on the PF interface) -- by the time the caller + proceeds. That gap has been observed to race nicctl's VF/PF rebind + checks (``_wait_vfio_idle`` / ``bind_kernel`` / ``disable_vf``): if the + fd is still open, those calls time out and fall back to a PCI + remove+rescan, which force-reprobes the PF (``ice_probe``) and can hit + an ``ice`` driver GPF (RSS flow-profile UAF in ``ice_add_prof``). + Polling for actual daemon death closes that race. Returns True once + confirmed dead (or if liveness can't be probed); False on timeout. + """ + deadline = time.monotonic() + timeout_s + while time.monotonic() < deadline: + try: + res = host.connection.execute_command(f"pgrep -x {name}", expected_return_codes=None) + except Exception: + return True # cannot probe; don't block the caller forever + if res.return_code != 0: + return True + time.sleep(0.1) + return False + + def _reap_ptp_daemons(host, *, patterns=("phc2sys", "ptp4l")) -> None: - """Forcefully kill any ptp4l/phc2sys daemons. + """Forcefully kill any ptp4l/phc2sys daemons and wait for them to exit. Required because ``host.connection.start_process('sudo ...')`` wraps the daemon in ``bash -c 'sudo ...'``; ``process.kill(SIGTERM)`` only signals @@ -391,6 +417,9 @@ def _reap_ptp_daemons(host, *, patterns=("phc2sys", "ptp4l")) -> None: stale ``struct ptp_clock *`` across SR-IOV VF cycling and have been seen to trigger ``ice``-driver use-after-free in ``ptp_clock_index()``, hanging the host. Always cleanup via ``pkill`` on the argv, not via the process handle. + We also wait for the kill to actually take effect (see + :func:`_wait_daemon_dead`) instead of a blind sleep, since callers rebind + the PF right after this returns. """ for name in patterns: try: @@ -399,7 +428,8 @@ def _reap_ptp_daemons(host, *, patterns=("phc2sys", "ptp4l")) -> None: ) except Exception as e: logger.debug("pkill -TERM %s: %s", name, e) - time.sleep(_REAP_GRACE_SEC) + for name in patterns: + _wait_daemon_dead(host, name, _REAP_POLL_TIMEOUT_SEC) for name in patterns: try: host.connection.execute_command( @@ -407,6 +437,14 @@ def _reap_ptp_daemons(host, *, patterns=("phc2sys", "ptp4l")) -> None: ) except Exception as e: logger.debug("pkill -KILL %s: %s", name, e) + for name in patterns: + if not _wait_daemon_dead(host, name, _REAP_POLL_TIMEOUT_SEC): + logger.warning( + "%s on %s still alive %ss after SIGKILL; PF rebind may race it", + name, + host.name, + _REAP_POLL_TIMEOUT_SEC, + ) def _host_tai_utc_offset(host) -> int: From 02ef58b72083706913212d2c0c9d47dc510629fd Mon Sep 17 00:00:00 2001 From: "Wilczynski, Andrzej" Date: Tue, 7 Jul 2026 16:56:16 +0000 Subject: [PATCH 2/4] tests(validation): improve physical port PTP sync and fix IOMMU viability crash - Dynamically probe for interface presence in /sys/class/net inside conftest.py to skip starting ptp4l on ports bound to DPDK PMDs. - Gracefully skip single-host mixed-mode (PF/VF) tests when physical functions share an IOMMU group, resolving the EAL/VFIO viability failure on shared environments. - Enforce strict non-empty capture compliance checks in pytest validation run. - Add ptp_sync request to mixed-mode PT/VF test signature. Signed-off-by: Wilczynski, Andrzej --- tests/validation/common/nicctl.py | 39 ++++++++ tests/validation/conftest.py | 91 ++++++++++++++----- .../test_st20_interfaces_mix_refactored.py | 1 + 3 files changed, 107 insertions(+), 24 deletions(-) diff --git a/tests/validation/common/nicctl.py b/tests/validation/common/nicctl.py index 334bdd7a5..4a4bf4aa9 100644 --- a/tests/validation/common/nicctl.py +++ b/tests/validation/common/nicctl.py @@ -382,6 +382,45 @@ def get_mixed_interfaces_list_single( f"Found {len(host.network_interfaces)} interface(s)." ) + tx_pci = host.network_interfaces[tx_index].pci_address.lspci + rx_pci = host.network_interfaces[rx_index].pci_address.lspci + + # Check IOMMU group of TX and RX PFs. + # One PF is bound to PMD (vfio-pci), while the other PF remains bound to the kernel (host for run/VFs). + # This is impossible if they share the same IOMMU group because VFIO group viability is violated. + try: + tx_group = ( + self.nicctl_objs[host.name] + .connection.execute_command( + f"basename $(readlink /sys/bus/pci/devices/{tx_pci}/iommu_group 2>/dev/null) 2>/dev/null" + ) + .stdout + or "" + ).strip() + rx_group = ( + self.nicctl_objs[host.name] + .connection.execute_command( + f"basename $(readlink /sys/bus/pci/devices/{rx_pci}/iommu_group 2>/dev/null) 2>/dev/null" + ) + .stdout + or "" + ).strip() + if tx_group and rx_group and tx_group == rx_group: + if ( + tx_interface_type.lower() == "pf" + and rx_interface_type.lower() == "vf" + ) or ( + tx_interface_type.lower() == "vf" + and rx_interface_type.lower() == "pf" + ): + pytest.skip( + f"Skipping mixed PF/VF test: PF {tx_pci} and PF {rx_pci} share the same IOMMU group " + f"({tx_group}) " + f"and cannot be bound to different drivers (vfio-pci vs ice) simultaneously." + ) + except Exception as e: + logger.warning(f"Failed to check IOMMU group conflict: {e}") + tx_interface = self._get_single_interface_by_type( host, tx_interface_type, tx_index ) diff --git a/tests/validation/conftest.py b/tests/validation/conftest.py index 45ae8812a..b23c1720f 100755 --- a/tests/validation/conftest.py +++ b/tests/validation/conftest.py @@ -398,7 +398,9 @@ def _wait_daemon_dead(host, name: str, timeout_s: float) -> bool: deadline = time.monotonic() + timeout_s while time.monotonic() < deadline: try: - res = host.connection.execute_command(f"pgrep -x {name}", expected_return_codes=None) + res = host.connection.execute_command( + f"pgrep -x {name}", expected_return_codes=None + ) except Exception: return True # cannot probe; don't block the caller forever if res.return_code != 0: @@ -609,30 +611,71 @@ def ptp_sync(request, test_config: dict, hosts): host = _select_capture_host(hosts) is_single_host = len(hosts) == 1 - capture_iface = _select_sniff_interface_name( - host, capture_cfg, single_host=is_single_host - ) # Belt-and-braces: ensure no leftover daemon from a previous test/session # is holding a stale PHC handle before we start a new one. _reap_ptp_daemons(host) - logger.info(f"Starting ptp4l for PTP synchronization (iface={capture_iface})") - log_path = f"/tmp/ptp4l-{capture_iface}.log" - ptp4l_cmd = f"sudo ptp4l -i '{capture_iface}' -s -m -2" - ptp4l_process = host.connection.start_process( - ptp4l_cmd, - stderr_to_stdout=True, - output_file=log_path, - ) + # Determine interfaces to sync. If we are running a single-host PTP loopback test, + # we need to sync BOTH the active ports (TX on primary, RX on redundant) so that + # the hardware clocks on both ports are disciplined to the same GM. + # Note: Only sync interfaces that are currently in kernel mode (i.e. having active netdevs in /sys/class/net), + # otherwise ptp4l will fail to start. + interfaces_to_sync = [] + if is_single_host: + # In single-host mode, use both the primary and redundant physical interfaces if available + for nic in host.network_interfaces[:2]: + try: + # Check if the interface is currently in kernel mode + check_cmd = f"[ -d /sys/class/net/{nic.name} ]" + res = host.connection.execute_command( + check_cmd, expected_return_codes=None + ) + if res.return_code == 0: + interfaces_to_sync.append(nic.name) + else: + logger.debug( + f"PTP skip: Interface {nic.name} is not present in /sys/class/net/ (likely bound to DPDK PMD)" + ) + except Exception as e: + logger.warning( + f"Failed to check netdev directories for {nic.name}: {e}" + ) + else: + # Multi-host mode: only run on the resolved capture interface + capture_iface = _select_sniff_interface_name( + host, capture_cfg, single_host=is_single_host + ) + try: + # Check if the interface is currently in kernel mode + check_cmd = f"[ -d /sys/class/net/{capture_iface} ]" + res = host.connection.execute_command(check_cmd, expected_return_codes=None) + if res.return_code == 0: + interfaces_to_sync.append(capture_iface) + else: + logger.warning( + f"PTP skip: Capture interface {capture_iface} is not present in /sys/class/net/" + ) + except Exception as e: + logger.warning( + f"Failed to check netdev directories for capture interface {capture_iface}: {e}" + ) - # Give ptp4l a moment to fail fast (e.g., missing interface). - time.sleep(0.2) - if not ptp4l_process.running: - _reap_ptp_daemons(host) - raise RuntimeError( - f"Failed to start ptp4l (iface={capture_iface}). log={log_path}" + ptp4l_processes = [] + for iface in interfaces_to_sync: + logger.info(f"Starting ptp4l for PTP synchronization (iface={iface})") + log_path = f"/tmp/ptp4l-{iface}.log" + ptp4l_cmd = f"sudo ptp4l -i '{iface}' -s -m -2" + ptp4l_process = host.connection.start_process( + ptp4l_cmd, + stderr_to_stdout=True, + output_file=log_path, ) + time.sleep(0.2) + if not ptp4l_process.running: + _reap_ptp_daemons(host) + raise RuntimeError(f"Failed to start ptp4l (iface={iface}). log={log_path}") + ptp4l_processes.append(ptp4l_process) try: yield @@ -1284,12 +1327,12 @@ def pcap_capture( streams = (report or {}).get("streams") or [] if not streams: # Empty capture — interface may not see VF-to-VF - # loopback traffic. Not a real failure. - update_compliance_result(request.node.nodeid, "N/A") - logger.warning( - "PCAP compliance check skipped: capture " - "contains no streams (capture interface may " - "not see VF-to-VF loopback traffic)" + # loopback traffic. Reject with a test failure so we catch false passes. + update_compliance_result(request.node.nodeid, "Fail") + log_fail( + "PCAP compliance check failed: capture contains no streams. " + "Normally interface may not see VF-to-VF loopback traffic, " + "but empty capture is disallowed for strict validation/compliance." ) else: update_compliance_result(request.node.nodeid, "Fail") diff --git a/tests/validation/tests/single/ptp/st20_interfaces_mix/test_st20_interfaces_mix_refactored.py b/tests/validation/tests/single/ptp/st20_interfaces_mix/test_st20_interfaces_mix_refactored.py index 58802468f..5af18c8bd 100644 --- a/tests/validation/tests/single/ptp/st20_interfaces_mix/test_st20_interfaces_mix_refactored.py +++ b/tests/validation/tests/single/ptp/st20_interfaces_mix/test_st20_interfaces_mix_refactored.py @@ -42,6 +42,7 @@ def test_st20_interfaces_mix_refactored( test_time, interface_profile, test_config, + ptp_sync, pcap_capture, media_file, output_files, From 96ed3d2ef89b4daef83292015ff7b5fc8f7aa47f Mon Sep 17 00:00:00 2001 From: "Wilczynski, Andrzej" Date: Wed, 8 Jul 2026 15:31:38 +0000 Subject: [PATCH 3/4] Fix: Detect .local_install RxTxApp/MtlManager and 1G hugepages in setup_validation.sh Preflight hardcoded tests/tools/RxTxApp/build/RxTxApp and build/manager/MtlManager, but mtl_engine/const.py::PREFIX actually invokes .local_install/mtl/bin/* -- preflight reported MISSING on hosts built that way even though pytest itself worked fine. HugePages_Free*2 also silently under-reports free hugepage MiB on hosts booted with default_hugepagesz=1G, since the multiplier assumes 2MB pages; now reads Hugepagesize from /proc/meminfo instead of assuming. Signed-off-by: Wilczynski, Andrzej --- .../mtl-validation-tests.instructions.md | 2 ++ .github/scripts/setup_validation.sh | 27 +++++++++++++++---- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/.github/instructions/mtl-validation-tests.instructions.md b/.github/instructions/mtl-validation-tests.instructions.md index dccd64c6e..4726a2b2d 100644 --- a/.github/instructions/mtl-validation-tests.instructions.md +++ b/.github/instructions/mtl-validation-tests.instructions.md @@ -104,6 +104,8 @@ sudo grep -E "EAL|hugepage|VF|RxTxApp|RemoteProcess|Traceback|err:" \ | RxTxApp `Segmentation fault` inside `iavf_tm_node_add` (after `dev_if_init_pacing(0), try rl as drv support TM`) | **(setup)** Stock kernel ice loaded instead of the MTL out-of-tree patched ice (`versions.env::ICE_VER`). Re-run `setup_validation.sh` — the ice stage version-checks and reloads automatically. | | RxTxApp `Segmentation fault` anywhere else | **NOT setup.** Capture `gdb -batch -ex 'bt full' tests/tools/RxTxApp/build/RxTxApp /tmp/core.*` (or `coredumpctl gdb RxTxApp`) and report upstream as a real MTL/DPDK bug. Do **not** add a workaround. | | `Permission denied (publickey)` to `root@127.0.0.1` | **(setup)** Pubkey not in `/root/.ssh/authorized_keys`. | +| `preflight: MtlManager or RxTxApp missing` despite binaries present under `.local_install/mtl/bin/` | **(setup)** Fixed: preflight now checks both the legacy in-tree `build/` path and the `.local_install` prefix that `mtl_engine/const.py::PREFIX` actually invokes. Re-run setup. | +| `preflight: hugepages free is 64 MiB (<1024 MiB)` on a host with 1GB hugepages configured (`default_hugepagesz=1G` on kernel cmdline) | **(setup)** Fixed: hugepage check now reads `Hugepagesize` from `/proc/meminfo` instead of assuming 2MB pages. Re-run setup. | **(setup)** = delegate to the `MTL Validation Setup` subagent (it re-runs `setup_validation.sh` idempotently). diff --git a/.github/scripts/setup_validation.sh b/.github/scripts/setup_validation.sh index 9ae0fa0ed..c4659c901 100755 --- a/.github/scripts/setup_validation.sh +++ b/.github/scripts/setup_validation.sh @@ -172,8 +172,8 @@ print_summary() { "$CYN" "$CLR" "$s" "${STAGE_RESULT[$s]:-?}" "${STAGE_DURATION[$s]:-?}" >&2 done log "" - log " RxTxApp : $([[ -x tests/tools/RxTxApp/build/RxTxApp ]] && echo OK || echo MISSING)" - log " MtlManager : $([[ -x build/manager/MtlManager ]] && echo OK || echo MISSING)" + log " RxTxApp : $(mtl_rxtxapp_present && echo OK || echo MISSING)" + log " MtlManager : $(mtl_manager_present && echo OK || echo MISSING)" if ldconfig -p 2>/dev/null | grep -Eq 'libmtl\.so(\s|$)' || [[ -f /usr/local/lib/x86_64-linux-gnu/libmtl.so || -f /usr/local/lib64/libmtl.so || -f /usr/local/lib/libmtl.so ]]; then log " libmtl.so : OK" else @@ -181,7 +181,7 @@ print_summary() { fi log " libdpdk : $(pkg-config --modversion libdpdk 2>/dev/null || echo MISSING)" log " ice driver : $(modinfo ice 2>/dev/null | awk '/^version:/ {print $2; exit}' || echo MISSING) @ $(modinfo -n ice 2>/dev/null || echo '')" - log " hugepages free : $(awk '/HugePages_Free/ {print $2*2 " MiB"}' /proc/meminfo)" + log " hugepages free : $(hugepages_free_mb) MiB" if mountpoint -q /mnt/media; then log " /mnt/media : $(findmnt -no SOURCE /mnt/media) ($(df -h /mnt/media | awk 'NR==2{print $5" used of "$2}'))" log " media files : $(find /mnt/media -mindepth 1 -maxdepth 1 2>/dev/null | wc -l) entries" @@ -195,6 +195,23 @@ print_summary() { trap_arm } +# RxTxApp/MtlManager may live at the legacy in-tree build path or at the +# .local_install prefix that tests/validation/mtl_engine/const.py::PREFIX +# actually invokes — accept either so preflight matches what pytest runs. +mtl_rxtxapp_present() { + [[ -x tests/tools/RxTxApp/build/RxTxApp || -x .local_install/mtl/bin/RxTxApp ]] +} + +mtl_manager_present() { + [[ -x build/manager/MtlManager || -x .local_install/mtl/bin/MtlManager ]] +} + +# Hugepage size varies by host (2MB vs 1GB default_hugepagesz=1G on the +# kernel cmdline); HugePages_Free*2 silently under-reports on 1GB-page hosts. +hugepages_free_mb() { + awk '/Hugepagesize:/ {sz=$2} /HugePages_Free:/ {free=$2} END {printf "%d", free*sz/1024}' /proc/meminfo +} + # ============================================================================ # STAGE FUNCTIONS # ============================================================================ @@ -260,7 +277,7 @@ stage_preflight() { warn "preflight: libmtl.so missing in ld cache" missing=1 fi - if [[ ! -x build/manager/MtlManager || ! -x tests/tools/RxTxApp/build/RxTxApp ]]; then + if ! mtl_manager_present || ! mtl_rxtxapp_present; then warn "preflight: MtlManager or RxTxApp missing" missing=1 fi @@ -269,7 +286,7 @@ stage_preflight() { warn "preflight: out-of-tree ice driver not loaded (path=$ice_path)" missing=1 fi - free_mb=$(awk '/HugePages_Free/ {print $2*2}' /proc/meminfo) + free_mb=$(hugepages_free_mb) if ((free_mb < 1024)); then warn "preflight: hugepages free is ${free_mb} MiB (<1024 MiB)" missing=1 From f1e0a2e9d74702d33a19e2e889cc240e8b1481d5 Mon Sep 17 00:00:00 2001 From: "Wilczynski, Andrzej" Date: Wed, 8 Jul 2026 15:32:00 +0000 Subject: [PATCH 4/4] Add: PTP grandmaster convergence check to ptp_sync fixture ptp_sync started ptp4l on kernel-bound interfaces but only checked the process stayed alive 0.2s after spawn -- never that it actually locked onto a grandmaster. A free-running/unsynced PHC produced a real EBU compliance failure (~-55.8s packet_ts_vs_rtp_ts offset, 2110_21_vrx non-compliant) that only surfaced after a multi-minute run. ptp_sync now blocks on wait_for_ptp4l_foreign_master() for every interface it starts ptp4l on, before yielding to the test body, and reaps on timeout so a dead grandmaster cannot orphan the daemon into PID 1 (the same leak class that previously triggered an ice driver use-after-free and a BMC reset on mtl-runner-2/3). require_grandmaster no longer duplicates this wait since ptp_sync (its own dependency) already guarantees it. Also adds the grandmaster/ PTP conformance test package (test_mtl_internal_ptp_converges, test_mtl_and_ptp4l_agree_on_grandmaster) and shared ptp_helpers.py assertion helpers. Signed-off-by: Wilczynski, Andrzej --- tests/validation/conftest.py | 47 +++++- .../tests/single/ptp/grandmaster/__init__.py | 2 + .../tests/single/ptp/grandmaster/conftest.py | 33 +++++ .../ptp/grandmaster/test_ptp_conformance.py | 130 +++++++++++++++++ .../tests/single/ptp/ptp_helpers.py | 137 ++++++++++++++++++ 5 files changed, 348 insertions(+), 1 deletion(-) create mode 100644 tests/validation/tests/single/ptp/grandmaster/__init__.py create mode 100644 tests/validation/tests/single/ptp/grandmaster/conftest.py create mode 100644 tests/validation/tests/single/ptp/grandmaster/test_ptp_conformance.py create mode 100644 tests/validation/tests/single/ptp/ptp_helpers.py diff --git a/tests/validation/conftest.py b/tests/validation/conftest.py index b23c1720f..3e99cba2f 100755 --- a/tests/validation/conftest.py +++ b/tests/validation/conftest.py @@ -55,6 +55,7 @@ ) from pytest_mfd_config.models.topology import TopologyModel from pytest_mfd_logging.amber_log_formatter import AmberLogFormatter +from tests.single.ptp.ptp_helpers import wait_for_ptp4l_foreign_master logger = logging.getLogger(__name__) @@ -621,10 +622,39 @@ def ptp_sync(request, test_config: dict, hosts): # the hardware clocks on both ports are disciplined to the same GM. # Note: Only sync interfaces that are currently in kernel mode (i.e. having active netdevs in /sys/class/net), # otherwise ptp4l will fail to start. + # + # Also: some tests (e.g. the "mixed" pf_tx_vf_rx interface_profile) bind an + # interface fully to DPDK (as a PF, no VF) from inside the test BODY, which + # runs *after* this fixture. At fixture-setup time that interface is still + # kernel-bound, so the /sys/class/net check below would pass and we'd start + # ptp4l on it -- only for the test body's own bind_pmd() to yank the netdev + # away moments later, mid-convergence. ptp4l then faults (link down) and + # self-declares grandmaster instead of tracking the real one. Skip syncing + # whichever index this test's own interface_profile will bind fully to DPDK. + # (Single-host only: the index resolution below only makes sense for the + # loopback topology; a mixed profile has never been combined with a + # multi-host `hosts` config, and both tx_type/rx_type being "PF" is not a + # combination any current test uses -- only one side checked below.) interfaces_to_sync = [] if is_single_host: + full_pf_dpdk_index = None + callspec = getattr(request.node, "callspec", None) + profile = callspec.params.get("interface_profile") if callspec else None + if profile and profile.get("mode") == "mixed": + tx_index = test_config.get("tx_interface_index", 0) + rx_index = test_config.get("rx_interface_index", 1) + if profile.get("tx_type", "").upper() == "PF": + full_pf_dpdk_index = tx_index + elif profile.get("rx_type", "").upper() == "PF": + full_pf_dpdk_index = rx_index + # In single-host mode, use both the primary and redundant physical interfaces if available - for nic in host.network_interfaces[:2]: + for idx, nic in enumerate(host.network_interfaces[:2]): + if idx == full_pf_dpdk_index: + logger.debug( + f"PTP skip: interface {nic.name} (index {idx}) will be bound fully to DPDK by this test" + ) + continue try: # Check if the interface is currently in kernel mode check_cmd = f"[ -d /sys/class/net/{nic.name} ]" @@ -677,6 +707,21 @@ def ptp_sync(request, test_config: dict, hosts): raise RuntimeError(f"Failed to start ptp4l (iface={iface}). log={log_path}") ptp4l_processes.append(ptp4l_process) + # A hardware PTP clock that never actually locks to a grandmaster (or + # never even hears one) free-runs -- exactly the condition that produced + # a ~-55.8s packet_ts_vs_rtp_ts offset and VRX compliance failure in the + # past (see /memories/repo/ptp_sync_fixture_root_cause.md). Block here, + # before the test body/pcap_capture ever touch this clock, so a dead/ + # unreachable grandmaster surfaces as a fast, clear ERROR. Must reap on + # timeout too, else the just-started ptp4l processes are orphaned (same + # leak class _reap_ptp_daemons/reap_leaked_phc_daemons exist to prevent). + try: + for iface in interfaces_to_sync: + wait_for_ptp4l_foreign_master(host, f"/tmp/ptp4l-{iface}.log") + except Exception: + _reap_ptp_daemons(host) + raise + try: yield finally: diff --git a/tests/validation/tests/single/ptp/grandmaster/__init__.py b/tests/validation/tests/single/ptp/grandmaster/__init__.py new file mode 100644 index 000000000..a261053ca --- /dev/null +++ b/tests/validation/tests/single/ptp/grandmaster/__init__.py @@ -0,0 +1,2 @@ +# SPDX-License-Identifier: BSD-3-Clause +# Copyright(c) 2026 Intel Corporation diff --git a/tests/validation/tests/single/ptp/grandmaster/conftest.py b/tests/validation/tests/single/ptp/grandmaster/conftest.py new file mode 100644 index 000000000..4b401f6ab --- /dev/null +++ b/tests/validation/tests/single/ptp/grandmaster/conftest.py @@ -0,0 +1,33 @@ +# SPDX-License-Identifier: BSD-3-Clause +# Copyright(c) 2026 Intel Corporation +"""Fixtures local to tests/single/ptp/grandmaster/ (per-category, not framework).""" +import logging + +import pytest + +logger = logging.getLogger(__name__) + + +@pytest.fixture +def require_grandmaster(hosts, ptp_sync): + """Return the ptp4l log path, already confirmed synced by ``ptp_sync``. + + ``ptp_sync`` itself now blocks in fixture setup until every ptp4l it + starts has heard a foreign master (see conftest.py), so this fixture no + longer needs to re-run that wait -- it only resolves and returns the log + path for callers that want to inspect it further. + """ + host = list(hosts.values())[0] + log_path = ( + host.connection.execute_command( + "ls -t /tmp/ptp4l-*.log 2>/dev/null | head -1", expected_return_codes=None + ).stdout + or "" + ).strip() + if not log_path: + raise RuntimeError( + "ENVIRONMENT NOT READY: ptp_sync did not start ptp4l (check " + "capture_cfg.enable in test_config.yaml) -- cannot verify a " + "grandmaster is reachable" + ) + return log_path diff --git a/tests/validation/tests/single/ptp/grandmaster/test_ptp_conformance.py b/tests/validation/tests/single/ptp/grandmaster/test_ptp_conformance.py new file mode 100644 index 000000000..8bcf7d0a3 --- /dev/null +++ b/tests/validation/tests/single/ptp/grandmaster/test_ptp_conformance.py @@ -0,0 +1,130 @@ +# SPDX-License-Identifier: BSD-3-Clause +# Copyright(c) 2026 Intel Corporation +"""Real PTP conformance checks. + +Unlike test_st20_interfaces_mix_refactored (which only passes ``--ptp`` and +checks video integrity), these tests assert the PTP behavior itself: + +* ``test_mtl_internal_ptp_converges`` -- MTL's own software PTP client + (``mt_ptp.c``) actually reaches the "connected" state and its reported + offset from the grandmaster is within tolerance. Single host, no external + daemon required. +* ``test_mtl_and_ptp4l_agree_on_grandmaster`` -- external-GM interoperability: + runs ``ptp4l`` (via the shared ``ptp_sync`` fixture) as an independent PTP + client on the same PF/wire MTL's VF sits on, and asserts *both* clients + converge concurrently -- i.e. MTL's software PTP client and a standard + Linux PTP stack agree the link has a working, lockable grandmaster. +""" +import logging + +import pytest +from mtl_engine.media_files import yuv_files_422rfc10 + +from ..ptp_helpers import assert_mtl_ptp_converged, assert_ptp4l_converged + +logger = logging.getLogger(__name__) + + +def _run_short_ptp_session( + app_factory, setup_interfaces, mtl_path, host, test_time, media_file +): + """Smallest possible st20p session with PTP enabled; returns the app.""" + interfaces_list = setup_interfaces.get_interfaces_list_single("VF") + media_file_info, media_file_path = media_file + app = app_factory("rxtxapp") + app.create_command( + session_type="st20p", + nic_port_list=interfaces_list, + test_mode="multicast", + width=media_file_info["width"], + height=media_file_info["height"], + framerate=f"p{media_file_info['fps']}", + pixel_format=media_file_info["file_format"], + transport_format=media_file_info["format"], + input_file=media_file_path, + enable_ptp=True, + test_time=test_time, + ) + app.execute_test(build=mtl_path, test_time=test_time, host=host) + return app + + +@pytest.mark.nightly +@pytest.mark.ptp +@pytest.mark.refactored +@pytest.mark.parametrize( + "media_file", + [yuv_files_422rfc10["Crosswalk_720p"]], + indirect=["media_file"], + ids=["Crosswalk_720p"], +) +def test_mtl_internal_ptp_converges( + hosts, + mtl_path, + setup_interfaces, + test_time, + app_factory, + media_file, + require_grandmaster, +): + """MTL's internal PTP client (``--ptp``) must actually lock, not free-run. + + Regression target: a test marked ``@pytest.mark.ptp`` that never asserts + on PTP state passes even with no reachable grandmaster. The + ``require_grandmaster`` fixture already confirmed a grandmaster is + reachable (erroring out fast if not, before this body even runs) -- so + any assertion failure here is a genuine MTL-side PTP regression. + """ + host = list(hosts.values())[0] + app = _run_short_ptp_session( + app_factory, setup_interfaces, mtl_path, host, test_time, media_file + ) + assert_mtl_ptp_converged(app.last_output, port=0) + + +@pytest.mark.nightly +@pytest.mark.ptp +@pytest.mark.refactored +@pytest.mark.parametrize( + "media_file", + [yuv_files_422rfc10["Crosswalk_720p"]], + indirect=["media_file"], + ids=["Crosswalk_720p"], +) +def test_mtl_and_ptp4l_agree_on_grandmaster( + hosts, + mtl_path, + setup_interfaces, + test_time, + ptp_sync, + app_factory, + media_file, + require_grandmaster, +): + """External-GM interoperability: MTL's PTP client and ptp4l agree. + + ``ptp_sync`` starts ``ptp4l`` (an independent, standard Linux PTP client) + on the PF whose VFs MTL uses for this session, picking the interface via + its own resolution logic (explicit ``sniff_interface``/``sniff_pci_device`` + config, or an automatic topology-based fallback -- either way it writes + its log to ``/tmp/ptp4l-.log``). Both ptp4l and MTL are passive + BMCA slaves observing the same Announce/Sync stream, so they don't fight + over the PHC (unlike ptp4l+phc2sys) and can run concurrently. + + ``require_grandmaster`` already confirmed ptp4l heard a foreign master + (erroring out fast if not) and returns its log path -- so any assertion + failure below is a genuine MTL-side or ptp4l-side regression, not a + missing-grandmaster environment issue. + """ + host = list(hosts.values())[0] + app = _run_short_ptp_session( + app_factory, setup_interfaces, mtl_path, host, test_time, media_file + ) + ptp4l_log = ( + host.connection.execute_command( + f"cat {require_grandmaster}", expected_return_codes=None + ).stdout + or "" + ) + assert_mtl_ptp_converged(app.last_output, port=0) + assert_ptp4l_converged(ptp4l_log) diff --git a/tests/validation/tests/single/ptp/ptp_helpers.py b/tests/validation/tests/single/ptp/ptp_helpers.py new file mode 100644 index 000000000..57ab5de50 --- /dev/null +++ b/tests/validation/tests/single/ptp/ptp_helpers.py @@ -0,0 +1,137 @@ +# SPDX-License-Identifier: BSD-3-Clause +# Copyright(c) 2026 Intel Corporation +"""Shared assertion helpers for PTP conformance tests. + +Parses the periodic ``PTP(): ...`` stat lines MTL's own software PTP +client (``mt_ptp.c::ptp_stat``) prints to RxTxApp's stdout, and the ``rms`` +lines ``ptp4l`` prints to its own log, so tests can assert real +synchronization happened instead of only checking the app didn't crash. +""" +import re +import time + +# mt_ptp.c auto-tunes its own error tolerance to ~2x the running average +# (min 100us); 1ms is far looser than that internal threshold, so this only +# catches a genuinely unsynced/free-running clock, not measurement noise. +MTL_PTP_DELTA_TOLERANCE_NS = 1_000_000 # 1ms +PTP4L_RMS_TOLERANCE_NS = 1_000 # ptp4l settles to single/double-digit ns on a quiet LAN + +_PTP_DELTA_RE = re.compile( + r"PTP\((\d+)\): delta avg (-?\d+), min (-?\d+), max (-?\d+), cnt (\d+)" +) +_PTP4L_RMS_RE = re.compile(r"rms\s+(\d+)\s+max\s+(\d+)") +_PTP4L_FOREIGN_MASTER_RE = re.compile(r"new foreign master") + + +def wait_for_ptp4l_foreign_master( + host, log_path: str, timeout_s: float = 15.0, poll_s: float = 1.0 +) -> None: + """Fail fast if ptp4l never hears a foreign master on *log_path*. + + A reachable external PTP grandmaster is an environment precondition, + not a per-test assertion -- so this raises ``RuntimeError`` (never + ``pytest.skip``) on timeout. Skip is reserved for genuinely unsupported + configurations; "no grandmaster reachable" means the test fabric is not + ready, which must surface as a visible failure so it gets fixed, not a + silently-passing skip. Runs BEFORE the expensive RxTxApp session so a + dead fabric fails in ~15s instead of after a multi-minute run. + """ + deadline = time.monotonic() + timeout_s + while time.monotonic() < deadline: + text = ( + host.connection.execute_command( + f"cat {log_path} 2>/dev/null", expected_return_codes=None + ).stdout + or "" + ) + if _PTP4L_FOREIGN_MASTER_RE.search(text) or _PTP4L_RMS_RE.search(text): + return + time.sleep(poll_s) + raise RuntimeError( + f"ENVIRONMENT NOT READY: ptp4l on {host.name} never heard a foreign " + f"master within {timeout_s}s (log={log_path}). No PTP grandmaster is " + "reachable on this test fabric -- fix the network/grandmaster before " + "re-running; this is not a code defect." + ) + + +def parse_mtl_ptp_delta_samples(app_output: str, port: int = 0): + """Return ``[(avg, min, max, cnt), ...]`` for ``PTP(port)`` stat samples. + + An empty list means the app never printed a delta sample for *port* -- + i.e. its internal PTP client never left the "not connected" state (most + commonly: no reachable grandmaster on this test fabric). + """ + return [ + (int(m.group(2)), int(m.group(3)), int(m.group(4)), int(m.group(5))) + for m in _PTP_DELTA_RE.finditer(app_output) + if int(m.group(1)) == port + ] + + +def mtl_ptp_connected(app_output: str, port: int = 0) -> bool: + """True if MTL's internal PTP client on *port* ever left 'not connected'. + + Only meaningful once a grandmaster's reachability has already been + confirmed (see :func:`wait_for_ptp4l_foreign_master`) -- at that point + ``False`` means an MTL-side regression, not a missing grandmaster. + """ + return bool(parse_mtl_ptp_delta_samples(app_output, port)) + + +def assert_mtl_ptp_converged( + app_output: str, port: int = 0, tolerance_ns: int = MTL_PTP_DELTA_TOLERANCE_NS +) -> None: + """Assert MTL's own internal PTP client (``--ptp``) synced within tolerance. + + Precondition: grandmaster reachability already confirmed (see + :func:`wait_for_ptp4l_foreign_master`) -- so "never connected" here is a + genuine MTL-side regression, not a missing-grandmaster environment issue. + """ + samples = parse_mtl_ptp_delta_samples(app_output, port) + assert samples, ( + f"MTL internal PTP client on port {port} never reported a delta " + f"sample (stayed in 'not connected' state) -- look for " + f"'PTP({port}): not connected' in the app output" + ) + avg, _min, _max, cnt = samples[-1] + assert abs(avg) <= tolerance_ns, ( + f"MTL internal PTP client on port {port} did not converge: last " + f"delta avg={avg}ns exceeds tolerance {tolerance_ns}ns (cnt={cnt})" + ) + + +def parse_ptp4l_rms_samples(ptp4l_log_text: str): + """Return ``[(rms_ns, max_ns), ...]`` parsed from a ``ptp4l -m`` log.""" + return [ + (int(m.group(1)), int(m.group(2))) + for m in _PTP4L_RMS_RE.finditer(ptp4l_log_text) + ] + + +def ptp4l_connected(ptp4l_log_text: str) -> bool: + """True if ptp4l ever printed an 'rms' sample (reached SLAVE state). + + Only meaningful once grandmaster reachability has already been + confirmed (see :func:`wait_for_ptp4l_foreign_master`) -- at that point + ``False`` means ptp4l lost lock after acquiring it, a real regression. + """ + return bool(parse_ptp4l_rms_samples(ptp4l_log_text)) + + +def assert_ptp4l_converged( + ptp4l_log_text: str, tolerance_ns: int = PTP4L_RMS_TOLERANCE_NS +) -> None: + """Assert ptp4l (the external-GM reference client) converged within tolerance. + + Precondition: call :func:`ptp4l_connected` first and skip the test if + it's False -- this function assumes SLAVE state was reached. + """ + samples = parse_ptp4l_rms_samples(ptp4l_log_text) + assert ( + samples + ), "ptp4l never printed an 'rms' sample -- it never reached SLAVE state" + last_rms, _last_max = samples[-1] + assert ( + last_rms <= tolerance_ns + ), f"ptp4l did not converge: last rms={last_rms}ns exceeds tolerance {tolerance_ns}ns"