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.
  Stack allocations for large buffer with default size is exceeding
  limits. Disabling the Test file altogether for this platform would
  prevent all tests from running. Instead, only disabling TCP on
  nrfConnect for now, since it is unused.

Fixes Issues #31779, #33307.

Author: pidarped

config/nrfconnect/chip-module/CMakeLists.txt

@@ -129,7 +129,7 @@ matter_add_gn_arg_bool  ("chip_enable_nfc"                        CONFIG_CHIP_NF
 matter_add_gn_arg_bool  ("chip_enable_ota_requestor"              CONFIG_CHIP_OTA_REQUESTOR)
 matter_add_gn_arg_bool  ("chip_persist_subscriptions"             CONFIG_CHIP_PERSISTENT_SUBSCRIPTIONS)
 matter_add_gn_arg_bool  ("chip_monolithic_tests"                  CONFIG_CHIP_BUILD_TESTS)
-matter_add_gn_arg_bool  ("chip_inet_config_enable_tcp_endpoint"   CONFIG_CHIP_BUILD_TESTS)
+matter_add_gn_arg_bool  ("chip_inet_config_enable_tcp_endpoint"   FALSE)
 matter_add_gn_arg_bool  ("chip_error_logging"                     CONFIG_MATTER_LOG_LEVEL GREATER_EQUAL 1)
 matter_add_gn_arg_bool  ("chip_progress_logging"                  CONFIG_MATTER_LOG_LEVEL GREATER_EQUAL 3)
 matter_add_gn_arg_bool  ("chip_detail_logging"                    CONFIG_MATTER_LOG_LEVEL GREATER_EQUAL 4)

src/inet/TCPEndPoint.h

@@ -522,7 +522,7 @@ class DLL_EXPORT TCPEndPoint : public EndPointBasis<TCPEndPoint>
     /**
      * Size of the largest TCP packet that can be received.
      */
-    constexpr static size_t kMaxReceiveMessageSize = System::PacketBuffer::kMaxSizeWithoutReserve;
+    constexpr static size_t kMaxReceiveMessageSize = System::PacketBuffer::kMaxAllocSize;
 
 protected:
     friend class TCPTest;

src/inet/TCPEndPointImplSockets.cpp

@@ -947,10 +947,9 @@ void TCPEndPointImplSockets::ReceiveData()
         {
             VerifyOrDie(rcvLen > 0);
             size_t newDataLength = rcvBuf->DataLength() + static_cast<size_t>(rcvLen);
-            VerifyOrDie(CanCastTo<uint16_t>(newDataLength));
             if (isNewBuf)
             {
-                rcvBuf->SetDataLength(static_cast<uint16_t>(newDataLength));
+                rcvBuf->SetDataLength(newDataLength);
                 rcvBuf.RightSize();
                 if (mRcvQueue.IsNull())
                 {
@@ -963,7 +962,7 @@ void TCPEndPointImplSockets::ReceiveData()
             }
             else
             {
-                rcvBuf->SetDataLength(static_cast<uint16_t>(newDataLength), mRcvQueue);
+                rcvBuf->SetDataLength(newDataLength, mRcvQueue);
             }
         }
     }

src/system/SystemConfig.h

@@ -788,3 +788,21 @@ struct LwIPEvent;
 #define CHIP_SYSTEM_CONFIG_USE_ZEPHYR_EVENTFD 0
 #endif
 #endif // CHIP_SYSTEM_CONFIG_USE_ZEPHYR_EVENTFD
+
+/**
+ * @def CHIP_SYSTEM_CONFIG_MAX_LARGE_BUFFER_SIZE_BYTES
+ *
+ * @brief Maximum buffer allocation size of a 'Large' message
+ *
+ * This is the default maximum capacity (including both data and reserve
+ * space) of a large PacketBuffer(exceeding the IPv6 MTU of 1280 bytes).
+ * This shall be used over transports, such as TCP, that support large
+ * payload transfers. Fetching of large command responses or wildcard
+ * subscription responses may leverage this increased bandwidth transfer.
+ * Individual systems may override this size based on their requirements.
+ * Data transfers over MRP should not be using this size for allocating
+ * buffers as they are restricted by the IPv6 MTU.
+ */
+#ifndef CHIP_SYSTEM_CONFIG_MAX_LARGE_BUFFER_SIZE_BYTES
+#define CHIP_SYSTEM_CONFIG_MAX_LARGE_BUFFER_SIZE_BYTES (64000)
+#endif

src/system/SystemPacketBuffer.cpp

@@ -515,6 +515,21 @@ PacketBufferHandle PacketBufferHandle::New(size_t aAvailableSize, uint16_t aRese
     // Setting a static upper bound on the maximum buffer size allocation for regular sized messages (not large).
     static_assert(PacketBuffer::kMaxSizeWithoutReserve <= UINT16_MAX, "kMaxSizeWithoutReserve should not exceed UINT16_MAX.");
 
+#if INET_CONFIG_ENABLE_TCP_ENDPOINT
+    // Setting a static upper bound on the maximum buffer size allocation for
+    // large messages.
+#if CHIP_SYSTEM_CONFIG_USE_LWIP
+    // LwIP based systems are internally limited to using a u16_t type as the size of a buffer.
+    static_assert(PacketBuffer::kLargeBufMaxSizeWithoutReserve <= UINT16_MAX,
+                  "In LwIP, max size for Large payload buffers cannot exceed UINT16_MAX!");
+#else
+    // Messages over TCP are framed using a length field that is 32 bits in
+    // length.
+    static_assert(PacketBuffer::kLargeBufMaxSizeWithoutReserve <= UINT32_MAX,
+                  "Max size for Large payload buffers cannot exceed UINT32_MAX");
+#endif // CHIP_SYSTEM_CONFIG_USE_LWIP
+#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
+
     // Ensure that aAvailableSize is bound within a max and is not big enough to cause overflow during
     // subsequent addition of all the sizes.
     if (aAvailableSize > UINT32_MAX)
@@ -557,7 +572,7 @@ PacketBufferHandle PacketBufferHandle::New(size_t aAvailableSize, uint16_t aRese
 
     CHIP_SYSTEM_FAULT_INJECT(FaultInjection::kFault_PacketBufferNew, return PacketBufferHandle());
 
-    if (lAllocSize > PacketBuffer::kMaxSizeWithoutReserve)
+    if (lAllocSize > PacketBuffer::kMaxAllocSize)
     {
         ChipLogError(chipSystemLayer, "PacketBuffer: allocation exceeding buffer capacity limits.");
         return PacketBufferHandle();
@@ -621,18 +636,15 @@ PacketBufferHandle PacketBufferHandle::New(size_t aAvailableSize, uint16_t aRese
 PacketBufferHandle PacketBufferHandle::NewWithData(const void * aData, size_t aDataSize, size_t aAdditionalSize,
                                                    uint16_t aReservedSize)
 {
-    if (aDataSize > UINT16_MAX)
-    {
-        ChipLogError(chipSystemLayer, "PacketBuffer: allocation too large.");
-        return PacketBufferHandle();
-    }
     // Since `aDataSize` fits in uint16_t, the sum `aDataSize + aAdditionalSize` will not overflow.
     // `New()` will only return a non-null buffer if the total allocation size does not overflow.
     PacketBufferHandle buffer = New(aDataSize + aAdditionalSize, aReservedSize);
     if (buffer.mBuffer != nullptr)
     {
         memcpy(buffer.mBuffer->payload, aData, aDataSize);
 #if CHIP_SYSTEM_CONFIG_USE_LWIP
+        // The VerifyOrDie() in the New() call catches buffer allocations greater
+        // than UINT16_MAX for LwIP based platforms.
         buffer.mBuffer->len = buffer.mBuffer->tot_len = static_cast<uint16_t>(aDataSize);
 #else
         buffer.mBuffer->len = buffer.mBuffer->tot_len = aDataSize;
@@ -755,18 +767,18 @@ PacketBufferHandle PacketBufferHandle::CloneData() const
         size_t originalDataSize       = original->MaxDataLength();
         uint16_t originalReservedSize = original->ReservedSize();
 
-        if (originalDataSize + originalReservedSize > PacketBuffer::kMaxSizeWithoutReserve)
+        if (originalDataSize + originalReservedSize > PacketBuffer::kMaxAllocSize)
         {
             // The original memory allocation may have provided a larger block than requested (e.g. when using a shared pool),
             // and in particular may have provided a larger block than we are able to request from PackBufferHandle::New().
             // It is a genuine error if that extra space has been used.
-            if (originalReservedSize + original->DataLength() > PacketBuffer::kMaxSizeWithoutReserve)
+            if (originalReservedSize + original->DataLength() > PacketBuffer::kMaxAllocSize)
             {
                 return PacketBufferHandle();
             }
             // Otherwise, reduce the requested data size. This subtraction can not underflow because the above test
-            // guarantees originalReservedSize <= PacketBuffer::kMaxSizeWithoutReserve.
-            originalDataSize = PacketBuffer::kMaxSizeWithoutReserve - originalReservedSize;
+            // guarantees originalReservedSize <= PacketBuffer::kMaxAllocSize.
+            originalDataSize = PacketBuffer::kMaxAllocSize - originalReservedSize;
         }
 
         PacketBufferHandle clone = PacketBufferHandle::New(originalDataSize, originalReservedSize);

src/system/SystemPacketBuffer.h

@@ -119,7 +119,7 @@ class DLL_EXPORT PacketBuffer : private pbuf
 
 public:
     /**
-     * The maximum size buffer an application can allocate with no protocol header reserve.
+     * The maximum size of a regular buffer an application can allocate with no protocol header reserve.
      */
 #if CHIP_SYSTEM_CONFIG_USE_LWIP
     static constexpr size_t kMaxSizeWithoutReserve = LWIP_MEM_ALIGN_SIZE(PBUF_POOL_BUFSIZE);
@@ -134,10 +134,30 @@ class DLL_EXPORT PacketBuffer : private pbuf
     static constexpr uint16_t kDefaultHeaderReserve = CHIP_SYSTEM_CONFIG_HEADER_RESERVE_SIZE;
 
     /**
-     * The maximum size buffer an application can allocate with the default protocol header reserve.
+     * The maximum size of a regular buffer an application can allocate with the default protocol header reserve.
      */
     static constexpr size_t kMaxSize = kMaxSizeWithoutReserve - kDefaultHeaderReserve;
 
+#if INET_CONFIG_ENABLE_TCP_ENDPOINT
+    /**
+     * The maximum size of a large buffer(> IPv6 MTU) that an application can allocate with no protocol header reserve.
+     */
+    static constexpr size_t kLargeBufMaxSizeWithoutReserve = CHIP_SYSTEM_CONFIG_MAX_LARGE_BUFFER_SIZE_BYTES;
+
+    /**
+     * The maximum size of a large buffer(> IPv6 MTU) that an application can allocate with the default protocol header reserve.
+     */
+    static constexpr size_t kLargeBufMaxSize = kLargeBufMaxSizeWithoutReserve - kDefaultHeaderReserve;
+
+    /**
+     * Unified constant(both regular and large buffers) for the maximum size that an application can allocate with no
+     * protocol header reserve.
+     */
+    static constexpr size_t kMaxAllocSize = kLargeBufMaxSizeWithoutReserve;
+#else
+    static constexpr size_t kMaxAllocSize          = kMaxSizeWithoutReserve;
+#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
+
     /**
      * Return the size of the allocation including the reserved and payload data spaces but not including space
      * allocated for the PacketBuffer structure.

src/system/tests/TestSystemPacketBuffer.cpp

@@ -309,7 +309,7 @@ TEST_F_FROM_FIXTURE(TestSystemPacketBuffer, CheckNew)
     {
         const PacketBufferHandle buffer = PacketBufferHandle::New(0, config.reserved_size);
 
-        if (config.reserved_size > PacketBuffer::kMaxSizeWithoutReserve)
+        if (config.reserved_size > PacketBuffer::kMaxAllocSize)
         {
             EXPECT_TRUE(buffer.IsNull());
             continue;
@@ -1605,7 +1605,8 @@ TEST_F_FROM_FIXTURE(TestSystemPacketBuffer, CheckHandleRightSize)
 
 TEST_F_FROM_FIXTURE(TestSystemPacketBuffer, CheckHandleCloneData)
 {
-    uint8_t lPayload[2 * PacketBuffer::kMaxSizeWithoutReserve];
+    uint8_t lPayload[2 * PacketBuffer::kMaxAllocSize];
+
     for (uint8_t & payload : lPayload)
     {
         payload = static_cast<uint8_t>(random());
@@ -1684,7 +1685,7 @@ TEST_F_FROM_FIXTURE(TestSystemPacketBuffer, CheckHandleCloneData)
     // This is only testable on heap allocation configurations, where pbuf records the allocation size and we can manually
     // construct an oversize buffer.
 
-    constexpr uint16_t kOversizeDataSize = PacketBuffer::kMaxSizeWithoutReserve + 99;
+    constexpr size_t kOversizeDataSize = PacketBuffer::kMaxAllocSize + 99;
     PacketBuffer * p = reinterpret_cast<PacketBuffer *>(chip::Platform::MemoryAlloc(kStructureSize + kOversizeDataSize));
     ASSERT_NE(p, nullptr);
 
@@ -1698,15 +1699,16 @@ TEST_F_FROM_FIXTURE(TestSystemPacketBuffer, CheckHandleCloneData)
     PacketBufferHandle handle = PacketBufferHandle::Adopt(p);
 
     // Fill the buffer to maximum and verify that it can be cloned.
+    size_t maxSize = PacketBuffer::kMaxAllocSize;
 
-    memset(handle->Start(), 1, PacketBuffer::kMaxSizeWithoutReserve);
-    handle->SetDataLength(PacketBuffer::kMaxSizeWithoutReserve);
-    EXPECT_EQ(handle->DataLength(), PacketBuffer::kMaxSizeWithoutReserve);
+    memset(handle->Start(), 1, maxSize);
+    handle->SetDataLength(maxSize);
+    EXPECT_EQ(handle->DataLength(), maxSize);
 
     PacketBufferHandle clone = handle.CloneData();
     ASSERT_FALSE(clone.IsNull());
-    EXPECT_EQ(clone->DataLength(), PacketBuffer::kMaxSizeWithoutReserve);
-    EXPECT_EQ(memcmp(handle->Start(), clone->Start(), PacketBuffer::kMaxSizeWithoutReserve), 0);
+    EXPECT_EQ(clone->DataLength(), maxSize);
+    EXPECT_EQ(memcmp(handle->Start(), clone->Start(), maxSize), 0);
 
     // Overfill the buffer and verify that it can not be cloned.
     memset(handle->Start(), 2, kOversizeDataSize);

src/transport/SecureMessageCodec.cpp

@@ -41,7 +41,6 @@ CHIP_ERROR Encrypt(const CryptoContext & context, CryptoContext::ConstNonceView
 {
     VerifyOrReturnError(!msgBuf.IsNull(), CHIP_ERROR_INVALID_ARGUMENT);
     VerifyOrReturnError(!msgBuf->HasChainedBuffer(), CHIP_ERROR_INVALID_MESSAGE_LENGTH);
-    VerifyOrReturnError(msgBuf->TotalLength() <= kMaxAppMessageLen, CHIP_ERROR_MESSAGE_TOO_LONG);
 
     ReturnErrorOnFailure(payloadHeader.EncodeBeforeData(msgBuf));
 

src/transport/SessionManager.cpp

@@ -201,6 +201,17 @@ CHIP_ERROR SessionManager::PrepareMessage(const SessionHandle & sessionHandle, P
         packetHeader.SetSecureSessionControlMsg(true);
     }
 
+    if (sessionHandle->AllowsMRP())
+    {
+        VerifyOrReturnError(message->TotalLength() <= kMaxAppMessageLen, CHIP_ERROR_MESSAGE_TOO_LONG);
+    }
+#if INET_CONFIG_ENABLE_TCP_ENDPOINT
+    else if (sessionHandle->AllowsLargePayload())
+    {
+        VerifyOrReturnError(message->TotalLength() <= kMaxLargeAppMessageLen, CHIP_ERROR_MESSAGE_TOO_LONG);
+    }
+#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
+
 #if CHIP_PROGRESS_LOGGING
     NodeId destination;
     FabricIndex fabricIndex;

src/transport/raw/MessageHeader.h

@@ -60,7 +60,6 @@ static constexpr size_t kMaxPacketBufferApplicationPayloadAndMICSizeBytes = Syst
 
 static constexpr size_t kMaxApplicationPayloadAndMICSizeBytes =
     min(kMaxPerSpecApplicationPayloadAndMICSizeBytes, kMaxPacketBufferApplicationPayloadAndMICSizeBytes);
-
 } // namespace detail
 
 static constexpr size_t kMaxTagLen = 16;
@@ -74,6 +73,18 @@ static constexpr size_t kMaxAppMessageLen = detail::kMaxApplicationPayloadAndMIC
 
 static constexpr uint16_t kMsgUnicastSessionIdUnsecured = 0x0000;
 
+#if INET_CONFIG_ENABLE_TCP_ENDPOINT
+// Minimum header size of TCP + IPv6 without options.
+static constexpr size_t kMaxTCPAndIPHeaderSizeBytes = 60;
+
+// Max space for the Application Payload and MIC for large packet buffers
+// This is the size _excluding_ the header reserve.
+static constexpr size_t kMaxLargeApplicationPayloadAndMICSizeBytes =
+    System::PacketBuffer::kLargeBufMaxSize - kMaxTCPAndIPHeaderSizeBytes;
+
+static constexpr size_t kMaxLargeAppMessageLen = kMaxLargeApplicationPayloadAndMICSizeBytes - kMaxTagLen;
+#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
+
 typedef int PacketHeaderFlags;
 
 namespace Header {

src/transport/raw/TCP.cpp

@@ -39,11 +39,11 @@ namespace {
 
 using namespace chip::Encoding;
 
-// Packets start with a 16-bit size
-constexpr size_t kPacketSizeBytes = 2;
+// Packets start with a 32-bit size field.
+constexpr size_t kPacketSizeBytes = sizeof(uint32_t);
 
-// TODO: Actual limit may be lower (spec issue #2119)
-constexpr uint16_t kMaxMessageSize = static_cast<uint16_t>(System::PacketBuffer::kMaxSizeWithoutReserve - kPacketSizeBytes);
+constexpr uint32_t kMaxTCPMessageSize =
+    static_cast<uint32_t>(System::PacketBuffer::kLargeBufMaxSizeWithoutReserve - kPacketSizeBytes);
 
 constexpr int kListenBacklogSize = 2;
 
@@ -197,12 +197,12 @@ ActiveTCPConnectionState * TCPBase::FindInUseConnection(const Inet::TCPEndPoint
 CHIP_ERROR TCPBase::SendMessage(const Transport::PeerAddress & address, System::PacketBufferHandle && msgBuf)
 {
     // Sent buffer data format is:
-    //    - packet size as a uint16_t
+    //    - packet size as a uint32_t
     //    - actual data
 
     VerifyOrReturnError(address.GetTransportType() == Type::kTcp, CHIP_ERROR_INVALID_ARGUMENT);
     VerifyOrReturnError(mState == TCPState::kInitialized, CHIP_ERROR_INCORRECT_STATE);
-    VerifyOrReturnError(kPacketSizeBytes + msgBuf->DataLength() <= std::numeric_limits<uint16_t>::max(),
+    VerifyOrReturnError(kPacketSizeBytes + msgBuf->DataLength() <= System::PacketBuffer::kLargeBufMaxSizeWithoutReserve,
                         CHIP_ERROR_INVALID_ARGUMENT);
 
     // 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::
     msgBuf->SetStart(msgBuf->Start() - kPacketSizeBytes);
 
     uint8_t * output = msgBuf->Start();
-    LittleEndian::Write16(output, static_cast<uint16_t>(msgBuf->DataLength() - kPacketSizeBytes));
+    LittleEndian::Write32(output, static_cast<uint32_t>(msgBuf->DataLength() - kPacketSizeBytes));
 
     // Reuse existing connection if one exists, otherwise a new one
     // will be established
@@ -324,11 +324,11 @@ CHIP_ERROR TCPBase::ProcessReceivedBuffer(Inet::TCPEndPoint * endPoint, const Pe
         {
             return err;
         }
-        uint16_t messageSize = LittleEndian::Get16(messageSizeBuf);
-        if (messageSize >= kMaxMessageSize)
+        uint32_t messageSize = LittleEndian::Get32(messageSizeBuf);
+        if (messageSize >= kMaxTCPMessageSize)
         {
-
-            // This message is too long for upper layers.
+            // Message is too big for node to process. Disconnect with peer.
+            CloseConnectionInternal(state, CHIP_ERROR_MESSAGE_TOO_LONG, SuppressCallback::No);
             return CHIP_ERROR_MESSAGE_TOO_LONG;
         }
         // The subtraction will not underflow because we successfully read kPacketSizeBytes.
@@ -344,7 +344,7 @@ CHIP_ERROR TCPBase::ProcessReceivedBuffer(Inet::TCPEndPoint * endPoint, const Pe
     return CHIP_NO_ERROR;
 }
 
-CHIP_ERROR TCPBase::ProcessSingleMessage(const PeerAddress & peerAddress, ActiveTCPConnectionState * state, uint16_t messageSize)
+CHIP_ERROR TCPBase::ProcessSingleMessage(const PeerAddress & peerAddress, ActiveTCPConnectionState * state, size_t messageSize)
 {
     // We enter with `state->mReceived` containing at least one full message, perhaps in a chain.
     // `state->mReceived->Start()` currently points to the message data.

src/transport/raw/TCP.h

@@ -258,7 +258,7 @@ class DLL_EXPORT TCPBase : public Base
      *                              is no other data).
      * @param[in]     messageSize   Size of the single message.
      */
-    CHIP_ERROR ProcessSingleMessage(const PeerAddress & peerAddress, ActiveTCPConnectionState * state, uint16_t messageSize);
+    CHIP_ERROR ProcessSingleMessage(const PeerAddress & peerAddress, ActiveTCPConnectionState * state, size_t messageSize);
 
     /**
      * Initiate a connection to the given peer. On connection completion,
@@ -306,10 +306,6 @@ class DLL_EXPORT TCPBase : public Base
     // giving up.
     uint32_t mConnectTimeout = CHIP_CONFIG_TCP_CONNECT_TIMEOUT_MSECS;
 
-    // The max payload size of data over a TCP connection that is transmissible
-    // at a time.
-    uint32_t mMaxTCPPayloadSize = CHIP_CONFIG_MAX_TCP_PAYLOAD_SIZE_BYTES;
-
     // Number of active and 'pending connection' endpoints
     size_t mUsedEndPointCount = 0;
 

src/transport/raw/TCPConfig.h

@@ -63,15 +63,6 @@ namespace chip {
 #define CHIP_CONFIG_MAX_TCP_PENDING_PACKETS 4
 #endif
 
-/**
- * @def CHIP_CONFIG_MAX_TCP_PAYLOAD_SIZE_BYTES
- *
- * @brief Maximum payload size of a message over a TCP connection
- */
-#ifndef CHIP_CONFIG_MAX_TCP_PAYLOAD_SIZE_BYTES
-#define CHIP_CONFIG_MAX_TCP_PAYLOAD_SIZE_BYTES 1000000
-#endif
-
 /**
  *  @def CHIP_CONFIG_TCP_CONNECT_TIMEOUT_MSECS
  *