Skip to content

socket: add get sockopt for connect#17729

Merged
zuercher merged 7 commits intoenvoyproxy:mainfrom
lambdai:ussockconnct
Sep 2, 2021
Merged

socket: add get sockopt for connect#17729
zuercher merged 7 commits intoenvoyproxy:mainfrom
lambdai:ussockconnct

Conversation

@lambdai
Copy link
Copy Markdown
Contributor

@lambdai lambdai commented Aug 16, 2021

Commit Message:

Client connection can be created with least changes using underlying user space io socket per comment #16763 (comment)

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

Additional Description:
Add a new constructor at ClientConnectionImpl to create an envoy's client connection using the user space io socket.
Add the getSocketOption used by connect().
The added test case verifies that client connection can be established and closed.

Risk Level:
Testing:
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>
Signed-off-by: Yuchen Dai <silentdai@gmail.com>
antoniovicente
antoniovicente previously approved these changes Aug 26, 2021
Copy link
Copy Markdown
Contributor

@antoniovicente antoniovicente left a comment

Choose a reason for hiding this comment

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

Change looks good. Sorry for the delay, I have been relatively busy ramping up on my new role.

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

lambdai commented Aug 27, 2021

/retest

@repokitteh-read-only
Copy link
Copy Markdown

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

🐱

Caused by: a #17729 (comment) was created by @lambdai.

see: more, trace.

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

lambdai commented Aug 27, 2021

/retest

@repokitteh-read-only
Copy link
Copy Markdown

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

🐱

Caused by: a #17729 (comment) was created by @lambdai.

see: more, trace.

antoniovicente
antoniovicente previously approved these changes Sep 1, 2021
@antoniovicente
Copy link
Copy Markdown
Contributor

/assign-from @envoyproxy/senior-maintainers

@repokitteh-read-only
Copy link
Copy Markdown

@envoyproxy/senior-maintainers assignee is @zuercher

🐱

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

see: more, trace.

Copy link
Copy Markdown
Member

@zuercher zuercher left a comment

Choose a reason for hiding this comment

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

Looks good. I have one suggestion for a log message that was confusing to me.

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

lambdai commented Sep 2, 2021

Looks good. I have one suggestion for a log message that was confusing to me.

Fixed. could you take another look? Thanks!

Copy link
Copy Markdown
Member

@zuercher zuercher left a comment

Choose a reason for hiding this comment

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

Thanks!

@zuercher zuercher merged commit 0ab1823 into envoyproxy:main Sep 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants