Skip to content

Commit f23073e

Browse files
pidarpedpidarped
authored
Restore original order of transports in TransportMgr. (#37050)
* Restore original order of transports in TransportMgr. Address post-merge comments from PR #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.
1 parent 6aa39e6 commit f23073e

11 files changed

+45
-68
lines changed

‎src/app/server/Server.cpp

+13-8
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,12 @@ CHIP_ERROR Server::Init(const ServerInitParams & initParams)
116116
mUserDirectedCommissioningPort = initParams.userDirectedCommissioningPort;
117117
mInterfaceId = initParams.interfaceId;
118118

119+
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
120+
auto tcpListenParams = TcpListenParameters(DeviceLayer::TCPEndPointManager())
121+
.SetAddressType(IPAddressType::kIPv6)
122+
.SetListenPort(mOperationalServicePort);
123+
#endif
124+
119125
CHIP_ERROR err = CHIP_NO_ERROR;
120126

121127
VerifyOrExit(initParams.persistentStorageDelegate != nullptr, err = CHIP_ERROR_INVALID_ARGUMENT);
@@ -214,12 +220,6 @@ CHIP_ERROR Server::Init(const ServerInitParams & initParams)
214220
.SetAddressType(IPAddressType::kIPv6)
215221
.SetListenPort(mOperationalServicePort)
216222
.SetNativeParams(initParams.endpointNativeParams)
217-
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
218-
,
219-
TcpListenParameters(DeviceLayer::TCPEndPointManager())
220-
.SetAddressType(IPAddressType::kIPv6)
221-
.SetListenPort(mOperationalServicePort)
222-
#endif
223223
#if INET_CONFIG_ENABLE_IPV4
224224
,
225225
UdpListenParameters(DeviceLayer::UDPEndPointManager())
@@ -230,8 +230,12 @@ CHIP_ERROR Server::Init(const ServerInitParams & initParams)
230230
,
231231
BleListenParameters(DeviceLayer::ConnectivityMgr().GetBleLayer())
232232
#endif
233-
#if CHIP_DEVICE_CONFIG_ENABLE_WIFIPAF
233+
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
234234
,
235+
tcpListenParams
236+
#endif
237+
#if CHIP_DEVICE_CONFIG_ENABLE_WIFIPAF
238+
,
235239
Transport::WiFiPAFListenParameters(DeviceLayer::ConnectivityMgr().GetWiFiPAF())
236240
#endif
237241
);
@@ -333,7 +337,8 @@ CHIP_ERROR Server::Init(const ServerInitParams & initParams)
333337
#endif // CHIP_CONFIG_ENABLE_ICD_SERVER
334338

335339
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
336-
app::DnssdServer::Instance().SetTCPServerEnabled(mTransports.GetTransport().GetImplAtIndex<1>().IsServerListenEnabled());
340+
// Enable the TCP Server based on the TCPListenParameters setting.
341+
app::DnssdServer::Instance().SetTCPServerEnabled(tcpListenParams.IsServerListenEnabled());
337342
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
338343

339344
if (GetFabricTable().FabricCount() != 0)

‎src/app/server/Server.h

+4-4
Original file line numberDiff line numberDiff line change
@@ -96,10 +96,6 @@ inline constexpr size_t kMaxTcpPendingPackets = CHIP_CONFIG_MAX_TCP_PENDING_PACK
9696
// in the Server impl depends on this.
9797
//
9898
using ServerTransportMgr = chip::TransportMgr<chip::Transport::UDP
99-
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
100-
,
101-
chip::Transport::TCP<kMaxTcpActiveConnectionCount, kMaxTcpPendingPackets>
102-
#endif
10399
#if INET_CONFIG_ENABLE_IPV4
104100
,
105101
chip::Transport::UDP
@@ -108,6 +104,10 @@ using ServerTransportMgr = chip::TransportMgr<chip::Transport::UDP
108104
,
109105
chip::Transport::BLE<kMaxBlePendingPackets>
110106
#endif
107+
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
108+
,
109+
chip::Transport::TCP<kMaxTcpActiveConnectionCount, kMaxTcpPendingPackets>
110+
#endif
111111
#if CHIP_DEVICE_CONFIG_ENABLE_WIFIPAF
112112
,
113113
chip::Transport::WiFiPAFBase

