Skip to content

[WIP] Envoy Internal connection strawman#12537

Closed
lambdai wants to merge 38 commits intoenvoyproxy:masterfrom
lambdai:hostconn
Closed

[WIP] Envoy Internal connection strawman#12537
lambdai wants to merge 38 commits intoenvoyproxy:masterfrom
lambdai:hostconn

Conversation

@lambdai
Copy link
Copy Markdown
Contributor

@lambdai lambdai commented Aug 7, 2020

Commit Message:
Prototype of #11725
The internal connection implementation is a toy.

But we we may want to discuss the

  • Address
    Envoy Internal address type is introduced in api: add envoy internal address #12837A new type of address, or specialize the ip/pipe?
  • Listener.
    I strongly believe we should introduce a new field internal_listener in message Listener. Similar to Api Listener
    We can add new internal-specific settings into this type of struct.

Implementations

  • buffered_io_socket_handle_impl
    This is an socket handle which owns a buffer. Think the buffer as the socket buffer in the kernel. This handle should be always created with a peer so that this handle could write to peer's buffer, and notify readable or writable signal along with read/write/shutdown.
  • new api in dispatcher
    Allow register internal listener and target listener when a client connection need to be created
  • new internal listener impl along with connection_handler
    Register internal listener by name and "accept" connection. Currently internal listener could simulate tcp listener but not udp listener.

Additional Description:
Risk Level:
Testing:
Docs Changes:
Release Notes:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Deprecated:]

lambdai added 12 commits June 22, 2020 03:54
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
write return close only when eos and buffer empty.
write won't set end of stream.

Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
@repokitteh-read-only
Copy link
Copy Markdown

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to api/envoy/.
CC @envoyproxy/api-watchers: FYI only for changes made to api/envoy/.

🐱

Caused by: #12537 was opened by lambdai.

see: more, trace.

: BaseListenerImpl(dispatcher, std::move(socket)), cb_(cb), listener_(nullptr) {
if (bind_to_port) {
setupServerSocket(dispatcher, *socket_);
setupPipeListener(dispatcher, name);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mattklein123
I registered each listener as internal connectable. This will lead to failure in the follow up.
I incline to create a new InternalListener type so that I can bootstrap the new type of listener,
adding very limited feature at the early phase.

WDYT?

lambdai added 14 commits August 12, 2020 19:19
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
For half close

Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: Yuchen Dai <silentdai@gmail.com>

add missing pipe file

Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Copy link
Copy Markdown
Contributor

@alyssawilk alyssawilk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is going to be epic once it lands.
To respond to your questions (with a few glances at code)

"Address.
A new type of address, or specialize the ip/pipe?"

given they're const I'd be inclined to pass through the original listener, whatever we are demuxing from, at least for first pass.

"Listener.
Do we want to reuse the TcpListener with restrictions since not all tcp can be implemented
O"
Examples of what can't be implemented please?

TransportSocketCallbacks* callbacks_{};
bool shutdown_{};
Buffer::WatermarkBuffer read_buffer_;
bool read_end_stream_{false};
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where is this set?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Completely drop this transport socket implementation and moved to lower level IoHandle implementation.

}
}

ClientPipeImpl::ClientPipeImpl(Event::Dispatcher& dispatcher,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please break this into its own file.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ClientPipeImpl is now deleted
Instead of introducing ClientPipeImpl and ServerPipeImpl, ConnectionImpl now could use new buffer source io handle.

lambdai added 11 commits August 18, 2020 11:57
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
@stale
Copy link
Copy Markdown

stale bot commented Aug 31, 2020

This pull request has been automatically marked as stale because it has not had activity in the last 7 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

@stale stale bot added the stale stalebot believes this issue/PR has not been touched recently label Aug 31, 2020
@stale
Copy link
Copy Markdown

stale bot commented Sep 7, 2020

This pull request has been automatically closed because it has not had activity in the last 14 days. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api stale stalebot believes this issue/PR has not been touched recently

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants