This is an automated email from the ASF dual-hosted git repository.
bneradt pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/master by this push:
new c1fcb01040 Break cycle between iocore/inknet and proxy/http (#10587)
c1fcb01040 is described below
commit c1fcb01040ad6fe489280d10b359240a58fff1fc
Author: JosiahWI <41302989+josia...@users.noreply.github.com>
AuthorDate: Mon Oct 16 13:07:35 2023 -0500
Break cycle between iocore/inknet and proxy/http (#10587)
Summary of changes:
* Remove unused headers.
* Break the PreWarm namespace out of PreWarmManager and leave it in
iocore/net.
* Move PreWarmManager back to proxy/http (I had moved it to iocore earlier).
* Use a callback to reconfigure the prewarm manager when sni.yaml is
reloaded.
Removing the unused headers is simply cleanup. They have been in the source
code for a long time and aren't currently used for anything. The other two
changes were more significant. I think there's a tradeoff between putting the
PreWarm namespace in iocore/net or proxy/http. The former makes it easy to
avoid the dependency cycle, but I think the code is more closely related to the
rest of the PreWarmManager than to iocore/net. The last change was to address
the issue of reloading the [...]
---
iocore/net/CMakeLists.txt | 1 -
iocore/net/Makefile.am | 1 -
iocore/net/PreWarm.h | 50 +++++++++++++++++++++
iocore/net/SSLConfig.cc | 2 -
iocore/net/SSLNetVConnection.cc | 2 -
iocore/net/SSLSNIConfig.cc | 16 +++++--
iocore/net/SSLSNIConfig.h | 10 +++++
iocore/net/TLSTunnelSupport.cc | 2 +-
iocore/net/TLSTunnelSupport.h | 3 +-
iocore/net/libinknet_stub.cc | 10 -----
iocore/net/unit_tests/test_SSLSNIConfig.cc | 9 ++++
proxy/http/CMakeLists.txt | 1 +
proxy/http/HttpSM.h | 7 ++-
proxy/http/Makefile.am | 3 +-
{iocore/net => proxy/http}/PreWarmManager.cc | 1 +
{iocore/net => proxy/http}/PreWarmManager.h | 65 ++++++++++++++--------------
16 files changed, 126 insertions(+), 57 deletions(-)
diff --git a/iocore/net/CMakeLists.txt b/iocore/net/CMakeLists.txt
index 2f7b3169c3..9c6a74acf0 100644
--- a/iocore/net/CMakeLists.txt
+++ b/iocore/net/CMakeLists.txt
@@ -31,7 +31,6 @@ add_library(inknet STATIC
NetAcceptEventIO.cc
NetVConnection.cc
PollCont.cc
- PreWarmManager.cc
ProxyProtocol.cc
ReadWriteEventIO.cc
Socks.cc
diff --git a/iocore/net/Makefile.am b/iocore/net/Makefile.am
index 504732eed5..24852ace96 100644
--- a/iocore/net/Makefile.am
+++ b/iocore/net/Makefile.am
@@ -204,7 +204,6 @@ libinknet_a_SOURCES = \
P_UnixUDPConnection.h \
PollCont.h \
PollCont.cc \
- PreWarmManager.cc \
ProxyProtocol.h \
ProxyProtocol.cc \
ReadWriteEventIO.h \
diff --git a/iocore/net/PreWarm.h b/iocore/net/PreWarm.h
new file mode 100644
index 0000000000..72bc5f3bc1
--- /dev/null
+++ b/iocore/net/PreWarm.h
@@ -0,0 +1,50 @@
+/** @file
+
+ Pre-Warming NetVConnection
+
+ @section license License
+
+ 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.
+ */
+
+#pragma once
+
+// inknet
+#include "SSLTypes.h"
+
+// records
+#include "records/I_RecHttp.h"
+
+#include <netinet/in.h>
+
+#include <memory>
+#include <string>
+#include <string_view>
+
+namespace PreWarm
+{
+struct Dst {
+ Dst(std::string_view h, in_port_t p, SNIRoutingType t, int a) : host(h),
port(p), type(t), alpn_index(a) {}
+
+ std::string host;
+ in_port_t port = 0;
+ SNIRoutingType type = SNIRoutingType::NONE;
+ int alpn_index = SessionProtocolNameRegistry::INVALID;
+};
+
+using SPtrConstDst = std::shared_ptr<const Dst>;
+} // namespace PreWarm
diff --git a/iocore/net/SSLConfig.cc b/iocore/net/SSLConfig.cc
index 620c6d6820..8483e2ad07 100644
--- a/iocore/net/SSLConfig.cc
+++ b/iocore/net/SSLConfig.cc
@@ -43,8 +43,6 @@
#include "tscore/I_Layout.h"
#include "records/I_RecHttp.h"
-#include "HttpConfig.h"
-
#include "P_Net.h"
#include "P_SSLClientUtils.h"
#include "P_SSLCertLookup.h"
diff --git a/iocore/net/SSLNetVConnection.cc b/iocore/net/SSLNetVConnection.cc
index ac68ecf4a0..cee694cb51 100644
--- a/iocore/net/SSLNetVConnection.cc
+++ b/iocore/net/SSLNetVConnection.cc
@@ -28,9 +28,7 @@
#include "tscore/TSSystemState.h"
#include "api/InkAPIInternal.h" // Added to include the ssl_hook definitions
-#include "HttpTunnel.h"
#include "ProxyProtocol.h"
-#include "HttpConfig.h"
#include "SSLSNIConfig.h"
#include "P_Net.h"
diff --git a/iocore/net/SSLSNIConfig.cc b/iocore/net/SSLSNIConfig.cc
index 1124b76a2f..cce85ccc23 100644
--- a/iocore/net/SSLSNIConfig.cc
+++ b/iocore/net/SSLSNIConfig.cc
@@ -32,8 +32,6 @@
#include "SSLSNIConfig.h"
#include "P_SNIActionPerformer.h"
-#include "PreWarmManager.h"
-
#include "tscore/Diags.h"
#include "tscore/SimpleTokenizer.h"
#include "tscore/ink_memory.h"
@@ -50,6 +48,7 @@
#include <utility>
#include <pcre.h>
#include <algorithm>
+#include <functional>
static constexpr int OVECSIZE{30};
@@ -328,7 +327,8 @@ SNIConfigParams::~SNIConfigParams()
////
// SNIConfig
//
-int SNIConfig::_configid = 0;
+int SNIConfig::_configid = 0;
+std::function<void()> SNIConfig::on_reconfigure = nullptr;
void
SNIConfig::startup()
@@ -348,7 +348,9 @@ SNIConfig::reconfigure()
bool retStatus = params->initialize();
if (retStatus) {
_configid = configProcessor.set(_configid, params);
- prewarmManager.reconfigure();
+ if (SNIConfig::on_reconfigure) {
+ SNIConfig::on_reconfigure();
+ }
} else {
delete params;
}
@@ -375,6 +377,12 @@ SNIConfig::release(SNIConfigParams *params)
configProcessor.release(_configid, params);
}
+void
+SNIConfig::set_on_reconfigure_callback(std::function<void()> cb)
+{
+ SNIConfig::on_reconfigure = cb;
+}
+
// See if any of the client-side actions would trigger for this combination of
servername and client IP
// host_sni_policy is an in/out parameter. It starts with the global policy
from the records.yaml
// setting proxy.config.http.host_sni_policy and is possibly overridden if the
sni policy
diff --git a/iocore/net/SSLSNIConfig.h b/iocore/net/SSLSNIConfig.h
index 88c714a26c..deb503f1f9 100644
--- a/iocore/net/SSLSNIConfig.h
+++ b/iocore/net/SSLSNIConfig.h
@@ -43,6 +43,8 @@
#include "SNIActionPerformer.h"
#include "YamlSNIConfig.h"
+#include <functional>
+
// Properties for the next hop server
struct NextHopProperty {
std::string client_cert_file;
// full path to client cert file for lookup
@@ -126,9 +128,17 @@ public:
static SNIConfigParams *acquire();
static void release(SNIConfigParams *params);
+ /**
+ * Sets a callback to be invoked when the SNIConfig is reconfigured.
+ *
+ * This is used to reconfigure the pre-warm manager on SNI reload.
+ */
+ static void set_on_reconfigure_callback(std::function<void()> cb);
+
static bool test_client_action(const char *servername, uint16_t
dest_incoming_port, const IpEndpoint &ep,
int &enforcement_policy);
private:
static int _configid;
+ static std::function<void()> on_reconfigure;
};
diff --git a/iocore/net/TLSTunnelSupport.cc b/iocore/net/TLSTunnelSupport.cc
index 1ae0bd391b..aedfaa3987 100644
--- a/iocore/net/TLSTunnelSupport.cc
+++ b/iocore/net/TLSTunnelSupport.cc
@@ -22,8 +22,8 @@
limitations under the License.
*/
+#include "PreWarm.h"
#include "SSLTypes.h"
-#include "PreWarmManager.h"
#include "TLSTunnelSupport.h"
#include "tscore/ink_assert.h"
#include "tscore/Diags.h"
diff --git a/iocore/net/TLSTunnelSupport.h b/iocore/net/TLSTunnelSupport.h
index 813334ba35..491940e000 100644
--- a/iocore/net/TLSTunnelSupport.h
+++ b/iocore/net/TLSTunnelSupport.h
@@ -24,9 +24,10 @@
#pragma once
+#include "PreWarm.h"
+
#include <openssl/ssl.h>
-#include "PreWarmManager.h"
#include "tscore/ink_memory.h"
#include "tscore/ink_inet.h"
#include "YamlSNIConfig.h"
diff --git a/iocore/net/libinknet_stub.cc b/iocore/net/libinknet_stub.cc
index b5bf8cec33..98fce8d5ed 100644
--- a/iocore/net/libinknet_stub.cc
+++ b/iocore/net/libinknet_stub.cc
@@ -99,16 +99,6 @@ HttpRequestData::get_client_ip()
StatPagesManager statPagesManager;
-#include "PreWarmManager.h"
-void
-PreWarmManager::reconfigure()
-{
- ink_assert(false);
- return;
-}
-
-PreWarmManager prewarmManager;
-
#include "api/FetchSM.h"
ClassAllocator<FetchSM> FetchSMAllocator("unusedFetchSMAllocator");
void
diff --git a/iocore/net/unit_tests/test_SSLSNIConfig.cc
b/iocore/net/unit_tests/test_SSLSNIConfig.cc
index e7f24c7bb2..beb6a78850 100644
--- a/iocore/net/unit_tests/test_SSLSNIConfig.cc
+++ b/iocore/net/unit_tests/test_SSLSNIConfig.cc
@@ -114,3 +114,12 @@ TEST_CASE("Test SSLSNIConfig")
REQUIRE(actions.first->size() == 5); ///< three H2 config + early data +
fqdn
}
}
+
+TEST_CASE("SNIConfig reconfigure callback is invoked")
+{
+ int result{0};
+ auto set_result{[&result]() { result = 42; }};
+ SNIConfig::set_on_reconfigure_callback(set_result);
+ SNIConfig::reconfigure();
+ CHECK(result == 42);
+}
diff --git a/proxy/http/CMakeLists.txt b/proxy/http/CMakeLists.txt
index 96a8f84708..d184bb1d50 100644
--- a/proxy/http/CMakeLists.txt
+++ b/proxy/http/CMakeLists.txt
@@ -38,6 +38,7 @@ add_library(http STATIC
ConnectingEntry.cc
ForwardedConfig.cc
PreWarmConfig.cc
+ PreWarmManager.cc
)
add_library(ts::http ALIAS http)
diff --git a/proxy/http/HttpSM.h b/proxy/http/HttpSM.h
index 3934cb1217..33b54afe56 100644
--- a/proxy/http/HttpSM.h
+++ b/proxy/http/HttpSM.h
@@ -34,7 +34,6 @@
#include <string_view>
#include <optional>
-#include "TLSTunnelSupport.h"
#include "tscore/ink_platform.h"
#include "I_EventSystem.h"
#include "HttpCacheSM.h"
@@ -46,6 +45,11 @@
#include "api/InkAPIInternal.h"
#include "../ProxyTransaction.h"
#include "HdrUtils.h"
+
+// inknet
+#include "PreWarmManager.h"
+#include "TLSTunnelSupport.h"
+
#include "tscore/History.h"
#include "tscore/PendingAction.h"
@@ -68,7 +72,6 @@ static size_t const HTTP_SERVER_RESP_HDR_BUFFER_INDEX =
BUFFER_SIZE_INDEX_8K;
class PoolableSession;
class AuthHttpAdapter;
-class PreWarmSM;
class HttpSM;
using HttpSMHandler = int (HttpSM::*)(int, void *);
diff --git a/proxy/http/Makefile.am b/proxy/http/Makefile.am
index b9d3255870..28c47ad065 100644
--- a/proxy/http/Makefile.am
+++ b/proxy/http/Makefile.am
@@ -80,7 +80,8 @@ libhttp_a_SOURCES = \
HttpVCTable.cc \
HttpVCTable.h \
ForwardedConfig.cc \
- PreWarmConfig.cc
+ PreWarmConfig.cc \
+ PreWarmManager.cc
if BUILD_TESTS
libhttp_a_SOURCES += RegressionHttpTransact.cc
diff --git a/iocore/net/PreWarmManager.cc b/proxy/http/PreWarmManager.cc
similarity index 99%
rename from iocore/net/PreWarmManager.cc
rename to proxy/http/PreWarmManager.cc
index 51ee2f3eff..45fca9ac96 100644
--- a/iocore/net/PreWarmManager.cc
+++ b/proxy/http/PreWarmManager.cc
@@ -28,6 +28,7 @@
#include "SSLSNIConfig.h"
#include "P_VConnection.h"
#include "I_NetProcessor.h"
+#include "PreWarm.h"
#include "api/Metrics.h"
#include "tscore/ink_time.h"
diff --git a/iocore/net/PreWarmManager.h b/proxy/http/PreWarmManager.h
similarity index 92%
rename from iocore/net/PreWarmManager.h
rename to proxy/http/PreWarmManager.h
index cc1b866ce4..a6cd5c92de 100644
--- a/iocore/net/PreWarmManager.h
+++ b/proxy/http/PreWarmManager.h
@@ -27,43 +27,34 @@
#include "I_EventSystem.h"
#include "I_NetVConnection.h"
-#include "I_HostDB.h"
+
+// inknet
+#include "PreWarm.h"
+#include "SSLSNIConfig.h"
#include "YamlSNIConfig.h"
+
+#include "I_HostDB.h"
#include "NetTimeout.h"
#include "Milestones.h"
#include "api/Metrics.h"
-#include "records/I_RecHttp.h"
+// tscore
+#include "tscore/CryptoHash.h"
+#include "tscore/ink_hrtime.h"
+
+#include <array>
+#include <cstddef>
+#include <cstdint>
#include <map>
-#include <unordered_map>
#include <memory>
#include <queue>
-#include <string_view>
-
-class PreWarmSM;
-class PreWarmManager;
-class SNIConfigParams;
-
-extern ClassAllocator<PreWarmSM> preWarmSMAllocator;
-extern PreWarmManager prewarmManager;
+#include <string>
+#include <unordered_map>
+// PreWarm::Dst and PreWarm::SPtrConstDst are defined in iocore.
namespace PreWarm
{
-////
-// Dst
-//
-struct Dst {
- Dst(std::string_view h, in_port_t p, SNIRoutingType t, int a) : host(h),
port(p), type(t), alpn_index(a) {}
-
- std::string host;
- in_port_t port = 0;
- SNIRoutingType type = SNIRoutingType::NONE;
- int alpn_index = SessionProtocolNameRegistry::INVALID;
-};
-
-using SPtrConstDst = std::shared_ptr<const Dst>;
-
struct DstHash {
size_t
operator()(const PreWarm::SPtrConstDst &dst) const
@@ -90,9 +81,6 @@ struct DstKeyEqual {
}
};
-////
-// Conf
-//
struct Conf {
Conf(uint32_t min, int32_t max, double rate, ink_hrtime connect_timeout,
ink_hrtime inactive_timeout, bool srv_enabled,
YamlSNIConfig::Policy verify_server_policy, YamlSNIConfig::Property
verify_server_properties, const std::string &sni)
@@ -122,9 +110,6 @@ struct Conf {
using SPtrConstConf = std::shared_ptr<const Conf>;
using ParsedSNIConf = std::unordered_map<SPtrConstDst, SPtrConstConf, DstHash,
DstKeyEqual>;
-////
-// Stats
-//
enum class Stat {
INIT_LIST_SIZE = 0,
OPEN_LIST_SIZE,
@@ -139,9 +124,15 @@ enum class Stat {
using StatsIds = std::array<ts::Metrics::IntType *,
static_cast<size_t>(PreWarm::Stat::LAST_ENTRY)>;
using SPtrConstStatsIds = std::shared_ptr<const StatsIds>;
using StatsIdMap = std::unordered_map<SPtrConstDst, SPtrConstStatsIds,
DstHash, DstKeyEqual>;
-
} // namespace PreWarm
+class PreWarmSM;
+class PreWarmManager;
+class SNIConfigParams;
+
+extern ClassAllocator<PreWarmSM> preWarmSMAllocator;
+extern PreWarmManager prewarmManager;
+
/**
@class PreWarmSM
@brief A state machine to pre-warm connection
@@ -312,6 +303,16 @@ private:
class PreWarmManager
{
public:
+ PreWarmManager()
+ {
+ // We use the callback because it would introduce a circular dependency
+ // for SNIConfig to explicitly call prewarmManager.reconfigure().
+ // Because prewarmManager is global there's not a good place to set
+ // this, but as long as there's only one instance of this class
+ // there should not be an issue.
+ SNIConfig::set_on_reconfigure_callback([this]() { this->reconfigure(); });
+ }
+
static void reconfigure_prewarming_on_threads();
// Controllers
