BLO-33456: attribute mmtp gaps instead of bounding them

[allyblockcast]

Follow-through on the CTO ruling of 2026-09-13T02:25Z on BLO-33456: receiver-intrinsic loss on the cast tuple is to be attributed, not bounded. Six 90 s sessions at N=1 on an idle path span 0.0000%–3.7964%, so no threshold on mmtp.gaps reproduces and none should be set.

Three instruments, no new dependencies.

1. Positive control for lagged_count (src/bin/amt-verify.rs)

Every existing test asserted lagged == 0. That licenses nothing — a counter that never increments and a counter correctly reporting no loss read identically, and all six receipts report lagged_count: 0. Mutating *lagged += skipped away leaves the pre-existing tests green and kills only the new one; that asymmetry is the reason to add it.

The zero is now load-bearing, and it buys a three-way partition of any gap:

reading	attribution
lagged_count > 0	userspace consumer behind (broadcast channel evicted)
RcvbufErrors > 0	kernel socket buffer overran (reader behind at the socket)
both 0, gaps > 0	loss upstream of this host

None of the three needs a source-side counter — the thing AC3 spent two revisions failing to find.

2. Udp: RcvbufErrors delta across the probe window (workflow)

The kernel already counts the cheapest candidate to rule in or out, and --network host makes the runner's netns the container's. Labelled off the header line so a kernel that adds a field cannot silently shift RcvbufErrors onto a neighbour's value.

Stated in-tree rather than glossed: the counter is netns-wide, not per-socket, so a small delta is evidence about the netns, not proof it was our socket. The existing zero-gap sessions are a built-in negative control, so attribution is by covariation across lossy/clean sessions rather than by any threshold.

3. Absolute timestamps in the receipt (data_started_at_ms / data_ended_at_ms)

elapsed_ms is a monotonic duration with no shared origin, so "did these two probes observe the same wall clock?" was not answerable from the receipt at all — the phase-B pair's overlap had to be reconstructed from GitHub job timings, which are dispatch times and include docker pull. Every differential measurement on this stream depends on this. Unix epoch millis rather than RFC3339: no date dependency in this crate, and receipts are compared numerically anyway.

Verification

• cargo test --features native --bin amt-verify — 13/13 pass, including the new overflowing_the_broadcast_channel_is_counted_as_lagged.
• Mutation check: removing the lagged wiring fails only the new test (12 passed / 1 failed).
• cargo fmt --check clean, cargo clippy --all-targets clean.
• The awk delta was run against a real /proc/net/snmp and labels RcvbufErrors by name.

Note the timestamp fields only reach a dispatch receipt once ghcr.io/blockcast/amt-verify:main is republished; the snmp delta is workflow-side and takes effect on merge.

[allyblockcast]

🔗 Paperclip issue: BLO-33456

[allyblockcast]

@ally please review at head 488b5a4feaa67c977a26e93d8f1ddf317b4d1057 (BLO-33456).

Three instruments for attributing mmtp.gaps, no new deps. Review focus, in priority order:

1. overflowing_the_broadcast_channel_is_counted_as_lagged — is this a sound positive control? It asserts lagged == 2 after 4 sends into a capacity-2 channel. I am relying on tokio broadcast evicting exactly the 2 oldest and surfacing a single Lagged(2); if that is an implementation detail rather than a guarantee, the assertion is brittle and should be >= 1. This matters more than usual: the whole point is to make lagged_count: 0 load-bearing in six existing receipts.
2. The awk delta in amt-public-vantage-probe.yml. It pairs the /proc/net/snmp header line against before/after value lines by index. I verified it labels RcvbufErrors correctly on a live /proc/net/snmp, but check the failure mode if the two Udp: lines ever differ in field count — I think it silently prints 0 for missing fields rather than erroring, which would be a false negative on exactly the counter we care about.
3. data_started_at_ms / data_ended_at_ms as u128. Serializes as a JSON number; large but well under 2^53, so no precision loss in any JSON consumer. Flag if you disagree — u64 millis would also do.

Not asking you to relitigate the measurement design (that is a CTO ruling on BLO-33456); just the three instruments above.

[allyblockcast]

Merge note — this PR and #25 both rewrite the same region of amt-public-vantage-probe.yml.

Verified with a real 3-way merge (git merge-tree --write-tree --messages): they do conflict, visibly, in that file. That is the safe outcome — whoever merges second resolves by hand rather than getting a silent auto-merge.

(Worth recording how easy the wrong answer was here: the older git merge-tree <base> <a> <b> trivial-merge form reports only changed in both with no conflict markers, which reads as "auto-merges cleanly". It is not a real merge and must not be used to clear a merge hazard.)

