fix(l7): resynchronize HTTP/2 parsing after any lost header block (#364 review)

[mayankpande88]

Summary

The review fixes for #364. #364 was merged before they were pushed, so they are here instead. Details are in the review reply on #364.

• Lost header blocks no longer decode to wrong values.
  • Ring-buffer drops: the kernel records a dropped event on its connection (connection.l7_lost), and the next delivered event carries it (l7_event.lost_before). The parser resets that direction, counted as a new events_lost stage.
  • Truncation: the table is also reset when frames after the cut were lost, when the cut frame's header wasn't captured, and when a frame ends inside a later read's missing tail.
  • Discarded blocks: the table is reset when a pending header block is replaced by a new HEADERS frame, and when a malformed HEADERS or oversized partial frame is discarded.
• Eviction at the 100-stream limit prefers requests still waiting for response headers, so a live long-lived stream (a watch) isn't evicted first.
• Trailers on an untracked stream no longer create an empty request.
• HPACK table: it is a ring, so eviction is O(1); a 64 KB block of minimal insertions takes ~0.3 ms instead of ~34 ms. Size updates above 64 KiB are clamped instead of failing the block.
• Write probe fallback: when the Go TLS write probe falls back to the function entry, that is now logged once per binary.
• Untracked connections (from the review of this PR): a TLS write on a connection the kernel wasn't tracking inserted the connection into active_connections but kept using a stack copy. The protocol detected on that first write was discarded, so a following write that started mid-frame could never be detected. It now continues on the map entry.

Engineering detail

• Regression tests are built from the review's repros. The truncation and pending-block tests fail on the parent commit, decoding /a for /b.
• l7_lost uses what was tail padding in struct connection, which stays 32 bytes. lost_before takes the event header's former padding byte.
• The read-modify-write isn't atomic, because atomic OR needs 5.12. Two CPUs racing on one connection can rarely lose a flag.

Local e2e: built the agent from this branch (the first three commits are identical to the head tested before #364 merged) and ran it in a local Docker VM (kernel 6.10). A Go net/http HTTP/2 client making ~1.5M requests per 60s had every request decoded on a fresh connection, with 0 decode errors. The short-lived-process suite captured 389 of 391. A test-only build with a 64 KB ring buffer forced 939 drops; the lost flag reached the parser and caused 2 resynchronizations, with 0 decode errors and every request outside the dropped events decoded. With the untracked-connection fix added: the same Go client decoded every request with 0 errors, and the scenario suite, including its OpenSSL clients, matched the earlier runs. Unit tests, vet and golangci-lint pass, and the eBPF programs load on 5.10 and 6.1.

[gemini-code-assist]

Code Review

This pull request introduces robust handling for lost L7 events in the HTTP/2 parser and optimizes the HPACK decoder. Specifically, it tracks lost events via a new l7_lost field in the connection state, allowing the Go-side parser to reset its decoder state and avoid decoding incorrect paths when gaps occur. Additionally, the HPACK decoder is refactored to use a ring buffer for O(1) evictions, and the HTTP/2 stream eviction logic is improved to prioritize streams without response headers. A critical issue was identified in the eBPF code where modifications to conn->l7_lost are not persisted back to the active_connections map when TCP connection tracking misses, as conn still points to a temporary stack variable.

[blue4209211]

Thanks, this covers everything from the #364 review. The lost-event flag, the wider truncation resets, the pending-block drop, eviction order and the HPACK ring all look right. The server-side preface explanation makes sense. go test ./ebpftracer/l7/ passes locally. LGTM, merging.

Three small things for a later pass, none blocking:

1. TLS mark on the untracked-connection switch (trace_enter_write, ~l7.c:577). mark_tls(conn) runs on conn_on_stack before conn switches to the map entry. If another CPU's insert won the BPF_NOEXIST race (for example the TCP connect path, which inserts with tls=0), the tracked entry never gets the mark, and that connection's socket-level ciphertext isn't skipped. Calling mark_tls(conn) again after the switch closes it.
2. The l7_lost race. As you noted, the non-atomic read/clear can lose a flag when CPUs race. To remove it without 5.12 atomics, use a u32 lost counter per direction, incremented with __sync_fetch_and_add (XADD works on older kernels) and carried on each event. Userspace would detect a gap by comparing with the last value it saw, so nothing needs clearing. It needs more than the padding byte, so struct connection would grow past 32 bytes. Worth it only if events_lost shows the race matters.
3. Copy failures aren't flagged. When COPY_PAYLOAD_RINGBUF fails (the bpf_probe_read of the user buffer), the event is dropped before send_event, so l7_lost isn't set. It's rare because those buffers are normally resident, but it's the one loss path the parser still can't see. Setting the same bit there, or counting it, would close it.

[mayankpande88]

Points 1 and 3 are in #367: the TLS mark is applied again after switching to the tracked entry, and failed copies set l7_lost. Point 2, the counter-based l7_lost, waits for events_lost numbers from dev to show the race matters. The CPU follow-up from #353 is #366.