quic: decouple UDP and QUIC packet writers for direct creation in ActiveQuicListener#45735
quic: decouple UDP and QUIC packet writers for direct creation in ActiveQuicListener#45735fishpan1209 wants to merge 13 commits into
Conversation
…iveQuicListener Signed-off-by: Ting Pan <panting@google.com>
Signed-off-by: Ting Pan <panting@google.com>
Signed-off-by: Ting Pan <panting@google.com>
Signed-off-by: Ting Pan <panting@google.com>
|
/retest |
|
/assign @wang178c @RyanTheOptimist |
|
@fishpan1209 this is waiting on an update from you, right? |
…erFactory Signed-off-by: Ting Pan <panting@google.com>
Signed-off-by: Ting Pan <panting@google.com>
Signed-off-by: Ting Pan <panting@google.com>
Signed-off-by: Ting Pan <panting@google.com>
|
ready for review again, thank you @wang178c @RyanTheOptimist |
|
/retest |
Signed-off-by: Ting Pan <panting@google.com>
|
|
||
| namespace Quic { | ||
| class QuicPacketWriterFactory; | ||
| } // namespace Quic |
There was a problem hiding this comment.
Can we not use forward declaration?
There was a problem hiding this comment.
We need to keep the #ifdef ENVOY_ENABLE_QUIC guards and forward declarations here to support non-QUIC builds. Direct inclusion unconditionally pulls in QUICHE headers that are not available or cause conflicts when QUIC is explicitly disabled.
| validation_visitor_, *listener_factory_context_); | ||
|
|
||
| if (config.udp_listener_config().has_udp_packet_packet_writer_config()) { | ||
| auto* quic_factory_factory = |
There was a problem hiding this comment.
quic_packet_writer_factory_factory
| // Network::UdpListenerConfig | ||
| Network::ActiveUdpListenerFactory& listenerFactory() override { return *listener_factory_; } | ||
| Network::UdpPacketWriterFactory& packetWriterFactory() override { return *writer_factory_; } | ||
| #ifdef ENVOY_ENABLE_QUIC |
There was a problem hiding this comment.
Why guarded by MACRO?
Shouldn't quic_writer_factory_ be null by default?
There was a problem hiding this comment.
same reason above, in non-quic builds, we don't declare Quic::QuicPacketWriterFactoryPtr quic_writer_factory_ because quic::packetWriter is not available
| const envoy::config::listener::v3::UdpListenerConfig config_; | ||
| Network::ActiveUdpListenerFactoryPtr listener_factory_; | ||
| Network::UdpPacketWriterFactoryPtr writer_factory_; | ||
| #ifdef ENVOY_ENABLE_QUIC |
| bool disable_and_do_not_enable_{}; | ||
| const bool enable_half_close_; | ||
| testing::NiceMock<ProtobufMessage::MockValidationVisitor> validation_visitor_; | ||
| #ifdef ENVOY_ENABLE_QUIC |
| quic_dispatcher_->InitializeWithWriter(quic_packet_writer); | ||
| udp_packet_writer.release(); | ||
| // Create quic_packet_writer | ||
| QuicPacketWriterFactory* quic_factory = |
There was a problem hiding this comment.
quic_packet_writer_factory
Signed-off-by: Ting Pan <panting@google.com>
3 participants
…iveQuicListener
Commit Message: quic: decouple UDP and QUIC packet writers for direct creation in ActiveQuicListener
Additional Description: This change separates the
UdpPacketWriterandQuicPacketWriterinterfaces, enabling the direct creation ofQuicPacketWriterwithinActiveQuicListener. By decoupling these writers, the system can bypass theEnvoyQuicPacketWriterwrapper 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:]