socket: Add socket type in SocketOption (envoyproxy#35285)

Commit Message: Add a socket `type` field in the `SocketOption` proto Additional Description: The `socket_option_impl.cc` implementation already has a logic to apply the socket option based on the socket type. This change is simply exposing the socket type filter in the `SocketOption` proto. Risk Level: low Testing: unit tests Docs Changes: updated Release Notes: updated Platform Specific Features: n/a --------- Signed-off-by: Fredy Wijaya <[email protected]>
daixiang0 · Jul 24, 2024 · 1d9910e · 1d9910e
1 parent 638a452
commit 1d9910e
Show file tree

Hide file tree

Showing 7 changed files with 167 additions and 3 deletions.
diff --git a/api/envoy/config/core/v3/socket_option.proto b/api/envoy/config/core/v3/socket_option.proto
@@ -36,7 +36,7 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE;
 // :ref:`admin's <envoy_v3_api_field_config.bootstrap.v3.Admin.socket_options>` socket_options etc.
 //
 // It should be noted that the name or level may have different values on different platforms.
-// [#next-free-field: 7]
+// [#next-free-field: 8]
 message SocketOption {
   option (udpa.annotations.versioning).previous_message_type = "envoy.api.v2.core.SocketOption";
 
@@ -51,6 +51,29 @@ message SocketOption {
     STATE_LISTENING = 2;
   }
 
+  // The `socket type <https://linux.die.net/man/2/socket>`_ to apply the socket option to.
+  // Only one field should be set. If multiple fields are set, the precedence order will determine
+  // the selected one. If none of the fields is set, the socket option will be applied to all socket types.
+  //
+  // For example:
+  // If :ref:`stream <envoy_v3_api_field_config.core.v3.SocketOption.SocketType.stream>` is set,
+  // it takes precedence over :ref:`datagram <envoy_v3_api_field_config.core.v3.SocketOption.SocketType.datagram>`.
+  message SocketType {
+    // The stream socket type.
+    message Stream {
+    }
+
+    // The datagram socket type.
+    message Datagram {
+    }
+
+    // Apply the socket option to the stream socket type.
+    Stream stream = 1;
+
+    // Apply the socket option to the datagram socket type.
+    Datagram datagram = 2;
+  }
+
   // An optional name to give this socket option for debugging, etc.
   // Uniqueness is not required and no special meaning is assumed.
   string description = 1;
@@ -74,6 +97,10 @@ message SocketOption {
   // The state in which the option will be applied. When used in BindConfig
   // STATE_PREBIND is currently the only valid value.
   SocketState state = 6 [(validate.rules).enum = {defined_only: true}];
+
+  // Apply the socket option to the specified `socket type <https://linux.die.net/man/2/socket>`_.
+  // If not specified, the socket option will be applied to all socket types.
+  SocketType type = 7;
 }
 
 message SocketOptionsOverride {

diff --git a/changelogs/current.yaml b/changelogs/current.yaml
@@ -68,5 +68,10 @@ new_features:
   change: |
     added %UPSTREAM_CLUSTER_RAW% access log formatter to log the original upstream cluster name, regadless of whether
     ``alt_stat_name`` is set.
+- area: sockets
+  change: |
+    Added socket ``type`` field for specifying a socket type to apply the socket option to under :ref:`SocketOption
+    <envoy_v3_api_msg_config.core.v3.SocketOption>`. If not specified, the socket option will be applied to all socket
+    types.
 
 deprecated:
diff --git a/source/common/network/socket_option_factory.cc b/source/common/network/socket_option_factory.cc
@@ -98,12 +98,24 @@ std::unique_ptr<Socket::Options> SocketOptionFactory::buildLiteralOptions(
                 socket_option.DebugString());
       continue;
     }
+
+    absl::optional<Network::Socket::Type> socket_type = absl::nullopt;
+    if (socket_option.has_type() && socket_option.type().has_stream()) {
+      if (socket_option.type().has_datagram()) {
+        ENVOY_LOG(
+            warn,
+            "Both Stream and Datagram socket types are set, setting the socket type to Stream.");
+      }
+      socket_type = Network::Socket::Type::Stream;
+    } else if (socket_option.has_type() && socket_option.type().has_datagram()) {
+      socket_type = Network::Socket::Type::Datagram;
+    }
     options->emplace_back(std::make_shared<Network::SocketOptionImpl>(
         socket_option.state(),
         Network::SocketOptionName(
             socket_option.level(), socket_option.name(),
             fmt::format("{}/{}", socket_option.level(), socket_option.name())),
-        buf));
+        buf, socket_type));
   }
   return options;
 }

