diff --git a/Cargo.lock b/Cargo.lock index 71284542b8..5a0370652d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6839,7 +6839,7 @@ dependencies = [ [[package]] name = "libredfish" version = "0.0.0" -source = "git+https://github.com/NVIDIA/libredfish.git?tag=v0.46.1#cee87be0dd8742d0bab9162f8419c780aaad0e3e" +source = "git+https://github.com/NVIDIA/libredfish.git?tag=v0.46.2#5bf822b5e7814df67e2b9a8f98f41fb96dea0a3b" dependencies = [ "chrono", "clap", diff --git a/Cargo.toml b/Cargo.toml index 96f524b0b3..b642e1b345 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -27,7 +27,7 @@ repository = "https://github.com/NVIDIA/infra-controller" [workspace.dependencies] clap = { version = "4", features = ["derive", "env"] } -libredfish = { git = "https://github.com/NVIDIA/libredfish.git", tag = "v0.46.1" } +libredfish = { git = "https://github.com/NVIDIA/libredfish.git", tag = "v0.46.2" } librms = { git = "https://github.com/NVIDIA/nv-rms-client.git", tag = "v0.10.0" } ansi-to-html = "0.2.2" diff --git a/crates/admin-cli/src/redfish/cmds.rs b/crates/admin-cli/src/redfish/cmds.rs index 4d162255c1..c4655d5cee 100644 --- a/crates/admin-cli/src/redfish/cmds.rs +++ b/crates/admin-cli/src/redfish/cmds.rs @@ -842,7 +842,7 @@ fn convert_ports_to_nice_table( .as_ref() .and_then(|ethernet| ethernet.mtu_size) .unwrap_or(0), - port.current_speed_gbps.unwrap_or(0), + port.current_speed_gbps.unwrap_or(0.0), ]); } } diff --git a/crates/redfish/src/libredfish/test_support.rs b/crates/redfish/src/libredfish/test_support.rs index 0646e8299d..af8c2c33e6 100644 --- a/crates/redfish/src/libredfish/test_support.rs +++ b/crates/redfish/src/libredfish/test_support.rs @@ -115,6 +115,10 @@ struct RedfishSimState { /// is password-redacted, and to exercise the quarantine-and-return-to-Ready /// path in host UEFI rotation. uefi_password_change_error: Option, + /// Optional ComputerSystem identifier used to drive platform classification. + system_id: Option, + /// Physical-port MAC addresses exposed through the adapter Ports collection. + network_adapter_port_mac_addresses: Vec, } /// Build the `HTTPErrorCode` a real BMC would return for a rejected request, so @@ -409,6 +413,22 @@ impl RedfishSim { self.state.lock().unwrap().chassis_manufacturer = manufacturer; } + /// Override the ComputerSystem identifier returned by the simulator. Site + /// Explorer classifies identifiers containing `bluefield` as DPUs. + pub fn set_system_id(&self, system_id: impl Into) { + self.state.lock().unwrap().system_id = Some(system_id.into()); + } + + /// Configure the physical-port MAC addresses returned by the simulated + /// `Chassis/.../NetworkAdapters/.../Ports` collection. A non-empty value + /// also advertises the parent `NetworkAdapters` link on `Card1`. + pub fn set_network_adapter_port_mac_addresses(&self, mac_addresses: Vec) { + self.state + .lock() + .unwrap() + .network_adapter_port_mac_addresses = mac_addresses; + } + /// Seed a credential into the sim's credential store -- the same store /// [`Self::credential_reader`] exposes. Controllers that resolve a credential /// through `redfish_client_pool.credential_reader()` (e.g. UEFI setup, which @@ -1029,13 +1049,11 @@ impl Redfish for RedfishSimClient { fn get_chassis<'a>( &'a self, - _id: &'a str, + id: &'a str, ) -> libredfish::RedfishFuture<'a, Result> { Box::pin(async move { - let manufacturer = self - .state - .lock() - .unwrap() + let state = self.state.lock().unwrap(); + let manufacturer = state .chassis_manufacturer .clone() .unwrap_or_else(|| "Nvidia".to_string()); @@ -1043,6 +1061,11 @@ impl Redfish for RedfishSimClient { manufacturer: Some(manufacturer), model: Some("Bluefield 3 SmartNIC Main Card".to_string()), name: Some("Card1".to_string()), + network_adapters: (id == "Card1" + && !state.network_adapter_port_mac_addresses.is_empty()) + .then(|| ODataId { + odata_id: "/redfish/v1/Chassis/Card1/NetworkAdapters".to_string(), + }), ..Default::default() }) }) @@ -1144,8 +1167,15 @@ impl Redfish for RedfishSimClient { ) -> libredfish::RedfishFuture<'a, Result> { Box::pin(async move { + let id = self + .state + .lock() + .unwrap() + .system_id + .clone() + .unwrap_or_else(|| "Bluefield".to_string()); Ok(libredfish::model::ComputerSystem { - id: "Bluefield".to_string(), + id, boot_progress: Some(libredfish::model::BootProgress { last_state: Some(libredfish::model::BootProgressTypes::OSRunning), last_state_time: Some(Utc::now().to_string()), @@ -1235,25 +1265,50 @@ impl Redfish for RedfishSimClient { _chassis_id: &'a str, _network_adapter: &'a str, ) -> libredfish::RedfishFuture<'a, Result, RedfishError>> { - Box::pin(async move { Ok(Vec::new()) }) + Box::pin(async move { + let count = self + .state + .lock() + .unwrap() + .network_adapter_port_mac_addresses + .len(); + Ok((0..count).map(|index| index.to_string()).collect()) + }) } fn get_port<'a>( &'a self, _chassis_id: &'a str, _network_adapter: &'a str, - _id: &'a str, + id: &'a str, ) -> libredfish::RedfishFuture<'a, Result> { Box::pin(async move { + let index = id + .parse::() + .map_err(|error| RedfishError::GenericError { + error: format!("invalid simulated network adapter port ID {id}: {error}"), + })?; + let state = self.state.lock().unwrap(); + let mac_address = state + .network_adapter_port_mac_addresses + .get(index) + .copied() + .ok_or_else(|| RedfishError::GenericError { + error: format!("unknown simulated network adapter port ID {id}"), + })?; Ok(libredfish::model::port::NetworkPort { odata: None, description: None, - id: None, + id: Some(id.to_string()), name: None, link_status: None, link_network_technology: None, current_speed_gbps: None, + ethernet: Some(libredfish::model::port::PortEthernet { + associated_mac_addresses: vec![mac_address.to_string()], + }), + oem: None, }) }) } diff --git a/crates/site-explorer/src/redfish.rs b/crates/site-explorer/src/redfish.rs index 9bd1b6a51f..aa7b332db6 100644 --- a/crates/site-explorer/src/redfish.rs +++ b/crates/site-explorer/src/redfish.rs @@ -287,18 +287,30 @@ impl RedfishClient { let manager = fetch_manager(client.as_ref()) .await .map_err(map_redfish_error)?; - let system = fetch_system(client.as_ref()).await?; - + let FetchedSystem { + mut system, + is_dpu, + is_host, + } = fetch_system(client.as_ref()).await?; + + let fetch_network_adapter_ports = + should_fetch_network_adapter_ports(is_host, &system.ethernet_interfaces); // TODO (spyda): once we test the BMC reset logic, we can enhance our logic here // to detect cases where the host's BMC is returning invalid (empty) chassis information, even though // an error is not returned. - let chassis = fetch_chassis(client.as_ref()) + let FetchedChassis { + chassis, + network_adapter_interfaces, + } = fetch_chassis(client.as_ref(), fetch_network_adapter_ports) .await .map_err(map_redfish_error)?; + system.ethernet_interfaces = merge_network_adapter_interfaces( + system.ethernet_interfaces, + network_adapter_interfaces, + ); let service = fetch_service(client.as_ref()) .await .map_err(map_redfish_error)?; - let is_dpu = system.id.to_lowercase().contains("bluefield"); let (machine_setup_status, remediation_error) = match fetch_machine_setup_status( client.as_ref(), boot_interface, @@ -784,7 +796,13 @@ async fn fetch_manager(client: &dyn Redfish) -> Result { }) } -async fn fetch_system(client: &dyn Redfish) -> Result { +struct FetchedSystem { + system: ComputerSystem, + is_dpu: bool, + is_host: bool, +} + +async fn fetch_system(client: &dyn Redfish) -> Result { let mut system = client.get_system().await.map_err(map_redfish_error)?; let is_dpu = system.id.to_lowercase().contains("bluefield"); let ethernet_interfaces = match fetch_ethernet_interfaces(client, true, is_dpu).await { @@ -889,21 +907,25 @@ async fn fetch_system(client: &dyn Redfish) -> Result Result, RedfishError> { +struct FetchedChassis { + chassis: Vec, + network_adapter_interfaces: Vec, +} + +fn should_fetch_network_adapter_ports( + is_host: bool, + system_interfaces: &[EthernetInterface], +) -> bool { + is_host + && !system_interfaces + .iter() + .any(|interface| interface.mac_address.is_some()) +} + +async fn fetch_network_adapter_port_interfaces( + client: &dyn Redfish, + chassis_id: &str, + network_adapter_id: &str, +) -> Vec { + let port_ids = match client.get_ports(chassis_id, network_adapter_id).await { + Ok(port_ids) => port_ids, + Err(error) => { + tracing::warn!( + %chassis_id, + %network_adapter_id, + %error, + "Failed to enumerate network adapter ports; continuing without port MAC addresses" + ); + return Vec::new(); + } + }; + + let mut interfaces = Vec::new(); + for port_id in port_ids { + let port = match client + .get_port(chassis_id, network_adapter_id, &port_id) + .await + { + Ok(port) => port, + Err(error) => { + tracing::warn!( + %chassis_id, + %network_adapter_id, + %port_id, + %error, + "Failed to read network adapter port; continuing without its MAC addresses" + ); + continue; + } + }; + let mac_addresses = match port.mac_addresses() { + Ok(mac_addresses) => mac_addresses, + Err(error) => { + tracing::warn!( + %chassis_id, + %network_adapter_id, + %port_id, + %error, + "Failed to parse network adapter port MAC addresses; continuing without them" + ); + continue; + } + }; + let link_status = port.link_status.map(|status| status.to_string()); + + interfaces.extend( + mac_addresses + .into_iter() + .map(|mac_address| EthernetInterface { + description: None, + // `Port.Id` names a physical port, not the firmware interface + // selector consumed by `BootInterfaceTarget`. Leaving this unset + // keeps a port-only discovery on the existing MAC-only path. + id: None, + interface_enabled: None, + mac_address: Some(mac_address), + link_status: link_status.clone(), + uefi_device_path: None, + }), + ); + } + interfaces +} + +async fn fetch_chassis( + client: &dyn Redfish, + fetch_network_adapter_ports: bool, +) -> Result { let mut chassis: Vec = Vec::new(); + let mut network_adapter_interfaces = Vec::new(); let chassis_list = client.get_chassis_all().await?; for chassis_id in &chassis_list { @@ -1106,6 +1217,12 @@ async fn fetch_chassis(client: &dyn Redfish) -> Result, RedfishErro .get_chassis_network_adapter(chassis_id, net_adapter_id) .await?; + if fetch_network_adapter_ports && value.ports.is_some() { + network_adapter_interfaces.extend( + fetch_network_adapter_port_interfaces(client, chassis_id, net_adapter_id).await, + ); + } + let net_adapter = NetworkAdapter { id: value.id, manufacturer: value.manufacturer, @@ -1163,7 +1280,25 @@ async fn fetch_chassis(client: &dyn Redfish) -> Result, RedfishErro }); } - Ok(chassis) + Ok(FetchedChassis { + chassis, + network_adapter_interfaces, + }) +} + +fn merge_network_adapter_interfaces( + mut system_interfaces: Vec, + network_adapter_interfaces: Vec, +) -> Vec { + for interface in network_adapter_interfaces { + if !system_interfaces + .iter() + .any(|existing| existing.mac_address == interface.mac_address) + { + system_interfaces.push(interface); + } + } + system_interfaces } async fn get_base_mac_from_bf4_ndf0(client: &dyn Redfish) -> Option { @@ -1570,14 +1705,16 @@ mod tests { use carbide_redfish::nv_redfish::NvRedfishClientPool; use carbide_secrets::credentials::Credentials; use carbide_test_support::Outcome::*; - use carbide_test_support::{Case, check_cases_async, value_scenarios}; + use carbide_test_support::{Case, Check, check_cases_async, check_values, value_scenarios}; use libredfish::model::service_root::RedfishVendor; use mac_address::MacAddress; use model::machine_boot_interface::{MachineBootInterface, MachineBootInterfaceTarget}; + use model::site_explorer::EthernetInterface; use super::{ BootInterfaceTarget, EndpointExplorationError, MachineSetupStatus, RedfishClient, - fetch_machine_setup_status, nv_bmc_explore_config, record_evaluated_boot_interface, + fetch_machine_setup_status, merge_network_adapter_interfaces, nv_bmc_explore_config, + record_evaluated_boot_interface, should_fetch_network_adapter_ports, }; fn test_addr() -> SocketAddr { @@ -1590,6 +1727,161 @@ mod tests { RedfishClient::new(sim, nv_pool) } + #[tokio::test] + async fn host_report_uses_network_adapter_port_mac_as_mac_only_interface() { + let mac_address = MacAddress::new([0x94, 0x6d, 0xae, 0x53, 0xcb, 0x9b]); + let sim = Arc::new(RedfishSim::default()); + sim.set_system_id("System"); + sim.set_network_adapter_port_mac_addresses(vec![mac_address]); + let redfish = build_redfish_client(sim); + + let report = redfish + .generate_exploration_report( + test_addr(), + Credentials::UsernamePassword { + username: "root".to_string(), + password: "password".to_string(), + }, + None, + None, + ) + .await + .unwrap(); + + let interface = report.systems[0] + .ethernet_interfaces + .iter() + .find(|interface| interface.mac_address == Some(mac_address)); + assert_eq!( + interface, + Some(&EthernetInterface { + mac_address: Some(mac_address), + ..Default::default() + }) + ); + assert_eq!(report.all_mac_addresses(), vec![mac_address]); + assert_eq!(report.find_interface_id_for_mac(mac_address), None); + assert_eq!(report.complete_boot_interfaces().count(), 0); + } + + #[tokio::test] + async fn network_adapter_port_invalid_ids_return_errors_without_poisoning_the_sim() { + let sim = RedfishSim::default(); + sim.set_network_adapter_port_mac_addresses(vec![MacAddress::new([ + 0x94, 0x6d, 0xae, 0x53, 0xcb, 0x9b, + ])]); + let client = sim + .create_client("test-host", None, RedfishAuth::Anonymous, None) + .await + .unwrap(); + + for (scenario, port_id) in [ + ("non-numeric identifier", "not-a-port"), + ("out-of-range identifier", "1"), + ] { + assert!( + client.get_port("Card1", "0", port_id).await.is_err(), + "{scenario} should return an error", + ); + assert!( + client.get_port("Card1", "0", "0").await.is_ok(), + "{scenario} should not poison simulator state", + ); + } + } + + #[test] + fn network_adapter_interfaces_keep_system_interface_ids() { + #[derive(Clone)] + struct Input { + system_interfaces: Vec, + network_adapter_interfaces: Vec, + } + + let mac_address = MacAddress::new([0x94, 0x6d, 0xae, 0x53, 0xcb, 0x9b]); + let system_interface = EthernetInterface { + id: Some("NIC.Slot.3-1-1".to_string()), + mac_address: Some(mac_address), + ..Default::default() + }; + let adapter_interface = EthernetInterface { + mac_address: Some(mac_address), + ..Default::default() + }; + + check_values( + [ + Check { + scenario: "port-only discovery stays MAC-only", + input: Input { + system_interfaces: vec![], + network_adapter_interfaces: vec![adapter_interface.clone()], + }, + expect: vec![adapter_interface.clone()], + }, + Check { + scenario: "System EthernetInterface wins for the same MAC", + input: Input { + system_interfaces: vec![system_interface.clone()], + network_adapter_interfaces: vec![adapter_interface], + }, + expect: vec![system_interface], + }, + ], + |input| { + merge_network_adapter_interfaces( + input.system_interfaces, + input.network_adapter_interfaces, + ) + }, + ); + } + + #[test] + fn network_adapter_port_fallback_gate_cases() { + #[derive(Clone)] + struct Input { + is_host: bool, + system_interfaces: Vec, + } + + let interface = |mac_address| EthernetInterface { + mac_address, + ..Default::default() + }; + let mac_address = MacAddress::new([0x94, 0x6d, 0xae, 0x53, 0xcb, 0x9b]); + + check_values( + [ + Check { + scenario: "host without a System MAC uses adapter ports", + input: Input { + is_host: true, + system_interfaces: vec![interface(None)], + }, + expect: true, + }, + Check { + scenario: "host with a System MAC skips adapter ports", + input: Input { + is_host: true, + system_interfaces: vec![interface(Some(mac_address))], + }, + expect: false, + }, + Check { + scenario: "non-host without a System MAC skips adapter ports", + input: Input { + is_host: false, + system_interfaces: vec![], + }, + expect: false, + }, + ], + |input| should_fetch_network_adapter_ports(input.is_host, &input.system_interfaces), + ); + } + async fn machine_setup_status_target( target: Option, ) -> Result<