[AI Generated] network: verify SR-IOV VF debugfs diagnostics - #4707
Open
VENNELA2132 wants to merge 2 commits into
Open
VENNELA2132 wants to merge 2 commits into
VENNELA2132 wants to merge 2 commits into
Conversation
Add verify_sriov_debugfs_diagnostics to the SR-IOV suite. It mounts debugfs if needed, resolves the VF's bound driver, then reads back every readable file under that driver's debugfs tree, each read bounded by a byte cap and a timeout. It asserts only what every driver owes its debugfs entries, which is that they can be read, since layouts differ by orders of magnitude between drivers and any rule written around the size or naming of one of them rejects the rest. A tree that is absent, or holds nothing but empty files, is skipped, because the driver published nothing to read back. A read that returns an error is logged rather than failed, since some entries are command interfaces that answer only once a command has been staged through them and a kernel in lockdown refuses every debugfs entry that is not read-only, while a read that never answers is a failure. A VF with no driver bound is split on the evidence in the kernel log. A logged probe failure against the VF means a driver was present and refused the device, so the VM has lost accelerated networking and the case fails with that log line. Silence means no driver in the kernel ever claimed the VF, so the distro predates it and the case skips. The VF modalias is not used to make this split, because a driver built into the kernel resolves to no module and would be mistaken for an absent one. Generated by the LISA Test Writer prompt. Reviewers: verify AAA logic, metadata, requirements, and node hygiene. Signed-off-by: Erni Sri Satya Vennela <ernis@microsoft.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The new test has a few failure/skip and timeout-handling issues that can cause misleading outcomes or premature command timeouts in large trees.
Key Test Cases:
verify_sriov_debugfs_diagnostics|verify_sriov_basic
Impacted LISA Features:
Sriov, NetworkInterface
Tested Azure Marketplace Images:
- canonical 0001-com-ubuntu-server-jammy 22_04-lts-gen2 latest
- canonical ubuntu-24_04-lts server latest
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds a new SR-IOV test case to validate that SR-IOV VF driver debugfs diagnostics are readable (bounded by per-file timeout and byte cap), aiming to catch “hung read” defects that block production triage.
Changes:
- Add
verify_sriov_debugfs_diagnosticsto mount debugfs if needed, resolve the VF’s bound driver, locate the driver’s debugfs subtree, and read all readable files with bounds. - Add helper
_read_debugfs_treeto enumerate readable files and capture per-file read outcomes (bytes read vs. error/timeout).
File summaries
| File | Description |
|---|---|
| lisa/microsoft/testsuites/network/sriov.py | Adds the new debugfs diagnostics test and a helper to walk/read the driver debugfs tree. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
LiliDeng
marked this pull request as ready for review
September 9, 2026 00:41
LiliDeng
requested review from
LiliDeng and
Johnson (johnsongeorge-w)
as code owners
September 9, 2026 00:41
LiliDeng
reviewed
Sep 16, 2026
Signed-off-by: Erni Sri Satya Vennela <ernis@microsoft.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved security and reliability issues remain in temporary-file handling, traversal status reporting, and timeout enforcement.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (2)
lisa/microsoft/testsuites/network/sriov.py:1217
- Major: the trailing
rm -fmakes the combined shell command return success even whenfindfails, soassert_exit_code(0)cannot detect an enumeration/traversal error and the test may silently inspect only a partial tree. Preserve thefindstatus, clean the scratch file, and then exit with the saved status.
f"-exec sh -c '{per_file}' _ {{}} '{scratch}' \\;; "
f"rm -f '{scratch}'",
lisa/microsoft/testsuites/network/sriov.py:1186
- Major: The raw
timeoutcommand is not checked before the walk. If it is absent or does not support-k, everyheadinvocation produces an infrastructure error (for example, exit 127), which is encoded as a refused read; when this happens for the whole tree, the laterSkippedExceptionmakes the case look like an expected driver with no readable content. Validate the Timeout tool up front or distinguish missing/invalid timeout failures from driver read errors so the test never passes by skipping without enforcing its bound.
per_file = (
f"if timeout -k {self.DEBUGFS_READ_KILL_GRACE_SECONDS} "
f"{self.DEBUGFS_READ_TIMEOUT_SECONDS} "
f'head -c {self.DEBUGFS_READ_CAP_BYTES} "$1" > "$2" 2>/dev/null; '
- Files reviewed: 1/1 changed files
- Comments generated: 2
- Review effort level: Lite
| ) -> List[Tuple[str, int]]: | ||
| # Emits "<byte_count>\t<path>" per readable file and | ||
| # "ERR<exit_code>\t<path>" per file whose read fails. | ||
| scratch = "/tmp/lisa_debugfs_read" |
Comment on lines
+1209
to
+1212
| overall_timeout = ( | ||
| candidate_count | ||
| * (self.DEBUGFS_READ_TIMEOUT_SECONDS + self.DEBUGFS_READ_KILL_GRACE_SECONDS) | ||
| + self.DEBUGFS_TREE_TIMEOUT_PADDING_SECONDS |
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
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.
Description
Adds
verify_sriov_debugfs_diagnosticsto the SR-IOV suite.debugfs is the first thing anyone opens when an SR-IOV VF misbehaves in
production, so a file that a driver publishes there and then refuses to answer
is a defect regardless of what it was meant to report. This case mounts debugfs
if needed, resolves the VF's bound driver, then reads back every readable file
under that driver's debugfs tree, with each read bounded by a byte cap and a
timeout.
It asserts only what every driver owes its debugfs entries, which is that they
can be read. Layouts differ by orders of magnitude between drivers, so any rule
written around the size, naming, or structure of one driver's tree rejects the
others. Concretely, mlx5 keys its tree by PCI slot (
mlx5/<slot>/...) whilemana keys it by ordinal (
mana/1/vport0), and file counts range from 39 toover 1700 on the same fleet.
Outcomes are graded so that only real defects fail:
blocks whoever is diagnosing the VF.
command interfaces that answer only once a command has been staged through
them, and a kernel in lockdown refuses every debugfs entry that is not
read-only.
driver published nothing to read back.
failure fails, because a driver was present, refused the device, and the
VM has lost accelerated networking; silence skips, because no driver in
that kernel ever claimed the VF.
Related Issue
Type of Change
Checklist
Test Validation
Key Test Cases:
verify_sriov_debugfs_diagnostics
Impacted LISA Features:
Sriov, NetworkInterface
Tested Azure Marketplace Images:
Test Results
Run across x64 gen1, x64 gen2, and arm64 matrices in westus3, covering both
NIC drivers Azure hands out (mana and mlx5_core).
Every branch of the case was exercised on hardware rather than reasoned about:
ERR124and failed as intendedNotes for reviewers:
mark_dirty. Adebugfs mount is benign and is usually already present, but flagging it in
case the suite's convention says otherwise.
timeout -k 1), so a read that ignoresTERM cannot stall the walk. A read wedged uninterruptibly in the kernel
cannot be signalled at all and would hit LISA's own command timeout instead.
is reported rather than hidden, so the skip count is expected to be non-zero
on any broad image matrix.