diff --git a/quic/codec/QuicPacketRebuilder.cpp b/quic/codec/QuicPacketRebuilder.cpp index 987adaeab..64f09978f 100644 --- a/quic/codec/QuicPacketRebuilder.cpp +++ b/quic/codec/QuicPacketRebuilder.cpp @@ -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_; diff --git a/quic/loss/QuicLossFunctions.cpp b/quic/loss/QuicLossFunctions.cpp index 9a5f56336..4070ffa5d 100644 --- a/quic/loss/QuicLossFunctions.cpp +++ b/quic/loss/QuicLossFunctions.cpp @@ -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( diff --git a/quic/state/QuicStreamFunctions.cpp b/quic/state/QuicStreamFunctions.cpp index ded62fd42..3f19fb0f1 100644 --- a/quic/state/QuicStreamFunctions.cpp +++ b/quic/state/QuicStreamFunctions.cpp @@ -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; diff --git a/quic/state/QuicStreamFunctions.h b/quic/state/QuicStreamFunctions.h index bb7429b58..28ef4ea7f 100644 --- a/quic/state/QuicStreamFunctions.h +++ b/quic/state/QuicStreamFunctions.h @@ -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); diff --git a/quic/state/stream/StreamOpenHandlers-inl.h b/quic/state/stream/StreamOpenHandlers-inl.h index b1351bfd8..d21651372 100644 --- a/quic/state/stream/StreamOpenHandlers-inl.h +++ b/quic/state/stream/StreamOpenHandlers-inl.h @@ -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() diff --git a/quic/state/test/QuicStreamFunctionsTest.cpp b/quic/state/test/QuicStreamFunctionsTest.cpp index 6d6185741..73b82cce0 100644 --- a/quic/state/test/QuicStreamFunctionsTest.cpp +++ b/quic/state/test/QuicStreamFunctionsTest.cpp @@ -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