Skip to content

Commit c0b2693

Browse files
committed
Changes for large Packetbuffer allocation to support TCP payloads
Changes to internal checks in SystemPacketBuffer. Update the length encoding for TCP payloads during send and receive. Max size config for large packetbuffer size limit in SystemPacketBuffer.h. Define Max application payload size for large buffers Test modifications for chainedbuffer receives for TCP. - Add test for Buffer length greater than MRP max size. - Disable TCP on nrfconnect platform because of resource requirements. Fixes Issues #31779, #33307.
1 parent 5656870 commit c0b2693

14 files changed

+127
-69
lines changed

config/nrfconnect/chip-module/CMakeLists.txt

+1-1
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ matter_add_gn_arg_bool ("chip_enable_nfc" CONFIG_CHIP_NF
129129
matter_add_gn_arg_bool ("chip_enable_ota_requestor" CONFIG_CHIP_OTA_REQUESTOR)
130130
matter_add_gn_arg_bool ("chip_persist_subscriptions" CONFIG_CHIP_PERSISTENT_SUBSCRIPTIONS)
131131
matter_add_gn_arg_bool ("chip_monolithic_tests" CONFIG_CHIP_BUILD_TESTS)
132-
matter_add_gn_arg_bool ("chip_inet_config_enable_tcp_endpoint" CONFIG_CHIP_BUILD_TESTS)
132+
matter_add_gn_arg_bool ("chip_inet_config_enable_tcp_endpoint" FALSE)
133133
matter_add_gn_arg_bool ("chip_error_logging" CONFIG_MATTER_LOG_LEVEL GREATER_EQUAL 1)
134134
matter_add_gn_arg_bool ("chip_progress_logging" CONFIG_MATTER_LOG_LEVEL GREATER_EQUAL 3)
135135
matter_add_gn_arg_bool ("chip_detail_logging" CONFIG_MATTER_LOG_LEVEL GREATER_EQUAL 4)

src/inet/TCPEndPoint.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -522,7 +522,7 @@ class DLL_EXPORT TCPEndPoint : public EndPointBasis<TCPEndPoint>
522522
/**
523523
* Size of the largest TCP packet that can be received.
524524
*/
525-
constexpr static size_t kMaxReceiveMessageSize = System::PacketBuffer::kMaxSizeWithoutReserve;
525+
constexpr static size_t kMaxReceiveMessageSize = System::PacketBuffer::kMaxAllocSize;
526526

527527
protected:
528528
friend class TCPTest;

src/inet/TCPEndPointImplSockets.cpp

+2-3
Original file line numberDiff line numberDiff line change
@@ -947,10 +947,9 @@ void TCPEndPointImplSockets::ReceiveData()
947947
{
948948
VerifyOrDie(rcvLen > 0);
949949
size_t newDataLength = rcvBuf->DataLength() + static_cast<size_t>(rcvLen);
950-
VerifyOrDie(CanCastTo<uint16_t>(newDataLength));
951950
if (isNewBuf)
952951
{
953-
rcvBuf->SetDataLength(static_cast<uint16_t>(newDataLength));
952+
rcvBuf->SetDataLength(newDataLength);
954953
rcvBuf.RightSize();
955954
if (mRcvQueue.IsNull())
956955
{
@@ -963,7 +962,7 @@ void TCPEndPointImplSockets::ReceiveData()
963962
}
964963
else
965964
{
966-
rcvBuf->SetDataLength(static_cast<uint16_t>(newDataLength), mRcvQueue);
965+
rcvBuf->SetDataLength(newDataLength, mRcvQueue);
967966
}
968967
}
969968
}

src/system/SystemConfig.h

+9
Original file line numberDiff line numberDiff line change
@@ -788,3 +788,12 @@ struct LwIPEvent;
788788
#define CHIP_SYSTEM_CONFIG_USE_ZEPHYR_EVENTFD 0
789789
#endif
790790
#endif // CHIP_SYSTEM_CONFIG_USE_ZEPHYR_EVENTFD
791+
792+
/**
793+
* @def CHIP_SYSTEM_CONFIG_MAX_LARGE_BUFFER_SIZE_BYTES
794+
*
795+
* @brief Maximum buffer allocation size of a 'Large' message
796+
*/
797+
#ifndef CHIP_SYSTEM_CONFIG_MAX_LARGE_BUFFER_SIZE_BYTES
798+
#define CHIP_SYSTEM_CONFIG_MAX_LARGE_BUFFER_SIZE_BYTES (64000)
799+
#endif

src/system/SystemPacketBuffer.cpp

+26-10
Original file line numberDiff line numberDiff line change
@@ -515,6 +515,23 @@ PacketBufferHandle PacketBufferHandle::New(size_t aAvailableSize, uint16_t aRese
515515
// Setting a static upper bound on the maximum buffer size allocation for regular sized messages (not large).
516516
static_assert(PacketBuffer::kMaxSizeWithoutReserve <= UINT16_MAX, "kMaxSizeWithoutReserve should not exceed UINT16_MAX.");
517517

518+
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
519+
// Setting a static upper bound on the maximum buffer size allocation for
520+
// large messages.
521+
#if CHIP_SYSTEM_CONFIG_USE_LWIP
522+
static_assert(PacketBuffer::kLargeBufMaxSizeWithoutReserve <= UINT16_MAX,
523+
"In LwIP, max size for Large payload buffers cannot exceed UINT16_MAX!");
524+
#else
525+
static_assert(PacketBuffer::kLargeBufMaxSizeWithoutReserve <= UINT32_MAX,
526+
"Max size for Large payload buffers cannot exceed UINT32_MAX");
527+
#endif // CHIP_SYSTEM_CONFIG_USE_LWIP
528+
529+
// Ensure that the definition of the max buffer allocation for regular
530+
// messages is less than that for large ones.
531+
static_assert(PacketBuffer::kMaxSizeWithoutReserve < PacketBuffer::kLargeBufMaxSizeWithoutReserve,
532+
"Large buffer configuration should be greater than the conventional buffer limit");
533+
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
534+
518535
// Ensure that aAvailableSize is bound within a max and is not big enough to cause overflow during
519536
// subsequent addition of all the sizes.
520537
if (aAvailableSize > UINT32_MAX)
@@ -557,7 +574,7 @@ PacketBufferHandle PacketBufferHandle::New(size_t aAvailableSize, uint16_t aRese
557574

558575
CHIP_SYSTEM_FAULT_INJECT(FaultInjection::kFault_PacketBufferNew, return PacketBufferHandle());
559576

560-
if (lAllocSize > PacketBuffer::kMaxSizeWithoutReserve)
577+
if (lAllocSize > PacketBuffer::kMaxAllocSize)
561578
{
562579
ChipLogError(chipSystemLayer, "PacketBuffer: allocation exceeding buffer capacity limits.");
563580
return PacketBufferHandle();
@@ -621,18 +638,15 @@ PacketBufferHandle PacketBufferHandle::New(size_t aAvailableSize, uint16_t aRese
621638
PacketBufferHandle PacketBufferHandle::NewWithData(const void * aData, size_t aDataSize, size_t aAdditionalSize,
622639
uint16_t aReservedSize)
623640
{
624-
if (aDataSize > UINT16_MAX)
625-
{
626-
ChipLogError(chipSystemLayer, "PacketBuffer: allocation too large.");
627-
return PacketBufferHandle();
628-
}
629641
// Since `aDataSize` fits in uint16_t, the sum `aDataSize + aAdditionalSize` will not overflow.
630642
// `New()` will only return a non-null buffer if the total allocation size does not overflow.
631643
PacketBufferHandle buffer = New(aDataSize + aAdditionalSize, aReservedSize);
632644
if (buffer.mBuffer != nullptr)
633645
{
634646
memcpy(buffer.mBuffer->payload, aData, aDataSize);
635647
#if CHIP_SYSTEM_CONFIG_USE_LWIP
648+
// The VerifyOrDie() in the New() call catches buffer allocations greater
649+
// than UINT16_MAX for LwIP based platforms.
636650
buffer.mBuffer->len = buffer.mBuffer->tot_len = static_cast<uint16_t>(aDataSize);
637651
#else
638652
buffer.mBuffer->len = buffer.mBuffer->tot_len = aDataSize;
@@ -755,18 +769,20 @@ PacketBufferHandle PacketBufferHandle::CloneData() const
755769
size_t originalDataSize = original->MaxDataLength();
756770
uint16_t originalReservedSize = original->ReservedSize();
757771

758-
if (originalDataSize + originalReservedSize > PacketBuffer::kMaxSizeWithoutReserve)
772+
uint32_t maxSize = PacketBuffer::kMaxAllocSize;
773+
774+
if (originalDataSize + originalReservedSize > maxSize)
759775
{
760776
// The original memory allocation may have provided a larger block than requested (e.g. when using a shared pool),
761777
// and in particular may have provided a larger block than we are able to request from PackBufferHandle::New().
762778
// It is a genuine error if that extra space has been used.
763-
if (originalReservedSize + original->DataLength() > PacketBuffer::kMaxSizeWithoutReserve)
779+
if (originalReservedSize + original->DataLength() > maxSize)
764780
{
765781
return PacketBufferHandle();
766782
}
767783
// Otherwise, reduce the requested data size. This subtraction can not underflow because the above test
768-
// guarantees originalReservedSize <= PacketBuffer::kMaxSizeWithoutReserve.
769-
originalDataSize = PacketBuffer::kMaxSizeWithoutReserve - originalReservedSize;
784+
// guarantees originalReservedSize <= maxSize.
785+
originalDataSize = maxSize - originalReservedSize;
770786
}
771787

772788
PacketBufferHandle clone = PacketBufferHandle::New(originalDataSize, originalReservedSize);

src/system/SystemPacketBuffer.h

+12
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,18 @@ class DLL_EXPORT PacketBuffer : private pbuf
138138
*/
139139
static constexpr size_t kMaxSize = kMaxSizeWithoutReserve - kDefaultHeaderReserve;
140140

141+
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
142+
static constexpr size_t kLargeBufMaxSizeWithoutReserve = CHIP_SYSTEM_CONFIG_MAX_LARGE_BUFFER_SIZE_BYTES;
143+
144+
static constexpr size_t kLargeBufMaxSize = kLargeBufMaxSizeWithoutReserve - kDefaultHeaderReserve;
145+
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
146+
147+
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
148+
static constexpr size_t kMaxAllocSize = kLargeBufMaxSizeWithoutReserve;
149+
#else
150+
static constexpr size_t kMaxAllocSize = kMaxSizeWithoutReserve;
151+
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
152+
141153
/**
142154
* Return the size of the allocation including the reserved and payload data spaces but not including space
143155
* allocated for the PacketBuffer structure.

src/system/tests/TestSystemPacketBuffer.cpp

+10-8
Original file line numberDiff line numberDiff line change
@@ -309,7 +309,7 @@ TEST_F_FROM_FIXTURE(TestSystemPacketBuffer, CheckNew)
309309
{
310310
const PacketBufferHandle buffer = PacketBufferHandle::New(0, config.reserved_size);
311311

312-
if (config.reserved_size > PacketBuffer::kMaxSizeWithoutReserve)
312+
if (config.reserved_size > PacketBuffer::kMaxAllocSize)
313313
{
314314
EXPECT_TRUE(buffer.IsNull());
315315
continue;
@@ -1605,7 +1605,8 @@ TEST_F_FROM_FIXTURE(TestSystemPacketBuffer, CheckHandleRightSize)
16051605

16061606
TEST_F_FROM_FIXTURE(TestSystemPacketBuffer, CheckHandleCloneData)
16071607
{
1608-
uint8_t lPayload[2 * PacketBuffer::kMaxSizeWithoutReserve];
1608+
uint8_t lPayload[2 * PacketBuffer::kMaxAllocSize];
1609+
16091610
for (uint8_t & payload : lPayload)
16101611
{
16111612
payload = static_cast<uint8_t>(random());
@@ -1684,7 +1685,7 @@ TEST_F_FROM_FIXTURE(TestSystemPacketBuffer, CheckHandleCloneData)
16841685
// This is only testable on heap allocation configurations, where pbuf records the allocation size and we can manually
16851686
// construct an oversize buffer.
16861687

1687-
constexpr uint16_t kOversizeDataSize = PacketBuffer::kMaxSizeWithoutReserve + 99;
1688+
constexpr size_t kOversizeDataSize = PacketBuffer::kMaxAllocSize + 99;
16881689
PacketBuffer * p = reinterpret_cast<PacketBuffer *>(chip::Platform::MemoryAlloc(kStructureSize + kOversizeDataSize));
16891690
ASSERT_NE(p, nullptr);
16901691

@@ -1698,15 +1699,16 @@ TEST_F_FROM_FIXTURE(TestSystemPacketBuffer, CheckHandleCloneData)
16981699
PacketBufferHandle handle = PacketBufferHandle::Adopt(p);
16991700

17001701
// Fill the buffer to maximum and verify that it can be cloned.
1702+
size_t maxSize = PacketBuffer::kMaxAllocSize;
17011703

1702-
memset(handle->Start(), 1, PacketBuffer::kMaxSizeWithoutReserve);
1703-
handle->SetDataLength(PacketBuffer::kMaxSizeWithoutReserve);
1704-
EXPECT_EQ(handle->DataLength(), PacketBuffer::kMaxSizeWithoutReserve);
1704+
memset(handle->Start(), 1, maxSize);
1705+
handle->SetDataLength(maxSize);
1706+
EXPECT_EQ(handle->DataLength(), maxSize);
17051707

17061708
PacketBufferHandle clone = handle.CloneData();
17071709
ASSERT_FALSE(clone.IsNull());
1708-
EXPECT_EQ(clone->DataLength(), PacketBuffer::kMaxSizeWithoutReserve);
1709-
EXPECT_EQ(memcmp(handle->Start(), clone->Start(), PacketBuffer::kMaxSizeWithoutReserve), 0);
1710+
EXPECT_EQ(clone->DataLength(), maxSize);
1711+
EXPECT_EQ(memcmp(handle->Start(), clone->Start(), maxSize), 0);
17101712

17111713
// Overfill the buffer and verify that it can not be cloned.
17121714
memset(handle->Start(), 2, kOversizeDataSize);

src/transport/SecureMessageCodec.cpp

-1
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@ CHIP_ERROR Encrypt(const CryptoContext & context, CryptoContext::ConstNonceView
4141
{
4242
VerifyOrReturnError(!msgBuf.IsNull(), CHIP_ERROR_INVALID_ARGUMENT);
4343
VerifyOrReturnError(!msgBuf->HasChainedBuffer(), CHIP_ERROR_INVALID_MESSAGE_LENGTH);
44-
VerifyOrReturnError(msgBuf->TotalLength() <= kMaxAppMessageLen, CHIP_ERROR_MESSAGE_TOO_LONG);
4544

4645
ReturnErrorOnFailure(payloadHeader.EncodeBeforeData(msgBuf));
4746

src/transport/SessionManager.cpp

+11
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,17 @@ CHIP_ERROR SessionManager::PrepareMessage(const SessionHandle & sessionHandle, P
201201
packetHeader.SetSecureSessionControlMsg(true);
202202
}
203203

204+
if (sessionHandle->AllowsMRP())
205+
{
206+
VerifyOrReturnError(message->TotalLength() <= kMaxAppMessageLen, CHIP_ERROR_MESSAGE_TOO_LONG);
207+
}
208+
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
209+
else if (sessionHandle->AllowsLargePayload())
210+
{
211+
VerifyOrReturnError(message->TotalLength() <= kMaxLargeAppMessageLen, CHIP_ERROR_MESSAGE_TOO_LONG);
212+
}
213+
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
214+
204215
#if CHIP_PROGRESS_LOGGING
205216
NodeId destination;
206217
FabricIndex fabricIndex;

src/transport/raw/MessageHeader.h

+12-1
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,6 @@ static constexpr size_t kMaxPacketBufferApplicationPayloadAndMICSizeBytes = Syst
6060

6161
static constexpr size_t kMaxApplicationPayloadAndMICSizeBytes =
6262
min(kMaxPerSpecApplicationPayloadAndMICSizeBytes, kMaxPacketBufferApplicationPayloadAndMICSizeBytes);
63-
6463
} // namespace detail
6564

6665
static constexpr size_t kMaxTagLen = 16;
@@ -74,6 +73,18 @@ static constexpr size_t kMaxAppMessageLen = detail::kMaxApplicationPayloadAndMIC
7473

7574
static constexpr uint16_t kMsgUnicastSessionIdUnsecured = 0x0000;
7675

76+
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
77+
// Minimum header size of TCP + IPv6 without options.
78+
static constexpr size_t kMaxTCPAndIPHeaderSizeBytes = 60;
79+
80+
// Max space for the Application Payload and MIC for large packet buffers
81+
// This is the size _excluding_ the header reserve.
82+
static constexpr size_t kMaxLargeApplicationPayloadAndMICSizeBytes =
83+
System::PacketBuffer::kLargeBufMaxSize - kMaxTCPAndIPHeaderSizeBytes;
84+
85+
static constexpr size_t kMaxLargeAppMessageLen = kMaxLargeApplicationPayloadAndMICSizeBytes - kMaxTagLen;
86+
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
87+
7788
typedef int PacketHeaderFlags;
7889

7990
namespace Header {

src/transport/raw/TCP.cpp

+12-12
Original file line numberDiff line numberDiff line change
@@ -39,11 +39,11 @@ namespace {
3939

4040
using namespace chip::Encoding;
4141

42-
// Packets start with a 16-bit size
43-
constexpr size_t kPacketSizeBytes = 2;
42+
// Packets start with a 32-bit size field.
43+
constexpr size_t kPacketSizeBytes = sizeof(uint32_t);
4444

45-
// TODO: Actual limit may be lower (spec issue #2119)
46-
constexpr uint16_t kMaxMessageSize = static_cast<uint16_t>(System::PacketBuffer::kMaxSizeWithoutReserve - kPacketSizeBytes);
45+
constexpr uint32_t kMaxTCPMessageSize =
46+
static_cast<uint32_t>(System::PacketBuffer::kLargeBufMaxSizeWithoutReserve - kPacketSizeBytes);
4747

4848
constexpr int kListenBacklogSize = 2;
4949

@@ -197,12 +197,12 @@ ActiveTCPConnectionState * TCPBase::FindInUseConnection(const Inet::TCPEndPoint
197197
CHIP_ERROR TCPBase::SendMessage(const Transport::PeerAddress & address, System::PacketBufferHandle && msgBuf)
198198
{
199199
// Sent buffer data format is:
200-
// - packet size as a uint16_t
200+
// - packet size as a uint32_t
201201
// - actual data
202202

203203
VerifyOrReturnError(address.GetTransportType() == Type::kTcp, CHIP_ERROR_INVALID_ARGUMENT);
204204
VerifyOrReturnError(mState == TCPState::kInitialized, CHIP_ERROR_INCORRECT_STATE);
205-
VerifyOrReturnError(kPacketSizeBytes + msgBuf->DataLength() <= std::numeric_limits<uint16_t>::max(),
205+
VerifyOrReturnError(kPacketSizeBytes + msgBuf->DataLength() <= System::PacketBuffer::kLargeBufMaxSizeWithoutReserve,
206206
CHIP_ERROR_INVALID_ARGUMENT);
207207

208208
// The check above about kPacketSizeBytes + msgBuf->DataLength() means it definitely fits in uint16_t.
@@ -211,7 +211,7 @@ CHIP_ERROR TCPBase::SendMessage(const Transport::PeerAddress & address, System::
211211
msgBuf->SetStart(msgBuf->Start() - kPacketSizeBytes);
212212

213213
uint8_t * output = msgBuf->Start();
214-
LittleEndian::Write16(output, static_cast<uint16_t>(msgBuf->DataLength() - kPacketSizeBytes));
214+
LittleEndian::Write32(output, static_cast<uint32_t>(msgBuf->DataLength() - kPacketSizeBytes));
215215

216216
// Reuse existing connection if one exists, otherwise a new one
217217
// will be established
@@ -324,11 +324,11 @@ CHIP_ERROR TCPBase::ProcessReceivedBuffer(Inet::TCPEndPoint * endPoint, const Pe
324324
{
325325
return err;
326326
}
327-
uint16_t messageSize = LittleEndian::Get16(messageSizeBuf);
328-
if (messageSize >= kMaxMessageSize)
327+
uint32_t messageSize = LittleEndian::Get32(messageSizeBuf);
328+
if (messageSize >= kMaxTCPMessageSize)
329329
{
330-
331-
// This message is too long for upper layers.
330+
// Message is too big for node to process. Disconnect with peer.
331+
CloseConnectionInternal(state, CHIP_ERROR_MESSAGE_TOO_LONG, SuppressCallback::No);
332332
return CHIP_ERROR_MESSAGE_TOO_LONG;
333333
}
334334
// The subtraction will not underflow because we successfully read kPacketSizeBytes.
@@ -344,7 +344,7 @@ CHIP_ERROR TCPBase::ProcessReceivedBuffer(Inet::TCPEndPoint * endPoint, const Pe
344344
return CHIP_NO_ERROR;
345345
}
346346

347-
CHIP_ERROR TCPBase::ProcessSingleMessage(const PeerAddress & peerAddress, ActiveTCPConnectionState * state, uint16_t messageSize)
347+
CHIP_ERROR TCPBase::ProcessSingleMessage(const PeerAddress & peerAddress, ActiveTCPConnectionState * state, size_t messageSize)
348348
{
349349
// We enter with `state->mReceived` containing at least one full message, perhaps in a chain.
350350
// `state->mReceived->Start()` currently points to the message data.

src/transport/raw/TCP.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -258,7 +258,7 @@ class DLL_EXPORT TCPBase : public Base
258258
* is no other data).
259259
* @param[in] messageSize Size of the single message.
260260
*/
261-
CHIP_ERROR ProcessSingleMessage(const PeerAddress & peerAddress, ActiveTCPConnectionState * state, uint16_t messageSize);
261+
CHIP_ERROR ProcessSingleMessage(const PeerAddress & peerAddress, ActiveTCPConnectionState * state, size_t messageSize);
262262

263263
/**
264264
* Initiate a connection to the given peer. On connection completion,
@@ -308,7 +308,7 @@ class DLL_EXPORT TCPBase : public Base
308308

309309
// The max payload size of data over a TCP connection that is transmissible
310310
// at a time.
311-
uint32_t mMaxTCPPayloadSize = CHIP_CONFIG_MAX_TCP_PAYLOAD_SIZE_BYTES;
311+
uint32_t mMaxTCPSendAllocSize = System::PacketBuffer::kMaxAllocSize;
312312

313313
// Number of active and 'pending connection' endpoints
314314
size_t mUsedEndPointCount = 0;

src/transport/raw/TCPConfig.h

-9
Original file line numberDiff line numberDiff line change
@@ -63,15 +63,6 @@ namespace chip {
6363
#define CHIP_CONFIG_MAX_TCP_PENDING_PACKETS 4
6464
#endif
6565

66-
/**
67-
* @def CHIP_CONFIG_MAX_TCP_PAYLOAD_SIZE_BYTES
68-
*
69-
* @brief Maximum payload size of a message over a TCP connection
70-
*/
71-
#ifndef CHIP_CONFIG_MAX_TCP_PAYLOAD_SIZE_BYTES
72-
#define CHIP_CONFIG_MAX_TCP_PAYLOAD_SIZE_BYTES 1000000
73-
#endif
74-
7566
/**
7667
* @def CHIP_CONFIG_TCP_CONNECT_TIMEOUT_MSECS
7768
*

0 commit comments

Comments
 (0)