diff --git a/source/common/network/socket_option_factory.cc b/source/common/network/socket_option_factory.cc index 8655d5ac971a..e6ed92c56e73 100644 --- a/source/common/network/socket_option_factory.cc +++ b/source/common/network/socket_option_factory.cc @@ -61,6 +61,14 @@ std::unique_ptr SocketOptionFactory::buildSocketMarkOptions(uin return options; } +std::unique_ptr SocketOptionFactory::buildSocketNoSigpipeOptions() { + // Provide additional handling for `SIGPIPE` at the socket layer by converting it to `EPIPE`. + std::unique_ptr options = std::make_unique(); + options->push_back(std::make_shared( + envoy::config::core::v3::SocketOption::STATE_PREBIND, ENVOY_SOCKET_SO_NOSIGPIPE, 1)); + return options; +} + std::unique_ptr SocketOptionFactory::buildLiteralOptions( const Protobuf::RepeatedPtrField& socket_options) { auto options = std::make_unique(); diff --git a/source/common/network/socket_option_factory.h b/source/common/network/socket_option_factory.h index 1da9f3c1780e..72c67bfd8996 100644 --- a/source/common/network/socket_option_factory.h +++ b/source/common/network/socket_option_factory.h @@ -26,6 +26,7 @@ class SocketOptionFactory : Logger::Loggable { static std::unique_ptr buildIpFreebindOptions(); static std::unique_ptr buildIpTransparentOptions(); static std::unique_ptr buildSocketMarkOptions(uint32_t mark); + static std::unique_ptr buildSocketNoSigpipeOptions(); static std::unique_ptr buildTcpFastOpenOptions(uint32_t queue_length); static std::unique_ptr buildLiteralOptions( const Protobuf::RepeatedPtrField& socket_options); diff --git a/source/common/network/socket_option_impl.h b/source/common/network/socket_option_impl.h index 69b4989b20a4..ce6ebb926579 100644 --- a/source/common/network/socket_option_impl.h +++ b/source/common/network/socket_option_impl.h @@ -47,6 +47,12 @@ namespace Network { #define ENVOY_SOCKET_SO_MARK Network::SocketOptionName() #endif +#ifdef SO_NOSIGPIPE +#define ENVOY_SOCKET_SO_NOSIGPIPE ENVOY_MAKE_SOCKET_OPTION_NAME(SOL_SOCKET, SO_NOSIGPIPE) +#else +#define ENVOY_SOCKET_SO_NOSIGPIPE Network::SocketOptionName() +#endif + #ifdef SO_REUSEPORT #define ENVOY_SOCKET_SO_REUSEPORT ENVOY_MAKE_SOCKET_OPTION_NAME(SOL_SOCKET, SO_REUSEPORT) #else diff --git a/source/common/upstream/BUILD b/source/common/upstream/BUILD index 50effb27094a..2d0fb940cf00 100644 --- a/source/common/upstream/BUILD +++ b/source/common/upstream/BUILD @@ -440,6 +440,7 @@ envoy_cc_library( "//source/common/network:address_lib", "//source/common/network:resolver_lib", "//source/common/network:socket_option_factory_lib", + "//source/common/network:socket_option_lib", "//source/common/network:utility_lib", "//source/common/protobuf", "//source/common/protobuf:utility_lib", diff --git a/source/common/upstream/upstream_impl.cc b/source/common/upstream/upstream_impl.cc index 69ef701a5cf1..269745bd765a 100644 --- a/source/common/upstream/upstream_impl.cc +++ b/source/common/upstream/upstream_impl.cc @@ -37,6 +37,7 @@ #include "common/network/address_impl.h" #include "common/network/resolver_impl.h" #include "common/network/socket_option_factory.h" +#include "common/network/socket_option_impl.h" #include "common/protobuf/protobuf.h" #include "common/protobuf/utility.h" #include "common/router/config_utility.h" @@ -102,6 +103,12 @@ parseClusterSocketOptions(const envoy::config::cluster::v3::Cluster& config, const envoy::config::core::v3::BindConfig bind_config) { Network::ConnectionSocket::OptionsSharedPtr cluster_options = std::make_shared(); + // The process-wide `signal()` handling may fail to handle SIGPIPE if overridden + // in the process (i.e., on a mobile client). Some OSes support handling it at the socket layer: + if (ENVOY_SOCKET_SO_NOSIGPIPE.hasValue()) { + Network::Socket::appendOptions(cluster_options, + Network::SocketOptionFactory::buildSocketNoSigpipeOptions()); + } // Cluster IP_FREEBIND settings, when set, will override the cluster manager wide settings. if ((bind_config.freebind().value() && !config.upstream_bind_config().has_freebind()) || config.upstream_bind_config().freebind().value()) { diff --git a/source/server/BUILD b/source/server/BUILD index 16e4ddfb8d63..33266f13f3f8 100644 --- a/source/server/BUILD +++ b/source/server/BUILD @@ -47,6 +47,7 @@ envoy_cc_library( "//source/common/config:utility_lib", "//source/common/network:resolver_lib", "//source/common/network:socket_option_factory_lib", + "//source/common/network:socket_option_lib", "//source/common/network:utility_lib", "//source/common/protobuf:utility_lib", "@envoy_api//envoy/config/bootstrap/v3:pkg_cc_proto", diff --git a/source/server/listener_impl.cc b/source/server/listener_impl.cc index 2e46bca84d12..bd6c81b6c62e 100644 --- a/source/server/listener_impl.cc +++ b/source/server/listener_impl.cc @@ -17,6 +17,7 @@ #include "common/network/connection_balancer_impl.h" #include "common/network/resolver_impl.h" #include "common/network/socket_option_factory.h" +#include "common/network/socket_option_impl.h" #include "common/network/utility.h" #include "common/protobuf/utility.h" #include "common/runtime/runtime_features.h" @@ -372,6 +373,11 @@ void ListenerImpl::buildUdpListenerFactory(Network::Socket::Type socket_type, } void ListenerImpl::buildListenSocketOptions(Network::Socket::Type socket_type) { + // The process-wide `signal()` handling may fail to handle SIGPIPE if overridden + // in the process (i.e., on a mobile client). Some OSes support handling it at the socket layer: + if (ENVOY_SOCKET_SO_NOSIGPIPE.hasValue()) { + addListenSocketOptions(Network::SocketOptionFactory::buildSocketNoSigpipeOptions()); + } if (PROTOBUF_GET_WRAPPED_OR_DEFAULT(config_, transparent, false)) { addListenSocketOptions(Network::SocketOptionFactory::buildIpTransparentOptions()); } diff --git a/test/common/upstream/cluster_manager_impl_test.cc b/test/common/upstream/cluster_manager_impl_test.cc index a5fb36b1f98e..ae2c282fdd4f 100644 --- a/test/common/upstream/cluster_manager_impl_test.cc +++ b/test/common/upstream/cluster_manager_impl_test.cc @@ -67,7 +67,7 @@ class ClusterManagerImplTest : public testing::Test { address: socket_address: address: 127.0.0.1 - port_value: 11002 + port_value: 11002 )EOF"; const std::string merge_window_enabled = R"EOF( common_lb_config: @@ -3125,6 +3125,8 @@ TEST_F(ClusterManagerInitHelperTest, RemoveClusterWithinInitLoop) { init_helper_.startInitializingSecondaryClusters(); } +using NameVals = std::vector>; + // Validate that when options are set in the ClusterManager and/or Cluster, we see the socket option // propagated to setsockopt(). This is as close to an end-to-end test as we have for this feature, // due to the complexity of creating an integration test involving the network stack. We only test @@ -3137,8 +3139,7 @@ class SockoptsTest : public ClusterManagerImplTest { void TearDown() override { factory_.tls_.shutdownThread(); } // TODO(tschroed): Extend this to support socket state as well. - void expectSetsockopts(const std::vector>& names_vals) { - + void expectSetsockopts(const NameVals& names_vals) { NiceMock os_sys_calls; TestThreadsafeSingletonInjector os_calls(&os_sys_calls); NiceMock socket; @@ -3181,8 +3182,15 @@ class SockoptsTest : public ClusterManagerImplTest { } void expectSetsockoptFreebind() { - std::vector> names_vals{ - {ENVOY_SOCKET_IP_FREEBIND, 1}}; + NameVals names_vals{{ENVOY_SOCKET_IP_FREEBIND, 1}}; + if (ENVOY_SOCKET_SO_NOSIGPIPE.hasValue()) { + names_vals.emplace_back(std::make_pair(ENVOY_SOCKET_SO_NOSIGPIPE, 1)); + } + expectSetsockopts(names_vals); + } + + void expectOnlyNoSigpipeOptions() { + NameVals names_vals{{std::make_pair(ENVOY_SOCKET_SO_NOSIGPIPE, 1)}}; expectSetsockopts(names_vals); } @@ -3222,7 +3230,11 @@ TEST_F(SockoptsTest, SockoptsUnset) { port_value: 11001 )EOF"; initialize(yaml); - expectNoSocketOptions(); + if (ENVOY_SOCKET_SO_NOSIGPIPE.hasValue()) { + expectOnlyNoSigpipeOptions(); + } else { + expectNoSocketOptions(); + } } TEST_F(SockoptsTest, FreebindClusterOnly) { @@ -3325,8 +3337,11 @@ TEST_F(SockoptsTest, SockoptsClusterOnly) { )EOF"; initialize(yaml); - std::vector> names_vals{ - {ENVOY_MAKE_SOCKET_OPTION_NAME(1, 2), 3}, {ENVOY_MAKE_SOCKET_OPTION_NAME(4, 5), 6}}; + NameVals names_vals{{ENVOY_MAKE_SOCKET_OPTION_NAME(1, 2), 3}, + {ENVOY_MAKE_SOCKET_OPTION_NAME(4, 5), 6}}; + if (ENVOY_SOCKET_SO_NOSIGPIPE.hasValue()) { + names_vals.emplace_back(std::make_pair(ENVOY_SOCKET_SO_NOSIGPIPE, 1)); + } expectSetsockopts(names_vals); } @@ -3354,8 +3369,11 @@ TEST_F(SockoptsTest, SockoptsClusterManagerOnly) { { level: 4, name: 5, int_value: 6, state: STATE_PREBIND }] )EOF"; initialize(yaml); - std::vector> names_vals{ - {ENVOY_MAKE_SOCKET_OPTION_NAME(1, 2), 3}, {ENVOY_MAKE_SOCKET_OPTION_NAME(4, 5), 6}}; + NameVals names_vals{{ENVOY_MAKE_SOCKET_OPTION_NAME(1, 2), 3}, + {ENVOY_MAKE_SOCKET_OPTION_NAME(4, 5), 6}}; + if (ENVOY_SOCKET_SO_NOSIGPIPE.hasValue()) { + names_vals.emplace_back(std::make_pair(ENVOY_SOCKET_SO_NOSIGPIPE, 1)); + } expectSetsockopts(names_vals); } @@ -3385,8 +3403,11 @@ TEST_F(SockoptsTest, SockoptsClusterOverride) { socket_options: [{ level: 7, name: 8, int_value: 9, state: STATE_PREBIND }] )EOF"; initialize(yaml); - std::vector> names_vals{ - {ENVOY_MAKE_SOCKET_OPTION_NAME(1, 2), 3}, {ENVOY_MAKE_SOCKET_OPTION_NAME(4, 5), 6}}; + NameVals names_vals{{ENVOY_MAKE_SOCKET_OPTION_NAME(1, 2), 3}, + {ENVOY_MAKE_SOCKET_OPTION_NAME(4, 5), 6}}; + if (ENVOY_SOCKET_SO_NOSIGPIPE.hasValue()) { + names_vals.emplace_back(std::make_pair(ENVOY_SOCKET_SO_NOSIGPIPE, 1)); + } expectSetsockopts(names_vals); } @@ -3435,6 +3456,14 @@ class TcpKeepaliveTest : public ClusterManagerImplTest { options, socket, envoy::config::core::v3::SocketOption::STATE_PREBIND))); return connection_; })); + if (ENVOY_SOCKET_SO_NOSIGPIPE.hasValue()) { + EXPECT_CALL(socket, setSocketOption(ENVOY_SOCKET_SO_NOSIGPIPE.level(), + ENVOY_SOCKET_SO_NOSIGPIPE.option(), _, sizeof(int))) + .WillOnce(Invoke([](int, int, const void* optval, socklen_t) -> Api::SysCallIntResult { + EXPECT_EQ(1, *static_cast(optval)); + return {0, 0}; + })); + } EXPECT_CALL(socket, setSocketOption(ENVOY_SOCKET_SO_KEEPALIVE.level(), ENVOY_SOCKET_SO_KEEPALIVE.option(), _, sizeof(int))) .WillOnce(Invoke([](int, int, const void* optval, socklen_t) -> Api::SysCallIntResult { @@ -3472,6 +3501,29 @@ class TcpKeepaliveTest : public ClusterManagerImplTest { EXPECT_EQ(connection_, conn_data.connection_.get()); } + void expectOnlyNoSigpipeOptions() { + NiceMock socket; + EXPECT_CALL(factory_.tls_.dispatcher_, createClientConnection_(_, _, _, _)) + .WillOnce(Invoke([this, &socket](Network::Address::InstanceConstSharedPtr, + Network::Address::InstanceConstSharedPtr, + Network::TransportSocketPtr&, + const Network::ConnectionSocket::OptionsSharedPtr& options) + -> Network::ClientConnection* { + EXPECT_NE(nullptr, options.get()); + EXPECT_TRUE((Network::Socket::applyOptions( + options, socket, envoy::config::core::v3::SocketOption::STATE_PREBIND))); + return connection_; + })); + EXPECT_CALL(socket, setSocketOption(ENVOY_SOCKET_SO_NOSIGPIPE.level(), + ENVOY_SOCKET_SO_NOSIGPIPE.option(), _, sizeof(int))) + .WillOnce(Invoke([](int, int, const void* optval, socklen_t) -> Api::SysCallIntResult { + EXPECT_EQ(1, *static_cast(optval)); + return {0, 0}; + })); + auto conn_data = cluster_manager_->tcpConnForCluster("TcpKeepaliveCluster", nullptr); + EXPECT_EQ(connection_, conn_data.connection_.get()); + } + void expectNoSocketOptions() { EXPECT_CALL(factory_.tls_.dispatcher_, createClientConnection_(_, _, _, _)) .WillOnce( @@ -3508,7 +3560,11 @@ TEST_F(TcpKeepaliveTest, TcpKeepaliveUnset) { port_value: 11001 )EOF"; initialize(yaml); - expectNoSocketOptions(); + if (ENVOY_SOCKET_SO_NOSIGPIPE.hasValue()) { + expectOnlyNoSigpipeOptions(); + } else { + expectNoSocketOptions(); + } } TEST_F(TcpKeepaliveTest, TcpKeepaliveCluster) {