Fix: Wait for ptp4l/phc2sys death before PF rebind in validation conftest#1646
Open
awilczyns wants to merge 4 commits into
Open
Fix: Wait for ptp4l/phc2sys death before PF rebind in validation conftest#1646awilczyns wants to merge 4 commits into
awilczyns wants to merge 4 commits into
Conversation
awilczyns
force-pushed
the
fix-ptp-hanging-tests
branch
2 times, most recently
from
89940b5 to
c96fe66
Compare
_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 <andrzej.wilczynski@intel.com>
awilczyns
force-pushed
the
fix-ptp-hanging-tests
branch
4 times, most recently
from
26f0ba7 to
02ef58b
Compare
…lity 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 <andrzej.wilczynski@intel.com>
…up_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 <andrzej.wilczynski@intel.com>
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 <andrzej.wilczynski@intel.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Nightly
run-pytest (e810, ptp)jobs on e810 runners (observed onmtl-runner-2) intermittently hang the host for the rest of the night.
Root cause traced through the host journal:
ptp4lholds the PF's PHCfd, test teardown killed it with a blind
sleep(0.3), and if the kernelhadn't yet released the fd,
nicctl's VF/PF rebind checks (bind_kernel/
disable_vf) timed out and fell back to a PCI remove+rescan. Thatforce-reprobes the PF (
ice_probe) outside the gracefulsriov_numvfs=0teardown path and can hit an
icedriver GPF (RSS flow-profileuse-after-free in
ice_add_prof).Also adds real PTP conformance coverage: the existing
@pytest.mark.ptptest never asserted anything about PTP state, so it passed even with no
reachable grandmaster.
Changes
Fix:_reap_ptp_daemonsnow polls for actual process death(
_wait_daemon_dead) after both SIGTERM and SIGKILL instead ofsleeping a fixed duration, closing the race before callers rebind the
PF.
Test: addstests/validation/tests/single/ptp/ptp_helpers.pyandtests/validation/tests/single/ptp/grandmaster/test_ptp_conformance.py:test_mtl_internal_ptp_converges— asserts MTL's own software PTPclient (
mt_ptp.c's periodicPTP(0): delta avg ...stat line)actually leaves "not connected" and settles within tolerance.
test_mtl_and_ptp4l_agree_on_grandmaster— cross-checks thatagainst
ptp4l(via the existingptp_syncfixture) converging onthe same wire, verifying external-grandmaster interoperability.
that's a topology precondition, not a regression.
Testing
pytest --collect-onlyclean (8 tests collected, no import errors).with a confirmed-reachable grandmaster to verify the new tests assert
(rather than skip).