Skip to content

Commit 38e7ce2

Browse files
authored
Deprecate quic_receive_ecn (#38100)
Commit Message: Deprecate envoy_reloadable_features_quic_receive_ecn Additional Description: N/A Risk Level: Low Testing: No new tests Docs Changes: N/A Release Notes: N/A Platform Specific Features: N/A [Optional Fixes #Issue] 38048 [Optional Deprecated:] envoy_reloadable_features_quic_receive_ecn --------- Signed-off-by: Martin Duke <martin.h.duke@gmail.com>
1 parent dc9aba8 commit 38e7ce2

File tree

8 files changed

+27
-48
lines changed

8 files changed

+27
-48
lines changed

source/common/network/io_socket_handle_impl.cc

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -412,11 +412,9 @@ Api::IoCallUint64Result IoSocketHandleImpl::recvmsg(Buffer::RawSlice* slices,
412412
}
413413
}
414414
#endif
415-
if (receive_ecn_) {
416-
absl::optional<uint8_t> maybe_tos = maybeGetTosFromHeader(*cmsg);
417-
if (maybe_tos) {
418-
output.msg_[0].tos_ = *maybe_tos;
419-
}
415+
absl::optional<uint8_t> maybe_tos = maybeGetTosFromHeader(*cmsg);
416+
if (maybe_tos) {
417+
output.msg_[0].tos_ = *maybe_tos;
420418
}
421419
}
422420
}
@@ -502,12 +500,10 @@ Api::IoCallUint64Result IoSocketHandleImpl::recvmmsg(RawSliceArrays& slices, uin
502500
output.msg_[0].saved_cmsg_ = cmsg_slice;
503501
}
504502
Address::InstanceConstSharedPtr addr = maybeGetDstAddressFromHeader(*cmsg, self_port);
505-
if (receive_ecn_) {
506-
absl::optional<uint8_t> maybe_tos = maybeGetTosFromHeader(*cmsg);
507-
if (maybe_tos) {
508-
output.msg_[0].tos_ = *maybe_tos;
509-
continue;
510-
}
503+
absl::optional<uint8_t> maybe_tos = maybeGetTosFromHeader(*cmsg);
504+
if (maybe_tos) {
505+
output.msg_[0].tos_ = *maybe_tos;
506+
continue;
511507
}
512508
if (addr != nullptr) {
513509
// This is a IP packet info message.

source/common/network/io_socket_handle_impl.h

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ class IoSocketHandleImpl : public IoSocketHandleBaseImpl {
3030
absl::optional<int> domain = absl::nullopt,
3131
size_t address_cache_max_capacity = 0)
3232
: IoSocketHandleBaseImpl(fd, socket_v6only, domain),
33-
receive_ecn_(Runtime::runtimeFeatureEnabled("envoy.reloadable_features.quic_receive_ecn")),
3433
address_cache_max_capacity_(address_cache_max_capacity) {
3534
if (address_cache_max_capacity > 0) {
3635
recent_received_addresses_ = std::vector<QuicEnvoyAddressPair>();
@@ -106,9 +105,6 @@ class IoSocketHandleImpl : public IoSocketHandleBaseImpl {
106105
const size_t cmsg_space_{CMSG_SPACE(sizeof(int)) + CMSG_SPACE(sizeof(struct in_pktinfo)) +
107106
CMSG_SPACE(sizeof(struct in6_pktinfo)) + CMSG_SPACE(sizeof(uint16_t))};
108107

109-
// Latches a copy of the runtime feature "envoy.reloadable_features.quic_receive_ecn".
110-
const bool receive_ecn_;
111-
112108
size_t addressCacheMaxSize() const { return address_cache_max_capacity_; }
113109

114110
private:

source/common/quic/active_quic_listener.cc

Lines changed: 16 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ ActiveQuicListener::ActiveQuicListener(
3333
Network::SocketSharedPtr&& listen_socket, Network::ListenerConfig& listener_config,
3434
const quic::QuicConfig& quic_config, bool kernel_worker_routing,
3535
const envoy::config::core::v3::RuntimeFeatureFlag& enabled, QuicStatNames& quic_stat_names,
36-
uint32_t packets_to_read_to_connection_count_ratio, bool receive_ecn,
36+
uint32_t packets_to_read_to_connection_count_ratio,
3737
EnvoyQuicCryptoServerStreamFactoryInterface& crypto_server_stream_factory,
3838
EnvoyQuicProofSourceFactoryInterface& proof_source_factory,
3939
QuicConnectionIdGeneratorPtr&& cid_generator, QuicConnectionIdWorkerSelector worker_selector,
@@ -72,25 +72,22 @@ ActiveQuicListener::ActiveQuicListener(
7272
auto alarm_factory =
7373
std::make_unique<EnvoyQuicAlarmFactory>(dispatcher_, *connection_helper->GetClock());
7474
// Set the socket to report incoming ECN.
75-
if (receive_ecn) {
76-
if (udp_listener_->localAddress() == nullptr ||
77-
udp_listener_->localAddress()->ip() == nullptr) {
78-
IS_ENVOY_BUG("UDP Listener does not have local IP address");
79-
} else {
80-
int optval = 1;
81-
socklen_t optlen = sizeof(optval);
82-
if (udp_listener_->localAddress()->ip()->ipv6() != nullptr) {
83-
listen_socket_.setSocketOption(IPPROTO_IPV6, IPV6_RECVTCLASS, &optval, optlen);
75+
if (udp_listener_->localAddress() == nullptr || udp_listener_->localAddress()->ip() == nullptr) {
76+
IS_ENVOY_BUG("UDP Listener does not have local IP address");
77+
} else {
78+
int optval = 1;
79+
socklen_t optlen = sizeof(optval);
80+
if (udp_listener_->localAddress()->ip()->ipv6() != nullptr) {
81+
listen_socket_.setSocketOption(IPPROTO_IPV6, IPV6_RECVTCLASS, &optval, optlen);
8482
#ifndef __APPLE__
85-
// Linux dual-stack sockets require setting IP_RECVTOS separately. Apple
86-
// sockets will return an error.
87-
if (!udp_listener_->localAddress()->ip()->ipv6()->v6only()) {
88-
listen_socket_.setSocketOption(IPPROTO_IP, IP_RECVTOS, &optval, optlen);
89-
}
90-
#endif // __APPLE__
91-
} else {
83+
// Linux dual-stack sockets require setting IP_RECVTOS separately. Apple
84+
// sockets will return an error.
85+
if (!udp_listener_->localAddress()->ip()->ipv6()->v6only()) {
9286
listen_socket_.setSocketOption(IPPROTO_IP, IP_RECVTOS, &optval, optlen);
9387
}
88+
#endif // __APPLE__
89+
} else {
90+
listen_socket_.setSocketOption(IPPROTO_IP, IP_RECVTOS, &optval, optlen);
9491
}
9592
}
9693
quic_dispatcher_ = std::make_unique<EnvoyQuicDispatcher>(
@@ -266,7 +263,6 @@ ActiveQuicListenerFactory::ActiveQuicListenerFactory(
266263
packets_to_read_to_connection_count_ratio_(
267264
PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, packets_to_read_to_connection_count_ratio,
268265
DEFAULT_PACKETS_TO_READ_PER_CONNECTION)),
269-
receive_ecn_(Runtime::runtimeFeatureEnabled("envoy.reloadable_features.quic_receive_ecn")),
270266
context_(context), reject_new_connections_(config.reject_new_connections()) {
271267
const int64_t idle_network_timeout_ms =
272268
config.has_idle_timeout() ? DurationUtil::durationToMilliseconds(config.idle_timeout())
@@ -436,8 +432,8 @@ ActiveQuicListenerFactory::createActiveQuicListener(
436432
return std::make_unique<ActiveQuicListener>(
437433
runtime, worker_index, concurrency, dispatcher, parent, std::move(listen_socket),
438434
listener_config, quic_config, kernel_worker_routing, enabled, quic_stat_names,
439-
packets_to_read_to_connection_count_ratio, receive_ecn_, crypto_server_stream_factory,
440-
proof_source_factory, std::move(cid_generator), worker_selector_,
435+
packets_to_read_to_connection_count_ratio, crypto_server_stream_factory, proof_source_factory,
436+
std::move(cid_generator), worker_selector_,
441437
makeOptRefFromPtr(connection_debug_visitor_factory_.get()), reject_new_connections_);
442438
}
443439

source/common/quic/active_quic_listener.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ class ActiveQuicListener : public Envoy::Server::ActiveUdpListenerBase,
3636
bool kernel_worker_routing,
3737
const envoy::config::core::v3::RuntimeFeatureFlag& enabled,
3838
QuicStatNames& quic_stat_names,
39-
uint32_t packets_to_read_to_connection_count_ratio, bool receive_ecn,
39+
uint32_t packets_to_read_to_connection_count_ratio,
4040
EnvoyQuicCryptoServerStreamFactoryInterface& crypto_server_stream_factory,
4141
EnvoyQuicProofSourceFactoryInterface& proof_source_factory,
4242
QuicConnectionIdGeneratorPtr&& cid_generator,
@@ -155,7 +155,6 @@ class ActiveQuicListenerFactory : public Network::ActiveUdpListenerFactory,
155155
envoy::config::core::v3::RuntimeFeatureFlag enabled_;
156156
QuicStatNames& quic_stat_names_;
157157
const uint32_t packets_to_read_to_connection_count_ratio_;
158-
bool receive_ecn_;
159158
const Network::Socket::OptionsSharedPtr options_{std::make_shared<Network::Socket::Options>()};
160159
QuicConnectionIdWorkerSelector worker_selector_;
161160
bool kernel_worker_routing_{};

source/common/quic/envoy_quic_utils.cc

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -217,9 +217,7 @@ createConnectionSocket(const Network::Address::InstanceConstSharedPtr& peer_addr
217217
}
218218
connection_socket->addOptions(Network::SocketOptionFactory::buildIpPacketInfoOptions());
219219
connection_socket->addOptions(Network::SocketOptionFactory::buildRxQueueOverFlowOptions());
220-
if (Runtime::runtimeFeatureEnabled("envoy.reloadable_features.quic_receive_ecn")) {
221-
connection_socket->addOptions(Network::SocketOptionFactory::buildIpRecvTosOptions());
222-
}
220+
connection_socket->addOptions(Network::SocketOptionFactory::buildIpRecvTosOptions());
223221
if (prefer_gro && Api::OsSysCallsSingleton::get().supportsUdpGro()) {
224222
connection_socket->addOptions(Network::SocketOptionFactory::buildUdpGroOptions());
225223
}

source/common/runtime/runtime_features.cc

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,6 @@ RUNTIME_GUARD(envoy_reloadable_features_proxy_104);
8080
RUNTIME_GUARD(envoy_reloadable_features_proxy_ssl_port);
8181
RUNTIME_GUARD(envoy_reloadable_features_proxy_status_mapping_more_core_response_flags);
8282
RUNTIME_GUARD(envoy_reloadable_features_quic_connect_client_udp_sockets);
83-
RUNTIME_GUARD(envoy_reloadable_features_quic_receive_ecn);
8483
// Ignore the automated "remove this flag" issue: we should keep this for 1 year. Confirm with
8584
// @danzh2010 or @RyanTheOptimist before removing.
8685
RUNTIME_GUARD(envoy_reloadable_features_quic_send_server_preferred_address_to_all_clients);

test/common/quic/active_quic_listener_test.cc

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -89,9 +89,8 @@ class TestActiveQuicListenerFactory : public ActiveQuicListenerFactory {
8989
return std::make_unique<TestActiveQuicListener>(
9090
runtime, worker_index, concurrency, dispatcher, parent, std::move(listen_socket),
9191
listener_config, quic_config, kernel_worker_routing, enabled, quic_stat_names,
92-
packets_to_read_to_connection_count_ratio, /*receive_ecn=*/true,
93-
crypto_server_stream_factory, proof_source_factory, std::move(cid_generator),
94-
testWorkerSelector, std::nullopt);
92+
packets_to_read_to_connection_count_ratio, crypto_server_stream_factory,
93+
proof_source_factory, std::move(cid_generator), testWorkerSelector, std::nullopt);
9594
}
9695
};
9796

@@ -141,7 +140,6 @@ class ActiveQuicListenerTest : public testing::TestWithParam<Network::Address::I
141140
}
142141

143142
void SetUp() override {
144-
Runtime::maybeSetRuntimeGuard("envoy.reloadable_features.quic_receive_ecn", true);
145143
envoy::config::bootstrap::v3::LayeredRuntime config;
146144
config.add_layers()->mutable_admin_layer();
147145
listen_socket_ =
@@ -647,7 +645,6 @@ TEST_P(ActiveQuicListenerTest, EcnReportingIsEnabled) {
647645
}
648646

649647
TEST_P(ActiveQuicListenerTest, EcnReporting) {
650-
Runtime::maybeSetRuntimeGuard("envoy.reloadable_features.quic_receive_ecn", true);
651648
initialize();
652649
maybeConfigureMocks(/* connection_count = */ 1);
653650
quic::QuicConnectionId connection_id = quic::test::TestConnectionId(1);
@@ -665,7 +662,6 @@ TEST_P(ActiveQuicListenerTest, EcnReportingDualStack) {
665662
if (local_address_->ip()->version() == Network::Address::IpVersion::v4) {
666663
return;
667664
}
668-
Runtime::maybeSetRuntimeGuard("envoy.reloadable_features.quic_receive_ecn", true);
669665
initialize();
670666
maybeConfigureMocks(/* connection_count = */ 1);
671667
quic::QuicConnectionId connection_id = quic::test::TestConnectionId(1);

test/common/quic/envoy_quic_client_session_test.cc

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,6 @@ class EnvoyQuicClientSessionTest : public testing::TestWithParam<quic::ParsedQui
126126
}
127127

128128
void SetUp() override {
129-
Runtime::maybeSetRuntimeGuard("envoy.reloadable_features.quic_receive_ecn", true);
130129
quic_connection_ = new TestEnvoyQuicClientConnection(
131130
quic::test::TestConnectionId(), connection_helper_, alarm_factory_, writer_, quic_version_,
132131
*dispatcher_, createConnectionSocket(peer_addr_, self_addr_, nullptr, /*prefer_gro=*/true),

0 commit comments

Comments
 (0)