(11) config by daniel-noland · Pull Request #1763 · githedgehog/dataplane

daniel-noland · 2026-08-26T03:01:28Z

todo:

The last commit needs cleanup. And a few lines of comments need trimmed.

I will push trivial fixes once (08)-(10) land. Till then this is ready for review

coderabbitai · 2026-08-26T03:01:55Z

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Essentials

Run ID: 52bc8676-5e7a-4d8b-9fad-b96f29a32922

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

_{Comment @coderabbitai help to get the list of available commands.}

codecov · 2026-08-26T03:25:23Z

Codecov Report

❌ Patch coverage is 93.20175% with 62 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
config/src/external/overlay/algebra.rs	94.01%	16 Missing and 11 partials ⚠️
dataplane/src/packet_processor/fuzz.rs	93.09%	15 Missing and 12 partials ⚠️
hardware/src/nic/mod.rs	0.00%	7 Missing ⚠️
config/src/external/overlay/completeness.rs	97.87%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!

daniel-noland · 2026-09-21T01:18:07Z

+    /// `gwgroup` names the gateway group that owns the peering, which is what decides
+    /// whether a given gateway renders it.


daniel-noland · 2026-09-21T01:20:00Z

+                // Clearing the matching `*_any_ports` is what makes the narrowing narrow.
+                // Those ranges mean "any address within the peering, restricted to these
+                // ports" and `AclRule::validate_patterns_coverage` materializes them back
+                // into `src`/`dst`. `PermitByProtocol` inherits `EVERY_PORT` on both sides
+                // from `rule`, so leaving them set unions the whole manifest back over the
+                // excepted prefix and the denial matches every TCP packet across the
+                // peering rather than the one prefix it names. `PermitExcept` carries empty
+                // vectors already, so this only bites the protocol-narrowed guard -- and it
+                // was invisible because every derived load is UDP.


remove prose

daniel-noland · 2026-09-21T01:20:05Z

+                    // A `-except` denial that names a prefix must not also carry
+                    // `*_any_ports` on that side. Those mean "any address within the
+                    // peering, restricted to these ports" and `validate_patterns_coverage`
+                    // materializes them back into `src`/`dst`, so keeping them unions the
+                    // whole manifest over the prefix and the rule denies its entire protocol
+                    // across the peering rather than the exception it was built to express.


remove prose

mvachhar

Wow, this is pretty thorough coverage. I thought 09 was thorough, then we get 10 and then this.

A missing driver symlink means a NIC is unbound, which is valid when its kernel module was never loaded or it was already detached. Treating that as an error made BindToVfioPci refuse the device before its override-and-bind path. Continue binding unclaimed NICs, recognize e1000 and e1000e, and link the matching DPDK PMD for QEMU and bare-metal hosts. Signed-off-by: Daniel Noland <daniel@githedgehog.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Conversation replied from the address a sender dialed instead of the address that arrived after NAT. Those differ when an expose both translates and accepts connections, making the far side send from an address it does not own and causing a misleading flow-filter drop. Build replies from the delivered packet so the existing oracle genuinely checks reverse translation. Signed-off-by: Daniel Noland <daniel@githedgehog.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Static NAT rewrites a destination before masquerade, so the packet's current key differs from the initial key where FlowLookup filed its forward flow. The intra-burst fallback searched only the rewritten key and allocated a new public tuple for every packet. Keep the current-key lookup first for replies, then retry the original key on a miss and cover the combined translation. Signed-off-by: Daniel Noland <daniel@githedgehog.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The operation algebra could not produce static NAT, leaving two schema degrees unreachable and making the config-apply StaticNat step store only empty tables. Add the missing expose flavour using the algebra's disjoint private and public address blocks, and update the completeness census. Mixed static NAT and masquerade configurations now reach their shared pipeline path. Signed-off-by: Daniel Noland <daniel@githedgehog.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Inbound loads used a peer's public translation range as the sender address and answered to the dialed address rather than the request's delivered source. Both values happened to agree before translated inbound exposes were generated; afterward the flow filter correctly rejected them as unowned. Separate dial destinations from peer source addresses and reply to the observed sender. Signed-off-by: Daniel Noland <daniel@githedgehog.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Port forwarding was the last missing translated expose flavour and the only source of generated port ranges, leaving several configuration fields untested. Add equal-width but distinct public and private ranges, forbid conflicting directional translations on the opposite side, and require evidence that inbound loads exercise the new flavour. Leave protocol and timeout variation to separate changes. Signed-off-by: Daniel Noland <daniel@githedgehog.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Every generated peering permitted all traffic, so pipeline properties never exercised a configuration intended to deny a packet. Add coarse ACL shapes whose verdicts remain predictable without copying the production evaluator. Update traffic derivation to omit denied loads and retain guards proving ACL decisions are actually reached. Signed-off-by: Daniel Noland <daniel@githedgehog.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The completeness survey stopped at whether a peering had an ACL, hiding fixed rule, pattern, and scope fields behind a single present value. Extend the census through the entire ACL schema. This reports eight pre-existing fixed degrees of freedom without claiming the algebra gained new reach. Signed-off-by: Daniel Noland <daniel@githedgehog.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Packet-level ACL rules never exercise reverse-flow permission, where the verdict depends on NAT state. Generate permit-by-flow peerings and verify their replies through AclFilter's reverse lookup. The first run exposes and pins a separate defect: port-forwarded flows retain generation one and lose permission after any configuration change. Signed-off-by: Daniel Noland <daniel@githedgehog.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

