Fix H3 quiche traffic handling#13213
Open
bneradt wants to merge 1 commit into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves HTTP/3 over QUIC handling for larger bodies and stream lifecycle edge cases, and adds H3 interoperability/timeout coverage around curl, Proxy Verifier, SNI, session tickets, and stream cleanup.
Changes:
- Keeps QUIC stream write data pending until accepted, schedules packet writes on stream updates, and adjusts H3 transaction cleanup.
- Drains UDP
recvmmsgbatches for edge-triggered sockets. - Adds/updates HTTP/3 gold tests and replay support for QUIC ports.
Reviewed changes
Copilot reviewed 32 out of 32 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
src/iocore/net/quic/QUICStream.cc |
Adds pending send-block handling and delayed consumption. |
src/iocore/net/quic/QUICStreamVCAdapter.cc |
Defers write consumption and schedules stream write updates. |
src/iocore/net/quic/QUICStreamAdapter.cc |
Adds explicit consume path. |
src/iocore/net/QUICNetVConnection.cc |
Schedules packet writes when streams update. |
src/iocore/net/UnixUDPNet.cc |
Drains full recvmmsg batches. |
src/proxy/http3/Http3Transaction.cc |
Updates H3 transaction read/write/close lifecycle. |
src/proxy/http3/Http3StreamDataVIOAdaptor.cc |
Buffers and finalizes DATA delivery via persistent reader. |
src/proxy/http3/Http3HeaderVIOAdaptor.cc |
Tracks decoded header bytes and signals decode completion. |
src/proxy/http3/Http3App.cc |
Notifies transactions when streams close. |
include/** QUIC/H3 headers |
Adds APIs and state needed by the implementation changes. |
include/iocore/net/quic/Mock.h |
Updates QUIC mocks for new interfaces. |
src/iocore/net/P_UDPNet.h |
Updates UDP batch-read return type. |
src/iocore/net/P_QUICNetVConnection.h |
Declares stream update callback. |
tests/gold_tests/autest-site/ats_replay.test.ext |
Adds HTTP/3 port wiring for replay tests. |
tests/gold_tests/h3/* |
Adds H3 curl, Proxy Verifier, stream lifetime, timeout, SNI, and session ticket tests/replays. |
tests/gold_tests/timeout/* |
Updates QUIC feature gating for timeout tests. |
Contributor
|
[approve ci autest 2] |
778e8bb to
f69c806
Compare
bneradt
commented
May 29, 2026
9 tasks
f69c806 to
ba62f1b
Compare
# Overview This patch extends the HTTP/3 autest coverage, using curl, golang, and Proxy Verifier HTTP/3 clients to generate their implementations of h3 traffic. It also adds request and response bodies of various sizes, including "large" 300k bodies to exercise multiple packet, buffer, and flow control ATS HTTP/3 implementations. It also exercises some interesting requests, such as HEAD and 204 responses. This patch also includes the various production fixes needed for these tests. # Issues Found and their Fixes ## UDP batches could stall large H3 transfers Large request and response bodies exposed a UDP receive starvation bug in the UDP read path. On systems using `recvmmsg()` with edge-triggered readiness, ATS could read one full batch of datagrams and then leave the rest queued in the kernel without another readable event to wake the QUIC stack. This changes `UDPNetProcessorInternal::read_multiple_messages_from_net()` in `src/iocore/net/UnixUDPNet.cc` to return whether the kernel supplied a full batch. `udp_read_from_net()` now loops while full batches are returned, draining the socket before handing packets to QUIC and avoiding large-transfer stalls caused by unread UDP bursts. ## QUIC stream writes consumed data before quiche accepted it The stream write path consumed the `QUICStreamVCAdapter` write reader inside `_read()`, before `QUICStream::send_data()` knew whether `quiche_conn_stream_send()` had accepted the bytes. When quiche accepted only a partial write or returned a flow-control error, ATS could lose stream data and report write progress too early. This makes `QUICStream::send_data()` keep a pending `IOBufferBlock`/FIN pair until quiche reports successful consumption, and only then calls the new `QUICStreamAdapter::consume()` hook. The concrete reader accounting lives in `QUICStreamVCAdapter::_consume()`, while `QUICStream::has_data_to_send()`, `QUICStream::on_write()`, and `QUICNetVConnection::on_stream_updated()` make newly writable stream data schedule packet writes again. This also treats completed finite writes with only FIN left as writable stream state, so empty bodies and fully consumed bodies still close the H3 stream cleanly. ## H3 transaction cleanup raced with stream closure The timeout and stream lifetime tests exposed cases where an `HQTransaction` could be deleted while an event handler was still active, or while the QUIC stream adapter still had read/write cleanup to finish. That left later stream-close and timeout paths touching state that had already been torn down. This adds explicit transaction lifetime state in `HQTransaction`: `_closed`, `_stream_closed`, `_event_handler_active`, and `_is_write_buffer_flushed()`. `Http3App::on_stream_close()` now calls `HQTransaction::stream_closed()`, and `HQTransaction::_delete_if_possible()` waits until the transaction is done, the stream is closed or no longer readable, and pending writes have flushed before deleting the transaction. ## H3 read completion could run before headers and DATA were settled The H3 request read path could signal completion before asynchronous QPACK header decode and buffered DATA delivery had finished updating the sink VIO. That showed up around HEAD, 204, and stream-close timing because the HTTP state machine needed a stable view of whether headers were decoded and whether a request body existed. This updates `Http3HeaderVIOAdaptor::_on_qpack_decode_complete()` to add the printed header length to the sink VIO and notify `Http3Transaction::on_header_decode_complete()`, which schedules the appropriate read event. `Http3StreamDataVIOAdaptor::finalize()` now uses a persistent reader, writes buffered DATA into the sink VIO exactly once, and updates `ndone`/`nbytes` consistently before the transaction is signaled. ## The QPACK static table had drifted from the standard table The HEAD, 204, and quic-go coverage exposed that ATS's static QPACK table was not the table used by external HTTP/3 implementations. The extra zstd entry and modified `accept-encoding` value in `src/proxy/http3/QPACK.cc` shifted later static indexes, so an externally encoded `:status 204` could decode as a different status. This restores the standard static table entries by using `accept-encoding: gzip, deflate, br` and removing the non-standard `content-encoding: zstd` entry. The new 204 cases in `tests/gold_tests/h3/replays/h3_proxy_verifier.replay.yaml` and `tests/gold_tests/h3/replays/h3_server_for_go_client.replay.yaml` cover this interoperability point with Proxy Verifier and quic-go.
ba62f1b to
6297520
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
This patch extends the HTTP/3 autest coverage, using curl, golang, and
Proxy Verifier HTTP/3 clients to generate their implementations of h3
traffic. It also adds request and response bodies of various sizes,
including "large" 300k bodies to exercise multiple packet, buffer, and
flow control ATS HTTP/3 implementations. It also exercises some
interesting requests, such as HEAD and 204 responses. This patch also
includes the various production fixes needed for these tests.
Issues Found and their Fixes
UDP batches could stall large H3 transfers
Large request and response bodies exposed a UDP receive starvation bug
in the UDP read path. On systems using
recvmmsg()withedge-triggered readiness, ATS could read one full batch of datagrams
and then leave the rest queued in the kernel without another readable
event to wake the QUIC stack.
This changes
UDPNetProcessorInternal::read_multiple_messages_from_net()insrc/iocore/net/UnixUDPNet.ccto return whether the kernel supplied afull batch.
udp_read_from_net()now loops while full batches arereturned, draining the socket before handing packets to QUIC and
avoiding large-transfer stalls caused by unread UDP bursts.
QUIC stream writes consumed data before quiche accepted it
The stream write path consumed the
QUICStreamVCAdapterwrite readerinside
_read(), beforeQUICStream::send_data()knew whetherquiche_conn_stream_send()had accepted the bytes. When quiche acceptedonly a partial write or returned a flow-control error, ATS could lose
stream data and report write progress too early.
This makes
QUICStream::send_data()keep a pendingIOBufferBlock/FIN pair until quiche reports successful consumption,and only then calls the new
QUICStreamAdapter::consume()hook. Theconcrete reader accounting lives in
QUICStreamVCAdapter::_consume(),while
QUICStream::has_data_to_send(),QUICStream::on_write(), andQUICNetVConnection::on_stream_updated()make newly writable streamdata schedule packet writes again. This also treats completed finite
writes with only FIN left as writable stream state, so empty bodies and
fully consumed bodies still close the H3 stream cleanly.
H3 transaction cleanup raced with stream closure
The timeout and stream lifetime tests exposed cases where an
HQTransactioncould be deleted while an event handler was stillactive, or while the QUIC stream adapter still had read/write cleanup to
finish. That left later stream-close and timeout paths touching state
that had already been torn down.
This adds explicit transaction lifetime state in
HQTransaction:_closed,_stream_closed,_event_handler_active, and_is_write_buffer_flushed().Http3App::on_stream_close()now callsHQTransaction::stream_closed(), andHQTransaction::_delete_if_possible()waits until the transaction isdone, the stream is closed or no longer readable, and pending writes
have flushed before deleting the transaction.
H3 read completion could run before headers and DATA were settled
The H3 request read path could signal completion before asynchronous
QPACK header decode and buffered DATA delivery had finished updating the
sink VIO. That showed up around HEAD, 204, and stream-close timing
because the HTTP state machine needed a stable view of whether headers
were decoded and whether a request body existed.
This updates
Http3HeaderVIOAdaptor::_on_qpack_decode_complete()to addthe printed header length to the sink VIO and notify
Http3Transaction::on_header_decode_complete(), which schedules theappropriate read event.
Http3StreamDataVIOAdaptor::finalize()now usesa persistent reader, writes buffered DATA into the sink VIO exactly
once, and updates
ndone/nbytesconsistently before the transactionis signaled.
The QPACK static table had drifted from the standard table
The HEAD, 204, and quic-go coverage exposed that ATS's static QPACK
table was not the table used by external HTTP/3 implementations. The
extra zstd entry and modified
accept-encodingvalue insrc/proxy/http3/QPACK.ccshifted later static indexes, so anexternally encoded
:status 204could decode as a different status.This restores the standard static table entries by using
accept-encoding: gzip, deflate, brand removing the non-standardcontent-encoding: zstdentry. The new 204 cases intests/gold_tests/h3/replays/h3_proxy_verifier.replay.yamlandtests/gold_tests/h3/replays/h3_server_for_go_client.replay.yamlcoverthis interoperability point with Proxy Verifier and quic-go.