Restore original order of transports in TransportMgr. (project-chip#3…

…7050)

* Restore original order of transports in TransportMgr.

Address post-merge comments from PR project-chip#36979.

Keep original order of raw transports in the TransportMgr tuple because
it is part of the public API on the controller side.

When setting the TCPServerListen flag in DnssdServer, use the same value
as in TcpListenParams directly, instead of fetching from the
TransportMgr.

* Address review comments.

src/app/server/Server.cpp

@@ -116,6 +116,12 @@ CHIP_ERROR Server::Init(const ServerInitParams & initParams)
     mUserDirectedCommissioningPort = initParams.userDirectedCommissioningPort;
     mInterfaceId                   = initParams.interfaceId;
 
+#if INET_CONFIG_ENABLE_TCP_ENDPOINT
+    auto tcpListenParams = TcpListenParameters(DeviceLayer::TCPEndPointManager())
+                               .SetAddressType(IPAddressType::kIPv6)
+                               .SetListenPort(mOperationalServicePort);
+#endif
+
     CHIP_ERROR err = CHIP_NO_ERROR;
 
     VerifyOrExit(initParams.persistentStorageDelegate != nullptr, err = CHIP_ERROR_INVALID_ARGUMENT);
@@ -214,12 +220,6 @@ 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())
@@ -230,8 +230,12 @@ CHIP_ERROR Server::Init(const ServerInitParams & initParams)
                                ,
                            BleListenParameters(DeviceLayer::ConnectivityMgr().GetBleLayer())
 #endif
-#if CHIP_DEVICE_CONFIG_ENABLE_WIFIPAF
+#if INET_CONFIG_ENABLE_TCP_ENDPOINT
                                ,
+                           tcpListenParams
+#endif
+#if CHIP_DEVICE_CONFIG_ENABLE_WIFIPAF
+                           ,
                            Transport::WiFiPAFListenParameters(DeviceLayer::ConnectivityMgr().GetWiFiPAF())
 #endif
     );
@@ -333,7 +337,8 @@ CHIP_ERROR Server::Init(const ServerInitParams & initParams)
 #endif // CHIP_CONFIG_ENABLE_ICD_SERVER
 
 #if INET_CONFIG_ENABLE_TCP_ENDPOINT
-    app::DnssdServer::Instance().SetTCPServerEnabled(mTransports.GetTransport().GetImplAtIndex<1>().IsServerListenEnabled());
+    // Enable the TCP Server based on the TCPListenParameters setting.
+    app::DnssdServer::Instance().SetTCPServerEnabled(tcpListenParams.IsServerListenEnabled());
 #endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
 
     if (GetFabricTable().FabricCount() != 0)

src/app/server/Server.h

@@ -96,10 +96,6 @@ 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
@@ -108,6 +104,10 @@ 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

src/controller/CHIPDeviceControllerFactory.cpp

@@ -132,6 +132,13 @@ CHIP_ERROR DeviceControllerFactory::InitSystemState(FactoryInitParams params)
     ChipLogError(Controller, "Warning: Device Controller Factory should be with a CHIP Device Layer...");
 #endif // CONFIG_DEVICE_LAYER
 
+#if INET_CONFIG_ENABLE_TCP_ENDPOINT
+    auto tcpListenParams = Transport::TcpListenParameters(stateParams.tcpEndPointManager)
+                               .SetAddressType(IPAddressType::kIPv6)
+                               .SetListenPort(params.listenPort)
+                               .SetServerListenEnabled(false); // Initialize as a TCP Client
+#endif
+
     if (params.dataModelProvider == nullptr)
     {
         ChipLogError(AppServer, "Device Controller Factory requires a `dataModelProvider` value.");
@@ -167,15 +174,8 @@ 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_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)
@@ -184,8 +184,12 @@ CHIP_ERROR DeviceControllerFactory::InitSystemState(FactoryInitParams params)
                                                             ,
                                                         Transport::BleListenParameters(stateParams.bleLayer)
 #endif
-#if CHIP_DEVICE_CONFIG_ENABLE_WIFIPAF
+#if INET_CONFIG_ENABLE_TCP_ENDPOINT
                                                             ,