With at most one rule per direction, generated ACLs could not make precedence affect a verdict. Add two predictable overlapping rules that partition a peering around one masquerade expose; reversing them now breaks the denial property. Track the named expose position and assign traffic variation before filtering so independent derivations remain comparable. Signed-off-by: Daniel Noland <daniel@githedgehog.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Gateway-group names, ACL logging, and idle timeouts remained fixed solely because the algebra never selected them. Vary all three and update the census. Record the current limits: disjoint exposes do not exercise gateway grouping semantics, and generated timeouts outlast these properties, so the fields lower successfully without testing expiration. Signed-off-by: Daniel Noland <daniel@githedgehog.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Generated ACLs still left protocol, port, and destination matching fixed, so dropping a field or lowering it as a wildcard could pass. Add predictable narrow rules whose target prefixes appear in exactly one traffic role, avoiding a duplicate ACL evaluator. Include a protocol guard that must not fire so both omission and wildcarding become observable. Signed-off-by: Daniel Noland <daniel@githedgehog.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Every generated port-forwarding expose accepted any protocol, leaving its protocol key and collision behavior untested. Cycle protocols across expose slots so one manifest can contain several. Because derived traffic is UDP, exclude TCP-only exposes from offered loads and make Draft::carries account for both expose protocol and peering ACL. Signed-off-by: Daniel Noland <daniel@githedgehog.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Generated exposes had no exclusions, so matchers, LPM tables, and RangeBuilder saw only contiguous prefixes. Remove a middle slice rather than one half, forcing the effective set into two ranges. This change measures reach and shape only; traffic derivation still selects from the effective set and therefore does not yet test the excluded hole. Signed-off-by: Daniel Noland <daniel@githedgehog.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The algebra could not express default-route exposes. Default routes overlap every neighboring route, so their validity depends on more than the operation's own arguments. Restrict them to otherwise empty peerings and let Op::reads inspect the draft, keeping applicability local and restoring the independence property. Skip loads whose expose protocol cannot carry their traffic. Signed-off-by: Daniel Noland <daniel@githedgehog.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Existing exclusion coverage drew traffic only from the effective address set, so a matcher that ignored every exclusion still passed. Aim packets directly at excluded addresses and require them to be unreachable. Exempt a manifest with a default expose because its default route legitimately covers the hole, and mark the census row as behaviorally enforced. Signed-off-by: Daniel Noland <daniel@githedgehog.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…t names The `-except` denial inherited `src_any_ports`/`dst_any_ports` = `[1..65535]` from the `rule` closure and only narrowed `pattern.src`/`dst`. Those ranges mean "any address within the peering, restricted to these ports", and `AclRule::validate_patterns_coverage` materializes them back into `src`/`dst`, so the narrowing was undone: the rule denied *all TCP across the peering* rather than the one prefix it was built to except. `PermitExcept` carries empty vectors already, so only the protocol-narrowed guard was affected -- and it was invisible because every derived load is UDP, so no property ever sent traffic the broken denial could match. It also contradicted the `AclPattern.src`/`dst` rows in the completeness census, which record the exception as address-scoped. Clear the matching `*_any_ports` when narrowing, and assert it over generated configurations: a `-except` denial that names a prefix on a side must not also carry any-ports on that side. Break-tested by restoring the old two lines -- `every_sequence_builds_a_valid_configuration` fails with the prefix and the ranges that undo it. Signed-off-by: Daniel Noland <daniel@githedgehog.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Signed-off-by: Daniel Noland <daniel@githedgehog.com>

