bneradt commented on a change in pull request #8323:
URL: https://github.com/apache/trafficserver/pull/8323#discussion_r711345017



##########
File path: proxy/http/HttpTunnel.cc
##########
@@ -1689,3 +1702,70 @@ void
 HttpTunnel::internal_error()
 {
 }
+
+void
+HttpTunnel::mark_tls_tunnel_active()
+{
+  _tls_tunnel_last_update = Thread::get_hrtime();
+
+  if (_tls_tunnel_active) {
+    return;
+  }
+
+  _tls_tunnel_active = true;
+  HTTP_INCREMENT_DYN_STAT(tunnel_current_active_connections_stat);
+
+  _schedule_tls_tunnel_activity_check_event();
+}
+
+void
+HttpTunnel::mark_tls_tunnel_inactive()
+{
+  if (!_tls_tunnel_active) {
+    return;
+  }
+
+  _tls_tunnel_active = false;
+  HTTP_DECREMENT_DYN_STAT(tunnel_current_active_connections_stat);
+
+  if (_tls_tunnel_activity_check_event) {
+    _tls_tunnel_activity_check_event->cancel();
+    _tls_tunnel_activity_check_event = nullptr;
+  }
+}
+
+void
+HttpTunnel::_schedule_tls_tunnel_activity_check_event()
+{
+  if (_tls_tunnel_activity_check_event) {
+    return;
+  }
+
+  ink_hrtime period = 
HRTIME_SECONDS(sm->t_state.txn_conf->tunnel_activity_check_period);
+
+  if (period > 0) {
+    EThread *ethread                 = this_ethread();
+    _tls_tunnel_activity_check_event = ethread->schedule_every_local(this, 
period, HTTP_TUNNEL_EVENT_ACTIVITY_CHECK);
+  }
+}
+
+bool
+HttpTunnel::_is_tls_tunnel_active() const
+{
+  ink_hrtime period = 
HRTIME_SECONDS(sm->t_state.txn_conf->tunnel_activity_check_period);
+
+  // This should not be called if period is 0
+  ink_release_assert(period > 0);
+
+  ink_hrtime now = Thread::get_hrtime();
+
+  Debug("http_tunnel", "now=%" PRId64 " last_update=%" PRId64, now, 
_tls_tunnel_last_update);
+
+  // In some cases, m_tls_tunnel_last_update could be lager than now, because 
it's using cached current time.

Review comment:
       lager -> larger

##########
File path: proxy/http/HttpSM.h
##########
@@ -403,6 +403,7 @@ class HttpSM : public Continuation, public 
PluginUserArgs<TS_USER_ARGS_TXN>
   // See if we should allow the transaction
   // based on sni and host name header values
   void check_sni_host();
+  SNIRoutingType tunnel_type() const;

Review comment:
       I suggest: `get_tunnel_type()`. 

##########
File path: doc/admin-guide/files/records.config.en.rst
##########
@@ -3935,6 +3935,14 @@ TLS v1.3 0-RTT Configuration
 
    Set to ``1`` to allow HTTP parameters on early data requests.
 
+SNI Routing
+-----------
+
+.. ts:cv:: CONDIG proxy.config.tunnel.activity_check_period INT 0
+
+   Frequency of checking the activity of SNI Routing Tunnel. The feature is 
disabled in default.

Review comment:
       A few small suggestions for improvement:
   
   * Let's say "The feature is disabled **by** default." ("by" instead of "in").
   * I'd also add a sentence like this in the middle, just to be explicit: `Set 
to ``0`` to disable monitoring of the activity of the SNI tunnels.`
   * The units of the period would be helpful here. I think seconds?




-- 
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