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",