Skip to content

Commit

Permalink
Fix somes todos
Browse files Browse the repository at this point in the history
  • Loading branch information
@guhetier
guhetier committed Jan 25, 2025
1 parent 1c2656c commit 3ca2f64
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 119 deletions.
115 changes: 45 additions & 70 deletions src/core/recv_buffer.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,46 +70,51 @@ QuicRecvBufferInitialize(
_In_opt_ QUIC_RECV_CHUNK* PreallocatedChunk
)
{
QUIC_STATUS Status;

CXPLAT_DBG_ASSERT(AllocBufferLength != 0 && (AllocBufferLength & (AllocBufferLength - 1)) == 0); // Power of 2
CXPLAT_DBG_ASSERT(VirtualBufferLength != 0 && (VirtualBufferLength & (VirtualBufferLength - 1)) == 0); // Power of 2
CXPLAT_DBG_ASSERT(AllocBufferLength != 0 || RecvMode == QUIC_RECV_BUF_MODE_EXTERNAL);
CXPLAT_DBG_ASSERT(VirtualBufferLength != 0 || RecvMode == QUIC_RECV_BUF_MODE_EXTERNAL);
CXPLAT_DBG_ASSERT((AllocBufferLength & (AllocBufferLength - 1)) == 0); // Power of 2
CXPLAT_DBG_ASSERT((VirtualBufferLength & (VirtualBufferLength - 1)) == 0); // Power of 2
CXPLAT_DBG_ASSERT(AllocBufferLength <= VirtualBufferLength);

QUIC_RECV_CHUNK* Chunk = NULL;
if (PreallocatedChunk != NULL) {
RecvBuffer->PreallocatedChunk = PreallocatedChunk;
Chunk = PreallocatedChunk;
} else {
RecvBuffer->PreallocatedChunk = NULL;
Chunk = CXPLAT_ALLOC_NONPAGED(sizeof(QUIC_RECV_CHUNK) + AllocBufferLength, QUIC_POOL_RECVBUF);
if (Chunk == NULL) {
QuicTraceEvent(
AllocFailure,
"Allocation of '%s' failed. (%llu bytes)",
"recv_buffer",
sizeof(QUIC_RECV_CHUNK) + AllocBufferLength);
Status = QUIC_STATUS_OUT_OF_MEMORY;
goto Error;
}
QuicRecvChunkInitialize(Chunk, AllocBufferLength, (uint8_t*)(Chunk + 1));
}

QuicRangeInitialize(QUIC_MAX_RANGE_ALLOC_SIZE, &RecvBuffer->WrittenRanges);
CxPlatListInitializeHead(&RecvBuffer->Chunks);
CxPlatListInsertHead(&RecvBuffer->Chunks, &Chunk->Link);
RecvBuffer->BaseOffset = 0;
RecvBuffer->ReadStart = 0;
RecvBuffer->ReadPendingLength = 0;
RecvBuffer->ReadLength = 0;
RecvBuffer->Capacity = AllocBufferLength;
RecvBuffer->VirtualBufferLength = VirtualBufferLength;
RecvBuffer->RecvMode = RecvMode;
Status = QUIC_STATUS_SUCCESS;
QuicRangeInitialize(QUIC_MAX_RANGE_ALLOC_SIZE, &RecvBuffer->WrittenRanges);
CxPlatListInitializeHead(&RecvBuffer->Chunks);

Error:
if (RecvMode != QUIC_RECV_BUF_MODE_EXTERNAL) {
//
// Setup an initial chunk.
//
QUIC_RECV_CHUNK* Chunk = NULL;
if (PreallocatedChunk != NULL) {
RecvBuffer->PreallocatedChunk = PreallocatedChunk;
Chunk = PreallocatedChunk;
} else {
RecvBuffer->PreallocatedChunk = NULL;
Chunk = CXPLAT_ALLOC_NONPAGED(sizeof(QUIC_RECV_CHUNK) + AllocBufferLength, QUIC_POOL_RECVBUF);
if (Chunk == NULL) {
QuicTraceEvent(
AllocFailure,
"Allocation of '%s' failed. (%llu bytes)",
"recv_buffer",
sizeof(QUIC_RECV_CHUNK) + AllocBufferLength);
return QUIC_STATUS_OUT_OF_MEMORY;
}
QuicRecvChunkInitialize(Chunk, AllocBufferLength, (uint8_t*)(Chunk + 1));
}
CxPlatListInsertHead(&RecvBuffer->Chunks, &Chunk->Link);
RecvBuffer->Capacity = AllocBufferLength;
RecvBuffer->VirtualBufferLength = VirtualBufferLength;
} else {
RecvBuffer->PreallocatedChunk = NULL;
RecvBuffer->Capacity = 0;
RecvBuffer->VirtualBufferLength = 0;
}

return Status;
return QUIC_STATUS_SUCCESS;
}

