Move happy eyeballs state to client state.

Summary: This doesn't belong in the generic state. Untangling it is a little difficult, but I think this solution is cleaner than having it in the generic state.

Reviewed By: JunqiWang

Differential Revision: D29856391

fbshipit-source-id: 1042109ed29cd1d20d139e08548d187b469c8398

quic/api/IoBufQuicBatch.cpp

@@ -18,7 +18,7 @@ IOBufQuicBatch::IOBufQuicBatch(
     folly::AsyncUDPSocket& sock,
     const folly::SocketAddress& peerAddress,
     QuicTransportStatsCallback* statsCallback,
-    QuicConnectionStateBase::HappyEyeballsState& happyEyeballsState)
+    QuicClientConnectionState::HappyEyeballsState* happyEyeballsState)
     : batchWriter_(std::move(batchWriter)),
       threadLocal_(threadLocal),
       sock_(sock),
@@ -76,38 +76,45 @@ bool IOBufQuicBatch::flushInternal() {
 
   bool written = false;
   folly::Optional<int> firstSocketErrno;
-  if (happyEyeballsState_.shouldWriteToFirstSocket) {
+  if (!happyEyeballsState_ || happyEyeballsState_->shouldWriteToFirstSocket) {
     auto consumed = batchWriter_->write(sock_, peerAddress_);
-    firstSocketErrno = errno;
+    if (consumed < 0) {
+      firstSocketErrno = errno;
+    }
     written = (consumed >= 0);
-    happyEyeballsState_.shouldWriteToFirstSocket =
-        (consumed >= 0 || isRetriableError(errno));
+    if (happyEyeballsState_) {
+      happyEyeballsState_->shouldWriteToFirstSocket =
+          (consumed >= 0 || isRetriableError(errno));
 
-    if (!happyEyeballsState_.shouldWriteToFirstSocket) {
-      sock_.pauseRead();
+      if (!happyEyeballsState_->shouldWriteToFirstSocket) {
+        sock_.pauseRead();
+      }
     }
   }
 
   // If error occured on first socket, kick off second socket immediately
-  if (!written && happyEyeballsState_.connAttemptDelayTimeout &&
-      happyEyeballsState_.connAttemptDelayTimeout->isScheduled()) {
-    happyEyeballsState_.connAttemptDelayTimeout->timeoutExpired();
-    happyEyeballsState_.connAttemptDelayTimeout->cancelTimeout();
+  if (!written && happyEyeballsState_ &&
+      happyEyeballsState_->connAttemptDelayTimeout &&
+      happyEyeballsState_->connAttemptDelayTimeout->isScheduled()) {
+    happyEyeballsState_->connAttemptDelayTimeout->timeoutExpired();
+    happyEyeballsState_->connAttemptDelayTimeout->cancelTimeout();
   }
 
   folly::Optional<int> secondSocketErrno;
-  if (happyEyeballsState_.shouldWriteToSecondSocket) {
+  if (happyEyeballsState_ && happyEyeballsState_->shouldWriteToSecondSocket) {
     auto consumed = batchWriter_->write(
-        *happyEyeballsState_.secondSocket,
-        happyEyeballsState_.secondPeerAddress);
-    secondSocketErrno = errno;
+        *happyEyeballsState_->secondSocket,
+        happyEyeballsState_->secondPeerAddress);
+    if (consumed < 0) {
+      secondSocketErrno = errno;
+    }
 
     // written is marked true if either socket write succeeds
     written |= (consumed >= 0);
-    happyEyeballsState_.shouldWriteToSecondSocket =
+    happyEyeballsState_->shouldWriteToSecondSocket =
         (consumed >= 0 || isRetriableError(errno));
-    if (!happyEyeballsState_.shouldWriteToSecondSocket) {
-      happyEyeballsState_.secondSocket->pauseRead();
+    if (!happyEyeballsState_->shouldWriteToSecondSocket) {
+      happyEyeballsState_->secondSocket->pauseRead();
     }
   }
 
@@ -128,8 +135,12 @@ bool IOBufQuicBatch::flushInternal() {
     }
   }
 
-  if (!happyEyeballsState_.shouldWriteToFirstSocket &&
-      !happyEyeballsState_.shouldWriteToSecondSocket) {
+  // If we have no happy eyeballs state, we only care if the first socket had
+  // an error. Otherwise we check both.
+  if ((!happyEyeballsState_ && firstSocketErrno.has_value() &&
+       !isRetriableError(firstSocketErrno.value())) ||
+      (happyEyeballsState_ && !happyEyeballsState_->shouldWriteToFirstSocket &&
+       !happyEyeballsState_->shouldWriteToSecondSocket)) {
     auto firstSocketErrorMsg = firstSocketErrno.has_value()
         ? folly::to<std::string>(
               folly::errnoStr(firstSocketErrno.value()), ", ")

quic/api/IoBufQuicBatch.h

@@ -9,6 +9,7 @@
 #pragma once
 #include <quic/QuicException.h>
 #include <quic/api/QuicBatchWriter.h>
+#include <quic/client/state/ClientStateMachine.h>
 #include <quic/state/QuicTransportStatsCallback.h>
 
 namespace quic {
@@ -25,7 +26,7 @@ class IOBufQuicBatch {
       folly::AsyncUDPSocket& sock,
       const folly::SocketAddress& peerAddress,
       QuicTransportStatsCallback* statsCallback,
-      QuicConnectionStateBase::HappyEyeballsState& happyEyeballsState);
+      QuicClientConnectionState::HappyEyeballsState* happyEyeballsState);
 
   ~IOBufQuicBatch() = default;
 
@@ -55,7 +56,7 @@ class IOBufQuicBatch {
   folly::AsyncUDPSocket& sock_;
   const folly::SocketAddress& peerAddress_;
   QuicTransportStatsCallback* statsCallback_{nullptr};
-  QuicConnectionStateBase::HappyEyeballsState& happyEyeballsState_;
+  QuicClientConnectionState::HappyEyeballsState* happyEyeballsState_;
   uint64_t pktSent_{0};
 };
 

quic/api/QuicTransportFunctions.cpp

@@ -1320,13 +1320,16 @@ uint64_t writeConnectionDataToSocket(
       connection.transportSettings.dataPathType,
       connection);
 
+  auto happyEyeballsState = connection.nodeType == QuicNodeType::Server
+      ? nullptr
+      : &static_cast<QuicClientConnectionState&>(connection).happyEyeballsState;
   IOBufQuicBatch ioBufBatch(
       std::move(batchWriter),
       connection.transportSettings.useThreadLocalBatching,
       sock,
       connection.peerAddress,
       connection.statsCallback,
-      connection.happyEyeballsState);
+      happyEyeballsState);
 
   if (connection.loopDetectorCallback) {
     connection.writeDebugState.schedulerName = scheduler.name().str();

quic/api/test/IoBufQuicBatchTest.cpp

@@ -27,7 +27,7 @@ void RunTest(int numBatch) {
   folly::SocketAddress peerAddress{"127.0.0.1", 1234};
   QuicClientConnectionState conn(
       FizzClientQuicHandshakeContext::Builder().build());
-  QuicConnectionStateBase::HappyEyeballsState happyEyeballsState;
+  QuicClientConnectionState::HappyEyeballsState happyEyeballsState;
 
   IOBufQuicBatch ioBufBatch(
       std::move(batchWriter),
@@ -35,7 +35,7 @@ void RunTest(int numBatch) {
       sock,
       peerAddress,
       conn.statsCallback,
-      happyEyeballsState);
+      nullptr /* happyEyeballsState */);
 
   std::string strTest("Test");
 

quic/client/QuicClientTransport.cpp

@@ -99,8 +99,8 @@ QuicClientTransport::~QuicClientTransport() {
           std::string("Closing from client destructor")),
       false);
 
-  if (conn_->happyEyeballsState.secondSocket) {
-    auto sock = std::move(conn_->happyEyeballsState.secondSocket);
+  if (clientConn_->happyEyeballsState.secondSocket) {
+    auto sock = std::move(clientConn_->happyEyeballsState.secondSocket);
     sock->pauseRead();
     sock->close();
   }
@@ -213,7 +213,7 @@ void QuicClientTransport::processPacketData(
 
     if (happyEyeballsEnabled_) {
       happyEyeballsOnDataReceived(
-          *conn_, happyEyeballsConnAttemptDelayTimeout_, socket_, peer);
+          *clientConn_, happyEyeballsConnAttemptDelayTimeout_, socket_, peer);
     }
     // Set the destination connection ID to be the value from the source
     // connection id of the retry packet
@@ -292,7 +292,7 @@ void QuicClientTransport::processPacketData(
   if (happyEyeballsEnabled_) {
     CHECK(socket_);
     happyEyeballsOnDataReceived(
-        *conn_, happyEyeballsConnAttemptDelayTimeout_, socket_, peer);
+        *clientConn_, happyEyeballsConnAttemptDelayTimeout_, socket_, peer);
   }
 
   LongHeader* longHeader = regularOptional->header.asLong();
@@ -1015,7 +1015,7 @@ void QuicClientTransport::errMessage(
     // exists, and the second socket is IPv4. Then we basically do the same
     // thing we would have done if we'd gotten a write error on that socket.
     // If both sockets are not functional we close the connection.
-    auto& happyEyeballsState = conn_->happyEyeballsState;
+    auto& happyEyeballsState = clientConn_->happyEyeballsState;
     if (!happyEyeballsState.finished) {
       if (cmsg.cmsg_level == SOL_IPV6 &&
           happyEyeballsState.shouldWriteToFirstSocket) {
@@ -1485,7 +1485,7 @@ void QuicClientTransport::
     happyEyeballsConnAttemptDelayTimeoutExpired() noexcept {
   // Declare 0-RTT data as lost so that they will be retransmitted over the
   // second socket.
-  happyEyeballsStartSecondSocket(conn_->happyEyeballsState);
+  happyEyeballsStartSecondSocket(clientConn_->happyEyeballsState);
   // If this gets called from the write path then we haven't added the packets
   // to the outstanding packet list yet.
   runOnEvbAsync([&](auto) { markZeroRttPacketsLost(*conn_, markPacketLoss); });
@@ -1495,7 +1495,7 @@ void QuicClientTransport::start(ConnectionCallback* cb) {
   if (happyEyeballsEnabled_) {
     // TODO Supply v4 delay amount from somewhere when we want to tune this
     startHappyEyeballs(
-        *conn_,
+        *clientConn_,
         evb_,
         happyEyeballsCachedFamily_,
         happyEyeballsConnAttemptDelayTimeout_,
@@ -1558,7 +1558,7 @@ void QuicClientTransport::addNewPeerAddress(folly::SocketAddress peerAddress) {
         conn_->udpSendPacketLen,
         (peerAddress.getFamily() == AF_INET6 ? kDefaultV6UDPSendPacketLen
                                              : kDefaultV4UDPSendPacketLen));
-    happyEyeballsAddPeerAddress(*conn_, peerAddress);
+    happyEyeballsAddPeerAddress(*clientConn_, peerAddress);
     return;
   }
 
@@ -1585,7 +1585,7 @@ void QuicClientTransport::setHappyEyeballsCachedFamily(
 
 void QuicClientTransport::addNewSocket(
     std::unique_ptr<folly::AsyncUDPSocket> socket) {
-  happyEyeballsAddSocket(*conn_, std::move(socket));
+  happyEyeballsAddSocket(*clientConn_, std::move(socket));
 }
 
 void QuicClientTransport::setHostname(const std::string& hostname) {

quic/client/state/ClientStateMachine.h

@@ -71,6 +71,36 @@ struct QuicClientConnectionState : public QuicConnectionStateBase {
   uint64_t peerAdvertisedInitialMaxStreamsBidi{0};
   uint64_t peerAdvertisedInitialMaxStreamsUni{0};
 
+  struct HappyEyeballsState {
+    // Delay timer
+    folly::HHWheelTimer::Callback* connAttemptDelayTimeout{nullptr};
+
+    // IPv6 peer address
+    folly::SocketAddress v6PeerAddress;
+
+    // IPv4 peer address
+    folly::SocketAddress v4PeerAddress;
+
+    // The address that this socket will try to connect to after connection
+    // attempt delay timeout fires
+    folly::SocketAddress secondPeerAddress;
+
+    // The UDP socket that will be used for the second connection attempt
+    std::unique_ptr<folly::AsyncUDPSocket> secondSocket;
+
+    // Whether should write to the first UDP socket
+    bool shouldWriteToFirstSocket{true};
+
+    // Whether should write to the second UDP socket
+    bool shouldWriteToSecondSocket{false};
+
+    // Whether HappyEyeballs has finished
+    // The signal of finishing is first successful decryption of a packet
+    bool finished{false};
+  };
+
+  HappyEyeballsState happyEyeballsState;
+
   // Short header packets we received but couldn't yet decrypt.
   std::vector<PendingClientData> pendingOneRttData;
   // Handshake packets we received but couldn't yet decrypt.

quic/dsr/backend/DSRPacketizer.cpp

@@ -104,14 +104,13 @@ size_t writePacketsGroup(
   auto batchWriter =
       BatchWriterPtr(new GSOPacketBatchWriter(kDefaultQuicMaxBatchSize));
   // This doesn't matter:
-  QuicConnectionStateBase::HappyEyeballsState happyEyeballsState;
   IOBufQuicBatch ioBufBatch(
       std::move(batchWriter),
       false /* thread local batching */,
       sock,
       reqGroup[0].clientAddress,
       nullptr /* statsCallback */,
-      happyEyeballsState);
+      nullptr /* happyEyeballsState */);
   // TODO: Instead of building ciphers every time, we should cache them into a
   // CipherMap and look them up.
   CipherBuilder cipherBuilder;

quic/dsr/backend/test/DSRPacketizerTest.cpp

@@ -42,7 +42,6 @@ class DSRPacketizerSingleWriteTest : public Test {
   }
 
   folly::EventBase evb;
-  QuicConnectionStateBase::HappyEyeballsState happyEyeballsState;
   folly::SocketAddress peerAddress{"127.0.0.1", 1234};
   std::unique_ptr<Aead> aead;
   std::unique_ptr<PacketNumberCipher> headerCipher;
@@ -59,7 +58,7 @@ TEST_F(DSRPacketizerSingleWriteTest, SingleWrite) {
       *socket,
       peerAddress,
       nullptr /* statsCallback */,
-      happyEyeballsState);
+      nullptr /* happyEyeballsState */);
   PacketNum packetNum = 20;
   PacketNum largestAckedByPeer = 0;
   StreamId streamId = 0;
@@ -102,7 +101,7 @@ TEST_F(DSRPacketizerSingleWriteTest, NotEnoughData) {
       *socket,
       peerAddress,
       nullptr /* statsCallback */,
-      happyEyeballsState);
+      nullptr /* happyEyeballsState */);
   PacketNum packetNum = 20;
   PacketNum largestAckedByPeer = 0;
   StreamId streamId = 0;

quic/happyeyeballs/QuicHappyEyeballsFunctions.cpp

@@ -28,7 +28,7 @@ namespace fsp = folly::portability::sockets;
 namespace quic {
 
 void happyEyeballsAddPeerAddress(
-    QuicConnectionStateBase& connection,
+    QuicClientConnectionState& connection,
     const folly::SocketAddress& peerAddress) {
   // TODO: Do not wait for both IPv4 and IPv6 addresses to return before
   // attempting connection establishment. -- RFC8305
@@ -52,13 +52,13 @@ void happyEyeballsAddPeerAddress(
 }
 
 void happyEyeballsAddSocket(
-    QuicConnectionStateBase& connection,
+    QuicClientConnectionState& connection,
     std::unique_ptr<folly::AsyncUDPSocket> socket) {
   connection.happyEyeballsState.secondSocket = std::move(socket);
 }
 
 void startHappyEyeballs(
-    QuicConnectionStateBase& connection,
+    QuicClientConnectionState& connection,
     folly::EventBase* evb,
     sa_family_t cachedFamily,
     folly::HHWheelTimer::Callback& connAttemptDelayTimeout,
@@ -166,14 +166,14 @@ void happyEyeballsSetUpSocket(
 }
 
 void happyEyeballsStartSecondSocket(
-    QuicConnectionStateBase::HappyEyeballsState& happyEyeballsState) {
+    QuicClientConnectionState::HappyEyeballsState& happyEyeballsState) {
   CHECK(!happyEyeballsState.finished);
 
   happyEyeballsState.shouldWriteToSecondSocket = true;
 }
 
 void happyEyeballsOnDataReceived(
-    QuicConnectionStateBase& connection,
+    QuicClientConnectionState& connection,
     folly::HHWheelTimer::Callback& connAttemptDelayTimeout,
     std::unique_ptr<folly::AsyncUDPSocket>& socket,
     const folly::SocketAddress& peerAddress) {

quic/happyeyeballs/QuicHappyEyeballsFunctions.h

@@ -8,7 +8,7 @@
 
 #pragma once
 
-#include <quic/state/StateData.h>
+#include <quic/client/state/ClientStateMachine.h>
 
 #include <folly/io/SocketOptionMap.h>
 #include <folly/io/async/AsyncUDPSocket.h>
@@ -26,15 +26,15 @@ namespace quic {
 struct TransportSettings;
 
 void happyEyeballsAddPeerAddress(
-    QuicConnectionStateBase& connection,
+    QuicClientConnectionState& connection,
     const folly::SocketAddress& peerAddress);
 
 void happyEyeballsAddSocket(
-    QuicConnectionStateBase& connection,
+    QuicClientConnectionState& connection,
     std::unique_ptr<folly::AsyncUDPSocket> socket);
 
 void startHappyEyeballs(
-    QuicConnectionStateBase& connection,
+    QuicClientConnectionState& connection,
     folly::EventBase* evb,
     sa_family_t cachedFamily,
     folly::HHWheelTimer::Callback& connAttemptDelayTimeout,
@@ -43,7 +43,7 @@ void startHappyEyeballs(
     folly::AsyncUDPSocket::ReadCallback* readCallback,
     const folly::SocketOptionMap& options);
 
-void resetHappyEyeballs(QuicConnectionStateBase& connection);
+void resetHappyEyeballs(QuicClientConnectionState& connection);
 
 void happyEyeballsSetUpSocket(
     folly::AsyncUDPSocket& socket,
@@ -55,10 +55,10 @@ void happyEyeballsSetUpSocket(
     const folly::SocketOptionMap& options);
 
 void happyEyeballsStartSecondSocket(
-    QuicConnectionStateBase::HappyEyeballsState& happyEyeballsState);
+    QuicClientConnectionState::HappyEyeballsState& happyEyeballsState);
 
 void happyEyeballsOnDataReceived(
-    QuicConnectionStateBase& connection,
+    QuicClientConnectionState& connection,
     folly::HHWheelTimer::Callback& connAttemptDelayTimeout,
     std::unique_ptr<folly::AsyncUDPSocket>& socket,
     const folly::SocketAddress& peerAddress);

quic/state/StateData.h

@@ -685,36 +685,6 @@ struct QuicConnectionStateBase : public folly::DelayedDestruction {
   // Track stats for various server events
   QuicTransportStatsCallback* statsCallback{nullptr};
 
-  struct HappyEyeballsState {
-    // Delay timer
-    folly::HHWheelTimer::Callback* connAttemptDelayTimeout{nullptr};
-
-    // IPv6 peer address
-    folly::SocketAddress v6PeerAddress;
-
-    // IPv4 peer address
-    folly::SocketAddress v4PeerAddress;
-
-    // The address that this socket will try to connect to after connection
-    // attempt delay timeout fires
-    folly::SocketAddress secondPeerAddress;
-
-    // The UDP socket that will be used for the second connection attempt
-    std::unique_ptr<folly::AsyncUDPSocket> secondSocket;
-
-    // Whether should write to the first UDP socket
-    bool shouldWriteToFirstSocket{true};
-
-    // Whether should write to the second UDP socket
-    bool shouldWriteToSecondSocket{false};
-
-    // Whether HappyEyeballs has finished
-    // The signal of finishing is first successful decryption of a packet
-    bool finished{false};
-  };
-
-  HappyEyeballsState happyEyeballsState;
-
   // Meta state of d6d, mostly useful for analytics. D6D can operate without it.
   struct D6DMetaState {
     // Cumulative count of acked packets