Revert "udp: set Don't Fragment(DF) bit in IP packet header" (#36424)

Reverts #36341 as it breaks iOS CI. Signed-off-by: Dan Zhang <danzh@google.com>
envoyproxy · Oct 2, 2024 · 520442a · 520442a
1 parent 9fa36da
commit 520442a
Show file tree

Hide file tree

Showing 12 changed files with 35 additions and 306 deletions.
diff --git a/changelogs/current.yaml b/changelogs/current.yaml
@@ -116,10 +116,6 @@ minor_behavior_changes:
   change: |
     Changes the default value of ``envoy.reloadable_features.http2_use_oghttp2`` to ``false``. This changes the codec used for HTTP/2
     requests and responses to address to address stability concerns. This behavior can be reverted by setting the feature to ``true``.
-- area: udp
-  change: |
-    Set Don't Fragment (DF) flag bit on IP packet header on UDP listener sockets and QUIC upstream connection sockets. This behavior
-    can be reverted by setting ``envoy.reloadable_features.udp_set_do_not_fragment`` to false.
 - area: access_log
   change: |
     Sanitize SNI for potential log injection. The invalid character will be replaced by ``_`` with an ``invalid:`` marker. If runtime

diff --git a/envoy/network/socket.h b/envoy/network/socket.h
@@ -175,25 +175,6 @@ static_assert(IP_RECVDSTADDR == IP_SENDSRCADDR);
 #define ENVOY_ATTACH_REUSEPORT_CBPF Network::SocketOptionName()
 #endif
 
-#if !defined(ANDROID) && defined(__APPLE__)
-// Only include TargetConditionals after testing ANDROID as some Android builds
-// on the Mac have this header available and it's not needed unless the target
-// is really an Apple platform.
-#include <TargetConditionals.h>
-#if !defined(TARGET_OS_IPHONE) || !TARGET_OS_IPHONE
-// MAC_OS
-#define ENVOY_IP_DONTFRAG ENVOY_MAKE_SOCKET_OPTION_NAME(IPPROTO_IP, IP_DONTFRAG)
-#define ENVOY_IPV6_DONTFRAG ENVOY_MAKE_SOCKET_OPTION_NAME(IPPROTO_IPV6, IPV6_DONTFRAG)
-#endif
-#endif
-
-#if !defined(ENVOY_IP_DONTFRAG) && defined(IP_PMTUDISC_DO)
-#define ENVOY_IP_MTU_DISCOVER ENVOY_MAKE_SOCKET_OPTION_NAME(IPPROTO_IP, IP_MTU_DISCOVER)
-#define ENVOY_IP_MTU_DISCOVER_VALUE IP_PMTUDISC_DO
-#define ENVOY_IPV6_MTU_DISCOVER ENVOY_MAKE_SOCKET_OPTION_NAME(IPPROTO_IPV6, IPV6_MTU_DISCOVER)
-#define ENVOY_IPV6_MTU_DISCOVER_VALUE IPV6_PMTUDISC_DO
-#endif
-
 /**
  * Interface representing a single filter chain info.
  */

diff --git a/source/common/listener_manager/listener_impl.cc b/source/common/listener_manager/listener_impl.cc
@@ -677,13 +677,6 @@ void ListenerImpl::buildListenSocketOptions(
         addListenSocketOptions(listen_socket_options_list_[i],
                                Network::SocketOptionFactory::buildUdpGroOptions());
       }
-      if (Runtime::runtimeFeatureEnabled("envoy.reloadable_features.udp_set_do_not_fragment")) {
-        addListenSocketOptions(
-            listen_socket_options_list_[i],
-            Network::SocketOptionFactory::buildDoNotFragmentOptions(
-                /*mapped_v6*/ addresses_[i]->ip()->version() == Network::Address::IpVersion::v6 &&
-                !addresses_[i]->ip()->ipv6()->v6only()));
-      }
 
       // Additional factory specific options.
       ASSERT(udp_listener_config_->listener_factory_ != nullptr,

diff --git a/source/common/network/socket_option_factory.cc b/source/common/network/socket_option_factory.cc
@@ -181,32 +181,5 @@ std::unique_ptr<Socket::Options> SocketOptionFactory::buildIpRecvTosOptions() {
   return options;
 }
 
-std::unique_ptr<Socket::Options>
-SocketOptionFactory::buildDoNotFragmentOptions(bool supports_v4_mapped_v6_addresses) {
-  auto options = std::make_unique<Socket::Options>();
-#ifdef ENVOY_IP_DONTFRAG
-  options->push_back(std::make_shared<AddrFamilyAwareSocketOptionImpl>(
-      envoy::config::core::v3::SocketOption::STATE_PREBIND, ENVOY_IP_DONTFRAG, ENVOY_IPV6_DONTFRAG,
-      1));
-  // v4 mapped v6 addresses don't support ENVOY_IP_DONTFRAG on MAC OS.
-  (void)supports_v4_mapped_v6_addresses;
-#elif defined(ENVOY_IP_MTU_DISCOVER)
-  options->push_back(std::make_shared<AddrFamilyAwareSocketOptionImpl>(
-      envoy::config::core::v3::SocketOption::STATE_PREBIND, ENVOY_IP_MTU_DISCOVER,
-      ENVOY_IP_MTU_DISCOVER_VALUE, ENVOY_IPV6_MTU_DISCOVER, ENVOY_IPV6_MTU_DISCOVER_VALUE));
-
-  if (supports_v4_mapped_v6_addresses) {
-    ENVOY_LOG_MISC(trace, "Also apply the V4 option to v6 socket to support v4-mapped addresses.");
-    options->push_back(
-        std::make_shared<SocketOptionImpl>(envoy::config::core::v3::SocketOption::STATE_PREBIND,
-                                           ENVOY_IP_MTU_DISCOVER, ENVOY_IP_MTU_DISCOVER_VALUE));
-  }
-#else
-  (void)supports_v4_mapped_v6_addresses;
-  static_assert(false, "Platform supports neither socket option IP_DONTFRAG nor IP_MTU_DISCOVER");
-#endif
-  return options;
-}
-
 } // namespace Network
 } // namespace Envoy