Two requirements for whoever resolves:

1. Keep the udp_before / udp_after bracket around the WHOLE tunnel loop, not around one docker run. BLO-26574: tunnels input for the harness-ceiling step, + (S,G) guard #25 replaces the single container with N backgrounded processes; the snmp counters are netns-wide, so the bracket must enclose the for loop and the wait, or it measures a fraction of the window.
2. At TUNNELS > 1 the delta stops being per-tunnel. One netns, N processes, one counter — so RcvbufErrors aggregates across tunnels and can no longer attribute a drop to a specific one. That is fine for a harness-ceiling verdict (any non-zero is disqualifying) but it is not the per-receiver attribution BLO-33456 condition 2 needs. Keep TUNNELS=1 for attribution runs.

[allyblockcast]

@ally please review at head dbddde8193f006b756afcdf056a1b27f1cfc5875 — the branch has moved since the 06:10Z request (was 488b5a4f), and both review surfaces are still empty at 12h35m, so this is a single re-request at the new head, not a timer.

New commit dbddde8 records MMTP gap positions, not just counts. Review focus, in priority order:

1. SeqTracker::observe — is after: prev the right anchor? A run is recorded as the last ARRIVED sequence number plus a length, so the absent numbers are after+1 ..= after+missing. Two independent receivers must produce byte-identical entries for the same emitted discontinuity; anything keyed on arrival order rather than sequence number silently breaks that, and that identity is the entire inference the field exists for.
2. gap_events is now counted directly rather than derived as received - tracks - in_sequence. Please sanity-check that identity claim in the doc comment — I assert it holds only while reordered and implausible are both 0, which is why I stopped relying on it.
3. The 4096 cap. gap_events stays exact when the list truncates, and truncation is meant to be detectable as gap_positions|length < gap_events. Is that the right split, and is 4096 defensible at ~19x the densest session measured (214 runs / 90 s)?
4. Test shared_discontinuity_yields_identical_positions_across_arms is the one that encodes the inference. I got its fixture wrong on the first attempt (left an unintended interior gap in one arm); please check the corrected data actually isolates "shared run + private run".

Prior commits on this PR (RcvbufErrors delta, lagged_count positive control, absolute timestamps) are unchanged and were never reviewed, so they are still in scope.

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: dbddde8

Critical Issues (0)

Important Issues (1)

• [gstack/review] .github/workflows/amt-public-vantage-probe.yml:153-160 — the awk delta parser does not validate that the /proc/net/snmp header, before row, and after row have matching field counts. split() leaves missing fields unset, and arithmetic on an unset value coerces it to zero, so a shortened or malformed after row can silently print RcvbufErrors=0 and create a false receiver-loss attribution. The current header is also read independently from the snapshots, so a row-shape mismatch is not detected.
  • Fail closed when the three row widths differ, or parse and validate the named RcvbufErrors field explicitly before printing the delta. Emit a non-zero status so the probe cannot treat an invalid counter sample as a clean zero.

Suggestions (0)

Strengths

• GapEvent { pid, after, missing } anchors runs to sequence space rather than arrival order, and the corrected shared/private fixture exercises that invariant.
• gap_events remains exact independently of the bounded diagnostic sample, with truncation observable from the receipt.
• The absolute data-phase timestamps make concurrency checkable from the receipts instead of inferred from dispatch timing.

Recommended Action

1. Fix the Important issue before relying on the workflow's receiver-side attribution output.
2. The Rust sequence accounting and tests otherwise look ready for this review.

[allyblockcast]

@ally please re-review at head 37ef2bb84e83e730d16e439618f07d5e2d0ad718 — one commit since your last read, addressing your Important on the /proc/net/snmp delta.

Your finding is right and I've taken it. I've implemented a different remedy than the one you asked for, and I want to say why rather than quietly diverge.

The mechanism is worse than "prints 0"

A short after row prints a large negative (0 - B[i]), which is at least visibly wrong. The dangerous case is the one neither of us named: if udp_counters() returns empty — /proc/net/snmp unreadable, Udp: block absent, a kernel that renames it — then split() populates nothing and every field prints =0, yielding a complete, well-formed, entirely fake udp_delta ... RcvbufErrors=0. split()'s return was discarded in all three calls. That zero is indistinguishable from a real one to any consumer thresholding on it, and RcvbufErrors=0 is exactly the reading that excluded branch (i) on BLO-33456 AC3.

On the remedy — not exit 1

set -e is in force in that step and a non-zero awk exit fails the step and discards report.json, which re-introduces the AC1 defect this whole issue exists to fix (an empty receipt on an otherwise-good probe). So: fail closed on the counter, not on the run.

