Skip to content

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

Draft
edsiper wants to merge 9 commits into
masterfrom
agent/tls-event-retry-11551
Draft

tls: yield downstream read retries to the event loop#12140
edsiper wants to merge 9 commits into
masterfrom
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. Forward and the other 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 partial secure-forward PING data across event callbacks; and
  • adds an integration regression that holds one TLS record one byte short and verifies that another Forward client remains responsive.

Approach

Registered downstream connections are already owned by an event loop, even when the 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.

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. Therefore:

  • a configured net.io_timeout caps an idle wait through the existing downstream timeout sweep;
  • the default net.io_timeout: 0 continues to mean no idle timeout; and
  • no private timeout or polling delay is introduced.

Relationship to #11547

This is an event-driven alternative to #11547.

#11547 avoids the hot loop by sleeping 5 ms between retries and adds a private 60-second timeout to synchronous TLS reads and writes. That reduces CPU consumption, but it still blocks the owning event loop and changes the meaning of net.io_timeout: 0.

This PR fixes the reported downstream read path by yielding ownership to the existing event loop and preserving the configured networking timeout semantics. It intentionally does not add a separate hard-coded timeout.

Validation

Configured and built with:

cmake -DFLB_DEV=on -DFLB_TESTS_RUNTIME=On -DFLB_TESTS_INTERNAL=On ../
cmake --build build -j8

The new regression timed out on unpatched master because the second TLS handshake could not be serviced.

Final focused integration run:

tests/integration/.venv/bin/python -m pytest \
  tests/integration/scenarios/in_forward/tests/test_in_forward_001.py::test_in_forward_tls_partial_record_does_not_block_other_connections \
  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: 3 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_tls_partial_record_does_not_block_other_connections \
  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: 3 passed, strict Valgrind clean.

Focused runtime coverage for in_http, in_mqtt, in_syslog, and in_tcp passed. The local flb-rt-in_forward binary could not bind its hard-coded port 24224 because the host's fluentd.service already owned that port; the Forward integration regression above uses an isolated dynamic port and passed.

The full PR commit range also passes .github/scripts/commit_prefix_check.py.

Summary by CodeRabbit

  • Bug Fixes
    • Improved handling of temporary network read conditions across TCP, HTTP, MQTT, Syslog, Unix socket, and secure forwarding inputs.
    • Prevented retryable reads from being misinterpreted as closed connections or errors.
    • Improved TLS event handling so partial records do not block other connections.
    • Added coverage for secure forwarding with partial TLS records.

edsiper added 9 commits July 22, 2026 15:06
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>
Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
@coderabbitai

coderabbitai Bot commented Jul 22, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Adds shared retryable network I/O codes, updates TLS event-driven reads, preserves partial forward handshake data, and prevents multiple network handlers from treating retryable reads as closed connections. An integration test covers partial TLS records alongside another active connection.

Changes

Network retry handling

Layer / File(s) Summary
Shared retry result contract
include/fluent-bit/flb_io.h, include/fluent-bit/tls/flb_tls.h
Defines shared want-read/write result codes and maps TLS status macros to them.
Event-driven TLS read flow
src/tls/flb_tls.c
Updates TLS event-mask switching and returns retryable results for event-driven downstream connections.
Forward handshake buffering
plugins/in_forward/fw_prot.c, plugins/in_forward/fw_conn.c
Uses the connection buffer for partial handshake reads and propagates retry results through handshake processing.
Retry-aware network handlers
plugins/in_http/http_conn.c, plugins/in_mqtt/mqtt_conn.c, plugins/in_syslog/syslog_conn.c, plugins/in_tcp/tcp_conn.c, plugins/in_unix_socket/unix_socket_conn.c, src/http_server/flb_http_server.c, tests/integration/scenarios/in_forward/tests/test_in_forward_001.py
Adds early retry handling across network event paths and tests partial TLS records with concurrent forward connections.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant TLSClient
  participant ForwardConnection
  participant flb_tls_net_read
  participant io_tls_event_switch
  participant ForwardBuffer
  TLSClient->>ForwardConnection: send partial TLS record
  ForwardConnection->>flb_tls_net_read: read encrypted data
  flb_tls_net_read->>io_tls_event_switch: register READ/WRITE interest
  flb_tls_net_read-->>ForwardConnection: retry result
  ForwardConnection->>ForwardBuffer: retain partial handshake data
  TLSClient->>ForwardConnection: send complete message on second connection
  ForwardConnection-->>TLSClient: process and forward message
Loading

Suggested reviewers: cosmo0920

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 12.50% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The code changes address #11551 by avoiding retry loops, rearming event-driven TLS reads, and preserving partial Forward data.
Out of Scope Changes check ✅ Passed The added test helper and regression test are directly related to validating the TLS retry fix, with no unrelated scope visible.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: downstream TLS read retries are handed back to the event loop.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch agent/tls-event-retry-11551

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 091ef93a0c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/tls/flb_tls.c
}
else if (ret == FLB_TLS_WANT_WRITE) {
if (event_driven) {
if (io_tls_event_switch(session, MK_EVENT_READ | MK_EVENT_WRITE) == -1) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve custom dispatch after TLS read wants write

When a downstream TLS read returns FLB_TLS_WANT_WRITE, this new path calls io_tls_event_switch(), which re-registers the connection as FLB_ENGINE_EV_THREAD; these event-driven input/http-server connections do not set connection->coroutine, and the engine/downstream worker dispatches handlers only for FLB_ENGINE_EV_CUSTOM. In the SSL_read WANT_WRITE case (e.g. post-handshake TLS work), the next readiness notification is therefore ignored and the connection stalls instead of resuming the plugin handler.

Useful? React with 👍 / 👎.

Comment on lines +160 to +161
ret = flb_io_net_read(conn->connection,
conn->buf + conn->buf_len, avail);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Bound secure-forward reads by the allocated buffer

With buffer_chunk_size configured below 1023 bytes, conn->buf is allocated to that smaller size in fw_conn_add(), but the secure-forward PING reader now reads up to the hard-coded size (1023) into that buffer. A normal secure-forward handshake larger than the configured chunk can therefore write past conn->buf; either grow the connection buffer first or cap avail by conn->buf_size - conn->buf_len.

Useful? React with 👍 / 👎.

@edsiper
edsiper marked this pull request as draft July 23, 2026 13:59
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

2 participants