From 9e466b8a70845d8fd8b0de1656c20f1eecc16d23 Mon Sep 17 00:00:00 2001 From: Mikhail Chusavitin Date: Wed, 26 Aug 2026 12:44:45 +0300 Subject: [PATCH] fix(pcie): stop misreporting empty/vanished slots as speed degradation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pcie-link's retrain check compared post-retrain speed against a port's max capability without checking whether anything was actually seated downstream. A switch downstream port with no card plugged in retrains to zero lanes but still reports a reset-state speed, so it was misreported as "retrained to Gen1, capable of Gen5" — indistinguishable from a real degraded link. Now a port that was already empty before the retrain is skipped (same treatment as a disabled device: legitimately unpopulated slots are common and shouldn't fail the check), while a device that answered before the retrain and disappeared right after it still fails, with an honest "no device detected downstream" message instead of a fabricated speed reading. Co-Authored-By: Claude Sonnet 5 --- audit/internal/platform/pcie_link_check.go | 43 +++++++++++++++-- .../internal/platform/pcie_link_check_test.go | 48 +++++++++++++++++++ 2 files changed, 87 insertions(+), 4 deletions(-) diff --git a/audit/internal/platform/pcie_link_check.go b/audit/internal/platform/pcie_link_check.go index 808b79b..e4951e2 100644 --- a/audit/internal/platform/pcie_link_check.go +++ b/audit/internal/platform/pcie_link_check.go @@ -35,6 +35,7 @@ type pcieLinkFinding struct { Width int MaxWidth int Degraded bool + NotPresent bool // true when the slot trained to zero lanes: nothing is plugged in (or it fell off the bus), not a speed regression } // RunPCIeLinkCheckPack forces every enabled PCIe device to retrain its link @@ -139,6 +140,20 @@ func retrainAndSamplePCIeDevice(ctx context.Context, verboseLog, bdf string, log return f } + if width == 0 { + // A downstream switch/root port with nothing seated reads zero + // trained lanes even before we touch it. Plenty of legitimate + // configs leave slots like this unpopulated (not every server ships + // every NIC/riser slot filled), so this is not by itself evidence + // of anything wrong — retraining an empty slot can't produce a + // meaningful speed reading, and there's no baseline here to say + // "this used to have a card." Skip it exactly like a disabled + // device: nothing to verify, no reason to fail the run over it. + f.NotPresent = true + f.Skipped = "no device present downstream (empty slot/riser — nothing to retrain)" + return f + } + if err := retrainPCIeLink(ctx, verboseLog, bdf, logFunc); err != nil { f.Skipped = "retrain failed: " + err.Error() f.AfterSpeed = before @@ -151,6 +166,17 @@ func retrainAndSamplePCIeDevice(ctx context.Context, verboseLog, bdf string, log after, _ := readPCIeSysfsString(bdf, "current_link_speed") widthAfter, _ := readPCIeSysfsInt(bdf, "current_link_width") + if widthAfter == 0 { + // The device answered before the retrain but is gone immediately + // after it (fell off the bus mid-check) — unlike the pre-retrain + // case above, this had a live link a moment ago, so it's worth + // surfacing rather than silently skipping. + f.AfterSpeed = after + f.Width = widthAfter + f.NotPresent = true + f.Degraded = true + return f + } f.AfterSpeed = after f.Width = widthAfter f.Degraded = after != maxSpeed @@ -315,10 +341,16 @@ func renderPCIeLinkCheckSummary(findings []pcieLinkFinding) string { fmt.Fprintln(&b, "overall_status=FAILED") var reasons []string for _, f := range findings { - if f.Degraded { - reasons = append(reasons, fmt.Sprintf("%s (%s): retrained to %s, capable of %s", - f.BDF, nonEmptyOr(f.Description, "unknown device"), f.AfterSpeed, f.MaxSpeed)) + if !f.Degraded { + continue } + if f.NotPresent { + reasons = append(reasons, fmt.Sprintf("%s (%s): no device detected downstream (link down / empty slot or riser, capable of %s)", + f.BDF, nonEmptyOr(f.Description, "unknown device"), f.MaxSpeed)) + continue + } + reasons = append(reasons, fmt.Sprintf("%s (%s): retrained to %s, capable of %s", + f.BDF, nonEmptyOr(f.Description, "unknown device"), f.AfterSpeed, f.MaxSpeed)) } fmt.Fprintf(&b, "warnings=%s\n", strings.Join(reasons, "; ")) } else { @@ -340,7 +372,10 @@ func renderPCIeLinkCheckReport(findings []pcieLinkFinding) string { continue } verdict := "OK" - if f.Degraded { + switch { + case f.NotPresent: + verdict = "FELL OFF BUS" + case f.Degraded: verdict = "DEGRADED" } fmt.Fprintf(&b, " %s: before=%s after=%s max=%s width=%d/%d\n", diff --git a/audit/internal/platform/pcie_link_check_test.go b/audit/internal/platform/pcie_link_check_test.go index c4fb17d..273b56c 100644 --- a/audit/internal/platform/pcie_link_check_test.go +++ b/audit/internal/platform/pcie_link_check_test.go @@ -64,3 +64,51 @@ func TestRenderPCIeLinkCheckSummaryAllClean(t *testing.T) { t.Fatalf("disabled/skipped-only device should not produce an other_status line, got:\n%s", summary) } } + +func TestRenderPCIeLinkCheckSummaryEmptySlotDoesNotFail(t *testing.T) { + // An unpopulated switch downstream port (no card ever seated) is a + // normal, common configuration — it must not fail the SAT the way a + // genuinely degraded link does. + findings := []pcieLinkFinding{ + {BDF: "0000:0d:00.0", IsGPU: true, GPUVendor: "nvidia", BeforeSpeed: "Gen5", AfterSpeed: "Gen5", MaxSpeed: "Gen5"}, + {BDF: "0000:2a:00.0", Description: "PEX890xx PCIe Gen 5 Switch", + NotPresent: true, Skipped: "no device present downstream (empty slot/riser — nothing to retrain)"}, + } + summary := renderPCIeLinkCheckSummary(findings) + if !strings.Contains(summary, "overall_status=OK") { + t.Fatalf("expected overall_status=OK for an empty slot, got:\n%s", summary) + } + if strings.Contains(summary, "other_status=") { + t.Fatalf("empty-slot-only device should not produce an other_status line, got:\n%s", summary) + } + + report := renderPCIeLinkCheckReport(findings) + if !strings.Contains(report, "skipped: no device present downstream") { + t.Fatalf("expected empty slot to be reported as skipped, not degraded, got:\n%s", report) + } + if strings.Contains(report, "retrained to") { + t.Fatalf("empty slot must not be reported with a fabricated retrain speed, got:\n%s", report) + } +} + +func TestRenderPCIeLinkCheckSummaryDeviceFellOffBusStillFails(t *testing.T) { + // A device that had a live link before the retrain and is gone right + // after it is a real regression, distinct from a slot that was never + // populated — this must still fail the check. + findings := []pcieLinkFinding{ + {BDF: "0000:ab:00.0", Description: "PEX890xx PCIe Gen 5 Switch", + BeforeSpeed: "Gen5", AfterSpeed: "Gen5", MaxSpeed: "Gen5", Width: 0, NotPresent: true, Degraded: true}, + } + summary := renderPCIeLinkCheckSummary(findings) + if !strings.Contains(summary, "overall_status=FAILED") { + t.Fatalf("expected overall_status=FAILED when a device disappears after retrain, got:\n%s", summary) + } + if !strings.Contains(summary, "no device detected downstream") { + t.Fatalf("expected an honest 'no device detected' reason, not a fabricated speed, got:\n%s", summary) + } + + report := renderPCIeLinkCheckReport(findings) + if !strings.Contains(report, "FELL OFF BUS") { + t.Fatalf("expected FELL OFF BUS verdict in report, got:\n%s", report) + } +}