espressif/openthread
1
0
Fork 0
mirror of https://github.com/espressif/openthread.git synced 2026-07-30 23:57:47 +00:00
Code Issues Packages Projects Releases Wiki Activity

[srp-server] fix removing services when the host is expired (#6649)

Browse Source 
The SRP server tries to notify the advertising proxy when a service is
expired.  The expired service is firstly removed from the service list
and later added back to the head of the service list if the service
name should be retailed. This will result in deadloop when there are
two or more services on the same host.

This commit fixes this issue by keeping the expired service in the
service list by simply mark the service deleted and pass all services
of the host to the advertising proxy. The advertising proxy will
republish those services that are not expired but this should be fine
since service expiration is not a common case.

Also update the advertising proxy test case to cover multiple
services.
This commit is contained in:
kangping
2021-05-20 09:07:39 -07:00
committed by GitHub
parent 713231e144
commit 9eff7616b3
2 changed files with 33 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/core/net/srp_server.cpp
+5 -16
View File
@@ -1223,11 +1223,12 @@ void Server::HandleLeaseTimer(void)
otLogInfoSrp("[server] LEASE of host %s expired", host->GetFullName());
// If the host expired, delete all resources of this host and its services.
RemoveHost(host, /* aRetainName */ true, /* aNotifyServiceHandler */ true);
while ((service = host->GetNextService(service)) != nullptr)
{
host->RemoveService(service, /* aRetainName */ true, /* aNotifyServiceHandler */ true);
// Don't need to notify the service handler as `RemoveHost` at below will do.
host->RemoveService(service, /* aRetainName */ true, /* aNotifyServiceHandler */ false);
}
RemoveHost(host, /* aRetainName */ true, /* aNotifyServiceHandler */ true);
earliestExpireTime = OT_MIN(earliestExpireTime, host->GetKeyExpireTime());
}
@@ -1576,15 +1577,8 @@ void Server::Host::RemoveService(Service *aService, bool aRetainName, bool aNoti
otLogInfoSrp("[server] fully remove service '%s'", aService->mFullName);
}
IgnoreError(mServices.Remove(*aService));
if (aNotifyServiceHandler && server.mServiceUpdateHandler != nullptr)
{
LinkedList<Service> remainingServices = mServices;
mServices.Clear();
IgnoreError(mServices.Add(*aService));
server.mServiceUpdateHandler(server.AllocateId(), this, kDefaultEventsHandlerTimeout,
server.mServiceUpdateHandlerContext);
// We don't wait for the reply from the service update handler,
@@ -1592,16 +1586,11 @@ void Server::Host::RemoveService(Service *aService, bool aRetainName, bool aNoti
// Because removing a service should fail only when there is system
// failure of the platform mDNS implementation and in which case the
// service is not expected to be still registered.
mServices = remainingServices;
}
if (aRetainName)
{
IgnoreError(mServices.Add(*aService));
}
else
if (!aRetainName)
{
IgnoreError(mServices.Remove(*aService));
aService->Free();
}
tests/scripts/thread-cert/border_router/test_advertising_proxy.py
+28 -13
View File
@@ -97,17 +97,20 @@ class SingleHostAndService(thread_cert.TestCase):
client.srp_client_set_host_name('my-host')
client.srp_client_set_host_address('2001::1')
client.srp_client_add_service('my-service', '_ipps._tcp', 12345)
client.srp_client_add_service('my-service-1', '_ipps._tcp', 12345)
client.srp_client_enable_auto_start_mode()
self.simulator.go(2)
self.check_host_and_service(server, client, '2001::1')
self.check_host_and_service(server, client, '2001::1', 'my-service')
self.check_host_and_service(server, client, '2001::1', 'my-service-1')
#
# 2. Discover the service by the HOST on the ethernet. This makes sure
# the Advertising Proxy multicasts the same service on ethernet.
#
self.host_check_mdns_service(host, '2001::1')
self.host_check_mdns_service(host, '2001::1', 'my-service')
self.host_check_mdns_service(host, '2001::1', 'my-service-1')
#
# 3. Check if the Advertising Proxy removes the service from ethernet
@@ -118,6 +121,7 @@ class SingleHostAndService(thread_cert.TestCase):
self.simulator.go(2)
self.assertIsNone(host.discover_mdns_service('my-service', '_ipps._tcp', 'my-host'))
self.assertIsNone(host.discover_mdns_service('my-service-1', '_ipps._tcp', 'my-host'))
#
# 4. Check if we can discover the mDNS service again when re-registering the
@@ -127,10 +131,13 @@ class SingleHostAndService(thread_cert.TestCase):
client.srp_client_set_host_name('my-host')
client.srp_client_set_host_address('2001::1')
client.srp_client_add_service('my-service', '_ipps._tcp', 12345)
client.srp_client_add_service('my-service-1', '_ipps._tcp', 12345)
self.simulator.go(2)
self.check_host_and_service(server, client, '2001::1')
self.host_check_mdns_service(host, '2001::1')
self.check_host_and_service(server, client, '2001::1', 'my-service')
self.check_host_and_service(server, client, '2001::1', 'my-service-1')
self.host_check_mdns_service(host, '2001::1', 'my-service')
self.host_check_mdns_service(host, '2001::1', 'my-service-1')
#
# 5. Update the SRP host address and make sure the Advertising Proxy
@@ -140,8 +147,10 @@ class SingleHostAndService(thread_cert.TestCase):
client.srp_client_set_host_address('2001::2')
self.simulator.go(8)
self.check_host_and_service(server, client, '2001::2')
self.host_check_mdns_service(host, '2001::2')
self.check_host_and_service(server, client, '2001::2', 'my-service')
self.check_host_and_service(server, client, '2001::2', 'my-service-1')
self.host_check_mdns_service(host, '2001::2', 'my-service')
self.host_check_mdns_service(host, '2001::2', 'my-service-1')
#
# 6. Check if the service is removed by the Advertising Proxy when the SRP server is stopped.
@@ -153,12 +162,15 @@ class SingleHostAndService(thread_cert.TestCase):
self.assertEqual(len(server.srp_server_get_hosts()), 0)
self.assertEqual(len(server.srp_server_get_services()), 0)
self.assertIsNone(host.discover_mdns_service('my-service', '_ipps._tcp', 'my-host'))
self.assertIsNone(host.discover_mdns_service('my-service-1', '_ipps._tcp', 'my-host'))
server.srp_server_set_enabled(True)
self.simulator.go(LEASE)
self.check_host_and_service(server, client, '2001::2')
self.host_check_mdns_service(host, '2001::2')
self.check_host_and_service(server, client, '2001::2', 'my-service')
self.check_host_and_service(server, client, '2001::2', 'my-service-1')
self.host_check_mdns_service(host, '2001::2', 'my-service')
self.host_check_mdns_service(host, '2001::2', 'my-service-1')
#
# 7. Check if the expired service is removed by the Advertising Proxy.
@@ -168,11 +180,12 @@ class SingleHostAndService(thread_cert.TestCase):
self.simulator.go(LEASE + 2)
self.assertIsNone(host.discover_mdns_service('my-service', '_ipps._tcp', 'my-host'))
self.assertIsNone(host.discover_mdns_service('my-service-1', '_ipps._tcp', 'my-host'))
def host_check_mdns_service(self, host, host_addr):
service = host.discover_mdns_service('my-service', '_ipps._tcp', 'my-host')
def host_check_mdns_service(self, host, host_addr, service_instance):
service = host.discover_mdns_service(service_instance, '_ipps._tcp', 'my-host')
self.assertIsNotNone(service)
self.assertEqual(service['instance'], 'my-service')
self.assertEqual(service['instance'], service_instance)
self.assertEqual(service['name'], '_ipps._tcp')
self.assertEqual(service['port'], 12345)
self.assertEqual(service['priority'], 0)
@@ -181,17 +194,18 @@ class SingleHostAndService(thread_cert.TestCase):
self.assertEqual(ipaddress.ip_address(service['addresses'][0]), ipaddress.ip_address(host_addr))
self.assertEqual(len(service['addresses']), 1)
def check_host_and_service(self, server, client, host_addr):
def check_host_and_service(self, server, client, host_addr, service_instance):
"""Check that we have properly registered host and service instance.
"""
client_services = client.srp_client_get_services()
print(client_services)
client_services = [service for service in client_services if service['instance'] == service_instance]
self.assertEqual(len(client_services), 1)
client_service = client_services[0]
# Verify that the client possesses correct service resources.
self.assertEqual(client_service['instance'], 'my-service')
self.assertEqual(client_service['instance'], service_instance)
self.assertEqual(client_service['name'], '_ipps._tcp')
self.assertEqual(int(client_service['port']), 12345)
self.assertEqual(int(client_service['priority']), 0)
@@ -202,6 +216,7 @@ class SingleHostAndService(thread_cert.TestCase):
server_services = server.srp_server_get_services()
print(server_services)
server_services = [service for service in server_services if service['instance'] == service_instance]
self.assertEqual(len(server_services), 1)
server_service = server_services[0]