Bring PR #31776(Adjust storage for large payloads) from tcp branch to…

… master (#33001)

* Adjust storage and processing in SystemPacketBuffer (#31776)

and associated code for large payloads.

Extend uint16_t type variables to size_t for the APIs
and all applicable places.

* Address review comments

src/app/BufferedReadCallback.cpp

@@ -65,7 +65,7 @@ CHIP_ERROR BufferedReadCallback::GenerateListTLV(TLV::ScopedBufferTLVReader & aR
     //
     // To avoid that, a single contiguous buffer is the best likely approach for now.
     //
-    uint32_t totalBufSize = 0;
+    size_t totalBufSize = 0;
     for (const auto & packetBuffer : mBufferedList)
     {
         totalBufSize += packetBuffer->TotalLength();

src/app/server/EchoHandler.cpp

@@ -40,7 +40,8 @@ chip::Protocols::Echo::EchoServer gEchoServer;
  */
 void HandleEchoRequestReceived(chip::Messaging::ExchangeContext * ec, chip::System::PacketBufferHandle && payload)
 {
-    ChipLogProgress(AppServer, "Echo Request, len=%u ... sending response.\n", payload->DataLength());
+    ChipLogProgress(AppServer, "Echo Request, len=%" PRIu32 "... sending response.\n",
+                    static_cast<uint32_t>(payload->DataLength()));
 }
 
 } // namespace

src/ble/BtpEngine.cpp

@@ -34,6 +34,7 @@
 #include <lib/support/BitFlags.h>
 #include <lib/support/BufferReader.h>
 #include <lib/support/CodeUtils.h>
+#include <lib/support/SafeInt.h>
 #include <lib/support/logging/CHIPLogging.h>
 #include <system/SystemPacketBuffer.h>
 
@@ -282,7 +283,7 @@ CHIP_ERROR BtpEngine::HandleCharacteristicReceived(System::PacketBufferHandle &&
         // mRxFragmentSize may be smaller than the characteristic size.  Make sure
         // we're not truncating to a data length smaller than what we have already consumed.
         VerifyOrExit(reader.OctetsRead() <= mRxFragmentSize, err = BLE_ERROR_REASSEMBLER_INCORRECT_STATE);
-        data->SetDataLength(chip::min(data->DataLength(), mRxFragmentSize));
+        data->SetDataLength(chip::min(data->DataLength(), static_cast<size_t>(mRxFragmentSize)));
 
         // Now mark the bytes we consumed as consumed.
         data->ConsumeHead(static_cast<uint16_t>(reader.OctetsRead()));
@@ -346,11 +347,12 @@ CHIP_ERROR BtpEngine::HandleCharacteristicReceived(System::PacketBufferHandle &&
     if (rx_flags.Has(HeaderFlags::kEndMessage))
     {
         // Trim remainder, if any, of the received packet buffer based on sender-specified length of reassembled message.
-        int padding = mRxBuf->DataLength() - mRxLength;
+        VerifyOrExit(CanCastTo<uint16_t>(mRxBuf->DataLength()), err = CHIP_ERROR_MESSAGE_TOO_LONG);
+        int padding = static_cast<uint16_t>(mRxBuf->DataLength()) - mRxLength;
 
         if (padding > 0)
         {
-            mRxBuf->SetDataLength(mRxLength);
+            mRxBuf->SetDataLength(static_cast<size_t>(mRxLength));
         }
 
         // Ensure all received fragments add up to sender-specified total message size.
@@ -375,7 +377,7 @@ CHIP_ERROR BtpEngine::HandleCharacteristicReceived(System::PacketBufferHandle &&
         }
         if (!mRxBuf.IsNull())
         {
-            ChipLogError(Ble, "With rx buf data length = %u", mRxBuf->DataLength());
+            ChipLogError(Ble, "With rx buf data length = %u", static_cast<unsigned>(mRxBuf->DataLength()));
         }
         LogState();
 
@@ -426,9 +428,10 @@ bool BtpEngine::HandleCharacteristicSend(System::PacketBufferHandle data, bool s
             return false;
         }
 
-        mTxBuf    = std::move(data);
-        mTxState  = kState_InProgress;
-        mTxLength = mTxBuf->DataLength();
+        mTxBuf   = std::move(data);
+        mTxState = kState_InProgress;
+        VerifyOrReturnError(CanCastTo<uint16_t>(mTxBuf->DataLength()), false);
+        mTxLength = static_cast<uint16_t>(mTxBuf->DataLength());
 
         ChipLogDebugBtpEngine(Ble, ">>> CHIPoBle preparing to send whole message:");
         PrintBufDebug(mTxBuf);

src/ble/tests/TestBtpEngine.cpp

@@ -67,7 +67,7 @@ TEST_F(TestBtpEngine, HandleCharacteristicReceivedOnePacket)
     };
 
     auto packet0 = System::PacketBufferHandle::NewWithData(packetData0, sizeof(packetData0));
-    EXPECT_EQ(packet0->DataLength(), 5);
+    EXPECT_EQ(packet0->DataLength(), static_cast<size_t>(5));
 
     SequenceNumber_t receivedAck;
     bool didReceiveAck;
@@ -81,15 +81,15 @@ TEST_F(TestBtpEngine, HandleCharacteristicReceivedTwoPacket)
     constexpr uint8_t packetData1[] = { to_underlying(BtpEngine::HeaderFlags::kEndMessage), 0x02, 0xff };
 
     auto packet0 = System::PacketBufferHandle::NewWithData(packetData0, sizeof(packetData0));
-    EXPECT_EQ(packet0->DataLength(), 5);
+    EXPECT_EQ(packet0->DataLength(), static_cast<size_t>(5));
 
     SequenceNumber_t receivedAck;
     bool didReceiveAck;
     EXPECT_EQ(mBtpEngine.HandleCharacteristicReceived(std::move(packet0), receivedAck, didReceiveAck), CHIP_NO_ERROR);
     EXPECT_EQ(mBtpEngine.RxState(), BtpEngine::kState_InProgress);
 
     auto packet1 = System::PacketBufferHandle::NewWithData(packetData1, sizeof(packetData1));
-    EXPECT_EQ(packet1->DataLength(), 3);
+    EXPECT_EQ(packet1->DataLength(), static_cast<size_t>(3));
 
     EXPECT_EQ(mBtpEngine.HandleCharacteristicReceived(std::move(packet1), receivedAck, didReceiveAck), CHIP_NO_ERROR);
     EXPECT_EQ(mBtpEngine.RxState(), BtpEngine::kState_Complete);
@@ -102,21 +102,21 @@ TEST_F(TestBtpEngine, HandleCharacteristicReceivedThreePacket)
     constexpr uint8_t packetData2[] = { to_underlying(BtpEngine::HeaderFlags::kEndMessage), 0x03, 0xff };
 
     auto packet0 = System::PacketBufferHandle::NewWithData(packetData0, sizeof(packetData0));
-    EXPECT_EQ(packet0->DataLength(), 5);
+    EXPECT_EQ(packet0->DataLength(), static_cast<size_t>(5));
 
     SequenceNumber_t receivedAck;
     bool didReceiveAck;
     EXPECT_EQ(mBtpEngine.HandleCharacteristicReceived(std::move(packet0), receivedAck, didReceiveAck), CHIP_NO_ERROR);
     EXPECT_EQ(mBtpEngine.RxState(), BtpEngine::kState_InProgress);
 
     auto packet1 = System::PacketBufferHandle::NewWithData(packetData1, sizeof(packetData1));
-    EXPECT_EQ(packet1->DataLength(), 3);
+    EXPECT_EQ(packet1->DataLength(), static_cast<size_t>(3));
 
     EXPECT_EQ(mBtpEngine.HandleCharacteristicReceived(std::move(packet1), receivedAck, didReceiveAck), CHIP_NO_ERROR);
     EXPECT_EQ(mBtpEngine.RxState(), BtpEngine::kState_InProgress);
 
     auto packet2 = System::PacketBufferHandle::NewWithData(packetData2, sizeof(packetData2));
-    EXPECT_EQ(packet2->DataLength(), 3);
+    EXPECT_EQ(packet2->DataLength(), static_cast<size_t>(3));
 
     EXPECT_EQ(mBtpEngine.HandleCharacteristicReceived(std::move(packet2), receivedAck, didReceiveAck), CHIP_NO_ERROR);
     EXPECT_EQ(mBtpEngine.RxState(), BtpEngine::kState_Complete);
@@ -133,7 +133,7 @@ TEST_F(TestBtpEngine, HandleCharacteristicSendOnePacket)
 
     EXPECT_TRUE(mBtpEngine.HandleCharacteristicSend(packet0.Retain(), false));
     EXPECT_EQ(mBtpEngine.TxState(), BtpEngine::kState_Complete);
-    EXPECT_EQ(packet0->DataLength(), 5);
+    EXPECT_EQ(packet0->DataLength(), static_cast<size_t>(5));
 }
 
 TEST_F(TestBtpEngine, HandleCharacteristicSendTwoPacket)
@@ -147,11 +147,11 @@ TEST_F(TestBtpEngine, HandleCharacteristicSendTwoPacket)
 
     EXPECT_TRUE(mBtpEngine.HandleCharacteristicSend(packet0.Retain(), false));
     EXPECT_EQ(mBtpEngine.TxState(), BtpEngine::kState_InProgress);
-    EXPECT_EQ(packet0->DataLength(), 20);
+    EXPECT_EQ(packet0->DataLength(), static_cast<size_t>(20));
 
     EXPECT_TRUE(mBtpEngine.HandleCharacteristicSend(nullptr, false));
     EXPECT_EQ(mBtpEngine.TxState(), BtpEngine::kState_Complete);
-    EXPECT_EQ(packet0->DataLength(), 16);
+    EXPECT_EQ(packet0->DataLength(), static_cast<size_t>(16));
 }
 
 // Send 40-byte payload.
@@ -169,15 +169,15 @@ TEST_F(TestBtpEngine, HandleCharacteristicSendThreePacket)
 
     EXPECT_TRUE(mBtpEngine.HandleCharacteristicSend(packet0.Retain(), false));
     EXPECT_EQ(mBtpEngine.TxState(), BtpEngine::kState_InProgress);
-    EXPECT_EQ(packet0->DataLength(), 20);
+    EXPECT_EQ(packet0->DataLength(), static_cast<size_t>(20));
 
     EXPECT_TRUE(mBtpEngine.HandleCharacteristicSend(nullptr, false));
     EXPECT_EQ(mBtpEngine.TxState(), BtpEngine::kState_InProgress);
-    EXPECT_EQ(packet0->DataLength(), 20);
+    EXPECT_EQ(packet0->DataLength(), static_cast<size_t>(20));
 
     EXPECT_TRUE(mBtpEngine.HandleCharacteristicSend(nullptr, false));
     EXPECT_EQ(mBtpEngine.TxState(), BtpEngine::kState_Complete);
-    EXPECT_EQ(packet0->DataLength(), 8);
+    EXPECT_EQ(packet0->DataLength(), static_cast<size_t>(8));
 }
 
 } // namespace

src/inet/TCPEndPoint.cpp

@@ -126,7 +126,7 @@ CHIP_ERROR TCPEndPoint::SetReceivedDataForTesting(System::PacketBufferHandle &&
     return CHIP_NO_ERROR;
 }
 
-uint32_t TCPEndPoint::PendingSendLength()
+size_t TCPEndPoint::PendingSendLength()
 {
     if (!mSendQueue.IsNull())
     {
@@ -135,7 +135,7 @@ uint32_t TCPEndPoint::PendingSendLength()
     return 0;
 }
 
-uint32_t TCPEndPoint::PendingReceiveLength()
+size_t TCPEndPoint::PendingReceiveLength()
 {
     if (!mRcvQueue.IsNull())
     {
@@ -333,8 +333,8 @@ void TCPEndPoint::DriveReceiving()
     {
         // Acknowledgement is done after handling the buffers to allow the
         // application processing to throttle flow.
-        uint16_t ackLength = mRcvQueue->TotalLength();
-        CHIP_ERROR err     = OnDataReceived(this, std::move(mRcvQueue));
+        size_t ackLength = mRcvQueue->TotalLength();
+        CHIP_ERROR err   = OnDataReceived(this, std::move(mRcvQueue));
         if (err != CHIP_NO_ERROR)
         {
             DoClose(err, false);

src/inet/TCPEndPoint.h

@@ -274,7 +274,7 @@ class DLL_EXPORT TCPEndPoint : public EndPointBasis<TCPEndPoint>
      *  received. The operational semantics are undefined if \c len is larger
      *  than the total outstanding unacknowledged received data.
      */
-    virtual CHIP_ERROR AckReceive(uint16_t len) = 0;
+    virtual CHIP_ERROR AckReceive(size_t len) = 0;
 
     /**
      * @brief   Set the receive queue, for testing.
@@ -295,15 +295,15 @@ class DLL_EXPORT TCPEndPoint : public EndPointBasis<TCPEndPoint>
      *
      * @return  Number of untransmitted bytes in the transmit queue.
      */
-    uint32_t PendingSendLength();
+    size_t PendingSendLength();
 
     /**
      * @brief   Extract the length of the unacknowledged receive data.
      *
      * @return  Number of bytes in the receive queue that have not yet been
      *      acknowledged with <tt>AckReceive(uint16_t len)</tt>.
      */
-    uint32_t PendingReceiveLength();
+    size_t PendingReceiveLength();
 
     /**
      * @brief   Initiate TCP half close, in other words, finished with sending.
@@ -447,7 +447,7 @@ class DLL_EXPORT TCPEndPoint : public EndPointBasis<TCPEndPoint>
      *  is the length of the message text added to the TCP transmit window,
      *  which are eligible for sending by the underlying network stack.
      */
-    typedef void (*OnDataSentFunct)(TCPEndPoint * endPoint, uint16_t len);
+    typedef void (*OnDataSentFunct)(TCPEndPoint * endPoint, size_t len);
 
     /**
      * The endpoint's message text transmission event handling function

src/inet/TCPEndPointImplLwIP.cpp

@@ -350,8 +350,8 @@ CHIP_ERROR TCPEndPointImplLwIP::DriveSendingImpl()
             do
             {
                 VerifyOrDie(!startOfUnsent.buffer.IsNull());
-
-                uint16_t bufDataLen = startOfUnsent.buffer->DataLength();
+                VerifyOrDie(CanCastTo<uint16_t>(startOfUnsent.buffer->DataLength()));
+                uint16_t bufDataLen = static_cast<uint16_t>(startOfUnsent.buffer->DataLength());
 
                 // Get a pointer to the start of unsent data within the first buffer on the unsent queue.
                 const uint8_t * sendData = startOfUnsent.buffer->Start() + startOfUnsent.offset;
@@ -503,16 +503,18 @@ void TCPEndPointImplLwIP::DoCloseImpl(CHIP_ERROR err, State oldState)
     }
 }
 
-CHIP_ERROR TCPEndPointImplLwIP::AckReceive(uint16_t len)
+CHIP_ERROR TCPEndPointImplLwIP::AckReceive(size_t len)
 {
     VerifyOrReturnError(IsConnected(), CHIP_ERROR_INCORRECT_STATE);
     CHIP_ERROR res = CHIP_NO_ERROR;
 
+    VerifyOrReturnError(CanCastTo<uint16_t>(len), CHIP_ERROR_INVALID_ARGUMENT);
+
     // Lock LwIP stack
     LOCK_TCPIP_CORE();
 
     if (mTCP != nullptr)
-        tcp_recved(mTCP, len);
+        tcp_recved(mTCP, static_cast<uint16_t>(len));
     else
         res = CHIP_ERROR_CONNECTION_ABORTED;
 
@@ -570,7 +572,8 @@ TCPEndPointImplLwIP::BufferOffset TCPEndPointImplLwIP::FindStartOfUnsent()
     while (leftToSkip > 0)
     {
         VerifyOrDie(!startOfUnsent.buffer.IsNull());
-        uint16_t bufDataLen = startOfUnsent.buffer->DataLength();
+        VerifyOrDie(CanCastTo<uint16_t>(startOfUnsent.buffer->DataLength()));
+        uint16_t bufDataLen = static_cast<uint16_t>(startOfUnsent.buffer->DataLength());
         if (leftToSkip >= bufDataLen)
         {
             // We have more to skip than current packet buffer size.

src/inet/TCPEndPointImplLwIP.h

@@ -51,7 +51,7 @@ class TCPEndPointImplLwIP : public TCPEndPoint, public EndPointStateLwIP
     CHIP_ERROR EnableNoDelay() override;
     CHIP_ERROR EnableKeepAlive(uint16_t interval, uint16_t timeoutCount) override;
     CHIP_ERROR DisableKeepAlive() override;
-    CHIP_ERROR AckReceive(uint16_t len) override;
+    CHIP_ERROR AckReceive(size_t len) override;
 #if INET_CONFIG_OVERRIDE_SYSTEM_TCP_USER_TIMEOUT
     void TCPUserTimeoutHandler() override;
 #endif // INET_CONFIG_OVERRIDE_SYSTEM_TCP_USER_TIMEOUT

src/inet/TCPEndPointImplOpenThread.cpp

@@ -54,7 +54,7 @@ CHIP_ERROR TCPEndPointImplOT::DisableKeepAlive()
 {
     return CHIP_ERROR_NOT_IMPLEMENTED;
 }
-CHIP_ERROR TCPEndPointImplOT::AckReceive(uint16_t len)
+CHIP_ERROR TCPEndPointImplOT::AckReceive(size_t len)
 {
     return CHIP_ERROR_NOT_IMPLEMENTED;
 }

src/inet/TCPEndPointImplOpenThread.h

@@ -46,7 +46,7 @@ class TCPEndPointImplOT : public TCPEndPoint, public EndPointStateOpenThread
     CHIP_ERROR EnableNoDelay() override;
     CHIP_ERROR EnableKeepAlive(uint16_t interval, uint16_t timeoutCount) override;
     CHIP_ERROR DisableKeepAlive() override;
-    CHIP_ERROR AckReceive(uint16_t len) override;
+    CHIP_ERROR AckReceive(size_t len) override;
 #if INET_CONFIG_OVERRIDE_SYSTEM_TCP_USER_TIMEOUT
     void TCPUserTimeoutHandler() override;
 #endif // INET_CONFIG_OVERRIDE_SYSTEM_TCP_USER_TIMEOUT

src/inet/TCPEndPointImplSockets.cpp

@@ -441,7 +441,7 @@ CHIP_ERROR TCPEndPointImplSockets::DisableKeepAlive()
     return CHIP_NO_ERROR;
 }
 
-CHIP_ERROR TCPEndPointImplSockets::AckReceive(uint16_t len)
+CHIP_ERROR TCPEndPointImplSockets::AckReceive(size_t len)
 {
     VerifyOrReturnError(IsConnected(), CHIP_ERROR_INCORRECT_STATE);
 
@@ -483,7 +483,7 @@ CHIP_ERROR TCPEndPointImplSockets::DriveSendingImpl()
 
     while (!mSendQueue.IsNull())
     {
-        uint16_t bufLen = mSendQueue->DataLength();
+        size_t bufLen = mSendQueue->DataLength();
 
         ssize_t lenSentRaw = send(mSocket, mSendQueue->Start(), bufLen, sendFlags);
 
@@ -496,14 +496,13 @@ CHIP_ERROR TCPEndPointImplSockets::DriveSendingImpl()
             break;
         }
 
-        if (lenSentRaw < 0 || lenSentRaw > bufLen)
+        if (lenSentRaw < 0 || bufLen < static_cast<size_t>(lenSentRaw))
         {
             err = CHIP_ERROR_INCORRECT_STATE;
             break;
         }
 
-        // Cast is safe because bufLen is uint16_t.
-        uint16_t lenSent = static_cast<uint16_t>(lenSentRaw);
+        size_t lenSent = static_cast<size_t>(lenSentRaw);
 
         // Mark the connection as being active.
         MarkActive();

src/inet/TCPEndPointImplSockets.h

@@ -46,7 +46,7 @@ class TCPEndPointImplSockets : public TCPEndPoint, public EndPointStateSockets
     CHIP_ERROR EnableNoDelay() override;
     CHIP_ERROR EnableKeepAlive(uint16_t interval, uint16_t timeoutCount) override;
     CHIP_ERROR DisableKeepAlive() override;
-    CHIP_ERROR AckReceive(uint16_t len) override;
+    CHIP_ERROR AckReceive(size_t len) override;
 #if INET_CONFIG_OVERRIDE_SYSTEM_TCP_USER_TIMEOUT
     void TCPUserTimeoutHandler() override;
 #endif // INET_CONFIG_OVERRIDE_SYSTEM_TCP_USER_TIMEOUT
@@ -72,7 +72,7 @@ class TCPEndPointImplSockets : public TCPEndPoint, public EndPointStateSockets
 
 #if INET_CONFIG_OVERRIDE_SYSTEM_TCP_USER_TIMEOUT
     /// This counts the number of bytes written on the TCP socket since thelast probe into the TCP outqueue was made.
-    uint32_t mBytesWrittenSinceLastProbe;
+    size_t mBytesWrittenSinceLastProbe;
 
     /// This is the measured size(in bytes) of the kernel TCP send queue at the end of the last user timeout window.
     uint32_t mLastTCPKernelSendQueueLen;

src/inet/UDPEndPointImplLwIP.cpp

@@ -352,7 +352,7 @@ void UDPEndPointImplLwIP::LwIPReceiveUDPMessage(void * arg, struct udp_pcb * pcb
     if (buf->HasChainedBuffer())
     {
         // Have to allocate a new big-enough buffer and copy.
-        uint16_t messageSize            = buf->TotalLength();
+        size_t messageSize              = buf->TotalLength();
         System::PacketBufferHandle copy = System::PacketBufferHandle::New(messageSize, 0);
         if (copy.IsNull() || buf->Read(copy->Start(), messageSize) != CHIP_NO_ERROR)
         {

src/inet/UDPEndPointImplOpenThread.cpp

@@ -225,7 +225,7 @@ CHIP_ERROR UDPEndPointImplOT::SendMsgImpl(const IPPacketInfo * aPktInfo, System:
     otMessageInfo messageInfo;
 
     // For now the entire message must fit within a single buffer.
-    VerifyOrReturnError(!msg->HasChainedBuffer(), CHIP_ERROR_MESSAGE_TOO_LONG);
+    VerifyOrReturnError(!msg->HasChainedBuffer() && msg->DataLength() <= UINT16_MAX, CHIP_ERROR_MESSAGE_TOO_LONG);
 
     memset(&messageInfo, 0, sizeof(messageInfo));
 
@@ -237,7 +237,7 @@ CHIP_ERROR UDPEndPointImplOT::SendMsgImpl(const IPPacketInfo * aPktInfo, System:
     message = otUdpNewMessage(mOTInstance, NULL);
     VerifyOrExit(message != NULL, error = OT_ERROR_NO_BUFS);
 
-    error = otMessageAppend(message, msg->Start(), msg->DataLength());
+    error = otMessageAppend(message, msg->Start(), static_cast<uint16_t>(msg->DataLength()));
 
     if (error == OT_ERROR_NONE)
     {