+                                                        tcpListenParams
+#endif
+#if CHIP_DEVICE_CONFIG_ENABLE_WIFIPAF
+                                                        ,
                                                         Transport::WiFiPAFListenParameters()
 #endif
                                                             ));
@@ -285,6 +289,9 @@ CHIP_ERROR DeviceControllerFactory::InitSystemState(FactoryInitParams params)
         // Consequently, reach in set the fabric table pointer to point to the right version.
         //
         app::DnssdServer::Instance().SetFabricTable(stateParams.fabricTable);
+
+        // Disable the TCP Server based on the TCPListenParameters setting.
+        app::DnssdServer::Instance().SetTCPServerEnabled(tcpListenParams.IsServerListenEnabled());
     }
 
     stateParams.sessionSetupPool = Platform::New<DeviceControllerSystemStateParams::SessionSetupPool>();

src/controller/CHIPDeviceControllerSystemState.h

@@ -71,10 +71,6 @@ 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 */
@@ -83,6 +79,10 @@ 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 */

src/transport/TransportMgrBase.cpp

@@ -44,11 +44,6 @@ 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)

src/transport/TransportMgrBase.h

@@ -50,8 +50,6 @@ 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;

src/transport/raw/Base.h

@@ -114,8 +114,6 @@ 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
 
     /**

src/transport/raw/TCP.cpp

@@ -78,14 +78,12 @@ CHIP_ERROR TCPBase::Init(TcpListenParameters & params)
 
     mEndpointType = params.GetAddressType();
 
-    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);
 
-    if (mIsServerListenEnabled)
+    if (params.IsServerListenEnabled())
     {
         err = mListenSocket->Bind(params.GetAddressType(), Inet::IPAddress::Any, params.GetListenPort(),
                                   params.GetInterfaceId().IsPresent());
@@ -677,11 +675,6 @@ 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++)

src/transport/raw/TCP.h

@@ -187,8 +187,6 @@ 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) &&
@@ -312,7 +310,6 @@ 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.

src/transport/raw/Tuple.h

@@ -107,7 +107,6 @@ 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>(); }
@@ -224,18 +223,6 @@ 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
 
     /**

src/transport/raw/tests/TestTCP.cpp

@@ -555,29 +555,26 @@ TEST_F(TestTCP, CheckSimpleInitTest6)
 TEST_F(TestTCP, InitializeAsTCPClient)
 {
     TCPImpl tcp;
-
-    CHIP_ERROR err = tcp.Init(Transport::TcpListenParameters(mIOContext->GetTCPEndPointManager())
-                                  .SetAddressType(IPAddressType::kIPv6)
-                                  .SetListenPort(gChipTCPPort)
-                                  .SetServerListenEnabled(false));
+    auto tcpListenParams = Transport::TcpListenParameters(mIOContext->GetTCPEndPointManager());
+    CHIP_ERROR err =
+        tcp.Init(tcpListenParams.SetAddressType(IPAddressType::kIPv6).SetListenPort(gChipTCPPort).SetServerListenEnabled(false));
 
     EXPECT_EQ(err, CHIP_NO_ERROR);
 
-    bool isServerEnabled = tcp.IsServerListenEnabled();
+    bool isServerEnabled = tcpListenParams.IsServerListenEnabled();
     EXPECT_EQ(isServerEnabled, false);
 }
 
 TEST_F(TestTCP, InitializeAsTCPClientServer)
 {
     TCPImpl tcp;
+    auto tcpListenParams = Transport::TcpListenParameters(mIOContext->GetTCPEndPointManager());
 
-    CHIP_ERROR err = tcp.Init(Transport::TcpListenParameters(mIOContext->GetTCPEndPointManager())
-                                  .SetAddressType(IPAddressType::kIPv6)
-                                  .SetListenPort(gChipTCPPort));
+    CHIP_ERROR err = tcp.Init(tcpListenParams.SetAddressType(IPAddressType::kIPv6).SetListenPort(gChipTCPPort));
 
     EXPECT_EQ(err, CHIP_NO_ERROR);
 
-    bool isServerEnabled = tcp.IsServerListenEnabled();
+    bool isServerEnabled = tcpListenParams.IsServerListenEnabled();
     EXPECT_EQ(isServerEnabled, true);
 }