fix(pcie): stop misreporting empty/vanished slots as speed degradation
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 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 5
parent
ccc781856e
commit
9e466b8a70
@@ -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,11 +341,17 @@ func renderPCIeLinkCheckSummary(findings []pcieLinkFinding) string {
|
||||
fmt.Fprintln(&b, "overall_status=FAILED")
|
||||
var reasons []string
|
||||
for _, f := range findings {
|
||||
if f.Degraded {
|
||||
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 {
|
||||
fmt.Fprintln(&b, "overall_status=OK")
|
||||
@@ -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",
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user