lib/mvfst
1
0
Fork 0
mirror of https://github.com/facebookincubator/mvfst.git synced 2025-11-25 15:43:13 +03:00
Code Activity

Rename ackFrameMatchesRetransmitBuffer to streamFrameMatchesRetransmitBuffer

Browse Source 
Summary:
The function indeed compares a Stream frame to a buffer, not an Ack
frame to a buffer.

Reviewed By: sharma95

Differential Revision: D18000632

fbshipit-source-id: fbbc36378404f8f52a199114af48b9020d0569e8
This commit is contained in:
Yang Chi
2019-10-18 15:52:22 -07:00
committed by Facebook Github Bot
parent fc71487c81
commit dc27ebf05d
6 changed files with 36 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
quic/codec/QuicPacketRebuilder.cpp
View File

@@ -244,7 +244,7 @@ Buf PacketRebuilder::cloneRetransmissionBuffer(
return buffer.offset < targetOffset;
});
if (iter != stream->retransmissionBuffer.end()) {
if (ackFrameMatchesRetransmitBuffer(*stream, frame, *iter)) {
if (streamFrameMatchesRetransmitBuffer(*stream, frame, *iter)) {
DCHECK(!frame.len || !iter->data.empty())
<< "WriteStreamFrame cloning: frame is not empty but StreamBuffer has"
<< " empty data. " << conn_;

2
quic/loss/QuicLossFunctions.cpp
View File

@@ -110,7 +110,7 @@ void markPacketLoss(
}
// The original rxmt offset might have been bumped up after it was
// shrunk due to egress partially reliable skip.
if (!ackFrameMatchesRetransmitBuffer(*stream, frame, *bufferItr)) {
if (!streamFrameMatchesRetransmitBuffer(*stream, frame, *bufferItr)) {
break;
}
stream->lossBuffer.insert(

39
quic/state/QuicStreamFunctions.cpp
View File

@@ -432,33 +432,33 @@ void processCryptoStreamAck(
cryptoStream.retransmissionBuffer.erase(ackedBuffer);
}
bool ackFrameMatchesRetransmitBuffer(
bool streamFrameMatchesRetransmitBuffer(
const QuicStreamState& stream,
const WriteStreamFrame& ackFrame,
const WriteStreamFrame& streamFrame,
const StreamBuffer& buf) {
// There are 3 possible situations.
// 1) Fully reliable mode: the buffer's and ack frame's offsets and lengths
// 1) Fully reliable mode: the buffer's and stream frame's offsets and lengths
// must match.
// 2) Partially reliable mode: the retransmit queue buffer has been
// fully removed by an egress skip before the ack frame arrived.
// fully removed by an egress skip before the stream frame arrived.
// 3) Partially reliable mode: the retransmit queue buffer was only
// partially trimmed.
// In this case, the retransmit buffer offset must be >= ack frame offset
// and retransmit buffer length must be <= ack frame length. Also, the
// retransmit buffer [offset + length] must match (ack) frame [offset +
// length].
// In this case, the retransmit buffer offset must be >= stream frame
// offset and retransmit buffer length must be <= stream frame len field
// vale. Also, the retransmit buffer [offset + length] must match stream
// frame [offset + length].
bool match = false;
if (stream.conn.partialReliabilityEnabled) {
auto frameRightOffset = ackFrame.offset + ackFrame.len;
auto frameRightOffset = streamFrame.offset + streamFrame.len;
if (frameRightOffset > buf.offset) {
// There is overlap, buffer fully or partially matches.
DCHECK(buf.offset >= ackFrame.offset);
DCHECK(buf.data.chainLength() <= ackFrame.len);
DCHECK(buf.offset >= streamFrame.offset);
DCHECK(buf.data.chainLength() <= streamFrame.len);
// The offsets and lengths in the ack frame and buffer may be different,
// but their sum should stay the same (e.g. offset grows, length shrinks
// but sum must be the same).
// The offsets and lengths in the stream frame and buffer may be
// different, but their sum should stay the same (e.g. offset grows,
// length shrinks but sum must be the same).
//
// Example: let's say we send data buf with offset=0 and len=11 and we
// save a copy in retransmission queue. Then we send egress skip to offset
@@ -468,14 +468,15 @@ bool ackFrameMatchesRetransmitBuffer(
// offsets and lengths are going to be different, but their sum will be
// the same.
DCHECK_EQ(
buf.offset + buf.data.chainLength(), ackFrame.offset + ackFrame.len);
DCHECK_EQ(buf.eof, ackFrame.fin);
buf.offset + buf.data.chainLength(),
streamFrame.offset + streamFrame.len);
DCHECK_EQ(buf.eof, streamFrame.fin);
match = true;
} // else frameRightOffset <= buf.offset { ignore }
} else {
DCHECK_EQ(buf.offset, ackFrame.offset);
DCHECK_EQ(buf.data.chainLength(), ackFrame.len);
DCHECK_EQ(buf.eof, ackFrame.fin);
DCHECK_EQ(buf.offset, streamFrame.offset);
DCHECK_EQ(buf.data.chainLength(), streamFrame.len);
DCHECK_EQ(buf.eof, streamFrame.fin);
match = true;
}
return match;

7
quic/state/QuicStreamFunctions.h
View File

@@ -137,13 +137,10 @@ void processCryptoStreamAck(
uint64_t len);
/**
* Checks if ack frame matches buffer from the retransmit queue.
* Checks if stream frame matches buffer from the retransmit queue.
*
* @param stream The stream.
* @param ackFrame Ack frame that allegedly acks the above buffer.
* @param buf Buffer from the retransmit queue.
*/
bool ackFrameMatchesRetransmitBuffer(
bool streamFrameMatchesRetransmitBuffer(
const QuicStreamState& stream,
const WriteStreamFrame& ackFrame,
const StreamBuffer& buf);

3
quic/state/stream/StreamOpenHandlers-inl.h
View File

@@ -95,7 +95,8 @@ inline void Handler<
});
if (ackedBuffer != stream.retransmissionBuffer.end()) {
if (ackFrameMatchesRetransmitBuffer(stream, ack.ackedFrame, *ackedBuffer)) {
if (streamFrameMatchesRetransmitBuffer(
stream, ack.ackedFrame, *ackedBuffer)) {
VLOG(10) << "Open: acked stream data stream=" << stream.id
<< " offset=" << ackedBuffer->offset
<< " len=" << ackedBuffer->data.chainLength()

18
quic/state/test/QuicStreamFunctionsTest.cpp
View File

@@ -1940,7 +1940,9 @@ TEST_F(QuicStreamFunctionsTest, AckCryptoStreamOffsetLengthMismatch) {
EXPECT_EQ(cryptoStream.retransmissionBuffer.size(), 1);
}
TEST_F(QuicStreamFunctionsTest, AckFrameMatchesRetransmitBufferFullyReliable) {
TEST_F(
QuicStreamFunctionsTest,
StreamFrameMatchesRetransmitBufferFullyReliable) {
conn.partialReliabilityEnabled = false;
StreamId id = 4;
QuicStreamState stream(id, conn);
@@ -1953,12 +1955,12 @@ TEST_F(QuicStreamFunctionsTest, AckFrameMatchesRetransmitBufferFullyReliable) {
0 /* offset */,
data->length() /* length */,
true /* eof */);
EXPECT_TRUE(ackFrameMatchesRetransmitBuffer(stream, ackFrame, buf));
EXPECT_TRUE(streamFrameMatchesRetransmitBuffer(stream, ackFrame, buf));
}
TEST_F(
QuicStreamFunctionsTest,
AckFrameMatchesRetransmitBufferPartiallyReliableNoSkip) {
StreamFrameMatchesRetransmitBufferPartiallyReliableNoSkip) {
conn.partialReliabilityEnabled = true;
StreamId id = 4;
QuicStreamState stream(id, conn);
@@ -1971,12 +1973,12 @@ TEST_F(
0 /* offset */,
data->length() /* length */,
true /* eof */);
EXPECT_TRUE(ackFrameMatchesRetransmitBuffer(stream, ackFrame, buf));
EXPECT_TRUE(streamFrameMatchesRetransmitBuffer(stream, ackFrame, buf));
}
TEST_F(
QuicStreamFunctionsTest,
AckFrameMatchesRetransmitBufferPartiallyReliableFullBufSkipped) {
StreamFrameMatchesRetransmitBufferPartiallyReliableFullBufSkipped) {
conn.partialReliabilityEnabled = true;
StreamId id = 4;
QuicStreamState stream(id, conn);
@@ -1989,12 +1991,12 @@ TEST_F(
0 /* offset */,
data->length() /* length */,
true /* eof */);
EXPECT_FALSE(ackFrameMatchesRetransmitBuffer(stream, ackFrame, buf));
EXPECT_FALSE(streamFrameMatchesRetransmitBuffer(stream, ackFrame, buf));
}
TEST_F(
QuicStreamFunctionsTest,
AckFrameMatchesRetransmitBufferPartiallyReliableHalfBufSkipped) {
StreamFrameMatchesRetransmitBufferPartiallyReliableHalfBufSkipped) {
conn.partialReliabilityEnabled = true;
StreamId id = 4;
QuicStreamState stream(id, conn);
@@ -2004,7 +2006,7 @@ TEST_F(
WriteStreamFrame ackFrame(
id /* streamId */, 0 /* offset */, 5 /* length */, true /* eof */);
EXPECT_TRUE(ackFrameMatchesRetransmitBuffer(stream, ackFrame, buf));
EXPECT_TRUE(streamFrameMatchesRetransmitBuffer(stream, ackFrame, buf));
}
} // namespace test
} // namespace quic