extension: User space io socket #14917

lambdai · 2021-02-02T19:48:36Z

Commit Message:
Userspace io socket, cont
Introduce user space io socket handle on top of user space event at #14712

Userspace Io socket handles are always created with a peering handle.
The write() methods populate the buffer in the peer handle.
The read() methods consume the owned buffer.

Extracted from #13418

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

Additional Description:
Risk Level: LOW, extension
Testing: Unit test
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Deprecated:]
[Optional API Considerations:]

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

lambdai · 2021-02-02T23:29:09Z

/assign @antoniovicente

antoniovicente

First pass. Sorry for not getting to this earlier.

source/extensions/io_socket/user_space/io_socket_handle_impl.h

source/extensions/io_socket/user_space/BUILD

source/extensions/io_socket/user_space/io_socket_handle_impl.cc

test/extensions/io_socket/user_space/io_socket_handle_impl_platform_test.cc

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

lambdai

Partial fix

source/extensions/io_socket/user_space/BUILD

source/extensions/io_socket/user_space/io_socket_handle_impl.cc

source/extensions/io_socket/user_space/io_socket_handle_impl.h

test/extensions/io_socket/user_space/io_socket_handle_impl_platform_test.cc

source/extensions/io_socket/user_space/io_socket_handle_impl.cc

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

test/extensions/io_socket/user_space/io_socket_handle_impl_platform_test.cc

test/extensions/io_socket/user_space/io_handle_impl_test.cc

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

lambdai · 2021-02-10T21:22:39Z

@antoniovicente All comments resolved.
Recent updates include using Factory to create pair of handles, and not throwing exception in local/remote address methods

source/extensions/io_socket/user_space/io_handle_impl.cc

test/extensions/io_socket/user_space/io_handle_impl_test.cc

antoniovicente · 2021-02-11T00:49:01Z

source/extensions/io_socket/user_space/io_handle_impl.cc

