webui/topo: attach memory DIMMs to their CPU column in the topology diagram

Memory used to render as one unattached row below the whole diagram regardless of which socket it belonged to. schema.HardwareMemory has no NUMANode field (unlike PCIe devices), so CPU affinity is instead read out of the DIMM's own Locator string: either a CPU number encoded directly in it ("CPU0_DIMM_A1"), or — when the Locator has no CPU number of its own, e.g. "DIMM000(A)" — a node number from Bank Locator ("_Node1_Channel0_Dimm0"), read from the persisted dmidecode-type17.txt techdump the same way the NVLink card already reads extra techdump for visualization only. DIMMs that can't be attached to a column via either heuristic still fall back to the old unattached "Memory" row so nothing silently disappears. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-07-09 17:15:06 +03:00
parent c1f0f7824e
commit 41f683de2b
2 changed files with 370 additions and 19 deletions
@@ -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(`</svg></div>`)

-	// 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",
@@ -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</div>") {
+		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</div>") {
+		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