feat: expose network adapter port MAC addresses by chet · Pull Request #180 · NVIDIA/nv-redfish

chet · 2026-08-01T02:26:16Z

We ran into a Lenovo host with an empty Systems/.../EthernetInterfaces collection, which meant site-explorer couldn't find the host MAC it needed to recognize the PXE interface. The NIC was there -- it was just exposed under Chassis/.../NetworkAdapters/.../Ports, with Lenovo XCC putting the physical-port address in Oem.Lenovo.PhysicalPortMacAddress.

I don't think every caller should have to learn that Lenovo detail. nv-redfish is our vendor abstraction layer, so this teaches it to traverse the standard port collection and fall back to Lenovo's OEM value only when Port.Ethernet.AssociatedMACAddresses is missing or empty. That gives callers one path for both cases, while standard values still win and the MAC text stays exactly as the BMC returned it.

What changed

add typed PortCollection and Port wrappers behind a new ports feature
expose NetworkAdapter::ports() for standard adapter-port traversal
prefer standard associated MAC addresses and fall back to Lenovo XCC's OEM physical-port value
cover standard, Lenovo-only, precedence, empty-value, missing-link, and end-to-end collection behavior

Testing

make ci
cargo test -p nv-redfish-tests --test test-network-adapter-port -- --no-capture

Related issues

This supports #176.

This supports NVIDIA/infra-controller#4436.

Closes #176

jayzhudev · 2026-08-01T03:48:37Z

