Skip to content

Commit 3bc527c

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 3bc527c

13 files changed

Lines changed: 249 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: 145 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,89 @@ impl Status {
754760
}
755761
}
756762

763+
/// How a caller identifies a boot interface to [`Redfish::machine_setup`]
764+
/// and supporting query methods.
765+
#[derive(Debug, Clone, Copy)]
766+
pub enum BootInterfaceRef<'a> {
767+
/// MAC address of the NIC to boot from. Vendor impl does its existing
768+
/// BMC-side lookup to translate this MAC into the vendor-native
769+
/// interface id its BIOS attributes actually consume. Use this when
770+
/// the BMC currently publishes the partition's MAC normally — the
771+
/// common steady-state case.
772+
Mac(&'a str),
773+
/// Vendor-native Redfish `EthernetInterface.Id` for the boot
774+
/// partition (e.g. `"NIC.Slot.7-1-1"`). Vendor impl uses it directly,
775+
/// skipping the MAC-side lookup entirely. Use this when the caller
776+
/// already has the id and wants to bypass the lookup — required when
777+
/// some BMCs wipe the MAC on partitions that aren't currently bound
778+
/// for boot, which makes [`BootInterfaceRef::Mac`] resolution fail
779+
/// (the motivating case is NIC-mode-DPU bring-up; the partition is
780+
/// Disabled until the BIOS PATCH activates it).
781+
InterfaceId(&'a str),
782+
}
783+
784+
impl<'a> BootInterfaceRef<'a> {
785+
/// Returns the MAC if this is the [`BootInterfaceRef::Mac`] variant.
786+
/// Returns `None` if this is the [`BootInterfaceRef::InterfaceId`]
787+
/// variant.
788+
pub fn mac(&self) -> Option<&'a str> {
789+
match self {
790+
BootInterfaceRef::Mac(mac) => Some(mac),
791+
BootInterfaceRef::InterfaceId(_) => None,
792+
}
793+
}
794+
}
795+
796+
/// Returns the current MAC address for a [`BootInterfaceRef`], fetching it
797+
/// from the BMC's `Systems/{}/EthernetInterfaces/{id}` resource when the
798+
/// caller supplied an [`BootInterfaceRef::InterfaceId`].
799+
///
800+
/// This is the cross-vendor primitive that lets every vendor's existing
801+
/// MAC-based `machine_setup_status` / `is_bios_setup` body keep working
802+
/// for both arms of [`BootInterfaceRef`]: callers that have a MAC pass it
803+
/// straight through, callers that have the BMC's stable id round-trip it
804+
/// to a MAC via the Redfish-standard `EthernetInterface` resource (which
805+
/// every vendor implements).
806+
///
807+
/// Errors when the resolved interface has no MAC populated, typically
808+
/// because the partition is currently Disabled/inactive — some BMCs wipe
809+
/// MAC tracking on partitions that aren't currently bound for boot. Once
810+
/// the partition is activated (e.g. via [`Redfish::machine_setup`] +
811+
/// reboot), the BMC re-publishes the MAC and a subsequent resolve will
812+
/// succeed.
813+
pub async fn resolve_boot_interface_mac<R: Redfish + ?Sized>(
814+
redfish: &R,
815+
boot_interface: BootInterfaceRef<'_>,
816+
) -> Result<String, RedfishError> {
817+
match boot_interface {
818+
BootInterfaceRef::Mac(mac) => Ok(mac.to_string()),
819+
BootInterfaceRef::InterfaceId(id) => {
820+
let eif = redfish.get_system_ethernet_interface(id).await?;
821+
extract_resolved_mac(eif.mac_address.as_deref(), id)
822+
}
823+
}
824+
}
825+
826+
/// Other half of [`resolve_boot_interface_mac`] split out for unit tests
827+
/// Returns the MAC if it's non-empty, otherwise an explicit error rather than
828+
/// passing an empty string through to vendor verification logic — some
829+
/// vendors do `display_name.contains(&mac)` to match boot options, which
830+
/// would silently match every option for an empty MAC.
831+
fn extract_resolved_mac(mac: Option<&str>, id: &str) -> Result<String, RedfishError> {
832+
let mac = mac.unwrap_or("");
833+
if mac.is_empty() {
834+
return Err(RedfishError::GenericError {
835+
error: format!(
836+
"Systems/.../EthernetInterfaces/{id} has no populated \
837+
MACAddress; the partition is likely Disabled or hasn't \
838+
been activated yet. Re-call after machine_setup + \
839+
reboot causes the BMC to re-publish the MAC."
840+
),
841+
});
842+
}
843+
Ok(mac.to_string())
844+
}
845+
757846
#[derive(Debug)]
758847
pub struct MachineSetupStatus {
759848
pub is_done: bool,
@@ -837,3 +926,55 @@ pub type BiosProfileVendor = HashMap<RedfishVendor, BiosProfileModel>;
837926
pub fn model_coerce(original: &str) -> String {
838927
str::replace(original, " ", "_")
839928
}
929+
930+
#[cfg(test)]
931+
mod tests {
932+
use super::*;
933+
934+
#[test]
935+
fn boot_interface_ref_mac_returns_inner() {
936+
let mac = "aa:bb:cc:dd:ee:01";
937+
let r = BootInterfaceRef::Mac(mac);
938+
assert_eq!(r.mac(), Some(mac));
939+
}
940+
941+
#[test]
942+
fn boot_interface_ref_interface_id_mac_is_none() {
943+
let r = BootInterfaceRef::InterfaceId("NIC.Slot.7-1-1");
944+
assert!(r.mac().is_none());
945+
}
946+
947+
#[test]
948+
fn extract_resolved_mac_passes_through_populated_mac() {
949+
let got = super::extract_resolved_mac(Some("AA:BB:CC:DD:EE:01"), "NIC.Slot.7-1-1")
950+
.expect("populated MAC should be returned as-is");
951+
assert_eq!(got, "AA:BB:CC:DD:EE:01");
952+
}
953+
954+
#[test]
955+
fn extract_resolved_mac_errors_on_empty_string_mac() {
956+
// The motivating case: some BMCs publish `MACAddress: ""` for a
957+
// Disabled partition. We must NOT silently return that empty
958+
// string — vendor verification logic that does
959+
// `display_name.contains(&mac)` would otherwise match every
960+
// boot option.
961+
let err = super::extract_resolved_mac(Some(""), "NIC.Slot.7-1-1")
962+
.expect_err("empty MAC should be an explicit error");
963+
let msg = err.to_string();
964+
assert!(
965+
msg.contains("NIC.Slot.7-1-1"),
966+
"error should name the interface id; got: {msg}",
967+
);
968+
assert!(
969+
msg.contains("Disabled") || msg.contains("activated"),
970+
"error should hint at the partition-state cause; got: {msg}",
971+
);
972+
}
973+
974+
#[test]
975+
fn extract_resolved_mac_errors_on_missing_mac_field() {
976+
let err = super::extract_resolved_mac(None, "NIC.Slot.7-1-1")
977+
.expect_err("None MAC should be an explicit error");
978+
assert!(err.to_string().contains("NIC.Slot.7-1-1"));
979+
}
980+
}

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
}

0 commit comments

Comments
 (0)