Skip to content

tls: yield downstream read retries to the event loop - #12140

Open
edsiper wants to merge 259 commits into
5.0from
agent/tls-event-retry-11551
Open

tls: yield downstream read retries to the event loop#12140
edsiper wants to merge 259 commits into
5.0from
agent/tls-event-retry-11551

Conversation

@edsiper

@edsiper edsiper commented Jul 22, 2026

Copy link
Copy Markdown
Member

Summary

Fixes #11551. (workaround for v5.0.x)

A downstream TLS connection can return FLB_TLS_WANT_READ after consuming an incomplete TLS record. Event-driven server consumers currently enter the synchronous flb_tls_net_read() retry loop, which prevents the event loop from servicing another connection and can consume 100% CPU.

This change:

  • returns retryable TLS read states to the downstream event loop;
  • updates downstream consumers to retain connections on retryable I/O results;
  • rearms read/write interest according to the TLS backend request;
  • preserves each connection's existing event dispatch type while changing TLS interest masks;
  • preserves partial secure-forward PING data across event callbacks;
  • grows and bounds the secure-forward handshake buffer within buffer_max_size; and
  • adds integration coverage for partial TLS records and secure-forward authentication with a 64-byte receive chunk.

Approach

Registered downstream connections are already owned by an event loop, even when a plugin does not use the coroutine-based FLB_IO_ASYNC path. For those connections, flb_tls_net_read() now returns FLB_IO_WANT_READ or FLB_IO_WANT_WRITE after updating event interest instead of retrying synchronously.

TLS event-mask changes reuse event->type, so FLB_ENGINE_EV_CUSTOM handlers remain dispatchable and coroutine-backed FLB_ENGINE_EV_THREAD connections retain their coroutine dispatch.

Secure-forward PING reads retain partial data in the connection buffer. When the configured chunk is smaller than the PING, the buffer grows in configured chunks up to the smaller of the protocol limit and buffer_max_size; each read is capped by currently allocated capacity.

Unregistered and upstream synchronous connections retain their existing behavior. Successful I/O remains the only operation that refreshes net.io_timeout; a retryable result does not count as activity.

Relationship to #11547

This is an event-driven alternative to #11547. It yields ownership to the existing event loop and preserves configured networking timeout semantics without adding a private polling delay or hard-coded timeout.

Validation

Configured and built successfully:

./tests/integration/setup-venv.sh
cmake -S . -B build -DFLB_TESTS_RUNTIME=On -DFLB_TESTS_INTERNAL=On
cmake --build build -j8

Focused runtime/internal coverage:

ctest --test-dir build -R '^(flb-rt-in_http|flb-rt-in_mqtt|flb-rt-in_syslog|flb-rt-in_tcp|flb-rt-in_forward|in_http_tls_expect\.sh|in_syslog_tcp_tls_expect\.sh|flb-it-downstream_worker|flb-it-upstream_tls|flb-it-http_server)$' --output-on-failure

Nine of ten CTest targets passed. flb-rt-in_forward could not bind its hard-coded 0.0.0.0:24224 because another host service owns that port. The dynamic-port Forward integration coverage below passed.

Focused integration run:

tests/integration/.venv/bin/python -m pytest \
  tests/integration/scenarios/in_forward/tests/test_in_forward_001.py::test_in_forward_downstream_coro_partial_record_keeps_listener_responsive \
  tests/integration/scenarios/in_forward/tests/test_in_forward_001.py::test_in_forward_tls_message_mode \
  tests/integration/scenarios/in_forward/tests/test_in_forward_001.py::test_in_forward_secure_forward_auth_success -q

Result: 9 passed.

Strict Valgrind run:

VALGRIND=1 VALGRIND_STRICT=1 tests/integration/.venv/bin/python -m pytest \
  tests/integration/scenarios/in_forward/tests/test_in_forward_001.py::test_in_forward_downstream_coro_partial_record_keeps_listener_responsive \
  tests/integration/scenarios/in_forward/tests/test_in_forward_001.py::test_in_forward_tls_message_mode \
  tests/integration/scenarios/in_forward/tests/test_in_forward_001.py::test_in_forward_secure_forward_auth_success -q

Result: 9 passed, strict Valgrind clean.

The full rebased PR commit range passes .github/scripts/commit_prefix_check.py in pull-request mode.

