From a3567dd5f60414fa9b1c51c9382b477eecc0f7a8 Mon Sep 17 00:00:00 2001 From: Mikhail Chusavitin Date: Tue, 18 Aug 2026 14:17:25 +0300 Subject: [PATCH] fix(collector): recover GPUs/NICs dropped on xFusion G5500 Redfish exports looksLikeGPU now falls back to resolving VendorId/DeviceId through the pci.ids database when the BMC leaves Name/Model/Manufacturer/ClassCode empty, so GPUs identifiable only by raw PCI IDs (e.g. NVIDIA H100 SXM5 0x10de/0x2330) are no longer misclassified as generic PCIe devices. The replay pipeline's "backed by canonical NIC" dedup used to trust a PCIeDevice's Links.NetworkDeviceFunctions reference at face value and drop the device, assuming a NetworkAdapters record existed elsewhere. On BMCs that expose resource IDs with characters (parentheses) that 404 on fetch, that canonical NIC never gets captured, so the device carrying its actual hardware identity vanished from the export entirely. hasResolvableLinkedMember now verifies the linked resource is actually present in the snapshot before treating it as authoritative. Also normalize PartNumber through normalizeRedfishIdentityField in the GPU/PCIe parsers so a BMC-supplied literal "null" string doesn't leak into exports verbatim. Co-Authored-By: Claude Sonnet 5 --- internal/collector/redfish.go | 30 ++++++- .../collector/redfish_replay_inventory.go | 68 +++++++-------- internal/collector/redfish_test.go | 84 +++++++++++++++++++ 3 files changed, 146 insertions(+), 36 deletions(-) diff --git a/internal/collector/redfish.go b/internal/collector/redfish.go index afbc1d1..f4d0ad4 100644 --- a/internal/collector/redfish.go +++ b/internal/collector/redfish.go @@ -4355,7 +4355,7 @@ func parseGPUWithSupplementalDocs(doc map[string]interface{}, functionDocs []map Model: firstNonEmpty(asString(doc["Model"]), asString(doc["Name"])), Manufacturer: asString(doc["Manufacturer"]), SerialNumber: findFirstNormalizedStringByKeys(doc, "SerialNumber"), - PartNumber: asString(doc["PartNumber"]), + PartNumber: normalizeRedfishIdentityField(asString(doc["PartNumber"])), Firmware: asString(doc["FirmwareVersion"]), Status: mapStatus(doc["Status"]), Details: redfishPCIeDetailsWithSupplementalDocs(doc, functionDocs, supplementalDocs), @@ -4439,7 +4439,7 @@ func parsePCIeDeviceWithSupplementalDocs(doc map[string]interface{}, functionDoc BDF: sanitizeRedfishBDF(asString(doc["BDF"])), DeviceClass: asString(doc["DeviceType"]), Manufacturer: asString(doc["Manufacturer"]), - PartNumber: asString(doc["PartNumber"]), + PartNumber: normalizeRedfishIdentityField(asString(doc["PartNumber"])), SerialNumber: findFirstNormalizedStringByKeys(doc, "SerialNumber"), VendorID: asHexOrInt(doc["VendorId"]), DeviceID: asHexOrInt(doc["DeviceId"]), @@ -5084,6 +5084,32 @@ func looksLikeGPU(doc map[string]interface{}, functionDocs []map[string]interfac } } + // Some BMCs (e.g. xFusion) leave Name/Model/Manufacturer/ClassCode empty on + // the PCIeDevice and its PCIeFunctions, exposing only raw VendorId/DeviceId. + // Resolve those through the pci.ids database so GH100/GA100/etc. GPUs are + // still recognized even without vendor-supplied model text. + vendorID := asHexOrInt(doc["VendorId"]) + deviceID := asHexOrInt(doc["DeviceId"]) + for _, fn := range functionDocs { + if vendorID == 0 { + vendorID = asHexOrInt(fn["VendorId"]) + } + if deviceID == 0 { + deviceID = asHexOrInt(fn["DeviceId"]) + } + } + if vendorID != 0 || deviceID != 0 { + resolvedText := strings.ToLower(strings.Join([]string{ + pciids.VendorName(vendorID), + pciids.DeviceName(vendorID, deviceID), + }, " ")) + for _, hint := range gpuHints { + if strings.Contains(resolvedText, hint) { + return true + } + } + } + return false } diff --git a/internal/collector/redfish_replay_inventory.go b/internal/collector/redfish_replay_inventory.go index 296dafa..3da8bdf 100644 --- a/internal/collector/redfish_replay_inventory.go +++ b/internal/collector/redfish_replay_inventory.go @@ -141,7 +141,7 @@ func (r redfishSnapshotReader) collectPCIeDevices(systemPaths, chassisPaths []st if looksLikeGPU(doc, functionDocs) { continue } - if replayPCIeDeviceBackedByCanonicalNIC(doc, functionDocs) { + if r.replayPCIeDeviceBackedByCanonicalNIC(doc, functionDocs) { continue } supplementalDocs := r.getLinkedSupplementalDocs(doc, "EnvironmentMetrics", "Metrics") @@ -150,7 +150,7 @@ func (r redfishSnapshotReader) collectPCIeDevices(systemPaths, chassisPaths []st supplementalDocs = append(supplementalDocs, r.getLinkedSupplementalDocs(fn, "EnvironmentMetrics", "Metrics")...) } dev := parsePCIeDeviceWithSupplementalDocs(doc, functionDocs, supplementalDocs) - if shouldSkipReplayPCIeDevice(doc, dev) { + if r.shouldSkipReplayPCIeDevice(doc, dev) { continue } out = append(out, dev) @@ -164,7 +164,7 @@ func (r redfishSnapshotReader) collectPCIeDevices(systemPaths, chassisPaths []st for idx, fn := range functionDocs { supplementalDocs := r.getLinkedSupplementalDocs(fn, "EnvironmentMetrics", "Metrics") dev := parsePCIeFunctionWithSupplementalDocs(fn, supplementalDocs, idx+1) - if shouldSkipReplayPCIeDevice(fn, dev) { + if r.shouldSkipReplayPCIeDevice(fn, dev) { continue } out = append(out, dev) @@ -173,11 +173,11 @@ func (r redfishSnapshotReader) collectPCIeDevices(systemPaths, chassisPaths []st return dedupePCIeDevices(out) } -func shouldSkipReplayPCIeDevice(doc map[string]interface{}, dev models.PCIeDevice) bool { +func (r redfishSnapshotReader) shouldSkipReplayPCIeDevice(doc map[string]interface{}, dev models.PCIeDevice) bool { if isUnidentifiablePCIeDevice(dev) { return true } - if replayNetworkFunctionBackedByCanonicalNIC(doc, dev) { + if r.replayNetworkFunctionBackedByCanonicalNIC(doc, dev) { return true } if isReplayStorageServiceEndpoint(doc, dev) { @@ -192,23 +192,48 @@ func shouldSkipReplayPCIeDevice(doc map[string]interface{}, dev models.PCIeDevic return false } -func replayPCIeDeviceBackedByCanonicalNIC(doc map[string]interface{}, functionDocs []map[string]interface{}) bool { +func (r redfishSnapshotReader) replayPCIeDeviceBackedByCanonicalNIC(doc map[string]interface{}, functionDocs []map[string]interface{}) bool { if !looksLikeReplayNetworkPCIeDevice(doc, functionDocs) { return false } for _, fn := range functionDocs { - if hasRedfishLinkedMember(fn, "NetworkDeviceFunctions") { + if r.hasResolvableLinkedMember(fn, "NetworkDeviceFunctions") { return true } } return false } -func replayNetworkFunctionBackedByCanonicalNIC(doc map[string]interface{}, dev models.PCIeDevice) bool { +func (r redfishSnapshotReader) replayNetworkFunctionBackedByCanonicalNIC(doc map[string]interface{}, dev models.PCIeDevice) bool { if !looksLikeReplayNetworkClass(dev.DeviceClass) { return false } - return hasRedfishLinkedMember(doc, "NetworkDeviceFunctions") + return r.hasResolvableLinkedMember(doc, "NetworkDeviceFunctions") +} + +// hasResolvableLinkedMember reports whether the resource(s) linked under +// doc.Links[key] were actually captured in the snapshot. A Links reference +// alone is not enough: some BMCs (e.g. xFusion) advertise linked +// NetworkAdapters/NetworkDeviceFunctions resources whose IDs contain +// characters (like parentheses) that 404 when fetched, so the "canonical" +// NIC never makes it into the snapshot even though the link exists. In that +// case the PCIe device carrying the NIC's hardware identity must not be +// dropped, or the NIC disappears from the inventory entirely. +func (r redfishSnapshotReader) hasResolvableLinkedMember(doc map[string]interface{}, key string) bool { + links, ok := doc["Links"].(map[string]interface{}) + if !ok { + return false + } + linked, ok := links[key] + if !ok { + return false + } + for _, path := range extractODataIDs(linked) { + if _, err := r.getJSON(path); err == nil { + return true + } + } + return false } func looksLikeReplayNetworkPCIeDevice(doc map[string]interface{}, functionDocs []map[string]interface{}) bool { @@ -250,31 +275,6 @@ func isReplayStorageServiceEndpoint(doc map[string]interface{}, dev models.PCIeD return false } -func hasRedfishLinkedMember(doc map[string]interface{}, key string) bool { - links, ok := doc["Links"].(map[string]interface{}) - if !ok { - return false - } - if asInt(links[key+"@odata.count"]) > 0 { - return true - } - linked, ok := links[key] - if !ok { - return false - } - switch v := linked.(type) { - case []interface{}: - return len(v) > 0 - case map[string]interface{}: - if asString(v["@odata.id"]) != "" { - return true - } - return len(v) > 0 - default: - return false - } -} - func isReplayNoisePCIeClass(class string) bool { switch strings.ToLower(strings.TrimSpace(class)) { case "bridge", "processor", "signalprocessingcontroller", "signal processing controller", "serialbuscontroller", "serial bus controller": diff --git a/internal/collector/redfish_test.go b/internal/collector/redfish_test.go index 7c72ece..98398b1 100644 --- a/internal/collector/redfish_test.go +++ b/internal/collector/redfish_test.go @@ -2639,6 +2639,12 @@ func TestReplayCollectPCIeDevices_SkipsNICsAlreadyRepresentedAsNetworkAdapters(t "NetworkDeviceFunctions@odata.count": 1, }, }, + // The linked NetworkDeviceFunctions resource was actually captured in + // the snapshot, so the canonical NIC is genuinely available and this + // PCIe duplicate should be skipped. + "/redfish/v1/Chassis/1/NetworkAdapters/NIC1/NetworkDeviceFunctions/Function0": map[string]interface{}{ + "Id": "Function0", + }, }} got := r.collectPCIeDevices(nil, []string{"/redfish/v1/Chassis/1"}) @@ -2647,6 +2653,60 @@ func TestReplayCollectPCIeDevices_SkipsNICsAlreadyRepresentedAsNetworkAdapters(t } } +// TestReplayCollectPCIeDevices_KeepsNICWhenCanonicalLinkIsUnresolvable covers +// xFusion BMCs (e.g. G5500 V7) that advertise a Links.NetworkDeviceFunctions +// reference on a PCIeDevice's Function, but whose target resource ID contains +// characters (parentheses) the BMC 404s on when fetched directly. Because +// that referenced resource was never actually captured in the snapshot, the +// PCIe device is the only surviving copy of the NIC's hardware identity and +// must not be dropped, or the NIC vanishes from the inventory entirely. +func TestReplayCollectPCIeDevices_KeepsNICWhenCanonicalLinkIsUnresolvable(t *testing.T) { + r := redfishSnapshotReader{tree: map[string]interface{}{ + "/redfish/v1/Chassis/1/PCIeDevices": map[string]interface{}{ + "Members": []interface{}{ + map[string]interface{}{"@odata.id": "/redfish/v1/Chassis/1/PCIeDevices/OCPCard1"}, + }, + }, + "/redfish/v1/Chassis/1/PCIeDevices/OCPCard1": map[string]interface{}{ + "Id": "OCPCard1", + "Name": "OCPCard1", + "CardManufacturer": "XFUSION", + "CardModel": "XC385", + "Manufacturer": "XFUSION", + "SerialNumber": "02Y238X6RC000058", + "PCIeFunctions": map[string]interface{}{ + "@odata.id": "/redfish/v1/Chassis/1/PCIeDevices/OCPCard1/Functions", + }, + }, + "/redfish/v1/Chassis/1/PCIeDevices/OCPCard1/Functions": map[string]interface{}{ + "Members": []interface{}{ + map[string]interface{}{"@odata.id": "/redfish/v1/Chassis/1/PCIeDevices/OCPCard1/Functions/1"}, + }, + }, + "/redfish/v1/Chassis/1/PCIeDevices/OCPCard1/Functions/1": map[string]interface{}{ + "DeviceClass": "NetworkController", + "VendorId": "0x15b3", + "DeviceId": "0x101f", + "Links": map[string]interface{}{ + "NetworkDeviceFunctions": []interface{}{ + map[string]interface{}{"@odata.id": "/redfish/v1/Chassis/1/NetworkAdapters/MainboardOCPCard1(XC385)/NetworkDeviceFunctions/1"}, + }, + "NetworkDeviceFunctions@odata.count": 1, + }, + }, + // Deliberately no tree entry for the linked NetworkDeviceFunctions + // resource — it 404'd during collection and was never captured. + }} + + got := r.collectPCIeDevices(nil, []string{"/redfish/v1/Chassis/1"}) + if len(got) != 1 { + t.Fatalf("expected the NIC's PCIe device to be kept when its canonical link is unresolvable, got %+v", got) + } + if got[0].SerialNumber != "02Y238X6RC000058" { + t.Fatalf("expected surviving NIC PCIe device, got %+v", got[0]) + } +} + func TestReplayCollectPCIeDevices_SkipsStorageServiceEndpoints(t *testing.T) { r := redfishSnapshotReader{tree: map[string]interface{}{ "/redfish/v1/Chassis/1/PCIeDevices": map[string]interface{}{ @@ -3824,6 +3884,30 @@ func TestLooksLikeGPU_NVSwitchExcluded(t *testing.T) { } } +// TestLooksLikeGPU_ResolvesVendorDeviceIDWhenModelTextMissing covers xFusion +// BMCs (e.g. G5500 V7) whose PCIeDevice doc and linked PCIeFunction leave +// Name/Model/Manufacturer/ClassCode empty but expose raw VendorId/DeviceId +// (0x10de/0x2330 = NVIDIA H100 SXM5). Without a pci.ids lookup these devices +// silently fall through to the generic pcie_devices list instead of gpus. +func TestLooksLikeGPU_ResolvesVendorDeviceIDWhenModelTextMissing(t *testing.T) { + doc := map[string]interface{}{ + "Id": "PCIeCard1", + "Name": "PCIeCard1", + "Model": nil, + "Manufacturer": nil, + } + functionDocs := []map[string]interface{}{ + { + "DeviceClass": "Other", + "VendorId": "0x10de", + "DeviceId": "0x2330", + }, + } + if !looksLikeGPU(doc, functionDocs) { + t.Fatal("expected NVIDIA H100 (0x10de/0x2330) to be classified as a GPU via VendorId/DeviceId resolution") + } +} + func TestFirmwareInventoryDeviceName_PrefersIDForGenericSoftwareInventory(t *testing.T) { doc := map[string]interface{}{ "Id": "HGX_FW_NVSwitch_0",