-
Notifications
You must be signed in to change notification settings - Fork 5.5k
Listener: Add listener filters. #2346
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 34 commits
2fb53f1
11766c4
ee60ccf
6874568
bc51bf8
0a8fd6f
da0c39b
044a089
d97257a
efdf2f1
31ea8a0
c6f4fb2
a7845db
f9c5650
d42ce02
aa81333
add8512
c32fcaa
818858a
228ac19
0703ecc
84854b2
ad6f413
d4b2140
597d05a
1ad7886
8bd512f
dca40d8
ab098f3
d0ad3d5
7f77eb0
4d72b93
07629cb
de46c3b
f047852
13cba72
7d82bbe
61da63f
3503579
b7e16d3
e75f50d
9a2c62a
94751c0
1021213
a390886
ae07e0f
393e2bc
f5c19a9
6b3e81d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,6 +9,7 @@ namespace Envoy { | |
| namespace Network { | ||
|
|
||
| class Connection; | ||
| class AcceptedSocket; | ||
|
|
||
| /** | ||
| * Status codes returned by filters that can cause future filters to not get iterated to. | ||
|
|
@@ -147,6 +148,67 @@ class FilterManager { | |
| virtual bool initializeReadFilters() PURE; | ||
| }; | ||
|
|
||
| /** | ||
| * Callbacks used by individual listener filter instances to communicate with the listener filter | ||
| * manager. | ||
| */ | ||
| class ListenerFilterCallbacks { | ||
| public: | ||
| virtual ~ListenerFilterCallbacks() {} | ||
|
|
||
| /** | ||
| * @return the AcceptedSocket that owns this manager and the managed filters. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: "that owns this manager and the managed filters" is a little confusing from the filter perspective. Can you reword?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reworded as "@return AcceptedSocket the socket the filter is operating on." |
||
| */ | ||
| virtual AcceptedSocket& socket() PURE; | ||
|
|
||
| /** | ||
| * @return the Dispatcher for this manager. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "for this manager" doesn't really apply in filter context. I would reword.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This better: "@return the Dispatcher for issuing events."? |
||
| */ | ||
| virtual Event::Dispatcher& dispatcher() PURE; | ||
|
|
||
| /** | ||
| * If a filter stopped filter iteration by returning FilterStatus::StopIteration, | ||
| * the filter should call continueFilterChain(true) when complete to continue the filter chain, | ||
| * or continueFilterChain(false) if the filter execution failed and the connection must be | ||
| * closed. | ||
| * @param success boolean telling whether the filter execution was successful or not. | ||
| */ | ||
| virtual void continueFilterChain(bool success) PURE; | ||
| }; | ||
|
|
||
| /** | ||
| * Listener Filter | ||
| */ | ||
| class ListenerFilter { | ||
| public: | ||
| virtual ~ListenerFilter() {} | ||
|
|
||
| /** | ||
| * Called when a new connection is accepted, but before a Connection is created. | ||
| * Filter chain iteration can be stopped if needed. | ||
| * @param cb the callbacks the filter instance can use to communicate with the filter chain. | ||
| * @return status used by the filter manager to manage further filter iteration. | ||
| */ | ||
| virtual FilterStatus onAccept(ListenerFilterCallbacks& cb) PURE; | ||
| }; | ||
|
|
||
| typedef std::unique_ptr<ListenerFilter> ListenerFilterPtr; | ||
|
|
||
| /** | ||
| * Interface for filter callbacks and adding listener filters to a manager. | ||
| */ | ||
| class ListenerFilterManager { | ||
| public: | ||
| virtual ~ListenerFilterManager() {} | ||
|
|
||
| /** | ||
| * Add a filter to the listener. Filters are invoked in FIFO order (the filter added | ||
| * first is called first). | ||
| * @param filter supplies the filter being added. | ||
| */ | ||
| virtual void addAcceptFilter(ListenerFilterPtr&& filter) PURE; | ||
| }; | ||
|
|
||
| /** | ||
| * Creates a chain of network filters for a new connection. | ||
| */ | ||
|
|
@@ -155,12 +217,19 @@ class FilterChainFactory { | |
| virtual ~FilterChainFactory() {} | ||
|
|
||
| /** | ||
| * Called to create the filter chain. | ||
| * Called to create the network filter chain. | ||
| * @param connection supplies the connection to create the chain on. | ||
| * @return true if filter chain was created successfully. Otherwise | ||
| * false, e.g. filter chain is empty. | ||
| */ | ||
| virtual bool createFilterChain(Connection& connection) PURE; | ||
| virtual bool createNetworkFilterChain(Connection& connection) PURE; | ||
|
|
||
| /** | ||
| * Called to create the listener filter chain. | ||
| * @param listener supplies the listener to create the chain on. | ||
| * @return true if filter chain was created successfully. Otherwise false. | ||
| */ | ||
| virtual bool createListenerFilterChain(ListenerFilterManager& listener) PURE; | ||
| }; | ||
|
|
||
| } // namespace Network | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -34,5 +34,75 @@ class ListenSocket { | |
| typedef std::unique_ptr<ListenSocket> ListenSocketPtr; | ||
| typedef std::shared_ptr<ListenSocket> ListenSocketSharedPtr; | ||
|
|
||
| /** | ||
| * A socket passed to a connection. | ||
| */ | ||
| class ConnectionSocket { | ||
| public: | ||
| virtual ~ConnectionSocket() {} | ||
|
|
||
| /** | ||
| * @return the local address of the socket. | ||
| */ | ||
| virtual const Address::InstanceConstSharedPtr& localAddress() const PURE; | ||
|
|
||
| /** | ||
| * @return the remote address of the socket. | ||
| */ | ||
| virtual const Address::InstanceConstSharedPtr& remoteAddress() const PURE; | ||
|
|
||
| /** | ||
| * @return true if the local address has been reset. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: I still really don't know what "reset" means in this context (as a casual reader). Can you add an example? WDYT about my previous suggestion of
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. +1
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I must have missed that suggestion. The real semantics is that the original destination cluster needs to know if the local address was "restored to it's original value, as it was before redirection". The significance of this is that missing that distinction will cause the upstream connection be opened to the same local address the Listener was listening at leading to recursion of open connections until Envoy reaches the max number of open connections and dies. This also explains the difference between the resetLocalAddress() and setLocalAddress(). The latter is used for client connections, where the local address is fully known only after the socket is connected. So, initially the local address is the one the listener received the new connection at, but it can be restored to the original destination address by the original dst listener filter. Maybe it would be clearer to add a separate flag to the address change method to mark as the address as 'restored'? This way the semantics would be more explicit and likely easier to understand. I'll try this out. |
||
| */ | ||
| virtual bool localAddressReset() const PURE; | ||
|
|
||
| /** | ||
| * @return fd the socket's file descriptor. | ||
| */ | ||
| virtual int fd() const PURE; | ||
|
|
||
| /** | ||
| * Close the underlying socket. | ||
| */ | ||
| virtual void close() PURE; | ||
| }; | ||
|
|
||
| typedef std::unique_ptr<ConnectionSocket> ConnectionSocketPtr; | ||
|
|
||
| /** | ||
| * An abstract accepted socket. | ||
| * | ||
| * TODO(jrajahalme): Hide internals (e.g., fd) from the filters by providing callbacks filters | ||
| * may need (set/getsockopt(), peek(), recv(), etc.) | ||
| */ | ||
| class AcceptedSocket : virtual public ConnectionSocket { | ||
| public: | ||
| virtual ~AcceptedSocket() {} | ||
|
|
||
| /** | ||
| * Reset the destination address of the socket to a different address than the one | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I still prefer "change" vs. "reset" but that's me. If you prefer "reset" I can live with it.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm trying out "set"... |
||
| * the socket was accepted at. | ||
| */ | ||
| virtual void resetLocalAddress(const Address::InstanceConstSharedPtr& local_address) PURE; | ||
|
|
||
| /** | ||
| * Reset the source address of the socket to a different address than the one | ||
| * the socket was accepted at. | ||
| */ | ||
| virtual void resetRemoteAddress(const Address::InstanceConstSharedPtr& remote_address) PURE; | ||
| }; | ||
|
|
||
| typedef std::unique_ptr<AcceptedSocket> AcceptedSocketPtr; | ||
|
|
||
| class ClientSocket : virtual public ConnectionSocket { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you please add some comments on what this interface is for? |
||
| public: | ||
| virtual ~ClientSocket() {} | ||
|
|
||
| /** | ||
| * Set the local address of the socket. | ||
| */ | ||
| virtual void setLocalAddress(const Address::InstanceConstSharedPtr& local_address) PURE; | ||
| }; | ||
|
|
||
| } // namespace Network | ||
| } // namespace Envoy | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please fix doc comments above
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, sorry for the sloppiness...