feat(api): add IPv6 deny prefixes#3882
Conversation
|
Warning Review limit reached
Next review available in: 3 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (6)
Walkthrough
ChangesDual-stack prefix handling
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Config
participant API
participant DPUHandler
participant Agent
Config->>API: Load IPv4 and IPv6 deny_prefixes
API->>API: Validate address-family-aware overlaps
API->>DPUHandler: Provide validated network configuration
DPUHandler->>Agent: Return filtered prefixes by virtualization type
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai full_review, thanks! |
|
🐇 ✅ Action performedFull review finished. |
![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
🤖 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/api-core/src/cfg/file.rs`:
- Around line 179-183: Update the deny_prefixes documentation to state that FNN
accepts IPv4 and IPv6 prefixes, while all non-FNN virtualizers receive IPv4
prefixes only; apply this wording in crates/api-core/src/cfg/file.rs lines
179-183 and crates/api-core/src/cfg/README.md line 29.
🪄 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: e5ba5f38-0b01-42ad-b57a-604058f06c36
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
crates/api-core/src/cfg/README.mdcrates/api-core/src/cfg/file.rscrates/api-core/src/ethernet_virtualization.rscrates/api-core/src/handlers/dpu.rscrates/api/Cargo.tomlcrates/api/src/run.rs
`deny_prefixes` stopped at `Ipv4Network` even though `FNN` already renders its ACL policy by address family. This moves the site config and runtime value to `IpNetwork`, so the existing `FNN` IPv6 path can receive configured prefixes. - Filter every prefix that reaches `deny_prefixes` or `deprecated_deny_prefixes` at the per-DPU boundary. `FNN` receives both families, while `ETV` and `Flat` keep IPv4-only values, including `site_fabric_prefixes` folded into mutual isolation. - Reject overlaps with `site_fabric_prefixes` in either containment direction while treating IPv4 and IPv6 as separate address spaces. - Keep `anycast_site_prefixes` IPv4-only. Tests added! This supports NVIDIA#2405 Signed-off-by: Chet Nichols III <chetn@nvidia.com>
|
@coderabbitai full_review, thanks! |
|
🐇🔍 ✅ Action performedFull review finished. Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 3 minutes. |
|
🌿 Preview your docs: https://nvidia-preview-pull-request-3882.docs.buildwithfern.com/infra-controller |
1 participant
deny_prefixesstopped atIpv4Networkeven thoughFNNalready renders its ACL policy by address family. This moves the site config and runtime value toIpNetwork, so the existingFNNIPv6 path can receive configured prefixes.deny_prefixesordeprecated_deny_prefixesat the per-DPU boundary.FNNreceives both families, whileETVandFlatkeep IPv4-only values, includingsite_fabric_prefixesfolded into mutual isolation.site_fabric_prefixesin either containment direction while treating IPv4 and IPv6 as separate address spaces.anycast_site_prefixesIPv4-only.Tests added!
Related issues
This supports #2405
Type of Change
Breaking Changes
Testing
Unit tests added/updated
Integration tests added/updated
Manual testing performed
No testing required (docs, internal refactor, etc.)
cargo test -p carbide-api deny_site_fabric_overlap_is_address_family_aware -- --nocapturecargo test -p carbide-api-core handlers::dpu::deny_prefix_tests --lib -- --nocapturecargo test -p carbide-api-core deny_prefixes_accept_both_address_families --libcargo test -p carbide-api-core anycast_site_prefixes_reject_ipv6 --libcargo make format-nightlycargo make clippycargo make carbide-lintscargo make check-licensescargo make check-banscargo xtask check-workspace-depsAdditional Notes
site_fabric_prefixesremains dual-stack in its dedicated wire field. Only prefixes folded intodeprecated_deny_prefixesare filtered for the IPv4-onlyETVandFlatcompatibility path.