Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TCP over IPv4 unsupported - inconsistent with UDP #37023

Open
samadDotDev opened this issue Jan 10, 2025 · 1 comment
Open

TCP over IPv4 unsupported - inconsistent with UDP #37023

samadDotDev opened this issue Jan 10, 2025 · 1 comment
Labels
bug Something isn't working needs triage

Comments

@samadDotDev
Copy link
Contributor

Reproduction steps

The UDP transport is allocated twice: once for IPv6, and if INET_CONFIG_ENABLE_IPV4 enabled (default for various platforms including linux) then also for IPv4, both on the server as well as controller/client side. However, the TCP transport seems to be only allocated with an endpoint and with listen parameters only for IPv6, even though the underlying endpoint supports IPv4 with the same compiler flag.

Other than probably expanding some unit tests, I believe the fix is the following which I also validated between a controller and server app:

diff --git a/src/app/server/Server.cpp b/src/app/server/Server.cpp
index 2afb524213..9ac1c7a94d 100644
--- a/src/app/server/Server.cpp
+++ b/src/app/server/Server.cpp
@@ -225,6 +225,12 @@ CHIP_ERROR Server::Init(const ServerInitParams & initParams)
                            UdpListenParameters(DeviceLayer::UDPEndPointManager())
                                .SetAddressType(IPAddressType::kIPv4)
                                .SetListenPort(mOperationalServicePort)
+#if INET_CONFIG_ENABLE_TCP_ENDPOINT
+                               ,
+                           TcpListenParameters(DeviceLayer::TCPEndPointManager())
+                               .SetAddressType(IPAddressType::kIPv4)
+                               .SetListenPort(mOperationalServicePort)
+#endif
 #endif
 #if CONFIG_NETWORK_LAYER_BLE
                                ,
diff --git a/src/app/server/Server.h b/src/app/server/Server.h
index d47a953ae5..4df40c97b9 100644
--- a/src/app/server/Server.h
+++ b/src/app/server/Server.h
@@ -103,6 +103,10 @@ using ServerTransportMgr = chip::TransportMgr<chip::Transport::UDP
 #if INET_CONFIG_ENABLE_IPV4
                                               ,
                                               chip::Transport::UDP
+#if INET_CONFIG_ENABLE_TCP_ENDPOINT
+                                              ,
+                                              chip::Transport::TCP<kMaxTcpActiveConnectionCount, kMaxTcpPendingPackets>
+#endif
 #endif
 #if CONFIG_NETWORK_LAYER_BLE
                                               ,
diff --git a/src/controller/CHIPDeviceControllerFactory.cpp b/src/controller/CHIPDeviceControllerFactory.cpp
index 50b725a791..9b2101f05c 100644
--- a/src/controller/CHIPDeviceControllerFactory.cpp
+++ b/src/controller/CHIPDeviceControllerFactory.cpp
@@ -179,9 +179,16 @@ CHIP_ERROR DeviceControllerFactory::InitSystemState(FactoryInitParams params)
                                                         Transport::UdpListenParameters(stateParams.udpEndPointManager)
                                                             .SetAddressType(Inet::IPAddressType::kIPv4)
                                                             .SetListenPort(params.listenPort)
+#if INET_CONFIG_ENABLE_TCP_ENDPOINT
+                                                            ,
+                                                        Transport::TcpListenParameters(stateParams.tcpEndPointManager)
+                                                            .SetAddressType(IPAddressType::kIPv4)
+                                                            .SetListenPort(params.listenPort)
+                                                            .SetServerListenEnabled(false) // Initialize as a TCP Client
+#endif
 #endif
 #if CONFIG_NETWORK_LAYER_BLE
-                                                            ,
+                                                        ,
                                                         Transport::BleListenParameters(stateParams.bleLayer)
 #endif
 #if CHIP_DEVICE_CONFIG_ENABLE_WIFIPAF
diff --git a/src/controller/CHIPDeviceControllerSystemState.h b/src/controller/CHIPDeviceControllerSystemState.h
index 7ee79ff9c0..96d7313b0b 100644
--- a/src/controller/CHIPDeviceControllerSystemState.h
+++ b/src/controller/CHIPDeviceControllerSystemState.h
@@ -78,6 +78,10 @@ using DeviceTransportMgr =
 #if INET_CONFIG_ENABLE_IPV4
                  ,
                  Transport::UDP /* IPv4 */
+#if INET_CONFIG_ENABLE_TCP_ENDPOINT
+                 ,
+                 Transport::TCP<kMaxDeviceTransportTcpActiveConnectionCount, kMaxDeviceTransportTcpPendingPackets> /* IPv4 */
+#endif
 #endif
 #if CONFIG_NETWORK_LAYER_BLE
                  ,

Bug prevalence

Consistent

GitHub hash of the SDK that was being used

3f62505

Platform

raspi, other, core

Platform Version(s)

No response

Anything else?

No response

@samadDotDev samadDotDev added bug Something isn't working needs triage labels Jan 10, 2025
@samadDotDev
Copy link
Contributor Author

@pidarped I'd also like to confirm if this was an intentional choice for TCP even if it is inconsistent with UDP, so we can make appropriate assumptions on the controller side for 1.4 devices.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working needs triage
Projects
None yet
Development

No branches or pull requests

1 participant