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
2 changes: 1 addition & 1 deletion Cargo.lock

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

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ authors = ["NVIDIA Carbide Engineering <carbide-dev@exchange.nvidia.com>"]

[workspace.dependencies]
clap = { version = "4", features = ["derive", "env"] }
libredfish = { git = "https://github.com/NVIDIA/libredfish.git", tag = "v0.44.21" }
libredfish = { git = "https://github.com/NVIDIA/libredfish.git", tag = "v0.44.22" }
librms = { git = "https://github.com/NVIDIA/nv-rms-client.git", tag = "v0.9.1" }
ansi-to-html = "0.2.2"

Expand Down
62 changes: 61 additions & 1 deletion crates/bmc-explorer/src/chassis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,17 @@ impl<B: Bmc> ExploredChassisCollection<B> {
})
}

pub fn is_mgx_c2(&self) -> bool {
self.members.iter().any(|m| {
let hardware_id = m.chassis.hardware_id();
is_mgx_c2_processor_module(
hardware_id.manufacturer.map(|v| v.into_inner()),
hardware_id.model.map(|v| v.into_inner()),
hardware_id.part_number.map(|v| v.into_inner()),
)
})
}

pub fn is_lenovo(&self) -> bool {
self.members
.iter()
Expand Down Expand Up @@ -489,6 +500,16 @@ fn is_delta_powershelf_chassis(chassis_id: &str, manufacturer: Option<&str>) ->
&& manufacturer.is_some_and(|mfg| mfg.to_lowercase().contains("delta"))
}

fn is_mgx_c2_processor_module(
manufacturer: Option<&str>,
model: Option<&str>,
part_number: Option<&str>,
) -> bool {
manufacturer.is_some_and(|v| v.eq_ignore_ascii_case("NVIDIA"))
&& (model.is_some_and(|v| v.starts_with("PG535"))
|| part_number.is_some_and(|v| v.contains("2G535")))
}

/// Aggregates per-PSU commanded on/off states into a single power-shelf state.
///
/// All PSUs reporting `Some(true)` means the shelf is on; all `Some(false)`
Expand Down Expand Up @@ -543,7 +564,46 @@ impl LiteOnSuppliesState<'_> {

