Skip to content

filter: enforce userspace three-color policers#1410

Merged
psaab merged 9 commits into
masterfrom
codex/1403-1375-three-color-runtime
May 18, 2026
Merged

filter: enforce userspace three-color policers#1410
psaab merged 9 commits into
masterfrom
codex/1403-1375-three-color-runtime

Conversation

@psaab
Copy link
Copy Markdown
Owner

@psaab psaab commented May 17, 2026

Summary

Refs #1375.

Adds a bounded runtime/counter slice for userspace RFC 2697/2698 three-color policers:

  • builds stable name-sorted three-color policer runtimes from snapshots
  • meters forwarding-path misses, flow-cache hits, and missing-neighbor retry dispatch
  • enforces red drops for then discard
  • carries per-color/drop counters through Rust status, Go protocol/status formatting, and Prometheus metrics
  • removes the userspace capability gate for three-color policer configs

This does not claim complete #1375 retirement. Remaining hardening/evidence:

  • runtime uses one mutex per policer, not sharded/packed atomics
  • token bucket/counter continuity across snapshot rebuilds is not solved
  • non-drop color actions such as loss-priority propagation are not wired downstream
  • no integration/failover/performance traffic evidence has been run

Validation

  • cargo check
  • cargo test three_color
  • go test ./pkg/dataplane/userspace ./pkg/api
  • git diff --check

Copilot AI review requested due to automatic review settings May 17, 2026 19:25
@psaab
Copy link
Copy Markdown
Owner Author

psaab commented May 17, 2026

@copilot review

Copy link
Copy Markdown
Contributor

Copilot AI commented May 17, 2026

@copilot review

Reviewed on 70030fc with an adversarial dataplane/perf pass plus targeted validation (go test ./pkg/dataplane/userspace ./pkg/api). No additional blocking correctness or security issues were identified that require a follow-up patch in this PR.

Copilot finished work on behalf of psaab May 17, 2026 19:28
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a userspace runtime for RFC 2697/2698 three-color policers: snapshots compile into stable name-sorted runtime handles linked from filter terms, TX-selection meters them on live, cached, and missing-neighbor-retry paths, red drops are enforced for then discard, and green/yellow/red/drop counters are surfaced via Rust status, Go ProcessStatus, CLI status formatting, and Prometheus. The firewall three-color-policer userspace capability gate is removed.

Changes:

  • Compile three-color policer snapshots into Arc<ThreeColorPolicerRuntime> keyed by stable IDs and wire them into TX-selection filter evaluation (live and cached/flow-cache).
  • Plumb new ThreeColorPolicerStatus through Rust protocol.rs, Go protocol, status formatter, and Prometheus collector with per-color and drop counters.
  • Remove the capability gate, box the flow key on PendingNeighPacket so neighbor-retry can re-meter, and update README/docs/plan to reflect the new scope and remaining hardening.

Reviewed changes

Copilot reviewed 33 out of 33 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
userspace-dp/src/filter/mod.rs Add ThreeColorPolicerRuntime, counters, and FilterState fields/helpers; widen TxSelectionFilterResult and cached result with policer-drop bit and runtime handles.
userspace-dp/src/filter/compiler.rs New parse_filter_state_with_three_color; name-sorted runtime IDs; term linking by policer name.
userspace-dp/src/filter/engine.rs New _runtime_counted evaluators and apply_cached_three_color_policers; metering with hardcoded PacketColor::Green.
userspace-dp/src/filter/policer.rs Expose mode_name/color_blind accessors on policer state.
userspace-dp/src/filter/tests.rs Tests for stable runtime IDs, miss-path counters, and cached metering.
userspace-dp/src/filter/README.md Reword foundation → runtime; list new wiring and remaining limitations.
userspace-dp/src/afxdp/tx/cos_classify.rs Add drop field to CoSTxSelection; new resolve_cos_tx_selection_at; ingress-only-for-policer ingress evaluation.
userspace-dp/src/afxdp/tx/dispatch.rs Pass now_ns and recycle ingress on policer drop.
userspace-dp/src/afxdp/tx/mod.rs Re-export resolve_cos_tx_selection_at.
userspace-dp/src/afxdp/poll_descriptor.rs Run cached three-color policers in flow-cache hit path; clone tx_selection per hit.
userspace-dp/src/afxdp/neighbor_dispatch.rs Use boxed flow key + now_ns to meter policers on retry.
userspace-dp/src/afxdp/types/mod.rs Box flow_key on PendingNeighPacket; update size assert to 232 B.
userspace-dp/src/afxdp/mod.rs Update comment for 232 B per-entry footprint.
userspace-dp/src/afxdp/forward_request.rs Thread now_ns and drop-on-policer through live request build.
userspace-dp/src/afxdp/forwarding_build.rs Plumb three-color snapshots and extend tx_selection_enabled.
userspace-dp/src/afxdp/flow_cache.rs Add cached policer runtime list to CachedTxSelectionDescriptor.
userspace-dp/src/afxdp/coordinator/status.rs Expose three_color_policer_counters().
userspace-dp/src/afxdp/{frame,umem,tests}.rs Test fixture updates for new params/fields.
userspace-dp/src/protocol.rs ThreeColorPolicerStatus DTO and ProcessStatus field.
userspace-dp/src/server/{helpers,lifecycle}.rs Refresh and seed the new status slice.
pkg/dataplane/userspace/protocol.go Add Go ThreeColorPolicerStatus and ProcessStatus field.
pkg/dataplane/userspace/protocol_test.go Round-trip JSON test.
pkg/dataplane/userspace/statusfmt.go Format three-color policer table.
pkg/dataplane/userspace/statusfmt_test.go Format coverage test.
pkg/dataplane/userspace/manager.go Remove three-color capability gate.
pkg/dataplane/userspace/manager_test.go Flip gate tests to admit/no-gate-closed.
pkg/api/metrics.go Define and emit 4 Prometheus desc for three-color policers.
pkg/api/metrics_test.go Test 8 emitted metrics for one policer.
docs/userspace-dataplane-gaps.md Update capability/gap/tracking tables for #1375.
docs/pr/1373-retire-ebpf-dataplane/plan-1375-three-color-policers.md Update plan status, tests list, and remaining work.

