Use a pre allocated endpoint for the TCP connection

project-chip · Feb 6, 2025 · 0847591 · 0847591
1 parent f86a812
commit 0847591
Show file tree

Hide file tree

Showing 7 changed files with 45 additions and 128 deletions.
diff --git a/src/inet/TCPEndPointImplLwIP.cpp b/src/inet/TCPEndPointImplLwIP.cpp
@@ -39,11 +39,6 @@
 
 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 {
 
@@ -92,13 +87,21 @@ CHIP_ERROR TCPEndPointImplLwIP::BindImpl(IPAddressType addrType, const IPAddress
 CHIP_ERROR TCPEndPointImplLwIP::ListenImpl(uint16_t backlog)
 {
     mLwIPEndPointType = LwIPEndPointType::TCP;
-    RunOnTCPIP([this]() {
-        // Start listening for incoming connections.
-        mTCP = tcp_listen(mTCP);
-        tcp_arg(mTCP, this);
-        tcp_accept(mTCP, LwIPHandleIncomingConnection);
-    });
-    return CHIP_NO_ERROR;
+    CHIP_ERROR err    = CHIP_NO_ERROR;
+    if (!mPreAllocatedConnectEP)
+    {
+        err =  GetEndPointManager().NewEndPoint(&mPreAllocatedConnectEP);
+    }
+    if (err == CHIP_NO_ERROR)
+    {
+        RunOnTCPIP([this]() {
+            // Start listening for incoming connections.
+            mTCP = tcp_listen(mTCP);
+            tcp_arg(mTCP, this);
+            tcp_accept(mTCP, LwIPHandleIncomingConnection);
+        });
+    }
+    return err;
 }
 
 CHIP_ERROR TCPEndPointImplLwIP::ConnectImpl(const IPAddress & addr, uint16_t port, InterfaceId intfId)
@@ -800,7 +803,8 @@ err_t TCPEndPointImplLwIP::LwIPHandleIncomingConnection(void * arg, struct tcp_p
         // Tell LwIP we've accepted the connection so it can decrement the listen PCB's pending_accepts counter.
         tcp_accepted(listenEP->mTCP);
 
-        // If we did in fact receive a connection, rather than an error, attempt to allocate an end point object.
+        // If we did in fact receive a connection, rather than an error, use the pre-allocated end point object for the incoming
+        // connection.
         //
         // NOTE: Although most of the LwIP callbacks defer the real work to happen on the endpoint's thread
         // (by posting events to the thread's event queue) we can't do that here because as soon as this
@@ -809,10 +813,12 @@ err_t TCPEndPointImplLwIP::LwIPHandleIncomingConnection(void * arg, struct tcp_p
         //
         if (err == CHIP_NO_ERROR)
         {
-            TCPEndPoint * connectEndPoint = nullptr;
-            err                           = lSystemLayer.RunWithMatterContextLock(
-                [&listenEP, &connectEndPoint]() { return listenEP->GetEndPointManager().NewEndPoint(&connectEndPoint); });
-            conEP = static_cast<TCPEndPointImplLwIP *>(connectEndPoint);
+            conEP = static_cast<TCPEndPointImplLwIP *>(listenEP->mPreAllocatedConnectEP);
+            if (conEP == nullptr)
+            {
+                // The listen endpoint receives a new incomming connection before it pre-allocates a new connection endpoint.
+                err = CHIP_ERROR_BUSY;
+            }
         }
 
         // Ensure that TCP timers have been started
@@ -854,7 +860,24 @@ err_t TCPEndPointImplLwIP::LwIPHandleIncomingConnection(void * arg, struct tcp_p
                 listenEP->Release();
                 err = CHIP_ERROR_CONNECTION_ABORTED;
                 conEP->Release(); // for the Retain() above
-                conEP->Release(); // for the implied Retain() on construction
+            }
+            else
+            {
+                // Pre-allocate another endpoint for next connection
+                listenEP->mPreAllocatedConnectEP = nullptr;
+                listenEP->Retain();
+                err = lSystemLayer.ScheduleLambda([listenEP]() {
+                    CHIP_ERROR error = listenEP->GetEndPointManager().NewEndPoint(&listenEP->mPreAllocatedConnectEP);
+                    if (error != CHIP_NO_ERROR)
+                    {
+                        listenEP->HandleError(error);
+                    }
+                    listenEP->Release();
+                });
+                if (err != CHIP_NO_ERROR)
+                {
+                    listenEP->Release();
+                }
             }
         }
 

diff --git a/src/inet/TCPEndPointImplLwIP.h b/src/inet/TCPEndPointImplLwIP.h
@@ -41,7 +41,7 @@ class TCPEndPointImplLwIP : public TCPEndPoint, public EndPointStateLwIP
 {
 public:
     TCPEndPointImplLwIP(EndPointManager<TCPEndPoint> & endPointManager) :
-        TCPEndPoint(endPointManager), mUnackedLength(0), mTCP(nullptr)
+        TCPEndPoint(endPointManager), mUnackedLength(0), mTCP(nullptr), mPreAllocatedConnectEP(nullptr)
     {}
 
     // TCPEndPoint overrides.
@@ -82,6 +82,9 @@ class TCPEndPointImplLwIP : public TCPEndPoint, public EndPointStateLwIP
     uint16_t mUnackedLength; // Amount sent but awaiting ACK. Used as a form of reference count
                              // to hang-on to backing packet buffers until they are no longer needed.
     tcp_pcb * mTCP;          // LwIP Transmission control protocol (TCP) control block.
+    // For TCP Listen endpoint, we will pre-allocate a connection endpoint to assign the incoming connection to it.
+    // when there is a new TCP connection established.
+    TCPEndPoint *mPreAllocatedConnectEP;
 
     uint16_t RemainingToSend();
     BufferOffset FindStartOfUnsent();

diff --git a/src/platform/BUILD.gn b/src/platform/BUILD.gn
@@ -18,7 +18,6 @@ import("//build_overrides/pigweed.gni")
 
 import("${build_root}/config/linux/pkg_config.gni")
 import("${chip_root}/build/chip/buildconfig_header.gni")
-import("${chip_root}/src/system/system.gni")
 
 import("device.gni")
 
@@ -527,10 +526,6 @@ if (chip_device_platform != "none") {
       "RuntimeOptionsProvider.cpp",
     ]
 
-    if (chip_system_config_locking != "none") {
-      sources += [ "PlatformLockSupport.cpp" ]
-    }
-
     # Linux has its own NetworkCommissioningThreadDriver
     if (chip_enable_openthread && chip_device_platform != "linux" &&
         chip_device_platform != "tizen" && chip_device_platform != "webos") {

diff --git a/src/platform/PlatformLockSupport.cpp b/src/platform/PlatformLockSupport.cpp
diff --git a/src/system/BUILD.gn b/src/system/BUILD.gn
@@ -256,10 +256,6 @@ static_library("system") {
     ]
   }
 
-  if (chip_system_config_locking != "none") {
-    sources += [ "PlatformLockSupport.h" ]
-  }
-
   cflags = [ "-Wconversion" ]
 
   public_deps = [

diff --git a/src/system/PlatformLockSupport.h b/src/system/PlatformLockSupport.h
diff --git a/src/system/SystemLayer.h b/src/system/SystemLayer.h
@@ -52,10 +52,6 @@
 #include <ev.h>
 #endif // CHIP_SYSTEM_CONFIG_USE_DISPATCH/LIBEV
 
-#if !CHIP_SYSTEM_CONFIG_NO_LOCKING
-#include <system/PlatformLockSupport.h>
-#endif
-
 namespace chip {
 namespace System {
 
@@ -224,29 +220,6 @@ class DLL_EXPORT Layer
         return ScheduleLambdaBridge(std::move(bridge));
     }
 
-#if !CHIP_SYSTEM_CONFIG_NO_LOCKING
-    /**
-     * @brief
-     *   Call the func with Matter context locked
-     *
-     * CRITICAL: The function should be non-blocking to avoid dead lock.
-     *
-     * @param[in] nonBlockingFunc The non-blocking lambda to be called with Matter stack lock held.
-     *
-     * @retval The return value of the non-blocking function
-     */
-    template <typename Lambda>
-    CHIP_ERROR RunWithMatterContextLock(const Lambda & nonBlockingFunc)
-    {
-        static_assert(std::is_invocable_v<Lambda>, "lambda argument must be an invocable with no arguments");
-        CHIP_ERROR err = CHIP_NO_ERROR;
-        PlatformLocking::LockMatterStack();
-        err = nonBlockingFunc();
-        PlatformLocking::UnlockMatterStack();
-        return err;
-    }
-#endif
-
 private:
     CHIP_ERROR ScheduleLambdaBridge(LambdaBridge && bridge);