Ship short header padding

Summary:
QE result post: https://fb.workplace.com/groups/646964542156536/permalink/1989360141250296/

We are going ahead with shipping this, with a default value of 32 for paddingModulo.

Leaving the transport knob settings still in place in case vips want to selectively disable the feature (not sure if that is the proper way to config that but I think so it is?)

Reviewed By: mjoras

Differential Revision: D34596608

fbshipit-source-id: 5603bb391113c29830f43a67e73c0f50154dcca1

quic/QuicConstants.h

@@ -652,4 +652,6 @@ enum class DataPathType : uint8_t {
 using PriorityLevel = uint8_t;
 constexpr uint8_t kDefaultMaxPriority = 7;
 
+constexpr size_t kShortHeaderPaddingModulo = 32;
+
 } // namespace quic

quic/api/test/QuicTransportFunctionsTest.cpp

@@ -87,6 +87,17 @@ void writeCryptoDataProbesToSocketForTest(
       version);
 }
 
+RegularQuicWritePacket stripPaddingFrames(RegularQuicWritePacket packet) {
+  SmallVec<QuicWriteFrame, 4, uint16_t> trimmedFrames{};
+  for (auto frame : packet.frames) {
+    if (!frame.asPaddingFrame()) {
+      trimmedFrames.push_back(frame);
+    }
+  }
+  packet.frames = trimmedFrames;
+  return packet;
+}
+
 auto buildEmptyPacket(
     QuicServerConnectionState& conn,
     PacketNumberSpace pnSpace,
@@ -4018,7 +4029,8 @@ TEST_F(QuicTransportFunctionsTest, ProbeWriteNewFunctionalFrames) {
       getVersion(*conn),
       1 /* limit to 1 packet */);
   EXPECT_EQ(2, conn->outstandings.packets.size());
-  EXPECT_EQ(1, conn->outstandings.packets[1].packet.frames.size());
+  auto packet = stripPaddingFrames(conn->outstandings.packets[1].packet);
+  EXPECT_EQ(1, packet.frames.size());
   EXPECT_EQ(
       QuicWriteFrame::Type::MaxDataFrame,
       conn->outstandings.packets[1].packet.frames[0].type());

quic/api/test/QuicTransportTest.cpp

@@ -11,6 +11,7 @@
 #include <folly/Random.h>
 #include <folly/io/Cursor.h>
 #include <folly/io/async/test/MockAsyncUDPSocket.h>
+#include <quic/QuicConstants.h>
 #include <quic/api/QuicTransportBase.h>
 #include <quic/api/QuicTransportFunctions.h>
 #include <quic/api/test/Mocks.h>
@@ -122,6 +123,17 @@ class QuicTransportTest : public Test {
   std::shared_ptr<TestQuicTransport> transport_;
 };
 
+RegularQuicWritePacket stripPaddingFrames(RegularQuicWritePacket packet) {
+  SmallVec<QuicWriteFrame, 4, uint16_t> trimmedFrames{};
+  for (auto frame : packet.frames) {
+    if (!frame.asPaddingFrame()) {
+      trimmedFrames.push_back(frame);
+    }
+  }
+  packet.frames = trimmedFrames;
+  return packet;
+}
+
 size_t bufLength(
     const SocketAddress&,
     const std::unique_ptr<folly::IOBuf>& buf) {
@@ -1704,7 +1716,7 @@ TEST_F(QuicTransportTest, StopSending) {
       getLastOutstandingPacket(
           transport_->getConnectionState(), PacketNumberSpace::AppData)
           ->packet;
-  EXPECT_EQ(1, packet.frames.size());
+  EXPECT_EQ(1, stripPaddingFrames(packet).frames.size());
   bool foundStopSending = false;
   for (auto& frame : packet.frames) {
     const QuicSimpleFrame* simpleFrame = frame.asQuicSimpleFrame();
@@ -1734,7 +1746,7 @@ TEST_F(QuicTransportTest, StopSendingReadCallbackDefault) {
       getLastOutstandingPacket(
           transport_->getConnectionState(), PacketNumberSpace::AppData)
           ->packet;
-  EXPECT_EQ(1, packet.frames.size());
+  EXPECT_EQ(1, stripPaddingFrames(packet).frames.size());
   bool foundStopSending = false;
   for (auto& frame : packet.frames) {
     const QuicSimpleFrame* simpleFrame = frame.asQuicSimpleFrame();
@@ -1765,7 +1777,7 @@ TEST_F(QuicTransportTest, StopSendingReadCallback) {
       getLastOutstandingPacket(
           transport_->getConnectionState(), PacketNumberSpace::AppData)
           ->packet;
-  EXPECT_EQ(1, packet.frames.size());
+  EXPECT_EQ(1, stripPaddingFrames(packet).frames.size());
   bool foundStopSending = false;
   for (auto& frame : packet.frames) {
     const QuicSimpleFrame* simpleFrame = frame.asQuicSimpleFrame();
@@ -3760,9 +3772,11 @@ TEST_F(QuicTransportTest, WriteStreamFromMiddleOfMap) {
       transport_->getVersion(),
       conn.transportSettings.writeConnectionDataPacketsLimit);
   EXPECT_EQ(1, conn.outstandings.packets.size());
-  auto& packet2 = *getFirstOutstandingPacket(conn, PacketNumberSpace::AppData);
+  auto& outstandingPacket2 =
-  EXPECT_EQ(1, packet2.packet.frames.size());
+      *getFirstOutstandingPacket(conn, PacketNumberSpace::AppData);
-  auto& frame2 = packet2.packet.frames.front();
+  auto packet2 = stripPaddingFrames(outstandingPacket2.packet);
+  EXPECT_EQ(1, packet2.frames.size());
+  auto& frame2 = packet2.frames.front();
   const WriteStreamFrame* streamFrame2 = frame2.asWriteStreamFrame();
   EXPECT_TRUE(streamFrame2);
   EXPECT_EQ(streamFrame2->streamId, s2);
@@ -3782,10 +3796,12 @@ TEST_F(QuicTransportTest, WriteStreamFromMiddleOfMap) {
       transport_->getVersion(),
       conn.transportSettings.writeConnectionDataPacketsLimit);
   EXPECT_EQ(1, conn.outstandings.packets.size());
-  auto& packet3 = *getFirstOutstandingPacket(conn, PacketNumberSpace::AppData);
+  auto& outstandingPacket3 =
-  EXPECT_EQ(2, packet3.packet.frames.size());
+      *getFirstOutstandingPacket(conn, PacketNumberSpace::AppData);
-  auto& frame3 = packet3.packet.frames.front();
+  auto packet3 = stripPaddingFrames(outstandingPacket3.packet);
-  auto& frame4 = packet3.packet.frames.back();
+  EXPECT_EQ(2, packet3.frames.size());
+  auto& frame3 = packet3.frames.front();
+  auto& frame4 = packet3.frames.back();
   const WriteStreamFrame* streamFrame3 = frame3.asWriteStreamFrame();
   EXPECT_TRUE(streamFrame3);
   EXPECT_EQ(streamFrame3->streamId, s2);

quic/loss/test/QuicLossFunctionsTest.cpp

@@ -304,6 +304,17 @@ PacketNum QuicLossFunctionsTest::sendPacket(
   return conn.lossState.largestSent.value();
 }
 
+RegularQuicWritePacket stripPaddingFrames(RegularQuicWritePacket packet) {
+  SmallVec<QuicWriteFrame, 4, uint16_t> trimmedFrames{};
+  for (auto frame : packet.frames) {
+    if (!frame.asPaddingFrame()) {
+      trimmedFrames.push_back(frame);
+    }
+  }
+  packet.frames = trimmedFrames;
+  return packet;
+}
+
 TEST_F(QuicLossFunctionsTest, AllPacketsProcessed) {
   auto conn = createConn();
   EXPECT_CALL(*quicStats_, onPTO()).Times(0);
@@ -1592,9 +1603,9 @@ TEST_F(QuicLossFunctionsTest, TestMarkPacketLossProcessedPacket) {
   ASSERT_TRUE(conn->outstandings.packetEvents.empty());
   uint32_t streamDataCounter = 0, streamWindowUpdateCounter = 0,
            connWindowUpdateCounter = 0;
-  for (const auto& frame :
+  auto strippedPacket = stripPaddingFrames(
-       getLastOutstandingPacket(*conn, PacketNumberSpace::AppData)
+      getLastOutstandingPacket(*conn, PacketNumberSpace::AppData)->packet);
-           ->packet.frames) {
+  for (const auto& frame : strippedPacket.frames) {
     switch (frame.type()) {
       case QuicWriteFrame::Type::WriteStreamFrame:
         streamDataCounter++;

quic/state/TransportSettings.h

@@ -278,7 +278,7 @@ struct TransportSettings {
   // in a packet is always an increment of paddingModulo, hiding the actual
   // packet size from packet analysis.
   // Padding Modulo of 0 turns off padding for short header packets.
-  size_t paddingModulo{0};
+  size_t paddingModulo{kShortHeaderPaddingModulo};
   // Whether to use adaptive loss thresholds for reodering and timeout
   bool useAdaptiveLossThresholds{false};
   // Whether to automatically increase receive conn flow control. The