Add origin server tcp_info logging

[moonchen]

Add an overridable proxy.config.http.log_server_tcp_info option to log origin server TCP_INFO fields: srtt (smoothed RTT), srtv (RTT variation), sret (cumulative retransmits), and scwn (send congestion window).

[Copilot]

🟡 Changes recommended

There are a couple of correctness/robustness issues in the new TCP_INFO sampling and verification code (potential uninitialized reads / unsupported-field handling and a test flake risk) that should be addressed before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds a new runtime configuration (proxy.config.http.log_server_tcp_info) and related access-log fields to capture a snapshot of origin-connection TCP_INFO values at origin response-header parse time, enabling RTT / RTT variation / retransmits / congestion window reporting without depending on the connection still being open at log-write time.

Changes:

• Add TcpInfoSnapshot plumbing from NetVConnection → HttpSM → TransactionLogData → LogAccess and register new log format symbols (srtt, srtv, sret, scwn).
• Add config record + HTTP config wiring to enable/disable sampling.
• Add AuTest coverage via replay YAMLs plus a verifier script, and document the new config + log fields.

File summaries

File	Description
tests/gold_tests/logging/verify_origin_tcp_info.py	Verifies access-log output contains expected cache result + TCP_INFO values per sampling mode.
tests/gold_tests/logging/replay/origin-tcp-info-retry.replay.yaml	Replay scenario ensuring a TCP_INFO sample from a first parent is not reused after a retry path.
tests/gold_tests/logging/replay/origin-tcp-info-global-disabled.replay.yaml	Replay scenario validating sampling is skipped when access logging is globally disabled.
tests/gold_tests/logging/replay/origin-tcp-info-enabled.replay.yaml	Replay scenario covering miss, hit, and “txn logging disabled at sampling time” behavior.
tests/gold_tests/logging/replay/origin-tcp-info-disabled.replay.yaml	Replay scenario validating default behavior (no sampling) reports -1 fields.
tests/gold_tests/logging/origin-tcp-info.rewrite.config	Uses header_rewrite to disable txn logging at sampling time for a specific request.
tests/gold_tests/logging/log-origin-tcp-info.test.py	Test driver running the replay scenarios and invoking the verifier script.
src/records/RecordsConfig.cc	Registers proxy.config.http.log_server_tcp_info.
src/proxy/logging/TransactionLogData.cc	Exposes origin TCP_INFO snapshot from the transaction’s HttpSM.
src/proxy/logging/LogAccess.cc	Marshals origin TCP_INFO fields as signed integers with -1 sentinel.
src/proxy/logging/Log.cc	Adds new log fields and symbols: srtt, srtv, sret, scwn.
src/proxy/http/HttpSM.cc	Samples TCP_INFO after successful origin response-header parsing and resets sample across attempts.
src/proxy/http/HttpConfig.cc	Wires the new config into HttpConfigParams.
src/iocore/net/P_UnixNetVConnection.h	Implements UnixNetVConnection::get_tcp_info() using getsockopt(TCP_INFO) and debug logging.
include/proxy/logging/TransactionLogData.h	Declares get_server_tcp_info() and includes TcpInfoSnapshot.
include/proxy/logging/LogAccess.h	Declares new marshalers and helper for origin TCP_INFO fields.
include/proxy/http/HttpSM.h	Adds server_tcp_info storage in the state machine.
include/proxy/http/HttpConfig.h	Adds log_server_tcp_info config param to HttpConfigParams.
include/iocore/net/TcpInfoSnapshot.h	Introduces a small POD struct for the sampled TCP_INFO subset.
include/iocore/net/NetVConnection.h	Adds a virtual get_tcp_info() hook with a safe default (returns false).
doc/admin-guide/logging/formatting.en.rst	Documents the new log field symbols and their semantics.
doc/admin-guide/files/records.yaml.en.rst	Documents proxy.config.http.log_server_tcp_info behavior and constraints.

Review details

• Files reviewed: 22/22 changed files
• Comments generated: 4
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟢 Approval recommended

The implementation is opt-in, guarded appropriately, and includes targeted AuTest coverage plus clear documentation for both the record and new log fields.

Review details

• Files reviewed: 22/22 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

Validate the returned TCP_INFO length before accepting and logging incomplete fields.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

src/iocore/net/P_UnixNetVConnection.h:323

• getsockopt(TCP_INFO) may succeed with a shorter payload and updates tinfo_len, but this path ignores that length and always returns true. A binary built with newer headers running on a kernel that omits a later field such as tcpi_total_retrans will therefore log the zero-initialized member as a valid sret=0 sample. Check that the returned length covers every field used, including the platform-specific retransmit member, before accepting the snapshot.

• Files reviewed: 23/23 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

Unresolved critical portability and moderate correctness, platform-coverage, and documentation findings remain.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (5)

doc/admin-guide/files/records.yaml.en.rst:2438

• This new proxy.config.http record is documented inside the Security section, so the generated administrator guide categorizes an origin TCP_INFO/access-logging setting as a security control. Move the block to the HTTP engine/origin or logging-configuration section so the record is discoverable under the behavior it controls.

.. ts:cv:: CONFIG proxy.config.http.log_server_tcp_info INT 0
   :reloadable:
   :overridable:

src/iocore/net/P_UnixNetVConnection.h:323

