lib/mvfst
1
0
Fork 0
mirror of https://github.com/facebookincubator/mvfst.git synced 2025-07-29 03:41:11 +03:00
Code Activity

Do not add ack frames to packet when it was rebuilt in cloning scheduler

Browse Source 
Summary:
We may end up writing acks after a length-less stream frame and it
will be considered garbage data by the peer. The acks will be written later as
usual, outside of cloning scheduler.

Reviewed By: mjoras

Differential Revision: D35561912

fbshipit-source-id: 9334523b984870fa525c856a504ae7d2ae4f34c3
This commit is contained in:
Konstantin Tsoy
2022-04-12 09:38:19 -07:00
committed by Facebook GitHub Bot
parent 2f4b66bb64
commit d31df23ca5
2 changed files with 2 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
quic/api/QuicPacketScheduler.cpp
View File

@ -887,14 +887,6 @@ SchedulingResult CloningScheduler::scheduleFramesForPacket(
// Rebuilder will write the rest of frames
auto rebuildResult = rebuilder.rebuildFromPacket(outstandingPacket);
if (rebuildResult) {
if (conn_.version.has_value() &&
conn_.version.value() == QuicVersion::MVFST_EXPERIMENTAL2) {
// Check if we have any acks pending and write those into the cloned
// packet as well.
if (frameScheduler_.hasPendingAcks()) {
frameScheduler_.writeNextAcks(*internalBuilder);
}
}
return SchedulingResult(
std::move(rebuildResult), std::move(*internalBuilder).buildPacket());
} else if (

4
quic/api/test/QuicPacketSchedulerTest.cpp
View File

@ -792,7 +792,7 @@ TEST_F(QuicPacketSchedulerTest, CloneSchedulerHasHandshakeDataAndAcks) {
EXPECT_TRUE(result.packetEvent.has_value());
EXPECT_TRUE(result.packet.has_value());
// Cloned packet has to have at least one ack frame AND the crypto data.
// Cloned packet has to have crypto data and no acks.
bool hasAckFrame = false;
bool hasCryptoFrame = false;
for (auto iter = result.packet->packet.frames.cbegin();
@ -810,7 +810,7 @@ TEST_F(QuicPacketSchedulerTest, CloneSchedulerHasHandshakeDataAndAcks) {
break;
}
}
EXPECT_TRUE(hasAckFrame);
EXPECT_FALSE(hasAckFrame);
EXPECT_TRUE(hasCryptoFrame);
}