Skip to content
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.

Sign up for GitHub

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

listen_socket: Support multiple options and allow socket options to be set before bind(). #2734

Merged
merged 22 commits into from
Mar 25, 2018
Merged

listen_socket: Support multiple options and allow socket options to be set before bind(). #2734

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
3109a2c
listen_socket: Allow socket options to be set before bind().
jrajahalme Mar 13, 2018
3495654
listen_socket: Support multiple socket options.
jrajahalme Mar 14, 2018
1b58b6f
listener: Add support for LDS API "transparent" option.
jrajahalme Mar 14, 2018
a8fed75
listen_socket: Use a vector instead of a list for options.
jrajahalme Mar 14, 2018
824d8c7
cluster_manager: Fix socket options passing.
jrajahalme Mar 14, 2018
fa23036
cluster_manager: Fix format.
jrajahalme Mar 15, 2018
66b1ca3
listener: Use enum for socket state, fix IP_TRANSPARENT handling.
jrajahalme Mar 15, 2018
7524e53
listener_manager: Fix nits.
jrajahalme Mar 15, 2018
d569fec
listener_manager: Fix errno value.
jrajahalme Mar 16, 2018
a71b544
listener_manager: Fix setting socket options for non-IP sockets.
jrajahalme Mar 16, 2018
d8664f5
listener_manager: Remove throw and clean up the option setting code.
jrajahalme Mar 16, 2018
5650236
Merge master, update envoy API reference
jrajahalme Mar 19, 2018
07792b7
listener_manager: Update to optional "transparent" option.
jrajahalme Mar 19, 2018
33dda97
listener_manager: Use absl::optional for 'transparent_', fix format.
jrajahalme Mar 20, 2018
8cf748f
Merge branch 'master' into socket-options-before-and-after-bind
jrajahalme Mar 20, 2018
13887b8
listen_socket: Rename Socket::setOption() as Socket::addOption().
jrajahalme Mar 20, 2018
4dc4b04
listen_socket: Transfer ownership of options to the Socket.
jrajahalme Mar 20, 2018
b154e73
test: Don't use get() on unique_ptr's when not needed.
jrajahalme Mar 20, 2018
14a9ec7
os_sys_calls: add setsockopt() and getsockopt().
jrajahalme Mar 21, 2018
da28038
listener_manager_impl_test: Fix build when IP_TRANSPARENT option is n…
jrajahalme Mar 21, 2018
a46328b
listener_manager: Remove 'transparent' handling from this PR.
jrajahalme Mar 23, 2018
d48f9b6
listen_socket: Fix comment.
jrajahalme Mar 23, 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
10 changes: 5 additions & 5 deletions include/envoy/network/listen_socket.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,16 +56,16 @@ class Socket {
*/
virtual void hashKey(std::vector<uint8_t>& key) const PURE;
};
typedef std::shared_ptr<Option> OptionSharedPtr;
typedef std::shared_ptr<std::vector<OptionSharedPtr>> OptionsSharedPtr;
typedef std::unique_ptr<Option> OptionPtr;
typedef std::shared_ptr<std::vector<OptionPtr>> OptionsSharedPtr;

/**
* Set the socket options for later retrieval with options().
* Add a socket option visitor for later retrieval with options().
*/
virtual void setOption(const OptionSharedPtr&) PURE;
virtual void addOption(OptionPtr&&) PURE;

/**
* @return the socket options stored earlier with setOption() calls, if any.
* @return the socket options stored earlier with addOption() calls, if any.
*/
virtual const OptionsSharedPtr& options() const PURE;
};
Expand Down
2 changes: 1 addition & 1 deletion include/envoy/server/filter_config.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ class ListenerFactoryContext : public FactoryContext {
/**
* Store socket options to be set on the listen socket before listening.
*/
virtual void setListenSocketOption(const Network::Socket::OptionSharedPtr& option) PURE;
virtual void addListenSocketOption(Network::Socket::OptionPtr&& option) PURE;
};