• getsockopt can succeed while returning a shorter tcp_info than the requested struct. The zero initialization prevents undefined reads, but this still returns true and logs zero-filled, unavailable fields as a valid sample instead of the documented -1; validate that the returned length covers every field used, including the platform-specific retransmit field, before accepting the sample.

  if (0 != safe_getsockopt(fd, IPPROTO_TCP, TCP_INFO, &tinfo, &tinfo_len)) {
    Dbg(_dbg_ctl_socket_tcp_info, "failed getsockopt(%d, TCP_INFO): %s", fd, strerror(errno));
    return false;
  }

src/iocore/net/P_UnixNetVConnection.h:320

• QUICNetVConnection derives from UnixNetVConnection, so this override also runs for UDP-based HTTP/3 origin connections. With sampling enabled it performs a failing getsockopt(IPPROTO_TCP, TCP_INFO) for every QUIC response and emits the socket_tcp_info debug message, even though non-TCP connections are supposed to report no sample. Check con.sock_type before the syscall (or override this in the QUIC class) to avoid the per-transaction syscall and debug noise.

  if (0 != safe_getsockopt(fd, IPPROTO_TCP, TCP_INFO, &tinfo, &tinfo_len)) {

src/iocore/net/P_UnixNetVConnection.h:333

• The new test is gated to Linux (log-origin-tcp-info.test.py:25), so the FreeBSD-specific fallback here (tcpi_snd_rexmitpack) is never exercised even though this branch supplies the documented sret value on FreeBSD. Please add a FreeBSD run or a focused test seam that validates this platform path and its field mapping.

#elif HAVE_STRUCT_TCP_INFO___TCPI_RETRANS
  // FreeBSD spells the cumulative count differently; __tcpi_retrans is the
  // currently outstanding count, which is not what this reports.
  info.retrans = tinfo.tcpi_snd_rexmitpack;

tests/gold_tests/logging/log-origin-tcp-info.test.py:25

• This test is skipped on every platform except Linux, but the implementation and documentation add a separate FreeBSD TCP_INFO path (tcpi_snd_rexmitpack and byte-based scwn). FreeBSD is a required CI platform, so that branch has no runtime coverage and can regress while the replay still passes; run the replay on BSD as well, with platform-specific value assertions if necessary.

Test.SkipUnless(Condition.IsPlatform('linux'), Condition.PluginExists('header_rewrite.so'), Condition.PluginExists('conf_remap.so'))

• Files reviewed: 27/27 changed files
• Comments generated: 1
• Review effort level: Lite

[moonchen]

I've updated this PR to make proxy.config.http.log_server_tcp_info overridable per transaction through conf_remap or header_rewrite, so collection can be enabled or disabled for selected remaps. The global default remains 0.

The override change is in 4aeb84a. Integration tests cover enabling and disabling collection with both plugins and confirm that overrides do not carry over to later requests.

[Copilot]

🔵 Needs a closer look

TCP_INFO length validation and QUIC socket handling need to be addressed.

Review details

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

src/iocore/net/P_UnixNetVConnection.h:315

• QUICNetVConnection derives from UnixNetVConnection and does not override this method, so enabling the option for a QUIC origin sends its UDP socket through the TCP implementation. Each sampled response then performs a failing getsockopt(IPPROTO_TCP, TCP_INFO) (and emits the failure debug message) instead of taking the base class's no-op path; guard non-TCP sockets or override this in the QUIC class.
  src/iocore/net/P_UnixNetVConnection.h:323
• The output length from getsockopt is ignored here. A kernel can successfully return a shorter TCP_INFO payload than the build-time struct tcp_info (for example when the runtime kernel predates fields), so the zero-initialized tail is then copied as a valid sample and may log zero for missing srtt, sret, or scwn instead of -1. Validate that tinfo_len covers every field read below, including the platform-specific retransmit field, before returning success.

• Files reviewed: 27/27 changed files
• Comments generated: 0 new
• Review effort level: Lite

[moonchen]

Addressed the remaining Copilot feedback in 909ee65:

• Added a probe for tcpi_snd_rexmitpack and use that result to guard the FreeBSD accessor.
• Skip non-stream sockets before getsockopt(TCP_INFO). A regression test verifies that a UDP socket causes no TCP_INFO syscall.
• Moved the record documentation into Logging Configuration.
• Keeping AuTest Linux-only. I cannot validate a native FreeBSD run locally, so I am not adding an untested FreeBSD test path.

Update in 004c8b9: the accessor now requires the returned length to match sizeof(tinfo) exactly. A size mismatch leaves all four fields unavailable (-1). This keeps validation simple while conservatively rejecting short replies, including replies that might still contain some usable early fields.

The Linux build, UDP and LogAccess unit tests, origin TCP_INFO AuTest, and Sphinx build passed. The new CMake probe was also checked on Linux against the published FreeBSD 14.3 header, with a negative fixture omitting the member. Native FreeBSD execution remains unverified.

[Copilot]

🟢 Approval recommended

The reviewed changes have no unresolved blocking issues.

Review details

• Files reviewed: 31/31 changed files
• Comments generated: 0 new
• Review effort level: Lite

[moonchen]

Added the exact-size TCP_INFO check in 004c8b9. A successful getsockopt call is accepted only when tinfo_len == sizeof(tinfo); a size mismatch leaves all four log fields at -1.

The Linux build, TCP_INFO and LogAccess unit tests, and existing AuTest pass. A local syscall shim also verified the short-reply case: a successful call reporting one byte less than requested was accepted by the previous build and rejected by the new build. Normal replies still log real values.

[Copilot]

🟡 Changes recommended

TCP_INFO length validation must support shorter kernel-returned structures that contain the fields being read.

Get a fresh assessment by requesting another Copilot review.

Review details

• Files reviewed: 31/31 changed files
• Comments generated: 1
• Review effort level: Lite