Skip to content

Commit

Permalink
envoy: Patch original_dst_cluster
Browse files Browse the repository at this point in the history
Patch original destination cluster to avoid multiple hosts for the same
address.

Connection pool containers use HostSharedPtr as map keys, rather than the
address of the host. This leads to multiple connections when there are
multiple Host instances for the same address. This is breaking use of the
original source address and port for upstream connections since only one
such connection can exist at any one time.

Original destination cluster implementation creates such duplicate Host
instances when two worker threads are racing to create a Host for the
same destination at the same time.

Fix this by keeping a separate 'updates_map' where each worker places a
newly created Host for the original destination. This map is used to look
for the Host is it can not be found from the shared read-only
'host_map'. Access to 'updates_map' is syncronized so that it can be
safely shared by the worker threads. The main threads consolidates the
updates from the 'updates_map' to a new instance of the shared, read-only
hosts map, so that the workers do not need to stall for possibly large
map updates.

Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
  • Loading branch information
jrajahalme authored and sayboras committed Jun 3, 2024
1 parent c4bf82e commit 90249c0
Show file tree
Hide file tree
Showing 8 changed files with 486 additions and 53 deletions.
1 change: 1 addition & 0 deletions WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ git_repository(
# This patch is needed to fix the build with clang for envoy 1.29+
# https://github.com/envoyproxy/envoy/pull/31894
"@//patches:0005-Patch-cel-cpp-to-not-break-build.patch",
"@//patches:0006-original_dst_cluster-Avoid-multiple-hosts-for-the-sa.patch",
],
# // clang-format off: Envoy's format check: Only repository_locations.bzl may contains URL references
remote = "https://github.com/envoyproxy/envoy.git",
Expand Down
12 changes: 9 additions & 3 deletions cilium/socket_option.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#include "envoy/config/core/v3/base.pb.h"
#include "envoy/network/listen_socket.h"

#include "source/common/common/hex.h"
#include "source/common/common/logger.h"
#include "source/common/common/utility.h"