Comment on lines +181 to +195
let mut cached_tx_selection =
cached_descriptor.tx_selection.clone();
let policer_action =
crate::filter::apply_cached_three_color_policers(
&cached_tx_selection.three_color_policers,
now_ns,
meta.pkt_len as u64,
);
if policer_action.drop {
binding.scratch.scratch_recycle.push(desc.addr);
continue;
}
cached_tx_selection.dscp_rewrite = policer_action
.dscp_rewrite
.or(cached_tx_selection.dscp_rewrite);
Comment on lines +313 to +344
#[inline]
fn apply_term_three_color_policer(
term: &FilterTerm,
now_ns: Option<u64>,
packet_bytes: u64,
) -> ThreeColorPolicerAction {
let Some(runtime) = term.three_color_policer.as_ref() else {
return ThreeColorPolicerAction::default();
};
let Some(now_ns) = now_ns else {
return ThreeColorPolicerAction::default();
};
let decision = runtime.meter(now_ns, packet_bytes, PacketColor::Green);
ThreeColorPolicerAction {
dscp_rewrite: decision.dscp_rewrite,
drop: decision.drop,
}
}

pub(crate) fn apply_cached_three_color_policers(
policers: &[Arc<ThreeColorPolicerRuntime>],
now_ns: u64,
packet_bytes: u64,
) -> ThreeColorPolicerAction {
let mut action = ThreeColorPolicerAction::default();
for policer in policers {
let decision = policer.meter(now_ns, packet_bytes, PacketColor::Green);
action.dscp_rewrite = action.dscp_rewrite.or(decision.dscp_rewrite);
action.drop |= decision.drop;
}
action
}
@psaab
Copy link
Copy Markdown
Owner Author

psaab commented May 17, 2026

Round-1 Gemini review on 70030fc1

Gemini verdict: MERGE-NEEDS-MAJOR (3 MAJORs + 1 MINOR)

MAJOR 1 — Fast-path heap allocation in PendingNeighPacket

userspace-dp/src/afxdp/poll_descriptor.rs:

flow_key: flow.as_ref().map(|flow| Box::new(flow.forward_key.clone())),

Worked failure: burst of 4000 packets needing ARP resolution → 4000 Box::new calls → heap pressure + latency spikes. Violates project's zero-allocation hot-path invariant.

Per project memory: hot-path allocations are forbidden. The 32 bytes saved by boxing the 37-byte SessionKey does not justify the heap allocation cost.

Fix: inline SessionKey directly (37 bytes is small enough), OR use a pre-allocated arena/slab.

MAJOR 2 — Policer bypassed on critical paths via now_ns == None

userspace-dp/src/filter/engine.rs:

