Skip to content

Commit

Permalink
Rename offset in RstStreamFrame to finalSize
Browse files Browse the repository at this point in the history
Summary: This will make it easier to distinguish between the `finalSize` and the `reliableSize` when we implement reliable resets

Reviewed By: hanidamlaj

Differential Revision: D64836474

fbshipit-source-id: d811d64a2538d4d1acba1ea10a7790d5905f02a4
  • Loading branch information
Aman Sharma authored and facebook-github-bot committed Nov 14, 2024
1 parent 5b309db commit 3f27fde
Show file tree
Hide file tree
Showing 10 changed files with 28 additions and 27 deletions.
6 changes: 3 additions & 3 deletions quic/api/test/QuicTransportTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2103,7 +2103,7 @@ TEST_F(QuicTransportTest, RstStream) {
continue;
}
EXPECT_EQ(streamId, rstFrame->streamId);
EXPECT_EQ(0, rstFrame->offset);
EXPECT_EQ(0, rstFrame->finalSize);
EXPECT_EQ(GenericApplicationErrorCode::UNKNOWN, rstFrame->errorCode);
rstFound = true;
}
Expand Down Expand Up @@ -2870,7 +2870,7 @@ TEST_F(QuicTransportTest, RstWrittenStream) {
continue;
}
EXPECT_EQ(streamId, rstStream->streamId);
EXPECT_EQ(currentWriteOffset, rstStream->offset);
EXPECT_EQ(currentWriteOffset, rstStream->finalSize);
EXPECT_EQ(GenericApplicationErrorCode::UNKNOWN, rstStream->errorCode);
foundReset = true;
}
Expand Down Expand Up @@ -2975,7 +2975,7 @@ TEST_F(QuicTransportTest, WriteAfterSendRst) {
continue;
}
EXPECT_EQ(streamId, rstFrame->streamId);
EXPECT_EQ(currentWriteOffset, rstFrame->offset);
EXPECT_EQ(currentWriteOffset, rstFrame->finalSize);
EXPECT_EQ(GenericApplicationErrorCode::UNKNOWN, rstFrame->errorCode);
foundReset = true;
}
Expand Down
6 changes: 3 additions & 3 deletions quic/codec/Decode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -376,15 +376,15 @@ RstStreamFrame decodeRstStreamFrame(folly::io::Cursor& cursor) {
quic::TransportErrorCode::FRAME_ENCODING_ERROR,
quic::FrameType::RST_STREAM);
}
auto offset = decodeQuicInteger(cursor);
if (!offset) {
auto finalSize = decodeQuicInteger(cursor);
if (!finalSize) {
throw QuicTransportException(
"Bad offset",
quic::TransportErrorCode::FRAME_ENCODING_ERROR,
quic::FrameType::RST_STREAM);
}
return RstStreamFrame(
folly::to<StreamId>(streamId->first), errorCode, offset->first);
folly::to<StreamId>(streamId->first), errorCode, finalSize->first);
}

