Make commonly-used d6d types individual target

Summary: This reduces dependencies for both testing and instrumentation.

Reviewed By: mjoras

Differential Revision: D23997313

fbshipit-source-id: 5eb3a790c7bb2569dc1e941e3911ad4aac4e9258

quic/api/QuicPacketScheduler.cpp

@@ -727,7 +727,7 @@ SchedulingResult D6DProbeScheduler::scheduleFramesForPacket(
       << " seq: " << conn_.d6d.lastProbe->packetNum
       << " packet size: " << conn_.d6d.lastProbe->packetSize;
 
-  conn_.d6d.lastProbe = QuicConnectionStateBase::D6DProbePacket(
+  conn_.d6d.lastProbe = D6DProbePacket(
       resultPacket.packet.header.getPacketSequenceNum(), probeSize_);
 
   probeSent_ = true;

quic/api/test/QuicTransportFunctionsTest.cpp

@@ -364,7 +364,7 @@ TEST_F(QuicTransportFunctionsTest, TestUpdateConnectionD6DNotConsumeSendPing) {
   auto packet = buildEmptyPacket(*conn, PacketNumberSpace::AppData);
   packet.packet.frames.push_back(PingFrame());
   auto packetNum = packet.packet.header.getPacketSequenceNum();
-  conn->d6d.lastProbe = QuicConnectionStateBase::D6DProbePacket(packetNum, 50);
+  conn->d6d.lastProbe = D6DProbePacket(packetNum, 50);
   updateConnection(*conn, folly::none, packet.packet, Clock::now(), 50);
   EXPECT_EQ(1, conn->outstandings.packets.size());
   EXPECT_TRUE(conn->outstandings.packets.front().metadata.isD6DProbe);

quic/d6d/CMakeLists.txt

@@ -3,6 +3,7 @@
 # This source code is licensed under the MIT license found in the
 # LICENSE file in the root directory of this source tree.
 
+# Probe Raiser
 add_library(
   mvfst_d6d_probe_raiser INTERFACE
 )
@@ -13,6 +14,7 @@ target_include_directories(
   $<INSTALL_INTERFACE:include/>
 )
 
+# State Functions
 add_library(
   mvfst_d6d_state_functions STATIC
   QuicD6DStateFunctions.cpp
@@ -41,6 +43,35 @@ target_link_libraries(
   mvfst_state_machine
 )
 
+# Types
+add_library(
+  mvfst_d6d_types STATIC
+  Types.cpp
+)
+
+target_include_directories(
+  mvfst_d6d_types PUBLIC
+  $<BUILD_INTERFACE:${QUIC_FBCODE_ROOT}>
+  $<INSTALL_INTERFACE:include/>
+)
+
+target_compile_options(
+  mvfst_d6d_types
+  PRIVATE
+  ${_QUIC_COMMON_COMPILE_OPTIONS}
+)
+
+add_dependencies(
+  mvfst_d6d_types
+  mvfst_codec_types
+)
+
+target_link_libraries(
+  mvfst_d6d_types PUBLIC
+  Folly::folly
+  mvfst_codec_types
+)
+
 file(
   GLOB_RECURSE QUIC_API_HEADERS_TOINSTALL
   RELATIVE ${CMAKE_CURRENT_SOURCE_DIR}
@@ -64,4 +95,10 @@ install(
   DESTINATION lib
 )
 
+install(
+  TARGETS mvfst_d6d_types
+  EXPORT mvfst-exports
+  DESTINATION lib
+)
+
 add_subdirectory(test)

quic/d6d/QuicD6DStateFunctions.cpp

@@ -10,28 +10,6 @@
 
 namespace quic {
 
-namespace {
-using D6DMachineState = QuicConnectionStateBase::D6DMachineState;
-
-std::string toString(
-    const ::quic::QuicConnectionStateBase::D6DMachineState state) {
-  switch (state) {
-    case D6DMachineState::DISABLED:
-      return "DISABLED";
-    case D6DMachineState::BASE:
-      return "BASE";
-    case D6DMachineState::SEARCHING:
-      return "SEARCHING";
-    case D6DMachineState::SEARCH_COMPLETE:
-      return "SEARCH_COMPLETE";
-    case D6DMachineState::ERROR:
-      return "ERROR";
-  }
-  folly::assume_unreachable();
-}
-
-} // namespace
-
 inline void scheduleProbeAfterDelay(
     QuicConnectionStateBase& conn,
     std::chrono::milliseconds delay) {

quic/d6d/Types.cpp

@@ -0,0 +1,39 @@
+/*
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ */
+
+#include <quic/d6d/Types.h>
+#include <folly/lang/Assume.h>
+
+namespace quic {
+std::string toString(const D6DMachineState state) {
+  switch (state) {
+    case D6DMachineState::DISABLED:
+      return "DISABLED";
+    case D6DMachineState::BASE:
+      return "BASE";
+    case D6DMachineState::SEARCHING:
+      return "SEARCHING";
+    case D6DMachineState::SEARCH_COMPLETE:
+      return "SEARCH_COMPLETE";
+    case D6DMachineState::ERROR:
+      return "ERROR";
+  }
+  folly::assume_unreachable();
+}
+
+std::string toString(const ProbeSizeRaiserType type) {
+  switch (type) {
+    case ProbeSizeRaiserType::ConstantStep:
+      return "ConstantStep";
+    case ProbeSizeRaiserType::BinarySearch:
+      return "BinarySearch";
+  }
+  folly::assume_unreachable();
+}
+
+} // namespace quic

quic/d6d/Types.h

@@ -0,0 +1,54 @@
+/*
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ */
+
+#pragma once
+
+#include <quic/codec/Types.h>
+#include <string>
+
+namespace quic {
+struct D6DProbePacket {
+  D6DProbePacket() = delete;
+
+  explicit D6DProbePacket(PacketNum packetNumIn, uint32_t packetSizeIn)
+      : packetNum(packetNumIn), packetSize(packetSizeIn) {}
+
+  // Packet num
+  PacketNum packetNum;
+
+  // Udp packet payload size
+  uint32_t packetSize;
+};
+
+// States of d6d state machine, see
+// https://tools.ietf.org/id/draft-ietf-tsvwg-datagram-plpmtud-21.html#name-state-machine
+enum class D6DMachineState : uint8_t {
+  // Connection is not established yet
+  DISABLED = 0,
+  // Probe using base pmtu
+  BASE = 1,
+  // Incrementally probe using larger pmtu
+  SEARCHING = 2,
+  // Sleep for raise timeout before going to SEARCHING
+  SEARCH_COMPLETE = 3,
+  // Effective pmtu is less than base pmtu, continue probing with smaller
+  // packet
+  ERROR = 4
+};
+
+/**
+ * Two simple probe size raiser. Only server makes use of this value.
+ * ConstantSize: raise pmtu at constant step size
+ * BinarySearch: raise pmtu using binary search
+ */
+enum class ProbeSizeRaiserType : uint8_t { ConstantStep = 0, BinarySearch = 1 };
+
+std::string toString(const D6DMachineState state);
+std::string toString(const ProbeSizeRaiserType type);
+
+} // namespace quic

quic/d6d/test/QuicD6DStateFunctionsTest.cpp

@@ -19,10 +19,6 @@ using namespace testing;
 namespace quic {
 namespace test {
 
-namespace {
-using D6DMachineState = QuicConnectionStateBase::D6DMachineState;
-} // namespace
-
 struct D6DProbeLostTestFixture {
   D6DMachineState stateBegin;
   D6DMachineState stateEnd;
@@ -149,7 +145,7 @@ TEST_F(QuicD6DStateFunctionsTest, D6DProbeAckedInBase) {
       false,
       true,
       d6d.currentProbeSize);
-  d6d.lastProbe = QuicConnectionStateBase::D6DProbePacket(
+  d6d.lastProbe = D6DProbePacket(
       pkt.packet.header.getPacketSequenceNum(), pkt.metadata.encodedSize);
   d6d.raiser = std::make_unique<MockProbeSizeRaiser>();
   auto mockRaiser = dynamic_cast<MockProbeSizeRaiser*>(d6d.raiser.get());
@@ -177,7 +173,7 @@ TEST_F(QuicD6DStateFunctionsTest, D6DProbeAckedInSearchingOne) {
       false,
       true,
       d6d.currentProbeSize);
-  d6d.lastProbe = QuicConnectionStateBase::D6DProbePacket(
+  d6d.lastProbe = D6DProbePacket(
       pkt.packet.header.getPacketSequenceNum(), pkt.metadata.encodedSize);
   d6d.raiser = std::make_unique<MockProbeSizeRaiser>();
   auto mockRaiser = dynamic_cast<MockProbeSizeRaiser*>(d6d.raiser.get());
@@ -205,7 +201,7 @@ TEST_F(QuicD6DStateFunctionsTest, D6DProbeAckedInSearchingMax) {
       false,
       true,
       d6d.currentProbeSize);
-  d6d.lastProbe = QuicConnectionStateBase::D6DProbePacket(
+  d6d.lastProbe = D6DProbePacket(
       pkt.packet.header.getPacketSequenceNum(), pkt.metadata.encodedSize);
   d6d.raiser = std::make_unique<MockProbeSizeRaiser>();
   auto mockRaiser = dynamic_cast<MockProbeSizeRaiser*>(d6d.raiser.get());
@@ -232,7 +228,7 @@ TEST_F(QuicD6DStateFunctionsTest, D6DProbeAckedInError) {
       false,
       true,
       d6d.currentProbeSize);
-  d6d.lastProbe = QuicConnectionStateBase::D6DProbePacket(
+  d6d.lastProbe = D6DProbePacket(
       pkt.packet.header.getPacketSequenceNum(), pkt.metadata.encodedSize);
   d6d.raiser = std::make_unique<MockProbeSizeRaiser>();
   auto mockRaiser = dynamic_cast<MockProbeSizeRaiser*>(d6d.raiser.get());

quic/loss/test/QuicLossFunctionsTest.cpp

@@ -33,10 +33,6 @@ using namespace folly;
 namespace quic {
 namespace test {
 
-namespace {
-using D6DMachineState = QuicConnectionStateBase::D6DMachineState;
-} // namespace
-
 class MockLossTimeout {
  public:
   MOCK_METHOD0(cancelLossTimeout, void());
@@ -265,8 +261,8 @@ PacketNum QuicLossFunctionsTest::sendPacket(
   conn.pendingEvents.setLossDetectionAlarm = true;
   if (isD6DProbe) {
     ++conn.d6d.outstandingProbes;
-    conn.d6d.lastProbe = QuicConnectionStateBase::D6DProbePacket(
+    conn.d6d.lastProbe =
-        encodedSize, conn.lossState.largestSent.value());
+        D6DProbePacket(encodedSize, conn.lossState.largestSent.value());
   }
   return conn.lossState.largestSent.value();
 }
@@ -377,8 +373,8 @@ struct PMTUBlackholeDetectionTestFixture {
   std::chrono::microseconds detectLossTimeDelta;
   std::chrono::microseconds largestLostTimeDelta;
   std::chrono::microseconds smallestLostTimeDelta;
-  QuicConnectionStateBase::D6DMachineState beginState;
+  D6DMachineState beginState;
-  QuicConnectionStateBase::D6DMachineState endState;
+  D6DMachineState endState;
 
   struct PacketSpec {
     std::chrono::microseconds timeDelta;
@@ -394,7 +390,7 @@ void QuicLossFunctionsTest::runDetectPMTUBlackholeTest(
   // 0 rtt means all packets are automatically lost upon sending
   conn->lossState.srtt = 0s;
   conn->lossState.lrtt = 0s;
-  conn->d6d.state = QuicConnectionStateBase::D6DMachineState::BASE;
+  conn->d6d.state = D6DMachineState::BASE;
   conn->d6d.currentProbeSize = fixture.beginPMTU;
   conn->udpSendPacketLen = fixture.beginPMTU;
   conn->d6d.thresholdCounter =

quic/server/QuicServerTransport.cpp

@@ -513,16 +513,16 @@ void QuicServerTransport::maybeNotifyTransportReady() {
 
 void QuicServerTransport::maybeStartD6DProbing() {
   if (!d6dProbingStarted_ && hasReadCipher() &&
-      conn_->d6d.state == QuicConnectionStateBase::D6DMachineState::BASE) {
+      conn_->d6d.state == D6DMachineState::BASE) {
     QUIC_TRACE(fst_trace, *conn_, "start d6d probing");
     d6dProbingStarted_ = true;
     auto& d6d = conn_->d6d;
     switch (conn_->transportSettings.d6dConfig.raiserType) {
-      case D6DConfig::ProbeSizeRaiserType::ConstantStep:
+      case ProbeSizeRaiserType::ConstantStep:
         d6d.raiser = std::make_unique<ConstantStepProbeSizeRaiser>(
             conn_->transportSettings.d6dConfig.probeRaiserConstantStepSize);
         break;
-      case D6DConfig::ProbeSizeRaiserType::BinarySearch:
+      case ProbeSizeRaiserType::BinarySearch:
         d6d.raiser = std::make_unique<BinarySearchProbeSizeRaiser>(
             kMinMaxUDPPayload, d6d.maxPMTU);
     }

quic/server/state/ServerStateMachine.cpp

@@ -234,9 +234,8 @@ void processClientInitialParams(
         VLOG(10) << "conn.d6d.basePMTU=" << conn.d6d.basePMTU;
 
         // Start from base
-        conn.d6d.state = QuicConnectionStateBase::D6DMachineState::BASE;
+        conn.d6d.state = D6DMachineState::BASE;
-        conn.d6d.meta.lastNonSearchState =
+        conn.d6d.meta.lastNonSearchState = D6DMachineState::DISABLED;
-            QuicConnectionStateBase::D6DMachineState::DISABLED;
         conn.d6d.meta.timeLastNonSearchState = Clock::now();
       } else {
         LOG(ERROR) << "client d6dBasePMTU fails sanity check: " << *d6dBasePMTU;

quic/state/CMakeLists.txt

@@ -32,6 +32,7 @@ add_dependencies(
   mvfst_codec
   mvfst_codec_types
   mvfst_handshake
+  mvfst_d6d_types
   mvfst_d6d_probe_raiser
 )
 
@@ -44,6 +45,7 @@ target_link_libraries(
   mvfst_codec
   mvfst_codec_types
   mvfst_handshake
+  mvfst_d6d_types
   mvfst_d6d_probe_raiser
 )
 

quic/state/StateData.h

@@ -772,35 +772,6 @@ struct QuicConnectionStateBase : public folly::DelayedDestruction {
 
   HappyEyeballsState happyEyeballsState;
 
-  struct D6DProbePacket {
-    D6DProbePacket() = delete;
-
-    explicit D6DProbePacket(PacketNum packetNumIn, uint32_t packetSizeIn)
-        : packetNum(packetNumIn), packetSize(packetSizeIn) {}
-
-    // Packet num
-    PacketNum packetNum;
-
-    // Udp packet payload size
-    uint32_t packetSize;
-  };
-
-  // States of d6d state machine, see
-  // https://tools.ietf.org/id/draft-ietf-tsvwg-datagram-plpmtud-21.html#name-state-machine
-  enum class D6DMachineState : uint8_t {
-    // Connection is not established yet
-    DISABLED = 0,
-    // Probe using base pmtu
-    BASE = 1,
-    // Incrementally probe using larger pmtu
-    SEARCHING = 2,
-    // Sleep for raise timeout before going to SEARCHING
-    SEARCH_COMPLETE = 3,
-    // Effective pmtu is less than base pmtu, continue probing with smaller
-    // packet
-    ERROR = 4
-  };
-
   // Meta state of d6d, mostly useful for analytics. D6D can operate without it.
   struct D6DMetaState {
     // Cumulative count of acked packets

quic/state/TransportSettings.h

@@ -10,6 +10,7 @@
 
 #include <quic/QuicConstants.h>
 #include <quic/codec/QuicConnectionId.h>
+#include <quic/d6d/Types.h>
 #include <chrono>
 
 namespace quic {
@@ -93,16 +94,6 @@ struct D6DConfig {
   std::chrono::seconds blackholeDetectionWindow{
       kDefaultD6DBlackholeDetectionWindow};
 
-  /**
-   * Two simple probe size raiser. Only server makes use of this value.
-   * ConstantSize: raise pmtu at constant step size
-   * BinarySearch: raise pmtu using binary search
-   */
-  enum class ProbeSizeRaiserType : uint8_t {
-    ConstantStep = 0,
-    BinarySearch = 1
-  };
-
   /**
    * Default raiser is constant step , since overshot caused by binary
    * search slows down convergence. Might change in the future when we