_IRQL_requires_max_(DISPATCH_LEVEL)
Expand Down Expand Up @@ -176,16 +181,19 @@ QuicRecvBufferHasUnreadData(
}

_IRQL_requires_max_(DISPATCH_LEVEL)
void
QuicRecvBufferIncreaseVirtualBufferLength(
BOOLEAN
QuicRecvBufferTryIncreaseVirtualBufferLength(
_In_ QUIC_RECV_BUFFER* RecvBuffer,
_In_ uint32_t NewLength
)
{
// TODO guhetier: Must make sure this can't be called in new mode
CXPLAT_DBG_ASSERT(RecvBuffer->RecvMode != QUIC_RECV_BUF_MODE_EXTERNAL);
if (RecvBuffer->RecvMode == QUIC_RECV_BUF_MODE_EXTERNAL) {
return FALSE;
}

CXPLAT_DBG_ASSERT(NewLength >= RecvBuffer->VirtualBufferLength); // Don't support decrease.
RecvBuffer->VirtualBufferLength = NewLength;
return TRUE;
}

_IRQL_requires_max_(DISPATCH_LEVEL)
Expand All @@ -195,25 +203,10 @@ QuicRecvBufferProvideChunks(
_Inout_ CXPLAT_LIST_ENTRY* /* QUIC_RECV_CHUNKS */ Chunks
)
{
// TODO guhetier: Consider making this valid in external mode only
// and have a reset function to change the mode.
CXPLAT_DBG_ASSERT(RecvBuffer->RecvMode == QUIC_RECV_BUF_MODE_EXTERNAL);
CXPLAT_DBG_ASSERT(!CxPlatListIsEmpty(Chunks));

//
// External chunks can be provided only if already in external mode, or if
// nothing has been written to the buffer yet.
//
uint64_t RangeMax;
if (RecvBuffer->RecvMode != QUIC_RECV_BUF_MODE_EXTERNAL &&
(QuicRangeGetMaxSafe(&RecvBuffer->WrittenRanges, &RangeMax) && RangeMax > 0)) {
return QUIC_STATUS_INVALID_STATE;
}

uint64_t NewBufferLength = RecvBuffer->VirtualBufferLength;
if (RecvBuffer->RecvMode != QUIC_RECV_BUF_MODE_EXTERNAL) {
NewBufferLength = 0;
}

for (CXPLAT_LIST_ENTRY* Link = Chunks->Flink;
Link != Chunks;
Link = Link->Flink) {
Expand All @@ -234,24 +227,6 @@ QuicRecvBufferProvideChunks(
return QUIC_STATUS_INVALID_PARAMETER;
}

if (RecvBuffer->RecvMode != QUIC_RECV_BUF_MODE_EXTERNAL) {
//
// Remove all existing chunks.
//
while (!CxPlatListIsEmpty(&RecvBuffer->Chunks)) {
QUIC_RECV_CHUNK* Chunk =
CXPLAT_CONTAINING_RECORD(
CxPlatListRemoveHead(&RecvBuffer->Chunks),
QUIC_RECV_CHUNK,
Link);
if (Chunk != RecvBuffer->PreallocatedChunk) {
CXPLAT_FREE(Chunk, QUIC_POOL_RECVBUF);
}
}

RecvBuffer->RecvMode = QUIC_RECV_BUF_MODE_EXTERNAL;
}

if (CxPlatListIsEmpty(&RecvBuffer->Chunks)) {
//
// If a new chunk becomes the first chunk, update the capacity.
Expand Down Expand Up @@ -280,7 +255,7 @@ QuicRecvBufferResize(
_In_ uint32_t TargetBufferLength
)
{
CXPLAT_DBG_ASSERT(RecvBuffer->RecvMode != QUIC_RECV_BUF_MODE_EXTERNAL); // Should never resize in external mode
CXPLAT_DBG_ASSERTMSG(RecvBuffer->RecvMode != QUIC_RECV_BUF_MODE_EXTERNAL, "Should never resize in External mode");
CXPLAT_DBG_ASSERT(
TargetBufferLength != 0 &&
(TargetBufferLength & (TargetBufferLength - 1)) == 0); // Power of 2
Expand Down
4 changes: 2 additions & 2 deletions src/core/recv_buffer.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,8 +140,8 @@ QuicRecvBufferHasUnreadData(
// Changes the buffer's virtual buffer length.
//
_IRQL_requires_max_(DISPATCH_LEVEL)
void
QuicRecvBufferIncreaseVirtualBufferLength(
BOOLEAN
QuicRecvBufferTryIncreaseVirtualBufferLength(
_In_ QUIC_RECV_BUFFER* RecvBuffer,
_In_ uint32_t NewLength
);
Expand Down
73 changes: 26 additions & 47 deletions src/core/unittest/RecvBufferTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,28 +48,30 @@ struct RecvBuffer {
printf("Initializing: [mode=%u,vlen=%u,alen=%u]\n", RecvMode, VirtualBufferLength, AllocBufferLength);

auto Result = QUIC_STATUS_SUCCESS;
if (RecvMode == QUIC_RECV_BUF_MODE_EXTERNAL) {
Result = QuicRecvBufferInitialize(&RecvBuf, AllocBufferLength, VirtualBufferLength, RecvMode, PreallocChunk);
if (Result != QUIC_STATUS_SUCCESS) {
return Result;
}

if (RecvMode == QUIC_RECV_BUF_MODE_EXTERNAL && AllocBufferLength > 0) {
//
// If the receive mode is EXTERNAL, simulate an initialization followed by providing external buffers.
// If the receive mode is EXTERNAL, provide external buffers.
// Provide up to two chunks, so that:
// - the first chunk has `AllocBufferLength` bytes
// - the sum of the two is `VirtualBufferLength` bytes
//
Result = QuicRecvBufferInitialize(&RecvBuf, AllocBufferLength, VirtualBufferLength, QUIC_RECV_BUF_MODE_CIRCULAR, PreallocChunk);
if (Result == QUIC_STATUS_SUCCESS) {
ExternalBuffer = (uint8_t *)CXPLAT_ALLOC_NONPAGED(VirtualBufferLength, QUIC_POOL_TEST);
auto Chunk = (QUIC_RECV_CHUNK *)CXPLAT_ALLOC_NONPAGED(sizeof(QUIC_RECV_CHUNK), QUIC_POOL_RECVBUF);
QuicRecvChunkInitialize(Chunk, AllocBufferLength, ExternalBuffer);
CXPLAT_LIST_ENTRY ChunkList;
CxPlatListInitializeHead(&ChunkList);
CxPlatListInsertHead(&ChunkList, &Chunk->Link);
if (VirtualBufferLength > AllocBufferLength) {
auto Chunk2 = (QUIC_RECV_CHUNK *)CXPLAT_ALLOC_NONPAGED(sizeof(QUIC_RECV_CHUNK), QUIC_POOL_RECVBUF);
QuicRecvChunkInitialize(Chunk2, VirtualBufferLength - AllocBufferLength, ExternalBuffer + AllocBufferLength);
CxPlatListInsertTail(&ChunkList, &Chunk2->Link);
}
Result = QuicRecvBufferProvideChunks(&RecvBuf, &ChunkList);
CXPLAT_LIST_ENTRY ChunkList;
CxPlatListInitializeHead(&ChunkList);
ExternalBuffer = (uint8_t *)CXPLAT_ALLOC_NONPAGED(VirtualBufferLength, QUIC_POOL_TEST);
auto Chunk = (QUIC_RECV_CHUNK *)CXPLAT_ALLOC_NONPAGED(sizeof(QUIC_RECV_CHUNK), QUIC_POOL_RECVBUF);
QuicRecvChunkInitialize(Chunk, AllocBufferLength, ExternalBuffer);
CxPlatListInsertHead(&ChunkList, &Chunk->Link);
if (VirtualBufferLength > AllocBufferLength) {
auto Chunk2 = (QUIC_RECV_CHUNK *)CXPLAT_ALLOC_NONPAGED(sizeof(QUIC_RECV_CHUNK), QUIC_POOL_RECVBUF);
QuicRecvChunkInitialize(Chunk2, VirtualBufferLength - AllocBufferLength, ExternalBuffer + AllocBufferLength);
CxPlatListInsertTail(&ChunkList, &Chunk2->Link);
}
Result = QuicRecvBufferProvideChunks(&RecvBuf, &ChunkList);
}
else
{
Expand All @@ -91,7 +93,7 @@ struct RecvBuffer {
return Result;
}
void IncreaseVirtualBufferLength(uint32_t Length) {
QuicRecvBufferIncreaseVirtualBufferLength(&RecvBuf, Length);
QuicRecvBufferTryIncreaseVirtualBufferLength(&RecvBuf, Length);
}
QUIC_STATUS Write(
_In_ uint64_t WriteOffset,
Expand Down Expand Up @@ -1653,10 +1655,10 @@ void FreeChunkList(_Inout_ CXPLAT_LIST_ENTRY* ChunkList) {
}
}

TEST(ExternalBuffersTest, EnableExternalMode)
TEST(ExternalBuffersTest, ProvideChunks)
{
RecvBuffer RecvBuf;
ASSERT_EQ(QUIC_STATUS_SUCCESS, RecvBuf.Initialize());
ASSERT_EQ(QUIC_STATUS_SUCCESS, RecvBuf.Initialize(QUIC_RECV_BUF_MODE_EXTERNAL, false, 0, 0));

//
// Providing external chunks succeeds and change the buffer mode to external.
Expand All @@ -1683,33 +1685,10 @@ TEST(ExternalBuffersTest, EnableExternalMode)
ASSERT_TRUE(CxPlatListIsEmpty(&ChunkList));
}

TEST(ExternalBuffersTest, ProvideChunksInOtherMode)
{
RecvBuffer RecvBuf;
ASSERT_EQ(QUIC_STATUS_SUCCESS, RecvBuf.Initialize());

uint64_t InOutWriteLength = DEF_TEST_BUFFER_LENGTH;
BOOLEAN NewDataReady = FALSE;
RecvBuf.Write(0, 8, &InOutWriteLength, &NewDataReady);

//
// Adding external chunks fails if data has already been written in another
// mode.
//
std::array<uint8_t, 16> Buffer{};
std::vector ChunkSizes{8u, 8u};
CXPLAT_LIST_ENTRY ChunkList;
ASSERT_EQ(QUIC_STATUS_SUCCESS, MakeExternalChunks(ChunkSizes, Buffer.size(), Buffer.data(), &ChunkList));
ASSERT_EQ(QUIC_STATUS_INVALID_STATE, RecvBuf.ProvideChunks(&ChunkList));
ASSERT_FALSE(CxPlatListIsEmpty(&ChunkList));

FreeChunkList(&ChunkList);
}

TEST(ExternalBuffersTest, ProvideChunksOverflow)
{
RecvBuffer RecvBuf;
ASSERT_EQ(QUIC_STATUS_SUCCESS, RecvBuf.Initialize());
ASSERT_EQ(QUIC_STATUS_SUCCESS, RecvBuf.Initialize(QUIC_RECV_BUF_MODE_EXTERNAL, false, 0, 0));

//
// Ensure external buffers cannot be provided in a way that would overflow
Expand Down Expand Up @@ -1740,7 +1719,7 @@ TEST(ExternalBuffersTest, ProvideChunksOverflow)
TEST(ExternalBuffersTest, ReadWriteManyChunks)
{
RecvBuffer RecvBuf;
ASSERT_EQ(QUIC_STATUS_SUCCESS, RecvBuf.Initialize());
ASSERT_EQ(QUIC_STATUS_SUCCESS, RecvBuf.Initialize(QUIC_RECV_BUF_MODE_EXTERNAL, false, 0, 0));

const uint32_t NbChunks = 5;
std::array<uint8_t, NbChunks * 8> Buffer{};
Expand Down Expand Up @@ -1771,7 +1750,7 @@ TEST(ExternalBuffersTest, ReadWriteManyChunks)
TEST(ExternalBuffersTest, WriteTooLong)
{
RecvBuffer RecvBuf;
ASSERT_EQ(QUIC_STATUS_SUCCESS, RecvBuf.Initialize());
ASSERT_EQ(QUIC_STATUS_SUCCESS, RecvBuf.Initialize(QUIC_RECV_BUF_MODE_EXTERNAL, false, 0, 0));

std::array<uint8_t, 16> Buffer{};
std::vector ChunkSizes{8u, 8u};
Expand All @@ -1790,7 +1769,7 @@ TEST(ExternalBuffersTest, WriteTooLong)
TEST(ExternalBuffersTest, OutOfBuffers)
{
RecvBuffer RecvBuf;
ASSERT_EQ(QUIC_STATUS_SUCCESS, RecvBuf.Initialize());
ASSERT_EQ(QUIC_STATUS_SUCCESS, RecvBuf.Initialize(QUIC_RECV_BUF_MODE_EXTERNAL, false, 0, 0));

std::array<uint8_t, DEF_TEST_BUFFER_LENGTH> Buffer{};
std::vector ChunkSizes{DEF_TEST_BUFFER_LENGTH};
Expand Down Expand Up @@ -1839,7 +1818,7 @@ TEST(ExternalBuffersTest, OutOfBuffers)
TEST(ExternalBuffersTest, FreeBufferBeforeDrain)
{
RecvBuffer RecvBuf;
ASSERT_EQ(QUIC_STATUS_SUCCESS, RecvBuf.Initialize());
ASSERT_EQ(QUIC_STATUS_SUCCESS, RecvBuf.Initialize(QUIC_RECV_BUF_MODE_EXTERNAL, false, 0, 0));

auto Buffer1 = std::make_unique<uint8_t[]>(DEF_TEST_BUFFER_LENGTH);
std::array<uint8_t, DEF_TEST_BUFFER_LENGTH> Buffer2{};
Expand Down

0 comments on commit 3ca2f64

Please sign in to comment.