Apply server socket options before bind

Summary:
Pass configured server socket options into the bind-time listener creation path for both classic Proxygen and proxygen-coro so `PRE_BIND` options such as `TCP_MAXSEG` reach `AsyncServerSocket::bind()` instead of depending on wangle `Acceptor` applying them after `listen()` (which may or may not work depending on when the first accept() happens -- see D107476342).

Context is that we need to do mss clamping on manifold http ports until shiv supports large MTU. Before this stack, it relies on wangle Acceptor to apply the socket options which may be too late for PRE_BIND socket options like the mss one. The stack fixes folly and wangle. In this diff we noticed that libproxygen doesn't copy the socket options map to wangle serverbootstrap.

Reviewed By: ngoyal

Differential Revision: D107599166

fbshipit-source-id: b8e680ae110cb19da64b455c987ff8a29da1a3f3

Author: Xiangyu Bu

third-party/proxygen/src/proxygen/httpserver/HTTPServer.cpp

@@ -167,8 +167,17 @@ folly::Expected<folly::Unit, std::exception_ptr> HTTPServer::startTcpServer(
         bootstrap_[i].setReusePort(true);
       }
       if (options_->preboundSockets_.size() > i) {
+        // Intentionally not copying socket options map on prebound sockets
+        // branch because 1) the caller has freedom to apply them, and 2) wangle
+        // does not pass it to AsyncServerSocket, and AsyncServerSocket doesn't
+        // consume it on pre-bound path anyway.
         bootstrap_[i].bind(std::move(options_->preboundSockets_[i]));
       } else {
+        // Copy socket options to ServerBootstrap so that wangle can pass them
+        // to AsyncServerSocket when calling bind(). This way the pre-bind
+        // options can be applied properly pre-bind.
+        bootstrap_[i].socketConfig.getSocketOptions() =
+            accConfig->getSocketOptions();
         bootstrap_[i].bind(addresses_[i].address);
       }
     }

third-party/proxygen/src/proxygen/httpserver/tests/CMakeLists.txt

