fix: support SMC ARS-221GL-FNB-NC24B#3675
Merged
Merged
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
Contributor
Summary by CodeRabbit
WalkthroughThe change adds MGX-C2 chassis classification based on NVIDIA hardware identifiers and integrates the result into Supermicro exploration configuration and lockdown status evaluation using the in-band IPMI host-interface state. It also updates Supermicro BIOS attributes and the libredfish dependency. ChangesMGX-C2 Supermicro lockdown handling
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant nv_generate_exploration_report
participant ExploredChassisCollection
participant manager_Config
participant lockdown_status
nv_generate_exploration_report->>ExploredChassisCollection: call is_mgx_c2()
ExploredChassisCollection-->>nv_generate_exploration_report: return MGX-C2 classification
nv_generate_exploration_report->>manager_Config: set need_oem_supermicro_kcs_interface to !is_mgx_c2
nv_generate_exploration_report->>lockdown_status: pass explored chassis
lockdown_status-->>nv_generate_exploration_report: compute Supermicro lockdown status
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/bmc-explorer/src/lib.rs`:
- Around line 668-703: Add table-driven tests for the lockdown status logic
around the MGX-C2 and Supermicro model branches, covering
ipmi_host_interface_enabled values Some(true), Some(false), and None for both
ARS-121L-DNR and a normal Supermicro model. Assert the resulting
InternalLockdownStatus and externally reported message/state, including the
ARS-121L-DNR PXE host-interface exception.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: c73e4cbf-599d-4418-8539-ffdec1009a7e
📒 Files selected for processing (2)
crates/bmc-explorer/src/chassis.rscrates/bmc-explorer/src/lib.rs
Signed-off-by: Krish Dandiwala <kdandiwala@nvidia.com>
Signed-off-by: Krish Dandiwala <kdandiwala@nvidia.com>
poroh
approved these changes
Jul 20, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/bmc-explorer/src/lib.rs`:
- Around line 688-699: Update the MGX-C2 branch assigning inband_locked and
inband_unlocked so ipmi_host_interface_enabled == None is treated as
unknown/partial rather than satisfying the fully Enabled status path; preserve
the existing Some(true) and Some(false) behavior. Add table-driven coverage for
Some(true), Some(false), and None around the relevant status evaluation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: e8eed262-d302-4466-8f39-126002287bf2
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
Cargo.tomlcrates/bmc-explorer/src/chassis.rscrates/bmc-explorer/src/hw/supermicro.rscrates/bmc-explorer/src/lib.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/bmc-explorer/src/chassis.rs
Comment on lines
+688
to
+699
| let (inband_locked, inband_unlocked) = if explored_chassis.is_mgx_c2() { | ||
| ( | ||
| ipmi_host_interface_enabled.is_none_or(|enabled| !enabled), | ||
| ipmi_host_interface_enabled.is_none_or(identity), | ||
| ) | ||
| } else { | ||
| match (hi_enabled, kcs_privilege, is_syslockdown) { | ||
| (false, Some(SupermicroPrivilege::Callback), true) => { | ||
| Ok(InternalLockdownStatus::Enabled) | ||
| } | ||
| (true, Some(SupermicroPrivilege::Administrator), false) => { | ||
| Ok(InternalLockdownStatus::Disabled) | ||
| } | ||
| (true, None, false) => Ok(InternalLockdownStatus::Disabled), | ||
| _ => Ok(InternalLockdownStatus::Partial), | ||
| } | ||
| } | ||
| .map(|status| Some(LockdownStatus { status, message })) | ||
| ( | ||
| kcs_privilege == Some(SupermicroPrivilege::Callback), | ||
| kcs_privilege.is_none() | ||
| || kcs_privilege == Some(SupermicroPrivilege::Administrator), | ||
| ) | ||
| }; |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the file and inspect the relevant region.
echo "== crates/bmc-explorer/src/lib.rs outline =="
ast-grep outline crates/bmc-explorer/src/lib.rs --view expanded | sed -n '1,220p'
echo
echo "== relevant lines around 688-699 =="
sed -n '660,730p' crates/bmc-explorer/src/lib.rs | cat -n
echo
echo "== search for ipmi_host_interface_enabled usages =="
rg -n "ipmi_host_interface_enabled|inband_locked|inband_unlocked|InternalLockdownStatus::Enabled|host_interface_locked|is_syslockdown" crates/bmc-explorer/src/lib.rs crates -g '!target'
echo
echo "== tests mentioning MGX-C2 or lockdown status =="
rg -n "MGX-C2|mgx_c2|lockdown|host_interface|ipmi_host_interface_enabled" crates -g '*test*' -g '*tests*' -g '*.rs'
echo
echo "== if present, inspect nearby tests in the same file =="
rg -n "mod tests|#[[:space:]]*test" crates/bmc-explorer/src/lib.rsRepository: NVIDIA/infra-controller
Length of output: 50380
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== lockdown_status function context =="
sed -n '433,735p' crates/bmc-explorer/src/lib.rs | cat -n | sed -n '1,340p'
echo
echo "== tests in crates/bmc-explorer =="
fd . crates/bmc-explorer/tests crates/bmc-explorer/src -t f | sed -n '1,200p'
echo
echo "== search for table-driven coverage around lockdown status in bmc-explorer tests =="
rg -n "lockdown_status|InternalLockdownStatus|mgx_c2|MGX|ipmi_host_interface_enabled|SupermicroPrivilege|Callback|Administrator" crates/bmc-explorer/tests crates/bmc-explorer/src -g '*.rs' -A 4 -B 4
echo
echo "== look for none_or patterns in the same function or nearby branches =="
rg -n "is_none_or\\(|is_some_and\\(" crates/bmc-explorer/src/lib.rs -A 2 -B 2Repository: NVIDIA/infra-controller
Length of output: 47463
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== supermicro GB300 integration test =="
sed -n '1,220p' crates/bmc-explorer/tests/integration/supermicro_gb300_explore.rs | cat -n
echo
echo "== supermicro mock data references =="
rg -n "service_enabled|ipmi_host_interface|sys_lockdown|lockdown" crates/bmc-explorer/tests crates/bmc-mock crates -g '*.rs' -A 3 -B 3
echo
echo "== fixture builder for supermicro GB300 =="
sed -n '1,260p' crates/bmc-explorer/tests/integration/common.rs | cat -n
echo
echo "== any tests or comments about missing MGX-C2 host-interface fields =="
rg -n "MGX-C2|mgx_c2|when available|missing field|reference semantics|partial" crates/bmc-explorer crates -g '*.rs' -A 2 -B 2Repository: NVIDIA/infra-controller
Length of output: 50380
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Redfish serial console model for service_enabled =="
rg -n "service_enabled|ServiceEnabled" crates -g '*.rs' -A 3 -B 3 | sed -n '1,240p'
echo
echo "== Supermicro mock that sets IPMI service_enabled =="
sed -n '180,230p' crates/bmc-mock/src/hw/supermicro_gb300_nvl.rs | cat -n
echo
echo "== supermicro OEM manager state defaults =="
sed -n '1,120p' crates/bmc-mock/src/redfish/oem/supermicro/manager.rs | cat -n
echo
echo "== inspect the generated/typed host interface accessors if present =="
rg -n "interface_enabled\\(|service_enabled\\(" crates -g '*.rs' -A 3 -B 3 | sed -n '1,240p'Repository: NVIDIA/infra-controller
Length of output: 34636
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate the IPMI host interface type =="
rg -n "ipmi_host_interface|ServiceEnabled|service_enabled" crates/redfish crates/nv_redfish crates -g '*.rs' -A 4 -B 4 | sed -n '1,260p'
echo
echo "== inspect the relevant Redfish model definition if found =="
fd ".*(serial_console|host_interface|computer_system|manager).*\.rs$" crates/redfish crates/nv_redfish -t f | sed -n '1,120p'
echo
echo "== show the direct model around any ServiceEnabled field definition =="
rg -n "pub .*service_enabled|service_enabled:|ServiceEnabled" crates/redfish crates/nv_redfish -g '*.rs' -A 6 -B 6 | sed -n '1,220p'Repository: NVIDIA/infra-controller
Length of output: 12062
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== find the Redfish type that carries ServiceEnabled =="
rg -n "pub .*service_enabled|service_enabled: Option<bool>|service_enabled: bool|ServiceEnabled" crates/redfish/src -g '*.rs' -A 6 -B 6 | sed -n '1,220p'
echo
echo "== inspect likely model files around the matching definition(s) =="
fd ".*(serial_console|host_interface|ethernet_interface|session_service|telemetry_service).*\.rs$" crates/redfish/src -t f | sed -n '1,120p'Repository: NVIDIA/infra-controller
Length of output: 3739
Do not let missing MGX-C2 ServiceEnabled report Enabled crates/bmc-explorer/src/lib.rs:688-699
ipmi_host_interface_enabled is optional here, so None can still satisfy the is_syslockdown && inband_locked && host_interface_locked path and produce a false Enabled status. Treat the missing case as unknown/partial instead, and add table coverage for Some(true), Some(false), and None.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/bmc-explorer/src/lib.rs` around lines 688 - 699, Update the MGX-C2
branch assigning inband_locked and inband_unlocked so
ipmi_host_interface_enabled == None is treated as unknown/partial rather than
satisfying the fully Enabled status path; preserve the existing Some(true) and
Some(false) behavior. Add table-driven coverage for Some(true), Some(false), and
None around the relevant status evaluation.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR:
PG535/2G535processor module.IPMIHostInterfacewhen available and evaluate Supermicro lockdown consistently with libredfish.ARS-121L-DNRHostInterface PXE exception.v0.44.22Related issues
#3623
Type of Change
Breaking Changes
Testing
Additional Notes