diff --git a/quic/QuicConstants.h b/quic/QuicConstants.h index d95527f6b..d12cfa13d 100644 --- a/quic/QuicConstants.h +++ b/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 diff --git a/quic/api/test/QuicTransportFunctionsTest.cpp b/quic/api/test/QuicTransportFunctionsTest.cpp index 60178e282..bc3a3e9d6 100644 --- a/quic/api/test/QuicTransportFunctionsTest.cpp +++ b/quic/api/test/QuicTransportFunctionsTest.cpp @@ -87,6 +87,17 @@ void writeCryptoDataProbesToSocketForTest( version); } +RegularQuicWritePacket stripPaddingFrames(RegularQuicWritePacket packet) { + SmallVec 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()); diff --git a/quic/api/test/QuicTransportTest.cpp b/quic/api/test/QuicTransportTest.cpp index 3137be937..df2aebeab 100644 --- a/quic/api/test/QuicTransportTest.cpp +++ b/quic/api/test/QuicTransportTest.cpp @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -122,6 +123,17 @@ class QuicTransportTest : public Test { std::shared_ptr transport_; }; +RegularQuicWritePacket stripPaddingFrames(RegularQuicWritePacket packet) { + SmallVec 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& 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); - EXPECT_EQ(1, packet2.packet.frames.size()); - auto& frame2 = packet2.packet.frames.front(); + auto& outstandingPacket2 = + *getFirstOutstandingPacket(conn, PacketNumberSpace::AppData); + 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); - EXPECT_EQ(2, packet3.packet.frames.size()); - auto& frame3 = packet3.packet.frames.front(); - auto& frame4 = packet3.packet.frames.back(); + auto& outstandingPacket3 = + *getFirstOutstandingPacket(conn, PacketNumberSpace::AppData); + auto packet3 = stripPaddingFrames(outstandingPacket3.packet); + 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); diff --git a/quic/loss/test/QuicLossFunctionsTest.cpp b/quic/loss/test/QuicLossFunctionsTest.cpp index de5d7217d..99d42c363 100644 --- a/quic/loss/test/QuicLossFunctionsTest.cpp +++ b/quic/loss/test/QuicLossFunctionsTest.cpp @@ -304,6 +304,17 @@ PacketNum QuicLossFunctionsTest::sendPacket( return conn.lossState.largestSent.value(); } +RegularQuicWritePacket stripPaddingFrames(RegularQuicWritePacket packet) { + SmallVec 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 : - getLastOutstandingPacket(*conn, PacketNumberSpace::AppData) - ->packet.frames) { + auto strippedPacket = stripPaddingFrames( + getLastOutstandingPacket(*conn, PacketNumberSpace::AppData)->packet); + for (const auto& frame : strippedPacket.frames) { switch (frame.type()) { case QuicWriteFrame::Type::WriteStreamFrame: streamDataCounter++; diff --git a/quic/state/TransportSettings.h b/quic/state/TransportSettings.h index f2f09dc43..2d2e11d58 100644 --- a/quic/state/TransportSettings.h +++ b/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