diff --git a/audit/internal/webui/page_topo.go b/audit/internal/webui/page_topo.go index c004943..cda9e35 100644 --- a/audit/internal/webui/page_topo.go +++ b/audit/internal/webui/page_topo.go @@ -147,6 +147,103 @@ func buildSocketIndex(cpus []schema.HardwareCPU) map[int]int { return idx } +// --------------------------------------------------------------------------- +// DIMM -> CPU column attachment +// +// schema.HardwareMemory has no NUMANode field (unlike HardwarePCIeDevice), so +// unlike the GPU/NIC/RAID placement above, a DIMM's CPU affinity has to be +// read out of its own Locator/Bank Locator strings — DMI type 17 gives no +// other hint. Both patterns below have been observed on real boards. +// --------------------------------------------------------------------------- + +var ( + topoMemCPULocatorRe = regexp.MustCompile(`(?i)^cpu\s*0*(\d+)`) + topoMemBankNodeRe = regexp.MustCompile(`(?i)node\s*0*(\d+)`) +) + +// parseDIMMBankLocatorNodes maps a DIMM's Locator (matches +// schema.HardwareMemory.Slot) to the node number embedded in its Bank +// Locator field (e.g. "_Node1_Channel0_Dimm0"), read from a raw +// "dmidecode -t 17" techdump capture. Bank Locator never reaches audit.json +// (schema.HardwareMemory.Location is json:"-", used only for internal DIMM +// telemetry matching), so boards whose Locator has no CPU number of its own +// (e.g. "DIMM000(A)" rather than "CPU0_DIMM_A1") need this fallback to +// attach a DIMM to a CPU column at all — matches the existing convention of +// reading extra techdump for this page's visualization only (see +// readTopoTechDump). +func parseDIMMBankLocatorNodes(raw string) map[string]int { + result := map[string]int{} + for _, sec := range strings.Split(raw, "Memory Device") { + var locator string + node := -1 + for _, line := range strings.Split(sec, "\n") { + trimmed := strings.TrimSpace(line) + if v, ok := strings.CutPrefix(trimmed, "Locator:"); ok { + locator = strings.TrimSpace(v) + } + if v, ok := strings.CutPrefix(trimmed, "Bank Locator:"); ok { + if m := topoMemBankNodeRe.FindStringSubmatch(v); m != nil { + if n, err := strconv.Atoi(m[1]); err == nil { + node = n + } + } + } + } + if locator != "" && node >= 0 { + result[locator] = node + } + } + return result +} + +// dimmRawNode returns the raw (vendor-numbered, not yet column-mapped) +// CPU/NUMA node number for a DIMM, trying two heuristics in order: +// 1. A CPU number encoded directly in the Locator itself, e.g. +// "CPU0_DIMM_A1". +// 2. A node number from the Bank Locator via parseDIMMBankLocatorNodes, +// e.g. Locator "DIMM000(A)" whose Bank Locator is +// "_Node1_Channel0_Dimm0" -> 1. +// ok=false means neither pattern matched, so this DIMM can't be confidently +// attached to a CPU column and falls back to the unattached Memory row. +func dimmRawNode(mem schema.HardwareMemory, bankNodeByLocator map[string]int) (int, bool) { + if mem.Slot == nil { + return 0, false + } + if m := topoMemCPULocatorRe.FindStringSubmatch(*mem.Slot); m != nil { + if n, err := strconv.Atoi(m[1]); err == nil { + return n, true + } + } + if n, ok := bankNodeByLocator[*mem.Slot]; ok { + return n, true + } + return 0, false +} + +// buildMemoryColumnIndex ranks the distinct raw node numbers seen across all +// DIMMs and maps the i-th smallest to column i — the same "node order +// follows socket order" assumption buildSocketIndex makes for PCIe NUMA +// nodes, but computed independently from PCIe's own numbering: PCIe's +// NUMANode is 0-based Linux numbering, while a DIMM's Bank Locator "NodeN" +// has been observed 1-based on at least one real board, so the two node +// spaces are not guaranteed to share a base. +func buildMemoryColumnIndex(rawNodes []int) map[int]int { + seen := map[int]bool{} + var distinct []int + for _, n := range rawNodes { + if !seen[n] { + seen[n] = true + distinct = append(distinct, n) + } + } + sort.Ints(distinct) + idx := map[int]int{} + for col, n := range distinct { + idx[n] = col + } + return idx +} + // --------------------------------------------------------------------------- // GPU pairwise NVLink adjacency (from a live "nvidia-smi topo -m" query) // --------------------------------------------------------------------------- @@ -483,6 +580,11 @@ type topoBox struct { topoCardInfo } +type topoEdge struct { + x1, y1, x2, y2 int + color string +} + func renderTopoMainDiagram(hw schema.HardwareSnapshot, exportDir string) string { socketIdx := buildSocketIndex(hw.CPUs) numCols := len(hw.CPUs) @@ -581,11 +683,41 @@ func renderTopoMainDiagram(hw schema.HardwareSnapshot, exportDir string) string kindOrder := []string{"gpu", "nic", "raid"} kindLabel := map[string]string{"gpu": "GPU", "nic": "NIC", "raid": "RAID"} - var boxes []topoBox - var pcieEdges []struct { - x1, y1, x2, y2 int - color string + // Attach memory DIMMs to their CPU column too, the same way GPU/NIC/RAID + // PCIe devices are attached via NUMANode — memory has no NUMANode field + // in the schema, so this reads the DIMM's own Locator/Bank Locator + // strings instead (see dimmRawNode). DIMMs that can't be confidently + // attached fall back to the unattached "Memory" row below the diagram, + // same as before this existed. + memBankNodes := map[string]int{} + if raw, err := readTopoTechDump(exportDir, "dmidecode-type17.txt"); err == nil { + memBankNodes = parseDIMMBankLocatorNodes(raw) } + memCol := make([]int, len(hw.Memory)) + memMatched := make([]bool, len(hw.Memory)) + var memRawNodes []int + for i, m := range hw.Memory { + if node, ok := dimmRawNode(m, memBankNodes); ok { + memCol[i] = node + memMatched[i] = true + memRawNodes = append(memRawNodes, node) + } + } + memColIdx := buildMemoryColumnIndex(memRawNodes) + for i := range hw.Memory { + if !memMatched[i] { + continue + } + col := memColIdx[memCol[i]] + if col >= numCols { + memMatched[i] = false + continue + } + memCol[i] = col + } + + var boxes []topoBox + var pcieEdges []topoEdge for col := 0; col < totalCols; col++ { colX := (col+1)*24 + col*topoColWidth @@ -614,6 +746,50 @@ func renderTopoMainDiagram(hw schema.HardwareSnapshot, exportDir string) string } y := topoTopMargin + topoBoxHeight + topoDeviceGap*2 + + // Memory goes first in the chain, directly under the CPU box: DIMMs + // are wired straight to the socket's memory controller, not reached + // over PCIe like the GPU/NIC/RAID chain below it. + var memGroup []schema.HardwareMemory + for i, m := range hw.Memory { + if memMatched[i] && memCol[i] == col { + memGroup = append(memGroup, m) + } + } + if len(memGroup) > 0 { + var tally topoStatusTally + sizeGB := 0 + for _, m := range memGroup { + tally.add(classifyTopoSeverity(m.Status)) + if m.SizeMB != nil { + sizeGB += *m.SizeMB / 1024 + } + } + fill, stroke, text := topoSeverityColors(tally.worst()) + sublabel := "" + if sizeGB > 0 { + sublabel = fmt.Sprintf("%d GB total", sizeGB) + } + stackLayers := topoStackLayers(len(memGroup)) + boxes = append(boxes, topoBox{ + x: colX, y: y, w: topoBoxWidth, h: topoBoxHeight, + topoCardInfo: topoCardInfo{ + label: "Memory", sublabel: sublabel, count: len(memGroup), + statusLine: tally.line(), + fillVar: fill, strokeVar: stroke, textVar: text, + detailType: "memory", + }, + }) + if col < len(hw.CPUs) { + pcieEdges = append(pcieEdges, topoEdge{ + x1: colX + topoBoxWidth/2, y1: topoTopMargin + topoBoxHeight, + x2: colX + topoBoxWidth/2, y2: y, + color: "var(--ok-fg)", + }) + } + y += topoBoxHeight + topoDeviceGap + stackLayers*topoStackStep + } + for _, kind := range kindOrder { var group []placedDevice for _, p := range placed { @@ -654,10 +830,7 @@ func renderTopoMainDiagram(hw schema.HardwareSnapshot, exportDir string) string }) if col < len(hw.CPUs) { - pcieEdges = append(pcieEdges, struct { - x1, y1, x2, y2 int - color string - }{ + pcieEdges = append(pcieEdges, topoEdge{ x1: colX + topoBoxWidth/2, y1: topoTopMargin + topoBoxHeight, x2: colX + topoBoxWidth/2, y2: y, color: edgeColor, @@ -695,21 +868,31 @@ func renderTopoMainDiagram(hw schema.HardwareSnapshot, exportDir string) string } b.WriteString(``) - // Memory, Firmware (BMC/BIOS/...) and PSUs have no PCIe/CPU affinity to - // anchor them to a column, and there can be an arbitrary number of any - // of them — so unlike the diagram above, they're plain flex-wrap HTML - // below the SVG rather than absolutely-positioned SVG boxes. A - // fixed-size SVG canvas has no way to wrap overflow onto a new row, - // which is exactly what caused these to pile up and overlap once a - // board had more PSUs/firmware records than fit in one fixed-width row. - if len(hw.Memory) > 0 { + // Firmware (BMC/BIOS/...) and PSUs have no PCIe/CPU affinity to anchor + // them to a column, and there can be an arbitrary number of any of them + // — so unlike the diagram above, they're plain flex-wrap HTML below the + // SVG rather than absolutely-positioned SVG boxes. A fixed-size SVG + // canvas has no way to wrap overflow onto a new row, which is exactly + // what caused these to pile up and overlap once a board had more + // PSUs/firmware records than fit in one fixed-width row. + // + // Memory DIMMs that were matched to a CPU column above already got a + // box in the SVG diagram; only DIMMs that couldn't be attached to a + // column (see memMatched above) fall back to this row. + var unmatchedMem []schema.HardwareMemory + for i, m := range hw.Memory { + if !memMatched[i] { + unmatchedMem = append(unmatchedMem, m) + } + } + if len(unmatchedMem) > 0 { var tally topoStatusTally - for _, m := range hw.Memory { + for _, m := range unmatchedMem { tally.add(classifyTopoSeverity(m.Status)) } fill, stroke, text := topoSeverityColors(tally.worst()) sizeGB := 0 - for _, m := range hw.Memory { + for _, m := range unmatchedMem { if m.SizeMB != nil { sizeGB += *m.SizeMB / 1024 } @@ -719,7 +902,7 @@ func renderTopoMainDiagram(hw schema.HardwareSnapshot, exportDir string) string sublabel = fmt.Sprintf("%d GB total", sizeGB) } b.WriteString(renderTopoFlexRow("Memory", []topoCardInfo{{ - label: "Memory", sublabel: sublabel, count: len(hw.Memory), + label: "Memory", sublabel: sublabel, count: len(unmatchedMem), statusLine: tally.line(), fillVar: fill, strokeVar: stroke, textVar: text, detailType: "memory", diff --git a/audit/internal/webui/page_topo_test.go b/audit/internal/webui/page_topo_test.go index 5548e22..b4c1a62 100644 --- a/audit/internal/webui/page_topo_test.go +++ b/audit/internal/webui/page_topo_test.go @@ -392,6 +392,174 @@ func TestTopoMainDiagramGroupsSameKindSameColumnIntoOneStackedCard(t *testing.T) } } +func TestDimmRawNodeFromCPULocatorPrefix(t *testing.T) { + slot := "CPU1_DIMM_A1" + mem := schema.HardwareMemory{Slot: &slot} + node, ok := dimmRawNode(mem, nil) + if !ok || node != 1 { + t.Fatalf("dimmRawNode() = (%d, %v), want (1, true)", node, ok) + } +} + +func TestDimmRawNodeFromBankLocatorFallback(t *testing.T) { + // Boards whose Locator carries no CPU number of its own (e.g. + // "DIMM000(A)") still need to be attached to a column via the node + // number in Bank Locator, read separately from techdump. + slot := "DIMM100(A)" + mem := schema.HardwareMemory{Slot: &slot} + bankNodes := map[string]int{"DIMM100(A)": 2} + node, ok := dimmRawNode(mem, bankNodes) + if !ok || node != 2 { + t.Fatalf("dimmRawNode() = (%d, %v), want (2, true)", node, ok) + } +} + +func TestDimmRawNodeUnmatched(t *testing.T) { + slot := "SOMETHING_UNRECOGNIZED" + mem := schema.HardwareMemory{Slot: &slot} + if _, ok := dimmRawNode(mem, nil); ok { + t.Fatalf("expected no match for an unrecognized locator") + } + if _, ok := dimmRawNode(schema.HardwareMemory{}, nil); ok { + t.Fatalf("expected no match when Slot is nil") + } +} + +func TestParseDIMMBankLocatorNodes(t *testing.T) { + // Real dmidecode -t 17 shape: "Locator" precedes "Bank Locator" within + // each "Memory Device" section. + raw := `Handle 0x0017, DMI type 17, 92 bytes +Memory Device + Size: 64 GB + Locator: DIMM000(A) + Bank Locator: _Node1_Channel0_Dimm0 + Type: DDR5 +Handle 0x0018, DMI type 17, 92 bytes +Memory Device + Size: No Module Installed + Locator: DIMM001(I) + Bank Locator: _Node1_Channel0_Dimm1 +Handle 0x0019, DMI type 17, 92 bytes +Memory Device + Size: 64 GB + Locator: DIMM100(A) + Bank Locator: _Node2_Channel0_Dimm0 +` + got := parseDIMMBankLocatorNodes(raw) + want := map[string]int{"DIMM000(A)": 1, "DIMM001(I)": 1, "DIMM100(A)": 2} + if len(got) != len(want) { + t.Fatalf("got=%#v want=%#v", got, want) + } + for k, v := range want { + if got[k] != v { + t.Fatalf("got[%q]=%d want %d (full: %#v)", k, got[k], v, got) + } + } +} + +func TestBuildMemoryColumnIndex(t *testing.T) { + // Node numbers observed on real boards are not guaranteed 0-based (Bank + // Locator "NodeN" has been seen starting at 1) — this must rank by + // order, not treat the raw value as a column index. + idx := buildMemoryColumnIndex([]int{1, 2, 1, 2}) + if idx[1] != 0 || idx[2] != 1 { + t.Fatalf("idx=%#v want {1:0, 2:1}", idx) + } +} + +// TestTopoMainDiagramAttachesMemoryToItsCPUColumn is the regression test for +// the actual feature request: memory used to render as one unattached +// "MEMORY" row below the whole diagram regardless of which CPU it belonged +// to. DIMMs whose Locator carries a CPU number must now render as their own +// box directly under that CPU, wired to it with an edge, the same as +// GPU/NIC/RAID — and must NOT also show up in the leftover flex row. +func TestTopoMainDiagramAttachesMemoryToItsCPUColumn(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "audit.json") + + socket0, socket1 := 0, 1 + okStatus := "OK" + slot0, slot1 := "CPU0_DIMM_A1", "CPU1_DIMM_A1" + sizeMB := 98304 + + ingest := schema.HardwareIngestRequest{ + CollectedAt: "2026-03-15T00:00:00Z", + Hardware: schema.HardwareSnapshot{ + CPUs: []schema.HardwareCPU{ + {HardwareComponentStatus: schema.HardwareComponentStatus{Status: &okStatus}, Socket: &socket0}, + {HardwareComponentStatus: schema.HardwareComponentStatus{Status: &okStatus}, Socket: &socket1}, + }, + Memory: []schema.HardwareMemory{ + {HardwareComponentStatus: schema.HardwareComponentStatus{Status: &okStatus}, Slot: &slot0, SizeMB: &sizeMB}, + {HardwareComponentStatus: schema.HardwareComponentStatus{Status: &okStatus}, Slot: &slot1, SizeMB: &sizeMB}, + }, + }, + } + data, err := json.Marshal(ingest) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, data, 0644); err != nil { + t.Fatal(err) + } + + handler := NewHandler(HandlerOptions{AuditPath: path}) + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/topo", nil)) + body := rec.Body.String() + + if strings.Count(body, `>Memory<`) != 2 { + t.Fatalf("expected 2 attached Memory boxes (one per CPU column), got body: %s", body) + } + if strings.Contains(body, "Memory") { + t.Fatalf("both DIMMs matched a CPU column — the leftover unattached Memory row must not render: %s", body) + } + if strings.Count(body, "openComponentDetail('memory')") != 2 { + t.Fatalf("expected 2 clickable Memory boxes, got body: %s", body) + } +} + +// TestTopoMainDiagramUnattachableMemoryFallsBackToFlexRow guards that a DIMM +// whose Locator can't be parsed into a CPU/node number still shows up +// somewhere (the old unattached row) instead of silently disappearing. +func TestTopoMainDiagramUnattachableMemoryFallsBackToFlexRow(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "audit.json") + + socket0 := 0 + okStatus := "OK" + slot := "SOMETHING_UNRECOGNIZED" + sizeMB := 32768 + + ingest := schema.HardwareIngestRequest{ + CollectedAt: "2026-03-15T00:00:00Z", + Hardware: schema.HardwareSnapshot{ + CPUs: []schema.HardwareCPU{ + {HardwareComponentStatus: schema.HardwareComponentStatus{Status: &okStatus}, Socket: &socket0}, + }, + Memory: []schema.HardwareMemory{ + {HardwareComponentStatus: schema.HardwareComponentStatus{Status: &okStatus}, Slot: &slot, SizeMB: &sizeMB}, + }, + }, + } + data, err := json.Marshal(ingest) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, data, 0644); err != nil { + t.Fatal(err) + } + + handler := NewHandler(HandlerOptions{AuditPath: path}) + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/topo", nil)) + body := rec.Body.String() + + if !strings.Contains(body, "Memory") { + t.Fatalf("unattachable DIMM should still appear in the leftover flex row: %s", body) + } +} + func TestParseTopoNVLinkStatus(t *testing.T) { input := `GPU 0: NVIDIA H100 80GB HBM3 (UUID: GPU-a59f6931-c099-8fba-a0b3-08469d86f140) Link 0: 26.562 GB/s