diff --git a/source/common/network/socket_option_impl.cc b/source/common/network/socket_option_impl.cc
@@ -61,6 +61,8 @@ SocketOptionImpl::getOptionDetails(const Socket&,
 
 bool SocketOptionImpl::isSupported() const { return optname_.hasValue(); }
 
+absl::optional<Socket::Type> SocketOptionImpl::socketType() const { return socket_type_; }
+
 Api::SysCallIntResult SocketOptionImpl::setSocketOption(Socket& socket,
                                                         const Network::SocketOptionName& optname,
                                                         const void* value, size_t size) {

diff --git a/source/common/network/socket_option_impl.h b/source/common/network/socket_option_impl.h
@@ -38,6 +38,14 @@ class SocketOptionImpl : public Socket::Option, Logger::Loggable<Logger::Id::con
                    envoy::config::core::v3::SocketOption::SocketState state) const override;
   bool isSupported() const override;
 
+  /**
+   * Gets the socket type for this socket option. Empty means, the socket option is not specific to
+   * a particular socket type.
+   *
+   * @return the socket type
+   */
+  absl::optional<Network::Socket::Type> socketType() const;
+
   /**
    * Set the option on the given socket.
    * @param socket the socket on which to apply the option.

diff --git a/test/common/network/socket_option_factory_test.cc b/test/common/network/socket_option_factory_test.cc
@@ -148,6 +148,7 @@ TEST_F(SocketOptionFactoryTest, TestBuildLiteralOptions) {
   auto linger_option =
       absl::StrFormat(linger_option_format, SOL_SOCKET, SO_LINGER, linger_bstr_formatted);
   ASSERT_TRUE(parser.ParseFromString(linger_option, &socket_option_proto));
+
   *socket_options_proto.Add() = socket_option_proto;
   static const char keepalive_option_format[] = R"proto(
     state: STATE_PREBIND
@@ -159,8 +160,74 @@ TEST_F(SocketOptionFactoryTest, TestBuildLiteralOptions) {
   ASSERT_TRUE(parser.ParseFromString(keepalive_option, &socket_option_proto));
   *socket_options_proto.Add() = socket_option_proto;
 
+  static constexpr char socket_type_not_set_option_format[] = R"proto(
+    state: STATE_PREBIND
+    level: %d
+    name: %d
+    int_value: 1
+  )proto";
+  auto socket_type_not_set_textproto =
+      absl::StrFormat(socket_type_not_set_option_format, SOL_SOCKET, SO_KEEPALIVE);
+  ASSERT_TRUE(parser.ParseFromString(socket_type_not_set_textproto, &socket_option_proto));
+  *socket_options_proto.Add() = socket_option_proto;
+
+  static constexpr char empty_socket_type_option_format[] = R"proto(
+    state: STATE_PREBIND
+    level: %d
+    name: %d
+    int_value: 1
+    type: {}
+  )proto";
+  auto empty_socket_type_textproto =
+      absl::StrFormat(empty_socket_type_option_format, SOL_SOCKET, SO_KEEPALIVE);
+  ASSERT_TRUE(parser.ParseFromString(empty_socket_type_textproto, &socket_option_proto));
+  *socket_options_proto.Add() = socket_option_proto;
+
+  static constexpr char all_socket_types_set_option_format[] = R"proto(
+    state: STATE_PREBIND
+    level: %d
+    name: %d
+    int_value: 1
+    type: {
+      stream: {}
+      datagram: {}
+    }
+  )proto";
+  auto all_socket_types_set_textproto =
+      absl::StrFormat(all_socket_types_set_option_format, SOL_SOCKET, SO_KEEPALIVE);
+  ASSERT_TRUE(parser.ParseFromString(all_socket_types_set_textproto, &socket_option_proto));
+  *socket_options_proto.Add() = socket_option_proto;
+
+  static constexpr char stream_socket_type_option_format[] = R"proto(
+    state: STATE_PREBIND
+    level: %d
+    name: %d
+    int_value: 1
+    type: {
+      stream: {}
+    }
+  )proto";
+  auto stream_socket_type_textproto =
+      absl::StrFormat(stream_socket_type_option_format, SOL_SOCKET, SO_KEEPALIVE);
+  ASSERT_TRUE(parser.ParseFromString(stream_socket_type_textproto, &socket_option_proto));
+  *socket_options_proto.Add() = socket_option_proto;
+
+  static constexpr char datagram_socket_type_option_format[] = R"proto(
+    state: STATE_PREBIND
+    level: %d
+    name: %d
+    int_value: 1
+    type: {
+      datagram: {}
+    }
+  )proto";
+  auto datagram_socket_type_textproto =
+      absl::StrFormat(datagram_socket_type_option_format, SOL_SOCKET, SO_KEEPALIVE);
+  ASSERT_TRUE(parser.ParseFromString(datagram_socket_type_textproto, &socket_option_proto));
+  *socket_options_proto.Add() = socket_option_proto;
+
   auto socket_options = SocketOptionFactory::buildLiteralOptions(socket_options_proto);
-  EXPECT_EQ(2, socket_options->size());
+  EXPECT_EQ(7, socket_options->size());
   auto option_details = socket_options->at(0)->getOptionDetails(
       socket_mock_, envoy::config::core::v3::SocketOption::STATE_PREBIND);
   EXPECT_TRUE(option_details.has_value());
@@ -176,6 +243,29 @@ TEST_F(SocketOptionFactoryTest, TestBuildLiteralOptions) {
   int value = 1;
   absl::string_view value_bstr{reinterpret_cast<const char*>(&value), sizeof(int)};
   EXPECT_EQ(value_bstr, option_details->value_);
+
+  auto socket_type_not_set_option =
+      dynamic_pointer_cast<const SocketOptionImpl>(socket_options->at(2));
+  EXPECT_FALSE(socket_type_not_set_option->socketType().has_value());
+
+  auto empty_socket_type_option =
+      dynamic_pointer_cast<const SocketOptionImpl>(socket_options->at(3));
+  EXPECT_FALSE(empty_socket_type_option->socketType().has_value());
+
+  auto all_socket_types_option =
+      dynamic_pointer_cast<const SocketOptionImpl>(socket_options->at(4));
+  EXPECT_TRUE(all_socket_types_option->socketType().has_value());
+  EXPECT_EQ(Socket::Type::Stream, *all_socket_types_option->socketType());
+
+  auto stream_socket_type_option =
+      dynamic_pointer_cast<const SocketOptionImpl>(socket_options->at(5));
+  EXPECT_TRUE(stream_socket_type_option->socketType().has_value());
+  EXPECT_EQ(Socket::Type::Stream, *stream_socket_type_option->socketType());
+
+  auto datagram_socket_type_option =
+      dynamic_pointer_cast<const SocketOptionImpl>(socket_options->at(6));
+  EXPECT_TRUE(datagram_socket_type_option->socketType().has_value());
+  EXPECT_EQ(Socket::Type::Datagram, *datagram_socket_type_option->socketType());
 }
 
 TEST_F(SocketOptionFactoryTest, TestBuildZeroSoLingerOptions) {

diff --git a/test/common/network/socket_option_impl_test.cc b/test/common/network/socket_option_impl_test.cc
@@ -6,6 +6,8 @@ namespace Envoy {
 namespace Network {
 namespace {
 
+using ::testing::Return;
+
 class SocketOptionImplTest : public SocketOptionTest {};
 
 TEST_F(SocketOptionImplTest, BadFd) {
@@ -59,6 +61,24 @@ TEST_F(SocketOptionImplTest, SetOptionSuccessTrue) {
       socket_option.setOption(socket_, envoy::config::core::v3::SocketOption::STATE_PREBIND));
 }
 
+TEST_F(SocketOptionImplTest, SetDatagramOptionOnStreamSocketType) {
+  ON_CALL(socket_, socketType()).WillByDefault(Return(Socket::Type::Stream));
+  SocketOptionImpl socket_option{envoy::config::core::v3::SocketOption::STATE_PREBIND,
+                                 ENVOY_MAKE_SOCKET_OPTION_NAME(5, 10), 1, Socket::Type::Datagram};
+  EXPECT_CALL(socket_, setSocketOption(_, _, _, _)).Times(0);
+  EXPECT_TRUE(
+      socket_option.setOption(socket_, envoy::config::core::v3::SocketOption::STATE_PREBIND));
+}
+
+TEST_F(SocketOptionImplTest, SetStreamOptionOnDatagramSocketType) {
+  ON_CALL(socket_, socketType()).WillByDefault(Return(Socket::Type::Datagram));
+  SocketOptionImpl socket_option{envoy::config::core::v3::SocketOption::STATE_PREBIND,
+                                 ENVOY_MAKE_SOCKET_OPTION_NAME(5, 10), 1, Socket::Type::Stream};
+  EXPECT_CALL(socket_, setSocketOption(_, _, _, _)).Times(0);
+  EXPECT_TRUE(
+      socket_option.setOption(socket_, envoy::config::core::v3::SocketOption::STATE_PREBIND));
+}
+
 TEST_F(SocketOptionImplTest, GetOptionDetailsCorrectState) {
   SocketOptionImpl socket_option{envoy::config::core::v3::SocketOption::STATE_PREBIND,
                                  ENVOY_MAKE_SOCKET_OPTION_NAME(5, 10), 1};