Expand Down Expand Up @@ -189,9 +190,14 @@ class SocketMarkOption : public Network::Socket::Option,
absl::uint128 raw_address = ip->ipv6()->address();
addressIntoVector(key, raw_address);
}
// Add source port to the hash key
key.emplace_back(uint8_t(port >> 16));
key.emplace_back(uint8_t(port));
// Add source port to the hash key if defined
if (port != 0) {
ENVOY_LOG(trace, "hashKey port: {:x}", port);
key.emplace_back(uint8_t(port >> 8));
key.emplace_back(uint8_t(port));
}
ENVOY_LOG(trace, "hashKey after Cilium: {}, source: {}", Hex::encode(key),
original_source_address_->asString());
} else {
// Add the source identity to the hash key. This will separate upstream
// connection pools per security ID.
Expand Down
21 changes: 2 additions & 19 deletions patches/0001-network-Add-callback-for-upstream-authorization.patch
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
From 0d3d01a4d6918efc629716913b3e43dc3f129836 Mon Sep 17 00:00:00 2001
From 527272bbdfb624250c0cf5bc5e7eae219126f3b8 Mon Sep 17 00:00:00 2001
From: Jarno Rajahalme <jarno@isovalent.com>
Date: Mon, 24 Jan 2022 15:40:28 +0200
Subject: [PATCH 1/6] network: Add callback for upstream authorization
Expand All @@ -25,23 +25,6 @@ done from the tcp_proxy or router filter in the same filter chain.
Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>

Signed-off-by: Tam Mach <sayboras@yahoo.com>
---
envoy/http/filter.h | 8 ++++++
envoy/network/filter.h | 28 +++++++++++++++++++++
envoy/tcp/upstream.h | 5 ++++
source/common/http/async_client_impl.h | 5 ++++
source/common/http/conn_manager_impl.h | 6 +++++
source/common/http/filter_manager.cc | 6 +++++
source/common/http/filter_manager.h | 10 +++++++-
source/common/network/filter_manager_impl.h | 21 ++++++++++++++++
source/common/router/router.cc | 8 ++++++
source/common/router/upstream_request.h | 5 ++++
source/common/tcp_proxy/tcp_proxy.cc | 7 ++++++
source/common/tcp_proxy/tcp_proxy.h | 1 +
source/common/tcp_proxy/upstream.cc | 8 ++++++
source/common/tcp_proxy/upstream.h | 2 ++
source/server/api_listener_impl.h | 3 +++
15 files changed, 122 insertions(+), 1 deletion(-)

diff --git a/envoy/http/filter.h b/envoy/http/filter.h
index e250b3ab66..7bc9480ac6 100644
Expand Down Expand Up @@ -373,5 +356,5 @@ index 5ac8a3b7c0..f27bd198f6 100644
// Synthetic class that acts as a stub for the connection backing the
// Network::ReadFilterCallbacks.
--
2.44.0
2.45.0

Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
From 0d5c620f92cc6ff8cd492094944495be8c4f05a2 Mon Sep 17 00:00:00 2001
From d5069995b4a4a0f9da3de3cda5271820532cfa16 Mon Sep 17 00:00:00 2001
From: Jarno Rajahalme <jarno@isovalent.com>
Date: Mon, 8 Apr 2024 15:21:08 +0200
Subject: [PATCH 2/6] tcp_proxy: Add filter state proxy_read_before_connect
Expand Down Expand Up @@ -29,13 +29,6 @@ reliant on the tcp_proxy internal detail, such as balancing and timing of
the readDisable() calls on the downstream connection.

Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
---
source/common/tcp_proxy/BUILD | 1 +
source/common/tcp_proxy/tcp_proxy.cc | 34 ++++++++--
source/common/tcp_proxy/tcp_proxy.h | 6 ++
test/integration/BUILD | 1 +
.../integration/tcp_proxy_integration_test.cc | 68 +++++++++++++------
5 files changed, 84 insertions(+), 26 deletions(-)

diff --git a/source/common/tcp_proxy/BUILD b/source/common/tcp_proxy/BUILD
index 07d5c5995d..75f9047969 100644
Expand Down Expand Up @@ -296,5 +289,5 @@ index 90ad072b81..95a6b1cf80 100644

INSTANTIATE_TEST_SUITE_P(TcpProxyIntegrationTestParams, TcpProxySslIntegrationTest,
--
2.44.0
2.45.0

11 changes: 2 additions & 9 deletions patches/0003-listener-add-socket-options.patch
Original file line number Diff line number Diff line change
@@ -1,18 +1,11 @@
From 4f3ed287031b7fac68c73e9d18215b5010ed19e5 Mon Sep 17 00:00:00 2001
From 6535bcc53e1289465a36f87f418c05a30e8730f3 Mon Sep 17 00:00:00 2001
From: Jarno Rajahalme <jarno@isovalent.com>
Date: Mon, 14 Aug 2023 10:01:21 +0300
Subject: [PATCH 3/6] listener: add socket options

This reverts commit 170c89eb0b2afb7a39d44d0f8dfb77444ffc038f.

Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
---
envoy/server/factory_context.h | 8 +++++++-
source/common/listener_manager/listener_impl.cc | 3 +++
source/common/listener_manager/listener_impl.h | 9 +++++++++
test/mocks/server/factory_context.h | 1 +
test/mocks/server/listener_factory_context.h | 1 +
5 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/envoy/server/factory_context.h b/envoy/server/factory_context.h
index 8a139fbcb0..e9c0c25a99 100644
Expand Down Expand Up @@ -99,5 +92,5 @@ index 10fbe1e217..b05c37c36a 100644
MOCK_METHOD(TransportSocketFactoryContext&, getTransportSocketFactoryContext, (), (const));
MOCK_METHOD(const Network::DrainDecision&, drainDecision, ());
--
2.44.0
2.45.0

9 changes: 3 additions & 6 deletions patches/0004-factory_base-Backport-is_upstream.patch
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
From 5eab57efac5beb0f2e6edb4b2aaf7493b03c532b Mon Sep 17 00:00:00 2001
From a940e129f8dfedd434961509de351a24bf68c29e Mon Sep 17 00:00:00 2001
From: Jarno Rajahalme <jarno@isovalent.com>
Date: Thu, 29 Feb 2024 13:21:20 +0100
Subject: [PATCH 6/6] factory_base: Backport "is_upstream"
Subject: [PATCH 4/6] factory_base: Backport "is_upstream"

Upstream commit 26a4eb87428dbf3a39ff4f6f61b69538c34d07b6 introduced
'is_upstream' field in 'DualInfo' that allows filter operation to know if
Expand All @@ -14,9 +14,6 @@ version bump with the upstream commit
26a4eb87428dbf3a39ff4f6f61b69538c34d07b6.

Signed-off-by: Jarno Rajahalme <jarno@isovalent.com>
---
source/extensions/filters/http/common/factory_base.h | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/source/extensions/filters/http/common/factory_base.h b/source/extensions/filters/http/common/factory_base.h
index ae0288c8e1..3773237ae8 100644
Expand All @@ -38,5 +35,5 @@ index ae0288c8e1..3773237ae8 100644

absl::StatusOr<Envoy::Http::FilterFactoryCb>
--
2.44.0
2.45.0

9 changes: 2 additions & 7 deletions patches/0005-Patch-cel-cpp-to-not-break-build.patch
Original file line number Diff line number Diff line change
@@ -1,14 +1,9 @@
From 1ac8bd0dbc9ce72e13fa1bab42ba5511cfa9c4ac Mon Sep 17 00:00:00 2001
From bf362edf3af51a2df5de26a9c5f472c69b685e8b Mon Sep 17 00:00:00 2001
From: Raven Black <ravenblack@dropbox.com>
Date: Thu, 18 Jan 2024 16:34:15 +0000
Subject: [PATCH 5/6] Patch cel-cpp to not break build

Signed-off-by: Raven Black <ravenblack@dropbox.com>
---
bazel/cel-cpp-memory.patch | 44 ++++++++++++++++++++++++++++++++++++++
bazel/repositories.bzl | 5 ++++-
2 files changed, 48 insertions(+), 1 deletion(-)
create mode 100644 bazel/cel-cpp-memory.patch

diff --git a/bazel/cel-cpp-memory.patch b/bazel/cel-cpp-memory.patch
new file mode 100644
Expand Down Expand Up @@ -77,5 +72,5 @@ index aa93c9c838..3220aeb2ec 100644
)

--
2.44.0
2.45.0

Loading

0 comments on commit 90249c0

Please sign in to comment.