From 843db1e82d261e7cbe6d561531eb93bee7feb562 Mon Sep 17 00:00:00 2001 From: Handa Wang <7058128+superwhd@users.noreply.github.com> Date: Thu, 4 Apr 2024 01:51:50 +0800 Subject: [PATCH] [srp-server] retry other ports when failing to `prepareSocket` (#9981) Unicast SRP dataset uses a ephemeral UDP port which could be taken by another process. Currently SRP server creates the socket at the port after the server is added into netdata. However, at that moment the port may not be available on the platform so it may fail to create the socket and start the server. This commit adds the logic to restart the enabling process with another port candidate if SRP server fails to create the socket. --- src/core/net/srp_server.cpp | 41 ++++++++++++++----- src/core/net/srp_server.hpp | 4 +- .../border_router/test_advertising_proxy.py | 8 +++- tests/scripts/thread-cert/node.py | 3 ++ .../test_srp_server_reboot_port.py | 4 +- 5 files changed, 45 insertions(+), 15 deletions(-) diff --git a/src/core/net/srp_server.cpp b/src/core/net/srp_server.cpp index c5dd7c70e..1485e588c 100644 --- a/src/core/net/srp_server.cpp +++ b/src/core/net/srp_server.cpp @@ -91,7 +91,7 @@ Server::Server(Instance &aInstance) , mOutstandingUpdatesTimer(aInstance) , mCompletedUpdateTask(aInstance) , mServiceUpdateId(Random::NonCrypto::GetUint32()) - , mPort(kUdpPortMin) + , mPort(kUninitializedPort) , mState(kStateDisabled) , mAddressMode(kDefaultAddressMode) , mAnycastSequenceNumber(0) @@ -595,7 +595,7 @@ exit: } } -void Server::SelectPort(void) +void Server::InitPort(void) { mPort = kUdpPortMin; @@ -605,24 +605,35 @@ void Server::SelectPort(void) if (Get().Read(info) == kErrorNone) { - mPort = info.GetPort() + 1; - if (mPort < kUdpPortMin || mPort > kUdpPortMax) - { - mPort = kUdpPortMin; - } + mPort = info.GetPort(); } } #endif +} + +void Server::SelectPort(void) +{ + if (mPort == kUninitializedPort) + { + InitPort(); + } + ++mPort; + if (mPort < kUdpPortMin || mPort > kUdpPortMax) + { + mPort = kUdpPortMin; + } LogInfo("Selected port %u", mPort); } void Server::Start(void) { + Error error = kErrorNone; + VerifyOrExit(mState == kStateStopped); mState = kStateRunning; - PrepareSocket(); + SuccessOrExit(error = PrepareSocket()); LogInfo("Start listening on port %u", mPort); #if OPENTHREAD_CONFIG_SRP_SERVER_ADVERTISING_PROXY_ENABLE @@ -630,10 +641,15 @@ void Server::Start(void) #endif exit: - return; + // Re-enable server to select a new port. + if (error != kErrorNone) + { + Disable(); + Enable(); + } } -void Server::PrepareSocket(void) +Error Server::PrepareSocket(void) { Error error = kErrorNone; @@ -659,8 +675,11 @@ exit: if (error != kErrorNone) { LogCrit("Failed to prepare socket: %s", ErrorToString(error)); + IgnoreError(mSocket.Close()); Stop(); } + + return error; } Ip6::Udp::Socket &Server::GetSocket(void) @@ -689,7 +708,7 @@ void Server::HandleDnssdServerStateChange(void) if (mState == kStateRunning) { - PrepareSocket(); + IgnoreError(PrepareSocket()); } } diff --git a/src/core/net/srp_server.hpp b/src/core/net/srp_server.hpp index 4420593d3..ebe67d8fe 100644 --- a/src/core/net/srp_server.hpp +++ b/src/core/net/srp_server.hpp @@ -912,6 +912,7 @@ private: static constexpr AddressMode kDefaultAddressMode = static_cast(OPENTHREAD_CONFIG_SRP_SERVER_DEFAULT_ADDRESS_MODE); + static constexpr uint16_t kUninitializedPort = 0; static constexpr uint16_t kAnycastAddressModePort = 53; // Metadata for a received SRP Update message. @@ -971,8 +972,9 @@ private: void Disable(void); void Start(void); void Stop(void); + void InitPort(void); void SelectPort(void); - void PrepareSocket(void); + Error PrepareSocket(void); Ip6::Udp::Socket &GetSocket(void); LinkedList &GetHosts(void) { return mHosts; } diff --git a/tests/scripts/thread-cert/border_router/test_advertising_proxy.py b/tests/scripts/thread-cert/border_router/test_advertising_proxy.py index 5c66bf973..2ae828b55 100755 --- a/tests/scripts/thread-cert/border_router/test_advertising_proxy.py +++ b/tests/scripts/thread-cert/border_router/test_advertising_proxy.py @@ -27,7 +27,6 @@ # POSSIBILITY OF SUCH DAMAGE. # import ipaddress -import logging import unittest import config @@ -81,6 +80,12 @@ class SingleHostAndService(thread_cert.TestCase): host.start(start_radvd=False) self.simulator.go(5) + # Reserve UDP ports to verify that SRP server can skip the unavailable + # ports correctly + server.reserve_udp_port(53535) + server.reserve_udp_port(53536) + server.reserve_udp_port(53537) + self.assertEqual(server.srp_server_get_state(), 'disabled') server.srp_server_set_enabled(True) server.srp_server_set_lease_range(LEASE, LEASE, KEY_LEASE, KEY_LEASE) @@ -88,6 +93,7 @@ class SingleHostAndService(thread_cert.TestCase): self.simulator.go(config.BORDER_ROUTER_STARTUP_DELAY) self.assertEqual('leader', server.get_state()) self.assertEqual(server.srp_server_get_state(), 'running') + self.assertNotIn(server.get_srp_server_port(), [53535, 53536, 53537]) client.start() self.simulator.go(config.ROUTER_STARTUP_DELAY) diff --git a/tests/scripts/thread-cert/node.py b/tests/scripts/thread-cert/node.py index e7aa2bb38..9f5d337f9 100755 --- a/tests/scripts/thread-cert/node.py +++ b/tests/scripts/thread-cert/node.py @@ -197,6 +197,9 @@ class OtbrDocker: self.pexpect.wait() self.pexpect.proc.kill() + def reserve_udp_port(self, port): + self.bash(f'socat -u UDP6-LISTEN:{port},bindtodevice=wpan0 - &') + def destroy(self): logging.info("Destroying %s", self) self._shutdown_docker() diff --git a/tests/scripts/thread-cert/test_srp_server_reboot_port.py b/tests/scripts/thread-cert/test_srp_server_reboot_port.py index d78dc118b..b38bcfe9e 100755 --- a/tests/scripts/thread-cert/test_srp_server_reboot_port.py +++ b/tests/scripts/thread-cert/test_srp_server_reboot_port.py @@ -94,14 +94,14 @@ class SrpServerRebootPort(thread_cert.TestCase): # # 2. Reboot the server without any service registered. The server should - # listen to the same port after the reboot. + # switch to a new port after re-enabling. # old_port = server.get_srp_server_port() server.srp_server_set_enabled(False) self.simulator.go(5) server.srp_server_set_enabled(True) self.simulator.go(5) - self.assertEqual(old_port, server.get_srp_server_port()) + self.assertNotEqual(old_port, server.get_srp_server_port()) # # 3. Register a service