StopSendingFrame decodeStopSendingFrame(folly::io::Cursor& cursor) {
Expand Down
2 changes: 1 addition & 1 deletion quic/codec/QuicWriteCodec.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -816,7 +816,7 @@ size_t writeFrame(QuicWriteFrame&& frame, PacketBuilderInterface& builder) {
RstStreamFrame& rstStreamFrame = *frame.asRstStreamFrame();
QuicInteger intFrameType(static_cast<uint8_t>(FrameType::RST_STREAM));
QuicInteger streamId(rstStreamFrame.streamId);
QuicInteger offset(rstStreamFrame.offset);
QuicInteger offset(rstStreamFrame.finalSize);
QuicInteger errorCode(static_cast<uint64_t>(rstStreamFrame.errorCode));
size_t errorSize = errorCode.getSize();
auto rstStreamFrameSize = intFrameType.getSize() + errorSize +
Expand Down
8 changes: 4 additions & 4 deletions quic/codec/Types.h
Original file line number Diff line number Diff line change
Expand Up @@ -298,17 +298,17 @@ struct WriteAckFrameResult {
struct RstStreamFrame {
StreamId streamId;
ApplicationErrorCode errorCode;
uint64_t offset;
uint64_t finalSize;

RstStreamFrame(
StreamId streamIdIn,
ApplicationErrorCode errorCodeIn,
uint64_t offsetIn)
: streamId(streamIdIn), errorCode(errorCodeIn), offset(offsetIn) {}
uint64_t finalSizeIn)
: streamId(streamIdIn), errorCode(errorCodeIn), finalSize(finalSizeIn) {}

bool operator==(const RstStreamFrame& rhs) const {
return streamId == rhs.streamId && errorCode == rhs.errorCode &&
offset == rhs.offset;
finalSize == rhs.finalSize;
}
};

Expand Down
4 changes: 2 additions & 2 deletions quic/codec/test/QuicWriteCodecTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2097,7 +2097,7 @@ TEST_F(QuicWriteCodecTest, WriteRstStream) {
auto& resultRstStreamFrame = *regularPacket.frames[0].asRstStreamFrame();
EXPECT_EQ(errorCode, resultRstStreamFrame.errorCode);
EXPECT_EQ(id, resultRstStreamFrame.streamId);
EXPECT_EQ(offset, resultRstStreamFrame.offset);
EXPECT_EQ(offset, resultRstStreamFrame.finalSize);

auto wireBuf = std::move(builtOut.second);
BufQueue queue;
Expand All @@ -2106,7 +2106,7 @@ TEST_F(QuicWriteCodecTest, WriteRstStream) {
auto& wireRstStreamFrame = *decodedFrame.asRstStreamFrame();
EXPECT_EQ(errorCode, wireRstStreamFrame.errorCode);
EXPECT_EQ(id, wireRstStreamFrame.streamId);
EXPECT_EQ(offset, wireRstStreamFrame.offset);
EXPECT_EQ(offset, wireRstStreamFrame.finalSize);
// At last, verify there is nothing left in the wire format bytes:
EXPECT_EQ(queue.chainLength(), 0);
}
Expand Down
4 changes: 2 additions & 2 deletions quic/logging/BaseQLogger.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ std::unique_ptr<QLogPacketEvent> BaseQLogger::createPacketEvent(
case QuicFrame::Type::RstStreamFrame: {
const auto& frame = *quicFrame.asRstStreamFrame();
event->frames.push_back(std::make_unique<RstStreamFrameLog>(
frame.streamId, frame.errorCode, frame.offset));
frame.streamId, frame.errorCode, frame.finalSize));
break;
}
case QuicFrame::Type::ConnectionCloseFrame: {
Expand Down Expand Up @@ -242,7 +242,7 @@ std::unique_ptr<QLogPacketEvent> BaseQLogger::createPacketEvent(
case QuicWriteFrame::Type::RstStreamFrame: {
const RstStreamFrame& frame = *quicFrame.asRstStreamFrame();
event->frames.push_back(std::make_unique<RstStreamFrameLog>(
frame.streamId, frame.errorCode, frame.offset));
frame.streamId, frame.errorCode, frame.finalSize));
break;
}
case QuicWriteFrame::Type::ConnectionCloseFrame: {
Expand Down
4 changes: 2 additions & 2 deletions quic/loss/test/QuicLossFunctionsTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -944,7 +944,7 @@ TEST_F(QuicLossFunctionsTest, TestMarkRstLoss) {
auto& retxRstFrame = conn->pendingEvents.resets.at(stream->id);
EXPECT_EQ(stream->id, retxRstFrame.streamId);
EXPECT_EQ(GenericApplicationErrorCode::UNKNOWN, retxRstFrame.errorCode);
EXPECT_EQ(currentOffset, retxRstFrame.offset);
EXPECT_EQ(currentOffset, retxRstFrame.finalSize);

// write again:
writeQuicDataToSocket(
Expand All @@ -967,7 +967,7 @@ TEST_F(QuicLossFunctionsTest, TestMarkRstLoss) {
}
EXPECT_EQ(stream->id, resetFrame->streamId);
EXPECT_EQ(GenericApplicationErrorCode::UNKNOWN, resetFrame->errorCode);
EXPECT_EQ(currentOffset, resetFrame->offset);
EXPECT_EQ(currentOffset, resetFrame->finalSize);
rstFound = true;
}
EXPECT_TRUE(rstFound);
Expand Down
2 changes: 1 addition & 1 deletion quic/server/test/QuicServerTransportTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3318,7 +3318,7 @@ TEST_F(QuicServerTransportTest, ResetDSRStream) {
continue;
}
EXPECT_EQ(streamId, rstStream->streamId);
EXPECT_GT(rstStream->offset, 200);
EXPECT_GT(rstStream->finalSize, 200);
EXPECT_EQ(GenericApplicationErrorCode::UNKNOWN, rstStream->errorCode);
foundReset = true;
}
Expand Down
15 changes: 8 additions & 7 deletions quic/state/stream/StreamStateFunctions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,32 +32,33 @@ void resetQuicStream(QuicStreamState& stream, ApplicationErrorCode error) {

void onResetQuicStream(QuicStreamState& stream, const RstStreamFrame& frame) {
if (stream.finalReadOffset &&
stream.finalReadOffset.value() != frame.offset) {
stream.finalReadOffset.value() != frame.finalSize) {
throw QuicTransportException(
"Read offset mismatch, " +
folly::to<std::string>(stream.finalReadOffset.value()) +
" != " + folly::to<std::string>(frame.offset),
" != " + folly::to<std::string>(frame.finalSize),
TransportErrorCode::FINAL_SIZE_ERROR);
}
// Mark eofoffset:
if (stream.maxOffsetObserved > frame.offset) {
if (stream.maxOffsetObserved > frame.finalSize) {
throw QuicTransportException(
"Reset in middle of stream", TransportErrorCode::FINAL_SIZE_ERROR);
}
// Verify that the flow control is consistent.
updateFlowControlOnStreamData(stream, stream.maxOffsetObserved, frame.offset);
updateFlowControlOnStreamData(
stream, stream.maxOffsetObserved, frame.finalSize);
// Drop read buffer:
stream.readBuffer.clear();
stream.finalReadOffset = frame.offset;
stream.finalReadOffset = frame.finalSize;
stream.streamReadError = frame.errorCode;
bool appReadAllBytes = stream.currentReadOffset > *stream.finalReadOffset;
if (!appReadAllBytes) {
// If the currentReadOffset > finalReadOffset we have already processed
// all the bytes until FIN, so we don't need to do anything for the read
// side of the flow controller.
auto lastReadOffset = stream.currentReadOffset;
stream.currentReadOffset = frame.offset;
stream.maxOffsetObserved = frame.offset;
stream.currentReadOffset = frame.finalSize;
stream.maxOffsetObserved = frame.finalSize;
updateFlowControlOnRead(stream, lastReadOffset, Clock::now());
}
stream.conn.streamManager->updateReadableStreams(stream);
Expand Down
4 changes: 2 additions & 2 deletions quic/state/test/QuicStreamFunctionsTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2135,7 +2135,7 @@ TEST_F(QuicServerStreamFunctionsTest, TestAppendPendingStreamResetAllData) {
appendPendingStreamReset(conn, stream, GenericApplicationErrorCode::UNKNOWN);
auto rst = conn.pendingEvents.resets.at(id);
EXPECT_EQ(rst.errorCode, GenericApplicationErrorCode::UNKNOWN);
EXPECT_EQ(rst.offset, len);
EXPECT_EQ(rst.finalSize, len);
}

TEST_F(
Expand All @@ -2154,7 +2154,7 @@ TEST_F(
appendPendingStreamReset(conn, stream, GenericApplicationErrorCode::UNKNOWN);
auto rst = conn.pendingEvents.resets.at(id);
EXPECT_EQ(rst.errorCode, GenericApplicationErrorCode::UNKNOWN);
EXPECT_EQ(rst.offset, len);
EXPECT_EQ(rst.finalSize, len);
}

TEST_P(QuicStreamFunctionsTestBase, LargestWriteOffsetSeenFIN) {
Expand Down

0 comments on commit 3f27fde

Please sign in to comment.