‎src/controller/CHIPDeviceControllerFactory.cpp

+16-9
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,13 @@ CHIP_ERROR DeviceControllerFactory::InitSystemState(FactoryInitParams params)
132132
ChipLogError(Controller, "Warning: Device Controller Factory should be with a CHIP Device Layer...");
133133
#endif // CONFIG_DEVICE_LAYER
134134

135+
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
136+
auto tcpListenParams = Transport::TcpListenParameters(stateParams.tcpEndPointManager)
137+
.SetAddressType(IPAddressType::kIPv6)
138+
.SetListenPort(params.listenPort)
139+
.SetServerListenEnabled(false); // Initialize as a TCP Client
140+
#endif
141+
135142
if (params.dataModelProvider == nullptr)
136143
{
137144
ChipLogError(AppServer, "Device Controller Factory requires a `dataModelProvider` value.");
@@ -167,15 +174,8 @@ CHIP_ERROR DeviceControllerFactory::InitSystemState(FactoryInitParams params)
167174
ReturnErrorOnFailure(stateParams.transportMgr->Init(Transport::UdpListenParameters(stateParams.udpEndPointManager)
168175
.SetAddressType(Inet::IPAddressType::kIPv6)
169176
.SetListenPort(params.listenPort)
170-
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
171-
,
172-
Transport::TcpListenParameters(stateParams.tcpEndPointManager)
173-
.SetAddressType(IPAddressType::kIPv6)
174-
.SetListenPort(params.listenPort)
175-
.SetServerListenEnabled(false) // Initialize as a TCP Client
176-
#endif
177177
#if INET_CONFIG_ENABLE_IPV4
178-
,
178+
,
179179
Transport::UdpListenParameters(stateParams.udpEndPointManager)
180180
.SetAddressType(Inet::IPAddressType::kIPv4)
181181
.SetListenPort(params.listenPort)
@@ -184,8 +184,12 @@ CHIP_ERROR DeviceControllerFactory::InitSystemState(FactoryInitParams params)
184184
,
185185
Transport::BleListenParameters(stateParams.bleLayer)
186186
#endif
187-
#if CHIP_DEVICE_CONFIG_ENABLE_WIFIPAF
187+
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
188188
,
189+
tcpListenParams
190+
#endif
191+
#if CHIP_DEVICE_CONFIG_ENABLE_WIFIPAF
192+
,
189193
Transport::WiFiPAFListenParameters()
190194
#endif
191195
));
@@ -285,6 +289,9 @@ CHIP_ERROR DeviceControllerFactory::InitSystemState(FactoryInitParams params)
285289
// Consequently, reach in set the fabric table pointer to point to the right version.
286290
//
287291
app::DnssdServer::Instance().SetFabricTable(stateParams.fabricTable);
292+
293+
// Disable the TCP Server based on the TCPListenParameters setting.
294+
app::DnssdServer::Instance().SetTCPServerEnabled(tcpListenParams.IsServerListenEnabled());
288295
}
289296

290297
stateParams.sessionSetupPool = Platform::New<DeviceControllerSystemStateParams::SessionSetupPool>();

‎src/controller/CHIPDeviceControllerSystemState.h

+4-4
Original file line numberDiff line numberDiff line change
@@ -71,10 +71,6 @@ inline constexpr size_t kMaxDeviceTransportTcpPendingPackets = CHIP_CONFIG_MAX_T
7171

7272
using DeviceTransportMgr =
7373
TransportMgr<Transport::UDP /* IPv6 */
74-
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
75-
,
76-
Transport::TCP<kMaxDeviceTransportTcpActiveConnectionCount, kMaxDeviceTransportTcpPendingPackets>
77-
#endif
7874
#if INET_CONFIG_ENABLE_IPV4
7975
,
8076
Transport::UDP /* IPv4 */
@@ -83,6 +79,10 @@ using DeviceTransportMgr =
8379
,
8480
Transport::BLE<kMaxDeviceTransportBlePendingPackets> /* BLE */
8581
#endif
82+
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
83+
,
84+
Transport::TCP<kMaxDeviceTransportTcpActiveConnectionCount, kMaxDeviceTransportTcpPendingPackets>
85+
#endif
8686
#if CHIP_DEVICE_CONFIG_ENABLE_WIFIPAF
8787
,
8888
Transport::WiFiPAF<kMaxDeviceTransportWiFiPAFPendingPackets> /* WiFiPAF */

