Skip to content

Commit 650ae6a

Browse files
committed
feat: support interface ID on machine_setup via BootInterfaceRef
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). 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. 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. Signed-off-by: Chet Nichols III <chetn@nvidia.com>
1 parent dd2152a commit 650ae6a

13 files changed

Lines changed: 239 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: 21 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,14 @@ 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+
let resolved_mac = match boot_interface {
1331+
Some(b) => Some(crate::resolve_boot_interface_mac(self, b).await?),
1332+
None => None,
1333+
};
1334+
let boot_interface_mac = resolved_mac.as_deref();
13191335
let diffs = self.diff_bios_bmc_attr(boot_interface_mac).await?;
13201336
Ok(diffs.is_empty())
13211337
})

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: 136 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,80 @@ 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 derive the native id. Use this whenever the
769+
/// partition is in a state where the BMC publishes its MAC normally,
770+
/// allowing a subsequent lookup of MAC -> InterfaceId.
771+
Mac(&'a str),
772+
/// Vendor-native Redfish `EthernetInterface.Id` (e.g. Dell
773+
/// `"NIC.Slot.7-1-1"`). Vendor impl uses it directly. Use this when
774+
/// the interface ID/interface partition ID is already known, and we
775+
/// don't want or need to do a MAC address lookup for it.
776+
InterfaceId(&'a str),
777+
}
778+
779+
impl<'a> BootInterfaceRef<'a> {
780+
/// Returns the MAC if this is the [`BootInterfaceRef::Mac`] variant.
781+
/// Returns `None` if this is the [`BootInterfaceRef::InterfaceId`]
782+
/// variant.
783+
pub fn mac(&self) -> Option<&'a str> {
784+
match self {
785+
BootInterfaceRef::Mac(mac) => Some(mac),
786+
BootInterfaceRef::InterfaceId(_) => None,
787+
}
788+
}
789+
}
790+
791+
/// Returns the current MAC address for a [`BootInterfaceRef`], fetching it
792+
/// from the BMC's `Systems/{}/EthernetInterfaces/{id}` resource when the
793+
/// caller supplied an [`BootInterfaceRef::InterfaceId`].
794+
///
795+
/// This is the cross-vendor primitive that lets every vendor's existing
796+
/// MAC-based `machine_setup_status` / `is_bios_setup` body keep working
797+
/// for both arms of [`BootInterfaceRef`]: callers that have a MAC pass it
798+
/// straight through, callers that have the BMC's stable id round-trip it
799+
/// to a MAC via the Redfish-standard `EthernetInterface` resource (which
800+
/// every vendor implements).
801+
///
802+
/// Errors when the resolved interface has no MAC populated, typically
803+
/// because the partition is currently Disabled/inactive (e.g., a
804+
/// NIC-mode-DPU partition that hasn't been activated yet).
805+
pub async fn resolve_boot_interface_mac<R: Redfish + ?Sized>(
806+
redfish: &R,
807+
boot_interface: BootInterfaceRef<'_>,
808+
) -> Result<String, RedfishError> {
809+
match boot_interface {
810+
BootInterfaceRef::Mac(mac) => Ok(mac.to_string()),
811+
BootInterfaceRef::InterfaceId(id) => {
812+
let eif = redfish.get_system_ethernet_interface(id).await?;
813+
extract_resolved_mac(eif.mac_address.as_deref(), id)
814+
}
815+
}
816+
}
817+
818+
/// Helper for the interface ID handling side of
819+
/// resolve_boot_interface_mac, and also split out
820+
/// for tests.
821+
fn extract_resolved_mac(mac: Option<&str>, id: &str) -> Result<String, RedfishError> {
822+
let mac = mac.unwrap_or("");
823+
if mac.is_empty() {
824+
return Err(RedfishError::GenericError {
825+
error: format!(
826+
"Systems/.../EthernetInterfaces/{id} has no populated \
827+
MACAddress; the partition is likely Disabled or hasn't \
828+
been activated yet. Re-call after the BIOS PATCH \
829+
(machine_setup) + reboot causes the BMC to repopulate \
830+
the MAC."
831+
),
832+
});
833+
}
834+
Ok(mac.to_string())
835+
}
836+
757837
#[derive(Debug)]
758838
pub struct MachineSetupStatus {
759839
pub is_done: bool,
@@ -837,3 +917,55 @@ pub type BiosProfileVendor = HashMap<RedfishVendor, BiosProfileModel>;
837917
pub fn model_coerce(original: &str) -> String {
838918
str::replace(original, " ", "_")
839919
}
920+
921+
#[cfg(test)]
922+
mod tests {
923+
use super::*;
924+
925+
#[test]
926+
fn boot_interface_ref_mac_returns_inner() {
927+
let mac = "aa:bb:cc:dd:ee:01";
928+
let r = BootInterfaceRef::Mac(mac);
929+
assert_eq!(r.mac(), Some(mac));
930+
}
931+
932+
#[test]
933+
fn boot_interface_ref_interface_id_mac_is_none() {
934+
let r = BootInterfaceRef::InterfaceId("NIC.Slot.7-1-1");
935+
assert!(r.mac().is_none());
936+
}
937+
938+
#[test]
939+
fn extract_resolved_mac_passes_through_populated_mac() {
940+
let got = super::extract_resolved_mac(Some("AA:BB:CC:DD:EE:01"), "NIC.Slot.7-1-1")
941+
.expect("populated MAC should be returned as-is");
942+
assert_eq!(got, "AA:BB:CC:DD:EE:01");
943+
}
944+
945+
#[test]
946+
fn extract_resolved_mac_errors_on_empty_string_mac() {
947+
// The motivating case: iDRAC publishes `MACAddress: ""` for a
948+
// Disabled partition. We must NOT silently return that empty
949+
// string — vendor verification logic that does
950+
// `display_name.contains(&mac)` would otherwise match every
951+
// boot option.
952+
let err = super::extract_resolved_mac(Some(""), "NIC.Slot.7-1-1")
953+
.expect_err("empty MAC should be an explicit error");
954+
let msg = err.to_string();
955+
assert!(
956+
msg.contains("NIC.Slot.7-1-1"),
957+
"error should name the interface id; got: {msg}",
958+
);
959+
assert!(
960+
msg.contains("Disabled") || msg.contains("activated"),
961+
"error should hint at the partition-state cause; got: {msg}",
962+
);
963+
}
964+
965+
#[test]
966+
fn extract_resolved_mac_errors_on_missing_mac_field() {
967+
let err = super::extract_resolved_mac(None, "NIC.Slot.7-1-1")
968+
.expect_err("None MAC should be an explicit error");
969+
assert!(err.to_string().contains("NIC.Slot.7-1-1"));
970+
}
971+
}

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)