Skip to content

Commit 0129a57

Browse files
pidarpedandy31415
andauthored
Add separate knob for TCP server listen enabling. (#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 <andy314@gmail.com> --------- Co-authored-by: Andrei Litvin <andy314@gmail.com>
1 parent b83c27a commit 0129a57

13 files changed

+123
-37
lines changed

src/app/server/Dnssd.cpp

+2-1
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,8 @@ CHIP_ERROR DnssdServer::AdvertiseOperational()
214214
#endif
215215

216216
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
217-
advertiseParameters.SetTCPSupportModes(chip::Dnssd::TCPModeAdvertise::kTCPClientServer);
217+
advertiseParameters.SetTCPSupportModes(mTCPServerEnabled ? chip::Dnssd::TCPModeAdvertise::kTCPClientServer
218+
: chip::Dnssd::TCPModeAdvertise::kTCPClient);
218219
#endif
219220
auto & mdnsAdvertiser = chip::Dnssd::ServiceAdvertiser::Instance();
220221

src/app/server/Dnssd.h

+8
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,10 @@ class DLL_EXPORT DnssdServer : public ICDStateObserver
9797
void SetICDManager(ICDManager * manager) { mICDManager = manager; };
9898
#endif
9999

100+
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
101+
void SetTCPServerEnabled(bool serverEnabled) { mTCPServerEnabled = serverEnabled; };
102+
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
103+
100104
/// Start operational advertising
101105
CHIP_ERROR AdvertiseOperational();
102106

@@ -179,6 +183,10 @@ class DLL_EXPORT DnssdServer : public ICDStateObserver
179183
ICDManager * mICDManager = nullptr;
180184
#endif // CHIP_CONFIG_ENABLE_ICD_SERVER
181185

186+
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
187+
bool mTCPServerEnabled = true;
188+
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
189+
182190
uint16_t mSecuredPort = CHIP_PORT;
183191
uint16_t mUnsecuredPort = CHIP_UDC_PORT;
184192
Inet::InterfaceId mInterfaceId = Inet::InterfaceId::Null();

src/app/server/Server.cpp

+10-7
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,12 @@ CHIP_ERROR Server::Init(const ServerInitParams & initParams)
214214
.SetAddressType(IPAddressType::kIPv6)
215215
.SetListenPort(mOperationalServicePort)
216216
.SetNativeParams(initParams.endpointNativeParams)
217-
217+
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
218+
,
219+
TcpListenParameters(DeviceLayer::TCPEndPointManager())
220+
.SetAddressType(IPAddressType::kIPv6)
221+
.SetListenPort(mOperationalServicePort)
222+
#endif
218223
#if INET_CONFIG_ENABLE_IPV4
219224
,
220225
UdpListenParameters(DeviceLayer::UDPEndPointManager())
@@ -225,12 +230,6 @@ CHIP_ERROR Server::Init(const ServerInitParams & initParams)
225230
,
226231
BleListenParameters(DeviceLayer::ConnectivityMgr().GetBleLayer())
227232
#endif
228-
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
229-
,
230-
TcpListenParameters(DeviceLayer::TCPEndPointManager())
231-
.SetAddressType(IPAddressType::kIPv6)
232-
.SetListenPort(mOperationalServicePort)
233-
#endif
234233
#if CHIP_DEVICE_CONFIG_ENABLE_WIFIPAF
235234
,
236235
Transport::WiFiPAFListenParameters(DeviceLayer::ConnectivityMgr().GetWiFiPAF())
@@ -330,6 +329,10 @@ CHIP_ERROR Server::Init(const ServerInitParams & initParams)
330329
app::DnssdServer::Instance().SetICDManager(&mICDManager);
331330
#endif // CHIP_CONFIG_ENABLE_ICD_SERVER
332331

332+
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
333+
app::DnssdServer::Instance().SetTCPServerEnabled(mTransports.GetTransport().GetImplAtIndex<1>().IsServerListenEnabled());
334+
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
335+
333336
if (GetFabricTable().FabricCount() != 0)
334337
{
335338
#if CONFIG_NETWORK_LAYER_BLE

src/app/server/Server.h

+4-4
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,10 @@ 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
99103
#if INET_CONFIG_ENABLE_IPV4
100104
,
101105
chip::Transport::UDP
@@ -104,10 +108,6 @@ using ServerTransportMgr = chip::TransportMgr<chip::Transport::UDP
104108
,
105109
chip::Transport::BLE<kMaxBlePendingPackets>
106110
#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

+8-7
Original file line numberDiff line numberDiff line change
@@ -167,8 +167,15 @@ CHIP_ERROR DeviceControllerFactory::InitSystemState(FactoryInitParams params)
167167
ReturnErrorOnFailure(stateParams.transportMgr->Init(Transport::UdpListenParameters(stateParams.udpEndPointManager)
168168
.SetAddressType(Inet::IPAddressType::kIPv6)
169169
.SetListenPort(params.listenPort)
170-
#if INET_CONFIG_ENABLE_IPV4
170+
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
171171
,
172+
Transport::TcpListenParameters(stateParams.tcpEndPointManager)
173+
.SetAddressType(IPAddressType::kIPv6)
174+
.SetListenPort(params.listenPort)
175+
.SetServerListenEnabled(false) // Initialize as a TCP Client
176+
#endif
177+
#if INET_CONFIG_ENABLE_IPV4
178+
,
172179
Transport::UdpListenParameters(stateParams.udpEndPointManager)
173180
.SetAddressType(Inet::IPAddressType::kIPv4)
174181
.SetListenPort(params.listenPort)
@@ -177,12 +184,6 @@ CHIP_ERROR DeviceControllerFactory::InitSystemState(FactoryInitParams params)
177184
,
178185
Transport::BleListenParameters(stateParams.bleLayer)
179186
#endif
180-
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
181-
,
182-
Transport::TcpListenParameters(stateParams.tcpEndPointManager)
183-
.SetAddressType(IPAddressType::kIPv6)
184-
.SetListenPort(params.listenPort)
185-
#endif
186187
#if CHIP_DEVICE_CONFIG_ENABLE_WIFIPAF
187188
,
188189
Transport::WiFiPAFListenParameters()

src/controller/CHIPDeviceControllerSystemState.h

+4-4
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,10 @@ 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
7478
#if INET_CONFIG_ENABLE_IPV4
7579
,
7680
Transport::UDP /* IPv4 */
@@ -79,10 +83,6 @@ using DeviceTransportMgr =
7983
,
8084
Transport::BLE<kMaxDeviceTransportBlePendingPackets> /* BLE */
8185
#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,6 +44,11 @@ void TransportMgrBase::TCPDisconnect(Transport::ActiveTCPConnectionState * conn,
4444
{
4545
mTransport->TCPDisconnect(conn, shouldAbort);
4646
}
47+
48+
bool TransportMgrBase::IsServerListenEnabled()
49+
{
50+
return mTransport->IsServerListenEnabled();
51+
}
4752
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
4853

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

src/transport/TransportMgrBase.h

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

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

53+
bool IsServerListenEnabled();
54+
5355
void HandleConnectionReceived(Transport::ActiveTCPConnectionState * conn) override;
5456

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

src/transport/raw/Base.h

+2
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,8 @@ 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; }
117119
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
118120

119121
/**

src/transport/raw/TCP.cpp

+23-14
Original file line numberDiff line numberDiff line change
@@ -76,25 +76,29 @@ CHIP_ERROR TCPBase::Init(TcpListenParameters & params)
7676

7777
VerifyOrExit(mState == TCPState::kNotReady, err = CHIP_ERROR_INCORRECT_STATE);
7878

79-
#if INET_CONFIG_ENABLE_TCP_ENDPOINT
80-
err = params.GetEndPointManager()->NewEndPoint(&mListenSocket);
81-
#else
82-
err = CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE;
83-
#endif
84-
SuccessOrExit(err);
79+
mEndpointType = params.GetAddressType();
8580

86-
err = mListenSocket->Bind(params.GetAddressType(), Inet::IPAddress::Any, params.GetListenPort(),
87-
params.GetInterfaceId().IsPresent());
81+
mIsServerListenEnabled = params.IsServerListenEnabled();
82+
83+
// Primary socket endpoint created to help get EndPointManager handle for creating multiple
84+
// connection endpoints at runtime.
85+
err = params.GetEndPointManager()->NewEndPoint(&mListenSocket);
8886
SuccessOrExit(err);
8987

90-
mListenSocket->mAppState = reinterpret_cast<void *>(this);
91-
mListenSocket->OnConnectionReceived = HandleIncomingConnection;
92-
mListenSocket->OnAcceptError = HandleAcceptError;
88+
if (mIsServerListenEnabled)
89+
{
90+
err = mListenSocket->Bind(params.GetAddressType(), Inet::IPAddress::Any, params.GetListenPort(),
91+
params.GetInterfaceId().IsPresent());
92+
SuccessOrExit(err);
9393

94-
mEndpointType = params.GetAddressType();
94+
mListenSocket->mAppState = reinterpret_cast<void *>(this);
95+
mListenSocket->OnConnectionReceived = HandleIncomingConnection;
96+
mListenSocket->OnAcceptError = HandleAcceptError;
9597

96-
err = mListenSocket->Listen(kListenBacklogSize);
97-
SuccessOrExit(err);
98+
err = mListenSocket->Listen(kListenBacklogSize);
99+
SuccessOrExit(err);
100+
ChipLogProgress(Inet, "TCP server listening on port %d for incoming connections", params.GetListenPort());
101+
}
98102

99103
mState = TCPState::kInitialized;
100104

@@ -673,6 +677,11 @@ void TCPBase::TCPDisconnect(Transport::ActiveTCPConnectionState * conn, bool sho
673677
}
674678
}
675679

680+
bool TCPBase::IsServerListenEnabled()
681+
{
682+
return mIsServerListenEnabled;
683+
}
684+
676685
bool TCPBase::HasActiveConnections() const
677686
{
678687
for (size_t i = 0; i < mActiveConnectionsSize; i++)

src/transport/raw/TCP.h

+12
Original file line numberDiff line numberDiff line change
@@ -79,11 +79,20 @@ class TcpListenParameters
7979
return *this;
8080
}
8181

82+
bool IsServerListenEnabled() const { return mServerListenEnabled; }
83+
TcpListenParameters & SetServerListenEnabled(bool listenEnable)
84+
{
85+
mServerListenEnabled = listenEnable;
86+
87+
return *this;
88+
}
89+
8290
private:
8391
Inet::EndPointManager<Inet::TCPEndPoint> * mEndPointManager; ///< Associated endpoint factory
8492
Inet::IPAddressType mAddressType = Inet::IPAddressType::kIPv6; ///< type of listening socket
8593
uint16_t mListenPort = CHIP_PORT; ///< TCP listen port
8694
Inet::InterfaceId mInterfaceId = Inet::InterfaceId::Null(); ///< Interface to listen on
95+
bool mServerListenEnabled = true; ///< TCP Server mode enabled
8796
};
8897

8998
/**
@@ -178,6 +187,8 @@ class DLL_EXPORT TCPBase : public Base
178187
// and release from the pool.
179188
void TCPDisconnect(Transport::ActiveTCPConnectionState * conn, bool shouldAbort = false) override;
180189

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

305317
// The configured timeout for the connection attempt to the peer, before
306318
// giving up.

src/transport/raw/Tuple.h

+14
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,8 @@ class Tuple : public Base
106106
{
107107
return TCPDisconnectImpl<0>(conn, shouldAbort);
108108
}
109+
110+
bool IsServerListenEnabled() override { return IsServerListenEnabledImpl<0>(); }
109111
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
110112

111113
void Close() override { return CloseImpl<0>(); }
@@ -222,6 +224,18 @@ class Tuple : public Base
222224
template <size_t N, typename std::enable_if<(N >= sizeof...(TransportTypes))>::type * = nullptr>
223225
void TCPDisconnectImpl(Transport::ActiveTCPConnectionState * conn, bool shouldAbort = 0)
224226
{}
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+
}
225239
#endif // INET_CONFIG_ENABLE_TCP_ENDPOINT
226240

227241
/**

src/transport/raw/tests/TestTCP.cpp

+29
Original file line numberDiff line numberDiff line change
@@ -552,6 +552,35 @@ TEST_F(TestTCP, CheckSimpleInitTest6)
552552
CheckSimpleInitTest(IPAddressType::kIPv6);
553553
}
554554

555+
TEST_F(TestTCP, InitializeAsTCPClient)
556+
{
557+
TCPImpl tcp;
558+
559+
CHIP_ERROR err = tcp.Init(Transport::TcpListenParameters(mIOContext->GetTCPEndPointManager())
560+
.SetAddressType(IPAddressType::kIPv6)
561+
.SetListenPort(gChipTCPPort)
562+
.SetServerListenEnabled(false));
563+
564+
EXPECT_EQ(err, CHIP_NO_ERROR);
565+
566+
bool isServerEnabled = tcp.IsServerListenEnabled();
567+
EXPECT_EQ(isServerEnabled, false);
568+
}
569+
570+
TEST_F(TestTCP, InitializeAsTCPClientServer)
571+
{
572+
TCPImpl tcp;
573+
574+
CHIP_ERROR err = tcp.Init(Transport::TcpListenParameters(mIOContext->GetTCPEndPointManager())
575+
.SetAddressType(IPAddressType::kIPv6)
576+
.SetListenPort(gChipTCPPort));
577+
578+
EXPECT_EQ(err, CHIP_NO_ERROR);
579+
580+
bool isServerEnabled = tcp.IsServerListenEnabled();
581+
EXPECT_EQ(isServerEnabled, true);
582+
}
583+
555584
TEST_F(TestTCP, CheckMessageTest6)
556585
{
557586
IPAddress addr;

0 commit comments

Comments
 (0)