Skip to content

Commit 21ae162

Browse files
committed
feat: accept Redfish interface id on machine_setup via BootInterfaceRef
This closes NVIDIA/infra-controller#2169. Enhances the `boot_interface_mac: Option<&str>` parameter on `machine_setup`, `machine_setup_status`, and `is_bios_setup` with a new `boot_interface: Option<BootInterfaceRef>`, which is an enum whose variants are `Mac` (existing behavior) and `InterfaceId`. The problem we're running into is when we flip a DPU from DPU mode to NIC mode, we run into this state where the `NetworkDeviceFunction` becomes `Disabled` (because it's now in NIC mode), and since the interface changed, it gets wiped as a boot device (and boot device goes back to `Disabled` with the default integraed NIC). ..AND when we go to `machine_setup` with the known MAC, that fails, because as part of switching from DPU mode to NIC mode (and the device function being reported as `Disabled`, it appears the vendor hardware doesn't re-probe the interface, so it doesn't populate a MAC address, so `machine_setup` fails, because there's no matching MAC address across all of these): * `Chassis/.../NetworkDeviceFunctions/<id>.Ethernet.MACAddress` * `Systems/.../EthernetInterfaces/<id>.MACAddress` * `Chassis/.../NetworkAdapters/<adapter>/NetworkPorts/<id>` fields However, if we just directly set the boot interface back to the previously set boot interface from before the flip, it forces a re-probe that interface partition ID, the MAC re-populates, and it UEFI HTTP boots. I had tried flipping the NDF back to `Enabled`, but it looks like some BMCs ALSO lock the `Enable` knob: `NetworkDeviceFunction.NetDevFuncCapabilities` is published as `["Disabled"]`, so the `NetDevFuncType` field literally can't be PATCHed away from `Disabled` via the NDF Settings URL. The only mechanism that actually activates a Disabled partition is to PATCH the vendor's HTTP-boot binding BIOS attribute to the partition's id and reboot — BIOS at POST then enables the underlying PCI function as a side effect, and the BMC re-inventories the MAC. That side-effect path is exactly what `machine_setup` already does; the only obstacle was that it derived the id from a MAC the BMC had wiped. By accepting an `InterfaceId` directly, callers that captured the id earlier (typically site-explorer during initial discovery, before the partition went Disabled) skip the broken lookup. So, this adds support to just let us specify the interface ID directly, and not need to pass a MAC to do a MAC -> interface ID lookup. This will have a corresponding change on the NICo side as well as part of pulling it in. Signed-off-by: Chet Nichols III <chetn@nvidia.com>
1 parent dd2152a commit 21ae162

13 files changed

Lines changed: 224 additions & 43 deletions

