Emit non-PBR userspace filter log events

[psaab]

Summary

• Carry compiled filter/term/action identity through Rust filter evaluation,
  live TX selection, and cached TX descriptors.
• Emit userspace RT_FLOW filter-log events for non-PBR input filters,
  live output filters, cached output-filter hits, and lo0/local-delivery
  filters while skipping routing-instance terms in the non-PBR helper.
• Preserve policy deny RT_FLOW shape while adding stable compiled policy IDs
  from the userspace snapshot.
• Add deterministic Go syslog fanout coverage for raw userspace policy-deny,
  screen-drop, and filter-log event-stream frames.
• Update Retire the eBPF dataplane; userspace AF_XDP dataplane becomes the default and only path #1373/Feature gap: dataplane events (PolicyDeny, ScreenDrop, FilterLog) not emitted by userspace-dp #1379 docs and module READMEs with implemented call sites and
  remaining evidence scope.

Validation

• cargo test --manifest-path userspace-dp/Cargo.toml
• go test -count=1 ./pkg/dataplane/userspace ./pkg/logging
• git diff --check

One earlier full Rust run hit the existing timing-sensitive
afxdp::umem::tests::tx_latency_hist_cross_thread_snapshot_skew_within_bound
flap; rerunning the same full suite passed.

Remaining #1379 Evidence Gap

Live userspace-cluster syslog evidence is still needed if #1373 Phase 4 wants
operator artifacts beyond the deterministic local UDP syslog harness. That
should include deny policy, screen drop, PBR filter log, non-PBR input/output
filter log, lo0/local-delivery filter log, and deny-storm starvation/loss
counter checks.

[Copilot]

Pull request overview

Adds non-PBR userspace filter-log emission and stable compiled policy IDs to the userspace AF_XDP dataplane, extending the existing event-stream wiring so RT_FLOW filter-log frames now cover non-PBR input filters, live/cached output filters, and lo0/local-delivery filters, and so RT_FLOW policy-deny frames carry the snapshot's numeric policy ID. Adds deterministic Go syslog fanout coverage for raw userspace event-stream frames and updates the #1379/#1373 plan/docs.

Changes:

• Carry compiled filter ID, term ID, action through filter evaluation, live TX selection, and cached TX descriptors; emit RT_FLOW filter-log events at non-PBR input, live/cached output, and lo0/local-delivery sites in poll_descriptor.rs and forward_request.rs (the non-PBR helper skips routing-instance terms to avoid double emit against the PBR path).
• Plumb policy_id through PolicyRuleSnapshot, PolicyRule, and a new evaluate_policy_result_with_len so emit_policy_deny_event carries a stable compiled ID; userspace snapshot builds the ID with policySetID * MaxRulesPerPolicy + ruleIndex and accounts for per-application expansion.
• Add a deterministic UDP syslog fanout test that feeds raw policy-deny, screen-drop, and filter-log frames through EventReader.ProcessRawEvent, plus docs/README updates describing the new call sites and remaining live-evidence work.

Reviewed changes

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

Show a summary per file

File	Description
userspace-dp/src/policy.rs	Adds PolicyEvaluationResult and a _result_with_len evaluator that returns policy_id alongside the action.
userspace-dp/src/policy_tests.rs	Adds a test that asserts the new evaluator returns the snapshot's policy_id.
userspace-dp/src/protocol.rs	Adds policy_id to PolicyRuleSnapshot with default/round-trip coverage.
userspace-dp/src/filter/mod.rs	Adds FilterLogMatch, has_log_terms, and log-match fields on filter results / cached TX descriptors.
userspace-dp/src/filter/compiler.rs	Sets has_log_terms and includes log-only filters in fast-path indexes.
userspace-dp/src/filter/engine.rs	Adds evaluate_lo0_filter_log_match / evaluate_interface_filter_log_match and threads log matches into TX-selection paths.
userspace-dp/src/filter/tests.rs	Covers the new log-match helper, including skip-routing-instance behavior.
userspace-dp/src/filter/README.md	Documents non-PBR/lo0/output filter-log behavior and ID stability.
userspace-dp/src/afxdp/poll_descriptor.rs	Wires non-PBR input, cached-output, and lo0 filter-log emissions and passes policy_id into policy-deny events.
userspace-dp/src/afxdp/forward_request.rs	Adds an event-stream parameter; emits live output filter-log events from the live TX-selection path.
userspace-dp/src/afxdp/flow_cache.rs	Adds filter_log to CachedTxSelectionDescriptor.
userspace-dp/src/afxdp/tx/cos_classify.rs	Plumbs filter_log through live and cached CoS selection.
userspace-dp/src/afxdp/event_emit.rs	emit_policy_deny_event now takes and emits a policy_id.
userspace-dp/src/afxdp/forwarding/tests.rs	Asserts policy-deny event carries the new policy_id.
userspace-dp/src/afxdp/frame/tests.rs	Adds output-filter-log emission test for the live request builder.
userspace-dp/src/afxdp/tests.rs / umem/tests.rs	Updates fixtures for the new optional filter_log / event-stream arguments.
userspace-dp/src/afxdp/README.md	Documents the broader RT_FLOW filter-log producer scope.
pkg/dataplane/userspace/protocol.go	Adds PolicyID to the Go PolicyRuleSnapshot.
pkg/dataplane/userspace/snapshot.go	Computes stable compiled PolicyIDs (with application-expansion stride) into the snapshot.
pkg/dataplane/userspace/manager_test.go	Asserts the new PolicyID values across round-trips.
pkg/dataplane/userspace/eventstream_test.go	Adds the deterministic raw-frame → syslog fanout coverage with a typed payload builder.
pkg/logging/README.md	Documents the new userspace syslog harness as the canonical place for raw-frame coverage.
docs/userspace-dataplane-gaps.md, docs/pr/1373-retire-ebpf-dataplane/plan-1379-dataplane-events.md	Updates #1379/#1373 status, call sites, and remaining evidence/gaps.

