Add IMMEDIATE_ACK frame

Summary: Add the new IMMEDIATE_ACK frame from the ack frequency draft.

Reviewed By: mjoras

Differential Revision: D38523438

fbshipit-source-id: 79f4a26160ccf4333fb79897ab4ace2ed262fa01

quic/QuicConstants.h

@@ -194,6 +194,7 @@ enum class FrameType : uint64_t {
   DATAGRAM = 0x30,
   DATAGRAM_LEN = 0x31,
   KNOB = 0x1550,
+  IMMEDIATE_ACK = 0xAC,
   ACK_FREQUENCY = 0xAF,
   // Stream groups.
   GROUP_STREAM = 0x32,

quic/api/QuicPacketScheduler.cpp

@@ -186,6 +186,11 @@ FrameScheduler::Builder& FrameScheduler::Builder::datagramFrames() {
   return *this;
 }
 
+FrameScheduler::Builder& FrameScheduler::Builder::immediateAckFrames() {
+  immediateAckFrameScheduler_ = true;
+  return *this;
+}
+
 FrameScheduler FrameScheduler::Builder::build() && {
   FrameScheduler scheduler(name_, conn_);
   if (streamFrameScheduler_) {
@@ -217,6 +222,10 @@ FrameScheduler FrameScheduler::Builder::build() && {
   if (datagramFrameScheduler_) {
     scheduler.datagramFrameScheduler_.emplace(DatagramFrameScheduler(conn_));
   }
+  if (immediateAckFrameScheduler_) {
+    scheduler.immediateAckFrameScheduler_.emplace(
+        ImmediateAckFrameScheduler(conn_));
+  }
   return scheduler;
 }
 
@@ -259,6 +268,13 @@ SchedulingResult FrameScheduler::scheduleFramesForPacket(
       ackScheduler_->writeNextAcks(builder);
     }
   }
+  // Immediate ACK frames are subject to congestion control but should be sent
+  // before other frames to maximize their chance of being included in the
+  // packet since they are time sensitive
+  if (immediateAckFrameScheduler_ &&
+      immediateAckFrameScheduler_->hasPendingImmediateAckFrame()) {
+    immediateAckFrameScheduler_->writeImmediateAckFrame(wrapper);
+  }
   if (windowUpdateScheduler_ &&
       windowUpdateScheduler_->hasPendingWindowUpdates()) {
     windowUpdateScheduler_->writeWindowUpdates(wrapper);
@@ -317,7 +333,7 @@ void FrameScheduler::writeNextAcks(PacketBuilderInterface& builder) {
 }
 
 bool FrameScheduler::hasData() const {
-  return (hasPendingAcks()) || hasImmediateData();
+  return hasPendingAcks() || hasImmediateData();
 }
 
 bool FrameScheduler::hasPendingAcks() const {
@@ -335,7 +351,9 @@ bool FrameScheduler::hasImmediateData() const {
        simpleFrameScheduler_->hasPendingSimpleFrames()) ||
       (pingFrameScheduler_ && pingFrameScheduler_->hasPingFrame()) ||
       (datagramFrameScheduler_ &&
-       datagramFrameScheduler_->hasPendingDatagramFrames());
+       datagramFrameScheduler_->hasPendingDatagramFrames()) ||
+      (immediateAckFrameScheduler_ &&
+       immediateAckFrameScheduler_->hasPendingImmediateAckFrame());
 }
 
 folly::StringPiece FrameScheduler::name() const {
@@ -779,6 +797,19 @@ bool CryptoStreamScheduler::hasData() const {
       !cryptoStream_.lossBuffer.empty();
 }
 
+ImmediateAckFrameScheduler::ImmediateAckFrameScheduler(
+    const QuicConnectionStateBase& conn)
+    : conn_(conn) {}
+
+bool ImmediateAckFrameScheduler::hasPendingImmediateAckFrame() const {
+  return conn_.pendingEvents.requestImmediateAck;
+}
+
+bool ImmediateAckFrameScheduler::writeImmediateAckFrame(
+    PacketBuilderInterface& builder) {
+  return 0 != writeFrame(ImmediateAckFrame(), builder);
+}
+
 CloningScheduler::CloningScheduler(
     FrameScheduler& scheduler,
     QuicConnectionStateBase& conn,

quic/api/QuicPacketScheduler.h

@@ -242,6 +242,18 @@ class CryptoStreamScheduler {
   const QuicCryptoStream& cryptoStream_;
 };
 
+class ImmediateAckFrameScheduler {
+ public:
+  explicit ImmediateAckFrameScheduler(const QuicConnectionStateBase& conn);
+
+  [[nodiscard]] bool hasPendingImmediateAckFrame() const;
+
+  bool writeImmediateAckFrame(PacketBuilderInterface& builder);
+
+ private:
+  const QuicConnectionStateBase& conn_;
+};
+
 class FrameScheduler : public QuicPacketScheduler {
  public:
   ~FrameScheduler() override = default;
@@ -262,6 +274,7 @@ class FrameScheduler : public QuicPacketScheduler {
     Builder& simpleFrames();
     Builder& pingFrames();
     Builder& datagramFrames();
+    Builder& immediateAckFrames();
 
     FrameScheduler build() &&;
 
@@ -281,6 +294,7 @@ class FrameScheduler : public QuicPacketScheduler {
     bool simpleFrameScheduler_{false};
     bool pingFrameScheduler_{false};
     bool datagramFrameScheduler_{false};
+    bool immediateAckFrameScheduler_{false};
   };
 
   FrameScheduler(folly::StringPiece name, QuicConnectionStateBase& conn);
@@ -313,6 +327,7 @@ class FrameScheduler : public QuicPacketScheduler {
   folly::Optional<SimpleFrameScheduler> simpleFrameScheduler_;
   folly::Optional<PingFrameScheduler> pingFrameScheduler_;
   folly::Optional<DatagramFrameScheduler> datagramFrameScheduler_;
+  folly::Optional<ImmediateAckFrameScheduler> immediateAckFrameScheduler_;
   folly::StringPiece name_;
   QuicConnectionStateBase& conn_;
 };

quic/api/QuicTransportFunctions.cpp

@@ -151,7 +151,8 @@ WriteQuicDataResult writeQuicDataToSocketImpl(
             .simpleFrames()
             .resetFrames()
             .streamFrames()
-            .pingFrames();
+            .pingFrames()
+            .immediateAckFrames();
     if (!exceptCryptoStream) {
       probeSchedulerBuilder.cryptoFrames();
     }
@@ -187,7 +188,8 @@ WriteQuicDataResult writeQuicDataToSocketImpl(
           .blockedFrames()
           .simpleFrames()
           .pingFrames()
-          .datagramFrames();
+          .datagramFrames()
+          .immediateAckFrames();
   if (!exceptCryptoStream) {
     schedulerBuilder.cryptoFrames();
   }
@@ -832,6 +834,12 @@ void updateConnection(
         // do not mark Datagram frames as retransmittable
         break;
       }
+      case QuicWriteFrame::Type::ImmediateAckFrame: {
+        // do not mark immediate acks as retranmittable.
+        // turn off the immediate ack pending event.
+        conn.pendingEvents.requestImmediateAck = false;
+        break;
+      }
       default:
         retransmittable = true;
     }

quic/api/test/QuicPacketSchedulerTest.cpp

@@ -1235,6 +1235,35 @@ TEST_F(QuicPacketSchedulerTest, LargestAckToSend) {
   EXPECT_EQ(folly::none, largestAckToSend(conn.ackStates.appDataAckState));
 }
 
+TEST_F(QuicPacketSchedulerTest, NeedsToSendAckWithoutAcksAvailable) {
+  // This covers the scheduler behavior when an IMMEDIATE_ACK frame is received.
+  QuicClientConnectionState conn(
+      FizzClientQuicHandshakeContext::Builder().build());
+  AckScheduler initialAckScheduler(
+      conn, getAckState(conn, PacketNumberSpace::Initial));
+  AckScheduler handshakeAckScheduler(
+      conn, getAckState(conn, PacketNumberSpace::Handshake));
+  AckScheduler appDataAckScheduler(
+      conn, getAckState(conn, PacketNumberSpace::AppData));
+  EXPECT_FALSE(initialAckScheduler.hasPendingAcks());
+  EXPECT_FALSE(handshakeAckScheduler.hasPendingAcks());
+  EXPECT_FALSE(appDataAckScheduler.hasPendingAcks());
+
+  conn.ackStates.initialAckState.needsToSendAckImmediately = true;
+  conn.ackStates.handshakeAckState.needsToSendAckImmediately = true;
+  conn.ackStates.appDataAckState.needsToSendAckImmediately = true;
+
+  conn.ackStates.initialAckState.acks.insert(0, 100);
+  EXPECT_TRUE(initialAckScheduler.hasPendingAcks());
+
+  conn.ackStates.handshakeAckState.acks.insert(0, 100);
+  conn.ackStates.handshakeAckState.largestAckScheduled = 200;
+  EXPECT_FALSE(handshakeAckScheduler.hasPendingAcks());
+
+  conn.ackStates.handshakeAckState.largestAckScheduled = folly::none;
+  EXPECT_TRUE(handshakeAckScheduler.hasPendingAcks());
+}
+
 TEST_F(QuicPacketSchedulerTest, StreamFrameSchedulerAllFit) {
   QuicClientConnectionState conn(
       FizzClientQuicHandshakeContext::Builder().build());
@@ -2300,6 +2329,80 @@ TEST_F(QuicPacketSchedulerTest, ShortHeaderPaddingMaxPacketLength) {
   EXPECT_EQ(packetLength, conn.udpSendPacketLen);
 }
 
+TEST_F(QuicPacketSchedulerTest, ImmediateAckFrameSchedulerOnRequest) {
+  QuicClientConnectionState conn(
+      FizzClientQuicHandshakeContext::Builder().build());
+  conn.pendingEvents.requestImmediateAck = true;
+  auto connId = getTestConnectionId();
+
+  LongHeader longHeader(
+      LongHeader::Types::Initial,
+      getTestConnectionId(1),
+      connId,
+      getNextPacketNum(conn, PacketNumberSpace::Initial),
+      QuicVersion::MVFST);
+  increaseNextPacketNum(conn, PacketNumberSpace::Initial);
+
+  RegularQuicPacketBuilder builder(
+      conn.udpSendPacketLen,
+      std::move(longHeader),
+      conn.ackStates.initialAckState.largestAckedByPeer.value_or(0));
+
+  FrameScheduler immediateAckOnlyScheduler =
+      std::move(
+          FrameScheduler::Builder(
+              conn,
+              EncryptionLevel::Initial,
+              LongHeader::typeToPacketNumberSpace(LongHeader::Types::Initial),
+              "ImmediateAckOnlyScheduler")
+              .immediateAckFrames())
+          .build();
+
+  auto result = immediateAckOnlyScheduler.scheduleFramesForPacket(
+      std::move(builder), conn.udpSendPacketLen);
+  auto packetLength = result.packet->header->computeChainDataLength() +
+      result.packet->body->computeChainDataLength();
+  EXPECT_EQ(conn.udpSendPacketLen, packetLength);
+}
+
+TEST_F(QuicPacketSchedulerTest, ImmediateAckFrameSchedulerNotRequested) {
+  QuicClientConnectionState conn(
+      FizzClientQuicHandshakeContext::Builder().build());
+  conn.pendingEvents.requestImmediateAck = false;
+  auto connId = getTestConnectionId();
+
+  LongHeader longHeader(
+      LongHeader::Types::Initial,
+      getTestConnectionId(1),
+      connId,
+      getNextPacketNum(conn, PacketNumberSpace::Initial),
+      QuicVersion::MVFST);
+  increaseNextPacketNum(conn, PacketNumberSpace::Initial);
+
+  RegularQuicPacketBuilder builder(
+      conn.udpSendPacketLen,
+      std::move(longHeader),
+      conn.ackStates.initialAckState.largestAckedByPeer.value_or(0));
+
+  FrameScheduler immediateAckOnlyScheduler =
+      std::move(
+          FrameScheduler::Builder(
+              conn,
+              EncryptionLevel::Initial,
+              LongHeader::typeToPacketNumberSpace(LongHeader::Types::Initial),
+              "ImmediateAckOnlyScheduler")
+              .immediateAckFrames())
+          .build();
+
+  auto result = immediateAckOnlyScheduler.scheduleFramesForPacket(
+      std::move(builder), conn.udpSendPacketLen);
+  auto packetLength = result.packet->header->computeChainDataLength() +
+      result.packet->body->computeChainDataLength();
+  // The immediate ACK scheduler was not triggered. This packet has no frames
+  // and it shouldn't get padded.
+  EXPECT_LT(packetLength, conn.udpSendPacketLen);
+}
+
 INSTANTIATE_TEST_SUITE_P(
     QuicPacketSchedulerTests,
     QuicPacketSchedulerTest,

quic/client/QuicClientTransport.cpp

@@ -614,6 +614,21 @@ void QuicClientTransport::processPacketData(
         handleDatagram(*conn_, frame, receiveTimePoint);
         break;
       }
+      case QuicFrame::Type::ImmediateAckFrame: {
+        if (!conn_->transportSettings.minAckDelay.hasValue()) {
+          // We do not accept IMMEDIATE_ACK frames. This is a protocol
+          // violation.
+          throw QuicTransportException(
+              "Received IMMEDIATE_ACK frame without announcing min_ack_delay",
+              TransportErrorCode::PROTOCOL_VIOLATION,
+              FrameType::IMMEDIATE_ACK);
+        }
+        // Send an ACK from any packet number space.
+        conn_->ackStates.initialAckState.needsToSendAckImmediately = true;
+        conn_->ackStates.handshakeAckState.needsToSendAckImmediately = true;
+        conn_->ackStates.appDataAckState.needsToSendAckImmediately = true;
+        break;
+      }
       default:
         break;
     }

quic/codec/Decode.cpp

@@ -136,6 +136,10 @@ AckFrequencyFrame decodeAckFrequencyFrame(folly::io::Cursor& cursor) {
   return frame;
 }
 
+ImmediateAckFrame decodeImmediateAckFrame(folly::io::Cursor&) {
+  return ImmediateAckFrame();
+}
+
 ReadAckFrame decodeAckFrame(
     folly::io::Cursor& cursor,
     const PacketHeader& header,
@@ -836,6 +840,8 @@ QuicFrame parseFrame(
         return QuicFrame(decodeKnobFrame(cursor));
       case FrameType::ACK_FREQUENCY:
         return QuicFrame(decodeAckFrequencyFrame(cursor));
+      case FrameType::IMMEDIATE_ACK:
+        return QuicFrame(decodeImmediateAckFrame(cursor));
     }
   } catch (const std::exception& e) {
     error = true;

quic/codec/Decode.h

@@ -93,6 +93,8 @@ KnobFrame decodeKnobFrame(folly::io::Cursor& cursor);
 
 AckFrequencyFrame decodeAckFrequencyFrame(folly::io::Cursor& cursor);
 
+ImmediateAckFrame decodeImmediateAckFrame(folly::io::Cursor& cursor);
+
 DataBlockedFrame decodeDataBlockedFrame(folly::io::Cursor& cursor);
 
 StreamDataBlockedFrame decodeStreamDataBlockedFrame(folly::io::Cursor& cursor);

quic/codec/QuicWriteCodec.cpp

@@ -726,6 +726,17 @@ size_t writeFrame(QuicWriteFrame&& frame, PacketBuilderInterface& builder) {
       // no space left in packet
       return size_t(0);
     }
+    case QuicWriteFrame::Type::ImmediateAckFrame: {
+      const ImmediateAckFrame& immediateAckFrame = *frame.asImmediateAckFrame();
+      QuicInteger intFrameType(static_cast<uint8_t>(FrameType::IMMEDIATE_ACK));
+      if (packetSpaceCheck(spaceLeft, intFrameType.getSize())) {
+        builder.write(intFrameType);
+        builder.appendFrame(immediateAckFrame);
+        return intFrameType.getSize();
+      }
+      // no space left in packet
+      return size_t(0);
+    }
     default: {
       // TODO add support for: RETIRE_CONNECTION_ID and NEW_TOKEN frames
       auto errorStr = folly::to<std::string>(

quic/codec/Types.cpp

@@ -435,6 +435,8 @@ std::string toString(FrameType frame) {
       return "KNOB";
     case FrameType::ACK_FREQUENCY:
       return "ACK_FREQUENCY";
+    case FrameType::IMMEDIATE_ACK:
+      return "IMMEDIATE_ACK";
     case FrameType::GROUP_STREAM:
     case FrameType::GROUP_STREAM_FIN:
     case FrameType::GROUP_STREAM_LEN:

quic/codec/Types.h

@@ -125,6 +125,14 @@ struct AckFrequencyFrame {
   }
 };
 
+struct ImmediateAckFrame {
+  ImmediateAckFrame() = default;
+
+  bool operator==(const ImmediateAckFrame& /*rhs*/) const {
+    return true;
+  }
+};
+
 /**
  * AckBlock represents a series of continuous packet sequences from
  * [startPacket, endPacket]
@@ -760,7 +768,8 @@ DECLARE_VARIANT_TYPE(QuicSimpleFrame, QUIC_SIMPLE_FRAME)
   F(QuicSimpleFrame, __VA_ARGS__)        \
   F(PingFrame, __VA_ARGS__)              \
   F(NoopFrame, __VA_ARGS__)              \
-  F(DatagramFrame, __VA_ARGS__)
+  F(DatagramFrame, __VA_ARGS__)          \
+  F(ImmediateAckFrame, __VA_ARGS__)
 
 DECLARE_VARIANT_TYPE(QuicFrame, QUIC_FRAME)
 
@@ -779,7 +788,8 @@ DECLARE_VARIANT_TYPE(QuicFrame, QUIC_FRAME)
   F(QuicSimpleFrame, __VA_ARGS__)        \
   F(PingFrame, __VA_ARGS__)              \
   F(NoopFrame, __VA_ARGS__)              \
-  F(DatagramFrame, __VA_ARGS__)
+  F(DatagramFrame, __VA_ARGS__)          \
+  F(ImmediateAckFrame, __VA_ARGS__)
 
 // Types of frames which are written.
 DECLARE_VARIANT_TYPE(QuicWriteFrame, QUIC_WRITE_FRAME)

quic/logging/BaseQLogger.cpp

@@ -193,6 +193,10 @@ std::unique_ptr<QLogPacketEvent> BaseQLogger::createPacketEvent(
             std::make_unique<quic::DatagramFrameLog>(frame.length));
         break;
       }
+      case QuicFrame::Type::ImmediateAckFrame: {
+        event->frames.push_back(std::make_unique<quic::ImmediateAckFrameLog>());
+        break;
+      }
     }
   }
   if (numPaddingFrames > 0) {
@@ -299,7 +303,12 @@ std::unique_ptr<QLogPacketEvent> BaseQLogger::createPacketEvent(
         // TODO
         break;
       }
-      default: {
+      case QuicWriteFrame::Type::ImmediateAckFrame: {
+        event->frames.push_back(std::make_unique<quic::ImmediateAckFrameLog>());
+        break;
+      }
+      case QuicWriteFrame::Type::PingFrame: {
+        event->frames.push_back(std::make_unique<quic::PingFrameLog>());
         break;
       }
     }

quic/logging/QLoggerConstants.cpp

@@ -79,6 +79,8 @@ folly::StringPiece toQlogString(FrameType frame) {
       return "knob";
     case FrameType::ACK_FREQUENCY:
       return "ack_frequency";
+    case FrameType::IMMEDIATE_ACK:
+      return "immediate_ack";
     case FrameType::GROUP_STREAM:
     case FrameType::GROUP_STREAM_FIN:
     case FrameType::GROUP_STREAM_LEN:

quic/logging/QLoggerTypes.cpp

@@ -127,6 +127,12 @@ folly::dynamic AckFrequencyFrameLog::toDynamic() const {
   return d;
 }
 
+folly::dynamic ImmediateAckFrameLog::toDynamic() const {
+  folly::dynamic d = folly::dynamic::object();
+  d["frame_type"] = toQlogString(FrameType::IMMEDIATE_ACK);
+  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

@@ -172,6 +172,13 @@ class AckFrequencyFrameLog : public QLogFrame {
   FOLLY_NODISCARD folly::dynamic toDynamic() const override;
 };
 
+class ImmediateAckFrameLog : public QLogFrame {
+ public:
+  ImmediateAckFrameLog() = default;
+  ~ImmediateAckFrameLog() override = default;
+  FOLLY_NODISCARD folly::dynamic toDynamic() const override;
+};
+
 class StreamDataBlockedFrameLog : public QLogFrame {
  public:
   StreamId streamId;

quic/server/state/ServerStateMachine.cpp

@@ -121,6 +121,7 @@ bool isUnprotectedPacketFrameInvalid(const QuicFrame& quicFrame) {
     case QuicFrame::Type::ReadNewTokenFrame:
     case QuicFrame::Type::DatagramFrame:
     case QuicFrame::Type::NoopFrame:
+    case QuicFrame::Type::ImmediateAckFrame:
     case QuicFrame::Type::QuicSimpleFrame:
       return true;
   }
@@ -148,6 +149,7 @@ bool isZeroRttPacketFrameInvalid(const QuicFrame& quicFrame) {
     case QuicFrame::Type::ReadAckFrame:
     case QuicFrame::Type::ReadCryptoFrame:
     case QuicFrame::Type::ReadNewTokenFrame:
+    case QuicFrame::Type::ImmediateAckFrame:
       return true;
     case QuicFrame::Type::PingFrame:
     case QuicFrame::Type::ConnectionCloseFrame:
@@ -1264,6 +1266,21 @@ void onServerReadDataFromOpen(
           handleDatagram(conn, frame, readData.networkData.receiveTimePoint);
           break;
         }
+        case QuicFrame::Type::ImmediateAckFrame: {
+          if (!conn.transportSettings.minAckDelay.hasValue()) {
+            // We do not accept IMMEDIATE_ACK frames. This is a protocol
+            // violation.
+            throw QuicTransportException(
+                "Received IMMEDIATE_ACK frame without announcing min_ack_delay",
+                TransportErrorCode::PROTOCOL_VIOLATION,
+                FrameType::IMMEDIATE_ACK);
+          }
+          // Send an ACK from any packet number space.
+          conn.ackStates.initialAckState.needsToSendAckImmediately = true;
+          conn.ackStates.handshakeAckState.needsToSendAckImmediately = true;
+          conn.ackStates.appDataAckState.needsToSendAckImmediately = true;
+          break;
+        }
         default: {
           break;
         }

quic/server/test/QuicServerTransportTest.cpp

@@ -2858,6 +2858,65 @@ TEST_F(QuicServerTransportTest, PingIsTreatedAsRetransmittable) {
   EXPECT_TRUE(server->getConn().pendingEvents.scheduleAckTimeout);
 }
 
+TEST_F(QuicServerTransportTest, ImmediateAckValid) {
+  // Verify that an incoming IMMEDIATE_ACK frame flags all
+  // packet number spaces to generate ACKs immediately.
+  ImmediateAckFrame immediateAckFrame;
+  // We support receiving IMMEDIATE_ACK
+  server->getNonConstConn().transportSettings.minAckDelay = 1ms;
+
+  auto packetNum = clientNextAppDataPacketNum++;
+  ShortHeader header(
+      ProtectionType::KeyPhaseZero,
+      *server->getConn().serverConnectionId,
+      packetNum);
+  RegularQuicPacketBuilder builder(
+      server->getConn().udpSendPacketLen,
+      std::move(header),
+      0 /* largestAcked */);
+  builder.encodePacketHeader();
+  writeFrame(immediateAckFrame, builder);
+  auto packet = std::move(builder).buildPacket();
+  ASSERT_NO_THROW(deliverData(packetToBuf(packet)));
+  // An ACK has been scheduled for AppData number space.
+  EXPECT_TRUE(server->getConn()
+                  .ackStates.appDataAckState.largestAckScheduled.hasValue());
+  EXPECT_EQ(
+      server->getConn().ackStates.appDataAckState.largestAckScheduled.value_or(
+          packetNum + 1),
+      packetNum);
+}
+
+TEST_F(QuicServerTransportTest, ImmediateAckProtocolViolation) {
+  // Verify that an incoming IMMEDIATE_ACK frame flags all
+  // packet number spaces to generate ACKs immediately.
+  ImmediateAckFrame immediateAckFrame;
+  // We do not support IMMEDIATE_ACK frames
+  server->getNonConstConn().transportSettings.minAckDelay.clear();
+
+  auto packetNum = clientNextAppDataPacketNum++;
+  ShortHeader header(
+      ProtectionType::KeyPhaseZero,
+      *server->getConn().serverConnectionId,
+      packetNum);
+  RegularQuicPacketBuilder builder(
+      server->getConn().udpSendPacketLen,
+      std::move(header),
+      0 /* largestAcked */);
+  builder.encodePacketHeader();
+  writeFrame(immediateAckFrame, builder);
+  auto packet = std::move(builder).buildPacket();
+  // This should throw a protocol violation error
+  ASSERT_THROW(deliverData(packetToBuf(packet)), std::runtime_error);
+  // Verify that none of the ack states have changed
+  EXPECT_FALSE(
+      server->getConn().ackStates.initialAckState.needsToSendAckImmediately);
+  EXPECT_FALSE(
+      server->getConn().ackStates.handshakeAckState.needsToSendAckImmediately);
+  EXPECT_FALSE(
+      server->getConn().ackStates.appDataAckState.needsToSendAckImmediately);
+}
+
 TEST_F(QuicServerTransportTest, ReceiveDatagramFrameAndDiscard) {
   ShortHeader header(
       ProtectionType::KeyPhaseZero,

quic/state/AckStates.h

@@ -36,8 +36,10 @@ struct AckState {
   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
+  // to true in either of the following cases:
+  // - we got packets with retransmittable data and haven't sent the
   // ack for the first time.
+  // - the peer has requested it through an immediate ack frame.
   bool needsToSendAckImmediately{false};
   // Count of oustanding packets received with retransmittable data.
   uint8_t numRxPacketsRecvd{0};

quic/state/SimpleFrameFunctions.cpp

@@ -250,7 +250,12 @@ bool updateSimpleFrameOnPacketReceived(
     }
     case QuicSimpleFrame::Type::AckFrequencyFrame: {
       if (!conn.transportSettings.minAckDelay.hasValue()) {
-        return true;
+        // We do not accept ACK_FREQUENCY frames. This is a protocol
+        // violation.
+        throw QuicTransportException(
+            "Received ACK_FREQUENCY frame without announcing min_ack_delay",
+            TransportErrorCode::PROTOCOL_VIOLATION,
+            FrameType::ACK_FREQUENCY);
       }
       const auto ackFrequencyFrame = frame.asAckFrequencyFrame();
       auto& ackState = conn.ackStates.appDataAckState;

quic/state/StateData.h

@@ -495,6 +495,9 @@ struct QuicConnectionStateBase : public folly::DelayedDestruction {
 
     // Do we need to send data blocked frame when connection is blocked.
     bool sendDataBlocked{false};
+
+    // Send an immediate ack frame (requesting an ack)
+    bool requestImmediateAck{false};
   };
 
   PendingEvents pendingEvents;
@@ -549,7 +552,8 @@ 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.
+  // The value of the peer's min_ack_delay, for creating ACK_FREQUENCY and
+  // IMMEDIATE_ACK frames.
   folly::Optional<std::chrono::microseconds> peerMinAckDelay;
 
   // Idle timeout advertised by the peer. Initially sets it to the maximum value

quic/state/TransportSettings.h

@@ -216,6 +216,9 @@ struct TransportSettings {
       kDefaultRxPacketsBeforeAckInitThreshold};
   uint16_t rxPacketsBeforeAckBeforeInit{kDefaultRxPacketsBeforeAckBeforeInit};
   uint16_t rxPacketsBeforeAckAfterInit{kDefaultRxPacketsBeforeAckAfterInit};
+  // The minimum amount of time in microseconds by which an ack can be delayed
+  // Setting a value here also indicates to the peer that it can send
+  // ACK_FREQUENCY and IMMEDIATE_ACK frames
   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