From 507816ba8c31fde6a2d5329af6d7a4bf8883462b Mon Sep 17 00:00:00 2001 From: Dmitry Porokh Date: Thu, 25 Jun 2026 21:05:28 -0700 Subject: [PATCH] refactor(site-explorer): clarify BlueField part-number checks Rename BlueField detection helpers and call sites to reflect that they operate on Redfish part numbers rather than model strings. Use the discovered DPU Card1 part number for fallback-serial mode heuristics, and keep explicit operator DPU mode handling independent of part-number availability. Tighten Lenovo BF3 DPU part-number matching and add coverage for Card1 part-number extraction. Signed-off-by: Dmitry Porokh --- crates/api-core/src/ipxe.rs | 6 +- crates/api-model/src/machine/mod.rs | 2 +- crates/api-model/src/site_explorer/mod.rs | 128 ++++++++++++++++------ crates/site-explorer/src/lib.rs | 75 ++++++------- 4 files changed, 134 insertions(+), 77 deletions(-) diff --git a/crates/api-core/src/ipxe.rs b/crates/api-core/src/ipxe.rs index 14cd663514..473a0ce5c7 100644 --- a/crates/api-core/src/ipxe.rs +++ b/crates/api-core/src/ipxe.rs @@ -353,8 +353,8 @@ exit || // use: // - If we don't have an exploration report for this MAC address, don't PXE boot at all // - If it's X86 and we have an exploration report, assume it's a Host. - // - If it's ARM and we have an exploration report, check if the report is a bluefield - // model. + // - If it's ARM and we have an exploration report, check if the report has a + // BlueField part number. let Some(endpoint) = db::explored_endpoints::find_by_mac_address(&mut *txn, interface.mac_address) .await? @@ -370,7 +370,7 @@ exit || let (machine_type, console) = match target.arch { rpc::MachineArchitecture::X86 => (MachineType::PredictedHost, console), rpc::MachineArchitecture::Arm => { - if endpoint.is_bluefield_model() { + if endpoint.has_bluefield_part_number() { (MachineType::Dpu, console) } else { (MachineType::PredictedHost, "ttyAMA0") diff --git a/crates/api-model/src/machine/mod.rs b/crates/api-model/src/machine/mod.rs index f176f1ed27..ec17a653cb 100644 --- a/crates/api-model/src/machine/mod.rs +++ b/crates/api-model/src/machine/mod.rs @@ -2908,7 +2908,7 @@ pub fn dpf_based_dpu_provisioning_possible( dpu.hardware_info .as_ref() .and_then(|hardware_info| hardware_info.dpu_info.as_ref()) - .map(|dpu_data| crate::site_explorer::is_bf2_dpu(&dpu_data.part_number)) + .map(|dpu_data| crate::site_explorer::is_bf2_dpu_part_number(&dpu_data.part_number)) .unwrap_or(false) }) { tracing::info!( diff --git a/crates/api-model/src/site_explorer/mod.rs b/crates/api-model/src/site_explorer/mod.rs index 2f0a943401..c663b391b1 100644 --- a/crates/api-model/src/site_explorer/mod.rs +++ b/crates/api-model/src/site_explorer/mod.rs @@ -291,16 +291,16 @@ impl ExploredEndpoint { versions } - pub fn is_bluefield_model(&self) -> bool { + pub fn has_bluefield_part_number(&self) -> bool { self.report.chassis.iter().any(|chassis| { chassis .part_number .as_ref() - .is_some_and(|p| is_bluefield_model(p.trim())) + .is_some_and(|p| is_bluefield_part_number(p.trim())) || chassis.network_adapters.iter().any(|n| { n.part_number .as_ref() - .is_some_and(|p| is_bluefield_model(p.trim())) + .is_some_and(|p| is_bluefield_part_number(p.trim())) }) }) } @@ -538,13 +538,13 @@ pub struct PCIeDevice { impl PCIeDevice { // is_bluefield returns whether the device is a Bluefield pub fn is_bluefield(&self) -> bool { - let Some(model) = &self.part_number else { - // TODO: maybe model this as an enum that has "Indeterminable" if there's no model + let Some(part_number) = &self.part_number else { + // TODO: maybe model this as an enum that has "Indeterminable" if there's no part number // but for now it's 'technically' true return false; }; - is_bluefield_model(model) + is_bluefield_part_number(part_number) } } #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] @@ -791,6 +791,19 @@ impl EndpointExplorationReport { } } + pub fn dpu_part_number(&self) -> Option<&str> { + if !self.is_dpu() { + return None; + } + + self.chassis + .iter() + .find(|chassis| chassis.id == "Card1") + .and_then(|chassis| chassis.part_number.as_deref()) + .map(str::trim) + .filter(|part_number| !part_number.is_empty()) + } + /// Return `true` if the explored endpoint is a DPU pub fn is_dpu(&self) -> bool { self.identify_dpu().is_some() @@ -1549,40 +1562,41 @@ impl Display for NicMode { } } -// returns true if the model is for a Bluefield-3 DPU -pub fn is_bf3_dpu(model: &str) -> bool { - let normalized_model = model.to_lowercase(); +// returns true if the part number is for a Bluefield-3 DPU +pub fn is_bf3_dpu_part_number(part_number: &str) -> bool { + let normalized_part_number = part_number.trim().to_lowercase(); // prefix matching for BlueField-3 DPUs (https://docs.nvidia.com/networking/display/bf3dpu) - normalized_model.starts_with("900-9d3b6") + normalized_part_number.starts_with("900-9d3b6") // looks like Lenovo ThinkSystem SR675 V3s will report the part number of NVIDIA BlueField-3 VPI QSFP112 2P 200G PCIe Gen5 x16 as SN37B36732 // https://windows-server.lenovo.com/repo/2024_05/html/SR675V3_7D9Q_7D9R-Windows_Server_2019.html - || normalized_model.starts_with("sn37b36732") + || normalized_part_number == "sn37b36732" } -// returns true if the model is for a Bluefield-3 SuperNIC -pub fn is_bf3_supernic(model: &str) -> bool { - let normalized_model = model.to_lowercase(); +// returns true if the part number is for a Bluefield-3 SuperNIC +pub fn is_bf3_supernic_part_number(part_number: &str) -> bool { + let normalized_part_number = part_number.trim().to_lowercase(); // prefix matching for BlueField-3 SuperNICs (https://docs.nvidia.com/networking/display/bf3dpu) - normalized_model.starts_with("900-9d3b4") || normalized_model.starts_with("900-9d3d4") + normalized_part_number.starts_with("900-9d3b4") + || normalized_part_number.starts_with("900-9d3d4") } -// returns true if the model is for a Bluefield-2 -pub fn is_bf2_dpu(model: &str) -> bool { - let normalized_model = model.to_lowercase(); +// returns true if the part number is for a Bluefield-2 +pub fn is_bf2_dpu_part_number(part_number: &str) -> bool { + let normalized_part_number = part_number.trim().to_lowercase(); // prefix matching for BlueField-2 DPU (https://docs.nvidia.com/nvidia-bluefield-2-ethernet-dpu-user-guide.pdf) - normalized_model.starts_with("mbf2") + normalized_part_number.starts_with("mbf2") } -// is_bluefield_model returns true if the passed in string is a bluefield model -pub fn is_bluefield_model(model: &str) -> bool { - let normalized_model = model.to_lowercase(); +// returns true if the passed in string is a BlueField part number +pub fn is_bluefield_part_number(part_number: &str) -> bool { + let normalized_part_number = part_number.trim().to_lowercase(); - normalized_model.contains("bluefield") - || is_bf3_dpu(&normalized_model) + normalized_part_number.contains("bluefield") + || is_bf3_dpu_part_number(&normalized_part_number) // prefix matching for BlueField-3 SuperNICs (https://docs.nvidia.com/networking/display/bf3dpu) - || is_bf3_supernic(&normalized_model) + || is_bf3_supernic_part_number(&normalized_part_number) // prefix matching for BlueField-2 DPU (https://docs.nvidia.com/nvidia-bluefield-2-ethernet-dpu-user-guide.pdf) // TODO (sp): should we be matching on all the individual models listed ("MBF2M516C-CECOT", .. etc) - || is_bf2_dpu(&normalized_model) + || is_bf2_dpu_part_number(&normalized_part_number) } /// The kind of BlueField/Mellanox device, classified from its Redfish part number. @@ -1611,20 +1625,20 @@ impl MlxDeviceKind { /// [`MlxDeviceKind::Unknown`] for a BlueField whose part number matches no /// known prefix (or is absent). pub fn from_part_number(part_number: Option<&str>) -> Self { - let Some(model) = part_number else { + let Some(part_number) = part_number else { return Self::Unknown; }; - let model = model.trim().to_lowercase(); - // `is_bf3_supernic` deliberately groups `900-9d3b4` and `900-9d3d4`; here + let part_number = part_number.trim().to_lowercase(); + // `is_bf3_supernic_part_number` deliberately groups `900-9d3b4` and `900-9d3d4`; here // we split them, because a NIC-mode DPU (`b4`) and a native SuperNIC // (`d4`) are exactly what an operator needs told apart. - if model.starts_with("900-9d3b6") || model.starts_with("sn37b36732") { + if part_number.starts_with("900-9d3b6") || part_number == "sn37b36732" { Self::Bf3DpuMode - } else if model.starts_with("900-9d3b4") { + } else if part_number.starts_with("900-9d3b4") { Self::Bf3NicMode - } else if model.starts_with("900-9d3d4") { + } else if part_number.starts_with("900-9d3d4") { Self::Bf3SuperNic - } else if model.starts_with("mbf2") { + } else if part_number.starts_with("mbf2") { Self::Bf2Dpu } else { Self::Unknown @@ -1840,6 +1854,42 @@ mod explored_mlx_device_tests { } } + fn dpu_report_with_card1_part_number(part_number: Option<&str>) -> EndpointExplorationReport { + EndpointExplorationReport { + systems: vec![ComputerSystem { + id: "Bluefield".to_string(), + ..Default::default() + }], + chassis: vec![Chassis { + id: "Card1".to_string(), + model: Some("BlueField-3 DPU".to_string()), + part_number: part_number.map(str::to_string), + ..Default::default() + }], + ..Default::default() + } + } + + #[test] + fn dpu_part_number_reads_card1_part_number() { + assert_eq!( + dpu_report_with_card1_part_number(Some("900-9D3B6-00CV-AA0")).dpu_part_number(), + Some("900-9D3B6-00CV-AA0") + ); + assert_eq!( + dpu_report_with_card1_part_number(Some("900-9D3B6-00CV-AA0 ")).dpu_part_number(), + Some("900-9D3B6-00CV-AA0") + ); + assert_eq!( + dpu_report_with_card1_part_number(None).dpu_part_number(), + None + ); + assert_eq!( + dpu_report_with_card1_part_number(Some(" ")).dpu_part_number(), + None + ); + } + #[test] fn classifies_bluefield_kind_by_part_number() { struct Case { @@ -1873,6 +1923,16 @@ mod explored_mlx_device_tests { part_number: Some("SN37B36732"), expected: MlxDeviceKind::Bf3DpuMode, }, + Case { + name: "lenovo-branded bf3 dpu with trailing spaces", + part_number: Some("SN37B36732 "), + expected: MlxDeviceKind::Bf3DpuMode, + }, + Case { + name: "serial-like lenovo prefix", + part_number: Some("SN37B36732XYZ"), + expected: MlxDeviceKind::Unknown, + }, Case { name: "bluefield without a known prefix", part_number: Some("NVIDIA BlueField mystery board"), @@ -1892,6 +1952,8 @@ mod explored_mlx_device_tests { case.name ); } + assert!(is_bf3_dpu_part_number(" SN37B36732 ")); + assert!(!is_bf3_dpu_part_number("SN37B36732XYZ")); } #[test] diff --git a/crates/site-explorer/src/lib.rs b/crates/site-explorer/src/lib.rs index b555056163..bd21f1ac4f 100644 --- a/crates/site-explorer/src/lib.rs +++ b/crates/site-explorer/src/lib.rs @@ -52,7 +52,8 @@ use model::resource_pool::common::CommonPools; use model::site_explorer::{ EndpointExplorationError, EndpointExplorationReport, EndpointType, ExploredDpu, ExploredEndpoint, ExploredManagedHost, ExploredManagedSwitch, MachineExpectation, NicMode, - PowerState, PreingestionState, Service, is_bf3_dpu, is_bf3_supernic, is_bluefield_model, + PowerState, PreingestionState, Service, is_bf3_dpu_part_number, is_bf3_supernic_part_number, + is_bluefield_part_number, }; use sqlx::PgPool; use tokio::task::JoinSet; @@ -1134,7 +1135,7 @@ impl SiteExplorer { // Resolve the operator-declared DPU mode for this host once; // it drives both auto-correction (`check_and_configure_dpu_mode` - // below -- operator override wins over BF3 model heuristics) + // below -- operator override wins over BF3 part-number heuristics) // and the post-match attach decision (NicMode/NoDpu hosts emit // a bare managed host regardless of what matched). let host_dpu_mode = effective_mode(&ep.address); @@ -1198,7 +1199,7 @@ impl SiteExplorer { .part_number .as_deref() .map(str::trim) - .is_some_and(is_bluefield_model); + .is_some_and(is_bluefield_part_number); let chassis_has_serial = chassis .serial_number .as_deref() @@ -1254,18 +1255,13 @@ impl SiteExplorer { { for dpu_sn in &expected_machine.data.fallback_dpu_serial_numbers { if let Some(dpu_ep) = dpu_sn_to_endpoint.remove(dpu_sn.as_str()) { - // Enforce the host's declared DPU mode on a fallback-serial - // match the same way the host-reported path does, rather than - // trusting it as already-configured. A DPU still in the wrong - // mode gets a `set_nic_mode` here and has to wait for the host - // reset to apply it; without this, a DPU-mode BlueField on a - // `NicMode` host would be attached and then dropped to zero-DPU - // (the `NicMode` arm further down), leaving the database reading - // "NIC-mode host" while the hardware stayed in DPU mode. + // Fallback matching has only the expected serial and the + // discovered DPU BMC report; pass the DPU's own part number + // for the legacy BF3 heuristic. let mode_check = Some( self.check_and_configure_dpu_mode( &dpu_ep, - dpu_ep.report.model().unwrap_or_default(), + dpu_ep.report.dpu_part_number(), host_dpu_mode, metrics, ) @@ -1587,7 +1583,10 @@ impl SiteExplorer { ) { // Count every DPU the host reports, independent of whether we've // discovered its BMC yet. - if part_number.map(str::trim).is_some_and(is_bluefield_model) { + if part_number + .map(str::trim) + .is_some_and(is_bluefield_part_number) + { exploration.reported_total += 1; } @@ -1603,18 +1602,10 @@ impl SiteExplorer { // Resolve the DPU's mode against what the host declared. This is the only // I/O, and may issue a `set_nic_mode` (in which case it returns `Ok(false)`). - let mode_check = match part_number { - Some(model) => Some( - self.check_and_configure_dpu_mode( - dpu_ep, - model.to_string(), - host_dpu_mode, - metrics, - ) + let mode_check = Some( + self.check_and_configure_dpu_mode(dpu_ep, part_number, host_dpu_mode, metrics) .await, - ), - None => None, - }; + ); match classify_matched_dpu(dpu_ep, host_ep, mode_check) { DiscoveredDpu::RunningAsDpu(dpu) => exploration.running_as_dpu.push(dpu), @@ -2940,13 +2931,13 @@ impl SiteExplorer { /// DPU but operator said no DPU" gets surfaced as a health alert; /// we don't try to reconfigure in that case. /// 3. Otherwise (operator default `DpuMode::DpuMode`), fall back to - /// the existing BF3 SuperNIC / BF3 DPU model-based heuristic for + /// the existing BF3 SuperNIC / BF3 DPU part-number heuristic for /// backward compat: BF3 SuperNIC → NIC mode, BF3 DPU → DPU mode, /// BF2 / unknown → no-op. async fn check_and_configure_dpu_mode( &self, dpu_ep: &ExploredEndpoint, - dpu_model: String, + dpu_part_number: Option<&str>, host_dpu_mode: DpuMode, metrics: &mut SiteExplorationMetrics, ) -> SiteExplorerResult { @@ -2957,15 +2948,19 @@ impl SiteExplorer { DpuMode::NicMode => Some(NicMode::Nic), DpuMode::NoDpu => None, DpuMode::DpuMode => { - // Preserve existing BF3-model heuristics when the operator - // hasn't explicitly chosen a mode. - if is_bf3_supernic(&dpu_model) { - Some(NicMode::Nic) - } else if is_bf3_dpu(&dpu_model) { - Some(NicMode::Dpu) - } else { - None - } + // Preserve existing BF3 part-number heuristics when the operator + // hasn't explicitly chosen a mode. Missing part numbers only + // disable this heuristic fallback; explicit modes above do not + // require a part number. + dpu_part_number.and_then(|dpu_part_number| { + if is_bf3_supernic_part_number(dpu_part_number) { + Some(NicMode::Nic) + } else if is_bf3_dpu_part_number(dpu_part_number) { + Some(NicMode::Dpu) + } else { + None + } + }) } }; @@ -2978,7 +2973,7 @@ impl SiteExplorer { Some(observed) => { tracing::warn!( address = %dpu_ep.address, - model = %dpu_model, + part_number = ?dpu_part_number, %observed, ?target_nic_mode, ?host_dpu_mode, @@ -3357,8 +3352,8 @@ enum DiscoveredDpu { /// /// The only IO (`check_and_configure_dpu_mode`, which may issue a /// `set_nic_mode`) happens in the caller, which passes its result in as -/// `mode_check` (`None` when the device reported no model to check). Keeping the -/// decision here makes it unit-testable without a Redfish mock. +/// `mode_check` (`None` when the caller deliberately skipped the mode check). +/// Keeping the decision here makes it unit-testable without a Redfish mock. fn classify_matched_dpu( dpu_ep: &ExploredEndpoint, host_ep: &ExploredEndpoint, @@ -3367,7 +3362,7 @@ fn classify_matched_dpu( match mode_check { Some(Ok(false)) => return DiscoveredDpu::NeedsReconfig, Some(Err(err)) => return DiscoveredDpu::ModeCheckFailed(err), - // Mode already correct, or there was no model to check. + // Mode already correct, or the caller skipped the mode check. Some(Ok(true)) | None => {} } @@ -3620,7 +3615,7 @@ mod tests { classify_matched_dpu(&dpu, &host, Some(Ok(true))), DiscoveredDpu::RunningAsDpu(_) )); - // No model to check (`None`) behaves the same. + // A skipped mode check (`None`) behaves the same. assert!(matches!( classify_matched_dpu(&dpu, &host, None), DiscoveredDpu::RunningAsDpu(_)