-
Notifications
You must be signed in to change notification settings - Fork 5.3k
socket: add SO_KEEPALIVE option for upstream connections. #3042
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
Merged
Merged
Changes from all commits
Commits
Show all changes
27 commits
Select commit
Hold shift + click to select a range
0d71615
Add SO_KEEPALIVE option for upstream connections.
JonathanO 7017ffc
Remove incorrect import
JonathanO bd79a31
Hack data-plane-api version.
JonathanO c5ddd1b
Correct commit hash for data-plane-api
JonathanO 61537b7
Fix MacOS constant naming conflict
JonathanO fad89fd
Updated for data-plane-api changes.
JonathanO cb216a1
Parse tcp keepalive settings into a struct at construct time.
JonathanO 77aee92
Use new keepalive config struct in the option.
JonathanO 23acd5c
Fix format build failure.
JonathanO 894a6d4
Code review fixes.
JonathanO c813195
Refactoring to avoid use of Features.
JonathanO 8a88054
Updated to new proposed data-plane-api version
JonathanO 313743c
Merge branch 'master' into add-tcp-keepalive-option
JonathanO 937a793
Moved API changes from data-plane-api repo.
JonathanO 15d52ce
Code review changes.
JonathanO 43d117d
Merge remote-tracking branch 'upstream/master' into add-tcp-keepalive…
JonathanO 238894e
Include "level" in SocketOptionName
JonathanO d85f3dd
Generic socket options holder and factories.
JonathanO 0c81bbb
Code review update and style fixes.
JonathanO 3d15a3d
Merge remote-tracking branch 'upstream/master' into add-tcp-keepalive…
JonathanO e275246
Removed SocketOptionsImpl
JonathanO c0f4cdc
Renamed misnamed class, added test coverage.
JonathanO c1246bf
Merge remote-tracking branch 'upstream/master' into add-tcp-keepalive…
JonathanO 99ac729
Code style fixes.
JonathanO 588d234
Fix BUILD file format.
JonathanO 7a37ab7
Merge remote-tracking branch 'upstream/master' into add-tcp-keepalive…
JonathanO 4c40a6b
Added test case for all Keepalive options
JonathanO File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
58 changes: 58 additions & 0 deletions
58
source/common/network/addr_family_aware_socket_option_impl.cc
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,58 @@ | ||
| #include "common/network/addr_family_aware_socket_option_impl.h" | ||
|
|
||
| #include "envoy/common/exception.h" | ||
|
|
||
| #include "common/api/os_sys_calls_impl.h" | ||
| #include "common/common/assert.h" | ||
| #include "common/network/address_impl.h" | ||
| #include "common/network/socket_option_impl.h" | ||
|
|
||
| namespace Envoy { | ||
| namespace Network { | ||
|
|
||
| bool AddrFailyAwareSocketOptionImpl::setOption(Socket& socket, Socket::SocketState state) const { | ||
| return setIpSocketOption(socket, state, ipv4_option_, ipv6_option_); | ||
| } | ||
|
|
||
| bool AddrFailyAwareSocketOptionImpl::setIpSocketOption( | ||
| Socket& socket, Socket::SocketState state, const std::unique_ptr<SocketOptionImpl>& ipv4_option, | ||
| const std::unique_ptr<SocketOptionImpl>& ipv6_option) { | ||
| // If this isn't IP, we're out of luck. | ||
| Address::InstanceConstSharedPtr address; | ||
| const Address::Ip* ip = nullptr; | ||
| try { | ||
| // We have local address when the socket is used in a listener but have to | ||
| // infer the IP from the socket FD when initiating connections. | ||
| // TODO(htuch): Figure out a way to obtain a consistent interface for IP | ||
| // version from socket. | ||
| if (socket.localAddress()) { | ||
| ip = socket.localAddress()->ip(); | ||
| } else { | ||
| address = Address::addressFromFd(socket.fd()); | ||
| ip = address->ip(); | ||
| } | ||
| } catch (const EnvoyException&) { | ||
| // Ignore, we get here because we failed in getsockname(). | ||
| // TODO(htuch): We should probably clean up this logic to avoid relying on exceptions. | ||
| } | ||
| if (ip == nullptr) { | ||
| ENVOY_LOG(warn, "Failed to set IP socket option on non-IP socket"); | ||
| return false; | ||
| } | ||
|
|
||
| // If the FD is v4, we can only try the IPv4 variant. | ||
| if (ip->version() == Network::Address::IpVersion::v4) { | ||
| return ipv4_option->setOption(socket, state); | ||
| } | ||
|
|
||
| // If the FD is v6, we first try the IPv6 variant if the platfrom supports it and fallback to the | ||
| // IPv4 variant otherwise. | ||
| ASSERT(ip->version() == Network::Address::IpVersion::v6); | ||
| if (ipv6_option->isSupported()) { | ||
| return ipv6_option->setOption(socket, state); | ||
| } | ||
| return ipv4_option->setOption(socket, state); | ||
| } | ||
|
|
||
| } // namespace Network | ||
| } // namespace Envoy |
55 changes: 55 additions & 0 deletions
55
source/common/network/addr_family_aware_socket_option_impl.h
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| #pragma once | ||
|
|
||
| #include <netinet/in.h> | ||
| #include <netinet/ip.h> | ||
| #include <sys/socket.h> | ||
|
|
||
| #include "envoy/network/listen_socket.h" | ||
|
|
||
| #include "common/common/logger.h" | ||
| #include "common/network/socket_option_impl.h" | ||
|
|
||
| #include "absl/types/optional.h" | ||
|
|
||
| namespace Envoy { | ||
| namespace Network { | ||
|
|
||
| class AddrFailyAwareSocketOptionImpl : public Socket::Option, | ||
| Logger::Loggable<Logger::Id::connection> { | ||
| public: | ||
| AddrFailyAwareSocketOptionImpl(Socket::SocketState in_state, SocketOptionName ipv4_optname, | ||
| SocketOptionName ipv6_optname, int value) | ||
| : ipv4_option_(absl::make_unique<SocketOptionImpl>(in_state, ipv4_optname, value)), | ||
| ipv6_option_(absl::make_unique<SocketOptionImpl>(in_state, ipv6_optname, value)) {} | ||
|
|
||
| // Socket::Option | ||
| bool setOption(Socket& socket, Socket::SocketState state) const override; | ||
| // The common socket options don't require a hash key. | ||
| void hashKey(std::vector<uint8_t>&) const override {} | ||
|
|
||
| /** | ||
| * Set a socket option that applies at both IPv4 and IPv6 socket levels. When the underlying FD | ||
| * is IPv6, this function will attempt to set at IPv6 unless the platform only supports the | ||
| * option at the IPv4 level. | ||
| * @param socket. | ||
| * @param ipv4_optname SocketOptionName for IPv4 level. Set to empty if not supported on | ||
| * platform. | ||
| * @param ipv6_optname SocketOptionName for IPv6 level. Set to empty if not supported on | ||
| * platform. | ||
| * @param optval as per setsockopt(2). | ||
| * @param optlen as per setsockopt(2). | ||
| * @return int as per setsockopt(2). ENOTSUP is returned if the option is not supported on the | ||
| * platform for fd after the above option level fallback semantics are taken into account or the | ||
| * socket is non-IP. | ||
| */ | ||
| static bool setIpSocketOption(Socket& socket, Socket::SocketState state, | ||
| const std::unique_ptr<SocketOptionImpl>& ipv4_option, | ||
| const std::unique_ptr<SocketOptionImpl>& ipv6_option); | ||
|
|
||
| private: | ||
| const std::unique_ptr<SocketOptionImpl> ipv4_option_; | ||
| const std::unique_ptr<SocketOptionImpl> ipv6_option_; | ||
| }; | ||
|
|
||
| } // namespace Network | ||
| } // namespace Envoy |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 document the macos behavior. It looks like you added support for the right socket option, but please document that the other two must be unset in the config on macos.
Uh oh!
There was an error while loading. Please reload this page.
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.
I swear I replied to this, but the comment seems to be missing now.
The reason for the single special case is that MacOS uses a different name for that one constant (TCP_KEEPIDLE vs TCP_KEEPALIVE.) The other constants all have the same name as for Linux. All of these settings are valid for both Linux and MacOS. I've added a comment to that effect in the header where they're defined.