Provide ability to override socket API

[yanavlasov]

Recent changes to the socket related API no longer allow overriding methods such as bind or connect. This is both a bug report and proposal for how to bring this functionality back.

The "socket API" here and below refers to a collection of methods for creating communication endpoints in use by Envoy which mirrors Posix sockets API.

Background:
Envoy provide ability to extend name resolution mechanism by registering a custom address "resolver" factory identified by a string ID. Custom resolver by specifying its ID in the envoy.config.core.v3.SocketAddress configuration proto. A resolver returns Envoy::Network::Address::Instance interface to an IP address object.

Previously socket API was part of the Envoy::Network::Address::Instance which while admittedly unorthodox allowed vendors to override default behavior of the API based on the type of resolver that created given IP address object.

Recent redesign spread socket API between several interfaces:

• Envoy::Network::SocketInterface for creating descriptor objects. This interface is implemented by an object which is a singleton in Envoy process and can be customized through a factory.
• Envoy::Network::IoHandle for return values from SocketInterface methods.
• Envoy::Network::Socket. Objects of this interface are created from the Envoy::Network::IoHandle` to provide specializations for posix socket API.

The objects that implement Envoy::Network::Socket are created internally by Envoy and their types can not be modified by vendors. Since this interface defines methods like bind, listen and connect the behavior of these methods can be overridden no longer.

However some of our use cases require completely custom implementation of binding or connecting to some IP addresses (i.e. Cloud IPs). The customization is not just limited to providing additional socket options, but requires making calls to proprietary subsystems.

This proposal outlines new class hierarchy that would allow complete control over implementation of the socket API.
Stream socket descriptors can be placed into one of the 3 categories based on the use.

1. Listening socket used by a server to accept new connections from clients.
2. Connecting socket used by a client to establish new connection to a server.
3. Data socket that is used to send a receive data between peers.

As such class hierarchy can be designed as follows:

1. Socket interface with various methods common to all socket types (i.e. get/setsockopt)
2. DataSocket : Socket for sending and receiving data.
3. ListeningSocket : Socket with methods specific to listening sockets. A callback that accepts new connections provides DataSocket to communicate with the peer.
4. ConnectingSocket : Socket with methods specific to connecting sockets. A callback is invoked with the DataSocket when new connection is established.
5. SocketInterface has methods added for creating ListeningSocket and ConnectingSocket.

NB: We could make ConnectingSocket derive from DataSocket to reduce the number of changes in the code.

The SocketInterface implementation may need to distinguish between different types of IP addresses that are provided to it at the time of socket creation so it can instantiate different implementations of the socket interfaces. Use of RTTI is not desirable as it may require pulling in type definitions for IP address classes. However providing the string ID of the resolver that created the address is enough to cover existing use cases. As such the Envoy::Network::Address::Instance is proposed to be extended with the resolverId() method returning the unique ID of the resolver.

Datagram sockets have the superset of the DataSocket API with the additional bind and connect methods (if we use connect on UDP sockets).To support datagram sockets the following changes are proposed:

1. DatagramSocket : DataSocket with added bind method.
2. SocketInterface extended with the addition of the method for creating datagram sockets.

This class hierarchy would allow vendors to provide custom implementations for all classes involved in establishing a connection:

1. Custom name resolvers through named factories.
2. Custom SocketInterface that can instantiate different implementations of the Socket interface family that would allow all its methods to be overridden.

[yanavlasov]

I intend to start a separate discussion of how to avoid pulling the rug from under the vendors that may depend on "internal" Envoy interfaces. This one is to focus on the socket API specifically.

[mattklein123]

cc @florincoras

[florincoras]

@yanavlasov just some quick thoughts

Practically, Socket now offloads all interactions with the underlying host stack (both io and 'control') to IoHandle. So, in theory, the issue could be solved with a custom IoHandle, which would allow custom implementations for all apis, i.e., bind, connect, getOption etc.

Now as to how to do that and current limitations:

• The SocketInterface is the one that creates IoHandles that are wrapped in Sockets, i.e., it acts as a stack dependent factory. The implementation is not yet configurable, but I can push a PR that would make it by leveraging #11413. The main caveat is that it will need extra boostrap config. Is that an issue?
• Some IoHandle apis (e.g., IoHandle::bind) are too quick to pass the contents of an address instead of the Address::InstanceConstSharedPtr. I was already planning on fixing that

Does this make sense or did I miss something?

[yanavlasov]

@florincoras I'm unsure I'm following you. Can't we already override default SocketInterface implementation using Envoy::Network::SocketInterfaceLoader?

I suppose your second suggestion would solve our immediate issue. The changes have evolved so quickly that I did not notice that bind is implemented in both Socket and IoHandle interfaces. I think the only other missing piece is plumbing the ID of the resolver that created Address::Instance into the SocketInterface implementation.

However it is still seems to me that IoHandle role is superfluous as it seems to mirror the Socket interface. It would seem to be cleaner for the SocketInterface to create Sockets instead of IoHandles.

[florincoras]

@florincoras I'm unsure I'm following you. Can't we already override default SocketInterface implementation using Envoy::Network::SocketInterfaceLoader?

It's pretty much the same thing. The idea with the factory was that you could add a shim in Envoy that, when built, links to your custom socket layer implementation. Then, switching between implementations is just a startup config change.

I suppose your second suggestion would solve our immediate issue. The changes have evolved so quickly that I did not notice that bind is implemented in both Socket and IoHandle interfaces. I think the only other missing piece is plumbing the ID of the resolver that created Address::Instance into the SocketInterface implementation.

I guess that's possible. Personally, I'd be tempted to extend Address::Instance to carry additional information (or maybe add one more Address::Type) that could be used by the custom SocketInterface implementation to detect when 'special treatment' is needed.

However it is still seems to me that IoHandle role is superfluous as it seems to mirror the Socket interface. It would seem to be cleaner for the SocketInterface to create Sockets instead of IoHandles.

Agreed. The only issue is the fact that we rely a lot on constructors when instantiating specialized Sockets. As a result, we either have to make the SocketInterface aware of the specialized types or have these types behave as wrappers instead of deriving Socket. Personally, I like the second option better because a socket is just a handle that can be 'set up' (it shouldn't matter if through accept or connect) and subsequently can be read/written to. However, keep in mind that I'm C++ challenged and really new to the project, so take my opinions with a 'spoon' of salt :-)

[ggreenway]

@yanavlasov Are you needing separate route domains, so that for example you could have 10.0.0.1 go to different places in two different contexts, perhaps a "customer" context and an "admin" context?

If that's what you're needing I think it would make sense to allow arbitrary tagging of addresses, so that the creator of an Address::Instance can set the tag. It's a little bit more generic than tracking which resolver, but I think functionally equivalent (just a different name, really).

[yanavlasov]

@ggreenway something like that. I think whether it is some sort of extensible tagging or something else is not particularly relevant. It all sort of acts as poor man's RTTI.
@florincoras sure, adding some extensible types would work as well.
Let me know if you'd be willing to craft a PR. Otherwise I can do it too.

[florincoras]

@yanavlasov, I can start working on something once we're all in sync (cc @htuch). Still, if this ends up taking too long and you already have something that fixes the problem on your end, feel free to push it and I'll manage.

[florincoras]

Posting the link to the socket interface issues and next steps doc here as well for more visibility.

[florincoras]

@yanavlasov can we consider the issue solved after #12189?

[antoniovicente]

Could we make this change before calling it done? #12189 (comment)

[florincoras]

Could we make this change before calling it done? #12189 (comment)

Replying here although it might be better to move the conversation back to #12189. I just took a very quick look at what would be needed to keep SocketInterface pointers in Address::Instances and the biggest problem are going to be cyclic inclusions. Namely, SocketInterface needs Address::InstanceConstSharedPtr to build sockets.

Assuming Address::Instance factories properly validate socket interface names (without this we can't ensure much either way), if we still want to avoid the string lookups and memory footprint we can maybe assign some simple key (e.g., ints) to Address::Instances, as opposed to the strings, and lookup them up using a new data structure (e.g., vector) that we build at runtime in parallel to the Registry hash map.

[antoniovicente]

SocketInterface

I think that Address::Instance can forward declare SocketInterface and accept a plain pointer to SocketInterface. SocketInterface can continue including the full definition of the address interface defined at include/envoy/network/address.h

[florincoras]

@antoniovicente apologies for the delay! That works for me, what do you think @mattklein123 @yanavlasov ?

[mattklein123]

Sure SGTM.

[florincoras]

@antoniovicente would #12550 work?

[github-actions]

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions.

[github-actions]

This issue has been automatically closed because it has not had activity in the last 37 days. If this issue is still valid, please ping a maintainer and ask them to label it as "help wanted" or "no stalebot". Thank you for your contributions.