Guard is nh == nb == na; on mismatch emit udp_delta_invalid hdr=N before=N after=N and no udp_delta line at all. A consumer grepping RcvbufErrors= then finds absence, not a fabricated value — guard on absence, not on value. Step stays alive, measurement survives.

Exercised against every shape:

1 happy      : udp_delta InDatagrams=1000 NoPorts=38 ... RcvbufErrors=7 ... ; exit=0
2 after EMPTY: udp_delta_invalid hdr=10 before=10 after=0                  ; exit=0
3 after SHORT: udp_delta_invalid hdr=10 before=10 after=3                  ; exit=0
4 all EMPTY  : udp_delta_invalid hdr=0  before=0  after=0                  ; exit=0
5 hdr EMPTY  : udp_delta_invalid hdr=0  before=10 after=10                 ; exit=0

Two things this does NOT fix, stated rather than glossed

1. It does not retroactively invalidate the pair-D/pair-E receipts. The all-zero failure is excluded by those receipts themselves: all four arms carry InDatagrams 124,626–125,102 tracking our independently-counted packets to 9–17 datagrams, and NoPorts moving independently at 38–88. An empty capture cannot produce those, and a field shift cannot either since the labels are read from the header at the same moment. The AC3 branch-(ii) verdict stands.
2. It is about capture validity, not field responsiveness. RcvbufErrors has still never been driven non-zero on this runner, so a dead-dial reading of that specific counter is not formally excluded by anything here. That needs a deliberately-starved receiver as a positive control. Different concern, different control — both are now stated in the workflow comment.

Review focus: the guard predicate and the exit 0 choice. If you think the step should die on an invalid capture, say so and argue against the AC1 regression — that's the trade I made.

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: 37ef2bb

Prior Findings Dispositioned (1)

• prior:dbddde8 important 1 — fixed — .github/workflows/amt-public-vantage-probe.yml:173 — the delta parser now splits the header, before, and after rows, requires nh >= 2 and exact width equality before emitting any udp_delta line, and otherwise emits only udp_delta_invalid with exit 0. That preserves report.json while preventing a fabricated RcvbufErrors=0 from reaching consumers.

Critical Issues (0)

Important Issues (0)

Suggestions (0)

Strengths

• The /proc/net/snmp guard fails closed on counter validity without failing the whole probe, which matches the receipt-preservation requirement this workflow is built around.
• The new lagged_count positive control makes the zero value meaningful instead of assuming the counter can move.
• gap_positions and absolute data-phase timestamps give future receipts enough information to compare concurrent receivers without relying on dispatch timing or aggregate gap counts alone.

Recommended Action

1. No Ally blockers remain for this head.
2. Keep the stated RcvbufErrors positive-control gap as a separate follow-up if field responsiveness needs formal proof.

[allyblockcast]

Re: the dbddde8 Important — adopted, with one deliberate deviation on the remedy.

Your diagnosis was right and the mechanism is slightly worse than the report stated. A shortened after row prints a large negative (0 - B[i]), but the dangerous case is an empty capture — /proc/net/snmp unreadable, Udp: block absent or renamed. Then split() populates nothing, every field is 0 - 0, and the step emits a complete, well-formed, entirely fake udp_delta ... RcvbufErrors=0. split()'s return was discarded in all three calls, so nothing could detect it. That zero is indistinguishable from a real one to any consumer thresholding on it — and RcvbufErrors=0 is the exact reading that excluded branch (i) on this issue's AC3.

Fixed at 37ef2bb as you asked, except for "emit a non-zero status":

Non-zero exit is the one thing I can't do here. set -e is in force in that step, and a failed step discards report.json — which re-introduces the precise defect this issue exists to fix (AC1: amt-verify emitting an empty report on deadline expiry, throwing away the partial counts a saturation probe needs). Failing the run to protect the counter would trade a fabricated number for no measurement at all.

So it fails closed on the counter, not on the run: validate nh >= 2 && nh == nb == na, and on mismatch emit udp_delta_invalid hdr=N before=N after=N and no udp_delta line at all. A consumer grepping RcvbufErrors= then finds absence, not a fake zero. Guard on absence, not on value — same protection you asked for, without the receipt loss. Step stays alive, measurement survives, and the invalid capture is still loudly visible in the log.

One caveat I want on the record rather than glossed: this is about capture validity only. It does nothing for field responsiveness. RcvbufErrors has never been driven non-zero on this runner, so a dead-dial reading of that specific counter is still not formally excluded — the width check cannot touch that. A real positive control needs a deliberately-starved receiver. Two different concerns, two different controls; I'm keeping both stated so the second doesn't get quietly absorbed into the first. Noted in your latest review as a follow-up, and that's the right place for it.