The census asserts what the algebra reaches, and `Acl.rules` records that it reaches one-, two- and three-rule ACLs. A one-rule ACL turns up in roughly one draw in a hundred, so 512 draws miss it every few hundred runs. That is rare enough never to reproduce locally -- 60 consecutive runs here found it every time -- and common enough to fail CI, which it did, reporting only ["2", "3"] observed in 512 draws. Raise the draw count to 2048. This is a floor on the rarest value any `Reach::Spans` entry names rather than a runtime budget: the survey draws 2048 cases in about 0.8s at the rate the CI runner manages, well inside the budget `just` exports, and `enough_draws` still refuses to assert coverage on a sample cut short. The alternative was to record `Acl.rules` as reaching only two and three, which would have dropped a value the algebra genuinely produces -- the opposite of what this census is for. Validation: the three census tests pass, drawing 2048 cases with no skip. Signed-off-by: Daniel Noland <daniel@githedgehog.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

daniel-noland force-pushed the pr/daniel-noland/model-checking branch from 8e14aad to 68de4de Compare August 26, 2026 17:30

daniel-noland force-pushed the pr/daniel-noland/config-algebra branch from 47b6176 to 9ef4a02 Compare August 26, 2026 17:30

daniel-noland force-pushed the pr/daniel-noland/model-checking branch from 68de4de to bcbd57a Compare August 26, 2026 19:36

daniel-noland force-pushed the pr/daniel-noland/config-algebra branch 2 times, most recently from 120d507 to f65f958 Compare August 26, 2026 20:41

daniel-noland force-pushed the pr/daniel-noland/model-checking branch 2 times, most recently from 8cd913c to be74286 Compare August 26, 2026 21:02

daniel-noland force-pushed the pr/daniel-noland/config-algebra branch 2 times, most recently from 25deec3 to a75a2b7 Compare August 26, 2026 21:13

daniel-noland force-pushed the pr/daniel-noland/model-checking branch from be74286 to 95dd658 Compare August 26, 2026 21:13

daniel-noland force-pushed the pr/daniel-noland/config-algebra branch from a75a2b7 to 70fd2e6 Compare August 26, 2026 21:25

daniel-noland force-pushed the pr/daniel-noland/model-checking branch 2 times, most recently from 6446c90 to e58b3d3 Compare August 27, 2026 01:29

daniel-noland force-pushed the pr/daniel-noland/config-algebra branch from 70fd2e6 to 2c7edca Compare August 27, 2026 01:29

daniel-noland force-pushed the pr/daniel-noland/model-checking branch from e58b3d3 to bc6dcbb Compare August 27, 2026 01:41

daniel-noland force-pushed the pr/daniel-noland/config-algebra branch from 2c7edca to 806e9de Compare August 27, 2026 01:41

daniel-noland force-pushed the pr/daniel-noland/model-checking branch from bc6dcbb to 829e3f8 Compare August 27, 2026 02:25