@@ -7,6 +7,7 @@
 proxygen_add_test(TARGET HTTPServerTests
   SOURCES
     HTTPServerTest.cpp
+    HTTPServerTestUtils.cpp
     RequestHandlerAdaptorTest.cpp
   DEPENDS
     codectestutils

third-party/proxygen/src/proxygen/httpserver/tests/HTTPServerTest.cpp

@@ -11,12 +11,14 @@
 
 #include <folly/FileUtil.h>
 #include <folly/executors/IOThreadPoolExecutor.h>
+#include <folly/io/SocketOptionMap.h>
 #include <folly/io/async/AsyncSSLSocket.h>
 #include <folly/io/async/AsyncServerSocket.h>
 #include <folly/io/async/EventBaseManager.h>
 #include <folly/logging/xlog.h>
 #include <folly/portability/GMock.h>
 #include <folly/portability/GTest.h>
+#include <folly/portability/Sockets.h>
 #include <folly/ssl/OpenSSLCertUtils.h>
 #include <folly/ssl/OpenSSLPtrTypes.h>
 #include <folly/system/HardwareConcurrency.h>
@@ -25,6 +27,7 @@
 #include <proxygen/httpserver/HTTPServer.h>
 #include <proxygen/httpserver/ResponseBuilder.h>
 #include <proxygen/httpserver/ScopedHTTPServer.h>
+#include <proxygen/httpserver/tests/HTTPServerTestUtils.h>
 #include <proxygen/lib/http/HTTPConnector.h>
 #include <proxygen/lib/utils/TestUtils.h>
 #include <wangle/acceptor/Acceptor.h>
@@ -39,8 +42,7 @@ using namespace CurlService;
 namespace {
 
 const std::string kTestDir = getContainingDirectory(XLOG_FILENAME).str();
-
-}
+} // namespace
 
 class ServerThread {
  private:
@@ -813,6 +815,33 @@ TEST(GetListenSocket, TestBootstrapWithBinding) {
   ASSERT_NE(-1, socketFd);
 }
 
+TEST(SocketOptions, AcceptorSocketOptionsApplyTcpMaxSegmentBeforeListen) {
+  const auto tcpMaxSegment = proxygen::test::getDifferentTcpMaxSegment(
+      proxygen::test::getDefaultLoopbackTcpMaxSegment());
+  HTTPServer::IPConfig cfg{folly::SocketAddress("127.0.0.1", 0),
+                           HTTPServer::Protocol::HTTP};
+  cfg.acceptorSocketOptions = folly::SocketOptionMap{
+      {{.level = IPPROTO_TCP,
+        .optname = TCP_MAXSEG,
+        .applyPos_ = folly::SocketOptionKey::ApplyPos::PRE_BIND},
+       tcpMaxSegment},
+  };
+
+  HTTPServerOptions options;
+  options.handlerFactories =
+      RequestHandlerChain().addThen<TestHandlerFactory>().build();
+
+  auto server = std::make_unique<HTTPServer>(std::move(options));
+  server->bind({cfg});
+
+  auto st = std::make_unique<ServerThread>(server.get());
+  EXPECT_TRUE(st->start());
+
+  EXPECT_EQ(proxygen::test::getTcpMaxSegment(
+                folly::NetworkSocket::fromFd(server->getListenSocket())),
+            tcpMaxSegment);
+}
+
 TEST(UseExistingSocket, TestWithExistingAsyncServerSocket) {
   AsyncServerSocket::UniquePtr serverSocket(new folly::AsyncServerSocket);
   serverSocket->bind(0);

third-party/proxygen/src/proxygen/httpserver/tests/HTTPServerTestUtils.cpp

@@ -0,0 +1,38 @@
+/*
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ * All rights reserved.
+ *
+ * This source code is licensed under the BSD-style license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+#include <proxygen/httpserver/tests/HTTPServerTestUtils.h>
+
+#include <folly/portability/GTest.h>
+#include <folly/portability/Sockets.h>
+#include <folly/portability/Unistd.h>
+
+namespace proxygen::test {
+
+int getTcpMaxSegment(folly::NetworkSocket fd) {
+  int mss = 0;
+  socklen_t mssLength = sizeof(mss);
+  EXPECT_EQ(::getsockopt(fd.toFd(), IPPROTO_TCP, TCP_MAXSEG, &mss, &mssLength),
+            0);
+  return mss;
+}
+
+int getDefaultLoopbackTcpMaxSegment() {
+  auto fd = ::socket(AF_INET, SOCK_STREAM, 0);
+  EXPECT_NE(fd, -1);
+  const auto mss = getTcpMaxSegment(folly::NetworkSocket::fromFd(fd));
+  EXPECT_EQ(::close(fd), 0);
+  return mss;
+}
+
+int getDifferentTcpMaxSegment(int defaultTcpMaxSegment) {
+  EXPECT_GT(defaultTcpMaxSegment, 100);
+  return defaultTcpMaxSegment - 100;
+}
+
+} // namespace proxygen::test

third-party/proxygen/src/proxygen/httpserver/tests/HTTPServerTestUtils.h

@@ -0,0 +1,21 @@
+/*
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ * All rights reserved.
+ *
+ * This source code is licensed under the BSD-style license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+#pragma once
+
+#include <folly/net/NetworkSocket.h>
+
+namespace proxygen::test {
+
+int getTcpMaxSegment(folly::NetworkSocket fd);
+
+int getDefaultLoopbackTcpMaxSegment();
+
+int getDifferentTcpMaxSegment(int defaultTcpMaxSegment);
+
+} // namespace proxygen::test

third-party/proxygen/src/proxygen/lib/http/coro/server/HTTPServer.cpp

@@ -218,7 +218,8 @@ void HTTPServer::startTcp(const KeepAliveEventBaseVec& keepAliveEvbs) {
         serverSocket->useExistingSocket(
             folly::NetworkSocket::fromFd(config_.preboundSocket.value()));
       } else {
-        serverSocket->bind(config_.socketConfig.bindAddress);
+        serverSocket->bind(config_.socketConfig.bindAddress,
+                           config_.socketConfig.getSocketOptions());
       }
       serverSocket->listen(config_.socketConfig.acceptBacklog);
       serverSocket->startAccepting();

third-party/proxygen/src/proxygen/lib/http/coro/server/HTTPServer.h

@@ -24,10 +24,16 @@
 
 namespace proxygen::coro {
 
+namespace test {
+class HTTPServerTests;
+} // namespace test
+
 using folly::IOThreadPoolExecutor;
 using folly::ThreadPoolExecutor;
 
 class HTTPServer : public quic::QuicHandshakeSocketHolder::Callback {
+  friend class test::HTTPServerTests;
+
  public:
   struct QuicConfig {
     std::vector<quic::QuicVersion> quicVersions;

third-party/proxygen/src/proxygen/lib/http/coro/server/test/HTTPServerTest.cpp

@@ -8,6 +8,7 @@
 
 #include <folly/logging/xlog.h>
 #include <folly/system/HardwareConcurrency.h>
+#include <proxygen/httpserver/tests/HTTPServerTestUtils.h>
 #include <proxygen/lib/http/codec/test/TestUtils.h>
 #include <proxygen/lib/http/coro/HTTPCoroSession.h>
 #include <proxygen/lib/http/coro/HTTPFixedSource.h>
@@ -22,6 +23,7 @@
 
 #include <chrono>
 #include <folly/coro/GtestHelpers.h>
+#include <folly/io/SocketOptionMap.h>
 #include <folly/io/async/ScopedEventBaseThread.h>
 #include <folly/portability/GMock.h>
 #include <folly/portability/GTest.h>
@@ -45,6 +47,8 @@ std::string_view getTestDir() {
   return kTestDir;
 }
 
+constexpr int kTestTcpMaxSegment = 1200;
+
 struct StatsFactory : public ServerFilterFactory {
   std::pair<HTTPSourceFilter*, HTTPSourceFilter*> makeFilters() override {
     return StatsFilterUtil::makeFilters(&stats_);
@@ -206,6 +210,11 @@ class HTTPServerTests : public TestWithParam<TransportType> {
         std::make_shared<InsecureVerifierDangerousDoNotUseInProduction>());
   }
 
+  const std::vector<folly::AsyncServerSocket::UniquePtr>& getServerSockets() {
+    CHECK(server_);
+    return server_->getServer().serverSockets_;
+  }
+
   std::string listenAddress_{"127.0.0.1"};
   uint16_t listenPort_{0};
   std::shared_ptr<HTTPHandler> handler_{std::make_shared<TestHandler>()};
@@ -266,6 +275,29 @@ TEST_P(HTTPServerTests, TestExistingSocket) {
   stopServer();
 }
 
+TEST_F(HTTPServerTests, SocketConfigAppliesTcpMaxSegmentBeforeListen) {
+  const auto tcpMaxSegment = proxygen::test::getDifferentTcpMaxSegment(
+      proxygen::test::getDefaultLoopbackTcpMaxSegment());
+  serverConfig_.socketConfig.bindAddress.setFromIpPort(listenAddress_,
+                                                       listenPort_);
+  serverConfig_.socketConfig.setSocketOptions(folly::SocketOptionMap{
+      {{.level = IPPROTO_TCP,
+        .optname = TCP_MAXSEG,
+        .applyPos_ = folly::SocketOptionKey::ApplyPos::PRE_BIND},
+       tcpMaxSegment},
+  });
+  server_ =
+      ScopedHTTPServer::start(std::move(serverConfig_), handler_, nullptr);
+
+  auto& serverSockets = getServerSockets();
+  ASSERT_EQ(serverSockets.size(), 1);
+  EXPECT_EQ(proxygen::test::getTcpMaxSegment(
+                serverSockets.front()->getNetworkSocket()),
+            tcpMaxSegment);
+
+  stopServer();
+}
+
 TEST_P(HTTPServerTests, TestStopMultipleTimes) {
   MockServerObserver mockObserver;
   serverConfig_.numIOThreads = 4;