+    ) -> Result<Self, Error<B>> {
+        let data = nav.get(bmc.as_ref()).await.map_err(Error::Bmc)?;
+        #[cfg(feature = "oem-lenovo")]
+        let oem_lenovo = LenovoPort::new(&data).map_err(Error::Json)?;


Should Lenovo port parsing be deferred until a fallback is needed? With this feature enabled, an incompatible Lenovo value can fail Port::new() and the whole collection. But valid standard MAC address data may be available in the meantime.

Yes even worse whole members() would fail if even one fails here.

Yeah, good call -- I’ll move Lenovo parsing out of Port::new() and defer it to explicit OEM access, so a malformed Lenovo payload can’t fail members() or hide usable standard MAC data. I'll post back when it's done, lemme tweak.

yoks · 2026-08-01T04:38:50Z


+/// Support of Lenovo Port OEM attributes.
+#[cfg(feature = "ports")]
+pub(crate) mod port;


why pub(crate) here? Is not used anywhere inside.

Sooo, this is/was pub(crate) because port wasn't visible to oem::lenovo. BUT, @poroh's comment below will actually change this, with a pub LenovoPort that we'll make available to through Port::oem_lenovo(). I'll ping back once it's updated! Looking through the other comments still.

yoks · 2026-08-01T04:39:55Z

+}
+
+impl<B: Bmc> PortCollection<B> {
+    #[allow(dead_code)] // Used by NetworkAdapter when that feature is enabled.


I think you can drop it, it would be generated only when port is reachable which requires network adapters

Nice okay! Will drop.

yoks · 2026-08-01T04:41:03Z

+    collection: Arc<PortCollectionSchema>,
+}
+
+impl<B: Bmc> PortCollection<B> {


we usually spliting collection and port in different files in repo. Rule of thumb one primary struct per rs file

Okay nice -- yeah I like that. Give me a bit; I'll split this across port/mod.rs, port/collection.rs, and port/item.rs.

yoks · 2026-08-01T04:45:31Z

+
+    /// Get Lenovo's physical-port MAC address.
+    #[must_use]
+    pub fn physical_port_mac_address(&self) -> Option<&str> {


why erase type here?

Yupppp -- will update to return Option<MacAddress>.

yoks · 2026-08-01T04:53:40Z

+    /// returned when the standard array is absent or empty.
+    ///
+    #[must_use]
+    pub fn associated_mac_addresses(&self) -> Vec<MacAddress<'_>> {


This function doing a little too much. It is not clear from outside (by name at least), what it OEM dependent. associated_mac_addresses is something standart, this silently switches to Lenovo if it not found and Lenovo OEM enabled.

If we want something with vendor fallback (something smart) we need name it as such i think.

It also kinda pushes Site Explorer logic into nv-redfish, which i'm not fan of, better be other way around. But i will let @poroh to chime here.

nv-redfish rule of thumb is "do not try to be smart when provide data to library user". If data is available through OEM extension then nv-redfish should provide access to OEM extension and let library user to choose if they want to use LenovoPort::physical_port_mac_address as associated_mac_addresses.

This decision is not nv-redfish buisness.

Yeah agreed. I realize we're fixing a lot of anti-patterns we inherited and/or continued with libredfish with this repo -- thanks for ensuring it stays that way. I'll fiddle with it so port.associated_mac_addresses() returns just standard Ethernet.AssociatedMACAddresses, and then port.oem_lenovo() returns LenovoPort, so site-explorer would end up doing something like...

let addresses = port.associated_mac_addresses(); if addresses.is_empty() { if let Some(lenovo) = port.oem_lenovo()? { if let Some(address) = lenovo.physical_port_mac_address() { // and then site-explorer does some fallback work here or something like that } } }

Back in a bit!

chet · 2026-08-04T05:57:11Z

@yoks @poroh @jayzhudev Okay, I think this is ready for another pass. Let me know if you want any more adjustments. Thanks for looking through it!

poroh · 2026-08-04T06:29:56Z

+
+use crate::mac_address::MacAddress;
+#[cfg(feature = "oem-lenovo")]
+use crate::oem::lenovo::port::LenovoPort;


nitpick: I prefer to separate conditional imports and non-conditional imports.

@poroh Updated! I think I'll need a re-approval now though.

jayzhudev

LGTM

Some BMCs leave Systems/.../EthernetInterfaces empty even though the installed host NIC is visible through Chassis/.../NetworkAdapters/.../Ports. Add the standard Port collection and resource wrappers and let NetworkAdapter callers traverse that collection. Port::associated_mac_addresses prefers Ethernet.AssociatedMACAddresses, then falls back to Lenovo XCC's Oem.Lenovo.PhysicalPortMacAddress when standard data is absent or empty. Keep vendor handling inside nv-redfish, preserve server-provided MAC formatting, and cover standard, Lenovo-only, precedence, empty, missing-link, and end-to-end collection behavior. Closes NVIDIA#176 Signed-off-by: Chet Nichols III <chetn@nvidia.com>

jayzhudev

Sending extra love : )

…4968) Lenovo XCC can report usable onboard `ComputerSystem.EthernetInterfaces` while exposing an installed ConnectX NIC only through a linked chassis `NetworkAdapter.Port`. The adapter-Port fetch treated any usable System MAC as proof that inventory was complete, so the declared NoDpu boot NIC never reached `predicted_machine_interfaces`. So, this leaves the existing no-usable-System-MAC fallback intact and adds a separate supplemental case inside the same verified Lenovo + `ComputerSystem.Links.Chassis` boundary. That case collects adapter Ports even when usable System interfaces exist. Site Explorer still uses System interfaces as Host candidates; it adds a Port MAC alongside them only when that MAC was actually reported by hardware and `ExpectedMachine` declares it as a Host interface. It does not synthesize `EthernetInterfaces`, treat the declaration as an override, or use a Port ID as a boot-interface ID. A refreshed report can now add that MAC to an existing predicted host, keep its boot target MAC-only, and leave already-managed hosts alone. Retained boot metadata is consulted only when a predicted host has no primary, so an unrelated stale record cannot replace a settled primary. This means verified Lenovo XCC hosts fetch Ports from the explicitly linked chassis on each exploration. That extra inventory work stays behind the vendor + linked-chassis boundary because a generic chassis Port is not necessarily a host/PXE interface. Tests added! ## Related issues This supports #4952 ## Type of Change - [ ] **Add** - New feature or capability - [ ] **Change** - Changes in existing functionality - [x] **Fix** - Bug fixes - [ ] **Remove** - Removed features or deprecated functionality - [ ] **Internal** - Internal changes (refactoring, tests, docs, etc.) ## Breaking Changes - [ ] **This PR contains breaking changes** ## Testing - [x] Unit tests added/updated - [x] Integration tests added/updated - [ ] Manual testing performed - [ ] No testing required (docs, internal refactor, etc.) ```bash cargo test -p bmc-explorer -p carbide-site-explorer --lib make core/tests TEST_ARGS="-p carbide-site-explorer --test integration zero_dpu::test_ -- --nocapture" cargo make clippy carbide-lints --all-targets --all-features cargo make format-nightly cargo make check-licenses cargo make check-bans cargo xtask check-workspace-deps taplo fmt --check crates/site-explorer/Cargo.toml ``` ## Additional Notes This is the caller-side follow-up to #4469 and #4534, using the adapter-Port inventory exposed by [NVIDIA/nv-redfish#180](NVIDIA/nv-redfish#180). Signed-off-by: Chet Nichols III <chetn@nvidia.com>

chet requested review from jayzhudev, poroh and yoks as code owners August 1, 2026 02:26

chet force-pushed the gh-issue-176-review branch from 4903134 to c612bd6 Compare August 1, 2026 02:45

jayzhudev reviewed Aug 1, 2026

View reviewed changes

yoks reviewed Aug 1, 2026

View reviewed changes

poroh requested changes Aug 1, 2026

View reviewed changes

chet force-pushed the gh-issue-176-review branch from c612bd6 to 744e0d9 Compare August 4, 2026 05:52

poroh reviewed Aug 4, 2026

View reviewed changes

poroh approved these changes Aug 4, 2026

View reviewed changes

jayzhudev approved these changes Aug 4, 2026

View reviewed changes

chet force-pushed the gh-issue-176-review branch from 744e0d9 to d83b167 Compare August 4, 2026 15:47

poroh approved these changes Aug 4, 2026

View reviewed changes

yoks approved these changes Aug 4, 2026

View reviewed changes

jayzhudev approved these changes Aug 4, 2026

View reviewed changes

chet merged commit 436d106 into NVIDIA:main Aug 4, 2026
9 checks passed

This was referenced Aug 10, 2026

feat: discover host NICs through nv-redfish adapter ports NVIDIA/infra-controller#4534

Merged

fix(site-explorer): discover host NICs in partial Redfish inventory NVIDIA/infra-controller#4968

Merged

Uh oh!

Conversation

chet commented Aug 1, 2026

What changed

Testing

Related issues

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chet Aug 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yoks Aug 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chet commented Aug 4, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jayzhudev left a comment

Choose a reason for hiding this comment

Uh oh!

jayzhudev left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

chet Aug 3, 2026 •

edited

Loading

yoks Aug 1, 2026 •

edited

Loading