Skip to content

Commit 546a4ee

Browse files
fishpan1209fishpan1209
authored
quic: decouple UDP and QUIC packet writers for direct creation in ActiveQuicListener (#45735)
…iveQuicListener <!-- !!!ATTENTION!!! If you are fixing **any** crash or **any** potential security issue, **do not open a pull request**. Instead, please [open a GitHub Security Advisory](https://github.com/envoyproxy/envoy/security/advisories/new) (preferred). Alternatively, you may email envoy-security@googlegroups.com. Thank you in advance for helping to keep Envoy secure. !!!ATTENTION!!! For an explanation of how to fill out the fields, please see the relevant section in [PULL_REQUESTS.md](https://github.com/envoyproxy/envoy/blob/main/PULL_REQUESTS.md) !!!ATTENTION!!! Please check the [use of generative AI policy](https://github.com/envoyproxy/envoy/blob/main/CONTRIBUTING.md?plain=1#L41). You may use generative AI only if you fully understand the code. You need to disclose this usage in the PR description to ensure transparency. --> Commit Message: quic: decouple UDP and QUIC packet writers for direct creation in ActiveQuicListener Additional Description: This change separates the `UdpPacketWriter` and `QuicPacketWriter` interfaces, enabling the direct creation of `QuicPacketWriter` within `ActiveQuicListener`. By decoupling these writers, the system can bypass the `EnvoyQuicPacketWriter` wrapper when a custom writer factory is available, improving integration efficiency. Risk Level: low Testing: unit tests Docs Changes: Release Notes: Platform Specific Features: [Optional Runtime guard:] [Optional Fixes #Issue] [Optional Fixes commit #PR or SHA] [Optional Deprecated:] [Optional [API Considerations](https://github.com/envoyproxy/envoy/blob/main/api/review_checklist.md):] --------- Signed-off-by: Ting Pan <panting@google.com>
1 parent ff1664d commit 546a4ee

14 files changed

Lines changed: 254 additions & 15 deletions

File tree

‎envoy/network/listener.h‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,14 @@ class ListenSocketFactory {
8383
virtual absl::Status doFinalPreWorkerInit() PURE;
8484
};
8585

86+
} // namespace Network
87+
88+
namespace Quic {
89+
class QuicPacketWriterFactory;
90+
} // namespace Quic
91+
92+
namespace Network {
93+
8694
/**
8795
* Configuration for a UDP listener.
8896
*/
@@ -100,6 +108,11 @@ class UdpListenerConfig {
100108
*/
101109
virtual UdpPacketWriterFactory& packetWriterFactory() PURE;
102110

111+
/**
112+
* @return factory for creating QUIC packet writers.
113+
*/
114+
virtual Quic::QuicPacketWriterFactory* quicPacketWriterFactory() PURE;
115+
103116
/**
104117
* @param address is used to query the address specific router.
105118
* @return the UdpListenerWorkerRouter for this listener.

‎source/common/listener_manager/BUILD‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ envoy_cc_library(
7777
] + envoy_select_enable_http3([
7878
"//source/common/quic:active_quic_listener_lib",
7979
"//source/common/quic:client_connection_factory_lib",
80+
"//source/common/quic:quic_packet_writer_interface",
8081
"//source/common/quic:quic_server_factory_lib",
8182
"//source/common/quic:quic_server_transport_socket_factory_lib",
8283
"//source/common/quic:quic_transport_socket_factory_lib",

‎source/common/listener_manager/listener_impl.cc‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@
4040

4141
#ifdef ENVOY_ENABLE_QUIC
4242
#include "source/common/quic/active_quic_listener.h"
43+
#include "source/common/quic/envoy_quic_packet_writer.h"
44+
#include "source/common/quic/quic_packet_writer_interface.h"
4345
#include "source/common/quic/udp_gso_batch_writer.h"
4446
#endif
4547

@@ -714,6 +716,18 @@ ListenerImpl::buildUdpListenerFactory(const envoy::config::listener::v3::Listene
714716
udp_listener_config_->listener_factory_ = std::make_unique<Quic::ActiveQuicListenerFactory>(
715717
config.udp_listener_config().quic_options(), concurrency, quic_stat_names_,
716718
validation_visitor_, *listener_factory_context_);
719+
720+
if (config.udp_listener_config().has_udp_packet_packet_writer_config()) {
721+
auto* quic_packet_writer_factory_factory =
722+
Config::Utility::getFactory<Quic::QuicPacketWriterFactoryFactory>(
723+
config.udp_listener_config().udp_packet_packet_writer_config());
724+
if (quic_packet_writer_factory_factory != nullptr) {
725+
udp_listener_config_->quic_writer_factory_ =
726+
quic_packet_writer_factory_factory->createQuicPacketWriterFactory(
727+
config.udp_listener_config().udp_packet_packet_writer_config(),
728+
*listener_factory_context_);
729+
}
730+
}
717731
#if UDP_GSO_BATCH_WRITER_COMPILETIME_SUPPORT
718732
// TODO(mattklein123): We should be able to use GSO without QUICHE/QUIC. Right now this causes
719733
// non-QUIC integration tests to fail, which I haven't investigated yet. Additionally, from

‎source/common/listener_manager/listener_impl.h‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,9 @@
2222
#include "source/common/init/target_impl.h"
2323
#include "source/common/listener_manager/filter_chain_manager_impl.h"
2424
#include "source/common/listener_manager/listener_info_impl.h"
25+
#ifdef ENVOY_ENABLE_QUIC
26+
#include "source/common/quic/quic_packet_writer_interface.h"
27+
#endif
2528
#include "source/common/quic/quic_stat_names.h"
2629
#include "source/server/factory_context_impl.h"
2730
#include "source/server/transport_socket_config_impl.h"
@@ -374,6 +377,13 @@ class ListenerImpl final : public Network::ListenerConfig,
374377
// Network::UdpListenerConfig
375378
Network::ActiveUdpListenerFactory& listenerFactory() override { return *listener_factory_; }
376379
Network::UdpPacketWriterFactory& packetWriterFactory() override { return *writer_factory_; }
380+
#ifdef ENVOY_ENABLE_QUIC
381+
Envoy::Quic::QuicPacketWriterFactory* quicPacketWriterFactory() override {
382+
return quic_writer_factory_.get();
383+
}
384+
#else
385+
Envoy::Quic::QuicPacketWriterFactory* quicPacketWriterFactory() override { return nullptr; }
386+
#endif
377387
Network::UdpListenerWorkerRouter&
378388
listenerWorkerRouter(const Network::Address::Instance& address) override {
379389
auto iter = listener_worker_routers_.find(address.asString());
@@ -385,6 +395,9 @@ class ListenerImpl final : public Network::ListenerConfig,
385395
const envoy::config::listener::v3::UdpListenerConfig config_;
386396
Network::ActiveUdpListenerFactoryPtr listener_factory_;
387397
Network::UdpPacketWriterFactoryPtr writer_factory_;
398+
#ifdef ENVOY_ENABLE_QUIC
399+
Quic::QuicPacketWriterFactoryPtr quic_writer_factory_;
400+
#endif
388401
absl::flat_hash_map<std::string, Network::UdpListenerWorkerRouterPtr> listener_worker_routers_;
389402
};
390403

‎source/common/quic/BUILD‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -503,6 +503,7 @@ envoy_cc_library(
503503
":envoy_quic_proof_source_lib",
504504
":envoy_quic_server_preferred_address_config_factory_interface",
505505
":envoy_quic_utils_lib",
506+
":quic_packet_writer_interface",
506507
"//envoy/network:listener_interface",
507508
"//source/common/network:listener_lib",
508509
"//source/common/protobuf:utility_lib",
@@ -633,6 +634,22 @@ envoy_cc_library(
633634
hdrs = envoy_select_enable_http3(["envoy_quic_packet_writer.h"]),
634635
deps = envoy_select_enable_http3([
635636
":envoy_quic_utils_lib",
637+
"//envoy/network:udp_packet_writer_handler_interface",
638+
"@quiche//:quic_core_packet_writer_lib",
639+
"@quiche//:quic_platform",
640+
]),
641+
)
642+
643+
envoy_cc_library(
644+
name = "quic_packet_writer_interface",
645+
hdrs = envoy_select_enable_http3(["quic_packet_writer_interface.h"]),
646+
deps = envoy_select_enable_http3([
647+
"//envoy/network:io_handle_interface",
648+
"//envoy/stats:stats_interface",
649+
"//envoy/event:dispatcher_interface",
650+
"//envoy/config:typed_config_interface",
651+
"//envoy/server:factory_context_interface",
652+
"@envoy_api//envoy/config/core/v3:pkg_cc_proto",
636653
"@quiche//:quic_core_packet_writer_lib",
637654
"@quiche//:quic_platform",
638655
]),

‎source/common/quic/active_quic_listener.cc‎

Lines changed: 33 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
#include "source/common/quic/envoy_quic_proof_source.h"
2626
#include "source/common/quic/envoy_quic_utils.h"
2727
#include "source/common/quic/quic_network_connection.h"
28+
#include "source/common/quic/quic_packet_writer_interface.h"
2829
#include "source/common/runtime/runtime_features.h"
2930

3031
namespace Envoy {
@@ -104,22 +105,39 @@ ActiveQuicListener::ActiveQuicListener(
104105

105106
absl::AnyInvocable<void() &&> on_can_write_cb = [&]() { quic_dispatcher_->OnCanWrite(); };
106107

107-
// Create udp_packet_writer
108-
Network::UdpPacketWriterPtr udp_packet_writer =
109-
listener_config.udpListenerConfig()->packetWriterFactory().createUdpPacketWriter(
110-
listen_socket_.ioHandle(), listener_config.listenerScope(), dispatcher,
111-
std::move(on_can_write_cb));
112-
udp_packet_writer_ = udp_packet_writer.get();
113-
114-
// Some packet writers (like `UdpGsoBatchWriter`) already directly implement
115-
// `quic::QuicPacketWriter` and can be used directly here. Other types need
116-
// `EnvoyQuicPacketWriter` as an adapter.
117-
auto* quic_packet_writer = dynamic_cast<quic::QuicPacketWriter*>(udp_packet_writer.get());
118-
if (quic_packet_writer != nullptr) {
119-
quic_dispatcher_->InitializeWithWriter(quic_packet_writer);
120-
udp_packet_writer.release();
108+
// Create quic_packet_writer
109+
QuicPacketWriterFactory* quic_packet_writer_factory =
110+
listener_config.udpListenerConfig()->quicPacketWriterFactory();
111+
112+
if (quic_packet_writer_factory != nullptr) {
113+
QuicPacketWriterPtr quic_writer = quic_packet_writer_factory->createQuicPacketWriter(
114+
listen_socket_.ioHandle(), listener_config.listenerScope(), dispatcher,
115+
std::move(on_can_write_cb));
116+
quic_packet_writer_ = quic_writer.get();
117+
quic_dispatcher_->InitializeWithWriter(quic_writer.release());
121118
} else {
122-
quic_dispatcher_->InitializeWithWriter(new EnvoyQuicPacketWriter(std::move(udp_packet_writer)));
119+
// TODO(panting): This fallback is a temporary migration bridge. We must keep this
120+
// logic because there is currently no QUIC GSO batch factory implemented to create
121+
// GSO writers natively. Once a native QuicGsoBatchWriterFactory is implemented
122+
// and default configurations are migrated, this fallback can be removed.
123+
// Create udp_packet_writer
124+
Network::UdpPacketWriterPtr udp_packet_writer =
125+
listener_config.udpListenerConfig()->packetWriterFactory().createUdpPacketWriter(
126+
listen_socket_.ioHandle(), listener_config.listenerScope(), dispatcher,
127+
std::move(on_can_write_cb));
128+
udp_packet_writer_ = udp_packet_writer.get();
129+
130+
// Some packet writers (like `UdpGsoBatchWriter`) already directly implement
131+
// `quic::QuicPacketWriter` and can be used directly here. Other types need
132+
// `EnvoyQuicPacketWriter` as an adapter.
133+
auto* quic_packet_writer = dynamic_cast<quic::QuicPacketWriter*>(udp_packet_writer.get());
134+
if (quic_packet_writer != nullptr) {
135+
quic_dispatcher_->InitializeWithWriter(quic_packet_writer);
136+
udp_packet_writer.release();
137+
} else {
138+
quic_dispatcher_->InitializeWithWriter(
139+
new EnvoyQuicPacketWriter(std::move(udp_packet_writer)));
140+
}
123141
}
124142

125143
if (listener_config.udpListenerConfig()) {

‎source/common/quic/active_quic_listener.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
#include "source/common/runtime/runtime_protos.h"
1717
#include "source/server/active_udp_listener.h"
1818

19+
#include "quiche/quic/core/quic_packet_writer.h"
1920
#include "quiche/quic/load_balancer/load_balancer_encoder.h"
2021

2122
namespace Envoy {
@@ -58,6 +59,7 @@ class ActiveQuicListener : public Envoy::Server::ActiveUdpListenerBase,
5859
// No-op. Quic can't do anything upon listener error.
5960
}
6061
Network::UdpPacketWriter& udpPacketWriter() override { return *udp_packet_writer_; }
62+
quic::QuicPacketWriter* quicPacketWriter() { return quic_packet_writer_; }
6163
void onDataWorker(Network::UdpRecvData&& data) override;
6264
uint32_t destination(const Network::UdpRecvData& data) const override;
6365
size_t numPacketsExpectedPerEventLoop() const override;
@@ -94,6 +96,7 @@ class ActiveQuicListener : public Envoy::Server::ActiveUdpListenerBase,
9496
const bool kernel_worker_routing_;
9597
std::optional<Runtime::FeatureFlag> enabled_;
9698
Network::UdpPacketWriter* udp_packet_writer_;
99+
quic::QuicPacketWriter* quic_packet_writer_{nullptr};
97100

98101
// The number of runs of the event loop in which at least one CHLO was buffered.
99102
// TODO(ggreenway): Consider making this a published stat, or some variation of this information.

‎source/common/quic/quic_packet_writer_interface.h‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
#pragma once
2+
3+
#include <memory>
4+
#include <string>
5+
6+
#include "envoy/config/core/v3/extension.pb.h"
7+
#include "envoy/config/typed_config.h"
8+
#include "envoy/event/dispatcher.h"
9+
#include "envoy/network/io_handle.h"
10+
#include "envoy/server/factory_context.h"
11+
#include "envoy/stats/stats.h"
12+
13+
#include "absl/functional/any_invocable.h"
14+
#include "quiche/quic/core/quic_packet_writer.h"
15+
16+
namespace Envoy {
17+
namespace Quic {
18+
19+
using QuicPacketWriterPtr = std::unique_ptr<quic::QuicPacketWriter>;
20+
21+
/**
22+
* A factory for creating QuicPacketWriters.
23+
*/
24+
class QuicPacketWriterFactory {
25+
public:
26+
virtual ~QuicPacketWriterFactory() = default;
27+
28+
/**
29+
* Creates a QuicPacketWriter.
30+
* @param io_handle the IoHandle to write to.
31+
* @param scope the stats scope to write stats to.
32+
* @param dispatcher the dispatcher for the thread.
33+
* @param on_can_write_cb callback to invoke when the writer becomes writable.
34+
* @return the QuicPacketWriter created.
35+
*/
36+
virtual QuicPacketWriterPtr
37+
createQuicPacketWriter(Network::IoHandle& io_handle, Stats::Scope& scope,
38+
Event::Dispatcher& dispatcher,
39+
absl::AnyInvocable<void() &&> on_can_write_cb) PURE;
40+
};
41+
42+
using QuicPacketWriterFactoryPtr = std::unique_ptr<QuicPacketWriterFactory>;
43+
44+
/**
45+
* QuicPacketWriterFactoryFactory adds an extra layer of indirection In order to
46+
* support a QuicPacketWriterFactory whose behavior depends on the
47+
* TypedConfig for that factory.
48+
*/
49+
class QuicPacketWriterFactoryFactory : public Envoy::Config::TypedFactory {
50+
public:
51+
~QuicPacketWriterFactoryFactory() override = default;
52+
53+
/**
54+
* Creates a QuicPacketWriterFactory based on the specified config.
55+
* @return the QuicPacketWriterFactory created.
56+
*/
57+
virtual QuicPacketWriterFactoryPtr
58+
createQuicPacketWriterFactory(const envoy::config::core::v3::TypedExtensionConfig& config,
59+
Server::Configuration::ListenerFactoryContext& context) PURE;
60+
61+
std::string category() const override { return "envoy.quic.packet_writer"; }
62+
};
63+
64+
} // namespace Quic
65+
} // namespace Envoy

‎test/common/quic/BUILD‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,7 @@ envoy_cc_test(
237237
srcs = envoy_select_enable_http3(["active_quic_listener_test.cc"]),
238238
rbe_pool = "6gig",
239239
deps = envoy_select_enable_http3([
240+
":fake_quic_packet_writer_lib",
240241
":test_proof_source_lib",
241242
":test_utils_lib",
242243
"//source/common/http:utility_lib",
@@ -307,6 +308,14 @@ envoy_cc_test_library(
307308
],
308309
)
309310

311+
envoy_cc_test_library(
312+
name = "fake_quic_packet_writer_lib",
313+
hdrs = envoy_select_enable_http3(["fake_quic_packet_writer.h"]),
314+
deps = envoy_select_enable_http3([
315+
"@quiche//:quic_core_packet_writer_lib",
316+
]),
317+
)
318+
310319
envoy_cc_test(
311320
name = "client_connection_factory_impl_test",
312321
srcs = envoy_select_enable_http3(["client_connection_factory_impl_test.cc"]),

‎test/common/quic/active_quic_listener_test.cc‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,17 @@
1616
#include "source/common/network/utility.h"
1717
#include "source/common/quic/active_quic_listener.h"
1818
#include "source/common/quic/envoy_quic_clock.h"
19+
#include "source/common/quic/envoy_quic_packet_writer.h"
1920
#include "source/common/quic/envoy_quic_utils.h"
21+
#include "source/common/quic/quic_packet_writer_interface.h"
2022
#include "source/common/quic/udp_gso_batch_writer.h"
2123
#include "source/common/runtime/runtime_impl.h"
2224
#include "source/extensions/quic/crypto_stream/envoy_quic_crypto_server_stream.h"
2325
#include "source/extensions/quic/proof_source/envoy_quic_proof_source_factory_impl.h"
2426
#include "source/server/configuration_impl.h"
2527
#include "source/server/process_context_impl.h"
2628

29+
#include "test/common/quic/fake_quic_packet_writer.h"
2730
#include "test/common/quic/test_proof_source.h"
2831
#include "test/common/quic/test_utils.h"
2932
#include "test/mocks/network/mocks.h"
@@ -774,5 +777,47 @@ TEST_F(ActiveQuicListenerFactoryTest, DebugVisitorConfigured) {
774777
EXPECT_TRUE(debug_visitor_factory.has_value());
775778
}
776779

780+
namespace {
781+
782+
class MockQuicPacketWriterFactory : public QuicPacketWriterFactory {
783+
public:
784+
MockQuicPacketWriterFactory() = default;
785+
~MockQuicPacketWriterFactory() override = default;
786+
787+
MOCK_METHOD(QuicPacketWriterPtr, createQuicPacketWriter,
788+
(Network::IoHandle & io_handle, Stats::Scope& scope,
789+
Envoy::Event::Dispatcher& dispatcher, absl::AnyInvocable<void() &&> on_can_write_cb),
790+
(override));
791+
};
792+
793+
} // namespace
794+
795+
TEST_P(ActiveQuicListenerTest, DirectQuicPacketWriterCreation) {
796+
MockQuicPacketWriterFactory quic_packet_writer_factory;
797+
798+
// Override the quicPacketWriterFactory mock to return our QUIC factory.
799+
EXPECT_CALL(udp_listener_config_, quicPacketWriterFactory())
800+
.WillRepeatedly(Return(&quic_packet_writer_factory));
801+
802+
// Expect createQuicPacketWriter to be called, and return our dummy writer.
803+
FakeQuicPacketWriter* raw_writer = nullptr;
804+
EXPECT_CALL(quic_packet_writer_factory, createQuicPacketWriter(_, _, _, _))
805+
.WillOnce(Invoke([&raw_writer](Network::IoHandle&, Stats::Scope&, Envoy::Event::Dispatcher&,
806+
absl::AnyInvocable<void()&&>) -> QuicPacketWriterPtr {
807+
auto writer = std::make_unique<FakeQuicPacketWriter>();
808+
raw_writer = writer.get();
809+
return writer;
810+
}));
811+
812+
// Initialize the listener. This will trigger createQuicPacketWriter.
813+
initialize();
814+
815+
// Verify that the dispatcher was initialized with a writer (it shouldn't be null).
816+
EXPECT_NE(quic_dispatcher_, nullptr);
817+
818+
// Verify that the listener keeps a member pointing to the created writer.
819+
EXPECT_EQ(quic_listener_->quicPacketWriter(), raw_writer);
820+
}
821+
777822
} // namespace Quic
778823
} // namespace Envoy

0 commit comments

Comments
 (0)