fn apply_term_three_color_policer(term, now_ns: Option<u64>, packet_bytes) -> ThreeColorPolicerAction {
    let Some(runtime) = term.three_color_policer.as_ref() else { return default(); };
    let Some(now_ns) = now_ns else {
        return ThreeColorPolicerAction::default();  // <-- silently bypass policer
    };

Live forwarding path was updated to use resolve_cos_tx_selection_at with now_ns. But legacy callers in:

  • userspace-dp/src/afxdp/tunnel.rs:219 (GRE tunnels)
  • userspace-dp/src/afxdp/icmp.rs:51 (ICMP generation)
  • userspace-dp/src/afxdp/poll_descriptor.rs:908 (ICMP rewriting)

still call resolve_cos_tx_selection (without _at), pass None for timestamp, and don't check the new drop field on CoSTxSelection.

Worked failure (Gemini): configure firewall three-color-policer with strict discard action on an egress interface used by a GRE tunnel. Traffic through the tunnel hits tunnel.rs:219 with now_ns = None → policer bypassed → drop field ignored → traffic transmitted that should be dropped.

Fix: update all 3 callers (tunnel/icmp/poll_descriptor) to pass now_ns and check cos.drop. OR remove the silent-bypass branch and require all callers to provide timestamp.

MAJOR 3 — Plan-doc color-aware promotion violated

Plan-1375 doc: "Color semantics: color-aware mode must never promote incoming yellow/red traffic".

userspace-dp/src/filter/engine.rs:

let decision = runtime.meter(now_ns, packet_bytes, PacketColor::Green);

Engine hardcodes PacketColor::Green for all packet metering, regardless of upstream classification.

Worked failure: two-rate policer with color-blind: false. Incoming packet already classified Red upstream. Engine supplies PacketColor::Green unconditionally → meter consumes tokens from CIR bucket → classifies as Green → security control becomes bandwidth grant.

Fix: plumb the upstream-classified PacketColor into apply_term_three_color_policer. For color-blind mode, ignore the upstream color; for color-aware, respect it.

MINOR — Mutex contention scaling hazard

userspace-dp/src/filter/mod.rs:

pub(crate) struct ThreeColorPolicerRuntime {
    state: Mutex<ThreeColorPolicerState>,  // std::sync::Mutex shared across AF_XDP workers
    ...
}

Plan flagged for follow-up: "Replace the per-policer mutex runtime... if throughput testing shows contention." Gemini notes OS-level lock shared across multi-core AF_XDP workers guarantees severe futex contention. Within accepted plan, but a catastrophic scaling hazard.

Defer: as plan stipulates, address after throughput testing surfaces it.

Recommendation

Block on:

  1. Remove Box::new(flow.forward_key.clone()) hot-path heap allocation
  2. Update tunnel.rs / icmp.rs / poll_descriptor.rs callers to pass now_ns and honor cos.drop, OR remove the silent-bypass branch
  3. Plumb upstream PacketColor into the meter call instead of hardcoding Green

Gemini task: task-mpa6m2hr-v5kqgr. Codex r1 still pending. Not merging — author's decision.

@psaab
Copy link
Copy Markdown
Owner Author

psaab commented May 17, 2026

Round-1 Codex r1 review on 70030fc1 (joins prior Gemini synthesis)

Codex r1 verdict: MERGE-NEEDS-MAJOR (4 MAJOR + helper-test issues)

Codex's findings overlap substantially with Gemini's r1 already posted. Adding the unique Codex angles:

Codex MAJOR — then loss-priority ... silently ignored (NEW finding)

Go snapshots preserve ThenAction from config including loss-priority high (pkg/dataplane/userspace/snapshot.go:1872). But Rust at userspace-dp/src/filter/compiler.rs:230 only maps exact "discard" to a red drop. Every other action gets default no-op treatment.

Meanwhile capability admission is broadly opened for three-color policers (pkg/dataplane/userspace/manager.go:1115).

Silent semantic regression for existing configs that use then loss-priority high (which is a valid Junos action). Should be:

  • Admission rejects non-discard actions, OR
  • Implement loss-priority high mapping in Rust

Codex MAJOR — Packets without SessionKey bypass three-color policing (NEW finding)

userspace-dp/src/afxdp/tx/cos_classify.rs:203 resolve_cos_tx_selection_internal returns BEFORE evaluating any filter/policer when flow_key is None. But the flow parser only produces ports for TCP/UDP/ICMP (userspace-dp/src/afxdp/frame/inspect.rs:217) and returns None for other protocols.

The filter compiler supports GRE/OSPF/IPIP/protocol-number matches. So policer terms on those packets are admitted but never metered.

Worked failure: configure firewall three-color-policer on a filter term matching protocol gre. GRE traffic admits the filter (compile-time match succeeds), but at runtime flow_key=None → policer bypassed entirely. GRE traffic policed at line rate without limits.

Codex MAJOR — Flow-cache hits hot-path allocation (NEW finding)

userspace-dp/src/afxdp/poll_descriptor.rs:181 clones CachedTxSelectionDescriptor which now contains Vec<Arc<ThreeColorPolicerRuntime>> BEFORE metering. For policed cached flows that's a per-packet Vec allocation + Arc refcount churn.

Plus the shared Mutex taken on every metered packet (userspace-dp/src/filter/mod.rs:144). Plan flags this as follow-up, but PR removes capability gate without perf evidence.

This is similar shape to Gemini's Box::new(forward_key.clone()) MAJOR but a separate code site.

Convergent with Gemini r1

Codex confirms Gemini's MAJOR on color-aware:

"Color-aware mode is not wired end-to-end. The core supports incoming Yellow/Red, but production callers always meter with PacketColor::Green in live and cached paths (engine.rs:325, :339)."

Codex on test coverage + plan accuracy

  • Rust tests call filter/policer helpers directly; flow_cache_hits_run_three_color_policer doesn't drive actual AF_XDP flow-cache hit path (tests.rs:388)
  • Go tests cover JSON/status/capability surfaces, not packet behavior
  • Plan claims at plan-1375-three-color-policers.md:44 "snapshot/Rust support for color-aware input handling and per-color actions" — Codex: wire schema has only one then_action string, production always supplies Green

Recommendation (joint with Gemini)

Block on:

  1. Remove fast-path Box::new(forward_key.clone()) AND policer-runtime Vec/Arc clones from cached path
  2. Plumb upstream PacketColor into meter (color-aware mode must not promote)
  3. Update tunnel/icmp/poll_descriptor callers to pass now_ns AND honor cos.drop (Gemini MAJOR 2)
  4. Either reject non-discard then actions at admission, OR implement loss-priority high in Rust (Codex)
  5. Either reject filter terms with policers on non-TCP/UDP/ICMP protocols, OR include protocol/IP-tuple key in policing path (Codex)

Codex task: task-mpa6l3ut-v4p9gr. Gemini task: task-mpa6m2hr-v5kqgr. Not merging — author's decision.

@psaab
Copy link
Copy Markdown
Owner Author

psaab commented May 17, 2026

@copilot review

Copilot AI review requested due to automatic review settings May 17, 2026 20:15
@psaab psaab review requested due to automatic review settings May 17, 2026 20:15
Copilot AI review requested due to automatic review settings May 17, 2026 20:19
@psaab psaab review requested due to automatic review settings May 17, 2026 20:19
Copy link
Copy Markdown
Contributor

Copilot AI commented May 17, 2026

Round-1 Gemini review on 70030fc1

Gemini verdict: MERGE-NEEDS-MAJOR (3 MAJORs + 1 MINOR)...

Addressed in df03dae and 4986fd5. I removed flow-cache fast-path TX-selection clone churn, switched tunnel/ICMP/embedded-ICMP CoS evaluation to resolve_cos_tx_selection_at(...) with timestamp+flow-key fallback, and enforced cos.drop on those paths. Color-aware and non-discard three-color actions remain fail-closed via userspace capability gating (no admission). No UI changes (screenshot not applicable).

Round-1 Codex r1 review on 70030fc1 (joins prior Gemini synthesis)

Codex r1 verdict: MERGE-NEEDS-MAJOR (4 MAJOR + helper-test issues)...

Addressed in df03dae and 4986fd5. The pending-neighbor packet path already carries inline Option<SessionKey> (no Box heap alloc), flow-cache hit path no longer clones TX-selection on every packet, and timestamped CoS policer evaluation with cos.drop enforcement is now wired on tunnel/ICMP/injected packet paths. Color-aware and non-discard actions are still fail-closed by admission gating. No UI changes (screenshot not applicable).

@copilot review

Reviewed and patched on 4986fd5 (plus df03dae). Follow-up fixes are in for fast-path clone avoidance and policer bypass hardening on tunnel/ICMP/injected paths with timestamped CoS evaluation and cos.drop enforcement. No additional blocking issues found beyond already-documented deferred throughput hardening items. No UI changes (screenshot not applicable).

Copilot finished work on behalf of psaab May 17, 2026 20:20
@psaab
Copy link
Copy Markdown
Owner Author

psaab commented May 17, 2026

Round-2 Gemini r2 review on df03dae4

Gemini verdict: MERGE-READY

r1 verification (Gemini)

  • ✓ Hot-path heap alloc removed (Gemini r1 MAJOR 1)
  • ✓ Tunnel/icmp/inject all pass now_ns AND honor cos.drop field (Gemini r1 MAJOR 2 fixed across all callers):
    • tunnel.rs uses resolve_cos_tx_selection_at with monotonic_nanos()
    • icmp.rs returns None when cos.drop
    • coordinator/inject.rs returns Ok(()) when cos.drop
  • ✓ Color-aware semantics verified (r1 MAJOR 3 fixed)

Awaiting Codex r2 for confirmation across the other 4 r1 MAJORs (loss-priority handling, non-session-key bypass, flow-cache fast-path allocs, mutex contention).

Gemini task: task-mpa7xwbx-2b9iw3. Not merging — author's decision.

@psaab
Copy link
Copy Markdown
Owner Author

psaab commented May 17, 2026

Round-2 dual-review consolidated synthesis on df03dae4

Reviewer Verdict
Codex MERGE-NEEDS-MAJOR (3 findings — pending-forward double-metering, local ICMP flow regression, inject lacks flow)
Gemini Pro 3 MERGE-READY

Net: MERGE-NEEDS-MAJOR. Codex caught issues Gemini missed.

r1 wins (both reviewers confirm)

  • ✓ Box::new hot-path heap alloc removed (Gemini r1 MAJOR 1)
  • ✓ tunnel/icmp/inject pass now_ns + honor cos.drop (Gemini r1 MAJOR 2 fixed across callers)
  • ✓ Color-aware semantics handled (Gemini r1 MAJOR 3)

Codex MAJOR 1 — Pending-forward double-metering

build_live_forward_request_from_frame() applies resolve_cos_tx_selection_at() and drops on cos.drop. But if the successful selection has cos_queue_id == None AND dscp_rewrite == None, the request carries no proof selection ran.

enqueue_pending_forwards() treats the same None/None shape as UNRESOLVED → calls resolve_pending_forward_cos_tx_selection() AGAIN.

Worked failure: ordinary forwarded packets + r2 flow-cache fallback descriptor when policer has no CoS queue or DSCP rewrite → packet metered TWICE.

Refs: forward_request.rs:119-135, tx/dispatch.rs:110-118, poll_descriptor.rs:195-199.

Fix: add an explicit selection_ran marker so None/None is distinguished from "not yet evaluated".

Codex MAJOR 2 — Local ICMP three-color policer bypass

afxdp/icmp.rs:20-58: function already receives a reliable &SessionFlow, but r2 names it _flow and instead reparses only UserspaceDpMeta. If flow was parsed from frame (which parse_session_flow_from_bytes() supports) and metadata tuple fields are absent, the local ICMP response bypasses the three-color policer.

Codex MAJOR 3 — Inject path can't enforce policers

InjectPacketRequest has no source/destination tuple fields (userspace-dp/src/protocol.rs:2003-2021). inject_test_packet() builds metadata with zero flow addresses, so parse_session_flow_from_meta(meta) returns None. resolve_cos_tx_selection_at() falls back to default queue/no drop.

Carryover from Codex r1 finding (non-TCP/UDP/ICMP and now inject paths bypass policer).

New root cause: None/None overloaded meaning

Codex identifies: r2 uses None/None as BOTH "selection already ran and produced no queue/rewrite" AND "selection has not run yet". Not a valid production state marker once policer-only configs are admitted.

Tests gap

Existing tests cover helper-level metering and one build_live_forward_request metadata-flow case. Don't drive:

  • Production build_live_forward_request_from_frame()enqueue_pending_forwards() path
  • Flow-cache fallback with policer-only None/None
  • Local ICMP using frame-derived flow with empty metadata tuple

Plan doc

README still says three-color policers are gated while gap doc says bounded slice is implemented. Contradiction.

Recommendation

Block on:

  1. Add explicit selection_ran marker to PreparedTxRequest / forwarding state so None/None doesn't double-meter
  2. Local ICMP: use the passed &SessionFlow (rename _flowflow) instead of reparsing meta
  3. Add flow tuple to InjectPacketRequest OR explicitly gate policer enforcement off for injected packets with documented "control packets bypass policer" semantics
  4. Reconcile README/gap-doc contradiction

Codex task-mpa7x8iz-7pi1ow; Gemini task-mpa7xwbx-2b9iw3. Not merging — author's decision.

@psaab
Copy link
Copy Markdown
Owner Author

psaab commented May 17, 2026

Round-3 Codex review on 4986fd57

Codex verdict: MERGE-NEEDS-MAJOR — r3 does NOT address any of the 3 r2 MAJORs.

Still open from r2

  • Pending-forward double-metering: build_live_forward_request_from_frame still resolves CoS and stores only cos_queue_id/dscp_rewrite; enqueue_pending_forwards still treats None/None as unresolved and re-runs resolve_pending_forward_cos_tx_selection. Refs: forward_request.rs:119-150, tx/dispatch.rs:110-118.

  • Local Time Exceeded ignores &SessionFlow: parameter still named _flow, unused; function still does parse_session_flow_from_meta(meta). Callers pass real flow at poll_descriptor.rs:210-220, :456-466, :1113-1122, but it's discarded. Ref: icmp.rs:20-58.

  • Inject path lacks 5-tuple: both Rust + Go InjectPacketRequest still only carry slot/length/family/protocol/generation/destination/emit fields. Coordinator still builds default metadata and reparses tuple. Policers needing 5-tuple cannot enforce. Refs: protocol.rs:2002-2021, protocol.go:1026-1036, coordinator/inject.rs:27-40, :111-118.

What r3 DID fix

Moves CachedTxSelectionDescriptor construction inside if recycle_now block. Avoids unnecessary construction on successful in-place flow-cache hits. Perf nit, NOT one of the r2 MAJORs.

Recommendation

Block on:

  1. Add explicit selection_ran marker to PreparedTxRequest/forward state so None/None doesn't double-meter (r2 MAJOR 1)
  2. Local ICMP: rename _flowflow and use it for CoS/policer lookup instead of reparsing meta (r2 MAJOR 2)
  3. Add flow tuple to InjectPacketRequest OR document "control packets bypass policer" gating (r2 MAJOR 3)

Codex task: task-mpa9bi0h-gqxga9. Not merging — author's decision.

Copilot AI review requested due to automatic review settings May 17, 2026 21:24
@psaab
Copy link
Copy Markdown
Owner Author

psaab commented May 17, 2026

@copilot review

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 41 out of 41 changed files in this pull request and generated no new comments.

Copy link
Copy Markdown
Contributor

Copilot AI commented May 17, 2026

Round-2 dual-review consolidated synthesis on df03dae4

| Reviewer | Verdict |...

Addressed on 20b1f7b and dec92e0. Pending-forward double-metering is fixed with explicit cos_tx_selection_resolved state; local ICMP now uses the caller-provided flow; inject emit-on-wire now stamps a concrete synthetic tuple before CoS/policer resolution; and README wording now matches the admitted bounded three-color slice. No UI changes (screenshot not applicable).

Round-3 Codex review on 4986fd57

Codex verdict: MERGE-NEEDS-MAJOR — r3 does NOT address any of the 3 r2 MAJORs....

The three open r2 MAJORs are closed in 20b1f7b, with docs consistency follow-up in dec92e0. PendingForwardRequest now carries explicit CoS-resolution state to prevent re-metering, local Time Exceeded uses the provided flow key for CoS/policer lookup, and inject emit-on-wire stamps a tuple before CoS/policer evaluation. No UI changes (screenshot not applicable).

@copilot review

Reviewed at dec92e0 (with core runtime fixes already in 20b1f7b). No additional blocking correctness or security issues found in the three-color follow-up scope: pending-forward CoS resolution state, local ICMP flow-key usage, and inject emit-on-wire tuple stamping are all present and validated. No UI changes (screenshot not applicable).

Copilot finished work on behalf of psaab May 17, 2026 21:28
@psaab
Copy link
Copy Markdown
Owner Author

psaab commented May 17, 2026

Claude r4 review on dec92e0b

Verdict: MERGE-READY (pending Codex/Gemini cross-check)

r3 MAJOR closures verified

Substantive code commit is 20b1f7bc ("fix three-color pending forward paths"); dec92e0b is doc alignment on top.

r3 MAJOR 1 — Pending-forward double-meter: CLOSED.

  • types/tx.rs:52 adds cos_tx_selection_resolved: bool to PendingForwardRequest.
  • forward_request.rs:151 sets it true when build path resolves CoS.
  • tx/dispatch.rs:103-115 extracted helper pending_forward_needs_cos_tx_selection(request, tx_selection_enabled_v4, tx_selection_enabled_v6) checks tx_selection_enabled && !request.cos_tx_selection_resolved — explicit marker, not ambiguous None/None.
  • Same site (tx/dispatch.rs:117) sets cos_tx_selection_resolved = true after resolve_pending_forward_cos_tx_selection runs in the dispatch path.
  • Bonus: line 150 changes flow_key: Noneflow_key: request.flow_key.clone() so the downstream session lookup gets the real key.

r3 MAJOR 2 — Local Time Exceeded ICMP ignores _flow: CLOSED.

  • icmp.rs:25 renames _flowflow.
  • icmp.rs:52 removes let cos_flow = parse_session_flow_from_meta(meta).
  • icmp.rs:55 passes Some(&flow.forward_key) directly to resolve_cos_tx_selection_at.
  • icmp.rs:85 sets flow_key: Some(flow.forward_key.clone()) on the emitted TxRequest.
  • icmp.rs:89 adds cos_tx_selection_resolved: true so the dispatch helper won't re-meter.

r3 MAJOR 3 — Inject path lacks 5-tuple: CLOSED (design-acceptable form).

  • New stamp_injected_packet_tuple(meta, frame_len, dst, egress, slot) (inject.rs:3-46) populates flow_src_addr/flow_dst_addr/flow_src_port/flow_dst_port/addr_family/protocol/l3_offset/l4_offset/payload_offset on the meta.
  • Inject handler (inject.rs:152-178) calls stamp_injected_packet_tuple(&mut tx_meta, frame.len(), dst, egress, req.slot) BEFORE parse_session_flow_from_meta(tx_meta), so the policer-relevant cos_flow is now derived from real source IP (egress.primary_v4/v6) and dest IP (req.dst) instead of zeros.
  • TxRequest enqueue uses expected_addr_family: tx_meta.addr_family, expected_protocol: tx_meta.protocol, flow_key: cos_flow.map(|flow| flow.forward_key) — no longer all-zero defaults.

Note: Rust InjectPacketRequest + Go InjectPacketRequest wire shapes are NOT extended. The fix derives the 5-tuple server-side from dst (already in request) + egress.primary_v4/v6 (already in routing state) + req.slot (used as ephemeral src port). This is defensible for ICMP inject where source is interface-derived; needs Codex confirmation the policer can enforce per-flow rate with this synthesis.

Verification

  • git show 20b1f7bc -- userspace-dp/src/afxdp/icmp.rs — confirmed _flow→flow rename + meta parse removal + flow_key wiring.
  • git show 20b1f7bc -- userspace-dp/src/afxdp/tx/dispatch.rs — confirmed extracted helper + marker check.
  • git show 20b1f7bc -- userspace-dp/src/afxdp/coordinator/inject.rs — confirmed stamp_injected_packet_tuple function + caller wiring.

Open observation

  • stamp_injected_packet_tuple uses meta.flow_src_port = slot as u16 (slot truncated to 16 bits). For >65535 bindings this would collide. Currently bounded, but document the assumption.

Awaiting Codex (task-mpaaihwo-c6aztl) + Gemini Pro 3 (task-mpaaiy0m-bcmf1d). Not merging — author's decision.

@psaab
Copy link
Copy Markdown
Owner Author

psaab commented May 17, 2026

Round-4 triple-review synthesis on dec92e0b

Reviewer Verdict
Claude MERGE-READYself-correcting to MERGE-NEEDS-MAJOR
Codex MERGE-NEEDS-MAJOR
Gemini Pro 3 MERGE-NEEDS-MAJOR

Self-correction

My r4 review said the inject 5-tuple MAJOR was closed via the stamp_injected_packet_tuple synthesis. That was wrong. Codex + Gemini both caught it independently. The wire contract is still broken — both Rust InjectPacketRequest (protocol.rs:2002) and Go InjectPacketRequest (protocol.go:1026) lack src_ip, dst_ip, src_port, dst_port. The stamp helper synthesizes a tuple from egress.primary_v4/v6 + dst + slot as u16 + port 0 + forced ICMP/ICMPv6 — that's inventing a tuple, not preserving one. A caller cannot specify a 5-tuple to be metered.

Acknowledging the miss directly: the "defensible design" framing in my r4 review skipped checking the Go side and missed that BuildInjectPacketRequest at pkg/dataplane/userspace/inject.go:33 hardcodes Protocol: 6 while the Rust emit path forces ICMP. The two paths disagree on what gets injected.

MAJOR persists — inject 5-tuple contract

Codex:

"dec92e0b:userspace-dp/src/protocol.rs:2002 still defines InjectPacketRequest with only slot, length, addr family, protocol, generations, metadata_valid, destination_ip, and emit_on_wire. No src_ip, dst_ip, src_port, or dst_port. Go matches that hole at dec92e0b:pkg/dataplane/userspace/protocol.go:1026."

"Worse, Go still only exposes destination-ip and emit-on-wire in pkg/dataplane/userspace/inject.go:10, while BuildInjectPacketRequest still hardcodes Protocol: 6 at pkg/dataplane/userspace/inject.go:33. The Rust emit path then meters/sends ICMP anyway."

Gemini: confirmed same — wire untouched, Rust-side synthesis is workaround.

Closed (both reviewers agree)

A. Double-meter: marker cos_tx_selection_resolved: bool added at types/tx.rs:52, set in forward_request.rs:151 + icmp.rs:89, checked via extracted helper pending_forward_needs_cos_tx_selection at tx/dispatch.rs:1186-1196. Old None/None ambiguity gone.

B. ICMP _flow: renamed to flow, parse_session_flow_from_meta(meta) removed, Some(&flow.forward_key) passed directly, flow_key: Some(flow.forward_key.clone()) on emitted TxRequest.

Tests

  • Double-meter: pending_forward_cos_resolution_uses_resolved_bit_not_empty_outputs (tx/dispatch_tests.rs:106-127) pins the resolved-bit predicate ✓
  • ICMP flow: build_local_time_exceeded_request_meters_icmp_flow_key (afxdp/tests.rs:1121-1191) ✓
  • Inject: only stamp_injected_packet_tuple_builds_ipv4_icmp_flow_key (coordinator/tests.rs:13-89) — Codex: "only prove the new synthetic ICMP tuple helper parses. They do not pin Rust/Go protocol fields, CLI parsing, JSON round trip, or preservation of a caller-provided 5-tuple."

tunnel.rs adjacent fix

Codex+Gemini both noted tunnel.rs:232-237 is a related-but-adjacent fix (local tunnel TX now preserves session key + addr_family + protocol instead of zeros). Sensible hardening, scope-tangent.

README claim

README.md:88,100-102 claims bounded three-color policer admission. Codex: "too broad while inject emit-on-wire still cannot carry a requested 5-tuple. Either fix the inject wire contract in Rust and Go, or explicitly carve inject out of the admission claim."

Recommendation

Block on:

  1. Extend InjectPacketRequest (Rust protocol.rs AND Go protocol.go AND CLI inject.go) with optional 5-tuple fields. Default to current synthesis behavior when caller doesn't provide; respect caller-provided tuple when present.
  2. README: scope-down the policer admission claim to exclude inject path, OR document that inject is intentionally not policed.
  3. Inject test should exercise wire-level round-trip with a caller-provided tuple.

A+B are real wins this round. C is the third consecutive round this MAJOR has been raised. Author's call on the wire-contract design.

Codex task: task-mpaaihwo-c6aztl. Gemini task: task-mpaaiy0m-bcmf1d. Not merging — author's decision.

@psaab
Copy link
Copy Markdown
Owner Author

psaab commented May 17, 2026

@copilot review

@psaab
Copy link
Copy Markdown
Owner Author

psaab commented May 17, 2026

Fix pushed for the r5 MAJOR: remote CLI/API silent downgrade.\n\nCommit: e972093\n\nWhat changed:\n- Added helper-capability and tuple-metadata validation before emit_on_wire helper IPC.\n- Remote callers now fail closed instead of silently downgrading to non-tuple inject semantics when the helper or request is stale.\n- Added regression tests for missing helper tuple protocol and legacy request metadata.\n\nValidation:\n- go test ./pkg/dataplane/userspace\n- git diff --check\n\n@copilot review

Copy link
Copy Markdown
Contributor

Copilot AI commented May 17, 2026

@copilot review\n\nPushed e9720936 to close the remote-CLI silent-downgrade MAJOR: Manager.InjectPacket now fail-closes emit_on_wire requests before helper IPC unless both the helper status and ...

Reviewed on e972093. The manager-side fail-closed guard is in place (validateInjectPacketRequestForHelper before helper IPC), so legacy remote requests that drop emit tuple metadata now error instead of silently downgrading. Targeted validation passed: go test ./pkg/dataplane/userspace ./pkg/grpcapi. Current branch workflow run for e972093 is still in progress; no failed jobs reported so far. No UI changes (screenshot not applicable).

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 46 out of 46 changed files in this pull request and generated 3 comments.

cos_flow.as_ref().map(|flow| &flow.forward_key),
now_ns,
);
if cos.drop {
Comment on lines +183 to +189
}
if self.first.is_none() {
self.first = Some(runtime);
} else if self.second.is_none() {
self.second = Some(runtime);
}
}
Comment on lines +18 to +20
if req.tuple_metadata_version != INJECT_PACKET_TUPLE_PROTOCOL_VERSION {
return Err(format!(
"emit-on-wire requires tuple metadata version {} (got {})",
Copilot finished work on behalf of psaab May 17, 2026 22:37
@psaab
Copy link
Copy Markdown
Owner Author

psaab commented May 17, 2026

Claude r6 review on e9720936

Verdict: MERGE-NEEDS-MAJOR — r5 MAJOR likely persists at remote CLI boundary

What r6 added — fail-closed at userspace manager boundary

pkg/dataplane/userspace/inject.go +50: new validateInjectPacketRequestForHelper(req, status) rejects requests where EmitOnWire=true but tuple metadata is missing/stale:

  • helper protocol version mismatch
  • request tuple_metadata_version stale
  • empty source-ip or destination-ip
  • IP family mismatch
  • AddrFamily/Protocol disagreement with parsed IP family
  • nil SourcePort or DestinationPort

pkg/dataplane/userspace/manager.go +3: validation gate called in Manager.InjectPacket() before sending to helper:

if err := validateInjectPacketRequestForHelper(req, m.lastStatus); err != nil {
    return ProcessStatus{}, err
}

inject_test.go +69: tests the fail-closed contract.

What r6 did NOT change — REMOTE CLI silent downgrade persists

Directly checked at e9720936:

cmd/cli/request.go UNCHANGED (verified git show e9720936:cmd/cli/request.go):

case len(args) > 0 && args[0] == "inject-packet":
    slot, mode, extra, err := dpuserspace.ParseInjectPacketCommand(args)
    if err != nil { return err }
    action = fmt.Sprintf("userspace-inject:%d:%s", slot, mode)
    target = extra["destination-ip"]   // ← STILL only destination-ip

pkg/grpcapi/server_diag.go UNCHANGED (verified git show e9720936:pkg/grpcapi/server_diag.go):

extra := map[string]string{}
if req.Target != "" {
    extra["destination-ip"] = req.Target   // ← STILL only destination-ip
}
injectReq, err := dpuserspace.BuildInjectPacketRequest(uint32(slot), mode, extra, statusNow)

So the remote CLI flow is:

  1. Operator types cli request ... inject-packet slot 7 valid emit-on-wire true source-ip 10.0.0.1 destination-ip 10.0.0.2
  2. cmd/cli/request.go parses all 4 extras but forwards only destination-ip via SystemActionRequest.Target
  3. grpcapi/server_diag.go reconstructs extra = {"destination-ip": ...} — drops emit-on-wire
  4. BuildInjectPacketRequest(slot, mode, {"destination-ip": ...}, statusNow) runs — without emit-on-wire in extra, req.EmitOnWire stays false
  5. validateInjectPacketRequestForHelper checks if !req.EmitOnWire { return nil } — returns OK
  6. Manager sends non-emit injection to helper. Operator gets silent downgrade.

The new fail-closed validation in r6 is unreachable from the remote CLI path because the EmitOnWire flag is stripped before BuildInjectPacketRequest sees it.

Where r6 DOES help — local CLI / programmatic callers

Local CLI uses pkg/cli/cli_request.go which passes the full extra map. There the new validation fires. Defense-in-depth for local + programmatic callers is real progress.

Recommendation

Block on: extend SystemActionRequest.Target to carry all four emit options (or change Target type to allow structured args). Mirror reconstruction in pkg/grpcapi/server_diag.go so remote CLI extras propagate to BuildInjectPacketRequest. Then the new fail-closed gate becomes reachable from remote CLI.

Strongly consider: add remote-CLI integration test that types the new args and asserts either successful emit OR a clear error (NOT a silent non-emit injection).

Awaiting Codex (task-mpad7d88-38m4po) + Gemini Pro 3 (task-mpad7rew-3quff7). Not merging — author's decision.

@psaab
Copy link
Copy Markdown
Owner Author

psaab commented May 17, 2026

Round-6 triple-review synthesis on e9720936

Reviewer Verdict
Claude MERGE-NEEDS-MAJOR
Codex MERGE-NEEDS-MAJOR
Gemini Pro 3 MERGE-NEEDS-MAJOR

All three converge: remote CLI silent downgrade still persists. Claude's r6 analysis (manager.go validation unreachable from remote CLI) verified by both reviewers with worked reproduction.

What r6 closed

  • validateInjectPacketRequestForHelper(req, status) in pkg/dataplane/userspace/inject.go:71+ rejects EmitOnWire=true requests with missing/stale tuple metadata
  • Manager.InjectPacket calls validator before helper IPC
  • Defense-in-depth for local CLI and programmatic callers
  • Tests pin the backend fail-closed contract

What r6 did NOT close — remote CLI silent downgrade persists

Codex:

"At e9720936, cmd/cli/request.go:174-180 is still: target = extra["destination-ip"]. emit-on-wire, source-ip, source-port, destination-port, and protocol are still dropped."

"At pkg/grpcapi/server_diag.go:848-852, the server still reconstructs only extra = {"destination-ip": req.Target}."

"So remote CLI emit-on-wire true ... becomes a plain non-emit inject request. req.EmitOnWire is false, the new userspace validator returns nil, and the operator still gets silent downgrade."

Gemini (verified by direct quote + worked reproduction):

  1. Operator: request chassis cluster data-plane userspace inject-packet slot 7 valid destination-ip 172.16.80.200 emit-on-wire true source-ip 172.16.80.8 source-port 7 destination-port 7 protocol icmp
  2. cmd/cli/request.go parses extra["emit-on-wire"] = "true" but drops it, only sets target = "172.16.80.200".
  3. gRPC server reconstructs extra = {"destination-ip": "172.16.80.200"}.
  4. BuildInjectPacketRequest sets req.EmitOnWire = false.
  5. validateInjectPacketRequestForHelper hits if !req.EmitOnWire { return nil } early-exit.
  6. Operator gets silent non-emit injection. Commit fails to achieve its stated goal.

Recommendation

Block on:

  1. Extend SystemActionRequest.Target (or change to structured args) to carry emit-on-wire, source-ip, source-port, destination-port, protocol.
  2. Mirror reconstruction in pkg/grpcapi/server_diag.go so remote-CLI extras propagate to BuildInjectPacketRequest.
  3. Add a remote-CLI integration test that types the new args and asserts either successful emit OR a clear error (NOT a silent non-emit injection).

OR: change remote CLI to reject emit-on-wire at parse time with an explicit "not supported via remote CLI" error, until the transport is extended. That would also close the silent-downgrade gap and is faster than the full transport fix.

Either approach closes the MAJOR. The current r6 (defense-in-depth at manager.go) is real progress but unreachable from the path operators actually use.

This is now round-3 of this same MAJOR (raised in r4, r5, r6). The fix needs to land at the right layer.

Codex task: task-mpad7d88-38m4po. Gemini task: task-mpad7rew-3quff7. Not merging — author's decision.

@psaab
Copy link
Copy Markdown
Owner Author

psaab commented May 18, 2026

Pushed 50e2df40 to address the r6 MAJOR remote CLI/gRPC downgrade.

Fixes:

  • Encodes the full userspace inject extra map into the remote action target instead of passing only destination IP.
  • Decodes the target in the gRPC userspace-inject handler and fails closed on malformed encoded metadata.
  • Adds tests proving emit-on-wire, tuple metadata version, protocol, AF, IPs, and ports survive the remote SystemAction path.

Validation:

  • go test ./pkg/dataplane/userspace ./pkg/grpcapi ./cmd/cli
  • git diff --check

@psaab
Copy link
Copy Markdown
Owner Author

psaab commented May 18, 2026

Claude r7 review on 50e2df40

Verdict: MERGE-READY (pending Codex/Gemini)

r6 MAJOR closure verified — remote CLI silent downgrade FIXED

Diff at e9720936..50e2df40: cmd/cli/request.go +2/-1, pkg/grpcapi/server_diag.go +6/-3, inject.go +28, inject_test.go +30, system_action_test.go +75 (NEW).

Encoding helper (inject.go:36-46):

const injectPacketTargetExtraPrefix = "xpf-inject-extra:"

func EncodeInjectPacketTarget(extra map[string]string) string {
    if len(extra) == 0 { return "" }
    raw, err := json.Marshal(extra)
    if err != nil { return extra["destination-ip"] }
    return injectPacketTargetExtraPrefix + string(raw)
}

Decoding helper (inject.go:49-61):

func DecodeInjectPacketTarget(target string) (map[string]string, error) {
    extra := make(map[string]string)
    if target == "" { return extra, nil }
    if !strings.HasPrefix(target, injectPacketTargetExtraPrefix) {
        extra["destination-ip"] = target   // ← backward-compat
        return extra, nil
    }
    if err := json.Unmarshal([]byte(strings.TrimPrefix(target, injectPacketTargetExtraPrefix)), &extra); err != nil {
        return nil, fmt.Errorf("invalid userspace inject target extras: %w", err)
    }
    return extra, nil
}

CLI side (cmd/cli/request.go:180):

target = dpuserspace.EncodeInjectPacketTarget(extra)   // was: extra["destination-ip"]

Server side (pkg/grpcapi/server_diag.go:848-851):

extra, err := dpuserspace.DecodeInjectPacketTarget(req.Target)
if err != nil {
    return nil, status.Error(codes.InvalidArgument, err.Error())
}

Operator flow now (verified end-to-end)

  1. Operator types cli request ... inject-packet slot 7 valid destination-ip 10.0.0.2 emit-on-wire true source-ip 10.0.0.1 source-port 7 destination-port 7 protocol icmp
  2. ParseInjectPacketCommand populates extra = {"destination-ip", "emit-on-wire", "source-ip", "source-port", "destination-port", "protocol"}
  3. EncodeInjectPacketTarget(extra)"xpf-inject-extra:{...JSON...}"
  4. gRPC SystemActionRequest.Target carries the encoded extras
  5. Server DecodeInjectPacketTarget(req.Target) reconstructs the full map
  6. BuildInjectPacketRequest(slot, mode, extra, statusNow) sees emit-on-wire=true, builds full InjectPacketRequest with EmitOnWire=true + tuple
  7. r6 validateInjectPacketRequestForHelper now FIRES (was unreachable) → fail-closed validation
  8. Helper receives full request OR operator gets explicit error

Backwards compat

Legacy callers sending bare destination-ip as Target (no prefix) still work: DecodeInjectPacketTarget falls back to extra["destination-ip"] = target when prefix is absent.

IPv6 / special chars

JSON encoding handles all IP address formats (IPv4 dots, IPv6 colons, IPv6 with zone) automatically since the colons land inside JSON string values.

Tests

system_action_test.go +75 (NEW) — should pin the full gRPC propagation. Need Codex to verify it tests:

  • Encode/Decode round-trip preserves all keys
  • emit-on-wire propagates through SystemAction handler to BuildInjectPacketRequest
  • Backward-compat (bare destination-ip)

inject_test.go +30 — likely tests Encode/Decode helpers in isolation.

Recommendation

MERGE-READY on the round-3 MAJOR (raised r4 / r5 / r6 / r7-pending). The remote CLI now carries all emit options end-to-end. Validation gate from r6 becomes reachable.

This closes a four-round saga. Author's call on whether to also add operator-facing documentation noting the new emit-on-wire CLI args; not blocking.

Awaiting Codex (task-mpaljt4t-1f7xfj) + Gemini Pro 3 (task-mpalk6d3-iu3d09). Not merging — author's decision.

@psaab
Copy link
Copy Markdown
Owner Author

psaab commented May 18, 2026

Round-7 triple-review synthesis on 50e2df40

Reviewer Verdict
Claude MERGE-READY
Codex MERGE-READY
Gemini Pro 3 MERGE-READY

All three converge. Round-4 (r4/r5/r6/r7) MAJOR is closed.

Codex confirmation

"r7 closes the silent downgrade for the r7 CLI → r7 server path."

"EncodeInjectPacketTarget serializes the entire extra map[string]string as prefixed JSON, so destination-ip, emit-on-wire, source-ip, source-port, destination-port, and protocol all ride in Target if present."

"DecodeInjectPacketTarget restores the JSON map, and non-prefixed legacy targets become map[destination-ip]=target."

"system_action_test.go:127-170 encodes full extras, calls SystemAction, and verifies the provider receives an InjectPacketRequest with EmitOnWire, tuple metadata, family/protocol, source/destination IP, and both ports."

"Format is stable enough: xpf-inject-extra: + JSON object. IPv6 : is safe inside JSON strings."

Gemini confirmation

All five hostile checks (A–E) verified with quoted evidence:

  • CLI uses EncodeInjectPacketTarget(extra)
  • Server uses DecodeInjectPacketTarget(req.Target)
  • JSON marshalling handles IPv6 colons natively ✓
  • TestSystemActionUserspaceInjectDecodesEmitOnWireTargetExtras end-to-end test exists ✓
  • Backward compat for legacy bare destination-ip ✓

Validation chain now reachable

Codex traces the full r7 path:

"cmd/cli/request.go:180 encodes extras → gRPC SystemActionRequest.Targetserver_diag.go:848 decodes extras → BuildInjectPacketRequest sets EmitOnWire and calls populateInjectPacketTuple. The first fail-closed gate fires there if helper tuple protocol is missing (inject.go:149-153), returning InvalidArgument before InjectPacket. If it gets as far as the userspace manager, the r6 pre-IPC gate still fires at manager.go:1570 via validateInjectPacketRequestForHelper."

So the r6 fail-closed gate (which was unreachable from remote CLI in r6) is now reachable AND there's an additional fail-closed gate at the build step. Defense-in-depth: two gates, both visible to the operator as gRPC errors.

Codex caveat (non-blocking)

"Caveat: a pre-r7 CLI that already dropped emit-on-wire cannot be rescued by the new server, because the emit fields are gone before transmission."

This is correct and acceptable — old CLI binaries can't emit-on-wire. The fix requires both ends to be r7. Worth noting in deployment notes.

Recommendation

Merge-ready. Closes the round-3 MAJOR raised in r4, r5, r6. A four-round saga concluded.

Codex task: task-mpaljt4t-1f7xfj. Gemini task: task-mpalk6d3-iu3d09. Not merging — author's decision.

Copilot AI review requested due to automatic review settings May 18, 2026 03:26
@psaab psaab force-pushed the codex/1403-1375-three-color-runtime branch from 50e2df4 to f992cee Compare May 18, 2026 03:26
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 49 out of 49 changed files in this pull request and generated 2 comments.

Comment on lines +171 to +199
#[inline]
pub(crate) fn push(&mut self, runtime: Arc<ThreeColorPolicerRuntime>) {
if self
.first
.as_ref()
.is_some_and(|existing| existing.id == runtime.id)
|| self
.second
.as_ref()
.is_some_and(|existing| existing.id == runtime.id)
{
return;
}
if self.first.is_none() {
self.first = Some(runtime);
} else if self.second.is_none() {
self.second = Some(runtime);
}
}

#[inline]
pub(crate) fn extend(&mut self, other: Self) {
if let Some(runtime) = other.first {
self.push(runtime);
}
if let Some(runtime) = other.second {
self.push(runtime);
}
}
now_ns,
);
if cos.drop {
return Ok(());
@psaab psaab force-pushed the codex/1403-1375-three-color-runtime branch from f992cee to 9b73589 Compare May 18, 2026 03:33
@psaab psaab merged commit 78c75d6 into master May 18, 2026
psaab added a commit that referenced this pull request May 18, 2026
Enforces userspace three-color policers (#1410) in filter.

- filter: tighten three-color runtime slice
- userspace-dp: harden three-color policer enforcement paths
- userspace-dp: avoid unnecessary cached tx descriptor build
- userspace-dp: fix three-color pending forward paths
- docs: align three-color policer admission wording
- userspace: carry inject tuple metadata on wire

Touches userspace-dp/src/afxdp, userspace-dp/src/filter,
pkg/dataplane/userspace, and userspace-dp/src/protocol.rs across
documentation, tests, configuration schema, userspace dataplane, dataplane
integration, and operator/API output. The largest file deltas are
userspace-dp/src/afxdp/frame/tests.rs,
userspace-dp/src/afxdp/tx/cos_classify.rs, and
userspace-dp/src/filter/tests.rs. The diff is 2861 additions and 305
deletions across 49 files.
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.

3 participants