Skip to content

release (optional): take shared tables once per burst - #1988

Draft
daniel-noland wants to merge 1 commit into
mainfrom
pr/daniel-noland/tables-once-per-burst
Draft

daniel-noland wants to merge 1 commit into
mainfrom
pr/daniel-noland/tables-once-per-burst

Conversation

@daniel-noland

@daniel-noland daniel-noland commented Oct 25, 2026 •

Copy link
Copy Markdown
Collaborator

The optional set for this release: stacked on #2004 (advised), which stacks on #2003 (required). Rebased onto #2004 on 2026-10-27; the rustfmt failure from the earlier CI run is fixed in the commit. The measurements below were taken on the env-5 bench branch, not on this stack.

Counting ls_locks.all on the env-5 gateway showed about 34.5 locked instructions per packet on a DPDK worker, at every flow count. Each is a full barrier on x86 (on the order of 20 cycles on Zen 5), and it keeps later loads from running ahead. Most of them came from stages taking shared, rarely-changing state once per packet instead of once per burst:

  • egress and ingress entered the interface table for every packet, and egress entered the adjacency table too. Entering a left-right reader costs an atomic RMW and a SeqCst fence on the way in, and another RMW on the way out.
  • the FIB memo cached a reader per burst but still entered it for every packet.
  • the ACL filter loaded its tables for every packet. The load hands out an owned Arc, which is an atomic increment and decrement.
  • the flow filter cloned the packet's flow (Arc<FlowInfo>) into its summary.

What changed:

  • egress, ingress and the ACL filter take their tables once per burst.
  • the FIB memo keeps each reader it caches entered, through a new routing::FibPin. Entering it again for each packet then takes left-right's nested, thread-local path.
  • the flow filter copies the flags, status and genid it needs instead of holding the flow.

A writer now waits for at most one burst before readers leave.

Measurement

Measured on env-5 gateway-bm: EPYC 9555P, 8 DPDK workers, masquerade, 64 B frames, Mpps for the whole gateway. The bench build carries other flow-table work, so main's absolute numbers will differ.

1k flows 100k 1M 2M cycles/pkt @1k locked ops/pkt
before 12.48 11.04 10.35 10.15 2828 34.5
after 14.65 12.57 11.63 11.43 2411 10.8

That is +12-17 %, with instructions per packet almost unchanged (−1.5 %). Removing about 24 locked ops per packet saved about 420 cycles.

Review notes

  • FibPin holds a left-right ReadGuard with its lifetime extended to 'static, next to the Rc<FibReader> it borrows from. The guard field is declared first, so it is dropped first, and the guard is never handed out. This is the only unsafe here; a self_cell-style crate would avoid it if preferred.
  • In this tree, routing::cli::handler::tests_cli_handling::test_cli_handling failed once in a full parallel run and passed alone. It also fails alone on main plus an unrelated change, so it looks timing-sensitive and unrelated to this PR.

🤖 Generated with Claude Code

@coderabbitai

coderabbitai Bot commented Oct 25, 2026

Copy link
Copy Markdown

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@codecov

codecov Bot commented Oct 25, 2026 •

Copy link
Copy Markdown

Codecov Report

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

Files with missing lines Patch % Lines
flow-filter/src/lib.rs 70.58% 4 Missing and 1 partial ⚠️
dataplane/src/packet_processor/egress.rs 75.00% 1 Missing and 3 partials ⚠️
dataplane/src/packet_processor/ingress.rs 50.00% 0 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

@daniel-noland
daniel-noland changed the base branch from main to pr/daniel-noland/release-advised October 27, 2026 01:24
@daniel-noland
daniel-noland force-pushed the pr/daniel-noland/tables-once-per-burst branch from dea1615 to eaf886a Compare October 27, 2026 01:24
@daniel-noland daniel-noland changed the title perf(dataplane,routing,acl-filter,flow-filter): take shared tables once per burst release (optional): take shared tables once per burst Oct 27, 2026
@daniel-noland

Copy link
Copy Markdown
Collaborator Author

Local verification of this head (eaf886a, on #2004):

  • just clippy (the CI derivation, with loom): pass
  • rustfmt: clean
  • cargo nextest for routing, acl-filter, flow-filter and dataplane: 439 passed, 4 skipped, 0 failed

@daniel-noland
daniel-noland force-pushed the pr/daniel-noland/release-advised branch from 543c3de to 674766b Compare October 27, 2026 01:40
@daniel-noland
daniel-noland force-pushed the pr/daniel-noland/tables-once-per-burst branch from eaf886a to c476cef Compare October 27, 2026 01:40
@daniel-noland
daniel-noland force-pushed the pr/daniel-noland/release-advised branch from 674766b to ae4bfde Compare October 27, 2026 01:41
@daniel-noland
daniel-noland force-pushed the pr/daniel-noland/tables-once-per-burst branch from c476cef to 78aea22 Compare October 27, 2026 01:41
@daniel-noland
daniel-noland force-pushed the pr/daniel-noland/release-advised branch 2 times, most recently from 3546d2c to 6c7ca8b Compare October 27, 2026 07:03
@daniel-noland
daniel-noland force-pushed the pr/daniel-noland/tables-once-per-burst branch from 78aea22 to 4ab9d49 Compare October 27, 2026 16:03
Base automatically changed from pr/daniel-noland/release-advised to main October 27, 2026 17:12
…ce per burst

A worker ran about 34 locked instructions per packet, each a full
barrier on x86. Many were stages taking shared, rarely changing state
once per packet rather than once per burst:
- egress and ingress entered the interface table, and egress the
  adjacency table, for every packet. Entering a left-right reader is
  an atomic RMW and a fence on the way in, and another RMW on the way
  out.
- the FIB memo cached a reader per burst but entered it per packet.
- the ACL filter loaded its tables, an `Arc` reference both ways, per
  packet.
- the flow filter cloned the packet's flow into its summary.

Egress, ingress and the ACL filter now take their tables once per
burst. The FIB memo holds each reader it caches entered (`FibPin`),
so entering it per packet takes left-right's nested, thread-local
path. The flow filter copies what it needs from the flow instead of
holding it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Daniel Noland <daniel@githedgehog.com>
@daniel-noland
daniel-noland force-pushed the pr/daniel-noland/tables-once-per-burst branch from 4ab9d49 to b8727d2 Compare October 27, 2026 17:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant