Move things out of the NetVC, subtract early data

Author: zwoop

doc/admin-guide/logging/formatting.en.rst

@@ -455,10 +455,8 @@ cqql  Client Request         Client request header and content length combined,
                              in bytes.
 cqqtl Client Request         Same as cqql_, but for the first transaction on a
                              TLS connection, also includes TLS handshake bytes
-                             received from the client. Since the TLS handshake
-                             only occurs once per connection, the handshake bytes
-                             are only attributed to the first transaction to
-                             prevent double-counting.
+                             received from the client. Note that this metrics
+                             may not always be 100% accurate.
 csscl Cached Origin Response Content body length from cached origin response.
 csshl Cached Origin Response Header length from cached origin response.
 cssql Cached Origin Response Content and header length from cached origin
@@ -476,10 +474,8 @@ psql  Proxy Response         Content body and header length combined of the
                              |TS| response to client.
 psqtl Proxy Response         Same as psql_, but for the first transaction on a
                              TLS connection, also includes TLS handshake bytes
-                             sent to the client. Since the TLS handshake only
-                             occurs once per connection, the handshake bytes are
-                             only attributed to the first transaction to prevent
-                             double-counting.
+                             sent to the client. Note that this metric may not
+                             always be 100% accurate.
 sscl  Origin Response        Content body length of the origin server response
                              to |TS|.
 sshl  Origin Response        Header length of the origin server response.

include/iocore/net/NetVConnection.h

@@ -322,15 +322,6 @@ class NetVConnection : public VConnection, public PluginUserArgs<TS_USER_ARGS_VC
     return 0;
   }
 
-  /** Capture handshake byte statistics.  */
-  virtual bool
-  capture_handshake_bytes(uint64_t &bytes_in, uint64_t &bytes_out)
-  {
-    bytes_in  = 0;
-    bytes_out = 0;
-    return false;
-  }
-
   /** Structure holding user options. */
   NetVCOptions options;
 

include/iocore/net/TLSBasicSupport.h

@@ -51,6 +51,8 @@ class TLSBasicSupport
   std::string_view get_tls_group() const;
   ink_hrtime       get_tls_handshake_begin_time() const;
   ink_hrtime       get_tls_handshake_end_time() const;
+  bool             get_tls_handshake_bytes(uint64_t &bytes_in, uint64_t &bytes_out);
+
   /**
    * Returns a certificate that need to be verified.
    *
@@ -103,4 +105,6 @@ class TLSBasicSupport
 
   ink_hrtime _tls_handshake_begin_time = 0;
   ink_hrtime _tls_handshake_end_time   = 0;
+  uint64_t   _tls_handshake_bytes_in   = 0;
+  uint64_t   _tls_handshake_bytes_out  = 0;
 };

include/proxy/http/HttpUserAgent.h

@@ -31,6 +31,7 @@
 #include "proxy/ProxyTransaction.h"
 #include "records/RecHttp.h"
 #include "iocore/net/TLSBasicSupport.h"
+#include "iocore/net/TLSEarlyDataSupport.h"
 #include "iocore/net/TLSSessionResumptionSupport.h"
 #include "tscore/ink_assert.h"
 
@@ -59,6 +60,7 @@ struct ClientConnectionInfo {
   // TLS handshake bytes (rx = received from client, tx = sent to client)
   uint64_t tls_handshake_bytes_rx{0};
   uint64_t tls_handshake_bytes_tx{0};
+  size_t   tls_early_data_len{0};
 };
 
 class HttpUserAgent
@@ -105,6 +107,8 @@ class HttpUserAgent
 
   uint64_t get_client_tls_handshake_bytes_tx() const;
 
+  size_t get_client_tls_early_data_len() const;
+
 private:
   HttpVCTableEntry *m_entry{nullptr};
   IOBufferReader   *m_raw_buffer_reader{nullptr};
@@ -194,7 +198,11 @@ HttpUserAgent::set_txn(ProxyTransaction *txn, TransactionMilestones &milestones)
       milestones[TS_MILESTONE_TLS_HANDSHAKE_START] = tbs->get_tls_handshake_begin_time();
       milestones[TS_MILESTONE_TLS_HANDSHAKE_END]   = tbs->get_tls_handshake_end_time();
     }
-    netvc->capture_handshake_bytes(m_conn_info.tls_handshake_bytes_rx, m_conn_info.tls_handshake_bytes_tx);
+    tbs->get_tls_handshake_bytes(m_conn_info.tls_handshake_bytes_rx, m_conn_info.tls_handshake_bytes_tx);
+  }
+
+  if (auto eds = netvc->get_service<TLSEarlyDataSupport>()) {
+    m_conn_info.tls_early_data_len = eds->get_early_data_len();
   }
 
   if (auto as = netvc->get_service<ALPNSupport>()) {
@@ -322,6 +330,12 @@ HttpUserAgent::get_client_tls_handshake_bytes_tx() const
   return m_conn_info.tls_handshake_bytes_tx;
 }
 
+inline size_t
+HttpUserAgent::get_client_tls_early_data_len() const
+{
+  return m_conn_info.tls_early_data_len;
+}
+
 inline void
 HttpUserAgent::save_transaction_info()
 {

src/iocore/net/P_SSLNetVConnection.h

@@ -309,8 +309,6 @@ class SSLNetVConnection : public UnixNetVConnection,
   EThread        *getThreadForTLSEvents() override;
   Ptr<ProxyMutex> getMutexForTLSEvents() override;
 
-  bool capture_handshake_bytes(uint64_t &bytes_in, uint64_t &bytes_out) override;
-
 protected:
   // UnixNetVConnection
   bool _isReadyToTransferData() const override;
@@ -378,10 +376,6 @@ class SSLNetVConnection : public UnixNetVConnection,
    */
   char *_getCoalescedHandShakeBuffer(int64_t total_chain_size);
 
-  // TLS handshake byte tracking (bytes read/written during handshake only)
-  uint64_t _tls_handshake_bytes_in  = 0;
-  uint64_t _tls_handshake_bytes_out = 0;
-
   enum SSLHandshakeStatus sslHandshakeStatus          = SSLHandshakeStatus::SSL_HANDSHAKE_ONGOING;
   bool                    sslClientRenegotiationAbort = false;
   bool                    first_ssl_connect           = true;

src/iocore/net/SSLNetVConnection.cc

@@ -978,8 +978,6 @@ SSLNetVConnection::clear()
   sslLastWriteTime            = 0;
   sslTotalBytesSent           = 0;
   sslClientRenegotiationAbort = false;
-  _tls_handshake_bytes_in     = 0;
-  _tls_handshake_bytes_out    = 0;
   hookOpRequested             = SslVConnOp::SSL_HOOK_OP_DEFAULT;
 
   free_handshake_buffers();
@@ -2485,31 +2483,3 @@ SSLNetVConnection::_ssl_read_buffer(void *buf, int64_t nbytes, int64_t &nread)
 
   return ssl_error;
 }
-
-bool
-SSLNetVConnection::capture_handshake_bytes(uint64_t &bytes_in, uint64_t &bytes_out)
-{
-  if (_tls_handshake_bytes_in > 0 || _tls_handshake_bytes_out > 0) {
-    bytes_in  = _tls_handshake_bytes_in;
-    bytes_out = _tls_handshake_bytes_out;
-
-    return false;
-  }
-
-  // If no SSL object, nothing to capture
-  if (this->ssl == nullptr) {
-    bytes_in  = 0;
-    bytes_out = 0;
-
-    return false;
-  }
-
-  // Capture bytes from BIO statistics
-  BIO *rbio = SSL_get_rbio(this->ssl);
-  BIO *wbio = SSL_get_wbio(this->ssl);
-
-  bytes_in = _tls_handshake_bytes_in = rbio ? BIO_number_read(rbio) : 0;
-  bytes_out = _tls_handshake_bytes_out = wbio ? BIO_number_written(wbio) : 0;
-
-  return true;
-}

src/iocore/net/SSLUtils.cc

@@ -1079,11 +1079,11 @@ ssl_callback_info(const SSL *ssl, int where, int ret)
       Metrics::Counter::increment(it->second);
     }
 
-    // Capture TLS handshake byte statistics
     if (netvc && netvc->get_context() == NET_VCONNECTION_IN) {
       uint64_t bytes_in = 0, bytes_out = 0;
+      auto     tbs = TLSBasicSupport::getInstance(const_cast<SSL *>(ssl));
 
-      if (netvc->capture_handshake_bytes(bytes_in, bytes_out)) {
+      if (tbs && tbs->get_tls_handshake_bytes(bytes_in, bytes_out)) {
         Metrics::Counter::increment(ssl_rsb.tls_handshake_bytes_in_total, bytes_in);
         Metrics::Counter::increment(ssl_rsb.tls_handshake_bytes_out_total, bytes_out);
       }

src/iocore/net/TLSBasicSupport.cc

@@ -72,6 +72,36 @@ TLSBasicSupport::clear()
 {
   this->_tls_handshake_begin_time = 0;
   this->_tls_handshake_end_time   = 0;
+  this->_tls_handshake_bytes_in   = 0;
+  this->_tls_handshake_bytes_out  = 0;
+}
+
+bool
+TLSBasicSupport::get_tls_handshake_bytes(uint64_t &bytes_in, uint64_t &bytes_out)
+{
+  if (_tls_handshake_bytes_in > 0 || _tls_handshake_bytes_out > 0) {
+    bytes_in  = _tls_handshake_bytes_in;
+    bytes_out = _tls_handshake_bytes_out;
+    return false;
+  }
+
+  SSL *ssl = this->_get_ssl_object();
+  if (ssl == nullptr) {
+    bytes_in  = 0;
+    bytes_out = 0;
+    return false;
+  }
+
+  BIO *rbio = SSL_get_rbio(ssl);
+  BIO *wbio = SSL_get_wbio(ssl);
+
+  uint64_t bio_in  = rbio ? BIO_number_read(rbio) : 0;
+  uint64_t bio_out = wbio ? BIO_number_written(wbio) : 0;
+
+  bytes_in = _tls_handshake_bytes_in = bio_in;
+  bytes_out = _tls_handshake_bytes_out = bio_out;
+
+  return true;
 }
 
 TLSHandle

src/proxy/logging/LogAccess.cc

@@ -2098,6 +2098,9 @@ LogAccess::marshal_client_req_squid_len(char *buf)
 
 /*-------------------------------------------------------------------------
   Client request squid length plus TLS handshake bytes received for TLS connections.
+  For TLS 1.3 early data (0-RTT), we subtract the early data length from the
+  handshake bytes to avoid double-counting since the early data bytes are
+  already included in client_request_body_bytes.
   -------------------------------------------------------------------------*/
 int
 LogAccess::marshal_client_req_squid_len_tls(char *buf)
@@ -2110,7 +2113,14 @@ LogAccess::marshal_client_req_squid_len_tls(char *buf)
     }
 
     if (!m_http_sm->get_user_agent().get_client_tcp_reused()) {
-      val += m_http_sm->get_user_agent().get_client_tls_handshake_bytes_rx();
+      uint64_t handshake_rx   = m_http_sm->get_user_agent().get_client_tls_handshake_bytes_rx();
+      size_t   early_data_len = m_http_sm->get_user_agent().get_client_tls_early_data_len();
+
+      if (early_data_len > 0 && handshake_rx > early_data_len) {
+        handshake_rx -= early_data_len;
+      }
+
+      val += handshake_rx;
     }
     marshal_int(buf, val);
   }