#[cfg(test)]
mod tests {
use super::{ModelPowerState, is_delta_powershelf_chassis, powershelf_power_state};
use super::{
ModelPowerState, is_delta_powershelf_chassis, is_mgx_c2_processor_module,
powershelf_power_state,
};

#[test]
fn identifies_mgx_c2_processor_modules() {
let cases = [
("PG535 model", Some("NVIDIA"), Some("PG535-B02"), None, true),
(
"2G535 part number",
Some("Nvidia"),
None,
Some("699-2G535-0200-310"),
true,
),
(
"unrelated NVIDIA module",
Some("NVIDIA"),
Some("B4240"),
Some("900-9D3B4-00CC-EA0"),
false,
),
(
"non-NVIDIA PG535",
Some("Supermicro"),
Some("PG535-B02"),
Some("699-2G535-0200-310"),
false,
),
];

for (case, manufacturer, model, part_number, expected) in cases {
assert_eq!(
is_mgx_c2_processor_module(manufacturer, model, part_number),
expected,
"{case}",
);
}
}

// is_delta_powershelf_chassis gates Delta detection: a power-shelf chassis
// id ("chassis"/"powershelf") AND a Delta manufacturer. The manufacturer
Expand Down
3 changes: 2 additions & 1 deletion crates/bmc-explorer/src/hw/supermicro.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use crate::hw::BiosAttr;
// Real attribute names examples:
// "IPv4HTTPSupport_009F", "DeviceSelect_0034", "DeviceSelect_003D", "SR_IOVSupport_002B"
// This constant defines prefixes till last underscore.
pub const EXPECTED_BIOS_ATTRS_PREFIXES: [BiosAttr; 14] = [
pub const EXPECTED_BIOS_ATTRS_PREFIXES: [BiosAttr; 15] = [
BiosAttr::new_bool("QuietBoot", false),
BiosAttr::new_str("Re_tryBoot", "EFI Boot"),
BiosAttr::new_str("CSMSupport", "Disabled"),
Expand All @@ -33,6 +33,7 @@ pub const EXPECTED_BIOS_ATTRS_PREFIXES: [BiosAttr; 14] = [
// Not that some are "Enable" and some are "Enabled". Subtle.
BiosAttr::new_str("IntelVTforDirectedI_O_VT_d", "Enable"),
BiosAttr::new_str("IntelVirtualizationTechnology", "Enable"),
BiosAttr::new_str("SR-IOVSupport", "Enabled"),
BiosAttr::new_str("SR_IOVSupport", "Enabled"),
// UEFI NIC boot
BiosAttr::new_str("IPv4HTTPSupport", "Enabled"),
Expand Down
66 changes: 42 additions & 24 deletions crates/bmc-explorer/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,7 @@ pub async fn nv_generate_exploration_report<B: Bmc>(
let explored_system = ExploredComputerSystem::explore(system, &system_explore_config).await?;

let hw_type = hw_type(&root, &explored_system, &explored_chassis);
let is_mgx_c2 = explored_chassis.is_mgx_c2();
let manager_explore_config = hw_type
.map(|hw_type| match hw_type {
hw::HwType::Ami => manager::Config {
Expand All @@ -194,7 +195,7 @@ pub async fn nv_generate_exploration_report<B: Bmc>(
},
hw::HwType::Supermicro => manager::Config {
need_host_interfaces: true,
need_oem_supermicro_kcs_interface: true,
need_oem_supermicro_kcs_interface: !is_mgx_c2,
need_oem_supermicro_sys_lockdown: true,
..Default::default()
},
Expand Down Expand Up @@ -239,7 +240,14 @@ pub async fn nv_generate_exploration_report<B: Bmc>(
.await?;

let lockdown_status = hw_type
.map(|hw_type| lockdown_status(&hw_type, &explored_system, &explored_manager))
.map(|hw_type| {
lockdown_status(
&hw_type,
&explored_system,
&explored_manager,
&explored_chassis,
)
})
.transpose()?
.and_then(identity);

Expand Down Expand Up @@ -426,6 +434,7 @@ fn lockdown_status<B: Bmc>(
hw_type: &hw::HwType,
explored_system: &ExploredComputerSystem<B>,
explored_manager: &ExploredManager<B>,
explored_chassis: &ExploredChassisCollection<B>,
) -> Result<Option<LockdownStatus>, Error<B>> {
let bios = &explored_system.bios;
let system = &explored_system.system;
Expand Down Expand Up @@ -663,33 +672,42 @@ fn lockdown_status<B: Bmc>(
.as_ref()
.and_then(|lck| lck.sys_lockdown_enabled())
.ok_or_else(Error::bmc_not_provided("Supermicro lockdown status"))?;
let ipmi_host_interface_enabled = system
.raw()
.ipmi_host_interface
.as_ref()
.and_then(|interface| interface.service_enabled);
let message = format!(
"SysLockdownEnabled={is_syslockdown}, kcs_privilege={kcs_privilege:#?}, host_interface_enabled={hi_enabled}"
"SysLockdownEnabled={is_syslockdown}, kcs_privilege={kcs_privilege:#?}, \
host_interface_enabled={hi_enabled}, \
ipmi_host_interface_enabled={ipmi_host_interface_enabled:?}"
);

let model = system.hardware_id().model.map(|v| v.into_inner());
if model == Some("ARS-121L-DNR") {
// Grace-Grace SMCs (ARS-121L-DNR):
// 1. Need host_interface enabled even with lockdown
// 2. Doesn't provide KCSInterface
match (hi_enabled, is_syslockdown) {
(true, true) => Ok(InternalLockdownStatus::Enabled),
(true, false) => Ok(InternalLockdownStatus::Disabled),
_ => Ok(InternalLockdownStatus::Partial),
}
let is_ars_121l_dnr = model == Some("ARS-121L-DNR");
let (inband_locked, inband_unlocked) = if explored_chassis.is_mgx_c2() {
(
ipmi_host_interface_enabled.is_none_or(|enabled| !enabled),
ipmi_host_interface_enabled.is_none_or(identity),
)
} else {
match (hi_enabled, kcs_privilege, is_syslockdown) {
(false, Some(SupermicroPrivilege::Callback), true) => {
Ok(InternalLockdownStatus::Enabled)
}
(true, Some(SupermicroPrivilege::Administrator), false) => {
Ok(InternalLockdownStatus::Disabled)
}
(true, None, false) => Ok(InternalLockdownStatus::Disabled),
_ => Ok(InternalLockdownStatus::Partial),
}
}
.map(|status| Some(LockdownStatus { status, message }))
(
kcs_privilege == Some(SupermicroPrivilege::Callback),
kcs_privilege.is_none()
|| kcs_privilege == Some(SupermicroPrivilege::Administrator),
)
};
Comment on lines +688 to +699

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.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Map the file and inspect the relevant region.
echo "== crates/bmc-explorer/src/lib.rs outline =="
ast-grep outline crates/bmc-explorer/src/lib.rs --view expanded | sed -n '1,220p'

echo
echo "== relevant lines around 688-699 =="
sed -n '660,730p' crates/bmc-explorer/src/lib.rs | cat -n

echo
echo "== search for ipmi_host_interface_enabled usages =="
rg -n "ipmi_host_interface_enabled|inband_locked|inband_unlocked|InternalLockdownStatus::Enabled|host_interface_locked|is_syslockdown" crates/bmc-explorer/src/lib.rs crates -g '!target'

echo
echo "== tests mentioning MGX-C2 or lockdown status =="
rg -n "MGX-C2|mgx_c2|lockdown|host_interface|ipmi_host_interface_enabled" crates -g '*test*' -g '*tests*' -g '*.rs'

echo
echo "== if present, inspect nearby tests in the same file =="
rg -n "mod tests|#[[:space:]]*test" crates/bmc-explorer/src/lib.rs

Repository: NVIDIA/infra-controller

Length of output: 50380


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== lockdown_status function context =="
sed -n '433,735p' crates/bmc-explorer/src/lib.rs | cat -n | sed -n '1,340p'

echo
echo "== tests in crates/bmc-explorer =="
fd . crates/bmc-explorer/tests crates/bmc-explorer/src -t f | sed -n '1,200p'

echo
echo "== search for table-driven coverage around lockdown status in bmc-explorer tests =="
rg -n "lockdown_status|InternalLockdownStatus|mgx_c2|MGX|ipmi_host_interface_enabled|SupermicroPrivilege|Callback|Administrator" crates/bmc-explorer/tests crates/bmc-explorer/src -g '*.rs' -A 4 -B 4

echo
echo "== look for none_or patterns in the same function or nearby branches =="
rg -n "is_none_or\\(|is_some_and\\(" crates/bmc-explorer/src/lib.rs -A 2 -B 2

Repository: NVIDIA/infra-controller

Length of output: 47463


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== supermicro GB300 integration test =="
sed -n '1,220p' crates/bmc-explorer/tests/integration/supermicro_gb300_explore.rs | cat -n

echo
echo "== supermicro mock data references =="
rg -n "service_enabled|ipmi_host_interface|sys_lockdown|lockdown" crates/bmc-explorer/tests crates/bmc-mock crates -g '*.rs' -A 3 -B 3

echo
echo "== fixture builder for supermicro GB300 =="
sed -n '1,260p' crates/bmc-explorer/tests/integration/common.rs | cat -n

echo
echo "== any tests or comments about missing MGX-C2 host-interface fields =="
rg -n "MGX-C2|mgx_c2|when available|missing field|reference semantics|partial" crates/bmc-explorer crates -g '*.rs' -A 2 -B 2

Repository: NVIDIA/infra-controller

Length of output: 50380


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Redfish serial console model for service_enabled =="
rg -n "service_enabled|ServiceEnabled" crates -g '*.rs' -A 3 -B 3 | sed -n '1,240p'

echo
echo "== Supermicro mock that sets IPMI service_enabled =="
sed -n '180,230p' crates/bmc-mock/src/hw/supermicro_gb300_nvl.rs | cat -n

echo
echo "== supermicro OEM manager state defaults =="
sed -n '1,120p' crates/bmc-mock/src/redfish/oem/supermicro/manager.rs | cat -n

echo
echo "== inspect the generated/typed host interface accessors if present =="
rg -n "interface_enabled\\(|service_enabled\\(" crates -g '*.rs' -A 3 -B 3 | sed -n '1,240p'

Repository: NVIDIA/infra-controller

Length of output: 34636


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== locate the IPMI host interface type =="
rg -n "ipmi_host_interface|ServiceEnabled|service_enabled" crates/redfish crates/nv_redfish crates -g '*.rs' -A 4 -B 4 | sed -n '1,260p'

echo
echo "== inspect the relevant Redfish model definition if found =="
fd ".*(serial_console|host_interface|computer_system|manager).*\.rs$" crates/redfish crates/nv_redfish -t f | sed -n '1,120p'

echo
echo "== show the direct model around any ServiceEnabled field definition =="
rg -n "pub .*service_enabled|service_enabled:|ServiceEnabled" crates/redfish crates/nv_redfish -g '*.rs' -A 6 -B 6 | sed -n '1,220p'

Repository: NVIDIA/infra-controller

Length of output: 12062


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== find the Redfish type that carries ServiceEnabled =="
rg -n "pub .*service_enabled|service_enabled: Option<bool>|service_enabled: bool|ServiceEnabled" crates/redfish/src -g '*.rs' -A 6 -B 6 | sed -n '1,220p'

echo
echo "== inspect likely model files around the matching definition(s) =="
fd ".*(serial_console|host_interface|ethernet_interface|session_service|telemetry_service).*\.rs$" crates/redfish/src -t f | sed -n '1,120p'

Repository: NVIDIA/infra-controller

Length of output: 3739


Do not let missing MGX-C2 ServiceEnabled report Enabled crates/bmc-explorer/src/lib.rs:688-699
ipmi_host_interface_enabled is optional here, so None can still satisfy the is_syslockdown && inband_locked && host_interface_locked path and produce a false Enabled status. Treat the missing case as unknown/partial instead, and add table coverage for Some(true), Some(false), and None.

🤖 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-explorer/src/lib.rs` around lines 688 - 699, Update the MGX-C2
branch assigning inband_locked and inband_unlocked so
ipmi_host_interface_enabled == None is treated as unknown/partial rather than
satisfying the fully Enabled status path; preserve the existing Some(true) and
Some(false) behavior. Add table-driven coverage for Some(true), Some(false), and
None around the relevant status evaluation.

// ARS-121L-DNR must keep HostInterface enabled to PXE boot.
let host_interface_locked = hi_enabled == is_ars_121l_dnr;

let status = if is_syslockdown && inband_locked && host_interface_locked {
InternalLockdownStatus::Enabled
} else if !is_syslockdown && inband_unlocked && hi_enabled {
InternalLockdownStatus::Disabled
} else {
InternalLockdownStatus::Partial
};
Ok(Some(LockdownStatus { status, message }))
Comment thread
krish-nvidia marked this conversation as resolved.
}

hw::HwType::Hpe => {
Expand Down
Loading