Repository navigation
refactor(minvmd): move the drop limiter core into its own module - #2349
Conversation
dns_pins and egress_gate formed a non-test import cycle: the pins imported DropLimiter from egress_gate, and the gate's refusal lines reached back through dns_pins. Move the limiter's generic core -- the warn cadence constants, the DropKey and WarnDecision enums, and the DropLimiter struct with its window bookkeeping and the DNS refusal emitter -- into a new net::drop_limiter module, verbatim, raising visibility only as far as the net subtree needs. Both modules now import the limiter without importing each other; only cfg(test) test-support references remain between them. No behavior changes; every existing test passes unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Description check |  |
The description explains the motivation, scope, behavior impact, and verification results. It includes a detailed verification section, but it does not use the template's exact Testing heading and omi… |
| Docstring Coverage | |
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 3 files. (1 skipped: 1 … |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
| Title check | |
The title clearly and concisely describes the main change: moving the drop limiter core into its own module. |
Full details: Description check
-
Explanation
The description explains the motivation, scope, behavior impact, and verification results. It includes a detailed verification section, but it does not use the template's exact Testing heading and omits the Checklist section.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 3 files. (1 skipped: 1 too large.)
✨ Finishing Touches
-
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
A rabbit checks the warning clock,
One minute passes on the rock.
The names take turns; the overflow waits,
The keys stay bounded at their gates.
A fresh module keeps the code in view,
Then hops away beneath the moon.
Comment @coderabbitai help to get the list of available commands.
There was a problem hiding this comment.
A verbatim move: constants, DropKey, WarnDecision and DropLimiter with decide/should_warn_*/warn_dns_refusal land in net::drop_limiter unchanged, the gate keeps its own emit arms in an impl block of its own, and dns_pins/egress_gate no longer import each other outside cfg(test). The three pre-existing minvmd test failures are the known answerer-subscriber races. Approving.
Routing-Key: inbox-route/I_kwDOSUhdos8AAAABYJPq9w
The daemon's drop limiter lived in
net::egress_gatewhile the DNSpins imported it, and the gate's refusal paths reached back through
net::dns_pins— a non-test import cycle between the two modules.The limiter's core is shared policy machinery, not gate logic, so it
now lives in its own
net::drop_limitermodule: the warn-cadenceconstants, the
DropKeyandWarnDecisionenums, andDropLimiterwith its window bookkeeping and the DNS refusal emitter, moved
verbatim with only the visibility raises the
netsubtree needs.dns_pinsandegress_gateeach import the limiter now withoutimporting each other; the only remaining references between them are
cfg(test)test-support calls. No behavior changes.Verification
cargo fmt --all --check— cleancargo clippy --workspace --all-targets --locked -- -D warnings— cleancargo test --locked -p minvmd— 575 passed, 3 failedmain(verified by re-runningon the stashed clean tree: the same 2 answerer global-subscriber
races and 1 sock path assertion fail identically without this
change; none touch the modules moved here)
dns_pins/egress_gate/drop_limitertests passgrep egress_gate crates/minvmd/src/net/dns_pins.rs— onlycfg(test)test_support::capture_log()lines remaingrep 'dns_pins\|egress_gate' crates/minvmd/src/net/drop_limiter.rs— no imports of either module
Summary by CodeRabbit