Summary by CodeRabbit

  • Bug Fixes

    • Improved handling of temporary network read conditions across TCP, HTTP, MQTT, Syslog, Unix socket, and Forward inputs.
    • Prevented connections from being incorrectly closed when network operations require retrying.
    • Improved TLS event handling during non-blocking reads and writes.
    • Improved Secure Forward handshake processing, including operation with small receive buffers.
  • Tests

    • Expanded Secure Forward integration coverage to include small-buffer configurations.

edsiper and others added 30 commits July 8, 2026 10:55
Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
Bumps [actions/stale](https://github.com/actions/stale) from 10.3.0 to 10.4.0.
- [Release notes](https://github.com/actions/stale/releases)
- [Changelog](https://github.com/actions/stale/blob/main/CHANGELOG.md)
- [Commits](actions/stale@eb5cf3a...1e223db)

---
updated-dependencies:
- dependency-name: actions/stale
  dependency-version: 10.4.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Bumps [github/codeql-action/upload-sarif](https://github.com/github/codeql-action) from 4.36.3 to 4.37.0.
- [Release notes](https://github.com/github/codeql-action/releases)
- [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md)
- [Commits](github/codeql-action@54f647b...99df26d)

---
updated-dependencies:
- dependency-name: github/codeql-action/upload-sarif
  dependency-version: 4.37.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
tls_net_read() and tls_net_write() only set connection->net_error on
SSL_ERROR_SYSCALL. When the peer sends a TLS close_notify while the
underlying TCP socket stays open (for example a proxy in front of the
real upstream doing a graceful TLS-only teardown, or the upstream
itself), SSL_get_error() returns SSL_ERROR_ZERO_RETURN instead, which
falls through to the generic error path and leaves net_error unset.

flb_upstream_conn_release() only refuses to recycle a connection when
net_error is set, so this half-closed connection is returned to the
keepalive pool. The next reuse hits the same cached TLS shutdown
state immediately, with no new network read, fails the same way, and
gets recycled again -- repeating indefinitely until Retry_Limit is
exhausted and the records are dropped.

Set net_error to ECONNRESET on SSL_ERROR_ZERO_RETURN in both
tls_net_read() and tls_net_write(), mirroring the existing
SSL_ERROR_SYSCALL branch. This follows the same convention already
used for non-syscall connection errors elsewhere in the codebase:
net_io_propagate_critical_error() in flb_io.c already treats
ECONNRESET as a critical, non-recyclable error on the plain-socket
path. Once set, flb_upstream_conn_release() destroys the connection
instead of recycling it, so the next flush opens a fresh connection
rather than repeating the same failure.

Only SSL_ERROR_ZERO_RETURN (a clean close_notify) is handled here.
The connection is torn down via flb_tls_session_invalidate(), which
calls SSL_shutdown() to complete the bidirectional close, valid
after ZERO_RETURN. Fatal SSL_ERROR_SSL sessions are intentionally
left out of scope: SSL_shutdown() must not be called after a fatal
error, so recycling them safely needs a separate change to the
teardown path.

Verified with a local repro using a TLS receiver that injects a
close_notify alert on an idle connection while keeping the TCP
socket open. Before this change the same poisoned connection is
recycled and reused on every subsequent flush (repeated "cannot get
ack"); after this change the connection is destroyed after the
single in-flight failure and the following flush establishes a fresh
connection and succeeds.

Addresses #12076

Signed-off-by: ku524 <yeonjuyeong@gmail.com>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
…ffer

The frame loop advances to the next object with the plain
msgpack_unpacker_next(), which does not refresh 'bytes'. When several
frames arrive in a single buffer (common with compressed payloads, which
are small), iterations after the first add the previous object's stale
size to 'all_used'. That wrong total trims conn->buf by the wrong offset
and desyncs the remaining frames. Use msgpack_unpacker_next_with_size()
so 'bytes' is refreshed for every object, matching the initial parse.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: zshuang0316 <zshuang0316@163.com>
Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
Build temporary OTLP protobuf log trees with the public CFL arena and
release all allocations after packing.

Projected task-clock remains 7.94% lower for small requests, 11.54%
lower for 2,048 records, and 10.78% lower for 256 resources. Peak heap
is unchanged.

Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>
edsiper added 8 commits July 29, 2026 12:28
Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
@edsiper
edsiper force-pushed the agent/tls-event-retry-11551 branch from 858ef47 to 3c90e6f Compare July 29, 2026 18:28
@edsiper
edsiper marked this pull request as ready for review July 29, 2026 18:31
@edsiper

edsiper commented Jul 29, 2026

Copy link
Copy Markdown
Member Author

superseded by #12146

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Stuck TLS Connection Hangs Fluent-bit and Consumes 100% CPU

7 participants