fix(pcie): account for downstream link capability

2026-08-28 09:12:02 +03:00
parent af95216c6f
commit 2354599889
2 changed files with 205 additions and 15 deletions
@@ -22,20 +22,22 @@ const pcieLinkRetrainTimeout = 2 * time.Second

 // pcieLinkFinding is one device's before/after link-speed retrain result.
 type pcieLinkFinding struct {
-	BDF         string
-	Description string
-	VendorID    string
-	ClassCode   string
-	IsGPU       bool
-	GPUVendor   string // "nvidia" or "amd", only set when IsGPU
-	Skipped     string // non-empty reason this device wasn't retrained
-	BeforeSpeed string
-	AfterSpeed  string
-	MaxSpeed    string
-	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
+	BDF          string
+	Description  string
+	VendorID     string
+	ClassCode    string
+	IsGPU        bool
+	GPUVendor    string // "nvidia" or "amd", only set when IsGPU
+	Skipped      string // non-empty reason this device wasn't retrained
+	BeforeSpeed  string
+	AfterSpeed   string
+	MaxSpeed     string
+	PortMaxSpeed string // bridge's own capability when MaxSpeed is limited by its downstream peer
+	Width        int
+	MaxWidth     int
+	PortMaxWidth int // bridge's own capability when MaxWidth is limited by its downstream peer
+	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
@@ -154,6 +156,22 @@ func retrainAndSamplePCIeDevice(ctx context.Context, verboseLog, bdf string, log
 		return f
 	}

+	// A bridge/root port reports its own maximum capability in sysfs, not
+	// the maximum mutually supported by the device at the other end of the
+	// link.  Comparing a Gen4 x16 root port directly with a Gen3 x8 or Gen2
+	// x4 endpoint therefore produces a false degradation even though the
+	// link is running at the fastest rate the endpoint supports.  The child
+	// device is tested separately, so use its advertised capability to
+	// calculate the real target for this bridge-side view of the same link.
+	if isPCIeBridgeClass(class) {
+		if peerSpeed, peerWidth, ok := downstreamPCIeLinkCapability(bdf); ok {
+			f.PortMaxSpeed = f.MaxSpeed
+			f.PortMaxWidth = f.MaxWidth
+			f.MaxSpeed = minPCIeLinkSpeed(f.MaxSpeed, peerSpeed)
+			f.MaxWidth = minPositiveInt(f.MaxWidth, peerWidth)
+		}
+	}
+
 	if err := retrainPCIeLink(ctx, verboseLog, bdf, logFunc); err != nil {
 		f.Skipped = "retrain failed: " + err.Error()
 		f.AfterSpeed = before
@@ -271,6 +289,96 @@ func readPCIeSysfsHex(bdf, attr string) (string, bool) {
 	return strings.TrimSpace(string(raw)), true
 }

+// isPCIeBridgeClass matches PCI class 0x0604xx (PCI-to-PCI bridge).  These
+// functions describe the upstream side of a downstream link, so their own
+// max_link_* values must be capped by the peer's capability.
+func isPCIeBridgeClass(classHex string) bool {
+	c := strings.TrimPrefix(strings.ToLower(strings.TrimSpace(classHex)), "0x")
+	return len(c) >= 4 && c[:4] == "0604"
+}
+
+// downstreamPCIeLinkCapability returns the strongest capability advertised
+// by a bridge's immediate child functions.  In sysfs those functions are
+// direct entries below the bridge device directory.  Multifunction devices
+// expose several children for one physical link; taking the strongest values
+// avoids understating the link because one auxiliary function omitted data.
+func downstreamPCIeLinkCapability(bdf string) (speed string, width int, ok bool) {
+	bridgeDir := filepath.Join("/sys/bus/pci/devices", bdf)
+	return downstreamPCIeLinkCapabilityAt(bridgeDir)
+}
+
+func downstreamPCIeLinkCapabilityAt(bridgeDir string) (speed string, width int, ok bool) {
+	entries, err := os.ReadDir(bridgeDir)
+	if err != nil {
+		return "", 0, false
+	}
+	for _, entry := range entries {
+		if !isFullPCIBDF(entry.Name()) {
+			continue
+		}
+		childDir := filepath.Join(bridgeDir, entry.Name())
+		rawSpeed, speedErr := os.ReadFile(filepath.Join(childDir, "max_link_speed"))
+		if speedErr != nil {
+			continue
+		}
+		childSpeed := collector.NormalizePCILinkSpeed(strings.TrimSpace(string(rawSpeed)))
+		if pcieGeneration(childSpeed) > pcieGeneration(speed) {
+			speed = childSpeed
+		}
+		if rawWidth, widthErr := os.ReadFile(filepath.Join(childDir, "max_link_width")); widthErr == nil {
+			if childWidth, parseErr := strconv.Atoi(strings.TrimSpace(string(rawWidth))); parseErr == nil && childWidth > width {
+				width = childWidth
+			}
+		}
+		ok = true
+	}
+	return speed, width, ok && speed != ""
+}
+
+func isFullPCIBDF(s string) bool {
+	if len(s) != len("0000:00:00.0") || s[4] != ':' || s[7] != ':' || s[10] != '.' {
+		return false
+	}
+	for i, r := range s {
+		if i == 4 || i == 7 || i == 10 {
+			continue
+		}
+		if !((r >= '0' && r <= '9') || (r >= 'a' && r <= 'f') || (r >= 'A' && r <= 'F')) {
+			return false
+		}
+	}
+	return true
+}
+
+func minPCIeLinkSpeed(a, b string) string {
+	ga, gb := pcieGeneration(a), pcieGeneration(b)
+	switch {
+	case ga == 0:
+		return b
+	case gb == 0 || ga <= gb:
+		return a
+	default:
+		return b
+	}
+}
+
+func pcieGeneration(speed string) int {
+	v := strings.TrimPrefix(strings.TrimSpace(speed), "Gen")
+	gen, _ := strconv.Atoi(v)
+	return gen
+}
+
+func minPositiveInt(a, b int) int {
+	switch {
+	case a <= 0:
+		return b
+	case b <= 0 || a <= b:
+		return a
+	default:
+		return b
+	}
+}
+
 // classifyGPUFromVendorClass reports whether a device is a GPU die itself
 // (PCI base class 0x03 — Display Controller — under NVIDIA/AMD's vendor
 // ID), as opposed to a same-vendor companion device (NIC, storage
@@ -378,8 +486,12 @@ func renderPCIeLinkCheckReport(findings []pcieLinkFinding) string {
 		case f.Degraded:
 			verdict = "DEGRADED"
 		}
-		fmt.Fprintf(&b, "    %s: before=%s after=%s max=%s width=%d/%d\n",
+		fmt.Fprintf(&b, "    %s: before=%s after=%s max=%s width=%d/%d",
 			verdict, f.BeforeSpeed, f.AfterSpeed, f.MaxSpeed, f.Width, f.MaxWidth)
+		if f.PortMaxSpeed != "" && (f.PortMaxSpeed != f.MaxSpeed || f.PortMaxWidth != f.MaxWidth) {
+			fmt.Fprintf(&b, " (port capability %s x%d, limited by downstream device)", f.PortMaxSpeed, f.PortMaxWidth)
+		}
+		fmt.Fprintln(&b)
 		if f.Skipped != "" {
 			fmt.Fprintf(&b, "    note: %s\n", f.Skipped)
 		}
@@ -1,6 +1,8 @@
 package platform

 import (
+	"os"
+	"path/filepath"
 	"strings"
 	"testing"
 )
@@ -30,6 +32,82 @@ func TestClassifyGPUFromVendorClass(t *testing.T) {
 	}
 }

+func TestPCIeBridgeUsesDownstreamCapability(t *testing.T) {
+	cases := []struct {
+		name          string
+		portSpeed     string
+		portWidth     int
+		endpointSpeed string
+		endpointWidth int
+		wantSpeed     string
+		wantWidth     int
+	}{
+		{"ConnectX-5 behind Gen4 root port", "Gen4", 16, "Gen3", 8, "Gen3", 8},
+		{"Adaptec SAS behind Gen4 root port", "Gen4", 16, "Gen3", 8, "Gen3", 8},
+		{"I350 behind Gen4 root port", "Gen4", 16, "Gen2", 4, "Gen2", 4},
+		{"faster endpoint remains port-limited", "Gen3", 8, "Gen4", 16, "Gen3", 8},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			gotSpeed := minPCIeLinkSpeed(tc.portSpeed, tc.endpointSpeed)
+			gotWidth := minPositiveInt(tc.portWidth, tc.endpointWidth)
+			if gotSpeed != tc.wantSpeed || gotWidth != tc.wantWidth {
+				t.Fatalf("effective capability = %s x%d, want %s x%d", gotSpeed, gotWidth, tc.wantSpeed, tc.wantWidth)
+			}
+		})
+	}
+}
+
+func TestPCIeBridgeAtEndpointMaximumPasses(t *testing.T) {
+	maxSpeed := minPCIeLinkSpeed("Gen4", "Gen3")
+	maxWidth := minPositiveInt(16, 8)
+	finding := pcieLinkFinding{
+		BDF:          "0000:4a:02.0",
+		Description:  "Intel root port to ConnectX-5",
+		BeforeSpeed:  "Gen3",
+		AfterSpeed:   "Gen3",
+		MaxSpeed:     maxSpeed,
+		PortMaxSpeed: "Gen4",
+		Width:        8,
+		MaxWidth:     maxWidth,
+		PortMaxWidth: 16,
+		Degraded:     "Gen3" != maxSpeed,
+	}
+
+	summary := renderPCIeLinkCheckSummary([]pcieLinkFinding{finding})
+	if !strings.Contains(summary, "overall_status=OK") {
+		t.Fatalf("endpoint-limited bridge should pass, got:\n%s", summary)
+	}
+	report := renderPCIeLinkCheckReport([]pcieLinkFinding{finding})
+	if !strings.Contains(report, "limited by downstream device") {
+		t.Fatalf("report should explain the effective maximum, got:\n%s", report)
+	}
+}
+
+func TestDownstreamPCIeLinkCapabilityFromSysfsTopology(t *testing.T) {
+	bridgeDir := t.TempDir()
+	childDir := filepath.Join(bridgeDir, "0000:4b:00.0")
+	if err := os.Mkdir(childDir, 0755); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(filepath.Join(childDir, "max_link_speed"), []byte("8.0 GT/s PCIe\n"), 0644); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(filepath.Join(childDir, "max_link_width"), []byte("8\n"), 0644); err != nil {
+		t.Fatal(err)
+	}
+	// A non-BDF sysfs entry must not influence peer discovery.
+	if err := os.WriteFile(filepath.Join(bridgeDir, "max_link_speed"), []byte("16.0 GT/s PCIe\n"), 0644); err != nil {
+		t.Fatal(err)
+	}
+
+	speed, width, ok := downstreamPCIeLinkCapabilityAt(bridgeDir)
+	if !ok || speed != "Gen3" || width != 8 {
+		t.Fatalf("downstream capability = (%q, %d, %v), want (Gen3, 8, true)", speed, width, ok)
+	}
+}
+
 func TestRenderPCIeLinkCheckSummaryDegradedGPU(t *testing.T) {
 	findings := []pcieLinkFinding{
 		{BDF: "0000:0d:00.0", Description: "NVIDIA GPU", IsGPU: true, GPUVendor: "nvidia",