feat(api): add IPv6 deny prefixes#3882
Open
chet wants to merge 1 commit into
Open
Conversation
Contributor
|
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 (6)
🚧 Files skipped from review as they are similar to previous changes (5)
Summary by CodeRabbit
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 |
Contributor
Author
|
@coderabbitai full_review, thanks! |
Contributor
|
🐇 ✅ Action performedFull review 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/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>
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
1 participant
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.
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.