@@ -286,13 +293,11 @@ Api::SysCallIntResult IoHandleImpl::setBlocking(bool) { return makeInvalidSyscal
 absl::optional<int> IoHandleImpl::domain() { return absl::nullopt; }

 Network::Address::InstanceConstSharedPtr IoHandleImpl::localAddress() {
-  // TODO(lambdai): Rewrite when caller accept error as the return value.
-  throw EnvoyException(fmt::format("getsockname failed for IoHandleImpl"));
+  return IoHandleImpl::getCommonInternalAddress();


This seems odd. All internal handles have the same address, no way to log which is which.

Not sure how can this address be used....
This address doesn't provide the ability to connect to or listen. The connection socket could use AddressProvider to set the view of local address and remote address anyway

The address may be useful in logs to tell what is connected to what.

Let's leave it to the next phase when we do pass a reasonable address to IoHandleImpl

source/extensions/io_socket/user_space/io_handle_impl.h

test/extensions/io_socket/user_space/io_handle_impl_platform_test.cc

test/extensions/io_socket/user_space/io_handle_impl_test.cc

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

antoniovicente

cc @envoyproxy/senior-maintainers

I think changes look good. I think this PR is ready for review by a non-googler senior maintainer.

test/extensions/io_socket/user_space/io_handle_impl_platform_test.cc

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

antoniovicente · 2021-02-19T01:24:07Z

/retest

Windows CI looks like an unrelated flaky issue. These new libraries are not linked into the failing test.

repokitteh-read-only · 2021-02-19T01:24:10Z

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #14917 (comment) was created by @antoniovicente.

see: more, trace.

ggreenway

This looks great overall.

/wait

ggreenway · 2021-02-26T18:45:10Z

source/extensions/io_socket/user_space/io_handle_impl.cc

+
+Api::IoCallUint64Result IoHandleImpl::read(Buffer::Instance& buffer,
+                                           absl::optional<uint64_t> max_length_opt) {
+  // Below value comes from Buffer::OwnedImpl::default_read_reservation_size_.


I think it would be better to expose this value somehow and use it from OwnedImpl. Otherwise this may get out of sync at some point, which will be easy to not notice.

I am creating an issue to track the next step.

source/extensions/io_socket/user_space/io_handle_impl.cc

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

lambdai · 2021-02-26T23:59:06Z

merging main

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

lambdai · 2021-03-01T21:19:11Z

gentle ping @ggreenway

lambdai · 2021-03-02T19:15:55Z

Thank you!

fresh new user space io handle

c6bc79c

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

lambdai requested a review from antoniovicente February 2, 2021 19:48

repokitteh-read-only bot assigned antoniovicente Feb 2, 2021

antoniovicente reviewed Feb 4, 2021

View reviewed changes

address comment

adac3ad

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

lambdai commented Feb 4, 2021

View reviewed changes

lambdai added 3 commits February 4, 2021 01:14

fix multi slice readv

a27a17a

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

fix multi slice readv

e4458e8

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

Merge branch 'ush' of github.com:lambdai/envoy-dai into ush

cabab54

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

antoniovicente reviewed Feb 5, 2021

View reviewed changes

test/extensions/io_socket/user_space/io_socket_handle_impl_platform_test.cc Outdated Show resolved Hide resolved

test/extensions/io_socket/user_space/io_handle_impl_test.cc Show resolved Hide resolved

antoniovicente added the waiting label Feb 5, 2021

lambdai added 4 commits February 7, 2021 23:15

rewrite read to respect highwatermark

51e8ee8

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

rewrite read/write throttling

0d10682

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

remove a test helper

5d9ec10

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

IoHandleImpl: factory and noncopyable

8a266f8

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

repokitteh-read-only bot removed the waiting label Feb 8, 2021

lambdai added 5 commits February 8, 2021 00:25

comment

e20ec46

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

remove a header from no where

2e1ab6a

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

io handle impl return internal address

8e5a5a5

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

Merge branch 'ush' of github.com:lambdai/envoy-dai into ush

144a40c

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

fix typo and incr coverage

9e4153b

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

antoniovicente reviewed Feb 11, 2021

View reviewed changes

lambdai added 2 commits February 11, 2021 02:01

address comment

d3adf99

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

another read test case

9465670

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

antoniovicente previously approved these changes Feb 13, 2021

View reviewed changes

test/extensions/io_socket/user_space/io_handle_impl_platform_test.cc Outdated Show resolved Hide resolved

migrate platform test case to io_handle_impl_test

389dc5a

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

lambdai dismissed antoniovicente’s stale review via 389dc5a February 16, 2021 22:36

delete deleted

b2c675b

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

antoniovicente previously approved these changes Feb 17, 2021

View reviewed changes

antoniovicente mentioned this pull request Feb 17, 2021

Io socket handle for internal socket #13418

Closed

ggreenway self-assigned this Feb 24, 2021

ggreenway requested changes Feb 26, 2021

View reviewed changes

repokitteh-read-only bot added the waiting label Feb 26, 2021

lambdai added 2 commits February 26, 2021 13:43

Greg comment

cdafede

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

Merge branch 'ush' of github.com:lambdai/envoy-dai into ush

129bda5

lambdai dismissed antoniovicente’s stale review via 129bda5 February 26, 2021 22:02

repokitteh-read-only bot removed the waiting label Feb 26, 2021

antoniovicente mentioned this pull request Feb 26, 2021

Converge the buffer parameter and op, or not #15218

Closed

lambdai requested a review from ggreenway February 26, 2021 23:53

lambdai added 3 commits February 26, 2021 15:59

Merge branch 'main' into ush

d4e588c

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

Merge branch 'main' into ush

78fcd53

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

merge master and add ioctl

a8dffea

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

ggreenway approved these changes Mar 2, 2021

View reviewed changes

ggreenway merged commit 56eb1e2 into envoyproxy:main Mar 2, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

extension: User space io socket #14917

extension: User space io socket #14917

lambdai commented Feb 2, 2021

lambdai commented Feb 2, 2021

antoniovicente left a comment

lambdai left a comment

lambdai commented Feb 10, 2021

antoniovicente Feb 11, 2021

lambdai Feb 11, 2021

antoniovicente Feb 13, 2021

lambdai Feb 17, 2021

antoniovicente left a comment

antoniovicente commented Feb 19, 2021

repokitteh-read-only bot commented Feb 19, 2021

ggreenway left a comment

ggreenway Feb 26, 2021

lambdai Feb 26, 2021

lambdai Feb 26, 2021

lambdai commented Feb 26, 2021

lambdai commented Mar 1, 2021

lambdai commented Mar 2, 2021

extension: User space io socket #14917

extension: User space io socket #14917

Conversation

lambdai commented Feb 2, 2021

lambdai commented Feb 2, 2021

antoniovicente left a comment

Choose a reason for hiding this comment

lambdai left a comment

Choose a reason for hiding this comment

lambdai commented Feb 10, 2021

antoniovicente Feb 11, 2021

Choose a reason for hiding this comment

lambdai Feb 11, 2021

Choose a reason for hiding this comment

antoniovicente Feb 13, 2021

Choose a reason for hiding this comment

lambdai Feb 17, 2021

Choose a reason for hiding this comment

antoniovicente left a comment

Choose a reason for hiding this comment

antoniovicente commented Feb 19, 2021

repokitteh-read-only bot commented Feb 19, 2021

ggreenway left a comment

Choose a reason for hiding this comment

ggreenway Feb 26, 2021

Choose a reason for hiding this comment

lambdai Feb 26, 2021

Choose a reason for hiding this comment

lambdai Feb 26, 2021

Choose a reason for hiding this comment

lambdai commented Feb 26, 2021

lambdai commented Mar 1, 2021

lambdai commented Mar 2, 2021