feat(site-explorer): pair BF4 DPUs by chassis serial#2906
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. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (7)
Summary by CodeRabbit
WalkthroughBF4 pairing now uses a dedicated serial helper with Bluefield_BMC fallback, site-explorer matching and ingestion use that key, BF4 mocks and tests cover the path, and ChangesBF4 DPU pairing and site-explorer projection
BMC error source
Sequence Diagram(s)sequenceDiagram
participant identify_managed_hosts
participant can_ingest_dpu_endpoint
participant EndpointExplorationReport
identify_managed_hosts->>EndpointExplorationReport: dpu_pairing_serial_number()
identify_managed_hosts->>can_ingest_dpu_endpoint: evaluate DPU endpoint
can_ingest_dpu_endpoint->>EndpointExplorationReport: nic_mode()
can_ingest_dpu_endpoint->>EndpointExplorationReport: dpu_pairing_serial_number()
can_ingest_dpu_endpoint-->>identify_managed_hosts: continue when nic_mode is missing and pairing serial exists
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 OpenGrep (1.23.0)crates/api-model/src/site_explorer/mod.rs┌──────────────┐ �[32m✔�[39m �[1mOpengrep OSS�[0m [00.28][ERROR]: unable to find a config; path crates/bmc-explorer/src/error.rs┌──────────────┐ �[32m✔�[39m �[1mOpengrep OSS�[0m [00.19][ERROR]: unable to find a config; path crates/bmc-mock/src/hw/bluefield4.rs┌──────────────┐ �[32m✔�[39m �[1mOpengrep OSS�[0m [00.22][ERROR]: unable to find a config; path
Comment |
895767b to
79dcc0c
Compare
🔐 TruffleHog Secret Scan✅ No secrets or credentials found! Your code has been scanned for 700+ types of secrets and credentials. All clear! 🎉 🕐 Last updated: 2026-06-26 23:43:37 UTC | Commit: 79dcc0c |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/rpc/proto/site_explorer.proto (1)
191-199: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftAdd a BF4 wire enum instead of documenting it as
UNKNOWN.Line 193 still leaves BF4 without a dedicated
MlxDeviceKind, so the new BF4 path is forced to serialize asMLX_DEVICE_KIND_UNKNOWNdownstream. That loses the device-kind signal for RPC/CLI consumers and hard-codes the gap into the new BF4 regression coverage.Suggested proto change
enum MlxDeviceKind { MLX_DEVICE_KIND_UNKNOWN = 0; MLX_DEVICE_KIND_BF3_NIC_MODE = 1; MLX_DEVICE_KIND_BF3_DPU_MODE = 2; MLX_DEVICE_KIND_BF3_SUPER_NIC = 3; MLX_DEVICE_KIND_BF2_DPU = 4; + MLX_DEVICE_KIND_BF4_DPU = 5; }Append the new value to preserve wire compatibility, then update the Rust part-number mapping and tests to emit it. As per path instructions,
crates/rpc/proto/**: review core gRPC protobuf definitions for generated-client impact and wire compatibility.🤖 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/rpc/proto/site_explorer.proto` around lines 191 - 199, Add a dedicated BF4 entry to MlxDeviceKind in site_explorer.proto instead of routing BF4 through MLX_DEVICE_KIND_UNKNOWN. Preserve wire compatibility by appending the new enum value, then update the Rust part-number mapping logic that derives MlxDeviceKind from the device part number so BF4 serializes to the new value. Finally, adjust the existing BF4 regression tests and any RPC/CLI assertions that currently expect UNKNOWN so they validate the new BF4 wire enum.Source: Path instructions
🤖 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.
Outside diff comments:
In `@crates/rpc/proto/site_explorer.proto`:
- Around line 191-199: Add a dedicated BF4 entry to MlxDeviceKind in
site_explorer.proto instead of routing BF4 through MLX_DEVICE_KIND_UNKNOWN.
Preserve wire compatibility by appending the new enum value, then update the
Rust part-number mapping logic that derives MlxDeviceKind from the device part
number so BF4 serializes to the new value. Finally, adjust the existing BF4
regression tests and any RPC/CLI assertions that currently expect UNKNOWN so
they validate the new BF4 wire enum.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 29ba25b4-8269-4242-adbf-f60d7dcdac18
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
crates/api-model/src/site_explorer/mod.rscrates/bmc-explorer/src/error.rscrates/bmc-mock/src/test_support/mod.rscrates/rpc/proto/site_explorer.protocrates/site-explorer/Cargo.tomlcrates/site-explorer/src/lib.rscrates/site-explorer/tests/site_explorer.rs
79dcc0c to
55f7617
Compare
🔍 Container Scan Summary
Per-CVE detail lives in the per-service |
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-mock/src/hw/bluefield4.rs`:
- Around line 176-197: Add direct test coverage for Bluefield4::discovery_info()
instead of relying only on the Redfish pairing flow. Create a focused
table-driven test around discovery_info() that asserts the machine_arch,
dmi_data, and dpu_info values for BF4 stay correct, or point to an existing test
that already validates those fields. Use the Bluefield4 type and
discovery_info() method as the main anchors so the coverage is easy to find and
maintain.
🪄 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: ae65403f-17e3-402d-a848-4111bbea714f
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
crates/api-model/src/site_explorer/mod.rscrates/bmc-explorer/src/error.rscrates/bmc-mock/src/hw/bluefield4.rscrates/bmc-mock/src/test_support/mod.rscrates/rpc/proto/site_explorer.protocrates/site-explorer/Cargo.tomlcrates/site-explorer/src/lib.rscrates/site-explorer/tests/site_explorer.rs
✅ Files skipped from review due to trivial changes (1)
- crates/rpc/proto/site_explorer.proto
🚧 Files skipped from review as they are similar to previous changes (6)
- crates/site-explorer/Cargo.toml
- crates/bmc-mock/src/test_support/mod.rs
- crates/bmc-explorer/src/error.rs
- crates/api-model/src/site_explorer/mod.rs
- crates/site-explorer/tests/site_explorer.rs
- crates/site-explorer/src/lib.rs
| DiscoveryInfo { | ||
| machine_type: CpuArchitecture::Aarch64.to_string(), | ||
| machine_arch: Some(rpc::utils::cpu_architecture_to_rpc( | ||
| CpuArchitecture::Aarch64, | ||
| )), | ||
| dmi_data: Some(DmiData { | ||
| board_name: "BlueField-4 DPU".into(), | ||
| product_serial: self.product_serial_number.to_string(), | ||
| board_serial: carbide_utils::DEFAULT_DPU_DMI_BOARD_SERIAL_NUMBER.into(), | ||
| chassis_serial: carbide_utils::DEFAULT_DPU_DMI_CHASSIS_SERIAL_NUMBER.into(), | ||
| product_name: "BlueField-4 DPU".into(), | ||
| sys_vendor: "Nvidia".into(), | ||
| ..Default::default() | ||
| }), | ||
| dpu_info: Some(DpuData { | ||
| part_number: self.part_number().into(), | ||
| part_description: format!("NVIDIA BlueField-4 {}", self.part_number()), | ||
| factory_mac_address: self.host_mac_address.to_string(), | ||
| ..Default::default() | ||
| }), | ||
| ..Default::default() | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -e rs . crates | xargs rg -n "Bluefield4::discovery_info|discovery_info\(|DmiData|DpuData|900-9D4B4|BlueField-4 DPU"Repository: NVIDIA/infra-controller
Length of output: 13149
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== outline: crates/bmc-mock/src/hw/bluefield4.rs =="
ast-grep outline crates/bmc-mock/src/hw/bluefield4.rs --view expanded || true
echo
echo "== outline: crates/bmc-mock/src/machine_info.rs =="
ast-grep outline crates/bmc-mock/src/machine_info.rs --view expanded | sed -n '1,220p' || true
echo
echo "== search for Bluefield4 discovery tests =="
rg -n "bluefield4\\(|Bluefield4|discovery_info\\(" crates/bmc-mock/src crates/bmc-mock/tests crates/site-explorer/tests crates/host-support/src | sed -n '1,240p'Repository: NVIDIA/infra-controller
Length of output: 11305
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== bluefield4.rs relevant slice =="
sed -n '160,225p' crates/bmc-mock/src/hw/bluefield4.rs | cat -n
echo
echo "== machine_info.rs around Bluefield4 and tests =="
sed -n '130,230p' crates/bmc-mock/src/machine_info.rs | cat -n
echo
echo "== machine_info.rs test slice around BF4 usage =="
sed -n '460,540p' crates/bmc-mock/src/machine_info.rs | cat -n
echo
echo "== search for discovery_info assertions in bmc-mock tests/modules =="
rg -n "discovery_info\\(\\)|machine_arch|part_description|factory_mac_address|board_name|BlueField-4 DPU" crates/bmc-mock/src crates/bmc-mock/tests crates/site-explorer/tests | sed -n '1,220p'Repository: NVIDIA/infra-controller
Length of output: 15257
Add direct coverage for Bluefield4::discovery_info()
The BF4 regression currently exercises only the Redfish pairing path; add a focused table-driven test for this method, or point to existing coverage, so the machine_arch, dmi_data, and dpu_info fields do not drift silently.
🤖 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-mock/src/hw/bluefield4.rs` around lines 176 - 197, Add direct test
coverage for Bluefield4::discovery_info() instead of relying only on the Redfish
pairing flow. Create a focused table-driven test around discovery_info() that
asserts the machine_arch, dmi_data, and dpu_info values for BF4 stay correct, or
point to an existing test that already validates those fields. Use the
Bluefield4 type and discovery_info() method as the main anchors so the coverage
is easy to find and maintain.
Sources: Coding guidelines, Path instructions
55f7617 to
5e41a0f
Compare
|
|
||
| pub fn is_bf4_dpu_part_number(part_number: &str) -> bool { | ||
| let normalized_part_number = part_number.to_lowercase(); | ||
| normalized_part_number.starts_with("900-9d4b4") |
There was a problem hiding this comment.
Should be a const, no?
There was a problem hiding this comment.
It should but it follows pattern of surroundings. All other part numbers prefixes are defined in corresponding functions. So this refactoring waits its hero.
Classify BF4 devices by part number and use the Bluefield_BMC chassis serial as the DPU pairing key when system serial and Redfish mode are absent. Signed-off-by: Dmitry Porokh <dporokh@nvidia.com>
5e41a0f to
b8c3ac5
Compare
3 participants
Classify BF4 devices by part number and use the Bluefield_BMC chassis
serial as the DPU pairing key when system serial and Redfish mode are
absent.
Related issues
Type of Change
Breaking Changes
Testing
Additional Notes