Implement basic ACK_FREQUENCY support.

Summary: As in title. This doesn't actually send any frames, but implements basic support for the transport parameter and responding to the frames.

Reviewed By: yangchi

Differential Revision: D26134787

fbshipit-source-id: 2c48e01084034317c8f36f89c69d172e3cb42278

quic/QuicConstants.h

@@ -161,6 +161,7 @@ enum class FrameType : uint64_t {
   MIN_STREAM_DATA = 0xFE, // subject to change
   EXPIRED_STREAM_DATA = 0xFF, // subject to change
   KNOB = 0x1550,
+  ACK_FREQUENCY = 0xAF,
 };
 
 inline constexpr uint16_t toFrameError(FrameType frame) {

quic/api/QuicTransportBase.cpp

@@ -2507,7 +2507,7 @@ void QuicTransportBase::scheduleAckTimeout() {
       auto timeout = timeMax(
           std::chrono::duration_cast<std::chrono::microseconds>(
               wheelTimer.getTickInterval()),
-          timeMin(kMaxAckTimeout, factoredRtt));
+          timeMin(conn_->ackStates.maxAckDelay, factoredRtt));
       auto timeoutMs = folly::chrono::ceil<std::chrono::milliseconds>(timeout);
       VLOG(10) << __func__ << " timeout=" << timeoutMs.count() << "ms"
                << " factoredRtt=" << factoredRtt.count() << "us"

quic/api/test/QuicTransportTest.cpp

@@ -2942,6 +2942,20 @@ TEST_F(QuicTransportTest, ScheduleAckTimeout) {
   EXPECT_NEAR(transport_->getAckTimeout()->getTimeRemaining().count(), 25, 5);
 }
 
+TEST_F(QuicTransportTest, ScheduleAckTimeoutFromMaxAckDelay) {
+  // Make srtt large so we will use maxAckDelay
+  transport_->getConnectionState().lossState.srtt = 25000000us;
+  transport_->getConnectionState().ackStates.maxAckDelay = 10ms;
+  EXPECT_FALSE(transport_->getAckTimeout()->isScheduled());
+  transport_->getConnectionState().pendingEvents.scheduleAckTimeout = true;
+  transport_->onNetworkData(
+      SocketAddress("::1", 10003),
+      NetworkData(
+          IOBuf::copyBuffer("Never on time, always timeout"), Clock::now()));
+  EXPECT_TRUE(transport_->getAckTimeout()->isScheduled());
+  EXPECT_NEAR(transport_->getAckTimeout()->getTimeRemaining().count(), 10, 5);
+}
+
 TEST_F(QuicTransportTest, CloseTransportCancelsAckTimeout) {
   transport_->getConnectionState().lossState.srtt = 25000000us;
   EXPECT_FALSE(transport_->getAckTimeout()->isScheduled());

quic/client/QuicClientTransport.cpp

@@ -881,6 +881,7 @@ void QuicClientTransport::startCryptoHandshake() {
   setD6DBasePMTUTransportParameter();
   setD6DRaiseTimeoutTransportParameter();
   setD6DProbeTimeoutTransportParameter();
+  setSupportedExtensionTransportParameters();
 
   auto paramsExtension = std::make_shared<ClientTransportParametersExtension>(
       conn_->originalVersion.value(),
@@ -1501,6 +1502,7 @@ bool QuicClientTransport::setCustomTransportParameter(
   // described by the spec.
   if (static_cast<uint16_t>(customParam->getParameterId()) <
       kCustomTransportParameterThreshold) {
+    LOG(ERROR) << "invalid parameter id";
     return false;
   }
 
@@ -1515,6 +1517,7 @@ bool QuicClientTransport::setCustomTransportParameter(
 
   // if a match has been found, we return failure
   if (it != customTransportParameters_.end()) {
+    LOG(ERROR) << "transport parameter already present";
     return false;
   }
 
@@ -1605,6 +1608,15 @@ void QuicClientTransport::setD6DProbeTimeoutTransportParameter() {
   }
 }
 
+void QuicClientTransport::setSupportedExtensionTransportParameters() {
+  if (conn_->transportSettings.minAckDelay.hasValue()) {
+    auto minAckDelayParam = std::make_unique<CustomIntegralTransportParameter>(
+        static_cast<uint64_t>(TransportParameterId::min_ack_delay),
+        conn_->transportSettings.minAckDelay.value().count());
+    customTransportParameters_.push_back(minAckDelayParam->encode());
+  }
+}
+
 void QuicClientTransport::adjustGROBuffers() {
   if (socket_ && conn_) {
     if (conn_->transportSettings.numGROBuffers_ > kDefaultNumGROBuffers) {

quic/client/QuicClientTransport.h

@@ -208,6 +208,7 @@ class QuicClientTransport
   void setD6DBasePMTUTransportParameter();
   void setD6DRaiseTimeoutTransportParameter();
   void setD6DProbeTimeoutTransportParameter();
+  void setSupportedExtensionTransportParameters();
   void adjustGROBuffers();
   void trackDatagramReceived(size_t len);
 

quic/codec/Decode.cpp

@@ -99,6 +99,44 @@ KnobFrame decodeKnobFrame(folly::io::Cursor& cursor) {
   return KnobFrame(knobSpace->first, knobId->first, std::move(knobBlob));
 }
 
+AckFrequencyFrame decodeAckFrequencyFrame(folly::io::Cursor& cursor) {
+  auto sequenceNumber = decodeQuicInteger(cursor);
+  if (!sequenceNumber) {
+    throw QuicTransportException(
+        "Bad sequence number",
+        quic::TransportErrorCode::FRAME_ENCODING_ERROR,
+        quic::FrameType::ACK_FREQUENCY);
+  }
+  auto packetTolerance = decodeQuicInteger(cursor);
+  if (!packetTolerance) {
+    throw QuicTransportException(
+        "Bad packet tolerance",
+        quic::TransportErrorCode::FRAME_ENCODING_ERROR,
+        quic::FrameType::ACK_FREQUENCY);
+  }
+  auto updateMaxAckDelay = decodeQuicInteger(cursor);
+  if (!updateMaxAckDelay) {
+    throw QuicTransportException(
+        "Bad update max ack delay",
+        quic::TransportErrorCode::FRAME_ENCODING_ERROR,
+        quic::FrameType::ACK_FREQUENCY);
+  }
+  if (cursor.isAtEnd()) {
+    throw QuicTransportException(
+        "Bad ignore order",
+        quic::TransportErrorCode::FRAME_ENCODING_ERROR,
+        quic::FrameType::ACK_FREQUENCY);
+  }
+  auto ignoreOrder = cursor.readBE<uint8_t>();
+
+  AckFrequencyFrame frame;
+  frame.sequenceNumber = sequenceNumber->first;
+  frame.packetTolerance = packetTolerance->first;
+  frame.updateMaxAckDelay = updateMaxAckDelay->first;
+  frame.ignoreOrder = ignoreOrder;
+  return frame;
+}
+
 ReadAckFrame decodeAckFrame(
     folly::io::Cursor& cursor,
     const PacketHeader& header,
@@ -813,6 +851,8 @@ QuicFrame parseFrame(
         return QuicFrame(decodeHandshakeDoneFrame(cursor));
       case FrameType::KNOB:
         return QuicFrame(decodeKnobFrame(cursor));
+      case FrameType::ACK_FREQUENCY:
+        return QuicFrame(decodeAckFrequencyFrame(cursor));
     }
   } catch (const std::exception&) {
     error = true;

quic/codec/Decode.h

@@ -96,6 +96,8 @@ PingFrame decodePingFrame(folly::io::Cursor& cursor);
 
 KnobFrame decodeKnobFrame(folly::io::Cursor& cursor);
 
+AckFrequencyFrame decodeAckFrequencyFrame(folly::io::Cursor& cursor);
+
 DataBlockedFrame decodeDataBlockedFrame(folly::io::Cursor& cursor);
 
 StreamDataBlockedFrame decodeStreamDataBlockedFrame(folly::io::Cursor& cursor);

quic/codec/QuicWriteCodec.cpp

@@ -510,6 +510,27 @@ size_t writeSimpleFrame(
       // no space left in packet
       return size_t(0);
     }
+    case QuicSimpleFrame::Type::AckFrequencyFrame: {
+      const auto ackFrequencyFrame = frame.asAckFrequencyFrame();
+      QuicInteger intFrameType(static_cast<uint64_t>(FrameType::ACK_FREQUENCY));
+      QuicInteger intSequenceNumber(ackFrequencyFrame->sequenceNumber);
+      QuicInteger intPacketTolerance(ackFrequencyFrame->packetTolerance);
+      QuicInteger intUpdateMaxAckDelay(ackFrequencyFrame->updateMaxAckDelay);
+      size_t ackFrequencyFrameLen = intFrameType.getSize() +
+          intSequenceNumber.getSize() + intPacketTolerance.getSize() +
+          intUpdateMaxAckDelay.getSize() + 1 /* ignoreOrder */;
+      if (packetSpaceCheck(spaceLeft, ackFrequencyFrameLen)) {
+        builder.write(intFrameType);
+        builder.write(intSequenceNumber);
+        builder.write(intPacketTolerance);
+        builder.write(intUpdateMaxAckDelay);
+        builder.writeBE(ackFrequencyFrame->ignoreOrder);
+        builder.appendFrame(QuicSimpleFrame(*ackFrequencyFrame));
+        return ackFrequencyFrameLen;
+      }
+      // no space left in packet
+      return size_t(0);
+    }
   }
   folly::assume_unreachable();
 }

quic/codec/Types.cpp

@@ -423,6 +423,8 @@ std::string toString(FrameType frame) {
       return "HANDSHAKE_DONE";
     case FrameType::KNOB:
       return "KNOB";
+    case FrameType::ACK_FREQUENCY:
+      return "ACK_FREQUENCY";
   }
   LOG(WARNING) << "toString has unhandled frame type";
   return "UNKNOWN";

quic/codec/Types.h

@@ -109,6 +109,20 @@ struct KnobFrame {
   Buf blob;
 };
 
+struct AckFrequencyFrame {
+  uint64_t sequenceNumber; // Used to identify newest.
+  uint64_t packetTolerance; // How many packets before ACKing.
+  uint64_t updateMaxAckDelay; // New max_ack_delay to use.
+  uint8_t ignoreOrder; // Whether to ignore reordering ACKs.
+
+  bool operator==(const AckFrequencyFrame& other) const {
+    return other.sequenceNumber == sequenceNumber &&
+        other.packetTolerance == packetTolerance &&
+        other.updateMaxAckDelay == updateMaxAckDelay &&
+        other.ignoreOrder == ignoreOrder;
+  }
+};
+
 /**
  * AckBlock represents a series of continuous packet sequences from
  * [startPacket, endPacket]
@@ -644,7 +658,8 @@ struct RetryToken {
   F(MaxStreamsFrame, __VA_ARGS__)         \
   F(RetireConnectionIdFrame, __VA_ARGS__) \
   F(HandshakeDoneFrame, __VA_ARGS__)      \
-  F(KnobFrame, __VA_ARGS__)
+  F(KnobFrame, __VA_ARGS__)               \
+  F(AckFrequencyFrame, __VA_ARGS__)
 
 DECLARE_VARIANT_TYPE(QuicSimpleFrame, QUIC_SIMPLE_FRAME)
 

quic/handshake/TransportParameters.h

@@ -32,6 +32,7 @@ enum class TransportParameterId : uint64_t {
   active_connection_id_limit = 0x000e,
   initial_source_connection_id = 0x000f,
   retry_source_connection_id = 0x0010,
+  min_ack_delay = 0xff02de1a,
 };
 
 struct TransportParameter {

quic/logging/BaseQLogger.cpp

@@ -71,6 +71,15 @@ void addQuicSimpleFrameToEvent(
           frame.knobSpace, frame.id, frame.blob->length()));
       break;
     }
+    case quic::QuicSimpleFrame::Type::AckFrequencyFrame: {
+      const quic::AckFrequencyFrame& frame = *simpleFrame.asAckFrequencyFrame();
+      event->frames.push_back(std::make_unique<quic::AckFrequencyFrameLog>(
+          frame.sequenceNumber,
+          frame.packetTolerance,
+          frame.updateMaxAckDelay,
+          frame.ignoreOrder));
+      break;
+    }
   }
 }
 } // namespace

quic/logging/QLoggerConstants.cpp

@@ -79,6 +79,8 @@ folly::StringPiece toQlogString(FrameType frame) {
       return "handshake_done";
     case FrameType::KNOB:
       return "knob";
+    case FrameType::ACK_FREQUENCY:
+      return "ack_frequency";
   }
   folly::assume_unreachable();
 }

quic/logging/QLoggerTypes.cpp

@@ -110,6 +110,16 @@ folly::dynamic KnobFrameLog::toDynamic() const {
   return d;
 }
 
+folly::dynamic AckFrequencyFrameLog::toDynamic() const {
+  folly::dynamic d = folly::dynamic::object();
+  d["frame_type"] = toQlogString(FrameType::ACK_FREQUENCY);
+  d["sequence_number"] = sequenceNumber;
+  d["packet_tolerance"] = packetTolerance;
+  d["update_max_ack_delay"] = updateMaxAckDelay;
+  d["ignore_order"] = ignoreOrder;
+  return d;
+}
+
 folly::dynamic StreamDataBlockedFrameLog::toDynamic() const {
   folly::dynamic d = folly::dynamic::object();
   d["frame_type"] = toQlogString(FrameType::STREAM_DATA_BLOCKED);

quic/logging/QLoggerTypes.h

@@ -145,6 +145,26 @@ class KnobFrameLog : public QLogFrame {
   FOLLY_NODISCARD folly::dynamic toDynamic() const override;
 };
 
+class AckFrequencyFrameLog : public QLogFrame {
+ public:
+  uint64_t sequenceNumber;
+  uint64_t packetTolerance;
+  uint64_t updateMaxAckDelay;
+  bool ignoreOrder;
+
+  explicit AckFrequencyFrameLog(
+      uint64_t sequenceNumberIn,
+      uint64_t packetToleranceIn,
+      uint64_t updateMaxAckDelayIn,
+      bool ignoreOrderIn)
+      : sequenceNumber(sequenceNumberIn),
+        packetTolerance(packetToleranceIn),
+        updateMaxAckDelay(updateMaxAckDelayIn),
+        ignoreOrder(ignoreOrderIn) {}
+  ~AckFrequencyFrameLog() override = default;
+  FOLLY_NODISCARD folly::dynamic toDynamic() const override;
+};
+
 class StreamDataBlockedFrameLog : public QLogFrame {
  public:
   StreamId streamId;

quic/server/state/ServerStateMachine.cpp

@@ -136,6 +136,8 @@ void processClientInitialParams(
   auto d6dProbeTimeout = getIntegerParameter(
       static_cast<TransportParameterId>(kD6DProbeTimeoutParameterId),
       clientParams.parameters);
+  auto minAckDelay = getIntegerParameter(
+      TransportParameterId::min_ack_delay, clientParams.parameters);
   if (conn.version == QuicVersion::QUIC_DRAFT) {
     auto initialSourceConnId = getConnIdParameter(
         TransportParameterId::initial_source_connection_id,
@@ -186,6 +188,9 @@ void processClientInitialParams(
   }
   conn.peerAckDelayExponent =
       ackDelayExponent.value_or(kDefaultAckDelayExponent);
+  if (minAckDelay.hasValue()) {
+    conn.peerMinAckDelay = std::chrono::microseconds(minAckDelay.value());
+  }
 
   // Default to max because we can probe PMTU now, and this will be the upper
   // limit

quic/state/AckStates.h

@@ -33,6 +33,9 @@ struct AckState {
   // Next PacketNum we will send for packet in this packet number space
   PacketNum nextPacketNum{0};
   AckBlocks acks;
+  bool ignoreReorder{false};
+  folly::Optional<uint64_t> tolerance;
+  folly::Optional<uint64_t> ackFrequencySequenceNumber;
   // Flag indicating that if we need to send ack immediately. This will be set
   // to true if we got packets with retransmittable data and haven't sent the
   // ack for the first time.
@@ -57,6 +60,7 @@ struct AckStates {
   AckState handshakeAckState;
   // AckState for acks to peer packets in AppData packet number space.
   AckState appDataAckState;
+  std::chrono::microseconds maxAckDelay{kMaxAckTimeout};
 };
 
 } // namespace quic

quic/state/QuicStateFunctions.cpp

@@ -90,10 +90,17 @@ void updateAckSendStateOnRecvPacket(
   DCHECK(!pktHasCryptoData || pktHasRetransmittableData);
   auto thresh = kNonRtxRxPacketsPendingBeforeAck;
   if (pktHasRetransmittableData || ackState.numRxPacketsRecvd) {
-    thresh = ackState.largestReceivedPacketNum.value_or(0) >
+    if (ackState.tolerance.hasValue()) {
-            conn.transportSettings.rxPacketsBeforeAckInitThreshold
+      thresh = ackState.tolerance.value();
-        ? conn.transportSettings.rxPacketsBeforeAckAfterInit
+    } else {
-        : conn.transportSettings.rxPacketsBeforeAckBeforeInit;
+      thresh = ackState.largestReceivedPacketNum.value_or(0) >
+              conn.transportSettings.rxPacketsBeforeAckInitThreshold
+          ? conn.transportSettings.rxPacketsBeforeAckAfterInit
+          : conn.transportSettings.rxPacketsBeforeAckBeforeInit;
+    }
+  }
+  if (ackState.ignoreReorder) {
+    pktOutOfOrder = false;
   }
   if (pktHasRetransmittableData) {
     if (pktHasCryptoData || pktOutOfOrder ||

quic/state/SimpleFrameFunctions.cpp

@@ -55,6 +55,7 @@ folly::Optional<QuicSimpleFrame> updateSimpleFrameOnPacketClone(
     case QuicSimpleFrame::Type::MaxStreamsFrame:
     case QuicSimpleFrame::Type::HandshakeDoneFrame:
     case QuicSimpleFrame::Type::KnobFrame:
+    case QuicSimpleFrame::Type::AckFrequencyFrame:
     case QuicSimpleFrame::Type::RetireConnectionIdFrame:
       // TODO junqiw
       return QuicSimpleFrame(frame);
@@ -133,6 +134,7 @@ void updateSimpleFrameOnPacketLoss(
     case QuicSimpleFrame::Type::MaxStreamsFrame:
     case QuicSimpleFrame::Type::RetireConnectionIdFrame:
     case QuicSimpleFrame::Type::KnobFrame:
+    case QuicSimpleFrame::Type::AckFrequencyFrame:
       conn.pendingEvents.frames.push_back(frame);
       break;
   }
@@ -294,6 +296,24 @@ bool updateSimpleFrameOnPacketReceived(
           knobFrame.knobSpace, knobFrame.id, knobFrame.blob->clone());
       return true;
     }
+    case QuicSimpleFrame::Type::AckFrequencyFrame: {
+      if (!conn.transportSettings.minAckDelay.hasValue()) {
+        return true;
+      }
+      const auto ackFrequencyFrame = frame.asAckFrequencyFrame();
+      auto& ackState = conn.ackStates.appDataAckState;
+      if (!ackState.ackFrequencySequenceNumber ||
+          ackFrequencyFrame->sequenceNumber >
+              ackState.ackFrequencySequenceNumber.value()) {
+        ackState.tolerance = ackFrequencyFrame->packetTolerance;
+        ackState.ignoreReorder = ackFrequencyFrame->ignoreOrder;
+        conn.ackStates.maxAckDelay =
+            std::chrono::microseconds(std::max<uint64_t>(
+                conn.transportSettings.minAckDelay->count(),
+                ackFrequencyFrame->updateMaxAckDelay));
+      }
+      return true;
+    }
   }
   folly::assume_unreachable();
 }

quic/state/StateData.h

@@ -645,6 +645,9 @@ struct QuicConnectionStateBase : public folly::DelayedDestruction {
   // Value of the negotiated ack delay exponent.
   uint64_t peerAckDelayExponent{kDefaultAckDelayExponent};
 
+  // The value of the peer's min_ack_delay, for creating ACK_FREQUENCY frames.
+  folly::Optional<std::chrono::microseconds> peerMinAckDelay;
+
   // Idle timeout advertised by the peer. Initially sets it to the maximum value
   // until the handshake sets the timeout.
   std::chrono::milliseconds peerIdleTimeout{kMaxIdleTimeout};

quic/state/TransportSettings.h

@@ -198,6 +198,7 @@ struct TransportSettings {
       kDefaultRxPacketsBeforeAckInitThreshold};
   uint16_t rxPacketsBeforeAckBeforeInit{kDefaultRxPacketsBeforeAckBeforeInit};
   uint16_t rxPacketsBeforeAckAfterInit{kDefaultRxPacketsBeforeAckAfterInit};
+  folly::Optional<std::chrono::microseconds> minAckDelay;
   // Limits the amount of data that should be buffered in a QuicSocket.
   // If the amount of data in the buffer equals or exceeds this amount, then
   // the callback registered through notifyPendingWriteOnConnection() will

quic/state/test/QuicStateFunctionsTest.cpp

@@ -220,6 +220,38 @@ TEST_P(UpdateAckStateTest, TestUpdateAckStateFrequency) {
   EXPECT_FALSE(conn.pendingEvents.scheduleAckTimeout);
 }
 
+TEST_P(UpdateAckStateTest, TestUpdateAckStateFrequencyFromTolerance) {
+  QuicServerConnectionState conn(
+      FizzServerQuicHandshakeContext::Builder().build());
+  PacketNum nextPacketNum = 1;
+  auto& ackState = getAckState(conn, GetParam());
+  ackState.largestReceivedPacketNum = nextPacketNum - 1;
+  ackState.tolerance = 2;
+  for (nextPacketNum; nextPacketNum <= 10; nextPacketNum++) {
+    updateAckState(conn, GetParam(), nextPacketNum, true, false, Clock::now());
+    if (nextPacketNum % 2 == 0) {
+      EXPECT_TRUE(ackState.needsToSendAckImmediately);
+      EXPECT_FALSE(conn.pendingEvents.scheduleAckTimeout);
+    } else {
+      EXPECT_FALSE(ackState.needsToSendAckImmediately);
+      EXPECT_TRUE(conn.pendingEvents.scheduleAckTimeout);
+    }
+    EXPECT_EQ(ackState.numRxPacketsRecvd, nextPacketNum % 2);
+  }
+  ackState.tolerance = 10;
+  for (nextPacketNum; nextPacketNum <= 40; nextPacketNum++) {
+    updateAckState(conn, GetParam(), nextPacketNum, true, false, Clock::now());
+    if (nextPacketNum % 10 == 0) {
+      EXPECT_TRUE(ackState.needsToSendAckImmediately);
+      EXPECT_FALSE(conn.pendingEvents.scheduleAckTimeout);
+    } else {
+      EXPECT_FALSE(ackState.needsToSendAckImmediately);
+      EXPECT_TRUE(conn.pendingEvents.scheduleAckTimeout);
+    }
+    EXPECT_EQ(ackState.numRxPacketsRecvd, nextPacketNum % 10);
+  }
+}
+
 TEST_F(UpdateAckStateTest, UpdateAckStateOnAckTimeout) {
   QuicConnectionStateBase conn(QuicNodeType::Client);
   auto& initialAckState = getAckState(conn, PacketNumberSpace::Initial);
@@ -352,6 +384,16 @@ TEST_P(UpdateAckStateTest, UpdateAckSendStateOnRecvPacketsRxOutOfOrder) {
   EXPECT_FALSE(verifyToScheduleAckTimeout(conn));
 }
 
+TEST_P(UpdateAckStateTest, UpdateAckSendStateOnRecvPacketsRxOutOfOrderIgnore) {
+  // Retransmittable & out of order: don't ack immediately if ignoring order.
+  QuicConnectionStateBase conn(QuicNodeType::Client);
+  auto& ackState = getAckState(conn, GetParam());
+  ackState.ignoreReorder = true;
+  updateAckSendStateOnRecvPacket(conn, ackState, true, true, false);
+  EXPECT_FALSE(verifyToAckImmediately(conn, ackState));
+  EXPECT_TRUE(verifyToScheduleAckTimeout(conn));
+}
+
 TEST_P(UpdateAckStateTest, UpdateAckSendStateOnRecvPacketsNonRxOutOfOrder) {
   // Non-retransmittable & out of order: not ack immediately
   QuicConnectionStateBase conn(QuicNodeType::Client);