Skip to content
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
87bb932
Add TCP_FASTOPEN listener option
bmetzdorf Feb 3, 2018
cdfeb38
Merge branch 'master' into tfo
ggreenway Apr 2, 2018
982a27b
Update to use new socket setting facility
ggreenway Apr 2, 2018
7cc3768
Fix crash when no options present
ggreenway Apr 3, 2018
8dcf11d
Don't set TRANSPARENT option in state Listening
ggreenway Apr 3, 2018
6f55c84
Add more test coverage
ggreenway Apr 3, 2018
dea8e2c
Make the option tunable, not just boolean
ggreenway Apr 3, 2018
cc211dd
Add test for setting Listening socket options
ggreenway Apr 3, 2018
d9ba97a
Fixes
ggreenway Apr 3, 2018
e56a8d7
test typo
ggreenway Apr 3, 2018
f2db864
Fix misspelled comments
ggreenway Apr 3, 2018
9f3c97e
one more misspelled comment
ggreenway Apr 4, 2018
d2d4a27
Updated data-plane-api commit
bmetzdorf Apr 5, 2018
a03533a
fix typo
bmetzdorf Apr 5, 2018
89a3b99
Merge branch 'master' of https://github.com/envoyproxy/envoy into tfo…
bmetzdorf Apr 5, 2018
8289daa
set listening socket options in ListenerSocketOption
bmetzdorf Apr 5, 2018
e7ff081
move ListenerSocketOption into its own file
bmetzdorf Apr 5, 2018
8f40b58
add listener_socket_option_impl_test
bmetzdorf Apr 5, 2018
42ad7e4
move ENVOY_SOCKET_TCP_FASTOPEN to listener_socket_option_impl.h
bmetzdorf Apr 5, 2018
f90b61f
document Socket::SocketState
bmetzdorf Apr 6, 2018
1d5734a
removing testing::AtLeast
bmetzdorf Apr 6, 2018
9e57d17
fix formatting
bmetzdorf Apr 6, 2018
5abe337
just keep constructor prototypes in listener_socket_option_impl.h
bmetzdorf Apr 6, 2018
2b21dd5
move "#include <netinet/tcp.h>"
bmetzdorf Apr 6, 2018
8626895
document SocketState prettier
bmetzdorf Apr 6, 2018
521faad
Merge remote-tracking branch 'upstream/master' into tfo
ggreenway Apr 10, 2018
cb298fb
Merge remote-tracking branch 'upstream/master' into tfo
ggreenway Apr 12, 2018
4219d2e
document SocketState even prettier
bmetzdorf Apr 12, 2018
2f47e3d
Merge remote-tracking branch 'upstream-public/master' into tfo-public
bmetzdorf Apr 12, 2018
b9312fd
Call parent class setOption()
ggreenway Apr 12, 2018
06d426e
Merge branch 'tfo' of https://github.com/bmetzdorf/envoy into tfo-public
bmetzdorf Apr 12, 2018
f9f608a
add os_sys_calls bazel dependency
bmetzdorf Apr 12, 2018
2ca16c3
return false if TFO is requested but not supported
bmetzdorf Apr 12, 2018
470f934
Merge remote-tracking branch 'upstream-public/master' into tfo-public
bmetzdorf Apr 16, 2018
e0dbd9f
Add TCP_FASTOPEN listener option release notes
bmetzdorf Apr 16, 2018
3dd9372
move TCP_FASTOPEN into socket_option_impl.h
bmetzdorf Apr 17, 2018
3f11ee6
Don't ignore return value from super-call
ggreenway Apr 17, 2018
ec0fa87
Merge remote-tracking branch 'upstream/master' into tfo
ggreenway Apr 17, 2018
4494d2c
ListenerSocketOptionImplTest should inherit from SocketOptionImplTest
bmetzdorf Apr 18, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions bazel/repository_locations.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,8 @@ REPOSITORY_LOCATIONS = dict(
urls = ["https://github.com/google/protobuf/archive/v3.5.0.tar.gz"],
),
envoy_api = dict(
commit = "92ffa417b5d2e8ab3912ff4da4daa6aa93f8df0f",
remote = "https://github.com/envoyproxy/data-plane-api",
commit = "55f78650483140f8dbce89d6b215e85d52aaefba",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I just merged envoyproxy/data-plane-api#539, so this can be updated.

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.

data-plane-api commit updated!

remote = "https://github.com/bmetzdorf/data-plane-api.git", # TODO FIXME
),
grpc_httpjson_transcoding = dict(
commit = "e4f58aa07b9002befa493a0a82e10f2e98b51fc6",
Expand Down
2 changes: 1 addition & 1 deletion include/envoy/network/listen_socket.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ class Socket {
*/
virtual void close() PURE;

enum class SocketState { PreBind, PostBind };
enum class SocketState { PreBind, PostBind, Listening };

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think at this point we need clear comments on each of these to explain when they happen in the socket lifetime and relative to each other. Not sure why we need a Listening in addition to PostBind for example.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Agreed.

I'll put this in the comments, but the reason for the new state is that on macOS, you cannot set this option until after listen() has been called. PostBind is run between bind() and listen().

Can you think of a better name for PostBind, now that this 3rd state is added? Or just comment it heavily and keep the name?

@htuch htuch Apr 5, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think it's fine to just comment. As long as there is a clear order in the comments wrt the socket creation -> accept life cycle it's all good.

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.

SocketState is documented


/**
* Visitor class for setting socket options.
Expand Down
11 changes: 11 additions & 0 deletions source/common/network/listener_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,17 @@ ListenerImpl::ListenerImpl(Event::DispatcherImpl& dispatcher, Socket& socket, Li
fmt::format("cannot listen on socket: {}", socket.localAddress()->asString()));
}

Socket::OptionsSharedPtr options = socket.options();
if (options != nullptr) {
for (auto& option : *options) {
if (!option->setOption(socket, Socket::SocketState::Listening)) {
throw CreateListenerException(
fmt::format("cannot set post-listen socket option on socket: {}",
socket.localAddress()->asString()));
}
}
}

evconnlistener_set_error_cb(listener_.get(), errorCallback);
}
}
Expand Down
38 changes: 30 additions & 8 deletions source/common/network/socket_option_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,16 @@ namespace Envoy {
namespace Network {

bool SocketOptionImpl::setOption(Socket& socket, Socket::SocketState state) const {
if (transparent_.has_value()) {
const int should_transparent = transparent_.value() ? 1 : 0;
const int error =
setIpSocketOption(socket, ENVOY_SOCKET_IP_TRANSPARENT, ENVOY_SOCKET_IPV6_TRANSPARENT,
&should_transparent, sizeof(should_transparent));
if (error != 0) {
ENVOY_LOG(warn, "Setting IP_TRANSPARENT on listener socket failed: {}", strerror(error));
return false;
if (state == Socket::SocketState::PreBind || state == Socket::SocketState::PostBind) {
if (transparent_.has_value()) {
const int should_transparent = transparent_.value() ? 1 : 0;
const int error =
setIpSocketOption(socket, ENVOY_SOCKET_IP_TRANSPARENT, ENVOY_SOCKET_IPV6_TRANSPARENT,
&should_transparent, sizeof(should_transparent));
if (error != 0) {
ENVOY_LOG(warn, "Setting IP_TRANSPARENT on listener socket failed: {}", strerror(error));
return false;
}
}
}

Expand All @@ -34,6 +36,26 @@ bool SocketOptionImpl::setOption(Socket& socket, Socket::SocketState state) cons
}
}

if (state == Socket::SocketState::Listening) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this should live in ListenerSocketOption, since it is specific to the listener side.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Agreed

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.

ListenerSocketOption is addressed

if (tcp_fast_open_queue_length_.has_value()) {
const int tfo_value = tcp_fast_open_queue_length_.value();
const SocketOptionName option_name = ENVOY_SOCKET_TCP_FASTOPEN;
if (option_name) {
const int error = Api::OsSysCallsSingleton::get().setsockopt(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The IP options are more complicated because we have to deal with different options, only sometimes, between ipv4 and ipv6. AFAIK that isn't a thing with tcp socket options, and there's currently only 1 of them. Still think we should add a function for it at this time?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'd suggest to still do this, since we factor out the "Unsupported socket option .." detection and logging code. I think there's a bug in the code here as is; if the socket option is not supported, in this PR it just logs and returns true. It should log and return false (due to the ENOTSUP, which we would generate in the suggested setTcpSocketOption).

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.

I still have to do this..

socket.fd(), IPPROTO_TCP, option_name.value(),
reinterpret_cast<const void*>(&tfo_value), sizeof(tfo_value));
if (error != 0) {
ENVOY_LOG(warn, "Setting TCP_FASTOPEN on listener socket failed: {}", strerror(errno));
return false;
} else {
ENVOY_LOG(debug, "Successfully set socket option TCP_FASTOPEN to {}", tfo_value);
}
} else {
ENVOY_LOG(warn, "Unsupported socket option TCP_FASTOPEN");
}
}
}

return true;
}

Expand Down
14 changes: 12 additions & 2 deletions source/common/network/socket_option_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

#include <netinet/in.h>
#include <netinet/ip.h>
#include <netinet/tcp.h>
#include <sys/socket.h>

#include "envoy/network/listen_socket.h"
Expand Down Expand Up @@ -41,10 +42,18 @@ typedef absl::optional<int> SocketOptionName;
#define ENVOY_SOCKET_IPV6_FREEBIND Network::SocketOptionName()
#endif

#ifdef TCP_FASTOPEN
#define ENVOY_SOCKET_TCP_FASTOPEN Network::SocketOptionName(TCP_FASTOPEN)
#else
#define ENVOY_SOCKET_TCP_FASTOPEN Network::SocketOptionName()

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.

@htuch Do you mean just enforce TCP_FASTOPEN on all listeners by default and then run the tests?

#endif

class SocketOptionImpl : public Socket::Option, Logger::Loggable<Logger::Id::connection> {
public:
SocketOptionImpl(absl::optional<bool> transparent, absl::optional<bool> freebind)
: transparent_(transparent), freebind_(freebind) {}
SocketOptionImpl(absl::optional<bool> transparent, absl::optional<bool> freebind,
absl::optional<uint32_t> tcp_fast_open_queue_length)
: transparent_(transparent), freebind_(freebind),
tcp_fast_open_queue_length_(tcp_fast_open_queue_length) {}

// Socket::Option
bool setOption(Socket& socket, Socket::SocketState state) const override;
Expand Down Expand Up @@ -72,6 +81,7 @@ class SocketOptionImpl : public Socket::Option, Logger::Loggable<Logger::Id::con
private:
const absl::optional<bool> transparent_;
const absl::optional<bool> freebind_;
const absl::optional<uint32_t> tcp_fast_open_queue_length_;
};

} // namespace Network
Expand Down
8 changes: 5 additions & 3 deletions source/common/upstream/upstream_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,11 @@ uint64_t parseFeatures(const envoy::api::v2::Cluster& config,
class UpstreamSocketOption : public Network::SocketOptionImpl {
public:
UpstreamSocketOption(const ClusterInfo& cluster_info)
: Network::SocketOptionImpl({}, cluster_info.features() & ClusterInfo::Features::FREEBIND
? true
: absl::optional<bool>{}) {}
: Network::SocketOptionImpl({},
cluster_info.features() & ClusterInfo::Features::FREEBIND
? true
: absl::optional<bool>{},
{}) {}
};

} // namespace
Expand Down
8 changes: 7 additions & 1 deletion source/server/listener_manager_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,9 @@ class ListenerSocketOption : public Network::SocketOptionImpl {
ListenerSocketOption(const envoy::api::v2::Listener& config)
: Network::SocketOptionImpl(
PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, transparent, absl::optional<bool>{}),
PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, freebind, absl::optional<bool>{})) {}
PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, freebind, absl::optional<bool>{}),
PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, tcp_fast_open_queue_length,
absl::optional<uint32_t>{})) {}
};

ListenerImpl::ListenerImpl(const envoy::api::v2::Listener& config, ListenerManagerImpl& parent,
Expand Down Expand Up @@ -302,6 +304,10 @@ void ListenerImpl::setSocket(const Network::SocketSharedPtr& socket) {
} else {
ENVOY_LOG(debug, "{}", message);
}

// Add the options to the socket_ so that SocketState::Listening options can be
// set in the worker after listen()/evconnlistener_new() is called.
socket_->addOption(option);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Does adding these options here break anything else?

}
}
}
Expand Down
30 changes: 30 additions & 0 deletions test/common/network/listener_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "gmock/gmock.h"
#include "gtest/gtest.h"

using testing::AtLeast;
using testing::Invoke;
using testing::Return;
using testing::_;
Expand Down Expand Up @@ -86,6 +87,35 @@ INSTANTIATE_TEST_CASE_P(IpVersions, ListenerImplTest,
testing::ValuesIn(TestEnvironment::getIpVersionsForTest()),
TestUtility::ipTestParamsToString);

// Test that socket options are set after the listener is setup.
TEST_P(ListenerImplTest, SetListeningSocketOptionsSuccess) {
Event::DispatcherImpl dispatcher;
Network::MockListenerCallbacks listener_callbacks;
Network::MockConnectionHandler connection_handler;

Network::TcpListenSocket socket(Network::Test::getCanonicalLoopbackAddress(version_), nullptr,
true);
std::shared_ptr<MockSocketOption> option = std::make_shared<MockSocketOption>();
socket.addOption(option);
EXPECT_CALL(*option, setOption(_, Socket::SocketState::Listening)).WillOnce(Return(true));
TestListenerImpl listener(dispatcher, socket, listener_callbacks, true, false);
}

// Test that an exception is thrown if there is an error setting socket options.
TEST_P(ListenerImplTest, SetListeningSocketOptionsError) {
Event::DispatcherImpl dispatcher;
Network::MockListenerCallbacks listener_callbacks;
Network::MockConnectionHandler connection_handler;

Network::TcpListenSocket socket(Network::Test::getCanonicalLoopbackAddress(version_), nullptr,
true);
std::shared_ptr<MockSocketOption> option = std::make_shared<MockSocketOption>();
socket.addOption(option);
EXPECT_CALL(*option, setOption(_, Socket::SocketState::Listening)).WillOnce(Return(false));
EXPECT_THROW(TestListenerImpl(dispatcher, socket, listener_callbacks, true, false),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could make this EXPECT_THROW_WITH_MESSAGE to get better precision on why we see the exception.

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.

Yes, I'm working on this one.

CreateListenerException);
}

TEST_P(ListenerImplTest, UseActualDst) {
Stats::IsolatedStoreImpl stats_store;
Event::DispatcherImpl dispatcher;
Expand Down
148 changes: 76 additions & 72 deletions test/common/network/socket_option_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,42 @@ class SocketOptionImplTest : public testing::Test {
NiceMock<MockListenSocket> socket_;
Api::MockOsSysCalls os_sys_calls_;
TestThreadsafeSingletonInjector<Api::OsSysCallsImpl> os_calls{&os_sys_calls_};

void testSetSocketOptionSuccess(SocketOptionImpl& socket_option, int socket_level,
Network::SocketOptionName option_name, int option_val,
const std::set<Socket::SocketState>& when) {
Address::Ipv4Instance address("1.2.3.4", 5678);
const int fd = address.socket(Address::SocketType::Stream);
EXPECT_CALL(socket_, fd()).WillRepeatedly(Return(fd));

for (Socket::SocketState state : when) {
if (option_name.has_value()) {
EXPECT_CALL(os_sys_calls_,
setsockopt_(_, socket_level, option_name.value(), _, sizeof(int)))
.WillOnce(Invoke([option_val](int, int, int, const void* optval, socklen_t) -> int {
EXPECT_EQ(option_val, *static_cast<const int*>(optval));
return 0;
}));
EXPECT_TRUE(socket_option.setOption(socket_, state));
} else {
EXPECT_FALSE(socket_option.setOption(socket_, state));
}
}

// The set of SocketState for which this option should not be set. Initialize to all
// the states, and remove states that are passed in.
std::list<Socket::SocketState> unset_socketstates{
Socket::SocketState::PreBind,
Socket::SocketState::PostBind,
Socket::SocketState::Listening,
};
unset_socketstates.remove_if(
[&](Socket::SocketState state) -> bool { return when.find(state) != when.end(); });
for (Socket::SocketState state : unset_socketstates) {
EXPECT_CALL(os_sys_calls_, setsockopt_(_, _, _, _, _)).Times(0);
EXPECT_TRUE(socket_option.setOption(socket_, state));
}
}
};

// We fail to set the option if the socket FD is bad.
Expand All @@ -33,106 +69,74 @@ TEST_F(SocketOptionImplTest, BadFd) {

// Nop when there are no socket options set.
TEST_F(SocketOptionImplTest, SetOptionEmptyNop) {
SocketOptionImpl socket_option{{}, {}};
SocketOptionImpl socket_option{{}, {}, {}};
EXPECT_TRUE(socket_option.setOption(socket_, Socket::SocketState::PreBind));
EXPECT_TRUE(socket_option.setOption(socket_, Socket::SocketState::PostBind));
EXPECT_TRUE(socket_option.setOption(socket_, Socket::SocketState::Listening));
}

// We fail to set the option when the underlying setsockopt syscall fails.
TEST_F(SocketOptionImplTest, SetOptionTransparentFailure) {
SocketOptionImpl socket_option{true, {}};
SocketOptionImpl socket_option{true, {}, {}};
EXPECT_FALSE(socket_option.setOption(socket_, Socket::SocketState::PreBind));
}

// We fail to set the option when the underlying setsockopt syscall fails.
TEST_F(SocketOptionImplTest, SetOptionFreebindFailure) {
SocketOptionImpl socket_option{{}, true};
SocketOptionImpl socket_option{{}, true, {}};
EXPECT_FALSE(socket_option.setOption(socket_, Socket::SocketState::PreBind));
}

// We fail to set the tcp-fastopen option when the underlying setsockopt syscall fails.
TEST_F(SocketOptionImplTest, SetOptionTcpFastopenFailure) {
if (ENVOY_SOCKET_TCP_FASTOPEN.has_value()) {
SocketOptionImpl socket_option{{}, {}, 1};
EXPECT_CALL(os_sys_calls_, setsockopt_(_, IPPROTO_TCP, ENVOY_SOCKET_TCP_FASTOPEN.value(), _, _))
.WillOnce(Return(-1));
EXPECT_FALSE(socket_option.setOption(socket_, Socket::SocketState::Listening));
}
}

// The happy path for setOption(); IP_TRANSPARENT is set to true.
TEST_F(SocketOptionImplTest, SetOptionTransparentSuccessTrue) {
SocketOptionImpl socket_option{true, {}};
if (ENVOY_SOCKET_IP_TRANSPARENT.has_value()) {
Address::Ipv4Instance address("1.2.3.4", 5678);
const int fd = address.socket(Address::SocketType::Stream);
EXPECT_CALL(socket_, fd()).WillRepeatedly(Return(fd));
EXPECT_CALL(os_sys_calls_,
setsockopt_(_, IPPROTO_IP, ENVOY_SOCKET_IP_TRANSPARENT.value(), _, sizeof(int)))
.WillOnce(Invoke([](int, int, int, const void* optval, socklen_t) -> int {
EXPECT_EQ(1, *static_cast<const int*>(optval));
return 0;
}));
EXPECT_TRUE(socket_option.setOption(socket_, Socket::SocketState::PreBind));
} else {
EXPECT_FALSE(socket_option.setOption(socket_, Socket::SocketState::PreBind));
}
SocketOptionImpl socket_option{true, {}, {}};
testSetSocketOptionSuccess(socket_option, IPPROTO_IP, ENVOY_SOCKET_IP_TRANSPARENT, 1,
{Socket::SocketState::PreBind, Socket::SocketState::PostBind});
}

// The happy path for setOption(); IP_FREEBIND is set to true.
TEST_F(SocketOptionImplTest, SetOptionFreebindSuccessTrue) {
SocketOptionImpl socket_option{{}, true};
if (ENVOY_SOCKET_IP_FREEBIND.has_value()) {
Address::Ipv4Instance address("1.2.3.4", 5678);
const int fd = address.socket(Address::SocketType::Stream);
EXPECT_CALL(socket_, fd()).WillRepeatedly(Return(fd));
EXPECT_CALL(os_sys_calls_,
setsockopt_(_, IPPROTO_IP, ENVOY_SOCKET_IP_FREEBIND.value(), _, sizeof(int)))
.WillOnce(Invoke([](int, int, int, const void* optval, socklen_t) -> int {
EXPECT_EQ(1, *static_cast<const int*>(optval));
return 0;
}));
EXPECT_TRUE(socket_option.setOption(socket_, Socket::SocketState::PreBind));
} else {
EXPECT_FALSE(socket_option.setOption(socket_, Socket::SocketState::PreBind));
}
SocketOptionImpl socket_option{{}, true, {}};
testSetSocketOptionSuccess(socket_option, IPPROTO_IP, ENVOY_SOCKET_IP_FREEBIND, 1,
{Socket::SocketState::PreBind});
}

// The happy path for setOpion(); IP_TRANSPARENT is set to false.
// The happy path for setOption(); TCP_FASTOPEN is set to true.
TEST_F(SocketOptionImplTest, SetOptionTcpFastopenSuccessTrue) {
SocketOptionImpl socket_option{{}, {}, 42};
testSetSocketOptionSuccess(socket_option, IPPROTO_TCP, ENVOY_SOCKET_TCP_FASTOPEN, 42,
{Socket::SocketState::Listening});
}

// The happy path for setOption(); IP_TRANSPARENT is set to false.
TEST_F(SocketOptionImplTest, SetOptionTransparentSuccessFalse) {
SocketOptionImpl socket_option{false, {}};
if (ENVOY_SOCKET_IP_TRANSPARENT.has_value()) {
Address::Ipv4Instance address("1.2.3.4", 5678);
const int fd = address.socket(Address::SocketType::Stream);
EXPECT_CALL(socket_, fd()).WillRepeatedly(Return(fd));
EXPECT_CALL(os_sys_calls_,
setsockopt_(_, IPPROTO_IP, ENVOY_SOCKET_IP_TRANSPARENT.value(), _, sizeof(int)))
.Times(2)
.WillRepeatedly(Invoke([](int, int, int, const void* optval, socklen_t) -> int {
EXPECT_EQ(0, *static_cast<const int*>(optval));
return 0;
}));
EXPECT_TRUE(socket_option.setOption(socket_, Socket::SocketState::PreBind));
EXPECT_TRUE(socket_option.setOption(socket_, Socket::SocketState::PostBind));
} else {
EXPECT_FALSE(socket_option.setOption(socket_, Socket::SocketState::PreBind));
EXPECT_FALSE(socket_option.setOption(socket_, Socket::SocketState::PostBind));
}
SocketOptionImpl socket_option{false, {}, {}};
testSetSocketOptionSuccess(socket_option, IPPROTO_IP, ENVOY_SOCKET_IP_TRANSPARENT, 0,
{Socket::SocketState::PreBind, Socket::SocketState::PostBind});
}

// The happy path for setOpion(); IP_FREEBIND is set to false.
// The happy path for setOption(); IP_FREEBIND is set to false.
TEST_F(SocketOptionImplTest, SetOptionFreebindSuccessFalse) {
SocketOptionImpl socket_option{{}, false};
if (ENVOY_SOCKET_IP_FREEBIND.has_value()) {
Address::Ipv4Instance address("1.2.3.4", 5678);
const int fd = address.socket(Address::SocketType::Stream);
EXPECT_CALL(socket_, fd()).WillRepeatedly(Return(fd));
EXPECT_CALL(os_sys_calls_,
setsockopt_(_, IPPROTO_IP, ENVOY_SOCKET_IP_FREEBIND.value(), _, sizeof(int)))
.WillOnce(Invoke([](int, int, int, const void* optval, socklen_t) -> int {
EXPECT_EQ(0, *static_cast<const int*>(optval));
return 0;
}));
EXPECT_TRUE(socket_option.setOption(socket_, Socket::SocketState::PreBind));
} else {
EXPECT_FALSE(socket_option.setOption(socket_, Socket::SocketState::PreBind));
}
SocketOptionImpl socket_option{{}, false, {}};
testSetSocketOptionSuccess(socket_option, IPPROTO_IP, ENVOY_SOCKET_IP_FREEBIND, 0,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice cleanup!

{Socket::SocketState::PreBind});
}

// Freebind settings have no effect on post-bind behavior.
TEST_F(SocketOptionImplTest, SetOptionFreebindPostBind) {
SocketOptionImpl socket_option{{}, true};
EXPECT_TRUE(socket_option.setOption(socket_, Socket::SocketState::PostBind));
// The happy path for setOpion(); TCP_FASTOPEN is set to false.
TEST_F(SocketOptionImplTest, SetOptionTcpFastopenSuccessFalse) {
SocketOptionImpl socket_option{{}, {}, 0};
testSetSocketOptionSuccess(socket_option, IPPROTO_TCP, ENVOY_SOCKET_TCP_FASTOPEN, 0,
{Socket::SocketState::Listening});
}

// If a platform doesn't suppport IPv4 socket option variant for an IPv4 address, we fail
Expand Down