[psaab]

Claude r1 review on 5ef03573

Verdict: MERGE-READY (pending Codex/Gemini hostile-check — user flagged for hardest scrutiny)

r1 verification

event_emit.rs extension verified:

• emit_policy_deny_event now takes policy_id: u32 (was hardcoded 0)
• Sets both policy_id and rule_id: policy_id (compiled policy slot)
• Test updated: assert_eq!(event.policy_id, 101); assert_eq!(event.rule_id, 101);

PR also covers non-PBR filter-log emission across input/output/cached output/lo0 chains. Previous #1418 only had PBR routing-instance filter-log.

Hostile concerns for Codex

1. Hot-path allocation: non-PBR filter call sites in poll_descriptor.rs / forward_request.rs — does the emit go through userspace: add bounded dataplane event producer #1404's bounded-event-producer (try_acquire → encode → enqueue → release on ack/drop)?
2. Event kind disambiguation: PBR vs non-PBR filter-log — same DataplaneEventKind::FilterLog or distinct? Operator filter on RT_FLOW needs to differentiate.
3. Flow-cache fast-path: flow_cache.rs touched — does cached-hit re-emit filter-log on EVERY hit (regression) or only on first install (correct)?
4. End-to-end integration test: previous Emit userspace security dataplane events #1418 r3 MINOR was "helper-level only, would still pass if production call site removed". Does this PR's test exercise poll_binding_process_descriptor end-to-end?
5. IP assertions in tests: Emit userspace security dataplane events #1418 r3 also caught missing event.src_ip / event.dst_ip assertions. Are these added now?
6. Flake: PR body says "Full Rust suite passed on rerun after one known timing-sensitive flake" — which test? Categorically fixed or just retry-until-pass?
7. Per-filter rate limit: a flooded path matching a logged filter could exhaust the event budget. Is there rate-limit per-filter or only the global userspace: add bounded dataplane event producer #1404 cap?

policy_id contract

The change from hardcoded policy_id: 0 to policy_id: <numeric snapshot policy ID> is a wire-shape change. Existing operator dashboards / RT_FLOW consumers that parsed policy_id == 0 as "any deny" now see real IDs. Compatibility question — does this require coordinated rollout, or is it backward-compatible because operators key on zone+rule names not policy_id?

Recommendation

MERGE-READY structurally. The policy_id fix is the right semantic change (rule identity preserved through deny events). Non-PBR filter-log call sites need Codex hot-path verification.

Awaiting Codex (task-mpc374ys-jgo838) + Gemini Pro 3 (task-mpc37fjf-kl2lil). Not merging — author's decision.

[psaab]

Round-1 triple-review synthesis on 5ef03573

Reviewer	Verdict
Claude	MERGE-READY → MERGE-NEEDS-MAJOR (self-correcting)
Codex	MERGE-NEEDS-MAJOR
Gemini Pro 3	MERGE-NEEDS-MAJOR

Gemini MAJOR — hot-path O(N) filter scan per packet

"emit_non_pbr_input_filter_log calls evaluate_interface_filter_log_match directly in the RX loop before the flow cache lookup. This linearly scans all firewall filter terms for every packet on the hot path."

Quoted from poll_descriptor.rs:339-347:

emit_non_pbr_input_filter_log(
    worker_ctx.forwarding,
    worker_ctx.event_stream,
    flow.as_ref(),
    meta,
    ingress_zone_override,
    now_ns,
);
// ── Flow cache fast path ────────────────────────────

The emit fn runs BEFORE the cache check. So every packet — even ones that would hit the cache and bypass the full filter pipeline — pays the O(N) filter-term-scan cost. This is exactly the kind of regression where cached flows lose their fast-path advantage.

Codex MAJOR 1 — operator can't disambiguate PBR vs non-PBR

"PBR and non-PBR paths all emit the same DataplaneEventKind::FilterLog, with no chain/stage/PBR bit in the payload. Go logging path then makes this worse: FILTER_LOG syslog prints src/dst/proto/action/zone only, dropping filter id, term id, egress zone, and stage in ringbuf.go:626."

Operator sees FILTER_LOG events but can't tell which chain (input/output/cached output/lo0/PBR) matched. Triage breaks.

Codex MAJOR 2 — End-to-end production test gap STILL not closed

"I do not see a test driving non-PBR filter logging through poll_binding_process_descriptor, which is the production integration point for input, cached output, and lo0 logging."

Same class of failure as #1418 r3 — the prior round caught "helper-level test would still pass if production call site removed", and r2 added an end-to-end test for the policy-deny path. The non-PBR filter-log path needs the same.

Codex MEDIUM — snapshot protocol versioning fuzzy

"Filter-log itself does not add snapshot fields, so it does not require a version bump. But this same commit adds policy identity into snapshots while Go ProtocolVersion remains 2 in protocol.go:10. If policy id correctness is contractual, this needs versioning or an explicit compatibility story."

Both MINORs converged

IP assertions (#1418 r3 finding): Gemini quotes eventstream_test.go:816-820 — only substring match on "RT_FLOW POLICY_DENY", doesn't verify the actual src/dst IPs in the emitted frame. Codex flagged the same at event_emit.rs:378 and frame/tests.rs:2304.

Per-filter rate-limit: neither author found a per-filter throttle. Rate-limiting is bucketed per (zone × kind). A single noisy filter blinds the entire zone's FilterLog bucket.

Hot-path verification on the budget pattern

Codex AND Gemini both verify: budget acquire-encode-release lifecycle from #1404 is preserved. No leak on Full/Disconnected error paths.

But the hot-path FILTER EVALUATION (Gemini's MAJOR) runs BEFORE the budget try_acquire — so on cached-flow hits, even when the event would be dropped or rate-limited, the filter term scan still happens. That's the perf killer.

Self-correction

My r1 review said MERGE-READY structurally. I flagged hot-path concerns to verify but didn't check the call-site placement relative to the flow cache. Gemini did and found that filter scan runs pre-cache. Codex caught the operator disambiguation issue. Both are real.

Recommendation

Block on:

1. Move filter-log emit AFTER flow cache hit check — for cached flows, the previously-cached filter-log decision should re-fire without re-scanning all terms. Or gate the emit by a stored "this flow has a logged filter" flag in the flow cache entry.
2. Add PBR vs non-PBR distinction in DataplaneEventPayload (e.g., new chain enum or PBR bit). Update Go ringbuf.go to render filter_id/term_id/stage in the syslog output.
3. Add end-to-end production-call-site test via poll_binding_process_descriptor — same pattern as the Emit userspace security dataplane events #1418 r3 fix for policy-deny.
4. Pin IP assertions in the new test — assert event.src_ip and event.dst_ip match the encoded frame's actual IPs, not just a substring match.

Strongly consider: per-filter rate-limit (separate bucket per matched filter ID) to prevent zone-wide blinding.

Defer: snapshot protocol versioning audit for the policy-identity field (consolidate with whichever PR bumps the snapshot version next).

Codex task: task-mpc374ys-jgo838. Gemini task: task-mpc37fjf-kl2lil. Not merging — author's decision.

[psaab]

Claude round-2 review on dc96f693

Verdict: MERGE-NEEDS-MINOR

The two structural r1 MAJORs are fixed. One Junos-parity nuance is worth surfacing.

Verified fixes

1. Operator disambiguation: FilterLogSource {Pbr=1, Input=2, Output=3, CachedOutput=4, Lo0=5} at event_emit.rs:33-54. emit_filter_log_event now takes source: FilterLogSource and writes source.wire_reason() to the reason byte (was hardcoded 0). Five call sites in poll_descriptor.rs each pass the correct tag. Go side at pkg/logging/ringbuf.go:626 dispatches by EventType == EventTypeFilterLog and maps reason byte → "pbr"/"input"/"output"/"cached-output"/"lo0".

2. Hot-path placement: emit_non_pbr_input_filter_log removed from poll_descriptor.rs:339 (was before flow cache lookup, fired on every cached packet) and re-inserted at :944 after session-creation logic — slow path only.

3. Syslog/SSE format extended with source=, filter=, term=, zone=in->out fields in both formatSyslogMsg (ringbuf.go:626-635) and formatLogMessage (pkg/api/sse.go:197-205).

Hostile observations

M1 — Junos parity on cached flows: Junos firewall filters with then log accept fire one RT_FLOW_FILTER_LOG per matched packet (subject to Junos rate limiting). The r2 fix moves INPUT-side emission to session-creation only; on subsequent cached packets the INPUT match is silent and only the OUTPUT side (emit_cached_output_filter_log) emits per-packet — and only with source=cached-output, not source=input. Operators who expect per-packet INPUT logging on cached flows will see one-per-session instead. This is the intentional inverse of the r1 hot-path concern; worth documenting as a known semantic change in the PR description / gaps.md so operators understand the trade-off.

M2 — wire format byte 134 overload: the reason byte was close_reason for FLOW_CLOSE, always 0 for FILTER_LOG. Now it carries source 1-5 for FILTER_LOG, gated by EventType on the Go side. CONFIG_SNAPSHOT_PROTOCOL_VERSION was not bumped — acceptable since the event-stream isn't part of the control-protocol version surface and processes are upgraded together. Worth noting in the codec comment if not already.

M3 — end-to-end test: the r1 end-to-end test gap is partially closed. event_emit.rs tests assert reason byte propagates; ringbuf tests assert source-name mapping; but no single test exercises frame → emit → ringbuf encode → Go decode → syslog format end-to-end with the new source field. Not blocking, but useful follow-up.

Recommendation

Strongly consider: document the cached-flow INPUT under-emit semantic in the PR description or gaps.md so operators know what to expect.

Defer: end-to-end test bridging Rust → Go; could land as a follow-up.

Awaiting Codex (task-mpc55oqs-o478g3) and Gemini Pro 3 (task-mpc580ha-vzinv5).

Not merging — author's decision.

[psaab]

Round-2 triple-review synthesis on dc96f693 (Gemini pending)

Reviewer	Verdict
Claude (self-corrected)	MERGE-NEEDS-MAJOR (was MINOR; Codex caught 4 verified additional MAJORs)
Codex	MERGE-NEEDS-MAJOR (4 MAJORs + 1 MINOR)
Gemini Pro 3	pending (task-mpc5i0ml-idmdas, still running at 4:30)

Self-correction (Claude r2 → MAJOR)

My round-2 Claude review marked this MINOR after verifying the structural fixes for r1 MAJORs. Codex r2 found additional verified MAJORs I missed:

Verified Codex MAJORs

1. Junos parity broken: input filter logs under-emitted on cached flows. emit_non_pbr_input_filter_log only fires on the session-miss slow path at poll_descriptor.rs:944. Cache hits call emit_cached_output_filter_log at :391, which only emits FilterLogSource::CachedOutput. So if a packet matches both input and output filters with then log, INPUT events fire once per session (first slow-path packet) and are silent on subsequent cached hits. Operators expecting Junos "log every matched packet" semantics see one-per-session for INPUT but per-packet for OUTPUT — asymmetric and contradicting Junos behavior.

2. "After session creation" claim in PR description is false. emit_non_pbr_input_filter_log at poll_descriptor.rs:944 fires BEFORE policy evaluation, NAT finalization, route resolution, AND session install. Forward install at :1498, reverse at :1627, missing-neighbor seed at :2399 — all later. The emit is "after flow-cache miss," not "after session creation." Packets emit FILTER_LOG events even when session install or policy evaluation later fails.

3. Wire semantic change unversioned. userspace-dp/src/protocol.rs:10 and pkg/dataplane/userspace/protocol.go:10 both still = 2. No event-stream version exists. payload[134] now means filter-log source for FILTER_LOG and close reason elsewhere. Current Go gates correctly at ringbuf.go:324 and :566 by EventType, but an older Go reader without that gate would decode source byte 2 as "TCP FIN", 3 as "TCP RST", etc. Acceptable internally if helper/daemon lockstep is enforced; needs to be documented as a hard invariant.

4. r1 end-to-end test gap remains. pkg/dataplane/userspace/eventstream_test.go:773 injects synthetic Go-built payloads into the Go decode path; does NOT exercise Rust emit → wire → Go decode. Only asserts source=pbr at :896; no source=input end-to-end assertion. PBR-vs-input disambiguation is not proven through production paths.

Verified Codex MINOR

5. Lo0 tag is local-delivery logging, not lo0 ingress filter logging. emit_lo0_filter_log at poll_descriptor.rs:2218 is in the LocalDelivery branch and evaluates configured lo0 filter state. The actual evaluate_lo0_filter(_counted) enforcement path is not used in production Rust — kernel/nft applyLo0Filter handles real lo0 ingress filtering. The Lo0 tag means "userspace local-delivery matched configured lo0 log term", not proof of lo0 ingress filter behavior. Document operator semantics accordingly.

Codex Confirmed correct

• Cache fast path no longer calls emit_non_pbr_input_filter_log (verified).
• Current Go decoding is correctly gated by event type.
• FilterLogSource is Copy, wire_reason() is inline, no hot-path allocation.
• No downstream FILTER_LOG regex/parser dependency in pkg/api, pkg/grpcapi, or cmd/cli would break on the new format additions.

Recommendation

Block on MAJORs 1-4:

• M1 (Junos parity): decide intent — either accept under-emit as a Junos-parity divergence (document it explicitly in the PR / gaps.md so operators know) or extend cached-flow emission to also fire INPUT events.
• M2 (PR description correctness): update the PR description to say "after flow-cache miss" instead of "after session creation"; the audit/intent docs should accurately describe when emission happens.
• M3 (wire version): either bump CONFIG_SNAPSHOT_PROTOCOL_VERSION / introduce an event-stream version, OR document the helper/daemon lockstep invariant explicitly. Right now the version says "no protocol change" but payload[134] has changed semantics.
• M4 (end-to-end test): add a test that exercises Rust emit → wire encoding → Go decode → asserts source=input (PBR-vs-input disambiguation proven through the production path).

Strongly consider M5 (Lo0 tag): document the actual semantic ("local-delivery match against configured lo0 log term") so operators don't misinterpret it.

Codex task: task-mpc55oqs-o478g3. Gemini Pro 3 re-running (task-mpc5i0ml-idmdas); will update synthesis when complete.

Not merging — author's decision.

[psaab]

Round-2 Gemini Pro 3 verification — independently verified all 5 Codex findings

Gemini Pro 3 (task-mpc5i0ml-idmdas) landed and verified all 5 Codex findings with quoted line evidence. Triple-review now full-converged on MERGE-NEEDS-MAJOR.

Finding	Gemini verdict
1. Junos parity broken — INPUT events under-emitted on cached hits	VERIFIED (poll_descriptor.rs:339 removed; :941 inserted)
2. "After session creation" claim false — emit fires before route resolution / policy / install	VERIFIED (:941-950 fires before ingress_route_table_override, before policy eval, before install at :1498/1627/2399)
3. Wire semantic change unversioned (payload[134] repurposed)	VERIFIED
4. End-to-end test gap (only source=pbr asserted with synthetic Go payload)	VERIFIED (eventstream_test.go:890-894)
5. Lo0 tag conflates LocalDelivery disposition with lo0 ingress filter attachment	VERIFIED (poll_descriptor.rs:212)

Note on Gemini's "Codex hallucinated" sub-claim

Gemini's r2 finding 3 included this hostile assertion:

Codex hallucinated CONFIG_SNAPSHOT_PROTOCOL_VERSION and claimed the version was 2. The actual version defined in pkg/dataplane/userspace/protocol.go:10 is ProtocolVersion = 1.

Verified false. At dc96f693:

git show dc96f693:userspace-dp/src/protocol.rs:10
pub(crate) const CONFIG_SNAPSHOT_PROTOCOL_VERSION: i32 = 2;

git show dc96f693:pkg/dataplane/userspace/protocol.go:10
    ProtocolVersion                  = 2

Both names and values match Codex's r2 citation exactly. Gemini's "Codex hallucinated" sub-claim is itself a hallucination — the cited constants and values are correct. The core architectural finding (wire change is unversioned) stands.

Net

All 5 Codex findings independently verified. The MERGE-NEEDS-MAJOR verdict is solid. Recommendations from the earlier round-2 synthesis still apply (M1 Junos parity / M2 description accuracy / M3 wire versioning or lockstep doc / M4 end-to-end test / M5 Lo0 semantics).

Codex task: task-mpc55oqs-o478g3. Gemini Pro 3 task: task-mpc5i0ml-idmdas.

Not merging — author's decision.

[Copilot]

Pull request overview

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

[psaab]

Claude round-3 review on 5dc6b1d4

Verdict: MERGE-READY (pending Codex + Gemini hostile r3 verification, with one Junos-parity nuance to confirm)

All 4 r2 MAJORs + 1 MINOR have substantive fixes. Cached flows now replay input filter logs per packet; emission only fires post-install; event-stream wire semantics are documented.

Verified fixes

1. Junos parity — cached input log replay: new emit_cached_input_filter_log at poll_descriptor.rs:152 reads cached_descriptor.input_filter_log (stored when cache was populated) and emits per packet. Called at :417-423 BEFORE emit_cached_output_filter_log and BEFORE any continue/break in the fast path. New field RewriteDescriptor.input_filter_log: Option<crate::filter::FilterLogMatch> (flow_cache.rs:57) stores the match. Slow-path stamping at poll_descriptor.rs:2226 threads evaluate_non_pbr_input_filter_log(...).map(|(m, _)| m) into FlowCacheEntry::from_forward_decision. Test from_forward_decision_preserves_input_filter_log_for_cached_hits verifies the field survives the install/retrieve cycle.

2. Post-install gating: emit_non_pbr_input_filter_log is now split into evaluate_non_pbr_input_filter_log (pure eval, returns Option<(FilterLogMatch, ingress_zone_id)>) and emit_input_filter_log_match (emission only). Evaluation happens at :978-984 in slow path; emission only at :1576-1587 inside the if forward_installed branch. No emission on failed install / policy-deny / unroutable packets. Resolves the r2 Codex "fires before route resolution, policy, install" finding.

3. Wire semantic documentation: userspace-dp/src/event_stream/README.md (+6 lines) explicitly documents:

MSG_FILTER_LOG intentionally reuses the RT_FLOW reason byte as a filter-log source discriminator... helper and daemon must therefore be upgraded lockstep for this event-stream semantic; it is not governed by the config snapshot protocol version.

plan-1379-dataplane-events.md carries the same lockstep statement. Resolves the r2 wire-versioning concern by explicit invariant rather than a version bump.

4. End-to-end test: new poll_descriptor_input_filter_log_path_emits_rt_flow_event at tests.rs +220 LOC exercises frame ingress → filter eval → emit path through the production poll_descriptor function, asserting RT_FLOW event surfaces with the correct source byte. Also flow_cache_tests.rs verifies field preservation.

5. Lo0 tag clarification: plan-1379:60-62:

The lo0 source label means a userspace local-delivery packet matched the configured lo0 log term. It does not claim that kernel/nft lo0 ingress enforcement moved into the AF_XDP helper.

Documents the operator semantic explicitly.

Hostile observations

H1 — Junos then log discard parity: the new post-install gate emits only when forward_installed == true. For a filter term with then log discard, the packet is dropped before install. Does that mean log+discard filters now under-emit? Two possibilities:

• (a) Filter action discard causes the slow-path to skip session install and reach a separate drop branch; the log emit would silence — Junos parity regression.
• (b) Filter log evaluation is independent of filter action — log emits on every match regardless of accept/discard, separately from the new post-install gate.

Need a Codex/Gemini trace through evaluate_interface_filter_log_match + the discard branch to confirm. If (a), follow-up needed to keep log+discard semantics.

H2 — Failed-install drop pattern: when session install fails (table full / NAT allocator exhausted), the post-install gate also silences. Probably acceptable since failed installs are operational anomalies, but worth confirming this is the intended trade-off (vs. surfacing the log + later install failure as separate signals).

H3 — Cached emission order: emit_cached_input_filter_log fires BEFORE emit_cached_output_filter_log in the fast path. For a packet matching both input and output filters with log, both events fire per cache hit — matches Junos behavior of "log every matched filter".

Recommendation

Strongly consider: confirm H1 — trace through filter eval + discard path to verify log+discard filters still emit. If they don't, the fix needs to either emit at evaluation time (decoupled from install) OR document the divergence in plan-1379.

Defer: none from r2.

Awaiting Codex (task-mpc7gbcx-osossq) and Gemini Pro 3 (task-mpc7hsub-8flhbj) — they'll independently verify each fix and trace H1.

Not merging — author's decision.

[psaab]

Round-3 triple-review synthesis on 5dc6b1d4 — Claude r3 self-correction; 3 new Codex MAJORs (Gemini pending)

Reviewer	Verdict
Claude r3 (self-corrected)	MERGE-NEEDS-MAJOR (was MERGE-READY)
Codex r3	MERGE-NEEDS-MAJOR (3 new MAJORs)
Gemini Pro 3	pending (re-dispatched: task-mpc7ygfl-ttud6l)

Self-correction (Claude r3 → MAJOR)

My round-3 Claude review marked this MERGE-READY after verifying the post-install gate and cached input log replay. Codex r3 found 3 verified MAJORs my review missed. I read the structural fixes but didn't walk the consistency between slow-path and cached emission paths, or check whether filter ACTIONS (not just LOG matches) are enforced.

Verified Codex r3 MAJORs

M1 — Cached ingress_zone mismatch (cached emit always uses 0):

• Slow-path emission at poll_descriptor.rs:1576-1587 uses ingress_zone_id from evaluate_non_pbr_input_filter_log() (which calls filter_log_ingress_zone_id(forwarding, meta, ingress_zone_override, ingress_ifindex)).
• Cache population at :2221-2235 passes session_ingress_zone: Option<u16> into FlowCacheEntry::from_forward_decision. session_ingress_zone is initialized None at :707 and only set at :771 inside a different match resolved branch — NOT on session-miss install path that populates cache for the slow-path-installed flow.
• flow_cache.rs:318-320 stores metadata.ingress_zone = ingress_zone.unwrap_or(0) → 0 for None.
• Cached replay at :164-169 reads cached_metadata.ingress_zone (= 0) and passes to emit_input_filter_log_match.

Result: first slow-path packet emits RT_FLOW FILTER_LOG ... zone=<from_zone>->...; subsequent cached packets emit RT_FLOW FILTER_LOG ... zone=0->.... Inconsistent operator-visible field. Verified against the code at 5dc6b1d4.

M2 — End-to-end test stops before Go syslog formatter:

• poll_descriptor_input_filter_log_path_emits_rt_flow_event drives real poll_binding_process_descriptor into event_stream::test_worker_handle and decodes the Rust event-stream frame. It does NOT extend through the Go event-stream callback / EventReader / syslog formatter.
• Existing Go syslog test (eventstream_test.go:890-894) only asserts source=pbr, not source=input.
• Result: PBR-vs-input disambiguation is not proven through production paths.

M3 — then log discard action not enforced:

• evaluate_non_pbr_input_filter_log returns FilterLogMatch{action: Discard} for then discard log terms, but poll_descriptor.rs only consults the match for log emission, not for drop enforcement.
• Verified by grep at 5dc6b1d4: crate::filter:: calls in poll_descriptor.rs are limited to evaluate_interface_filter_log_match (log-only), evaluate_lo0_filter_log_match (log-only), record_filter_counter, and apply_cached_three_color_policers. No call to evaluate_filter or evaluate_filter_counted (the action-returning variants) for input filters.
• Result: a packet matching then discard log is logged as action=DENY but still forwarded. Cached hits replay the same misleading log while still forwarding.

This is likely pre-existing (input filter action enforcement is a separate gap, not introduced by #1428), but #1428's new logging makes it visible — operators reading the new FILTER_LOG action=DENY events will reasonably assume the packet was dropped when it wasn't. Either:

• (a) Block on this PR and add input filter action enforcement, OR
• (b) Land Emit non-PBR userspace filter log events #1428 and file an explicit follow-up issue + document the gap in plan-1379 / gaps.md so operators understand the current semantic.

Codex confirmed correct

• Post-install gating at :1576-1587 (only emits after install_with_protocol_with_origin returns true).
• Cached emission order at :420-427 (input log emit BEFORE output log emit, BEFORE continue/break).
• Wire semantics doc at event_stream/README.md:25-30 (lockstep invariant).
• Lo0 wording at plan-1379:56-58 (userspace local-delivery, not kernel/nft).

Recommendation

Block on M1 (ingress_zone mismatch) — purely a fix-the-cache-stamp problem; can be addressed in the same PR. Either set session_ingress_zone = Some(from_zone_id) before cache population, OR store the resolved ingress_zone_id alongside input_filter_log in RewriteDescriptor and replay it on cached hit.

Block-or-document M3 (discard not enforced) — author's call:

• If this is a pre-existing scope-defer, update plan-1379-dataplane-events.md to explicitly document that FILTER_LOG action=DENY events do NOT imply the packet was dropped under the current userspace AF_XDP path; drop enforcement is tracked separately.
• If the goal is parity, add input filter action enforcement in the same PR (the post-install gate makes this straightforward — short-circuit before install when action is Discard).

Strongly consider M2 (end-to-end test) — extend eventstream_test.go to drive a real Rust emit → Go decode for source=input and assert the syslog text matches.

Codex task: task-mpc7gbcx-osossq. Gemini Pro 3 re-dispatched (task-mpc7ygfl-ttud6l).

Not merging — author's decision.

[psaab]

Round-3 Gemini infrastructure unavailable — final r3 verdict on Codex + Claude

All 4 Gemini r3 re-dispatches hit ACP initialize timed out after 30s. Declaring final r3 verdict.

Final r3 verdict: MERGE-NEEDS-MAJOR

Three Codex MAJORs verified independently against the code at 5dc6b1d4. The r3 fixes for the prior round are correct (post-install gating, cached input log replay, wire-semantics doc, Lo0 wording), but Codex's deeper r3 walk caught 3 new issues:

• M1 Cached input filter log emits ingress_zone_id=0 because session_ingress_zone is None for session-miss installs and never set to from_zone_id before cache population. Slow-path emits zone correctly; cached subsequent packets emit zone=0. Inconsistent operator-visible field.
• M2 End-to-end test stops at the Rust event-stream frame; no Go-side syslog assertion for source=input.
• M3 then log discard input filter action is parsed into FilterLogMatch{action: Discard} but NOT enforced — poll_descriptor.rs only calls log-match variants. A discard log term emits FILTER_LOG action=DENY while still forwarding the packet. Likely pre-existing scope-defer (input filter action enforcement is a separate gap), but Emit non-PBR userspace filter log events #1428's new logging makes the inconsistency visible.

See the earlier synthesis comment for quoted-line evidence on each. Block on M1 (purely a cache-stamp fix); M3 is author's-call (block-and-fix vs document-and-defer).

Not merging — author's decision.

[psaab]

Claude round-4 review on f967a112

Verdict: MERGE-READY (pending Codex + Gemini hostile r4 verification, with two non-blocking observations)

All 3 r3 MAJORs have substantive fixes with end-to-end test coverage.

Verified fixes

M1 — Cached ingress_zone correct now. New CachedInputFilterLog { log_match, ingress_zone_id } struct (flow_cache.rs:35-40) bundles the match with the ingress_zone computed at slow-path eval. RewriteDescriptor.input_filter_log field type changed from Option<FilterLogMatch> to Option<CachedInputFilterLog>. emit_cached_input_filter_log at poll_descriptor.rs:208-218 now uses cached_log.ingress_zone_id directly, not cached_metadata.ingress_zone. No more zero-zone on cached replay.

M3 — Discard action enforcement is terminal. New evaluate_non_pbr_input_filter returns NonPbrInputFilterEval { action, cached_log } (poll_descriptor.rs:98-141). At slow-path top (:1029-1050):

• Run the eval.
• If action != FilterAction::Accept:
  • Emit log (if present) via emit_input_filter_log_match
  • Push frame to binding.scratch.scratch_recycle (drop)
  • continue (terminal — BEFORE route_table_override, policy eval, NAT, session install)

Test poll_descriptor_input_filter_discard_drops_and_logs (tests.rs +166 LOC) asserts the wire event correctly: kind=FilterLog, reason=Input.wire_reason(), action=0 (RT_FLOW_ACTION_DENY), ingress_zone_id=TEST_LAN_ZONE_ID (proves M1 ingress_zone fix), and scratch_forwards.is_empty() (proves M3 drop).

New evaluate_interface_filter_non_routing_counted (filter/engine.rs:696-720) — separate code path from the log-only evaluate_interface_filter_log_match. Walks filter terms, skips PBR terms (returns default Accept if PBR matches first), returns FilterResult { action, log_match, ... } for non-PBR matches.

Doc updates (plan-1379-dataplane-events.md:46-54) describe the terminal action behavior and cached-zone replay explicitly.

Hostile observations (non-blocking)

H1 — Reject action. enum FilterAction { Accept, Discard, Reject }. The slow-path gate action != Accept drops both Discard AND Reject silently. Junos's then reject is supposed to return ICMP unreachable. This implementation treats Reject as Discard (silent drop). Junos-parity gap, but operational impact is low because most operators use Discard. Worth either filing a follow-up or documenting in plan-1379 that Reject is currently treated as silent Discard.

H2 — Cache-stamp uses different filter eval function than slow-path. Slow path calls evaluate_interface_filter_non_routing_counted (action+log). Cache stamp at :2289 calls evaluate_interface_filter_log_match (log only). Both walk filter.terms in declaration order and return log info for the first matching term, with the same PBR-skip semantic (evaluate_filter_ref_log_match skips PBR via skip_routing_instance && !routing_instance.is_empty() → None). Within a single packet's slow-path execution, the filter state is stable (ArcSwap atomic publish), so both evaluators see the same filter and produce the same CachedInputFilterLog. Consistent.

Could be a single eval reused, but the current code is correct — minor optimization, not a bug.

Recommendation

MERGE-READY with two non-blocking observations:

• Document Reject vs Discard semantic in plan-1379 (or file a follow-up).
• Consider threading the slow-path eval result into cache stamp to avoid double-eval (optimization, not correctness).

Awaiting Codex (task-mpcpwcvt-g02bch) and Gemini Pro 3 (task-mpcpx5vl-r7icxh).

Not merging — author's decision.

[psaab]

Round-4 triple-review synthesis on f967a112 — Claude self-correction; Codex r4 found 3 MAJORs

Reviewer	Verdict
Claude r4 (self-corrected)	MERGE-NEEDS-MAJOR (was MERGE-READY)
Codex r4	MERGE-NEEDS-MAJOR (3 MAJORs)
Gemini Pro 3	infrastructure outage; re-dispatched

Self-correction (Claude r4 → MAJOR)

My round-4 Claude review marked this MERGE-READY with two non-blocking observations (Reject treated as Discard, cache-stamp double-eval). Codex r4 elevated Reject from non-blocking to MAJOR with a stronger argument I missed, and found a NEW MAJOR I didn't probe (lo0 path).

Codex r4 verified MAJORs

M2 — Still not fixed: end-to-end test gap. f967a112 adds Rust-side event-stream assertions only. The new poll_descriptor_input_filter_discard_drops_and_logs test decodes event.decode_dataplane_event() directly without going through Go EventReader → syslog formatter. Existing Go syslog fanout test at pkg/dataplane/userspace/eventstream_test.go:878-898 still uses reason: 1 (pbr) and asserts wantExtra: "source=pbr" — never proves source=input renders correctly through the Go path. The r3 M2 finding remains open.

Reject is silent drop — contract divergence (new MAJOR). enum FilterAction { ..., Reject /* Drop with ICMP unreachable */ } at userspace-dp/src/filter/mod.rs:35-41. Slow-path gate at poll_descriptor.rs:1038-1049 treats Reject identically to Discard (drop + continue, no ICMP emitted). But event_emit.rs:200-205 maps Reject → RT_FLOW action 2 (reject). Operators reading the log see "reject" but the dataplane silently dropped — log says one thing, wire behavior is another. Either implement ICMP-unreachable on Reject OR change the enum doc-comment + emit mapping so Reject behaves as Discard explicitly. My Claude r4 flagged this as non-blocking; Codex correctly elevates it to MAJOR because the contract divergence misleads operators.

lo0/local-delivery filter action bypassed (new MAJOR I missed). poll_descriptor.rs:255-282 evaluate_lo0_filter_log_match is log-only — returns FilterLogMatch but never an action. Local delivery at :2331-2347 emits the log then continues into maybe_reinject_slow_path. A then log discard/reject on a lo0 filter logs but does NOT drop. Same class of bug as r3 M3 was for non-PBR input filters, but on the lo0 path. The r4 fix didn't address it.

Codex confirmed correct

• M1 ingress_zone fix: CachedInputFilterLog { log_match, ingress_zone_id } stored in RewriteDescriptor; cached emit uses cached_log.ingress_zone_id, no longer cached_metadata.ingress_zone. (flow_cache.rs:35-40)
• M3 discard enforcement on non-PBR input: action gate at poll_descriptor.rs:1029-1050 is terminal before route override, policy eval, and session install. Verified by Codex against the surrounding control flow.
• evaluate_interface_filter_non_routing_counted is correct: first-match-wins, PBR-first returns default Accept (defers to PBR routing), non-PBR returns action+log.
• Cache-stamping consistency between evaluate_non_pbr_input_filter (slow path) and evaluate_non_pbr_input_filter_log_only (cache stamp): both walk terms in declaration order, return None on PBR-first-match, return same log_match on non-PBR-first-match.
• New Rust test asserts the right fields (event.reason, action=0, ingress_zone=TEST_LAN_ZONE_ID NOT 0, scratch_forwards.is_empty()).
• PBR + non-PBR mixed filters: PBR-first defers correctly.
• IPv6 covered by the same evaluator path.
• Fragment / non-flow packets return Accept default (no flow → no eval).

Recommendation

Block on three MAJORs:

• M2 (Go syslog test): add a Go-side test that asserts the Go decode + syslog format produces source=input end-to-end. The existing EventReader → syslog path is untested for non-PBR input source.
• Reject contract: decide one of: (a) implement Reject as ICMP-unreachable (match Junos), OR (b) update enum doc-comment + RT_FLOW action mapping to make Reject behave identically to Discard explicitly. Don't ship the log/wire divergence.
• lo0 action enforcement: mirror the non-PBR fix on the lo0 path. Either evaluate action there too, or document that lo0 filter action is currently log-only.

Codex task: task-mpcpwcvt-g02bch. Gemini Pro 3 re-dispatched after r4 task silently dropped from the queue.

Not merging — author's decision.

[psaab]

Round-4 Gemini Pro 3 verification — all 3 Codex MAJORs independently VERIFIED

Gemini Pro 3 re-dispatch (task-mpcq8773-i9ctux) landed and independently verified all 3 Codex r4 MAJORs with quoted-line evidence at f967a112.

Triple-review fully converged on MERGE-NEEDS-MAJOR

Reviewer	Verdict
Claude r4 (self-corrected)	MERGE-NEEDS-MAJOR
Codex r4	MERGE-NEEDS-MAJOR
Gemini Pro 3 r4 (re-dispatch)	MERGE-NEEDS-MAJOR (all 3 verified)

Gemini's quoted-line verification

M2 — Rust/Go test sync gap. Gemini quoted the new Rust test (event.decode_dataplane_event + assert_eq!(event.reason, FilterLogSource::Input.wire_reason())) and the existing Go test at eventstream_test.go:878-898 showing wantExtra: "source=pbr". Confirms: no Go-side assertion for source=input exists in this PR.

Reject is silent drop — contract divergence. Gemini quoted three lines:

• filter/mod.rs:35-41 — Reject /// Drop with ICMP unreachable
• poll_descriptor.rs:1038-1049 — gate drops + continues, no ICMP
• event_emit.rs:169-175 — filter_action_to_rt_flow maps Reject → RT_FLOW_ACTION_REJECT (2)

Operators see RT_FLOW FILTER_LOG ... action=reject in syslog but wire behavior is silent discard. Log/dataplane divergence verified.

lo0 filter action bypassed. Gemini quoted both emit_lo0_filter_log (log-only, no action consultation) AND the ForwardingDisposition::LocalDelivery branch where, after emitting the log, the code calls maybe_reinject_slow_path unconditionally. Host-bound traffic that should have been dropped/rejected by a then log discard/reject lo0 filter is still reinjected to the kernel slow path.

Gemini's framing on M3

The datapath implementation violates both its own logging contracts (Reject vs Discard) and security boundaries (host-bound traffic bypassing filters entirely).

Sharper than Codex's framing on lo0 — calls out the security boundary issue. Worth keeping in mind for the fix: lo0 filter is host-protection, so the bypass is a security visibility/enforcement gap, not just a Junos-parity nicety.

Recommendations (unchanged from earlier r4 synthesis)

• M2: add a Go-side test asserting source=input propagates through EventReader → syslog.
• Reject contract: implement ICMP-unreachable OR change enum + emit mapping to make Reject behave as Discard explicitly.
• lo0: evaluate action on the lo0 path; drop/reject before maybe_reinject_slow_path. Security boundary, not just log consistency.

Codex task: task-mpcpwcvt-g02bch. Gemini Pro 3 task: task-mpcq8773-i9ctux.

Not merging — author's decision.