From ba08ed6f3497495a52eb8934b307d5173dd10979 Mon Sep 17 00:00:00 2001 From: kangping Date: Tue, 2 Nov 2021 00:16:31 +0800 Subject: [PATCH] [tests] fix flaky advertising proxy tests (#7105) - double wait time for advertising proxy top update and multicast the SRP updates - adjust waiting time to avoid collision with lease time schedule - fully remove the service before starting next test case - add debug logging --- src/core/net/srp_server.cpp | 15 ++++++++++++--- .../border_router/test_advertising_proxy.py | 13 +++++++++++-- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/src/core/net/srp_server.cpp b/src/core/net/srp_server.cpp index 80b9adb7c..608e2a2fa 100644 --- a/src/core/net/srp_server.cpp +++ b/src/core/net/srp_server.cpp @@ -283,7 +283,10 @@ void Server::RemoveHost(Host *aHost, bool aRetainName, bool aNotifyServiceHandle if (aNotifyServiceHandler && mServiceUpdateHandler != nullptr) { - mServiceUpdateHandler(AllocateId(), aHost, kDefaultEventsHandlerTimeout, mServiceUpdateHandlerContext); + uint32_t updateId = AllocateId(); + + otLogInfoSrp("[server] SRP update handler is notified (updatedId = %u)", updateId); + mServiceUpdateHandler(updateId, aHost, kDefaultEventsHandlerTimeout, mServiceUpdateHandlerContext); // We don't wait for the reply from the service update handler, // but always remove the host (and its services) regardless of // host/service update result. Because removing a host should fail @@ -345,6 +348,9 @@ void Server::HandleServiceUpdateResult(ServiceUpdateId aId, Error aError) void Server::HandleServiceUpdateResult(UpdateMetadata *aUpdate, Error aError) { + otLogInfoSrp("[server] handler result of SRP update (id = %u) is received: %s", aUpdate->GetId(), + otThreadErrorToString(aError)); + IgnoreError(mOutstandingUpdates.Remove(*aUpdate)); CommitSrpUpdate(aError, aUpdate->GetDnsHeader(), aUpdate->GetHost(), aUpdate->GetMessageInfo()); aUpdate->Free(); @@ -1148,6 +1154,7 @@ exit: IgnoreError(mOutstandingUpdates.Add(*update)); mOutstandingUpdatesTimer.StartAt(mOutstandingUpdates.GetTail()->GetExpireTime(), 0); + otLogInfoSrp("[server] SRP update handler is notified (updatedId = %u)", update->GetId()); mServiceUpdateHandler(update->GetId(), &aHost, kDefaultEventsHandlerTimeout, mServiceUpdateHandlerContext); } else @@ -1809,8 +1816,10 @@ void Server::Host::RemoveService(Service *aService, bool aRetainName, bool aNoti if (aNotifyServiceHandler && server.mServiceUpdateHandler != nullptr) { - server.mServiceUpdateHandler(server.AllocateId(), this, kDefaultEventsHandlerTimeout, - server.mServiceUpdateHandlerContext); + uint32_t updateId = server.AllocateId(); + + otLogInfoSrp("[server] SRP update handler is notified (updatedId = %u)", updateId); + server.mServiceUpdateHandler(updateId, this, kDefaultEventsHandlerTimeout, server.mServiceUpdateHandlerContext); // We don't wait for the reply from the service update handler, // but always remove the service regardless of service update result. // Because removing a service should fail only when there is system 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 a6cb68921..86658ab12 100755 --- a/tests/scripts/thread-cert/border_router/test_advertising_proxy.py +++ b/tests/scripts/thread-cert/border_router/test_advertising_proxy.py @@ -181,12 +181,21 @@ class SingleHostAndService(thread_cert.TestCase): # client.srp_client_remove_service('my-service-1', '_ipps._tcp') - self.simulator.go(5) + + # We previously had self.simulator.go(5) but got the issue that the client is scheduling + # the refresh timer with half of the lease time (10 seconds) and there will be chances + # that the client will be just in status "kToRefresh" after self.simulator.go(5). This will + # fail the checks in self.check_host_and_service() so updated to wait for 2 seconds. + self.simulator.go(2) self.check_host_and_service(server, client, '2001::2', 'my-service') self.host_check_mdns_service(host, '2001::2', 'my-service') self.assertIsNone(host.discover_mdns_service('my-service-1', '_ipps._tcp', 'my-host')) + # Wait for KEY expiration of service 'my-service-1'. + # FIXME: workaround for https://github.com/openthread/ot-br-posix/issues/1071. + self.simulator.go(KEY_LEASE + 5) + # # 8. Update both the host and the service in a loop and make sure the # Advertising Proxy can follow. @@ -197,7 +206,7 @@ class SingleHostAndService(thread_cert.TestCase): client.srp_client_clear_service('my-service', '_ipps._tcp') client.srp_client_set_host_address(host_address) client.srp_client_add_service('my-service', '_ipps._tcp', service_port) - self.simulator.go(5) + self.simulator.go(10) self.check_host_and_service(server, client, host_address, 'my-service', service_port) self.host_check_mdns_service(host, host_address, 'my-service', service_port)