diff --git a/quic/QuicConstants.h b/quic/QuicConstants.h index 10cbba401..46814624d 100644 --- a/quic/QuicConstants.h +++ b/quic/QuicConstants.h @@ -137,6 +137,8 @@ enum class TransportKnobParamId : uint64_t { MAX_PACING_RATE_KNOB = 0x4444, // Set auto background mode (experimental) AUTO_BACKGROUND_MODE = 0x5555, + // Set short header padding modulo size + SHORT_HEADER_PADDING_KNOB = 0x6666, }; enum class FrameType : uint64_t { diff --git a/quic/api/QuicPacketScheduler.cpp b/quic/api/QuicPacketScheduler.cpp index 2177f1092..5f48eb6b4 100644 --- a/quic/api/QuicPacketScheduler.cpp +++ b/quic/api/QuicPacketScheduler.cpp @@ -188,7 +188,7 @@ FrameScheduler::Builder& FrameScheduler::Builder::datagramFrames() { } FrameScheduler FrameScheduler::Builder::build() && { - FrameScheduler scheduler(std::move(name_)); + FrameScheduler scheduler(name_, conn_); if (streamFrameScheduler_) { scheduler.streamFrameScheduler_.emplace(StreamFrameScheduler(conn_)); } @@ -221,7 +221,10 @@ FrameScheduler FrameScheduler::Builder::build() && { return scheduler; } -FrameScheduler::FrameScheduler(folly::StringPiece name) : name_(name) {} +FrameScheduler::FrameScheduler( + folly::StringPiece name, + QuicConnectionStateBase& conn) + : name_(name), conn_(conn) {} SchedulingResult FrameScheduler::scheduleFramesForPacket( PacketBuilderInterface&& builder, @@ -294,6 +297,17 @@ SchedulingResult FrameScheduler::scheduleFramesForPacket( writeFrame(PaddingFrame(), builder); } } + const ShortHeader* shortHeader = builder.getPacketHeader().asShort(); + if (shortHeader) { + size_t paddingModulo = conn_.transportSettings.paddingModulo; + if (paddingModulo > 0) { + size_t paddingIncrement = wrapper.remainingSpaceInPkt() % paddingModulo; + for (size_t i = 0; i < paddingIncrement; i++) { + writeFrame(PaddingFrame(), builder); + } + QUIC_STATS(conn_.statsCallback, onShortHeaderPadding, paddingIncrement); + } + } } return SchedulingResult(folly::none, std::move(builder).buildPacket()); diff --git a/quic/api/QuicPacketScheduler.h b/quic/api/QuicPacketScheduler.h index 4c9ca144a..53a898209 100644 --- a/quic/api/QuicPacketScheduler.h +++ b/quic/api/QuicPacketScheduler.h @@ -284,7 +284,7 @@ class FrameScheduler : public QuicPacketScheduler { bool datagramFrameScheduler_{false}; }; - explicit FrameScheduler(folly::StringPiece name); + FrameScheduler(folly::StringPiece name, QuicConnectionStateBase& conn); SchedulingResult scheduleFramesForPacket( PacketBuilderInterface&& builder, @@ -309,6 +309,7 @@ class FrameScheduler : public QuicPacketScheduler { folly::Optional pingFrameScheduler_; folly::Optional datagramFrameScheduler_; folly::StringPiece name_; + QuicConnectionStateBase& conn_; }; /** diff --git a/quic/api/test/Mocks.h b/quic/api/test/Mocks.h index c20b81d21..eea1e6d14 100644 --- a/quic/api/test/Mocks.h +++ b/quic/api/test/Mocks.h @@ -25,7 +25,8 @@ class MockFrameScheduler : public FrameScheduler { public: ~MockFrameScheduler() override = default; - MockFrameScheduler() : FrameScheduler("mock") {} + explicit MockFrameScheduler(QuicConnectionStateBase* conn) + : FrameScheduler("mock", *conn) {} // override methods accepting rvalue ref since gmock doesn't support it SchedulingResult scheduleFramesForPacket( diff --git a/quic/api/test/QuicPacketSchedulerTest.cpp b/quic/api/test/QuicPacketSchedulerTest.cpp index bebd21dea..11166a93d 100644 --- a/quic/api/test/QuicPacketSchedulerTest.cpp +++ b/quic/api/test/QuicPacketSchedulerTest.cpp @@ -6,10 +6,9 @@ * */ -#include - #include +#include #include #include #include @@ -517,7 +516,7 @@ TEST_F(QuicPacketSchedulerTest, StreamFrameSchedulerStreamNotExists) { TEST_F(QuicPacketSchedulerTest, NoCloningForDSR) { QuicClientConnectionState conn( FizzClientQuicHandshakeContext::Builder().build()); - FrameScheduler noopScheduler("frame"); + FrameScheduler noopScheduler("frame", conn); ASSERT_FALSE(noopScheduler.hasData()); CloningScheduler cloningScheduler(noopScheduler, conn, "Juice WRLD", 0); EXPECT_FALSE(cloningScheduler.hasData()); @@ -543,7 +542,7 @@ TEST_F(QuicPacketSchedulerTest, NoCloningForDSR) { TEST_F(QuicPacketSchedulerTest, CloningSchedulerTest) { QuicClientConnectionState conn( FizzClientQuicHandshakeContext::Builder().build()); - FrameScheduler noopScheduler("frame"); + FrameScheduler noopScheduler("frame", conn); ASSERT_FALSE(noopScheduler.hasData()); CloningScheduler cloningScheduler(noopScheduler, conn, "CopyCat", 0); EXPECT_FALSE(cloningScheduler.hasData()); @@ -610,7 +609,7 @@ TEST_P(QuicPacketSchedulerTest, NoCloningWithOnlyD6DProbes) { false /* isDSRPacket */); ASSERT_EQ(1, conn.outstandings.packets.size()); EXPECT_TRUE(conn.outstandings.packets.back().metadata.isD6DProbe); - FrameScheduler noopScheduler("noopScheduler"); + FrameScheduler noopScheduler("noopScheduler", conn); CloningScheduler cloningScheduler( noopScheduler, conn, "MarShall Mathers", cipherOverhead); EXPECT_FALSE(cloningScheduler.hasData()); @@ -619,7 +618,7 @@ TEST_P(QuicPacketSchedulerTest, NoCloningWithOnlyD6DProbes) { TEST_F(QuicPacketSchedulerTest, WriteOnlyOutstandingPacketsTest) { QuicClientConnectionState conn( FizzClientQuicHandshakeContext::Builder().build()); - FrameScheduler noopScheduler("frame"); + FrameScheduler noopScheduler("frame", conn); ASSERT_FALSE(noopScheduler.hasData()); CloningScheduler cloningScheduler(noopScheduler, conn, "CopyCat", 0); EXPECT_FALSE(cloningScheduler.hasData()); @@ -688,7 +687,7 @@ TEST_F(QuicPacketSchedulerTest, WriteOnlyOutstandingPacketsTest) { TEST_F(QuicPacketSchedulerTest, DoNotCloneProcessedClonedPacket) { QuicClientConnectionState conn( FizzClientQuicHandshakeContext::Builder().build()); - FrameScheduler noopScheduler("frame"); + FrameScheduler noopScheduler("frame", conn); CloningScheduler cloningScheduler(noopScheduler, conn, "CopyCat", 0); // Add two outstanding packets, but then mark the second one processed by // adding a PacketEvent that's missing from the outstandings.packetEvents set @@ -720,7 +719,7 @@ TEST_F(QuicPacketSchedulerTest, DoNotCloneProcessedClonedPacket) { TEST_F(QuicPacketSchedulerTest, CloneSchedulerHasHandshakeData) { QuicClientConnectionState conn( FizzClientQuicHandshakeContext::Builder().build()); - FrameScheduler noopScheduler("frame"); + FrameScheduler noopScheduler("frame", conn); CloningScheduler cloningScheduler(noopScheduler, conn, "CopyCat", 0); EXPECT_FALSE(cloningScheduler.hasData()); @@ -731,7 +730,7 @@ TEST_F(QuicPacketSchedulerTest, CloneSchedulerHasHandshakeData) { TEST_F(QuicPacketSchedulerTest, CloneSchedulerHasInitialData) { QuicClientConnectionState conn( FizzClientQuicHandshakeContext::Builder().build()); - FrameScheduler noopScheduler("frame"); + FrameScheduler noopScheduler("frame", conn); CloningScheduler cloningScheduler(noopScheduler, conn, "CopyCat", 0); EXPECT_FALSE(cloningScheduler.hasData()); @@ -742,7 +741,7 @@ TEST_F(QuicPacketSchedulerTest, CloneSchedulerHasInitialData) { TEST_F(QuicPacketSchedulerTest, CloneSchedulerHasAppDataData) { QuicClientConnectionState conn( FizzClientQuicHandshakeContext::Builder().build()); - FrameScheduler noopScheduler("frame"); + FrameScheduler noopScheduler("frame", conn); CloningScheduler cloningScheduler(noopScheduler, conn, "CopyCat", 0); EXPECT_FALSE(cloningScheduler.hasData()); @@ -753,7 +752,7 @@ TEST_F(QuicPacketSchedulerTest, CloneSchedulerHasAppDataData) { TEST_F(QuicPacketSchedulerTest, DoNotCloneHandshake) { QuicClientConnectionState conn( FizzClientQuicHandshakeContext::Builder().build()); - FrameScheduler noopScheduler("frame"); + FrameScheduler noopScheduler("frame", conn); CloningScheduler cloningScheduler(noopScheduler, conn, "CopyCat", 0); // Add two outstanding packets, with second one being handshake auto expected = addOutstandingPacket(conn); @@ -781,7 +780,7 @@ TEST_F(QuicPacketSchedulerTest, DoNotCloneHandshake) { TEST_F(QuicPacketSchedulerTest, CloneSchedulerUseNormalSchedulerFirst) { QuicClientConnectionState conn( FizzClientQuicHandshakeContext::Builder().build()); - NiceMock mockScheduler; + NiceMock mockScheduler(&conn); CloningScheduler cloningScheduler(mockScheduler, conn, "Mocker", 0); ShortHeader header( ProtectionType::KeyPhaseOne, @@ -831,7 +830,7 @@ TEST_F(QuicPacketSchedulerTest, CloneWillGenerateNewWindowUpdate) { FizzClientQuicHandshakeContext::Builder().build()); conn.streamManager->setMaxLocalBidirectionalStreams(10); auto stream = conn.streamManager->createNextBidirectionalStream().value(); - FrameScheduler noopScheduler("frame"); + FrameScheduler noopScheduler("frame", conn); CloningScheduler cloningScheduler(noopScheduler, conn, "GiantsShoulder", 0); PacketEvent expectedPacketEvent( PacketNumberSpace::AppData, addOutstandingPacket(conn)); @@ -917,7 +916,7 @@ TEST_F(QuicPacketSchedulerTest, CloningSchedulerWithInplaceBuilder) { bufAccessor.release(std::move(buf)); conn.bufAccessor = &bufAccessor; - FrameScheduler noopScheduler("frame"); + FrameScheduler noopScheduler("frame", conn); ASSERT_FALSE(noopScheduler.hasData()); CloningScheduler cloningScheduler(noopScheduler, conn, "93MillionMiles", 0); auto packetNum = addOutstandingPacket(conn); @@ -999,7 +998,7 @@ TEST_F(QuicPacketSchedulerTest, CloningSchedulerWithInplaceBuilderFullPacket) { buf->clear(); bufAccessor.release(std::move(buf)); - FrameScheduler noopScheduler("noopScheduler"); + FrameScheduler noopScheduler("noopScheduler", conn); ASSERT_FALSE(noopScheduler.hasData()); CloningScheduler cloningScheduler(noopScheduler, conn, "93MillionMiles", 0); EXPECT_TRUE(cloningScheduler.hasData()); @@ -1077,7 +1076,7 @@ TEST_F(QuicPacketSchedulerTest, CloneLargerThanOriginalPacket) { conn.udpSendPacketLen, std::move(cloneHeader), conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0)); - FrameScheduler noopScheduler("noopScheduler"); + FrameScheduler noopScheduler("noopScheduler", conn); CloningScheduler cloningScheduler( noopScheduler, conn, "CopyCat", cipherOverhead); auto cloneResult = cloningScheduler.scheduleFramesForPacket( @@ -1547,7 +1546,7 @@ TEST_F( bufAccessor.release(std::move(buf)); conn.bufAccessor = &bufAccessor; - FrameScheduler noopScheduler("frame"); + FrameScheduler noopScheduler("frame", conn); ASSERT_FALSE(noopScheduler.hasData()); CloningScheduler cloningScheduler(noopScheduler, conn, "Little Hurry", 0); addOutstandingPacket(conn); @@ -1591,7 +1590,7 @@ TEST_F( bufAccessor.release(std::move(buf)); conn.bufAccessor = &bufAccessor; - FrameScheduler noopScheduler("frame"); + FrameScheduler noopScheduler("frame", conn); ASSERT_FALSE(noopScheduler.hasData()); CloningScheduler cloningScheduler(noopScheduler, conn, "HotPot", 0); addOutstandingPacket(conn); @@ -2038,6 +2037,179 @@ TEST_F(QuicPacketSchedulerTest, DatagramFrameWriteWhenRoomAvailable) { ASSERT_EQ(frames.size(), 1); } +TEST_F(QuicPacketSchedulerTest, ShortHeaderPaddingWithSpaceForPadding) { + QuicServerConnectionState conn( + FizzServerQuicHandshakeContext::Builder().build()); + size_t paddingModulo = 16; + conn.transportSettings.paddingModulo = paddingModulo; + // create enough input data to partially fill packet + size_t inputDataLength1 = conn.udpSendPacketLen / 2; + size_t inputDataLength2 = inputDataLength1 + 1; + auto inputData1 = buildRandomInputData(inputDataLength1); + auto inputData2 = buildRandomInputData(inputDataLength2); + + FrameScheduler scheduler = std::move(FrameScheduler::Builder( + conn, + EncryptionLevel::AppData, + PacketNumberSpace::AppData, + "streamScheduler") + .streamFrames()) + .build(); + + ShortHeader shortHeader1( + ProtectionType::KeyPhaseOne, + conn.clientConnectionId.value_or(getTestConnectionId()), + getNextPacketNum(conn, PacketNumberSpace::AppData)); + ShortHeader shortHeader2( + ProtectionType::KeyPhaseOne, + conn.clientConnectionId.value_or(getTestConnectionId()), + getNextPacketNum(conn, PacketNumberSpace::AppData)); + + RegularQuicPacketBuilder builder1( + conn.udpSendPacketLen, + std::move(shortHeader1), + conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0)); + RegularQuicPacketBuilder builder2( + conn.udpSendPacketLen, + std::move(shortHeader2), + conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0)); + + DatagramFrame frame1(inputDataLength1, std::move(inputData1)); + DatagramFrame frame2(inputDataLength2, std::move(inputData2)); + writeFrame(frame1, builder1); + writeFrame(frame2, builder2); + + NiceMock quicStats; + conn.statsCallback = &quicStats; + + EXPECT_CALL(quicStats, onShortHeaderPadding(_)).Times(1); + auto result1 = scheduler.scheduleFramesForPacket( + std::move(builder1), conn.udpSendPacketLen); + EXPECT_CALL(quicStats, onShortHeaderPadding(_)).Times(1); + auto result2 = scheduler.scheduleFramesForPacket( + std::move(builder2), conn.udpSendPacketLen); + + auto headerLength1 = result1.packet->header->computeChainDataLength(); + auto bodyLength1 = result1.packet->body->computeChainDataLength(); + auto packetLength1 = headerLength1 + bodyLength1; + auto expectedPadding1 = + (conn.udpSendPacketLen - (inputDataLength1 + headerLength1)) % + paddingModulo; + + auto headerLength2 = result2.packet->header->computeChainDataLength(); + auto bodyLength2 = result2.packet->body->computeChainDataLength(); + auto packetLength2 = headerLength2 + bodyLength2; + auto expectedPadding2 = + (conn.udpSendPacketLen - (inputDataLength2 + headerLength2)) % + paddingModulo; + + EXPECT_EQ(packetLength1, headerLength1 + inputDataLength1 + expectedPadding1); + EXPECT_EQ(packetLength2, headerLength2 + inputDataLength2 + expectedPadding2); + // ensure two similar size input data get padded up to same length + EXPECT_EQ(packetLength1, packetLength2); +} + +TEST_F(QuicPacketSchedulerTest, ShortHeaderPaddingNearMaxPacketLength) { + QuicServerConnectionState conn( + FizzServerQuicHandshakeContext::Builder().build()); + conn.udpSendPacketLen = 1000; + size_t paddingModulo = 50; + conn.transportSettings.paddingModulo = paddingModulo; + // create enough input data to almost fill packet + size_t inputDataLength = conn.udpSendPacketLen - 20; + auto inputData = buildRandomInputData(inputDataLength); + + FrameScheduler scheduler = std::move(FrameScheduler::Builder( + conn, + EncryptionLevel::AppData, + PacketNumberSpace::AppData, + "streamScheduler") + .streamFrames()) + .build(); + + ShortHeader shortHeader( + ProtectionType::KeyPhaseOne, + conn.clientConnectionId.value_or(getTestConnectionId()), + getNextPacketNum(conn, PacketNumberSpace::AppData)); + + RegularQuicPacketBuilder builder( + conn.udpSendPacketLen, + std::move(shortHeader), + conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0)); + + DatagramFrame frame(inputDataLength, std::move(inputData)); + writeFrame(frame, builder); + + NiceMock quicStats; + conn.statsCallback = &quicStats; + + EXPECT_CALL(quicStats, onShortHeaderPadding(_)).Times(1); + auto result = scheduler.scheduleFramesForPacket( + std::move(builder), conn.udpSendPacketLen); + + auto headerLength = result.packet->header->computeChainDataLength(); + auto bodyLength = result.packet->body->computeChainDataLength(); + + auto packetLength = headerLength + bodyLength; + + auto expectedPadding = + (conn.udpSendPacketLen - (inputDataLength + headerLength)) % + paddingModulo; + + EXPECT_EQ(packetLength, headerLength + inputDataLength + expectedPadding); + EXPECT_EQ(packetLength, conn.udpSendPacketLen); +} + +TEST_F(QuicPacketSchedulerTest, ShortHeaderPaddingMaxPacketLength) { + QuicServerConnectionState conn( + FizzServerQuicHandshakeContext::Builder().build()); + conn.udpSendPacketLen = 1000; + size_t paddingModulo = 50; + conn.transportSettings.paddingModulo = paddingModulo; + + FrameScheduler scheduler = std::move(FrameScheduler::Builder( + conn, + EncryptionLevel::AppData, + PacketNumberSpace::AppData, + "streamScheduler") + .streamFrames()) + .build(); + + ShortHeader shortHeader( + ProtectionType::KeyPhaseOne, + conn.clientConnectionId.value_or(getTestConnectionId()), + getNextPacketNum(conn, PacketNumberSpace::AppData)); + + size_t largestAckedPacketNum = + conn.ackStates.appDataAckState.largestAckedByPeer.value_or(0); + auto packetNumberEncoding = encodePacketNumber( + shortHeader.getPacketSequenceNum(), largestAckedPacketNum); + auto connectionIdSize = shortHeader.getConnectionId().size(); + + RegularQuicPacketBuilder builder( + conn.udpSendPacketLen, std::move(shortHeader), largestAckedPacketNum); + + // create enough input data to fully fill packet + while (builder.remainingSpaceInPkt() > + connectionIdSize + packetNumberEncoding.length + 1) { + writeFrame(PaddingFrame(), builder); + } + + NiceMock quicStats; + conn.statsCallback = &quicStats; + + EXPECT_CALL(quicStats, onShortHeaderPadding(_)).Times(1); + auto result = scheduler.scheduleFramesForPacket( + std::move(builder), conn.udpSendPacketLen); + + auto headerLength = result.packet->header->computeChainDataLength(); + auto bodyLength = result.packet->body->computeChainDataLength(); + + auto packetLength = headerLength + bodyLength; + + EXPECT_EQ(packetLength, conn.udpSendPacketLen); +} + INSTANTIATE_TEST_CASE_P( QuicPacketSchedulerTests, QuicPacketSchedulerTest, diff --git a/quic/samples/echo/LogQuicStats.h b/quic/samples/echo/LogQuicStats.h index edd35c199..60163119b 100644 --- a/quic/samples/echo/LogQuicStats.h +++ b/quic/samples/echo/LogQuicStats.h @@ -230,6 +230,10 @@ class LogQuicStats : public quic::QuicTransportStatsCallback { VLOG(2) << prefix_ << "onDatagramDroppedOnRead"; } + void onShortHeaderPadding(size_t padSize) override { + VLOG(2) << prefix_ << "onShortHeaderPadding size=" << padSize; + } + private: std::string prefix_; }; diff --git a/quic/server/QuicServerTransport.cpp b/quic/server/QuicServerTransport.cpp index 1ba4ad8c7..21b3c2253 100644 --- a/quic/server/QuicServerTransport.cpp +++ b/quic/server/QuicServerTransport.cpp @@ -6,14 +6,13 @@ * */ -#include - #include #include #include #include #include #include +#include #include #include #include @@ -854,6 +853,16 @@ void QuicServerTransport::registerAllTransportKnobParamHandlers() { server_conn->congestionController->type())); } }); + + registerTransportKnobParamHandler( + static_cast(TransportKnobParamId::SHORT_HEADER_PADDING_KNOB), + [](QuicServerTransport* serverTransport, uint64_t val) { + CHECK(serverTransport); + serverTransport->serverConn_->transportSettings.paddingModulo = val; + VLOG(3) << fmt::format( + "SHORT_HEADER_PADDING_KNOB KnobParam received, setting paddingModulo={}", + val); + }); } QuicConnectionStats QuicServerTransport::getConnectionsStats() const { diff --git a/quic/state/QuicTransportStatsCallback.h b/quic/state/QuicTransportStatsCallback.h index f004e64f2..70263268d 100644 --- a/quic/state/QuicTransportStatsCallback.h +++ b/quic/state/QuicTransportStatsCallback.h @@ -182,6 +182,8 @@ class QuicTransportStatsCallback { virtual void onDatagramDroppedOnRead() = 0; + virtual void onShortHeaderPadding(size_t padSize) = 0; + static const char* toString(PacketDropReason reason) { switch (reason) { case PacketDropReason::NONE: diff --git a/quic/state/TransportSettings.h b/quic/state/TransportSettings.h index 8c033ed27..ca10c9dea 100644 --- a/quic/state/TransportSettings.h +++ b/quic/state/TransportSettings.h @@ -275,6 +275,14 @@ struct TransportSettings { // Whether to skip sending the ACK-only initial in response to crypto data in // initial packet num space bool skipInitPktNumSpaceCryptoAck{false}; + // Whether to issue new tokens via NewToken frames. + bool issueNewTokens{false}; + // Used to generate the number of frames to add to short header packets. + // Packets will have padding frames added such that the total space remaining + // 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}; }; } // namespace quic diff --git a/quic/state/test/MockQuicStats.h b/quic/state/test/MockQuicStats.h index a7cd0cf34..02eb215c5 100644 --- a/quic/state/test/MockQuicStats.h +++ b/quic/state/test/MockQuicStats.h @@ -67,6 +67,7 @@ class MockQuicStats : public QuicTransportStatsCallback { MOCK_METHOD0(onNewTokenReceived, void()); MOCK_METHOD0(onNewTokenIssued, void()); MOCK_METHOD0(onTokenDecryptFailure, void()); + MOCK_METHOD1(onShortHeaderPadding, void(size_t)); }; class MockQuicStatsFactory : public QuicTransportStatsCallbackFactory {