‎src/transport/TransportMgrBase.cpp

-5
Original file line numberDiff line numberDiff line change
@@ -44,11 +44,6 @@ void TransportMgrBase::TCPDisconnect(Transport::ActiveTCPConnectionState * conn,
4444
{
4545
mTransport->TCPDisconnect(conn, shouldAbort);
4646
}
47-
48-
bool TransportMgrBase::IsServerListenEnabled()
49-
{
50-
return mTransport->IsServerListenEnabled();
51-
}
5247
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
5348

5449
CHIP_ERROR TransportMgrBase::Init(Transport::Base * transport)

‎src/transport/TransportMgrBase.h

-2
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,6 @@ class TransportMgrBase : public Transport::RawTransportDelegate
5050

5151
void TCPDisconnect(Transport::ActiveTCPConnectionState * conn, bool shouldAbort = 0);
5252

53-
bool IsServerListenEnabled();
54-
5553
void HandleConnectionReceived(Transport::ActiveTCPConnectionState * conn) override;
5654

5755
void HandleConnectionAttemptComplete(Transport::ActiveTCPConnectionState * conn, CHIP_ERROR conErr) override;

‎src/transport/raw/Base.h

-2
Original file line numberDiff line numberDiff line change
@@ -114,8 +114,6 @@ class Base
114114
* Disconnect on the active connection that is passed in.
115115
*/
116116
virtual void TCPDisconnect(Transport::ActiveTCPConnectionState * conn, bool shouldAbort = 0) {}
117-
118-
virtual bool IsServerListenEnabled() { return true; }
119117
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
120118

121119
/**

‎src/transport/raw/TCP.cpp

+1-8
Original file line numberDiff line numberDiff line change
@@ -78,14 +78,12 @@ CHIP_ERROR TCPBase::Init(TcpListenParameters & params)
7878

7979
mEndpointType = params.GetAddressType();
8080

81-
mIsServerListenEnabled = params.IsServerListenEnabled();
82-
8381
// Primary socket endpoint created to help get EndPointManager handle for creating multiple
8482
// connection endpoints at runtime.
8583
err = params.GetEndPointManager()->NewEndPoint(&mListenSocket);
8684
SuccessOrExit(err);
8785

88-
if (mIsServerListenEnabled)
86+
if (params.IsServerListenEnabled())
8987
{
9088
err = mListenSocket->Bind(params.GetAddressType(), Inet::IPAddress::Any, params.GetListenPort(),
9189
params.GetInterfaceId().IsPresent());
@@ -677,11 +675,6 @@ void TCPBase::TCPDisconnect(Transport::ActiveTCPConnectionState * conn, bool sho
677675
}
678676
}
679677

680-
bool TCPBase::IsServerListenEnabled()
681-
{
682-
return mIsServerListenEnabled;
683-
}
684-
685678
bool TCPBase::HasActiveConnections() const
686679
{
687680
for (size_t i = 0; i < mActiveConnectionsSize; i++)

‎src/transport/raw/TCP.h

-3
Original file line numberDiff line numberDiff line change
@@ -187,8 +187,6 @@ class DLL_EXPORT TCPBase : public Base
187187
// and release from the pool.
188188
void TCPDisconnect(Transport::ActiveTCPConnectionState * conn, bool shouldAbort = false) override;
189189

190-
bool IsServerListenEnabled() override;
191-
192190
bool CanSendToPeer(const PeerAddress & address) override
193191
{
194192
return (mState == TCPState::kInitialized) && (address.GetTransportType() == Type::kTcp) &&
@@ -312,7 +310,6 @@ class DLL_EXPORT TCPBase : public Base
312310
Inet::TCPEndPoint * mListenSocket = nullptr; ///< TCP socket used by the transport
313311
Inet::IPAddressType mEndpointType = Inet::IPAddressType::kUnknown; ///< Socket listening type
314312
TCPState mState = TCPState::kNotReady; ///< State of the TCP transport
315-
bool mIsServerListenEnabled = true; ///< TCP Server mode enabled
316313

317314
// The configured timeout for the connection attempt to the peer, before
318315
// giving up.

‎src/transport/raw/Tuple.h

-13
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,6 @@ class Tuple : public Base
107107
return TCPDisconnectImpl<0>(conn, shouldAbort);
108108
}
109109

110-
bool IsServerListenEnabled() override { return IsServerListenEnabledImpl<0>(); }
111110
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
112111

113112
void Close() override { return CloseImpl<0>(); }
@@ -224,18 +223,6 @@ class Tuple : public Base
224223
template <size_t N, typename std::enable_if<(N >= sizeof...(TransportTypes))>::type * = nullptr>
225224
void TCPDisconnectImpl(Transport::ActiveTCPConnectionState * conn, bool shouldAbort = 0)
226225
{}
227-
228-
template <size_t N, typename std::enable_if<(N < sizeof...(TransportTypes))>::type * = nullptr>
229-
bool IsServerListenEnabledImpl()
230-
{
231-
return std::get<N>(mTransports).IsServerListenEnabled() || IsServerListenEnabledImpl<N + 1>();
232-
}
233-
234-
template <size_t N, typename std::enable_if<(N >= sizeof...(TransportTypes))>::type * = nullptr>
235-
bool IsServerListenEnabledImpl()
236-
{
237-
return false;
238-
}
239226
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
240227

241228
/**

‎src/transport/raw/tests/TestTCP.cpp

+7-10
Original file line numberDiff line numberDiff line change
@@ -555,29 +555,26 @@ TEST_F(TestTCP, CheckSimpleInitTest6)
555555
TEST_F(TestTCP, InitializeAsTCPClient)
556556
{
557557
TCPImpl tcp;
558-
559-
CHIP_ERROR err = tcp.Init(Transport::TcpListenParameters(mIOContext->GetTCPEndPointManager())
560-
.SetAddressType(IPAddressType::kIPv6)
561-
.SetListenPort(gChipTCPPort)
562-
.SetServerListenEnabled(false));
558+
auto tcpListenParams = Transport::TcpListenParameters(mIOContext->GetTCPEndPointManager());
559+
CHIP_ERROR err =
560+
tcp.Init(tcpListenParams.SetAddressType(IPAddressType::kIPv6).SetListenPort(gChipTCPPort).SetServerListenEnabled(false));
563561

564562
EXPECT_EQ(err, CHIP_NO_ERROR);
565563

566-
bool isServerEnabled = tcp.IsServerListenEnabled();
564+
bool isServerEnabled = tcpListenParams.IsServerListenEnabled();
567565
EXPECT_EQ(isServerEnabled, false);
568566
}
569567

570568
TEST_F(TestTCP, InitializeAsTCPClientServer)
571569
{
572570
TCPImpl tcp;
571+
auto tcpListenParams = Transport::TcpListenParameters(mIOContext->GetTCPEndPointManager());
573572

574-
CHIP_ERROR err = tcp.Init(Transport::TcpListenParameters(mIOContext->GetTCPEndPointManager())
575-
.SetAddressType(IPAddressType::kIPv6)
576-
.SetListenPort(gChipTCPPort));
573+
CHIP_ERROR err = tcp.Init(tcpListenParams.SetAddressType(IPAddressType::kIPv6).SetListenPort(gChipTCPPort));
577574

578575
EXPECT_EQ(err, CHIP_NO_ERROR);
579576

580-
bool isServerEnabled = tcp.IsServerListenEnabled();
577+
bool isServerEnabled = tcpListenParams.IsServerListenEnabled();
581578
EXPECT_EQ(isServerEnabled, true);
582579
}
583580

0 commit comments

Comments
 (0)