/**
Expand Down
6 changes: 3 additions & 3 deletions source/common/network/listen_socket_impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,11 @@ class SocketImpl : public virtual Socket {
fd_ = -1;
}
}
void setOption(const OptionSharedPtr& option) override {
void addOption(OptionPtr&& option) override {
if (!options_) {
options_ = std::make_shared<std::vector<OptionSharedPtr>>();
options_ = std::make_shared<std::vector<OptionPtr>>();
}
options_->emplace_back(option);
options_->emplace_back(std::move(option));
}
const OptionsSharedPtr& options() const override { return options_; }

Expand Down
2 changes: 1 addition & 1 deletion source/server/listener_manager_impl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ ListenerImpl::ListenerImpl(const envoy::api::v2::Listener& config, ListenerManag

// Add listen socket options from the config.
if (config.has_transparent()) {
setListenSocketOption(std::make_shared<ListenerSocketOption>(config));
addListenSocketOption(std::make_unique<ListenerSocketOption>(config));
}

if (!config.listener_filters().empty()) {
Expand Down
6 changes: 3 additions & 3 deletions source/server/listener_manager_impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,11 +249,11 @@ class ListenerImpl : public Network::ListenerConfig,
ThreadLocal::Instance& threadLocal() override { return parent_.server_.threadLocal(); }
Admin& admin() override { return parent_.server_.admin(); }
const envoy::api::v2::core::Metadata& listenerMetadata() const override { return metadata_; };
void setListenSocketOption(const Network::Socket::OptionSharedPtr& option) override {
void addListenSocketOption(Network::Socket::OptionPtr&& option) override {
if (!listen_socket_options_) {
listen_socket_options_ = std::make_shared<std::vector<Network::Socket::OptionSharedPtr>>();
listen_socket_options_ = std::make_shared<std::vector<Network::Socket::OptionPtr>>();
}
listen_socket_options_->emplace_back(option);
listen_socket_options_->emplace_back(std::move(option));
}

// Network::DrainDecision
Expand Down
14 changes: 8 additions & 6 deletions test/common/network/connection_impl_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,12 +252,13 @@ TEST_P(ConnectionImplTest, SocketOptions) {

read_filter_.reset(new NiceMock<MockReadFilter>());

auto option = std::make_shared<MockSocketOption>();
auto option = std::make_unique<MockSocketOption>();

EXPECT_CALL(*option, setOption(_, Network::Socket::SocketState::PreBind)).WillOnce(Return(true));
EXPECT_CALL(*(option.get()), setOption(_, Network::Socket::SocketState::PreBind))
.WillOnce(Return(true));
EXPECT_CALL(listener_callbacks_, onAccept_(_, _))
.WillOnce(Invoke([&](Network::ConnectionSocketPtr& socket, bool) -> void {
socket->setOption(option);
socket->addOption(std::move(option));
Network::ConnectionPtr new_connection = dispatcher_->createServerConnection(
std::move(socket), Network::Test::createRawBufferSocket());
listener_callbacks_.onNewConnection(std::move(new_connection));
Expand Down Expand Up @@ -300,12 +301,13 @@ TEST_P(ConnectionImplTest, SocketOptionsFailureTest) {

read_filter_.reset(new NiceMock<MockReadFilter>());

auto option = std::make_shared<MockSocketOption>();
auto option = std::make_unique<MockSocketOption>();

EXPECT_CALL(*option, setOption(_, Network::Socket::SocketState::PreBind)).WillOnce(Return(false));
EXPECT_CALL(*(option.get()), setOption(_, Network::Socket::SocketState::PreBind))
.WillOnce(Return(false));
EXPECT_CALL(listener_callbacks_, onAccept_(_, _))
.WillOnce(Invoke([&](Network::ConnectionSocketPtr& socket, bool) -> void {
socket->setOption(option);
socket->addOption(std::move(option));
Network::ConnectionPtr new_connection = dispatcher_->createServerConnection(
std::move(socket), Network::Test::createRawBufferSocket());
listener_callbacks_.onNewConnection(std::move(new_connection));
Expand Down
18 changes: 10 additions & 8 deletions test/common/network/listen_socket_impl_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,19 +42,21 @@ TEST_P(ListenSocketImplTest, BindSpecificPort) {
// bind failure (in the TcpListenSocket ctor) once isn't considered an error.
EXPECT_EQ(0, close(addr_fd.second));

auto option = std::make_shared<MockSocketOption>();
auto options = std::make_shared<std::vector<Network::Socket::OptionSharedPtr>>();
options->emplace_back(option);
EXPECT_CALL(*option, setOption(_, Network::Socket::SocketState::PreBind)).WillOnce(Return(true));
auto option = std::make_unique<MockSocketOption>();
auto options = std::make_shared<std::vector<Network::Socket::OptionPtr>>();
EXPECT_CALL(*(option.get()), setOption(_, Network::Socket::SocketState::PreBind))
.WillOnce(Return(true));
options->emplace_back(std::move(option));
TcpListenSocket socket1(addr, options, true);
EXPECT_EQ(0, listen(socket1.fd(), 0));
EXPECT_EQ(addr->ip()->port(), socket1.localAddress()->ip()->port());
EXPECT_EQ(addr->ip()->addressAsString(), socket1.localAddress()->ip()->addressAsString());

auto option2 = std::make_shared<MockSocketOption>();
auto options2 = std::make_shared<std::vector<Network::Socket::OptionSharedPtr>>();
options2->emplace_back(option2);
EXPECT_CALL(*option2, setOption(_, Network::Socket::SocketState::PreBind)).WillOnce(Return(true));
auto option2 = std::make_unique<MockSocketOption>();
auto options2 = std::make_shared<std::vector<Network::Socket::OptionPtr>>();
EXPECT_CALL(*(option2.get()), setOption(_, Network::Socket::SocketState::PreBind))
.WillOnce(Return(true));
options2->emplace_back(std::move(option2));
// The address and port are bound already, should throw exception.
EXPECT_THROW(Network::TcpListenSocket socket2(addr, options2, true), EnvoyException);

Expand Down
9 changes: 5 additions & 4 deletions test/common/network/listener_impl_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,11 +163,12 @@ TEST_P(ListenerImplTest, WildcardListenerUseActualDst) {
TEST_P(ListenerImplTest, WildcardListenerIpv4Compat) {
Stats::IsolatedStoreImpl stats_store;
Event::DispatcherImpl dispatcher;
auto option = std::make_shared<MockSocketOption>();
auto options = std::make_shared<std::vector<Network::Socket::OptionSharedPtr>>();
options->emplace_back(option);
auto option = std::make_unique<MockSocketOption>();
auto options = std::make_shared<std::vector<Network::Socket::OptionPtr>>();
EXPECT_CALL(*(option.get()), setOption(_, Network::Socket::SocketState::PreBind))
.WillOnce(Return(true));
options->emplace_back(std::move(option));

EXPECT_CALL(*option, setOption(_, Network::Socket::SocketState::PreBind)).WillOnce(Return(true));
Network::TcpListenSocket socket(Network::Test::getAnyAddress(version_, true), options, true);
Network::MockListenerCallbacks listener_callbacks;
Network::MockConnectionHandler connection_handler;
Expand Down
8 changes: 6 additions & 2 deletions test/mocks/network/mocks.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,10 +258,12 @@ class MockListenSocket : public Socket {
MockListenSocket();
~MockListenSocket();

void addOption(Socket::OptionPtr&& option) override { addOption_(option); }

MOCK_CONST_METHOD0(localAddress, const Address::InstanceConstSharedPtr&());
MOCK_CONST_METHOD0(fd, int());
MOCK_METHOD0(close, void());
MOCK_METHOD1(setOption, void(const Socket::OptionSharedPtr& options));
MOCK_METHOD1(addOption_, void(Socket::OptionPtr& option));
MOCK_CONST_METHOD0(options, const OptionsSharedPtr&());

Address::InstanceConstSharedPtr local_address_;
Expand All @@ -282,12 +284,14 @@ class MockConnectionSocket : public ConnectionSocket {
MockConnectionSocket();
~MockConnectionSocket();

void addOption(Socket::OptionPtr&& option) override { addOption_(option); }

MOCK_CONST_METHOD0(localAddress, const Address::InstanceConstSharedPtr&());
MOCK_METHOD2(setLocalAddress, void(const Address::InstanceConstSharedPtr&, bool));
MOCK_CONST_METHOD0(localAddressRestored, bool());
MOCK_CONST_METHOD0(remoteAddress, const Address::InstanceConstSharedPtr&());
MOCK_METHOD1(setRemoteAddress, void(const Address::InstanceConstSharedPtr&));
MOCK_METHOD1(setOption, void(const Network::ConnectionSocket::OptionSharedPtr&));
MOCK_METHOD1(addOption_, void(Socket::OptionPtr&));
MOCK_CONST_METHOD0(options, const Network::ConnectionSocket::OptionsSharedPtr&());
MOCK_CONST_METHOD0(fd, int());
MOCK_METHOD0(close, void());
Expand Down
8 changes: 6 additions & 2 deletions test/mocks/server/mocks.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -369,12 +369,16 @@ class MockTransportSocketFactoryContext : public TransportSocketFactoryContext {
MOCK_CONST_METHOD0(statsScope, Stats::Scope&());
};

class MockListenerFactoryContext : public MockFactoryContext {
class MockListenerFactoryContext : public virtual MockFactoryContext,
public virtual ListenerFactoryContext {
public:
MockListenerFactoryContext();
~MockListenerFactoryContext();

MOCK_METHOD1(setListenSocketOption, void(const Network::Socket::OptionSharedPtr&));
void addListenSocketOption(Network::Socket::OptionPtr&& option) override {
addListenSocketOption_(option);
}
MOCK_METHOD1(addListenSocketOption_, void(Network::Socket::OptionPtr&));
};

} // namespace Configuration
Expand Down
20 changes: 7 additions & 13 deletions test/server/listener_manager_impl_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1267,20 +1267,19 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, OriginalDstTestFilter) {

class OriginalDstTestConfigFactory : public Configuration::NamedListenerFilterConfigFactory {
public:
OriginalDstTestConfigFactory() : option_(std::make_shared<Network::MockSocketOption>()) {}

// NamedListenerFilterConfigFactory
Configuration::ListenerFilterFactoryCb
createFilterFactoryFromProto(const Protobuf::Message&,
Configuration::ListenerFactoryContext& context) override {
EXPECT_CALL(*option_, setOption(_, Network::Socket::SocketState::PreBind))
auto option = std::make_unique<Network::MockSocketOption>();
EXPECT_CALL(*(option.get()), setOption(_, Network::Socket::SocketState::PreBind))
Copy link
Member

Choose a reason for hiding this comment

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

EXPECT_CALL(*option, setOption should work, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right, an explicit get() is not needed here. Will simplify this in a commit ASAP.

.WillOnce(Return(true));
EXPECT_CALL(*option_, setOption(_, Network::Socket::SocketState::PostBind))
EXPECT_CALL(*(option.get()), setOption(_, Network::Socket::SocketState::PostBind))
.WillOnce(Invoke([](Network::Socket& socket, Network::Socket::SocketState) -> bool {
fd = socket.fd();
return true;
}));
context.setListenSocketOption(option_);
context.addListenSocketOption(std::move(option));
return [](Network::ListenerFilterManager& filter_manager) -> void {
filter_manager.addAcceptFilter(std::make_unique<OriginalDstTest>());
};
Expand All @@ -1291,8 +1290,6 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, OriginalDstTestFilter) {
}

std::string name() override { return "test.listener.original_dst"; }

std::shared_ptr<Network::MockSocketOption> option_;
};

/**
Expand Down Expand Up @@ -1345,15 +1342,14 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, OriginalDstTestFilter) {
TEST_F(ListenerManagerImplWithRealFiltersTest, OriginalDstTestFilterOptionFail) {
class OriginalDstTestConfigFactory : public Configuration::NamedListenerFilterConfigFactory {
public:
OriginalDstTestConfigFactory() : option_(std::make_shared<Network::MockSocketOption>()) {}

// NamedListenerFilterConfigFactory
Configuration::ListenerFilterFactoryCb
createFilterFactoryFromProto(const Protobuf::Message&,
Configuration::ListenerFactoryContext& context) override {
EXPECT_CALL(*option_, setOption(_, Network::Socket::SocketState::PreBind))
auto option = std::make_unique<Network::MockSocketOption>();
EXPECT_CALL(*(option.get()), setOption(_, Network::Socket::SocketState::PreBind))
.WillOnce(Return(false));
context.setListenSocketOption(option_);
context.addListenSocketOption(std::move(option));
return [](Network::ListenerFilterManager& filter_manager) -> void {
filter_manager.addAcceptFilter(std::make_unique<OriginalDstTest>());
};
Expand All @@ -1364,8 +1360,6 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, OriginalDstTestFilterOptionFail)
}

std::string name() override { return "testfail.listener.original_dst"; }

std::shared_ptr<Network::MockSocketOption> option_;
};

/**
Expand Down