tx/dispatch: extract compute_forwarded_egress_ptb (#4408 increment 1)#4642
Merged
Conversation
Pure code-motion outlining of the first cohesive sub-block out of the 1,131-LOC enqueue_pending_forwards TX-drain orchestrator, the first bounded increment of the #4408 hot-path god-function refactor. The #2301/#2330/#2845 egress-MTU Path-MTU-Discovery block (93 interior lines) was already a self-contained scoped { ... } unit inside the non-copied-source-frame branch: it reads only immutable inputs (source_frame, request.meta, request.decision, forwarding, the is_nat64 / uses_native_tunnel flags, ingress_ident, recent_exceptions), mutates neither target_binding nor the tx pipeline, has no early return / continue / break, and its only outputs are the two locals ptb_reply and mtu_signalled. That makes it an ideal extract-function target. It moves verbatim into a new private #[inline] helper fn compute_forwarded_egress_ptb(...) -> (Option<Vec<u8>>, bool) returning (ptb_reply, mtu_signalled). The call site becomes a destructuring assignment to the same outer mut locals, so behaviour is identical: same inner-MTU derivation via the #2300/#2331 GRE/WG/NAT64 SSOT helpers, the same forwarded_egress_mtu_decision, the same RFC 792 / RFC 4443 + #2472 rate-limit suppression, the same PTB build, and the same egress_mtu_exceeded exception recording with identical arguments and ordering. The moved body is character-identical modulo the request.meta -> meta and &request.decision -> decision field-to-param substitution. Hot-path invariants are untouched: no new allocation, the non-NAT64 zero-copy path is unchanged, and the helper only runs on the already oversized-frame slow path. Bounded single increment; #4408 continues with further outlining (segmentation / mirror paths, the waterfill split). The detailed #2301/#2330 rationale comment moves onto the helper's doc comment; the call site keeps a short pointer. Validation: full userspace-dp cargo test 3830/0. cargo fmt was NOT run on the tree (it is not canonical under the locally installed rustfmt, which would rewrite ~1474 unrelated files); the added lines were hand-matched to the surrounding style. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015oARShYtiJJ2H4UB4nXGqi
Contributor
There was a problem hiding this comment.
Pull request overview
This PR implements #4408 increment 1 by extracting the forwarded egress-MTU / PMTUD (ICMP PTB / Frag Needed) sub-block out of the large enqueue_pending_forwards TX dispatcher into a dedicated private helper, keeping behavior and side-effect ordering intact.
Changes:
- Extracted the PMTUD decision + PTB construction + exception recording logic into
compute_forwarded_egress_ptb(...) -> (Option<Vec<u8>>, bool)and replaced the inline block with a tuple assignment at the call site. - Added internal documentation describing the ongoing outlining effort and the invariants being preserved.
- Logged the edit per repo process in
_Log.md.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| userspace-dp/src/afxdp/tx/dispatch/mod.rs | Moves the forwarded egress-MTU/PTB logic into compute_forwarded_egress_ptb and wires it back into enqueue_pending_forwards as pure code motion. |
| userspace-dp/src/afxdp/tx/README.md | Documents the #4408 outlining effort and describes increment 1’s extracted helper and invariants. |
| _Log.md | Records the refactor increment and touched files per the project’s action logging convention. |
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
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.
Addresses #4408 (increment 1: extract
compute_forwarded_egress_ptbfromenqueue_pending_forwards; further increments tracked).What
Pure code-motion outlining of the first cohesive sub-block out of the
1,131-LOC
enqueue_pending_forwardsTX-drain orchestrator inuserspace-dp/src/afxdp/tx/dispatch/mod.rs.The #2301/#2330/#2845 egress-MTU Path-MTU-Discovery block (93 interior
lines) was already a self-contained scoped
{ ... }unit inside the!copied_source_framebranch. It reads only immutable inputs(
source_frame,request.meta,request.decision,forwarding, theis_nat64/uses_native_tunnelflags,ingress_ident,recent_exceptions), mutates neithertarget_bindingnor the txpipeline, has no early
return/continue/break, and its onlyoutputs are the two locals
ptb_replyandmtu_signalled.It moves verbatim into a new private helper:
The call site becomes
(ptb_reply, mtu_signalled) = compute_forwarded_egress_ptb(...).Why it is pure code-motion (behaviour-identical)
forwarded_egress_mtu_decision.build_frag_needed_v4/build_packet_too_big_v6).egress_mtu_exceededexception recording — identical args and ordering.request.meta->metaand&request.decision->decisionfield-to-param substitution.Hot-path invariants untouched: no new allocation, the non-NAT64
zero-copy path is unchanged, and the helper only runs on the already
oversized-frame slow path. This increment deliberately does NOT touch
the single-recycle invariant or the CoS guarantee-guard.
Validation
userspace-dpcargo test: 3830 passed, 0 failed.cargo checkclean (no new warnings).cargo fmtwas intentionally NOT run on the tree: it is not canonicalunder the locally installed rustfmt (it would rewrite ~1474 unrelated
files). The added lines were hand-matched to the surrounding style.
Scope
Bounded single increment. #4408 continues with further outlining
(cold segmentation / mirror paths, the
select_exact_cos_guarantee_queue_waterfillsplit). Docs updated:
userspace-dp/src/afxdp/tx/README.md(new"Ongoing refactor" section) +
_Log.md.🤖 Generated with Claude Code
https://claude.ai/code/session_015oARShYtiJJ2H4UB4nXGqi