fix: decode HTTP/2 headers reliably (HPACK, preface detection, duplicate Go TLS writes)

[mayankpande88]

Summary

HTTP/2 requests were decoded badly: on a test cluster about 2.5 HPACK decode errors per stream, and every error leaves headers undecoded. Five separate causes, each of which corrupts or desyncs the decoder's dynamic table, plus two capture gaps found while tracing them. This folds #356, #357, #359, #360, #361 and #362, which were tested together; each commit matches one of them and can be reviewed on its own.

What was wrong:

• The HTTP/2 preface was taken for Redis (fix(ebpf): detect the HTTP/2 connection preface before other protocols #360). On ports other than 443/8443 the detectors ran in order, and is_redis_query accepts anything starting with an uppercase letter, PRI * HTTP/2.0 included. Every write until the first server frame was lost, including the first request's headers, which carry most of the HPACK insertions. That affected every new HTTP/2 connection to such a port, internal gRPC included.
• The decoder never recovered (fix(l7): keep decoding HTTP/2 headers after the HPACK table falls out of sync #357). After any error the parser replaced its hpack.Decoder. That doesn't help once the peer references entries the decoder never saw: on a connection that does so every request, every block failed for good.
  • The parser also fed hpack.Decoder with Write and never Close, so it rejected the table size update Go clients send a few requests in. That put every Go client connection into the cascade.
  • A new decoder skips references it can't resolve, applies every insertion, and resets only when a block is known to be lost or decodes to impossible pseudo-headers.
  • Every way a header block can be lost is now known to the parser: truncation (including frames after the cut), a pending block replaced by a new HEADERS frame, malformed or oversized frames, and ring-buffer drops. For drops, the kernel flags the connection's next delivered event.
• Go TLS writes were sent twice (fix(tls): send each Go TLS write once #359). A goroutine whose stack grows restarts the function from its entry, where the probe sat. The probe now sits past the prologue's stack check.
• 4096-byte events ended in an unwritten byte (fix(ebpf): size L7 ring buffer records to their content #356). Copies stopped at 4095 bytes while the decoder read 4096. Go's HTTP/2 reads and flushes through 4 KB buffers.
• Streams were silently refused at 100 (fix(l7): make room for new HTTP/2 streams by dropping the oldest waiting one #361). Requests whose responses were lost piled up to the limit and blocked new streams until the GC ran. The oldest now makes room.

Also:

• Ring-buffer slots (fix(ebpf): size L7 ring buffer records to their content #356): L7 ring-buffer records are sized to their content instead of a fixed ~8 KB slot. Small HTTP/2 frames no longer fill the buffer with empty space.
• Stripped binaries (fix(tls): read a stripped binary's function table through the open file #362): a stripped Go binary's function table was read by reopening /proc/<pid>/exe. A process that exited mid-lookup got the binary cached as having no TLS functions, so every later process of it went unprobed.

Metrics: node_agent_hpack_decode_errors_total now counts only real decode failures. node_agent_http2_stage_total gets three new stages: hpack_partial (unresolvable references), stream_evicted (evictions) and events_lost (resynchronizations after a ring-buffer drop).

Engineering detail

Each original PR has the full analysis and its own review thread: #356, #357, #359, #360, #361, #362.

• Verifier: eBPF changes load on 5.10 and 6.1 (TestProgramsLoad, TestGoTLSFdWalk). The 4096-byte clamp is inline asm, because written in C the verifier lost the bound.
• Decoder correctness: with the whole connection seen, the decoder emits exactly what hpack.Decoder does. After a mid-stream join it never decodes a wrong value unless a later header block is lost without the parser knowing. Every loss path the parser can see now resets the table, and ring-buffer drops are flagged by the kernel. Reference names copied from entries that existed before the join (custom names like x-request-id) stay unknown; static-table names recover.
• Dynamic table: a ring, so eviction is O(1). Oversized size updates are clamped to 64 KiB.
• Write probe: TestStackCheckEnd covers amd64 and arm64 prologues, for small and large frames.
• Not covered: a Go net/http client joined mid-stream is still not detected as HTTP/2 (:authority first means no static-index byte for the kernel heuristic).

Local e2e: ran agent images built from this change and from its parent in a local Docker VM (kernel 6.10):

• Go net/http HTTP/2 client: ~1.6M requests per 60s run. Parent: 1.5M–3M HPACK decode errors per run. This change: 0 errors, and every request the client made was decoded, in both runs.
• 4096-byte writes: a raw HTTP/2 client writing 4096-byte TLS chunks, checked with a local-only patch that dumps every event the parser receives. Parent: 12 and 5 duplicated writes, and the first writes lost to the Redis misdetection. This change: 0 duplicates, the preface captured, and the whole client stream replays as aligned frames.
• Bulk downloads plus streaming: ring-buffer drops fell from 3.8%/5.5% to 1.0%/0.25%, and streams completed rose from 96.6%/5.1% to 98.6%/98.5%.
• Regression suite: 390 of 390 short-lived TLS clients captured, plus same-path, wrapper/exec, Python and pipe scenarios at parity, with no data races or panics.
• Build checks: unit tests, go vet and golangci-lint pass, and the eBPF programs load on 5.10 and 6.1.
• After the review fixes:
  • Go client: every request decoded on a fresh connection, with 0 decode errors.
  • Regression suite: 389 of 391 short-lived clients captured, other scenarios at parity, no races.
  • A test-only build with a 64 KB ring buffer forced 939 drops. The kernel's lost-event flag reached the parser and triggered 2 resynchronizations, with 0 decode errors, and every request outside the dropped events was decoded.
• Test cluster: only the ring-buffer commit has run there so far. Drops went from 0.22% to 0.09%, with no restarts.

[gemini-code-assist]

Code Review

This pull request introduces several optimizations and robustness improvements to the L7 HTTP/2 parser and eBPF tracer. Key changes include implementing a custom, tolerant HPACK decoder that can recover from mid-stream joins, optimizing ring buffer usage by building events in a per-CPU scratch map (l7_event_heap) before outputting them at their actual size, and probing Go TLS functions past their stack checks to avoid duplicate events when the stack grows. Additionally, the ELF reader is updated to keep the file descriptor open, preventing failures when processes exit. The reviewer identified a high-severity security vulnerability in the eBPF code where the scratch event buffer is not fully cleared, potentially leaking stale payload data from previous requests on the same CPU, and provided a code suggestion to zero the buffer up to the response field.

[blue4209211]

Nice work. The stack-growth diagnosis for duplicated Go TLS writes, preface-before-Redis detection, ring-buffer records sized to their content, the full 4096-byte copy and the stripped-binary fix all look right. Userspace already bounds-checks every slice, so a shorter record can't panic. On the decoder: integer, Huffman, string and static-table handling match RFC 7541. Client and server tables are separate. A ~25 s fuzz of decode (~614k inputs) found no panics.

The "never decodes a wrong value after a mid-stream join" claim holds as long as no later block is lost: the decoder's table is a suffix of the encoder's, and it never evicts later than the encoder does. It does not hold when a block is lost without the parser knowing. Then it emits wrong values, not unknown ones.

1. Loss the parser isn't told about gives wrong header values (ebpftracer/l7/http2.go)

These came from throwaway tests against Go's hpack.Encoder, with :path = /a, /b, /a, /b on successive streams:

• Ring-buffer drop. l7_ringbuf_drops is a global counter, and the parser never learns of a drop. Drop block 1, and block 2 decodes :path as unknown. Block 3 decodes :path="/a" (it should be /b), with nothing marked partial. A wrong :path that starts with / also passes the plausibility check. The PR measures 0.25–1% drops, so this happens in practice.
• Truncation that hides a following HEADERS frame (the truncated branch around line 706). The decoder is reset only if the cut frame is itself HEADERS or CONTINUATION. A single write of a 5000-byte DATA frame followed by HEADERS, captured at 4096 bytes, puts the HEADERS in the missing tail, and nothing is reset. A later stream then decodes /a instead of /b. This is common on gRPC servers, where response DATA plus trailers exceed 4 KB in one write. The same gap exists when rest < 9 (the frame header was never captured), and in the *skip case that returns early.
• A pending header block overwritten (FrameHeaders case, around line 628). A new HEADERS frame replaces *pendingHeaders, whose CONTINUATION was lost, without a reset. If the new block has END_HEADERS, the old one is silently ignored. Repro: HEADERS(3, no END_HEADERS), then HEADERS(5), then stream 7 decodes /a instead of /b.

Suggested fixes:

• On truncation, reset whenever length < captured+missing, when rest < 9, and in the *skip default case.
• Call p.dropPendingHeaders(method, pendingHeaders) at the top of the HEADERS case. Also reset when extractHeaderBlockFragment returns nil, and when an oversized partial frame is discarded.
• For ring-buffer drops, stamp a per-connection sequence number (or a "lost since last event" flag) on each event, so the parser can reset on a gap.
• Until then, narrow the description's claim to "never wrong unless a block is lost undetected".

2. Question: does the server side still miss the preface?

The preface is checked only on writes. looks_like_http2_frame recognizes it only for METHOD_HTTP2_CLIENT_FRAMES, and trace_exit_read calls it with METHOD_HTTP2_SERVER_FRAMES. A gRPC server on a port other than 443/8443 usually gets the preface, SETTINGS and the first HEADERS in one read. That read is still discarded, so the server-side decoder starts without the first block's insertions. Does server-side HTTP/2 capture depend on this path, or is that intended?

3. Eviction prefers long-lived streams (evictOldestRequest)

The oldest kernelTime on a busy gRPC connection is usually a live watch or bidirectional stream. Pairing stays correct, since a late response for an evicted id finds no request and ids aren't reused. But that stream's result is lost, recorded only as stream_evicted. A client HEADERS frame carrying trailers on an evicted id also creates a request with no method or path. Preferring streams whose request side ended longest ago, or that have no response headers yet, would keep these.

Minor

• evict shifts the whole slice per eviction. On a 64 KB block of minimal insertions this took ~34 ms after a size update to 64 KiB, which a client can force on the server side. A ring buffer or batched compaction avoids that.
• A table size update above 64 KiB returns errHpackInvalid and loses the rest of the block. Clamping to hpackMaxTableSize is cheaper and still safe.
• When StackCheckEnd returns 0, the write probe silently falls back to the entry, so duplicates would come back unnoticed. A once-per-binary log or counter would make that visible.
• On Gemini's stale-bytes note: userspace reads only payload_size bytes and the agent already sees those payloads, so I'd treat it as low. A comment or memset would still close the thread.

[mayankpande88]

Thanks for the careful review. All of it is addressed in three new commits (add8572, c3cc8df, a44c7d1).

1. Undetected loss. You're right: wrong values, not unknown ones. Each of your repros is now a test that fails on the previous head with /a decoded for /b.

• Ring-buffer drops: the kernel now records a dropped event on its connection, using connection.l7_lost, one bit per direction, in what was tail padding. The next event delivered on that connection carries it in l7_event.lost_before, the former padding byte. The parser then drops that direction's partial frame and pending block and resets its table, counted as the new events_lost stage. The read-modify-write isn't atomic (atomic OR needs 5.12), so two CPUs racing on one connection can rarely lose a flag. TestHttp2ParserResetsOnLostEvents.
• Truncation: the table is reset unless the missing bytes are known to be the tail of one non-header frame. That covers length < captured+missing, rest < 9, and the *skip default case. TestHttp2ParserResetsWhenTruncationHidesFollowingHeaders is your DATA(5000)+HEADERS repro.
• Pending block: dropPendingHeaders now runs at the top of the HEADERS case, and the table is also reset on a nil fragment and on an oversized partial frame being discarded. TestHttp2ParserResetsWhenPendingHeadersAreOverwritten.
• I've narrowed the claim in the description to "never wrong unless a block is lost undetected".

2. Server-side preface. Intended, and nothing depends on it: L7 is recorded from the client side.

• A plaintext read on an accepted socket ends in trace_exit_read, because only connected sockets are in active_connections (DNS is the exception).
• A TLS library event on an accepted socket gets a connection through ensure_connection_tracked, but userspace filters it: the socket's destination is the client's ephemeral port, which the default --ephemeral-port-range skips.
• The client side of the same connection sees the preface on its write.

3. Eviction. A request still waiting for response headers now goes first, oldest first. One whose response was lost never gets them, and a live watch has them, so it stays. TestHttp2ParserEvictsWaitingRequestsBeforeLongLivedStreams. A client block with no pseudo-headers on an untracked stream (trailers of an evicted one) no longer creates a request. TestHttp2ParserTrailersOnUntrackedStreamCreateNoRequest.

Minor:

• Eviction cost: the table is now a ring, so eviction is O(1). Your 64 KB minimal-insertion block decodes in ~0.3 ms instead of ~34 ms (BenchmarkHpackDecoderManySmallInsertions).
• Oversized size updates: clamped to 64 KiB instead of failing the block (TestHpackDecoderClampsOversizedTableUpdate).
• StackCheckEnd == 0: the entry fallback is now logged once per binary.
• Stale bytes: send_event now documents why they're left. I'm replacing them in a follow-up with a compact record layout, where the response goes right after the payload. That leaves nothing stale and halves request/response records.

Local verification (Docker VM, kernel 6.10):

• Unit, vet and golangci-lint pass, and the eBPF programs load on 5.10 and 6.1 (sys_enter_sendmsg at 203k processed instructions).
• A Go net/http HTTP/2 client: every request decoded on a fresh connection, with 0 decode errors.
• The short-lived-process suite: 389 of 391 captured.
• A throwaway 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 was decoded.

[mayankpande88]

#364 was merged before the review-fix commits I described above reached it (they were pushed after the merge had deleted the branch), so they are in #365, unchanged and tested as described there.