src/ami.rs

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -331,7 +331,7 @@ impl Redfish for Bmc {
331331
/// 3. BIOS settings
332332
fn machine_setup<'a>(
333333
&'a self,
334-
_boot_interface_mac: Option<&'a str>,
334+
_boot_interface: Option<crate::BootInterfaceRef<'a>>,
335335
_bios_profiles: &'a HashMap<
336336
RedfishVendor,
337337
HashMap<String, HashMap<BiosProfileType, HashMap<String, serde_json::Value>>>,
@@ -354,9 +354,17 @@ impl Redfish for Bmc {
354354
/// Check machine setup status for AMI BMC.
355355
fn machine_setup_status<'a>(
356356
&'a self,
357-
boot_interface_mac: Option<&'a str>,
357+
boot_interface: Option<crate::BootInterfaceRef<'a>>,
358358
) -> crate::RedfishFuture<'a, Result<MachineSetupStatus, RedfishError>> {
359359
Box::pin(async move {
360+
// Resolve `InterfaceId` to a MAC via the Redfish-standard
361+
// EthernetInterface resource.
362+
let resolved_mac = match boot_interface {
363+
Some(b) => Some(crate::resolve_boot_interface_mac(self, b).await?),
364+
None => None,
365+
};
366+
let boot_interface_mac = resolved_mac.as_deref();
367+
360368
let mut diffs = self.diff_bios_bmc_attr().await?;
361369

362370
if let Some(mac) = boot_interface_mac {
@@ -389,7 +397,7 @@ impl Redfish for Bmc {
389397

390398
fn is_bios_setup<'a>(
391399
&'a self,
392-
_boot_interface_mac: Option<&'a str>,
400+
_boot_interface: Option<crate::BootInterfaceRef<'a>>,
393401
) -> crate::RedfishFuture<'a, Result<bool, RedfishError>> {
394402
Box::pin(async move {
395403
let diffs = self.diff_bios_bmc_attr().await?;

src/dell.rs

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -270,7 +270,7 @@ impl Redfish for Bmc {
270270

271271
fn machine_setup<'a>(
272272
&'a self,
273-
boot_interface_mac: Option<&'a str>,
273+
boot_interface: Option<crate::BootInterfaceRef<'a>>,
274274
bios_profiles: &'a HashMap<
275275
RedfishVendor,
276276
HashMap<String, HashMap<BiosProfileType, HashMap<String, serde_json::Value>>>,
@@ -288,11 +288,14 @@ impl Redfish for Bmc {
288288
apply_time: dell::RedfishSettingsApplyTime::OnReset, // requires reboot to apply
289289
};
290290

291-
let (nic_slot, has_dpu) = match boot_interface_mac {
292-
Some(mac) => {
291+
let (nic_slot, has_dpu) = match boot_interface {
292+
Some(crate::BootInterfaceRef::Mac(mac)) => {
293293
let slot: String = self.dpu_nic_slot(mac).await?;
294294
(slot, true)
295295
}
296+
// Caller already knows the interface id/interface partition id,
297+
// so skip the MAC lookup and use the interface provided.
298+
Some(crate::BootInterfaceRef::InterfaceId(id)) => (id.to_string(), true),
296299
// Zero-DPU case
297300
None => ("".to_string(), false),
298301
};
@@ -367,9 +370,17 @@ impl Redfish for Bmc {
367370

368371
fn machine_setup_status<'a>(
369372
&'a self,
370-
boot_interface_mac: Option<&'a str>,
373+
boot_interface: Option<crate::BootInterfaceRef<'a>>,
371374
) -> crate::RedfishFuture<'a, Result<MachineSetupStatus, RedfishError>> {
372375
Box::pin(async move {
376+
// Resolve `InterfaceId` to a MAC via the Redfish-standard
377+
// EthernetInterface resource.
378+
let resolved_mac = match boot_interface {
379+
Some(b) => Some(crate::resolve_boot_interface_mac(self, b).await?),
380+
None => None,
381+
};
382+
let boot_interface_mac = resolved_mac.as_deref();
383+
373384
// Check BIOS and BMC attributes
374385
let mut diffs = self.diff_bios_bmc_attr(boot_interface_mac).await?;
375386

@@ -1313,9 +1324,15 @@ impl Redfish for Bmc {
13131324

13141325
fn is_bios_setup<'a>(
13151326
&'a self,
1316-
boot_interface_mac: Option<&'a str>,
1327+
boot_interface: Option<crate::BootInterfaceRef<'a>>,
13171328
) -> crate::RedfishFuture<'a, Result<bool, RedfishError>> {
13181329
Box::pin(async move {
1330+
// See `machine_setup_status` above for the resolver pattern.
1331+
let resolved_mac = match boot_interface {
1332+
Some(b) => Some(crate::resolve_boot_interface_mac(self, b).await?),
1333+
None => None,
1334+
};
1335+
let boot_interface_mac = resolved_mac.as_deref();
13191336
let diffs = self.diff_bios_bmc_attr(boot_interface_mac).await?;
13201337
Ok(diffs.is_empty())
13211338
})

src/hpe.rs

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -276,7 +276,7 @@ impl Redfish for Bmc {
276276

277277
fn machine_setup<'a>(
278278
&'a self,
279-
_boot_interface_mac: Option<&'a str>,
279+
_boot_interface: Option<crate::BootInterfaceRef<'a>>,
280280
_bios_profiles: &'a HashMap<
281281
RedfishVendor,
282282
HashMap<String, HashMap<BiosProfileType, HashMap<String, serde_json::Value>>>,
@@ -299,9 +299,17 @@ impl Redfish for Bmc {
299299

300300
fn machine_setup_status<'a>(
301301
&'a self,
302-
boot_interface_mac: Option<&'a str>,
302+
boot_interface: Option<crate::BootInterfaceRef<'a>>,
303303
) -> crate::RedfishFuture<'a, Result<MachineSetupStatus, RedfishError>> {
304304
Box::pin(async move {
305+
// Resolve `InterfaceId` to a MAC via the Redfish-standard
306+
// EthernetInterface resource.
307+
let resolved_mac = match boot_interface {
308+
Some(b) => Some(crate::resolve_boot_interface_mac(self, b).await?),
309+
None => None,
310+
};
311+
let boot_interface_mac = resolved_mac.as_deref();
312+
305313
// Check BIOS and BMC attributes
306314
let mut diffs = self.diff_bios_bmc_attr().await?;
307315

@@ -1133,7 +1141,7 @@ impl Redfish for Bmc {
11331141

11341142
fn is_bios_setup<'a>(
11351143
&'a self,
1136-
_boot_interface_mac: Option<&'a str>,
1144+
_boot_interface: Option<crate::BootInterfaceRef<'a>>,
11371145
) -> crate::RedfishFuture<'a, Result<bool, RedfishError>> {
11381146
Box::pin(async move {
11391147
let diffs = self.diff_bios_bmc_attr().await?;

src/lenovo.rs

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -251,7 +251,7 @@ impl Redfish for Bmc {
251251

252252
fn machine_setup<'a>(
253253
&'a self,
254-
_boot_interface_mac: Option<&'a str>,
254+
_boot_interface: Option<crate::BootInterfaceRef<'a>>,
255255
bios_profiles: &'a HashMap<
256256
RedfishVendor,
257257
HashMap<String, HashMap<BiosProfileType, HashMap<String, serde_json::Value>>>,
@@ -310,9 +310,17 @@ impl Redfish for Bmc {
310310

311311
fn machine_setup_status<'a>(
312312
&'a self,
313-
boot_interface_mac: Option<&'a str>,
313+
boot_interface: Option<crate::BootInterfaceRef<'a>>,
314314
) -> crate::RedfishFuture<'a, Result<MachineSetupStatus, RedfishError>> {
315315
Box::pin(async move {
316+
// Resolve `InterfaceId` to a MAC via the Redfish-standard
317+
// EthernetInterface resource.
318+
let resolved_mac = match boot_interface {
319+
Some(b) => Some(crate::resolve_boot_interface_mac(self, b).await?),
320+
None => None,
321+
};
322+
let boot_interface_mac = resolved_mac.as_deref();
323+
316324
// Check BIOS and BMC attributes
317325
let mut diffs = self.diff_bios_bmc_attr().await?;
318326

@@ -1166,7 +1174,7 @@ impl Redfish for Bmc {
11661174

11671175
fn is_bios_setup<'a>(
11681176
&'a self,
1169-
_boot_interface_mac: Option<&'a str>,
1177+
_boot_interface: Option<crate::BootInterfaceRef<'a>>,
11701178
) -> crate::RedfishFuture<'a, Result<bool, RedfishError>> {
11711179
Box::pin(async move {
11721180
let diffs = self.diff_bios_bmc_attr().await?;

src/lib.rs

Lines changed: 120 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,13 @@ pub trait Redfish: Send + Sync + 'static {
222222

223223
/// Sets up a reasonable UEFI configuration.
224224
/// remember to call lockdown() afterwards to secure the server
225-
/// - boot_interface_mac: MAC Address of the NIC you wish to boot from
225+
/// - boot_interface: identifies the NIC you wish to boot from. Either a
226+
/// `BootInterfaceRef::Mac` (existing behavior: vendor impl looks up
227+
/// the partition by MAC via its BMC enumeration) or a
228+
/// `BootInterfaceRef::InterfaceId` (vendor-native Redfish
229+
/// `EthernetInterface.Id` — used when we already know the interface
230+
/// partition ID (and don't need to look it up by MAC). One case
231+
/// being if we flip a DPU to NIC mode.
226232
/// If not given we look for a Mellanox Bluefield DPU and use that.
227233
/// Not applicable to Supermicro and the DPU itself.
228234
/// bios_profiles: Map of vendor/model (with spaces replaced by underscores)/profile/type
@@ -234,7 +240,7 @@ pub trait Redfish: Send + Sync + 'static {
234240
/// Ok(None) when no job is created. Caller should wait for job completion before configuring boot order.
235241
fn machine_setup<'a>(
236242
&'a self,
237-
boot_interface_mac: Option<&'a str>,
243+
boot_interface: Option<BootInterfaceRef<'a>>,
238244
bios_profiles: &'a BiosProfileVendor,
239245
selected_profile: BiosProfileType,
240246
oem_manager_profiles: &'a BiosProfileVendor,
@@ -243,13 +249,13 @@ pub trait Redfish: Send + Sync + 'static {
243249
/// Is everything that machine_setup does already done?
244250
fn machine_setup_status<'a>(
245251
&'a self,
246-
boot_interface_mac: Option<&'a str>,
252+
boot_interface: Option<BootInterfaceRef<'a>>,
247253
) -> RedfishFuture<'a, Result<MachineSetupStatus, RedfishError>>;
248254

249255
/// Check if only the BIOS/BMC setup is done
250256
fn is_bios_setup<'a>(
251257
&'a self,
252-
boot_interface_mac: Option<&'a str>,
258+
boot_interface: Option<BootInterfaceRef<'a>>,
253259
) -> RedfishFuture<'a, Result<bool, RedfishError>>;
254260

255261
/// Apply a standard BMC password policy. This varies a lot by vendor,
@@ -754,6 +760,70 @@ impl Status {
754760
}
755761
}
756762

763+
/// How a caller identifies a boot interface to [`Redfish::machine_setup`]
764+
/// and supporting query methods. Callers can pass either form; vendor
765+
/// impls handle both.
766+
#[derive(Debug, Clone, Copy)]
767+
pub enum BootInterfaceRef<'a> {
768+
/// MAC address of the boot interface. Vendor impl translates it
769+
/// into the vendor-native interface id its BIOS attributes consume.
770+
Mac(&'a str),
771+
/// Vendor-native Redfish `EthernetInterface.Id` for the boot
772+
/// interface (e.g. `"NIC.Slot.7-1-1"`). Vendor impl uses it
773+
/// directly.
774+
InterfaceId(&'a str),
775+
}
776+
777+
impl<'a> BootInterfaceRef<'a> {
778+
/// Returns the MAC if this is the [`BootInterfaceRef::Mac`] variant.
779+
/// Returns `None` if this is the [`BootInterfaceRef::InterfaceId`]
780+
/// variant.
781+
pub fn mac(&self) -> Option<&'a str> {
782+
match self {
783+
BootInterfaceRef::Mac(mac) => Some(mac),
784+
BootInterfaceRef::InterfaceId(_) => None,
785+
}
786+
}
787+
}
788+
789+
/// Returns the MAC address for a [`BootInterfaceRef`].
790+
/// [`BootInterfaceRef::Mac`] is a pass-through; [`BootInterfaceRef::InterfaceId`]
791+
/// is resolved by fetching `Systems/{}/EthernetInterfaces/{id}` via the
792+
/// Redfish-standard `EthernetInterface` resource (every vendor
793+
/// implements it).
794+
///
795+
/// Used by methods that compare against a MAC (verification paths that
796+
/// walk boot options by MAC substring, etc.) so the caller can pass
797+
/// either [`BootInterfaceRef`] variant uniformly.
798+
pub async fn resolve_boot_interface_mac<R: Redfish + ?Sized>(
799+
redfish: &R,
800+
boot_interface: BootInterfaceRef<'_>,
801+
) -> Result<String, RedfishError> {
802+
match boot_interface {
803+
BootInterfaceRef::Mac(mac) => Ok(mac.to_string()),
804+
BootInterfaceRef::InterfaceId(id) => {
805+
let eif = redfish.get_system_ethernet_interface(id).await?;
806+
extract_resolved_mac(eif.mac_address.as_deref(), id)
807+
}
808+
}
809+
}
810+
811+
/// Pure half of [`resolve_boot_interface_mac`], split out for unit
812+
/// tests. Returns the MAC if non-empty; errors otherwise rather than
813+
/// passing an empty string through to downstream `.contains(&mac)`
814+
/// matchers (which would match every option for an empty needle).
815+
fn extract_resolved_mac(mac: Option<&str>, id: &str) -> Result<String, RedfishError> {
816+
let mac = mac.unwrap_or("");
817+
if mac.is_empty() {
818+
return Err(RedfishError::GenericError {
819+
error: format!(
820+
"Systems/.../EthernetInterfaces/{id} has no populated MACAddress"
821+
),
822+
});
823+
}
824+
Ok(mac.to_string())
825+
}
826+
757827
#[derive(Debug)]
758828
pub struct MachineSetupStatus {
759829
pub is_done: bool,
@@ -837,3 +907,49 @@ pub type BiosProfileVendor = HashMap<RedfishVendor, BiosProfileModel>;
837907
pub fn model_coerce(original: &str) -> String {
838908
str::replace(original, " ", "_")
839909
}
910+
911+
#[cfg(test)]
912+
mod tests {
913+
use super::*;
914+
915+
#[test]
916+
fn boot_interface_ref_mac_returns_inner() {
917+
let mac = "aa:bb:cc:dd:ee:01";
918+
let r = BootInterfaceRef::Mac(mac);
919+
assert_eq!(r.mac(), Some(mac));
920+
}
921+
922+
#[test]
923+
fn boot_interface_ref_interface_id_mac_is_none() {
924+
let r = BootInterfaceRef::InterfaceId("NIC.Slot.7-1-1");
925+
assert!(r.mac().is_none());
926+
}
927+
928+
#[test]
929+
fn extract_resolved_mac_passes_through_populated_mac() {
930+
let got = super::extract_resolved_mac(Some("AA:BB:CC:DD:EE:01"), "NIC.Slot.7-1-1")
931+
.expect("populated MAC should be returned as-is");
932+
assert_eq!(got, "AA:BB:CC:DD:EE:01");
933+
}
934+
935+
#[test]
936+
fn extract_resolved_mac_errors_on_empty_string_mac() {
937+
// An empty MAC must error rather than pass through —
938+
// downstream `display_name.contains(&mac)` matchers would
939+
// otherwise match every boot option for an empty needle.
940+
let err = super::extract_resolved_mac(Some(""), "NIC.Slot.7-1-1")
941+
.expect_err("empty MAC should be an explicit error");
942+
let msg = err.to_string();
943+
assert!(
944+
msg.contains("NIC.Slot.7-1-1"),
945+
"error should name the interface id; got: {msg}",
946+
);
947+
}
948+
949+
#[test]
950+
fn extract_resolved_mac_errors_on_missing_mac_field() {
951+
let err = super::extract_resolved_mac(None, "NIC.Slot.7-1-1")
952+
.expect_err("None MAC should be an explicit error");
953+
assert!(err.to_string().contains("NIC.Slot.7-1-1"));
954+
}
955+
}

src/liteon_powershelf.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@ impl Redfish for Bmc {
223223

224224
fn machine_setup<'a>(
225225
&'a self,
226-
_boot_interface_mac: Option<&'a str>,
226+
_boot_interface: Option<crate::BootInterfaceRef<'a>>,
227227
_bios_profiles: &'a HashMap<
228228
RedfishVendor,
229229
HashMap<String, HashMap<BiosProfileType, HashMap<String, serde_json::Value>>>,
@@ -242,7 +242,7 @@ impl Redfish for Bmc {
242242

243243
fn machine_setup_status<'a>(
244244
&'a self,
245-
_boot_interface_mac: Option<&'a str>,
245+
_boot_interface: Option<crate::BootInterfaceRef<'a>>,
246246
) -> crate::RedfishFuture<'a, Result<MachineSetupStatus, RedfishError>> {
247247
Box::pin(async move {
248248
let diffs = vec![];
@@ -842,7 +842,7 @@ impl Redfish for Bmc {
842842

843843
fn is_bios_setup<'a>(
844844
&'a self,
845-
_boot_interface_mac: Option<&'a str>,
845+
_boot_interface: Option<crate::BootInterfaceRef<'a>>,
846846
) -> crate::RedfishFuture<'a, Result<bool, RedfishError>> {
847847
Box::pin(async move { Err(RedfishError::NotSupported("not supported".to_string())) })
848848
}

src/nvidia_dpu.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,7 @@ impl Redfish for Bmc {
235235

236236
fn machine_setup<'a>(
237237
&'a self,
238-
_boot_interface_mac: Option<&'a str>,
238+
_boot_interface: Option<crate::BootInterfaceRef<'a>>,
239239
_bios_profiles: &'a HashMap<
240240
RedfishVendor,
241241
HashMap<String, HashMap<BiosProfileType, HashMap<String, serde_json::Value>>>,
@@ -261,7 +261,7 @@ impl Redfish for Bmc {
261261

262262
fn machine_setup_status<'a>(
263263
&'a self,
264-
_boot_interface_mac: Option<&'a str>,
264+
_boot_interface: Option<crate::BootInterfaceRef<'a>>,
265265
) -> crate::RedfishFuture<'a, Result<MachineSetupStatus, RedfishError>> {
266266
Box::pin(async move {
267267
let mut diffs = vec![];
@@ -1015,10 +1015,10 @@ impl Redfish for Bmc {
10151015

10161016
fn is_bios_setup<'a>(
10171017
&'a self,
1018-
boot_interface_mac: Option<&'a str>,
1018+
boot_interface: Option<crate::BootInterfaceRef<'a>>,
10191019
) -> crate::RedfishFuture<'a, Result<bool, RedfishError>> {
10201020
Box::pin(async move {
1021-
let status = self.machine_setup_status(boot_interface_mac).await?;
1021+
let status = self.machine_setup_status(boot_interface).await?;
10221022
Ok(status.is_done)
10231023
})
10241024
}

0 commit comments

Comments
 (0)