socket options: add support for directly creating ipv4/ipv6 pairs#18769
socket options: add support for directly creating ipv4/ipv6 pairs#18769snowp merged 7 commits intoenvoyproxy:mainfrom
Conversation
Signed-off-by: Mike Schore <mike.schore@gmail.com>
goaway
changed the title
Signed-off-by: Mike Schore <mike.schore@gmail.com>
snowp
left a comment
snowp
left a comment
There was a problem hiding this comment.
Thanks, just the few comments on isSupported
envoy/network/socket.h
Outdated
| envoy::config::core::v3::SocketOption::SocketState state) const PURE; | ||
|
|
||
| /** | ||
| * Implementations should typically return true. Unsupported or placeholder implementations |
There was a problem hiding this comment.
Maybe phrase this as Whether the socket option is supported or Whether the socket option can be applied?
I think I'd normally see the comment begin with describing what the function is supposed to do, then fill in with implementation guidelines if appropriate
| * may indicate such by returning false. | ||
| * @return Whether this is an actual socket option. | ||
| */ | ||
| virtual bool isSupported() const PURE; |
There was a problem hiding this comment.
What calls this at this layer? Is this something we'll be calling via EM?
There was a problem hiding this comment.
It's called by AddrFamilyAwareSocketOptionImpl. EM itself won't be calling it, but I needed to move it to the parent class (it was actually already replicated across a couple subclasses).
Signed-off-by: Mike Schore <mike.schore@gmail.com>
Signed-off-by: Mike Schore <mike.schore@gmail.com>
Signed-off-by: Mike Schore <mike.schore@gmail.com>
|
Thanks for taking a look @snowp! Updated isSupported docstring. |
snowp
left a comment
snowp
left a comment
There was a problem hiding this comment.
Cool this LGTM, but seems like CI is red, can you take a look?
|
@snowp it seems spurious(?) - it looks like a bunch tasks timed out. |
Signed-off-by: Mike Schore <mike.schore@gmail.com>
|
/retest |
|
Retrying Azure Pipelines: |
|
Thanks @snowp! |
* main: (221 commits) deps: Bump `protobuf` -> 3.19.0 (envoyproxy#18471) tooling: auto-assign dependency shephards (envoyproxy#18794) clang-tidy: Return from diff fun if empty diff (envoyproxy#18815) repokitteh: Block PRs pending deps approval (envoyproxy#18814) deps: Bump `org_llvm_llvm` -> 12.0.1, `com_github_wavm_wavm` -> 9ffd3e2 (envoyproxy#18747) dns resolvers: add All lookup mode (envoyproxy#18464) doc: fix link formatting for TLS session_timeout (envoyproxy#18790) ext_authz: Set response flag and code details to UAEX when denied (envoyproxy#18740) socket options: add support for directly creating ipv4/ipv6 pairs (envoyproxy#18769) ecds: make onConfigUpdate generic over filter type (envoyproxy#18061) bazel: update CMake instructions in EXTERNAL_DEPS.md (envoyproxy#18799) upstream: fix typo in comment (envoyproxy#18798) runtime: removing envoy.reloadable_features.grpc_json_transcoder_adhere_to_buffer_limits (envoyproxy#18696) bazel: Add CC=clang to clang configuration (envoyproxy#18732) fix error request id in the dubbbo local reply (envoyproxy#18741) event: assert the case of both read and closed event registered (envoyproxy#18265) tcp proxy connect tunneling: improved testing (envoyproxy#18784) deps: Bump `protoc-gen-validate` -> 0.6.2 (envoyproxy#18742) deps: Bump `rules_pkg` -> ad57589 (envoyproxy#18746) bazel: copy .bazelversion for envoy filter examples (envoyproxy#18730) ... Signed-off-by: Michael Puncel <mpuncel@squareup.com>
2 participants
Commit Message: socket options: add support for directly creating ipv4/ipv6 pairs
Additional Description: Allows AddrFamilyAwareSocketOptionImpl to be constructed directly from Socket::Option pairs.
Needed for envoyproxy/envoy-mobile#1897
Risk Level: Low
Testing: Local & CI
Signed-off-by: Mike Schore mike.schore@gmail.com