moonchen commented on code in PR #13667:
URL: https://github.com/apache/trafficserver/pull/13667#discussion_r3981566115


##########
src/iocore/net/P_UnixNetVConnection.h:
##########
@@ -304,6 +307,39 @@ UnixNetVConnection::set_mptcp_state()
 #endif
 }
 
+// Copy the TCP_INFO fields ATS reports out of the kernel.
+inline bool
+UnixNetVConnection::get_tcp_info(TcpInfoSnapshot &info) const
+{
+#if defined(TCP_INFO) && defined(HAVE_STRUCT_TCP_INFO)
+  struct tcp_info tinfo;
+  int             tinfo_len = sizeof(tinfo);
+  int const       fd        = con.sock.get_fd();
+
+  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;
+  }
+  info.rtt      = tinfo.tcpi_rtt;
+  info.rttvar   = tinfo.tcpi_rttvar;
+  info.snd_cwnd = tinfo.tcpi_snd_cwnd;
+#if HAVE_STRUCT_TCP_INFO_TCPI_TOTAL_RETRANS
+  info.retrans = tinfo.tcpi_total_retrans;
+#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;
+#endif

Review Comment:
   Fixed in 3e3c87618d: sampling now requires one of the supported 
retransmit-counter macros. Otherwise, the accessor returns false without making 
the syscall, so all four fields log `-1`.



##########
tests/gold_tests/logging/verify_origin_tcp_info.py:
##########
@@ -0,0 +1,78 @@
+#  Licensed to the Apache Software Foundation (ASF) under one
+#  or more contributor license agreements.  See the NOTICE file
+#  distributed with this work for additional information
+#  regarding copyright ownership.  The ASF licenses this file
+#  to you under the Apache License, Version 2.0 (the
+#  "License"); you may not use this file except in compliance
+#  with the License.  You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+#  Unless required by applicable law or agreed to in writing, software
+#  distributed under the License is distributed on an "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#  See the License for the specific language governing permissions and
+#  limitations under the License.
+"""Validate origin TCP_INFO access-log fields after replay traffic 
completes."""
+
+import argparse
+from pathlib import Path
+import time
+
+
+def verify(log_path: Path, mode: str) -> None:
+    expected_keys = {
+        'disabled': {'miss'},
+        'enabled': {'miss', 'hit', 'guard', 'oversized', 'malformed'},
+        'retry': {'retry'},
+    }[mode]
+    # Wait for the asynchronous log writer, rather than sleeping a fixed time.
+    deadline = time.monotonic() + 15
+    while True:
+        lines = log_path.read_text().splitlines() if log_path.exists() else []
+        if len(lines) >= len(expected_keys):
+            break
+        if time.monotonic() >= deadline:
+            raise AssertionError(f'Timed out waiting for access-log records 
for {expected_keys}: {lines}')
+        time.sleep(0.1)

Review Comment:
   Fixed in 3e3c87618d: the checker now waits for newline-terminated records. 
It still rejects malformed completed lines. Verified both partial-line waiting 
and malformed-line rejection.



##########
include/proxy/http/HttpSM.h:
##########
@@ -531,8 +531,11 @@ class HttpSM : public Continuation, public 
PluginUserArgs<TS_USER_ARGS_TXN>
   //  do_api_callout_internal()
   bool                hooks_set = false;
   std::optional<bool> mptcp_state; // Don't initialize, that marks it as "not 
defined".
-  const char         *server_protocol       = "-";
-  int                 server_transact_count = 0;
+  /// TCP_INFO for the current origin response, sampled after successful 
header parsing.
+  /// Cleared when starting another origin attempt or reading another response 
header.
+  std::optional<TcpInfoSnapshot> server_tcp_info;

Review Comment:
   Fixed in 3e3c87618d: added a direct include of 
`iocore/net/TcpInfoSnapshot.h` in `HttpSM.h`.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to