daniel-noland force-pushed the pr/daniel-noland/config-algebra branch from 806e9de to ee7a16a Compare August 27, 2026 02:25

daniel-noland force-pushed the pr/daniel-noland/model-checking branch from 829e3f8 to 40659a3 Compare August 27, 2026 04:38

daniel-noland force-pushed the pr/daniel-noland/config-algebra branch from ee7a16a to bf31000 Compare August 27, 2026 04:38

daniel-noland force-pushed the pr/daniel-noland/model-checking branch from 40659a3 to 3a81a5b Compare August 27, 2026 05:12

daniel-noland force-pushed the pr/daniel-noland/config-algebra branch 2 times, most recently from 9049415 to 9e27aed Compare August 27, 2026 06:30

daniel-noland force-pushed the pr/daniel-noland/model-checking branch 2 times, most recently from 93cb81d to 7295f75 Compare August 27, 2026 18:00

daniel-noland force-pushed the pr/daniel-noland/config-algebra branch from 9e27aed to b4be140 Compare August 27, 2026 18:00

daniel-noland force-pushed the pr/daniel-noland/model-checking branch from 7295f75 to bdaa96a Compare August 27, 2026 18:29

daniel-noland force-pushed the pr/daniel-noland/config-algebra branch from b4be140 to 64c7005 Compare August 27, 2026 18:29

daniel-noland force-pushed the pr/daniel-noland/config-algebra branch from 27c49c1 to 20ff6f8 Compare September 3, 2026 00:21

daniel-noland changed the title ~~config~~ (11) config Sep 3, 2026

daniel-noland force-pushed the pr/daniel-noland/model-checking branch from e39bb32 to d36abaf Compare September 3, 2026 05:53

daniel-noland force-pushed the pr/daniel-noland/config-algebra branch from 20ff6f8 to a564e33 Compare September 3, 2026 05:53

daniel-noland force-pushed the pr/daniel-noland/model-checking branch from d36abaf to 4f1249a Compare September 3, 2026 05:59

daniel-noland force-pushed the pr/daniel-noland/config-algebra branch from a564e33 to 444884b Compare September 3, 2026 05:59

daniel-noland force-pushed the pr/daniel-noland/model-checking branch from 4f1249a to 9e024e1 Compare September 3, 2026 06:22

daniel-noland commented Sep 21, 2026

View reviewed changes

mvachhar approved these changes Sep 21, 2026

View reviewed changes

daniel-noland and others added 19 commits September 21, 2026 22:48

test(dataplane): use the shared fleet accessor in race regressions

ec2337c

Signed-off-by: Daniel Noland <daniel@githedgehog.com>

Navigation Menu

Repository navigation

(11) config - #1763

(11) config#1763
daniel-noland merged 19 commits into
pr/daniel-noland/model-checkingfrom
pr/daniel-noland/config-algebra

daniel-noland commented Aug 26, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Aug 26, 2026 •

edited

Loading

Review skipped

Uh oh!

codecov Bot commented Aug 26, 2026 •

edited

Loading

Uh oh!

daniel-noland Sep 21, 2026

Uh oh!

daniel-noland Sep 21, 2026

Uh oh!

daniel-noland Sep 21, 2026

Uh oh!

mvachhar left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		/// `gwgroup` names the gateway group that owns the peering, which is what decides
		/// whether a given gateway renders it.

Conversation

daniel-noland commented Aug 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented Aug 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Uh oh!

codecov Bot commented Aug 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

daniel-noland Sep 21, 2026

Choose a reason for hiding this comment

Uh oh!

daniel-noland Sep 21, 2026

Choose a reason for hiding this comment

Uh oh!

daniel-noland Sep 21, 2026

Choose a reason for hiding this comment

Uh oh!

mvachhar left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

daniel-noland commented Aug 26, 2026 •

edited

Loading

coderabbitai Bot commented Aug 26, 2026 •

edited

Loading

codecov Bot commented Aug 26, 2026 •

edited

Loading