refactor(site-explorer): clarify BlueField part-number checks#2909
Conversation
Rename BlueField detection helpers and call sites to reflect that they operate on Redfish part numbers rather than model strings. Use the discovered DPU Card1 part number for fallback-serial mode heuristics, and keep explicit operator DPU mode handling independent of part-number availability. Tighten Lenovo BF3 DPU part-number matching and add coverage for Card1 part-number extraction. Signed-off-by: Dmitry Porokh <dporokh@nvidia.com>
Summary by CodeRabbit
WalkthroughThe PR replaces model-based BlueField detection with part-number-based checks across endpoint exploration, DPU mode selection, and downstream machine classification. It also adds a DPU part-number accessor and updates supporting tests. ChangesBlueField part-number refactor
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/site-explorer/src/lib.rs (1)
2947-2963: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftPreserve explicit DPU-mode requests separately from heuristics.
DpuMode::DpuModecurrently depends ondpu_part_numberto chooseNicMode::Dpu. If an operator explicitly requests DPU mode and the part number is missing, this returnsOk(true)without issuingset_nic_mode(Dpu). Keep the mode source explicit-vs-default so only the implicit default uses the BF3 part-number heuristic.🤖 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/site-explorer/src/lib.rs` around lines 2947 - 2963, The Nic mode selection in the target_nic_mode logic is mixing explicit operator intent with the BF3 part-number fallback, so an explicit DPU request can be dropped when dpu_part_number is missing. Update the logic around host_dpu_mode and target_nic_mode so explicit DpuMode::DpuMode requests resolve directly to NicMode::Dpu, while only the implicit/default path uses is_bf3_supernic_part_number and is_bf3_dpu_part_number heuristics. Keep the existing mode-setting flow in this branch so set_nic_mode still runs whenever the operator explicitly asked for DPU mode.
🤖 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/site-explorer/src/lib.rs`:
- Around line 1605-1608: The normal-mode check in check_and_configure_dpu_mode
is using the host-reported part_number instead of the DPU endpoint’s value,
which can skip the BF3 heuristic when the host field is missing. Update the call
site that builds mode_check to prefer dpu_ep’s reported Card1.part_number, and
only fall back to the host device part number if the DPU value is unavailable.
Keep the fix localized around the check_and_configure_dpu_mode invocation and
the related DPU endpoint parsing logic.
---
Outside diff comments:
In `@crates/site-explorer/src/lib.rs`:
- Around line 2947-2963: The Nic mode selection in the target_nic_mode logic is
mixing explicit operator intent with the BF3 part-number fallback, so an
explicit DPU request can be dropped when dpu_part_number is missing. Update the
logic around host_dpu_mode and target_nic_mode so explicit DpuMode::DpuMode
requests resolve directly to NicMode::Dpu, while only the implicit/default path
uses is_bf3_supernic_part_number and is_bf3_dpu_part_number heuristics. Keep the
existing mode-setting flow in this branch so set_nic_mode still runs whenever
the operator explicitly asked for DPU mode.
🪄 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: 29ee87eb-3b81-40da-98c9-686c559fb284
📒 Files selected for processing (4)
crates/api-core/src/ipxe.rscrates/api-model/src/machine/mod.rscrates/api-model/src/site_explorer/mod.rscrates/site-explorer/src/lib.rs
| let mode_check = Some( | ||
| self.check_and_configure_dpu_mode(dpu_ep, part_number, host_dpu_mode, metrics) | ||
| .await, | ||
| ), | ||
| None => None, | ||
| }; | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Use the DPU endpoint part number for normal mode checks.
Line 1606 still passes the host-reported device part number. If that PCIe/chassis field is absent while the discovered DPU BMC has Card1.part_number, the BF3 heuristic is skipped and the DPU can remain in the wrong mode. Prefer the DPU report value, falling back to the host device value only when needed.
Proposed fix
- let mode_check = Some(
- self.check_and_configure_dpu_mode(dpu_ep, part_number, host_dpu_mode, metrics)
- .await,
- );
+ let dpu_part_number = dpu_ep.report.dpu_part_number().or(part_number);
+ let mode_check = Some(
+ self.check_and_configure_dpu_mode(dpu_ep, dpu_part_number, host_dpu_mode, metrics)
+ .await,
+ );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let mode_check = Some( | |
| self.check_and_configure_dpu_mode(dpu_ep, part_number, host_dpu_mode, metrics) | |
| .await, | |
| ), | |
| None => None, | |
| }; | |
| ); | |
| let dpu_part_number = dpu_ep.report.dpu_part_number().or(part_number); | |
| let mode_check = Some( | |
| self.check_and_configure_dpu_mode(dpu_ep, dpu_part_number, host_dpu_mode, metrics) | |
| .await, | |
| ); |
🤖 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/site-explorer/src/lib.rs` around lines 1605 - 1608, The normal-mode
check in check_and_configure_dpu_mode is using the host-reported part_number
instead of the DPU endpoint’s value, which can skip the BF3 heuristic when the
host field is missing. Update the call site that builds mode_check to prefer
dpu_ep’s reported Card1.part_number, and only fall back to the host device part
number if the DPU value is unavailable. Keep the fix localized around the
check_and_configure_dpu_mode invocation and the related DPU endpoint parsing
logic.
🔍 Container Scan Summary
Per-CVE detail lives in the per-service |
| // FORGE-7330: If site-explorer can't get the interface info from the bmc, then it won't associate the interface with a machine. | ||
| // if the pxe request included the product and its a DPU, the machine record is not needed and we can just use the DPU type. | ||
| if let Some(product) = target.product | ||
| && product.to_ascii_lowercase().contains("bluefield") |
There was a problem hiding this comment.
is this something you care about?
There was a problem hiding this comment.
I'm not sure that I understand question. Goal of this PR is to just use right names for the function (function name that checks part numbers called is_..._model). I came across it when reviewed code for BF4 where I saw mismatch between what value I check and how function was named. To prevent this confusion in future I just decided that it is better to rename them.
2 participants
Rename BlueField detection helpers and call sites to reflect that they operate on Redfish part numbers rather than model strings.
Use the discovered DPU Card1 part number for fallback-serial mode heuristics, and keep explicit operator DPU mode handling independent of part-number availability. Tighten Lenovo BF3 DPU part-number matching and add coverage for Card1 part-number extraction.
Related issues
Type of Change
Breaking Changes
Testing
Additional Notes