Add separate knob for TCP server listen enabling. (project-chip#36979)

* Add separate knob for TCP server listen enabling. Use a boolean indicator for TCP server enabling in the TCPListenParameters to signal Server enabling during initialization. By default, TCP Server is enabled on the Server side, and disabled on the client/controller side. Add API in TransportMgr to access this setting so that DNS-SD operational advertisements for TCP can be set accordingly. * Add 2 unit tests * Apply suggestions from code review Co-authored-by: Andrei Litvin <[email protected]> --------- Co-authored-by: Andrei Litvin <[email protected]>
joonhaengHeo · Jan 9, 2025 · 0129a57 · 0129a57
1 parent b83c27a
commit 0129a57
Show file tree

Hide file tree

Showing 13 changed files with 123 additions and 37 deletions.
diff --git a/src/app/server/Dnssd.cpp b/src/app/server/Dnssd.cpp
@@ -214,7 +214,8 @@ CHIP_ERROR DnssdServer::AdvertiseOperational()
 #endif
 
 #if INET_CONFIG_ENABLE_TCP_ENDPOINT
-        advertiseParameters.SetTCPSupportModes(chip::Dnssd::TCPModeAdvertise::kTCPClientServer);
+        advertiseParameters.SetTCPSupportModes(mTCPServerEnabled ? chip::Dnssd::TCPModeAdvertise::kTCPClientServer
+                                                                 : chip::Dnssd::TCPModeAdvertise::kTCPClient);
 #endif
         auto & mdnsAdvertiser = chip::Dnssd::ServiceAdvertiser::Instance();
 

diff --git a/src/app/server/Dnssd.h b/src/app/server/Dnssd.h
@@ -97,6 +97,10 @@ class DLL_EXPORT DnssdServer : public ICDStateObserver
     void SetICDManager(ICDManager * manager) { mICDManager = manager; };
 #endif
 
+#if INET_CONFIG_ENABLE_TCP_ENDPOINT
+    void SetTCPServerEnabled(bool serverEnabled) { mTCPServerEnabled = serverEnabled; };
+#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
+
     /// Start operational advertising
     CHIP_ERROR AdvertiseOperational();
 
@@ -179,6 +183,10 @@ class DLL_EXPORT DnssdServer : public ICDStateObserver
     ICDManager * mICDManager = nullptr;
 #endif // CHIP_CONFIG_ENABLE_ICD_SERVER
 
+#if INET_CONFIG_ENABLE_TCP_ENDPOINT
+    bool mTCPServerEnabled = true;
+#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
+
     uint16_t mSecuredPort          = CHIP_PORT;
     uint16_t mUnsecuredPort        = CHIP_UDC_PORT;
     Inet::InterfaceId mInterfaceId = Inet::InterfaceId::Null();

diff --git a/src/app/server/Server.cpp b/src/app/server/Server.cpp
@@ -214,7 +214,12 @@ CHIP_ERROR Server::Init(const ServerInitParams & initParams)
                                .SetAddressType(IPAddressType::kIPv6)
                                .SetListenPort(mOperationalServicePort)
                                .SetNativeParams(initParams.endpointNativeParams)
-
+#if INET_CONFIG_ENABLE_TCP_ENDPOINT
+                               ,
+                           TcpListenParameters(DeviceLayer::TCPEndPointManager())
+                               .SetAddressType(IPAddressType::kIPv6)
+                               .SetListenPort(mOperationalServicePort)
+#endif
 #if INET_CONFIG_ENABLE_IPV4
                                ,
                            UdpListenParameters(DeviceLayer::UDPEndPointManager())
@@ -225,12 +230,6 @@ CHIP_ERROR Server::Init(const ServerInitParams & initParams)
                                ,
                            BleListenParameters(DeviceLayer::ConnectivityMgr().GetBleLayer())
 #endif
-#if INET_CONFIG_ENABLE_TCP_ENDPOINT
-                               ,
-                           TcpListenParameters(DeviceLayer::TCPEndPointManager())
-                               .SetAddressType(IPAddressType::kIPv6)
-                               .SetListenPort(mOperationalServicePort)
-#endif
 #if CHIP_DEVICE_CONFIG_ENABLE_WIFIPAF
                                ,
                            Transport::WiFiPAFListenParameters(DeviceLayer::ConnectivityMgr().GetWiFiPAF())
@@ -330,6 +329,10 @@ CHIP_ERROR Server::Init(const ServerInitParams & initParams)
     app::DnssdServer::Instance().SetICDManager(&mICDManager);
 #endif // CHIP_CONFIG_ENABLE_ICD_SERVER
 
+#if INET_CONFIG_ENABLE_TCP_ENDPOINT
+    app::DnssdServer::Instance().SetTCPServerEnabled(mTransports.GetTransport().GetImplAtIndex<1>().IsServerListenEnabled());
+#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
+
     if (GetFabricTable().FabricCount() != 0)
     {
 #if CONFIG_NETWORK_LAYER_BLE

diff --git a/src/app/server/Server.h b/src/app/server/Server.h
@@ -96,6 +96,10 @@ inline constexpr size_t kMaxTcpPendingPackets = CHIP_CONFIG_MAX_TCP_PENDING_PACK
 //       in the Server impl depends on this.
 //
 using ServerTransportMgr = chip::TransportMgr<chip::Transport::UDP
+#if INET_CONFIG_ENABLE_TCP_ENDPOINT
+                                              ,
+                                              chip::Transport::TCP<kMaxTcpActiveConnectionCount, kMaxTcpPendingPackets>
+#endif
 #if INET_CONFIG_ENABLE_IPV4
                                               ,
                                               chip::Transport::UDP
@@ -104,10 +108,6 @@ using ServerTransportMgr = chip::TransportMgr<chip::Transport::UDP
                                               ,
                                               chip::Transport::BLE<kMaxBlePendingPackets>
 #endif
-#if INET_CONFIG_ENABLE_TCP_ENDPOINT
-                                              ,
-                                              chip::Transport::TCP<kMaxTcpActiveConnectionCount, kMaxTcpPendingPackets>
-#endif
 #if CHIP_DEVICE_CONFIG_ENABLE_WIFIPAF
                                               ,
                                               chip::Transport::WiFiPAFBase

diff --git a/src/controller/CHIPDeviceControllerFactory.cpp b/src/controller/CHIPDeviceControllerFactory.cpp
@@ -167,8 +167,15 @@ CHIP_ERROR DeviceControllerFactory::InitSystemState(FactoryInitParams params)
     ReturnErrorOnFailure(stateParams.transportMgr->Init(Transport::UdpListenParameters(stateParams.udpEndPointManager)
                                                             .SetAddressType(Inet::IPAddressType::kIPv6)
                                                             .SetListenPort(params.listenPort)
-#if INET_CONFIG_ENABLE_IPV4
+#if INET_CONFIG_ENABLE_TCP_ENDPOINT
                                                             ,
+                                                        Transport::TcpListenParameters(stateParams.tcpEndPointManager)
+                                                            .SetAddressType(IPAddressType::kIPv6)
+                                                            .SetListenPort(params.listenPort)
+                                                            .SetServerListenEnabled(false) // Initialize as a TCP Client
+#endif
+#if INET_CONFIG_ENABLE_IPV4
+                                                        ,
                                                         Transport::UdpListenParameters(stateParams.udpEndPointManager)
                                                             .SetAddressType(Inet::IPAddressType::kIPv4)
                                                             .SetListenPort(params.listenPort)
@@ -177,12 +184,6 @@ CHIP_ERROR DeviceControllerFactory::InitSystemState(FactoryInitParams params)
                                                             ,
                                                         Transport::BleListenParameters(stateParams.bleLayer)
 #endif
-#if INET_CONFIG_ENABLE_TCP_ENDPOINT
-                                                            ,
-                                                        Transport::TcpListenParameters(stateParams.tcpEndPointManager)
-                                                            .SetAddressType(IPAddressType::kIPv6)
-                                                            .SetListenPort(params.listenPort)
-#endif
 #if CHIP_DEVICE_CONFIG_ENABLE_WIFIPAF
                                                             ,
                                                         Transport::WiFiPAFListenParameters()

diff --git a/src/controller/CHIPDeviceControllerSystemState.h b/src/controller/CHIPDeviceControllerSystemState.h
@@ -71,6 +71,10 @@ inline constexpr size_t kMaxDeviceTransportTcpPendingPackets = CHIP_CONFIG_MAX_T
 
 using DeviceTransportMgr =
     TransportMgr<Transport::UDP /* IPv6 */
+#if INET_CONFIG_ENABLE_TCP_ENDPOINT
+                 ,
+                 Transport::TCP<kMaxDeviceTransportTcpActiveConnectionCount, kMaxDeviceTransportTcpPendingPackets>
+#endif
 #if INET_CONFIG_ENABLE_IPV4
                  ,
                  Transport::UDP /* IPv4 */
@@ -79,10 +83,6 @@ using DeviceTransportMgr =
                  ,
                  Transport::BLE<kMaxDeviceTransportBlePendingPackets> /* BLE */
 #endif
-#if INET_CONFIG_ENABLE_TCP_ENDPOINT
-                 ,
-                 Transport::TCP<kMaxDeviceTransportTcpActiveConnectionCount, kMaxDeviceTransportTcpPendingPackets>
-#endif
 #if CHIP_DEVICE_CONFIG_ENABLE_WIFIPAF
                  ,
                  Transport::WiFiPAF<kMaxDeviceTransportWiFiPAFPendingPackets> /* WiFiPAF */

diff --git a/src/transport/TransportMgrBase.cpp b/src/transport/TransportMgrBase.cpp
@@ -44,6 +44,11 @@ void TransportMgrBase::TCPDisconnect(Transport::ActiveTCPConnectionState * conn,
 {
     mTransport->TCPDisconnect(conn, shouldAbort);
 }
+
+bool TransportMgrBase::IsServerListenEnabled()
+{
+    return mTransport->IsServerListenEnabled();
+}
 #endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
 
 CHIP_ERROR TransportMgrBase::Init(Transport::Base * transport)

diff --git a/src/transport/TransportMgrBase.h b/src/transport/TransportMgrBase.h
@@ -50,6 +50,8 @@ class TransportMgrBase : public Transport::RawTransportDelegate
 
     void TCPDisconnect(Transport::ActiveTCPConnectionState * conn, bool shouldAbort = 0);
 
+    bool IsServerListenEnabled();
+
     void HandleConnectionReceived(Transport::ActiveTCPConnectionState * conn) override;
 
     void HandleConnectionAttemptComplete(Transport::ActiveTCPConnectionState * conn, CHIP_ERROR conErr) override;

diff --git a/src/transport/raw/Base.h b/src/transport/raw/Base.h
@@ -114,6 +114,8 @@ class Base
      * Disconnect on the active connection that is passed in.
      */
     virtual void TCPDisconnect(Transport::ActiveTCPConnectionState * conn, bool shouldAbort = 0) {}
+
+    virtual bool IsServerListenEnabled() { return true; }
 #endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
 
     /**

diff --git a/src/transport/raw/TCP.cpp b/src/transport/raw/TCP.cpp
@@ -76,25 +76,29 @@ CHIP_ERROR TCPBase::Init(TcpListenParameters & params)
 
     VerifyOrExit(mState == TCPState::kNotReady, err = CHIP_ERROR_INCORRECT_STATE);
 
-#if INET_CONFIG_ENABLE_TCP_ENDPOINT
-    err = params.GetEndPointManager()->NewEndPoint(&mListenSocket);
-#else
-    err = CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE;
-#endif
-    SuccessOrExit(err);
+    mEndpointType = params.GetAddressType();
 
-    err = mListenSocket->Bind(params.GetAddressType(), Inet::IPAddress::Any, params.GetListenPort(),
-                              params.GetInterfaceId().IsPresent());
+    mIsServerListenEnabled = params.IsServerListenEnabled();
+
+    // Primary socket endpoint created to help get EndPointManager handle for creating multiple
+    // connection endpoints at runtime.
+    err = params.GetEndPointManager()->NewEndPoint(&mListenSocket);
     SuccessOrExit(err);
 
-    mListenSocket->mAppState            = reinterpret_cast<void *>(this);
-    mListenSocket->OnConnectionReceived = HandleIncomingConnection;
-    mListenSocket->OnAcceptError        = HandleAcceptError;
+    if (mIsServerListenEnabled)
+    {
+        err = mListenSocket->Bind(params.GetAddressType(), Inet::IPAddress::Any, params.GetListenPort(),
+                                  params.GetInterfaceId().IsPresent());
+        SuccessOrExit(err);
 
-    mEndpointType = params.GetAddressType();
+        mListenSocket->mAppState            = reinterpret_cast<void *>(this);
+        mListenSocket->OnConnectionReceived = HandleIncomingConnection;
+        mListenSocket->OnAcceptError        = HandleAcceptError;
 
-    err = mListenSocket->Listen(kListenBacklogSize);
-    SuccessOrExit(err);
+        err = mListenSocket->Listen(kListenBacklogSize);
+        SuccessOrExit(err);
+        ChipLogProgress(Inet, "TCP server listening on port %d for incoming connections", params.GetListenPort());
+    }
 
     mState = TCPState::kInitialized;
 
@@ -673,6 +677,11 @@ void TCPBase::TCPDisconnect(Transport::ActiveTCPConnectionState * conn, bool sho
     }
 }
 
+bool TCPBase::IsServerListenEnabled()
+{
+    return mIsServerListenEnabled;
+}
+
 bool TCPBase::HasActiveConnections() const
 {
     for (size_t i = 0; i < mActiveConnectionsSize; i++)

diff --git a/src/transport/raw/TCP.h b/src/transport/raw/TCP.h
@@ -79,11 +79,20 @@ class TcpListenParameters
         return *this;
     }
 
+    bool IsServerListenEnabled() const { return mServerListenEnabled; }
+    TcpListenParameters & SetServerListenEnabled(bool listenEnable)
+    {
+        mServerListenEnabled = listenEnable;
+
+        return *this;
+    }
+
 private:
     Inet::EndPointManager<Inet::TCPEndPoint> * mEndPointManager;   ///< Associated endpoint factory
     Inet::IPAddressType mAddressType = Inet::IPAddressType::kIPv6; ///< type of listening socket
     uint16_t mListenPort             = CHIP_PORT;                  ///< TCP listen port
     Inet::InterfaceId mInterfaceId   = Inet::InterfaceId::Null();  ///< Interface to listen on
+    bool mServerListenEnabled        = true;                       ///< TCP Server mode enabled
 };
 
 /**
@@ -178,6 +187,8 @@ class DLL_EXPORT TCPBase : public Base
     // and release from the pool.
     void TCPDisconnect(Transport::ActiveTCPConnectionState * conn, bool shouldAbort = false) override;
 
+    bool IsServerListenEnabled() override;
+
     bool CanSendToPeer(const PeerAddress & address) override
     {
         return (mState == TCPState::kInitialized) && (address.GetTransportType() == Type::kTcp) &&
@@ -301,6 +312,7 @@ class DLL_EXPORT TCPBase : public Base
     Inet::TCPEndPoint * mListenSocket = nullptr;                       ///< TCP socket used by the transport
     Inet::IPAddressType mEndpointType = Inet::IPAddressType::kUnknown; ///< Socket listening type
     TCPState mState                   = TCPState::kNotReady;           ///< State of the TCP transport
+    bool mIsServerListenEnabled       = true;                          ///< TCP Server mode enabled
 
     // The configured timeout for the connection attempt to the peer, before
     // giving up.

diff --git a/src/transport/raw/Tuple.h b/src/transport/raw/Tuple.h
@@ -106,6 +106,8 @@ class Tuple : public Base
     {
         return TCPDisconnectImpl<0>(conn, shouldAbort);
     }
+
+    bool IsServerListenEnabled() override { return IsServerListenEnabledImpl<0>(); }
 #endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
 
     void Close() override { return CloseImpl<0>(); }
@@ -222,6 +224,18 @@ class Tuple : public Base
     template <size_t N, typename std::enable_if<(N >= sizeof...(TransportTypes))>::type * = nullptr>
     void TCPDisconnectImpl(Transport::ActiveTCPConnectionState * conn, bool shouldAbort = 0)
     {}
+
+    template <size_t N, typename std::enable_if<(N < sizeof...(TransportTypes))>::type * = nullptr>
+    bool IsServerListenEnabledImpl()
+    {
+        return std::get<N>(mTransports).IsServerListenEnabled() || IsServerListenEnabledImpl<N + 1>();
+    }
+
+    template <size_t N, typename std::enable_if<(N >= sizeof...(TransportTypes))>::type * = nullptr>
+    bool IsServerListenEnabledImpl()
+    {
+        return false;
+    }
 #endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
 
     /**

diff --git a/src/transport/raw/tests/TestTCP.cpp b/src/transport/raw/tests/TestTCP.cpp
@@ -552,6 +552,35 @@ TEST_F(TestTCP, CheckSimpleInitTest6)
     CheckSimpleInitTest(IPAddressType::kIPv6);
 }
 
+TEST_F(TestTCP, InitializeAsTCPClient)
+{
+    TCPImpl tcp;
+
+    CHIP_ERROR err = tcp.Init(Transport::TcpListenParameters(mIOContext->GetTCPEndPointManager())
+                                  .SetAddressType(IPAddressType::kIPv6)
+                                  .SetListenPort(gChipTCPPort)
+                                  .SetServerListenEnabled(false));
+
+    EXPECT_EQ(err, CHIP_NO_ERROR);
+
+    bool isServerEnabled = tcp.IsServerListenEnabled();
+    EXPECT_EQ(isServerEnabled, false);
+}
+
+TEST_F(TestTCP, InitializeAsTCPClientServer)
+{
+    TCPImpl tcp;
+
+    CHIP_ERROR err = tcp.Init(Transport::TcpListenParameters(mIOContext->GetTCPEndPointManager())
+                                  .SetAddressType(IPAddressType::kIPv6)
+                                  .SetListenPort(gChipTCPPort));
+
+    EXPECT_EQ(err, CHIP_NO_ERROR);
+
+    bool isServerEnabled = tcp.IsServerListenEnabled();
+    EXPECT_EQ(isServerEnabled, true);
+}
+
 TEST_F(TestTCP, CheckMessageTest6)
 {
     IPAddress addr;