diff --git a/source/common/network/socket_option_factory.h b/source/common/network/socket_option_factory.h
@@ -38,12 +38,6 @@ class SocketOptionFactory : Logger::Loggable<Logger::Id::connection> {
   static std::unique_ptr<Socket::Options> buildUdpGroOptions();
   static std::unique_ptr<Socket::Options> buildZeroSoLingerOptions();
   static std::unique_ptr<Socket::Options> buildIpRecvTosOptions();
-  /**
-   * @param supports_v4_mapped_v6_addresses true if this option is to be applied to a v6 socket with
-   * v4-mapped v6 address(i.e. ::ffff:172.21.0.6) support.
-   */
-  static std::unique_ptr<Socket::Options>
-  buildDoNotFragmentOptions(bool supports_v4_mapped_v6_addresses);
 };
 } // namespace Network
 } // namespace Envoy
diff --git a/source/common/quic/envoy_quic_utils.cc b/source/common/quic/envoy_quic_utils.cc
@@ -17,13 +17,11 @@ namespace Quic {
 namespace {
 
 Network::Address::InstanceConstSharedPtr
-getLoopbackAddress(Network::Address::InstanceConstSharedPtr peer_address) {
-  if (peer_address->ip()->version() == Network::Address::IpVersion::v6) {
-    return std::make_shared<Network::Address::Ipv6Instance>(
-        "::1", 0, &peer_address->socketInterface(), peer_address->ip()->ipv6()->v6only());
+getLoopbackAddress(const Network::Address::IpVersion version) {
+  if (version == Network::Address::IpVersion::v6) {
+    return std::make_shared<Network::Address::Ipv6Instance>("::1");
   }
-  return std::make_shared<Network::Address::Ipv4Instance>("127.0.0.1",
-                                                          &peer_address->socketInterface());
+  return std::make_shared<Network::Address::Ipv4Instance>("127.0.0.1");
 }
 
 } // namespace
@@ -202,7 +200,7 @@ createConnectionSocket(const Network::Address::InstanceConstSharedPtr& peer_addr
       Network::Socket::Type::Datagram,
       // Use the loopback address if `local_addr` is null, to pass in the socket interface used to
       // create the IoHandle, without having to make the more expensive `getifaddrs` call.
-      local_addr ? local_addr : getLoopbackAddress(peer_addr), peer_addr,
+      local_addr ? local_addr : getLoopbackAddress(peer_addr->ip()->version()), peer_addr,
       Network::SocketCreationOptions{false, max_addresses_cache_size});
   connection_socket->setDetectedTransportProtocol("quic");
   if (!connection_socket->isOpen()) {
@@ -217,25 +215,6 @@ createConnectionSocket(const Network::Address::InstanceConstSharedPtr& peer_addr
   if (prefer_gro && Api::OsSysCallsSingleton::get().supportsUdpGro()) {
     connection_socket->addOptions(Network::SocketOptionFactory::buildUdpGroOptions());
   }
-  if (Runtime::runtimeFeatureEnabled("envoy.reloadable_features.udp_set_do_not_fragment")) {
-    int v6_only = 0;
-    if (connection_socket->ipVersion().has_value() &&
-        connection_socket->ipVersion().value() == Network::Address::IpVersion::v6) {
-      socklen_t v6_only_len = sizeof(v6_only);
-      Api::SysCallIntResult result =
-          connection_socket->getSocketOption(IPPROTO_IPV6, IPV6_V6ONLY, &v6_only, &v6_only_len);
-      if (result.return_value_ != 0) {
-        ENVOY_LOG_MISC(
-            error, "Failed to get IPV6_V6ONLY socket option, getsockopt() returned {}, errno {}",
-            result.return_value_, result.errno_);
-        connection_socket->close();
-        return connection_socket;
-      }
-    }
-    connection_socket->addOptions(Network::SocketOptionFactory::buildDoNotFragmentOptions(
-        /*mapped_v6*/ connection_socket->ipVersion().value() == Network::Address::IpVersion::v6 &&
-        v6_only == 0));
-  }
   if (options != nullptr) {
     connection_socket->addOptions(options);
   }

diff --git a/source/common/runtime/runtime_features.cc b/source/common/runtime/runtime_features.cc
@@ -94,7 +94,6 @@ RUNTIME_GUARD(envoy_reloadable_features_skip_dns_lookup_for_proxied_requests);
 RUNTIME_GUARD(envoy_reloadable_features_strict_duration_validation);
 RUNTIME_GUARD(envoy_reloadable_features_tcp_tunneling_send_downstream_fin_on_upstream_trailers);
 RUNTIME_GUARD(envoy_reloadable_features_test_feature_true);
-RUNTIME_GUARD(envoy_reloadable_features_udp_set_do_not_fragment);
 RUNTIME_GUARD(envoy_reloadable_features_udp_socket_apply_aggregated_read_limit);
 RUNTIME_GUARD(envoy_reloadable_features_uhv_allow_malformed_url_encoding);
 RUNTIME_GUARD(envoy_reloadable_features_upstream_remote_address_use_connection);

diff --git a/test/common/listener_manager/listener_manager_impl_quic_only_test.cc b/test/common/listener_manager/listener_manager_impl_quic_only_test.cc
@@ -24,21 +24,6 @@ class MockSupportsUdpGso : public Api::OsSysCallsImpl {
 
 class ListenerManagerImplQuicOnlyTest : public ListenerManagerImplTest {
 public:
-  size_t expectedNumSocketOptions() {
-    // SO_REUSEPORT, IP_PKTINFO and IP_MTU_DISCOVER/IP_DONTFRAG.
-    const size_t num_platform_independent_socket_options =
-        Runtime::runtimeFeatureEnabled("envoy.reloadable_features.udp_set_do_not_fragment") ? 3 : 2;
-    size_t num_platform_dependent_socket_options = 0;
-#ifdef SO_RXQ_OVFL
-    ++num_platform_dependent_socket_options;
-#endif
-    if (Api::OsSysCallsSingleton::get().supportsUdpGro()) {
-      // SO_REUSEPORT
-      ++num_platform_dependent_socket_options;
-    }
-    return num_platform_dependent_socket_options + num_platform_independent_socket_options;
-  }
-
   NiceMock<MockSupportsUdpGso> udp_gso_syscall_;
   TestThreadsafeSingletonInjector<Api::OsSysCallsImpl> os_calls{&udp_gso_syscall_};
   Api::OsSysCallsImpl os_sys_calls_actual_;
@@ -121,7 +106,13 @@ TEST_P(ListenerManagerImplQuicOnlyTest, QuicListenerFactoryAndSslContext) {
       .WillByDefault(Return(os_sys_calls_actual_.supportsUdpGso()));
   EXPECT_CALL(server_.api_.random_, uuid());
   expectCreateListenSocket(envoy::config::core::v3::SocketOption::STATE_PREBIND,
-                           expectedNumSocketOptions(),
+#ifdef SO_RXQ_OVFL // SO_REUSEPORT is on as configured
+                           /* expected_num_options */
+                           Api::OsSysCallsSingleton::get().supportsUdpGro() ? 4 : 3,
+#else
+                           /* expected_num_options */
+                           Api::OsSysCallsSingleton::get().supportsUdpGro() ? 3 : 2,
+#endif
                            ListenerComponentFactory::BindType::ReusePort);
 
   expectSetsockopt(/* expected_sockopt_level */ IPPROTO_IP,
@@ -147,18 +138,6 @@ TEST_P(ListenerManagerImplQuicOnlyTest, QuicListenerFactoryAndSslContext) {
   }
 #endif
 
-#ifdef ENVOY_IP_DONTFRAG
-  expectSetsockopt(/* expected_sockopt_level */ IPPROTO_IP,
-                   /* expected_sockopt_name */ IP_DONTFRAG,
-                   /* expected_value */ 1,
-                   /* expected_num_calls */ 1);
-#else
-  expectSetsockopt(/* expected_sockopt_level */ IPPROTO_IP,
-                   /* expected_sockopt_name */ IP_MTU_DISCOVER,
-                   /* expected_value */ IP_PMTUDISC_DO,
-                   /* expected_num_calls */ 1);
-#endif
-
   addOrUpdateListener(listener_proto);
   EXPECT_EQ(1u, manager_->listeners().size());
   EXPECT_FALSE(manager_->listeners()[0]
@@ -216,7 +195,13 @@ TEST_P(ListenerManagerImplQuicOnlyTest, QuicWriterFromConfig) {
   ON_CALL(udp_gso_syscall_, supportsUdpGso()).WillByDefault(Return(true));
   EXPECT_CALL(server_.api_.random_, uuid());
   expectCreateListenSocket(envoy::config::core::v3::SocketOption::STATE_PREBIND,
-                           expectedNumSocketOptions(),
+#ifdef SO_RXQ_OVFL // SO_REUSEPORT is on as configured
+                           /* expected_num_options */
+                           Api::OsSysCallsSingleton::get().supportsUdpGro() ? 4 : 3,
+#else
+                           /* expected_num_options */
+                           Api::OsSysCallsSingleton::get().supportsUdpGro() ? 3 : 2,
+#endif
                            ListenerComponentFactory::BindType::ReusePort);
 
   expectSetsockopt(/* expected_sockopt_level */ IPPROTO_IP,
@@ -242,18 +227,6 @@ TEST_P(ListenerManagerImplQuicOnlyTest, QuicWriterFromConfig) {
   }
 #endif
 
-#ifdef ENVOY_IP_DONTFRAG
-  expectSetsockopt(/* expected_sockopt_level */ IPPROTO_IP,
-                   /* expected_sockopt_name */ IP_DONTFRAG,
-                   /* expected_value */ 1,
-                   /* expected_num_calls */ 1);
-#else
-  expectSetsockopt(/* expected_sockopt_level */ IPPROTO_IP,
-                   /* expected_sockopt_name */ IP_MTU_DISCOVER,
-                   /* expected_value */ IP_PMTUDISC_DO,
-                   /* expected_num_calls */ 1);
-#endif
-
   addOrUpdateListener(listener_proto);
   EXPECT_EQ(1u, manager_->listeners().size());
   EXPECT_FALSE(manager_->listeners()[0]
@@ -330,7 +303,13 @@ TEST_P(ListenerManagerImplQuicOnlyTest, QuicListenerFactoryWithExplictConnection
   ON_CALL(udp_gso_syscall_, supportsUdpGso()).WillByDefault(Return(true));
   EXPECT_CALL(server_.api_.random_, uuid());
   expectCreateListenSocket(envoy::config::core::v3::SocketOption::STATE_PREBIND,
-                           expectedNumSocketOptions(),
+#ifdef SO_RXQ_OVFL // SO_REUSEPORT is on as configured
+                           /* expected_num_options */
+                           Api::OsSysCallsSingleton::get().supportsUdpGro() ? 4 : 3,
+#else
+                           /* expected_num_options */
+                           Api::OsSysCallsSingleton::get().supportsUdpGro() ? 3 : 2,
+#endif
                            ListenerComponentFactory::BindType::ReusePort);
 
   expectSetsockopt(/* expected_sockopt_level */ IPPROTO_IP,
@@ -356,18 +335,6 @@ TEST_P(ListenerManagerImplQuicOnlyTest, QuicListenerFactoryWithExplictConnection
   }
 #endif
 
-#ifdef ENVOY_IP_DONTFRAG
-  expectSetsockopt(/* expected_sockopt_level */ IPPROTO_IP,
-                   /* expected_sockopt_name */ IP_DONTFRAG,
-                   /* expected_value */ 1,
-                   /* expected_num_calls */ 1);
-#else
-  expectSetsockopt(/* expected_sockopt_level */ IPPROTO_IP,
-                   /* expected_sockopt_name */ IP_MTU_DISCOVER,
-                   /* expected_value */ IP_PMTUDISC_DO,
-                   /* expected_num_calls */ 1);
-#endif
-
   addOrUpdateListener(listener_proto);
   EXPECT_EQ(1u, manager_->listeners().size());
   EXPECT_FALSE(manager_->listeners()[0]
@@ -392,7 +359,13 @@ TEST_P(ListenerManagerImplQuicOnlyTest, QuicListenerFilterFromConfig) {
   ON_CALL(udp_gso_syscall_, supportsUdpGso()).WillByDefault(Return(true));
   EXPECT_CALL(server_.api_.random_, uuid());
   expectCreateListenSocket(envoy::config::core::v3::SocketOption::STATE_PREBIND,
-                           expectedNumSocketOptions(),
+#ifdef SO_RXQ_OVFL // SO_REUSEPORT is on as configured
+                           /* expected_num_options */
+                           Api::OsSysCallsSingleton::get().supportsUdpGro() ? 4 : 3,
+#else
+                           /* expected_num_options */
+                           Api::OsSysCallsSingleton::get().supportsUdpGro() ? 3 : 2,
+#endif
                            ListenerComponentFactory::BindType::ReusePort);
 
   expectSetsockopt(/* expected_sockopt_level */ IPPROTO_IP,
@@ -418,18 +391,6 @@ TEST_P(ListenerManagerImplQuicOnlyTest, QuicListenerFilterFromConfig) {
   }
 #endif
 
-#ifdef ENVOY_IP_DONTFRAG
-  expectSetsockopt(/* expected_sockopt_level */ IPPROTO_IP,
-                   /* expected_sockopt_name */ IP_DONTFRAG,
-                   /* expected_value */ 1,
-                   /* expected_num_calls */ 1);
-#else
-  expectSetsockopt(/* expected_sockopt_level */ IPPROTO_IP,
-                   /* expected_sockopt_name */ IP_MTU_DISCOVER,
-                   /* expected_value */ IP_PMTUDISC_DO,
-                   /* expected_num_calls */ 1);
-#endif
-
   addOrUpdateListener(listener_proto);
   EXPECT_EQ(1u, manager_->listeners().size());
   // Verify that the right filter chain type is installed.

diff --git a/test/common/quic/BUILD b/test/common/quic/BUILD
@@ -323,7 +323,6 @@ envoy_cc_test(
         "//test/mocks/upstream:cluster_info_mocks",
         "//test/mocks/upstream:transport_socket_match_mocks",
         "//test/test_common:test_runtime_lib",
-        "//test/test_common:threadsafe_singleton_injector_lib",
     ],
 )