1
0
mirror of https://github.com/facebookincubator/mvfst.git synced 2025-07-30 14:43:05 +03:00

Default to PTOing PING frames.

Summary: This kinda makes sense to just use as a default.

Reviewed By: kvtsoy, sharmafb

Differential Revision: D64066392

fbshipit-source-id: 0915f163c0483af6bec014bde61e82b6ee2ac6cb
This commit is contained in:
Matt Joras
2024-10-08 20:29:33 -07:00
committed by Facebook GitHub Bot
parent 16ca1bc87c
commit 28748bfad2
5 changed files with 2 additions and 46 deletions

View File

@ -801,9 +801,7 @@ void updateConnection(
case QuicWriteFrame::Type::PingFrame: case QuicWriteFrame::Type::PingFrame:
conn.pendingEvents.sendPing = false; conn.pendingEvents.sendPing = false;
isPing = true; isPing = true;
if (conn.transportSettings.ptoPingFrames) { retransmittable = true;
retransmittable = true;
}
conn.numPingFramesSent++; conn.numPingFramesSent++;
break; break;
case QuicWriteFrame::Type::QuicSimpleFrame: { case QuicWriteFrame::Type::QuicSimpleFrame: {

View File

@ -206,21 +206,8 @@ class QuicTransportFunctionsTest : public Test {
std::unique_ptr<MockQuicStats> quicStats_; std::unique_ptr<MockQuicStats> quicStats_;
}; };
TEST_F(QuicTransportFunctionsTest, PingPacketGoesToOPList) {
auto conn = createConn();
auto packet = buildEmptyPacket(*conn, PacketNumberSpace::AppData);
packet.packet.frames.push_back(PingFrame());
EXPECT_EQ(0, conn->outstandings.packets.size());
updateConnection(
*conn, none, packet.packet, Clock::now(), 50, 0, false /* isDSRPacket */);
EXPECT_EQ(1, conn->outstandings.packets.size());
// But it won't set loss detection alarm
EXPECT_FALSE(conn->pendingEvents.setLossDetectionAlarm);
}
TEST_F(QuicTransportFunctionsTest, PingPacketGoesToOPListAndLossAlarm) { TEST_F(QuicTransportFunctionsTest, PingPacketGoesToOPListAndLossAlarm) {
auto conn = createConn(); auto conn = createConn();
conn->transportSettings.ptoPingFrames = true;
auto packet = buildEmptyPacket(*conn, PacketNumberSpace::AppData); auto packet = buildEmptyPacket(*conn, PacketNumberSpace::AppData);
packet.packet.frames.push_back(PingFrame()); packet.packet.frames.push_back(PingFrame());
EXPECT_EQ(0, conn->outstandings.packets.size()); EXPECT_EQ(0, conn->outstandings.packets.size());

View File

@ -176,9 +176,7 @@ Optional<ClonedPacketIdentifier> PacketRebuilder::rebuildFromPacket(
case QuicWriteFrame::Type::PingFrame: { case QuicWriteFrame::Type::PingFrame: {
const PingFrame& pingFrame = *frame.asPingFrame(); const PingFrame& pingFrame = *frame.asPingFrame();
writeSuccess = writeFrame(pingFrame, builder_) != 0; writeSuccess = writeFrame(pingFrame, builder_) != 0;
if (conn_.transportSettings.ptoPingFrames) { notPureAck |= writeSuccess;
notPureAck |= writeSuccess;
}
break; break;
} }
case QuicWriteFrame::Type::QuicSimpleFrame: { case QuicWriteFrame::Type::QuicSimpleFrame: {

View File

@ -522,30 +522,6 @@ TEST_F(QuicPacketRebuilderTest, CloneCounter) {
EXPECT_EQ(1, conn.outstandings.numClonedPackets()); EXPECT_EQ(1, conn.outstandings.numClonedPackets());
} }
TEST_F(QuicPacketRebuilderTest, PurePingWontRebuild) {
ShortHeader shortHeader1(
ProtectionType::KeyPhaseZero, getTestConnectionId(), 0);
RegularQuicPacketBuilder regularBuilder(
kDefaultUDPSendPacketLen, std::move(shortHeader1), 0);
regularBuilder.encodePacketHeader();
PingFrame pingFrame;
writeFrame(pingFrame, regularBuilder);
auto packet = std::move(regularBuilder).buildPacket();
auto outstandingPacket = makeDummyOutstandingPacket(packet.packet, 50);
EXPECT_EQ(1, outstandingPacket.packet.frames.size());
QuicServerConnectionState conn(
FizzServerQuicHandshakeContext::Builder().build());
ShortHeader shortHeader2(
ProtectionType::KeyPhaseZero, getTestConnectionId(), 0);
RegularQuicPacketBuilder regularBuilder2(
kDefaultUDPSendPacketLen, std::move(shortHeader2), 0);
regularBuilder2.encodePacketHeader();
PacketRebuilder rebuilder(regularBuilder2, conn);
EXPECT_EQ(none, rebuilder.rebuildFromPacket(outstandingPacket));
EXPECT_FALSE(outstandingPacket.maybeClonedPacketIdentifier.has_value());
EXPECT_EQ(0, conn.outstandings.numClonedPackets());
}
TEST_F(QuicPacketRebuilderTest, PurePingWillRebuild) { TEST_F(QuicPacketRebuilderTest, PurePingWillRebuild) {
ShortHeader shortHeader1( ShortHeader shortHeader1(
ProtectionType::KeyPhaseZero, getTestConnectionId(), 0); ProtectionType::KeyPhaseZero, getTestConnectionId(), 0);
@ -559,7 +535,6 @@ TEST_F(QuicPacketRebuilderTest, PurePingWillRebuild) {
EXPECT_EQ(1, outstandingPacket.packet.frames.size()); EXPECT_EQ(1, outstandingPacket.packet.frames.size());
QuicServerConnectionState conn( QuicServerConnectionState conn(
FizzServerQuicHandshakeContext::Builder().build()); FizzServerQuicHandshakeContext::Builder().build());
conn.transportSettings.ptoPingFrames = true;
ShortHeader shortHeader2( ShortHeader shortHeader2(
ProtectionType::KeyPhaseZero, getTestConnectionId(), 0); ProtectionType::KeyPhaseZero, getTestConnectionId(), 0);
RegularQuicPacketBuilder regularBuilder2( RegularQuicPacketBuilder regularBuilder2(

View File

@ -410,8 +410,6 @@ struct TransportSettings {
// receiving and then processing. // receiving and then processing.
bool networkDataPerSocketRead{false}; bool networkDataPerSocketRead{false};
bool cloneAllPacketsWithCryptoFrame{false}; bool cloneAllPacketsWithCryptoFrame{false};
// Whether we allow PTOing of packets with just PING frames.
bool ptoPingFrames{false};
// Always send multiple PTOs even if we don't have multiple packets. // Always send multiple PTOs even if we don't have multiple packets.
bool alwaysPtoMultiple{false}; bool alwaysPtoMultiple{false};
}; };