From cb5bd63fc43bae8a179b1a643292264f620a4d9c Mon Sep 17 00:00:00 2001 From: Susan Hinrichs Date: Mon, 4 Jan 2021 18:03:34 +0000 Subject: [PATCH 1/4] Reactivate accecpt_no_activity_timeout --- proxy/ProtocolProbeSessionAccept.cc | 8 ++- proxy/http/HttpConfig.cc | 2 - proxy/http/HttpConfig.h | 2 - proxy/http/HttpSM.cc | 2 +- .../gold_tests/timeout/accept_timeout.test.py | 70 +++++++++++++++++++ tests/gold_tests/timeout/create_request.sh | 3 + tests/gold_tests/timeout/time_client.sh | 20 ++++++ 7 files changed, 101 insertions(+), 6 deletions(-) create mode 100644 tests/gold_tests/timeout/accept_timeout.test.py create mode 100755 tests/gold_tests/timeout/create_request.sh create mode 100644 tests/gold_tests/timeout/time_client.sh diff --git a/proxy/ProtocolProbeSessionAccept.cc b/proxy/ProtocolProbeSessionAccept.cc index cdbffcf24ad..b259f7e96a8 100644 --- a/proxy/ProtocolProbeSessionAccept.cc +++ b/proxy/ProtocolProbeSessionAccept.cc @@ -169,6 +169,7 @@ struct ProtocolProbeTrampoline : public Continuation, public ProtocolProbeSessio int ProtocolProbeSessionAccept::mainEvent(int event, void *data) { + static int no_activity_timeout = 0; if (event == NET_EVENT_ACCEPT) { ink_assert(data); @@ -176,7 +177,12 @@ ProtocolProbeSessionAccept::mainEvent(int event, void *data) NetVConnection *netvc = static_cast(data); ProtocolProbeTrampoline *probe = new ProtocolProbeTrampoline(this, netvc->mutex, nullptr, nullptr); - // XXX we need to apply accept inactivity timeout here ... + // The connection has completed, set the accept inactivity timeout here to watch over the difference between the + // connection set up and the first transaction.. + if (no_activity_timeout == 0) { + REC_ReadConfigInteger(no_activity_timeout, "proxy.config.http.accept_no_activity_timeout"); + } + netvc->set_inactivity_timeout(HRTIME_SECONDS(no_activity_timeout)); if (!probe->reader->is_read_avail_more_than(0)) { Debug("http", "probe needs data, read.."); diff --git a/proxy/http/HttpConfig.cc b/proxy/http/HttpConfig.cc index daafac8b563..39ded16f0d7 100644 --- a/proxy/http/HttpConfig.cc +++ b/proxy/http/HttpConfig.cc @@ -1189,7 +1189,6 @@ HttpConfig::startup() HttpEstablishStaticConfigLongLong(c.oride.transaction_active_timeout_in, "proxy.config.http.transaction_active_timeout_in"); HttpEstablishStaticConfigLongLong(c.oride.transaction_active_timeout_out, "proxy.config.http.transaction_active_timeout_out"); - HttpEstablishStaticConfigLongLong(c.accept_no_activity_timeout, "proxy.config.http.accept_no_activity_timeout"); HttpEstablishStaticConfigLongLong(c.oride.background_fill_active_timeout, "proxy.config.http.background_fill_active_timeout"); HttpEstablishStaticConfigFloat(c.oride.background_fill_threshold, "proxy.config.http.background_fill_completed_threshold"); @@ -1461,7 +1460,6 @@ HttpConfig::reconfigure() params->oride.transaction_active_timeout_out = m_master.oride.transaction_active_timeout_out; params->oride.websocket_active_timeout = m_master.oride.websocket_active_timeout; params->oride.websocket_inactive_timeout = m_master.oride.websocket_inactive_timeout; - params->accept_no_activity_timeout = m_master.accept_no_activity_timeout; params->oride.background_fill_active_timeout = m_master.oride.background_fill_active_timeout; params->oride.background_fill_threshold = m_master.oride.background_fill_threshold; diff --git a/proxy/http/HttpConfig.h b/proxy/http/HttpConfig.h index 8bce9393f0c..71c88fcc60b 100644 --- a/proxy/http/HttpConfig.h +++ b/proxy/http/HttpConfig.h @@ -747,8 +747,6 @@ struct HttpConfigParams : public ConfigInfo { int proxy_request_via_string_len = 0; int proxy_response_via_string_len = 0; - MgmtInt accept_no_activity_timeout = 120; - /////////////////////////////////////////////////////////////////// // Privacy: fields which are removed from the user agent request // /////////////////////////////////////////////////////////////////// diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc index 6af96020aec..c381639fb6f 100644 --- a/proxy/http/HttpSM.cc +++ b/proxy/http/HttpSM.cc @@ -630,7 +630,7 @@ HttpSM::attach_client_session(ProxyTransaction *client_vc, IOBufferReader *buffe ///////////////////////// // set up timeouts // ///////////////////////// - client_vc->set_inactivity_timeout(HRTIME_SECONDS(t_state.http_config_param->accept_no_activity_timeout)); + client_vc->set_inactivity_timeout(HRTIME_SECONDS(t_state.txn_conf->transaction_no_activity_timeout_in)); client_vc->set_active_timeout(HRTIME_SECONDS(t_state.txn_conf->transaction_active_timeout_in)); ++reentrancy_count; diff --git a/tests/gold_tests/timeout/accept_timeout.test.py b/tests/gold_tests/timeout/accept_timeout.test.py new file mode 100644 index 00000000000..1d513271d70 --- /dev/null +++ b/tests/gold_tests/timeout/accept_timeout.test.py @@ -0,0 +1,70 @@ +''' +''' +# 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. + +Test.Summary = 'Testing ATS inactivity timeout' + +Test.SkipUnless( + Condition.HasCurlFeature('http2'), + Condition.HasProgram("telnet", "Need telnet to shutdown when server shuts down tcp"), + Condition.HasProgram("nc", "Need nc to send data to server") +) + +ts = Test.MakeATSProcess("ts", select_ports=True, enable_tls=True) + +ts.addSSLfile("../tls/ssl/server.pem") +ts.addSSLfile("../tls/ssl/server.key") + +ts.Disk.records_config.update({ + 'proxy.config.ssl.server.cert.path': '{0}'.format(ts.Variables.SSLDir), + 'proxy.config.ssl.server.private_key.path': '{0}'.format(ts.Variables.SSLDir), + 'proxy.config.http.transaction_no_activity_timeout_in': 6, + 'proxy.config.http.accept_no_activity_timeout': 2, + 'proxy.config.net.default_inactivity_timeout': 10, + 'proxy.config.net.defer_accept': 0 # Must turn off defer accept to test the raw TCP case +}) + +ts.Disk.ssl_multicert_config.AddLine( + 'dest_ip=* ssl_cert_name=server.pem ssl_key_name=server.key' +) + +# case 1 TLS with no data +tr = Test.AddTestRun("tr") +tr.Setup.Copy("time_client.sh") +tr.Processes.Default.StartBefore(ts) +tr.Processes.Default.Command = 'sh ./time_client.sh \'openssl s_client -ign_eof -connect 127.0.0.1:{0}\''.format( + ts.Variables.ssl_port) +tr.Processes.Default.Streams.stdout = Testers.ContainsExpression("Accept timeout", "Request should fail from the accept timeout") + +# case 2 TLS with incomplete request header +tr2 = Test.AddTestRun("tr") +tr2.Processes.Default.Command = 'echo "GET /.html HTTP/1.1" | sh ./time_client.sh \'openssl s_client -ign_eof -connect 127.0.0.1:{0}\''.format( + ts.Variables.ssl_port) +tr2.Processes.Default.Streams.stdout = Testers.ContainsExpression( + "Transaction inactivity timeout", "Request should fail with transaction inactivity timeout") + +# case 3 TCP with no data +tr3 = Test.AddTestRun("tr") +tr3.Processes.Default.Command = 'sh ./time_client.sh \'telnet 127.0.0.1 {0}\''.format(ts.Variables.port) +tr3.Processes.Default.Streams.stdout = Testers.ContainsExpression("Accept timeout", "Request should fail from the accept timeout") + +# case 4 TCP with incomplete request header +tr4 = Test.AddTestRun("tr") +tr4.Setup.Copy("create_request.sh") +tr4.Processes.Default.Command = 'sh ./time_client.sh \'nc -c ./create_request.sh 127.0.0.1 {0}\''.format(ts.Variables.port) +tr4.Processes.Default.Streams.stdout = Testers.ContainsExpression( + "Transaction inactivity timeout", "Request should fail with transaction inactivity timeout") diff --git a/tests/gold_tests/timeout/create_request.sh b/tests/gold_tests/timeout/create_request.sh new file mode 100755 index 00000000000..aa8da47eefd --- /dev/null +++ b/tests/gold_tests/timeout/create_request.sh @@ -0,0 +1,3 @@ +#!/bin/sh +printf GET / HTTP/1.1 +sleep 11 diff --git a/tests/gold_tests/timeout/time_client.sh b/tests/gold_tests/timeout/time_client.sh new file mode 100644 index 00000000000..8658f7bea2d --- /dev/null +++ b/tests/gold_tests/timeout/time_client.sh @@ -0,0 +1,20 @@ +start=`date +%s` +$1 +echo $? +#openssl s_client -ign_eof -connect 127.0.0.1:4443 > out 2>&1 +end=`date +%s` + +runtime=$((end-start)) +echo $runtime +if [ $runtime -lt 6 ] +then + echo "Accept timeout $runtime" + exit 0 +elif [ $runtime -lt 10 ] +then + echo "Transaction inactivity timeout $runtime" + exit 0 +else + echo "Default inactivity timeout $runtime" + exit 1 +fi From 5bc0901a07399a7e2d89d01bed8603b63a0f364a Mon Sep 17 00:00:00 2001 From: Susan Hinrichs Date: Tue, 5 Jan 2021 00:50:11 +0000 Subject: [PATCH 2/4] Fix rat and clang-format errors --- tests/gold_tests/timeout/create_request.sh | 19 ++++++++++++++++- tests/gold_tests/timeout/time_client.sh | 24 ++++++++++++++++++---- 2 files changed, 38 insertions(+), 5 deletions(-) diff --git a/tests/gold_tests/timeout/create_request.sh b/tests/gold_tests/timeout/create_request.sh index aa8da47eefd..7adddfa951b 100755 --- a/tests/gold_tests/timeout/create_request.sh +++ b/tests/gold_tests/timeout/create_request.sh @@ -1,3 +1,20 @@ -#!/bin/sh +#!/bin/bash + +# 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. + printf GET / HTTP/1.1 sleep 11 diff --git a/tests/gold_tests/timeout/time_client.sh b/tests/gold_tests/timeout/time_client.sh index 8658f7bea2d..1e4b8bcf62c 100644 --- a/tests/gold_tests/timeout/time_client.sh +++ b/tests/gold_tests/timeout/time_client.sh @@ -1,12 +1,28 @@ +# 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. + +nc -l -p $1 -c 'echo -e "This is a reply"' -o test.out & +echo "This is a test" | openssl s_client -servername bar.com -connect localhost:$2 -ign_eof start=`date +%s` $1 -echo $? -#openssl s_client -ign_eof -connect 127.0.0.1:4443 > out 2>&1 end=`date +%s` runtime=$((end-start)) echo $runtime -if [ $runtime -lt 6 ] +if [ $runtime -lt 6 ] then echo "Accept timeout $runtime" exit 0 @@ -14,7 +30,7 @@ elif [ $runtime -lt 10 ] then echo "Transaction inactivity timeout $runtime" exit 0 -else +else echo "Default inactivity timeout $runtime" exit 1 fi From 29ce699063f5080b46e78be7f69594851bd8510a Mon Sep 17 00:00:00 2001 From: Susan Hinrichs Date: Thu, 18 Feb 2021 20:47:19 +0000 Subject: [PATCH 3/4] Move the accept timeout config to HttpConfig so it picks up the reload logic --- proxy/ProtocolProbeSessionAccept.cc | 9 ++++----- proxy/http/HttpConfig.cc | 2 ++ proxy/http/HttpConfig.h | 2 ++ 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/proxy/ProtocolProbeSessionAccept.cc b/proxy/ProtocolProbeSessionAccept.cc index b259f7e96a8..36bba6856f7 100644 --- a/proxy/ProtocolProbeSessionAccept.cc +++ b/proxy/ProtocolProbeSessionAccept.cc @@ -27,6 +27,7 @@ #include "http2/HTTP2.h" #include "ProxyProtocol.h" #include "I_NetVConnection.h" +#include "http/HttpConfig.h" static bool proto_is_http2(IOBufferReader *reader) @@ -169,7 +170,6 @@ struct ProtocolProbeTrampoline : public Continuation, public ProtocolProbeSessio int ProtocolProbeSessionAccept::mainEvent(int event, void *data) { - static int no_activity_timeout = 0; if (event == NET_EVENT_ACCEPT) { ink_assert(data); @@ -179,10 +179,9 @@ ProtocolProbeSessionAccept::mainEvent(int event, void *data) // The connection has completed, set the accept inactivity timeout here to watch over the difference between the // connection set up and the first transaction.. - if (no_activity_timeout == 0) { - REC_ReadConfigInteger(no_activity_timeout, "proxy.config.http.accept_no_activity_timeout"); - } - netvc->set_inactivity_timeout(HRTIME_SECONDS(no_activity_timeout)); + HttpConfigParams *param = HttpConfig::acquire(); + netvc->set_inactivity_timeout(HRTIME_SECONDS(param->accept_no_activity_timeout)); + HttpConfig::release(param); if (!probe->reader->is_read_avail_more_than(0)) { Debug("http", "probe needs data, read.."); diff --git a/proxy/http/HttpConfig.cc b/proxy/http/HttpConfig.cc index 39ded16f0d7..daafac8b563 100644 --- a/proxy/http/HttpConfig.cc +++ b/proxy/http/HttpConfig.cc @@ -1189,6 +1189,7 @@ HttpConfig::startup() HttpEstablishStaticConfigLongLong(c.oride.transaction_active_timeout_in, "proxy.config.http.transaction_active_timeout_in"); HttpEstablishStaticConfigLongLong(c.oride.transaction_active_timeout_out, "proxy.config.http.transaction_active_timeout_out"); + HttpEstablishStaticConfigLongLong(c.accept_no_activity_timeout, "proxy.config.http.accept_no_activity_timeout"); HttpEstablishStaticConfigLongLong(c.oride.background_fill_active_timeout, "proxy.config.http.background_fill_active_timeout"); HttpEstablishStaticConfigFloat(c.oride.background_fill_threshold, "proxy.config.http.background_fill_completed_threshold"); @@ -1460,6 +1461,7 @@ HttpConfig::reconfigure() params->oride.transaction_active_timeout_out = m_master.oride.transaction_active_timeout_out; params->oride.websocket_active_timeout = m_master.oride.websocket_active_timeout; params->oride.websocket_inactive_timeout = m_master.oride.websocket_inactive_timeout; + params->accept_no_activity_timeout = m_master.accept_no_activity_timeout; params->oride.background_fill_active_timeout = m_master.oride.background_fill_active_timeout; params->oride.background_fill_threshold = m_master.oride.background_fill_threshold; diff --git a/proxy/http/HttpConfig.h b/proxy/http/HttpConfig.h index 71c88fcc60b..8bce9393f0c 100644 --- a/proxy/http/HttpConfig.h +++ b/proxy/http/HttpConfig.h @@ -747,6 +747,8 @@ struct HttpConfigParams : public ConfigInfo { int proxy_request_via_string_len = 0; int proxy_response_via_string_len = 0; + MgmtInt accept_no_activity_timeout = 120; + /////////////////////////////////////////////////////////////////// // Privacy: fields which are removed from the user agent request // /////////////////////////////////////////////////////////////////// From 3a1b00dede4497c0e8610db9a85141a07839b706 Mon Sep 17 00:00:00 2001 From: Susan Hinrichs Date: Tue, 9 Mar 2021 00:02:30 +0000 Subject: [PATCH 4/4] Remove unnecessary commands from test script --- tests/gold_tests/timeout/time_client.sh | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/gold_tests/timeout/time_client.sh b/tests/gold_tests/timeout/time_client.sh index 1e4b8bcf62c..e838b41ef06 100644 --- a/tests/gold_tests/timeout/time_client.sh +++ b/tests/gold_tests/timeout/time_client.sh @@ -14,8 +14,6 @@ # See the License for the specific language governing permissions and # limitations under the License. -nc -l -p $1 -c 'echo -e "This is a reply"' -o test.out & -echo "This is a test" | openssl s_client -servername bar.com -connect localhost:$2 -ign_eof start=`date +%s` $1 end=`date +%s`