Add logic for instantiating UdpListenSocket based on SocketAddress.protocol in listener config#5443
Add logic for instantiating UdpListenSocket based on SocketAddress.protocol in listener config#5443mattklein123 merged 12 commits intoenvoyproxy:masterfrom mpwarres:listener_socket_type
Conversation
Also add Server::ListenerImpl::socket_type_ field and accessor. Signed-off-by: Michael Warres <mpw@google.com>
Signed-off-by: Michael Warres <mpw@google.com>
…on URL scheme. Signed-off-by: Michael Warres <mpw@google.com>
Signed-off-by: Michael Warres <mpw@google.com>
Signed-off-by: Michael Warres <mpw@google.com>
Signed-off-by: Michael Warres <mpw@google.com>
|
@conqerAtapple @mattklein123 @alyssawilk @cmluciano Here is some (non-QUIC-specific) config plumbing needed for prototyping the QUIC listener that should hopefully also help with @conqerAtapple PR #5256. I think this should be ready for review. |
Signed-off-by: Michael Warres <mpw@google.com>
alyssawilk
left a comment
alyssawilk
left a comment
There was a problem hiding this comment.
Hoping @conqerAtapple can do a pass as well just to make sure you two are aligned on UDP plans
Signed-off-by: Michael Warres <mpw@google.com>
|
Thanks for the review, Alyssa! Agree it would be good for @conqerAtapple to take a look to make sure this is compatible with what he's doing. I think it should fit in nicely with PR #5256 in that this code gets the UdpListenSocket instantiated, and then #5256 takes it from there, but we'll see. |
|
@mpwarres @alyssawilk Thanks for checking. So far @mpwarres changes looks to be fine/orthogonal to the UDP proxy changes I have been working on. |
mattklein123
left a comment
mattklein123
left a comment
There was a problem hiding this comment.
Thanks very cool. One small drive by comment.
Modify ListenerManagerImplWithRealFiltersTest.UdpAddress to specify listener protobuf directly via protobuf text format, rather than deprecated JSON format. Signed-off-by: Michael Warres <mpw@google.com>
|
@mpwarres check CI? /wait |
…:translateAddress() changes. Signed-off-by: Michael Warres <mpw@google.com>
|
Oops! Forgot to revert a test when backing out the AddressJson::translateAddress() change. Should hopefully pass CI now. |
mattklein123
left a comment
mattklein123
left a comment
There was a problem hiding this comment.
LGTM, few small things.
/wait
source/common/network/utility.cc
Outdated
| case envoy::api::v2::core::SocketAddress::UDP: | ||
| return Address::SocketType::Datagram; | ||
| default: | ||
| throw EnvoyException(fmt::format("unknown protocol value: {}", protocol)); |
There was a problem hiding this comment.
Can this actually happen? Is that caught by proto validation such that the default can be not reached?
There was a problem hiding this comment.
Oh, got it, hadn't realized we can assume validation has occurred. Replaced with NOT_REACHED_GCOVR_EXCL_LINE.
| return "SocketType::Stream"; | ||
| case Network::Address::SocketType::Datagram: | ||
| return "SocketType::Datagram"; | ||
| default: |
There was a problem hiding this comment.
I think this case is not needed as the compiler will check all enums are handled?
There was a problem hiding this comment.
Yup, fixed. Thanks!
Signed-off-by: Michael Warres <mpw@google.com>
…otocol in listener config (envoyproxy#5443) Signed-off-by: Michael Warres <mpw@google.com> Signed-off-by: Fred Douglas <fredlas@google.com>
4 participants
Description:
Assorted plumbing to instantiate UdpListenSocket based on SocketAddress.protocol in listener config, and make the fact that it is a datagram socket visible to consuming code:
Add Network::Utility::protobufAddressSocketType() function, for fetching
SocketAddress.protocol values from listener config and translating to
Network::Address::SocketType values.
Modify Server::ListenerImpl to fetch SocketAddress.protocol value.
Modify Config::AddressJson::translateAddress() to set SocketAddress.protocol
based on URL scheme.
Add Network::Address::SocketType param to
ListenerComponentFactory::createListenSocket(), to allow caller to indicate
whether a TCP or UDP socket (or a Unix domain socket) should be created.
Add Socket::socketType() accessor, so code using a socket can determine
whether it is a stream or datagram socket.
With these changes, when a Listener is configured with
SocketAddress.protocol=UDP (either directly in protobuf or in JSON via udp://
URL), the listen socket created by
Server::ListenerManagerImpl::addOrUpdateListener() will be a UdpListenSocket,
and code that acts on the socket (e.g. Server::WorkerImpl::addListener(),
Server::ConnectionHandlerImpl::addListener()) can branch on socket type to set
up appropriate event notification and handling.
Risk Level: Low
Testing: Unit tests added
Docs Changes: N/A
Release Notes: N/A
[Optional Fixes #Issue]
[Optional Deprecated:]