Skip to content

Commit 9b4e465

Browse files
authored
[mobile]Create a real dummy socket option to manage network type hashing (#38099)
Commit Message: Create a real dummy socket option to manage network type hashing Additional Description: The previous approach uses IP_TTL and it might cause unwanted behavior in production. Risk Level: low Testing: existing tests Docs Changes: n/a Release Notes: n/a Platform Specific Features: mobile only Runtime guard: envoy_reloadable_features_use_network_type_socket_option --------- Signed-off-by: Renjie Tang <renjietang@google.com>
1 parent 8ded2fe commit 9b4e465

File tree

6 files changed

+117
-4
lines changed

6 files changed

+117
-4
lines changed

mobile/library/common/network/BUILD

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ envoy_cc_library(
1414
],
1515
repository = "@envoy",
1616
deps = [
17+
":network_type_socket_option_lib",
1718
":proxy_settings_lib",
1819
"//library/common:engine_types_lib",
1920
"//library/common/network:src_addr_socket_option_lib",
@@ -54,6 +55,18 @@ envoy_cc_library(
5455
],
5556
)
5657

58+
envoy_cc_library(
59+
name = "network_type_socket_option_lib",
60+
srcs = ["network_type_socket_option_impl.cc"],
61+
hdrs = ["network_type_socket_option_impl.h"],
62+
repository = "@envoy",
63+
deps = [
64+
"@envoy//source/common/common:scalar_to_byte_vector_lib",
65+
"@envoy//source/common/network:socket_option_lib",
66+
"@envoy_api//envoy/config/core/v3:pkg_cc_proto",
67+
],
68+
)
69+
5770
envoy_cc_library(
5871
name = "synthetic_address_lib",
5972
hdrs = ["synthetic_address_impl.h"],

mobile/library/common/network/connectivity_manager.cc

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#include "source/extensions/common/dynamic_forward_proxy/dns_cache_manager_impl.h"
1414

1515
#include "fmt/ostream.h"
16+
#include "library/common/network/network_type_socket_option_impl.h"
1617
#include "library/common/network/src_addr_socket_option_impl.h"
1718

1819
// Used on Linux (requires root/CAP_NET_RAW)
@@ -308,11 +309,15 @@ Socket::OptionsSharedPtr ConnectivityManagerImpl::getUpstreamSocketOptions(int n
308309
// Envoy uses the hash signature of overridden socket options to choose a connection pool.
309310
// Setting a dummy socket option is a hack that allows us to select a different
310311
// connection pool without materially changing the socket configuration.
311-
int ttl_value = DEFAULT_IP_TTL + static_cast<int>(network);
312312
auto options = std::make_shared<Socket::Options>();
313-
options->push_back(std::make_shared<AddrFamilyAwareSocketOptionImpl>(
314-
envoy::config::core::v3::SocketOption::STATE_PREBIND, ENVOY_SOCKET_IP_TTL,
315-
ENVOY_SOCKET_IPV6_UNICAST_HOPS, ttl_value));
313+
if (!Runtime::runtimeFeatureEnabled("envoy.reloadable_features.use_network_type_socket_option")) {
314+
options->push_back(std::make_shared<AddrFamilyAwareSocketOptionImpl>(
315+
envoy::config::core::v3::SocketOption::STATE_PREBIND, ENVOY_SOCKET_IP_TTL,
316+
ENVOY_SOCKET_IPV6_UNICAST_HOPS, DEFAULT_IP_TTL + static_cast<int>(network)));
317+
} else {
318+
options->push_back(std::make_shared<NetworkTypeSocketOptionImpl>(network));
319+
}
320+
316321
return options;
317322
}
318323

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
#include "library/common/network/network_type_socket_option_impl.h"
2+
3+
#include "envoy/config/core/v3/base.pb.h"
4+
#include "envoy/network/socket.h"
5+
6+
#include "source/common/common/scalar_to_byte_vector.h"
7+
8+
#include "absl/types/optional.h"
9+
10+
namespace Envoy {
11+
namespace Network {
12+
13+
NetworkTypeSocketOptionImpl::NetworkTypeSocketOptionImpl(int network_type)
14+
: optname_(0, 0, "network_type"), network_type_(network_type) {}
15+
16+
bool NetworkTypeSocketOptionImpl::setOption(
17+
Socket& /*socket*/, envoy::config::core::v3::SocketOption::SocketState /*state*/) const {
18+
return true;
19+
}
20+
21+
void NetworkTypeSocketOptionImpl::hashKey(std::vector<uint8_t>& hash_key) const {
22+
pushScalarToByteVector(network_type_, hash_key);
23+
}
24+
25+
absl::optional<Socket::Option::Details> NetworkTypeSocketOptionImpl::getOptionDetails(
26+
const Socket&, envoy::config::core::v3::SocketOption::SocketState /*state*/) const {
27+
Socket::Option::Details details;
28+
details.name_ = optname_;
29+
std::vector<uint8_t> data;
30+
pushScalarToByteVector(network_type_, data);
31+
details.value_ = std::string(reinterpret_cast<char*>(data.data()), data.size());
32+
return absl::make_optional(std::move(details));
33+
}
34+
35+
bool NetworkTypeSocketOptionImpl::isSupported() const { return true; }
36+
37+
} // namespace Network
38+
} // namespace Envoy
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
#pragma once
2+
3+
#include "envoy/config/core/v3/base.pb.h"
4+
#include "envoy/network/socket.h"
5+
6+
#include "absl/types/optional.h"
7+
8+
namespace Envoy {
9+
namespace Network {
10+
11+
/**
12+
* This is a "dummy" socket option implementation, which does not actually
13+
* set any socket option, but rather applies a network type tag to the socket so that requests from
14+
* different network types gets hashed to different connections.
15+
*/
16+
class NetworkTypeSocketOptionImpl : public Network::Socket::Option {
17+
public:
18+
NetworkTypeSocketOptionImpl(int network_type);
19+
20+
// Socket::Option
21+
bool setOption(Network::Socket& socket,
22+
envoy::config::core::v3::SocketOption::SocketState state) const override;
23+
void hashKey(std::vector<uint8_t>& hash_key) const override;
24+
absl::optional<Details>
25+
getOptionDetails(const Network::Socket& socket,
26+
envoy::config::core::v3::SocketOption::SocketState state) const override;
27+
bool isSupported() const override;
28+
29+
private:
30+
const Network::SocketOptionName optname_;
31+
32+
int network_type_;
33+
};
34+
35+
} // namespace Network
36+
} // namespace Envoy

mobile/test/common/network/connectivity_manager_test.cc

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -242,5 +242,23 @@ TEST_F(ConnectivityManagerTest, IgnoresDuplicatedProxySettingsUpdates) {
242242
EXPECT_EQ(proxy_settings1, connectivity_manager_->getProxySettings());
243243
}
244244

245+
TEST_F(ConnectivityManagerTest, NetworkChangeResultsInDifferentSocketOptionsHash) {
246+
Runtime::maybeSetRuntimeGuard("envoy.reloadable_features.use_network_type_socket_option", true);
247+
auto options1 = std::make_shared<Socket::Options>();
248+
connectivity_manager_->addUpstreamSocketOptions(options1);
249+
std::vector<uint8_t> hash1;
250+
for (const auto& option : *options1) {
251+
option->hashKey(hash1);
252+
}
253+
ConnectivityManagerImpl::setPreferredNetwork(64);
254+
auto options2 = std::make_shared<Socket::Options>();
255+
connectivity_manager_->addUpstreamSocketOptions(options2);
256+
std::vector<uint8_t> hash2;
257+
for (const auto& option : *options2) {
258+
option->hashKey(hash2);
259+
}
260+
EXPECT_NE(hash1, hash2);
261+
}
262+
245263
} // namespace Network
246264
} // namespace Envoy

source/common/runtime/runtime_features.cc

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,9 @@ FALSE_RUNTIME_GUARD(envoy_reloadable_features_google_grpc_disable_tls_13);
167167
// before downstream.
168168
FALSE_RUNTIME_GUARD(envoy_reloadable_features_allow_multiplexed_upstream_half_close);
169169

170+
// TODO(renjietang): Flip to true after prod testing.
171+
FALSE_RUNTIME_GUARD(envoy_reloadable_features_use_network_type_socket_option);
172+
170173
// Block of non-boolean flags. Use of int flags is deprecated. Do not add more.
171174
ABSL_FLAG(uint64_t, re2_max_program_size_error_level, 100, ""); // NOLINT
172175
ABSL_FLAG(uint64_t, re2_max_program_size_warn_level, // NOLINT

0 commit comments

Comments
 (0)