Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

113 changes: 94 additions & 19 deletions crates/api-model/src/site_explorer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -610,10 +610,8 @@ impl ExploredDpu {
pub fn hardware_info(&self) -> ModelResult<HardwareInfo> {
let serial_number = self
.report
.systems
.first()
.and_then(|system| system.serial_number.as_ref())
.unwrap();
.dpu_pairing_serial_number()
.ok_or(ModelError::MissingArgument("Missing DPU serial number"))?;
let vendor = self
.report
.systems
Expand All @@ -626,7 +624,7 @@ impl ExploredDpu {
.and_then(|system| system.model.as_ref());
let dmi_data = self
.report
.create_temporary_dmi_data(serial_number.as_str(), vendor, model);
.create_temporary_dmi_data(serial_number, vendor, model);

let chassis_map = self
.report
Expand Down Expand Up @@ -1634,17 +1632,23 @@ pub fn is_bf2_dpu_part_number(part_number: &str) -> bool {
// prefix matching for BlueField-2 DPU (https://docs.nvidia.com/nvidia-bluefield-2-ethernet-dpu-user-guide.pdf)
normalized_part_number.starts_with("mbf2")
}

pub fn is_bf4_dpu_part_number(part_number: &str) -> bool {
let normalized_part_number = part_number.to_lowercase();
normalized_part_number.starts_with("900-9d4b4")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be a const, no?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should but it follows pattern of surroundings. All other part numbers prefixes are defined in corresponding functions. So this refactoring waits its hero.

}

// 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_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_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_part_number(&normalized_part_number)
|| is_bf4_dpu_part_number(&normalized_part_number)
}

/// The kind of BlueField/Mellanox device, classified from its Redfish part number.
Expand Down Expand Up @@ -1806,6 +1810,32 @@ impl EndpointExplorationReport {
.map(str::to_string)
.collect()
}

/// Serial key used to join a DPU BMC endpoint to the same DPU as reported by
/// its host BMC.
pub fn dpu_pairing_serial_number(&self) -> Option<&str> {
if !self.is_dpu() {
return None;
}

self.systems
.first()
.and_then(|system| system.serial_number.as_deref())
.map(str::trim)
.filter(|serial| !serial.is_empty())
.or_else(|| {
// BF4 Redfish does not currently expose the product serial or
// DPU/NIC mode on the system object. The stable product serial
// lives on the Bluefield_BMC chassis and matches the serial the
// host BMC reports for the PCIe/network-adapter device.
self.chassis
.iter()
.find(|chassis| chassis.id == "Bluefield_BMC")
.and_then(|chassis| chassis.serial_number.as_deref())
.map(str::trim)
.filter(|serial| !serial.is_empty())
})
}
}

/// Builds the [`ExploredMlxDevice`] view across a set of explored endpoints.
Expand All @@ -1817,22 +1847,14 @@ impl EndpointExplorationReport {
/// without those two fields. This is the same serial correlation site
/// exploration already uses to attach DPUs to their hosts.
pub fn collect_explored_mlx_devices(endpoints: &[ExploredEndpoint]) -> Vec<ExploredMlxDevice> {
// Index explored DPU endpoints by their (trimmed) system serial number, so a
// host-reported device can be matched to the DPU's own BMC endpoint -- the
// same key site exploration uses in `record_host_dpu_device`. Empty serials
// are skipped, and a serial reported by more than one DPU endpoint is dropped
// as ambiguous: better to attach nothing than to join to the wrong DPU.
// Index explored DPU endpoints by the serial that host BMCs report for the
// same device. Empty serials are skipped, and a serial reported by more than
// one DPU endpoint is dropped as ambiguous: better to attach nothing than to
// join to the wrong DPU.
let mut dpu_by_serial: HashMap<&str, &ExploredEndpoint> = HashMap::new();
let mut ambiguous: HashSet<&str> = HashSet::new();
for ep in endpoints.iter().filter(|ep| ep.report.is_dpu()) {
let Some(serial) = ep
.report
.systems
.first()
.and_then(|system| system.serial_number.as_deref())
.map(str::trim)
.filter(|serial| !serial.is_empty())
else {
let Some(serial) = ep.report.dpu_pairing_serial_number() else {
continue;
};
if dpu_by_serial.insert(serial, ep).is_some() {
Expand Down Expand Up @@ -2073,6 +2095,59 @@ mod explored_mlx_device_tests {
assert_eq!(supernic.nic_mode, None);
}

#[test]
fn projects_bf4_and_joins_dpu_by_bluefield_bmc_chassis_serial() {
const BF4_SERIAL: &str = "MT020000000003";
let host = endpoint(
"192.0.2.20",
EndpointExplorationReport {
endpoint_type: EndpointType::Bmc,
systems: vec![ComputerSystem {
pcie_devices: vec![pcie(
"900-9D4B4-CWAA-TSA",
"82.48.0802",
BF4_SERIAL,
"mat_2",
)],
..Default::default()
}],
..Default::default()
},
);
let dpu = endpoint(
"192.0.2.50",
EndpointExplorationReport {
endpoint_type: EndpointType::Bmc,
systems: vec![ComputerSystem {
id: "Bluefield".to_string(),
// BF4 leaves the system serial unset; the stable product
// serial used for host pairing is on the Bluefield_BMC
// chassis below.
serial_number: None,
..Default::default()
}],
chassis: vec![Chassis {
id: "Bluefield_BMC".to_string(),
serial_number: Some(BF4_SERIAL.to_string()),
..Default::default()
}],
..Default::default()
},
);

let devices = collect_explored_mlx_devices(&[host, dpu]);
assert_eq!(devices.len(), 1);

let bf4 = &devices[0];
assert_eq!(bf4.device_kind, MlxDeviceKind::Unknown);
assert_eq!(bf4.part_number.as_deref(), Some("900-9D4B4-CWAA-TSA"));
assert_eq!(bf4.serial_number.as_deref(), Some(BF4_SERIAL));
assert_eq!(bf4.dpu_bmc_ip, Some("192.0.2.50".parse().unwrap()));
// BF4 does not currently expose DPU/NIC mode through Redfish; missing
// mode must not prevent the serial join.
assert_eq!(bf4.nic_mode, None);
}

#[test]
fn serial_join_skips_empty_and_ambiguous_serials() {
let dpu = |addr: &str, serial: &str| {
Expand Down
14 changes: 14 additions & 0 deletions crates/bmc-explorer/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
* limitations under the License.
*/

use std::error::Error as StdError;
use std::fmt;

use nv_redfish::{Bmc, Error as NvRedfishError};
Expand Down Expand Up @@ -54,6 +55,19 @@ impl<B: Bmc> fmt::Debug for Error<B> {
}
}

impl<B> StdError for Error<B>
where
B: Bmc,
NvRedfishError<B>: 'static,
{
fn source(&self) -> Option<&(dyn StdError + 'static)> {
match self {
Self::NvRedfish { err, .. } => Some(err),
Self::BmcNotProvided(_) | Self::InvalidValue(_) => None,
}
}
}

impl<B: Bmc> Error<B> {
pub(crate) fn nv_redfish(context: &'static str) -> impl Fn(NvRedfishError<B>) -> Self {
move |err| Self::NvRedfish { context, err }
Expand Down
30 changes: 27 additions & 3 deletions crates/bmc-mock/src/hw/bluefield4.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,9 @@
use std::borrow::Cow;
use std::sync::Arc;

use carbide_utils::arch::CpuArchitecture;
use mac_address::MacAddress;
use rpc::machine_discovery::DiscoveryInfo;
use rpc::machine_discovery::{DiscoveryInfo, DmiData, DpuData};
use serde_json::json;

use crate::{Callbacks, LogService, LogServices, hw, redfish};
Expand Down Expand Up @@ -122,7 +123,9 @@ impl Bluefield4<'_> {
.build(),
),
log_services: Some(Arc::new(Bf4LogServices {
event_log: DpuEventLog { entries: vec![] },
event_log: DpuEventLog {
entries: vec!["DPU Warm Reset".to_string()],
},
})),
storage: Some(vec![]),
processors: Some(vec![]),
Expand Down Expand Up @@ -170,7 +173,28 @@ impl Bluefield4<'_> {
}

pub fn discovery_info(&self) -> DiscoveryInfo {
DiscoveryInfo::default()
DiscoveryInfo {
machine_type: CpuArchitecture::Aarch64.to_string(),
machine_arch: Some(rpc::utils::cpu_architecture_to_rpc(
CpuArchitecture::Aarch64,
)),
dmi_data: Some(DmiData {
board_name: "BlueField-4 DPU".into(),
product_serial: self.product_serial_number.to_string(),
board_serial: carbide_utils::DEFAULT_DPU_DMI_BOARD_SERIAL_NUMBER.into(),
chassis_serial: carbide_utils::DEFAULT_DPU_DMI_CHASSIS_SERIAL_NUMBER.into(),
product_name: "BlueField-4 DPU".into(),
sys_vendor: "Nvidia".into(),
..Default::default()
}),
dpu_info: Some(DpuData {
part_number: self.part_number().into(),
part_description: format!("NVIDIA BlueField-4 {}", self.part_number()),
factory_mac_address: self.host_mac_address.to_string(),
..Default::default()
}),
..Default::default()
}
Comment on lines +176 to +197

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
fd -e rs . crates | xargs rg -n "Bluefield4::discovery_info|discovery_info\(|DmiData|DpuData|900-9D4B4|BlueField-4 DPU"

Repository: NVIDIA/infra-controller

Length of output: 13149


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== outline: crates/bmc-mock/src/hw/bluefield4.rs =="
ast-grep outline crates/bmc-mock/src/hw/bluefield4.rs --view expanded || true

echo
echo "== outline: crates/bmc-mock/src/machine_info.rs =="
ast-grep outline crates/bmc-mock/src/machine_info.rs --view expanded | sed -n '1,220p' || true

echo
echo "== search for Bluefield4 discovery tests =="
rg -n "bluefield4\\(|Bluefield4|discovery_info\\(" crates/bmc-mock/src crates/bmc-mock/tests crates/site-explorer/tests crates/host-support/src | sed -n '1,240p'

Repository: NVIDIA/infra-controller

Length of output: 11305


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== bluefield4.rs relevant slice =="
sed -n '160,225p' crates/bmc-mock/src/hw/bluefield4.rs | cat -n

echo
echo "== machine_info.rs around Bluefield4 and tests =="
sed -n '130,230p' crates/bmc-mock/src/machine_info.rs | cat -n

echo
echo "== machine_info.rs test slice around BF4 usage =="
sed -n '460,540p' crates/bmc-mock/src/machine_info.rs | cat -n

echo
echo "== search for discovery_info assertions in bmc-mock tests/modules =="
rg -n "discovery_info\\(\\)|machine_arch|part_description|factory_mac_address|board_name|BlueField-4 DPU" crates/bmc-mock/src crates/bmc-mock/tests crates/site-explorer/tests | sed -n '1,220p'

Repository: NVIDIA/infra-controller

Length of output: 15257


Add direct coverage for Bluefield4::discovery_info()
The BF4 regression currently exercises only the Redfish pairing path; add a focused table-driven test for this method, or point to existing coverage, so the machine_arch, dmi_data, and dpu_info fields do not drift silently.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/bmc-mock/src/hw/bluefield4.rs` around lines 176 - 197, Add direct test
coverage for Bluefield4::discovery_info() instead of relying only on the Redfish
pairing flow. Create a focused table-driven test around discovery_info() that
asserts the machine_arch, dmi_data, and dpu_info values for BF4 stay correct, or
point to an existing test that already validates those fields. Use the
Bluefield4 type and discovery_info() method as the main anchors so the coverage
is easy to find and maintain.

Sources: Coding guidelines, Path instructions

}

fn part_number(&self) -> &'static str {
Expand Down
14 changes: 14 additions & 0 deletions crates/bmc-mock/src/test_support/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,20 @@ async fn test_bmc((router, state): (axum::Router, BmcState)) -> TestBmcHandle {
}
}

pub async fn bmc_for_machine(machine_info: MachineInfo) -> TestBmcHandle {
let machine_id = match &machine_info {
MachineInfo::Host(_) => "test-host-id",
MachineInfo::Dpu(_) => "test-dpu-id",
};
test_bmc(machine_router(
&machine_info,
Arc::new(NoopCallbacks),
machine_id.to_string(),
false,
))
.await
}

fn host_info(hw_type: HostHardwareType) -> MachineInfo {
let ndpu = hw_type.fixed_number_of_dpu().unwrap_or(0);
let mut pool = TEST_MAC_POOL.lock().unwrap();
Expand Down
1 change: 1 addition & 0 deletions crates/site-explorer/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ thiserror = { workspace = true }
version-compare = { workspace = true }

[dev-dependencies]
bmc-mock = { path = "../bmc-mock" }
bmc-vendor = { path = "../bmc-vendor" }
carbide-api-model = { path = "../api-model", default-features = false, features = [
"test-support",
Expand Down
57 changes: 33 additions & 24 deletions crates/site-explorer/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1186,17 +1186,14 @@ impl SiteExplorer {
.map(|em| em.data.dpu_mode);
DpuMode::resolve(declared, site_dpu_mode)
};
// Match HOST and DPU using SerialNumber.
// Compare DPU system.serial_number with HOST chassis.network_adapters[].serial_number
// Match HOST and DPU using the serial Redfish reports for the same
// physical card. BF4 does not expose that serial on the DPU system
// object, so this uses `EndpointExplorationReport::dpu_pairing_serial_number`
// rather than reading `systems[0].serial_number` directly.
let mut dpu_sn_to_endpoint = HashMap::new();
for (_, ep) in explored_dpus {
if let Some(sn) = ep
.report
.systems
.first()
.and_then(|system| system.serial_number.as_ref())
{
dpu_sn_to_endpoint.insert(sn.trim().to_string(), ep);
if let Some(sn) = ep.report.dpu_pairing_serial_number() {
dpu_sn_to_endpoint.insert(sn.to_string(), ep);
}
}

Expand Down Expand Up @@ -2810,28 +2807,38 @@ impl SiteExplorer {
return Ok(true);
}

if let Some(nic_mode) = dpu_endpoint.report.nic_mode() {
// DPU's in NIC mode do not have full redfish functionality,
// for example, we will not be able to retrieve the base GUID
// from the redfish response. Skip the next check because the DPUs
// in NIC mode will not expose a pf0 interface to the host.
if nic_mode == NicMode::Nic {
match dpu_endpoint.report.nic_mode() {
Some(NicMode::Nic) => {
// DPU's in NIC mode do not have full redfish functionality,
// for example, we will not be able to retrieve the base GUID
// from the redfish response. Skip the next check because the DPUs
// in NIC mode will not expose a pf0 interface to the host.
tracing::info!(
"Site explorer found an uningested DPU (bmc ip: {}) in NIC mode",
dpu_endpoint.address
);
return Ok(true);
}
} else {
tracing::error!(
"Site explorer found an uningested DPU (bmc ip: {}) without being able to determine if it is in NIC mode",
dpu_endpoint.address
);
metrics.increment_host_dpu_pairing_blocker(PairingBlockerReason::DpuNicModeUnknown);
return Ok(false);
Some(NicMode::Dpu) => {}
None if dpu_endpoint.report.dpu_pairing_serial_number().is_some() => {
tracing::warn!(
"Site explorer found an uningested DPU (bmc ip: {}) without a Redfish DPU/NIC mode; continuing because it has a host-pairing serial",
dpu_endpoint.address
);
}
None => {
tracing::error!(
"Site explorer found an uningested DPU (bmc ip: {}) without being able to determine if it is in NIC mode",
dpu_endpoint.address
);
metrics.increment_host_dpu_pairing_blocker(PairingBlockerReason::DpuNicModeUnknown);
return Ok(false);
}
}

// This is a bluefield in DPU mode
// This is a BlueField that should be pairable as a managed DPU. BF4 may
// not report mode, so host pairing and the PF MAC check decide whether
// it can continue.
match find_host_pf_mac_address(dpu_endpoint) {
Ok(_) => Ok(true),
Err(error) => {
Expand Down Expand Up @@ -3205,7 +3212,9 @@ impl SiteExplorer {
// 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.
// require a part number. BF4 does not currently
// expose a reliable DPU/NIC mode signal over Redfish, so the
// default path does not infer or reconfigure BF4 mode,
dpu_part_number.and_then(|dpu_part_number| {
if is_bf3_supernic_part_number(dpu_part_number) {
Some(NicMode::Nic)
Expand Down
Loading
Loading