review changes and revert packet buffer changes

project-chip · Jan 13, 2025 · 58b91ab · 58b91ab
1 parent 410e158
commit 58b91ab
Show file tree

Hide file tree

Showing 3 changed files with 12 additions and 38 deletions.
diff --git a/src/inet/TCPEndPointImplLwIP.cpp b/src/inet/TCPEndPointImplLwIP.cpp
@@ -39,6 +39,11 @@
 
 static_assert(LWIP_VERSION_MAJOR > 1, "CHIP requires LwIP 2.0 or later");
 
+static_assert(!CHIP_SYSTEM_CONFIG_NO_LOCKING,
+              "CHIP_SYSTEM_CONFIG_NO_LOCKING not supported along with using LwIP for TCP, because handling incoming connection "
+              "attempts needs to be done synchronously in LwIP, but part of the work for it needs to happen in the Matter context. "
+              " If support for this configuration is needed, this part needs to be figured out.");
+
 namespace chip {
 namespace Inet {
 
@@ -805,14 +810,8 @@ err_t TCPEndPointImplLwIP::LwIPHandleIncomingConnection(void * arg, struct tcp_p
         if (err == CHIP_NO_ERROR)
         {
             TCPEndPoint * connectEndPoint = nullptr;
-#if CHIP_SYSTEM_CONFIG_NO_LOCKING
-            // TODO This should be in Matter context but we cannot use SystemLayer.ScheduleLambda() here as the allocated endpoint
-            // will be used in the following.
-            err = listenEP->GetEndPointManager().NewEndPoint(&connectEndPoint);
-#else
             err = lSystemLayer.RunWithMatterContextLock(
                 [&listenEP, &connectEndPoint]() { return listenEP->GetEndPointManager().NewEndPoint(&connectEndPoint); });
-#endif
             conEP = static_cast<TCPEndPointImplLwIP *>(connectEndPoint);
         }
 

diff --git a/src/system/SystemLayer.h b/src/system/SystemLayer.h
@@ -227,7 +227,7 @@ class DLL_EXPORT Layer
      *
      * CRITICAL: The function should be non-blocking to avoid dead lock.
      *
-     * @param[in] nonBlockingFunc The non-blocking function to be called with Matter stack lock
+     * @param[in] nonBlockingFunc The non-blocking function to be called with Matter stack lock held.
      *
      * @retval The return value of the non-blocking function
      */

diff --git a/src/system/SystemPacketBuffer.cpp b/src/system/SystemPacketBuffer.cpp
@@ -634,21 +634,12 @@ PacketBufferHandle PacketBufferHandle::New(size_t aAvailableSize, uint16_t aRese
     SYSTEM_STATS_INCREMENT(chip::System::Stats::kSystemLayer_NumPacketBufs);
 
     lPacket->payload = lPacket->ReserveStart() + aReservedSize;
+    lPacket->len = lPacket->tot_len = 0;
+    lPacket->next                   = nullptr;
+    lPacket->ref                    = 1;
 #if CHIP_SYSTEM_PACKETBUFFER_FROM_CHIP_HEAP
     lPacket->alloc_size = lAllocSize;
 #endif
-#if !CHIP_SYSTEM_CONFIG_USE_LWIP
-    // For non-lwip packet buffer, the allocated packet buffer will not have chained buffers, set it to null to avoid potential
-    // issues.
-    lPacket->next = nullptr;
-#endif
-    // Set ther length and total length of the head packet buffer and all the chained packet buffers to 0
-    // as we don't put any data in them. And set the ref to 1 for all the buffers.
-    for (PacketBuffer * pktBuf = lPacket; pktBuf != nullptr; pktBuf = pktBuf->ChainedBuffer())
-    {
-        pktBuf->len = pktBuf->tot_len = 0;
-        pktBuf->ref                   = 1;
-    }
 
     return PacketBufferHandle(lPacket);
 }
@@ -661,30 +652,14 @@ PacketBufferHandle PacketBufferHandle::NewWithData(const void * aData, size_t aD
     PacketBufferHandle buffer = New(aDataSize + aAdditionalSize, aReservedSize);
     if (buffer.mBuffer != nullptr)
     {
+        memcpy(buffer.mBuffer->payload, aData, aDataSize);
 #if CHIP_SYSTEM_CONFIG_USE_LWIP
         // Checks in the New() call catch buffer allocations greater
         // than UINT16_MAX for LwIP based platforms.
-        buffer.mBuffer->tot_len = static_cast<uint16_t>(aDataSize);
-#else
-        buffer.mBuffer->tot_len = aDataSize;
-#endif
-        // Copy the data to the first packet buffer, if the data size is larger than the MaxDataLength, copy the remaining
-        // data to the chained packet buffers.
-        PacketBuffer * currentBuffer = buffer.mBuffer;
-        const uint8_t * dataPtr      = static_cast<const uint8_t *>(aData);
-        while (currentBuffer && aDataSize > 0)
-        {
-            size_t copyLen = currentBuffer->MaxDataLength() > aDataSize ? aDataSize : currentBuffer->MaxDataLength();
-            memcpy(currentBuffer->payload, dataPtr, copyLen);
-            aDataSize -= copyLen;
-            dataPtr += copyLen;
-#if CHIP_SYSTEM_CONFIG_USE_LWIP
-            currentBuffer->len = static_cast<uint16_t>(copyLen);
+        buffer.mBuffer->len = buffer.mBuffer->tot_len = static_cast<uint16_t>(aDataSize);
 #else
-            currentBuffer->len = copyLen;
+        buffer.mBuffer->len = buffer.mBuffer->tot_len = aDataSize;
 #endif
-            currentBuffer = currentBuffer->ChainedBuffer();
-        }
     }
     return buffer;
 }