From e3f97e2362cf0d4f7fbea6e19014bc03751a7c2f Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Fri, 5 Apr 2024 09:30:26 -0700 Subject: [PATCH] [netdata-publisher] distinguish SRP/DNS unicast entries (#9937) This commit refines the `NetworkData::Publisher` to differentiate between DNS/SRP unicast entries based on whether the address information resides in service data (part of service TLV) or server data (part of server sub-TLV). Additionally, if another BR adds a service data unicast entry, `Publisher` will zero out the desired count for server data unicast entries, effectively removing any previously published server data unicast entries. The `test_netdata_publisher` has been updated to validate all the newly added behaviors. --- src/core/thread/network_data_publisher.cpp | 108 ++++++++++-------- src/core/thread/network_data_publisher.hpp | 5 +- .../thread-cert/test_netdata_publisher.py | 92 +++++++++++---- 3 files changed, 135 insertions(+), 70 deletions(-) diff --git a/src/core/thread/network_data_publisher.cpp b/src/core/thread/network_data_publisher.cpp index ec925b824..a64f0c39e 100644 --- a/src/core/thread/network_data_publisher.cpp +++ b/src/core/thread/network_data_publisher.cpp @@ -661,16 +661,17 @@ void Publisher::DnsSrpServiceEntry::Process(void) case kTypeUnicastMeshLocalEid: { Service::DnsSrpAnycast::Info anycastInfo; + bool hasServiceDataEntry; - CountUnicastEntries(numEntries, numPreferredEntries); + CountServerDataUnicastEntries(numEntries, numPreferredEntries, hasServiceDataEntry); desiredNumEntries = kDesiredNumUnicast; - if (Get().FindPreferredDnsSrpAnycastInfo(anycastInfo) == kErrorNone) + if (hasServiceDataEntry || (Get().FindPreferredDnsSrpAnycastInfo(anycastInfo) == kErrorNone)) { - // If there is any anycast entry in netdata, we set the - // desired number of unicast entries (with address added - // in server TLV) to zero to remove any added unicast - // entry. + // If there is any service data unicast entry or anycast + // entry, we set the desired number of server data + // unicast entries to zero to remove any such previously + // added unicast entry. desiredNumEntries = 0; } @@ -680,7 +681,7 @@ void Publisher::DnsSrpServiceEntry::Process(void) case kTypeUnicast: desiredNumEntries = kDesiredNumUnicast; - CountUnicastEntries(numEntries, numPreferredEntries); + CountServiceDataUnicastEntries(numEntries, numPreferredEntries); break; } @@ -721,9 +722,53 @@ void Publisher::DnsSrpServiceEntry::CountAnycastEntries(uint8_t &aNumEntries, ui } } -void Publisher::DnsSrpServiceEntry::CountUnicastEntries(uint8_t &aNumEntries, uint8_t &aNumPreferredEntries) const +void Publisher::DnsSrpServiceEntry::CountServerDataUnicastEntries(uint8_t &aNumEntries, + uint8_t &aNumPreferredEntries, + bool &aHasServiceDataEntry) const { - // Count the number of "DNS/SRP Unicast" service entries in + // Count the number of server data DNS/SRP unicast entries in the + // Network Data. Also determine whether there is any service data + // DNS/SRP unicast entry (update `aHasServiceDataEntry`). + + const ServiceTlv *serviceTlv = nullptr; + ServiceData data; + + aHasServiceDataEntry = false; + + data.InitFrom(Service::DnsSrpUnicast::kServiceData); + + while ((serviceTlv = Get().FindNextThreadService(serviceTlv, data, NetworkData::kServicePrefixMatch)) != + nullptr) + { + TlvIterator subTlvIterator(*serviceTlv); + const ServerTlv *serverSubTlv; + + if (serviceTlv->GetServiceDataLength() >= sizeof(Service::DnsSrpUnicast::ServiceData)) + { + aHasServiceDataEntry = true; + } + + while (((serverSubTlv = subTlvIterator.Iterate())) != nullptr) + { + if (serverSubTlv->GetServerDataLength() < sizeof(Service::DnsSrpUnicast::ServerData)) + { + continue; + } + + aNumEntries++; + + if (IsPreferred(serverSubTlv->GetServer16())) + { + aNumPreferredEntries++; + } + } + } +} + +void Publisher::DnsSrpServiceEntry::CountServiceDataUnicastEntries(uint8_t &aNumEntries, + uint8_t &aNumPreferredEntries) const +{ + // Count the number of service data DNS/SRP unicast entries in // the Network Data. const ServiceTlv *serviceTlv = nullptr; @@ -737,45 +782,18 @@ void Publisher::DnsSrpServiceEntry::CountUnicastEntries(uint8_t &aNumEntries, ui TlvIterator subTlvIterator(*serviceTlv); const ServerTlv *serverSubTlv; + if (serviceTlv->GetServiceDataLength() < sizeof(Service::DnsSrpUnicast::ServiceData)) + { + continue; + } + while (((serverSubTlv = subTlvIterator.Iterate())) != nullptr) { - if (serviceTlv->GetServiceDataLength() >= sizeof(Service::DnsSrpUnicast::ServiceData)) + aNumEntries++; + + if (IsPreferred(serverSubTlv->GetServer16())) { - aNumEntries++; - - // Generally, we prefer entries where the SRP/DNS server - // address/port info is included in the service TLV data - // over the ones where the info is included in the - // server TLV data (i.e., we prefer infra-provided - // SRP/DNS entry over a BR local one using ML-EID). If - // our entry itself uses the service TLV data, then we - // prefer based on the associated RLOC16. - - if (GetType() == kTypeUnicast) - { - if (IsPreferred(serverSubTlv->GetServer16())) - { - aNumPreferredEntries++; - } - } - else - { - aNumPreferredEntries++; - } - } - - if (serverSubTlv->GetServerDataLength() >= sizeof(Service::DnsSrpUnicast::ServerData)) - { - aNumEntries++; - - // If our entry also uses the server TLV data (with - // ML-EID address), then the we prefer based on the - // associated RLOC16. - - if ((GetType() == kTypeUnicastMeshLocalEid) && IsPreferred(serverSubTlv->GetServer16())) - { - aNumPreferredEntries++; - } + aNumPreferredEntries++; } } } diff --git a/src/core/thread/network_data_publisher.hpp b/src/core/thread/network_data_publisher.hpp index 3ac2c825a..4503efb1e 100644 --- a/src/core/thread/network_data_publisher.hpp +++ b/src/core/thread/network_data_publisher.hpp @@ -437,7 +437,10 @@ private: void Notify(Event aEvent) const; void Process(void); void CountAnycastEntries(uint8_t &aNumEntries, uint8_t &aNumPreferredEntries) const; - void CountUnicastEntries(uint8_t &aNumEntries, uint8_t &aNumPreferredEntries) const; + void CountServiceDataUnicastEntries(uint8_t &aNumEntries, uint8_t &aNumPreferredEntries) const; + void CountServerDataUnicastEntries(uint8_t &aNumEntries, + uint8_t &aNumPreferredEntries, + bool &aHasServiceDataEntry) const; Info mInfo; Callback mCallback; diff --git a/tests/scripts/thread-cert/test_netdata_publisher.py b/tests/scripts/thread-cert/test_netdata_publisher.py index 7648edaaa..c31ac6e52 100755 --- a/tests/scripts/thread-cert/test_netdata_publisher.py +++ b/tests/scripts/thread-cert/test_netdata_publisher.py @@ -70,7 +70,7 @@ DNSSRP_PORT = 49152 # The desired number of entries (based on related config). DESIRED_NUM_DNSSRP_ANYCAST = 8 -DESIRED_NUM_DNSSRP_UNCIAST = 2 +DESIRED_NUM_DNSSRP_UNICAST = 2 DESIRED_NUM_ON_MESH_PREFIX = 3 DESIRED_NUM_EXTERNAL_ROUTE = 10 @@ -263,52 +263,39 @@ class NetDataPublisher(thread_cert.TestCase): self.verify_anycast_services(services) #--------------------------------------------------------------------------------- - # DNS/SRP unicast entries + # DNS/SRP service data unicast entries - # Publish DNS/SRP unicast address on all routers, first using - # MLE-EID address, then change to use specific address. Verify - # that number of entries in network data is correct in each step - # and that entries are switched correctly. num = 0 - for node in routers: - node.netdata_publish_dnssrp_unicast_mleid(DNSSRP_PORT) - self.simulator.go(WAIT_TIME) - num += 1 - services = leader.get_services() - self.assertEqual(len(services), min(num, DESIRED_NUM_DNSSRP_UNCIAST)) - self.verify_unicast_services(services) - for node in routers: node.netdata_publish_dnssrp_unicast(DNSSRP_ADDRESS, DNSSRP_PORT) self.simulator.go(WAIT_TIME) + num += 1 services = leader.get_services() - self.assertEqual(len(services), min(num, DESIRED_NUM_DNSSRP_UNCIAST)) + self.assertEqual(len(services), min(num, DESIRED_NUM_DNSSRP_UNICAST)) self.verify_unicast_services(services) for node in routers: node.srp_server_set_enabled(True) self.simulator.go(WAIT_TIME) + self.assertEqual(sum(node.srp_server_get_state() == 'running' for node in routers), - min(len(routers), DESIRED_NUM_DNSSRP_UNCIAST)) + min(len(routers), DESIRED_NUM_DNSSRP_UNICAST)) self.assertEqual(sum(node.srp_server_get_state() == 'stopped' for node in routers), - max(len(routers) - DESIRED_NUM_DNSSRP_UNCIAST, 0)) + max(len(routers) - DESIRED_NUM_DNSSRP_UNICAST, 0)) for node in routers: node.netdata_unpublish_dnssrp() self.simulator.go(WAIT_TIME) num -= 1 services = leader.get_services() - self.assertEqual(len(services), min(num, DESIRED_NUM_DNSSRP_UNCIAST)) + self.assertEqual(len(services), min(num, DESIRED_NUM_DNSSRP_UNICAST)) self.verify_unicast_services(services) for node in routers: node.srp_server_set_enabled(False) self.assertEqual(node.srp_server_get_state(), 'disabled') #--------------------------------------------------------------------------------- - # DNS/SRP unicast and anycast entry - - # Verify that publishing an anycast entry will update the limit - # for the unicast MLE-EID address entry and all are removed. + # DNS/SRP server data unicast entries num = 0 for node in routers: @@ -316,20 +303,77 @@ class NetDataPublisher(thread_cert.TestCase): self.simulator.go(WAIT_TIME) num += 1 services = leader.get_services() - self.assertEqual(len(services), min(num, DESIRED_NUM_DNSSRP_UNCIAST)) + self.assertEqual(len(services), min(num, DESIRED_NUM_DNSSRP_UNICAST)) self.verify_unicast_services(services) + for node in routers: + node.srp_server_set_enabled(True) + self.simulator.go(WAIT_TIME) + self.assertEqual(sum(node.srp_server_get_state() == 'running' for node in routers), + min(len(routers), DESIRED_NUM_DNSSRP_UNICAST)) + self.assertEqual(sum(node.srp_server_get_state() == 'stopped' for node in routers), + max(len(routers) - DESIRED_NUM_DNSSRP_UNICAST, 0)) + + for node in routers: + node.netdata_unpublish_dnssrp() + self.simulator.go(WAIT_TIME) + num -= 1 + services = leader.get_services() + self.assertEqual(len(services), min(num, DESIRED_NUM_DNSSRP_UNICAST)) + self.verify_unicast_services(services) + for node in routers: + node.srp_server_set_enabled(False) + self.assertEqual(node.srp_server_get_state(), 'disabled') + + #--------------------------------------------------------------------------------- + # DNS/SRP server data unicast vs anycast + + num = 0 + for node in routers: + node.netdata_publish_dnssrp_unicast_mleid(DNSSRP_PORT) + self.simulator.go(WAIT_TIME) + num += 1 + services = leader.get_services() + self.assertEqual(len(services), min(num, DESIRED_NUM_DNSSRP_UNICAST)) + self.verify_unicast_services(services) + + # Verify that publishing an anycast entry will update the + # limit for the server data unicast address entry and all are + # removed. + leader.netdata_publish_dnssrp_anycast(ANYCAST_SEQ_NUM) self.simulator.go(WAIT_TIME) services = leader.get_services() self.assertEqual(len(services), 1) self.verify_anycast_services(services) + # Removing the anycast entry will cause the lower priority + # server data unicast entries to be added again. + leader.netdata_unpublish_dnssrp() self.simulator.go(WAIT_TIME) services = leader.get_services() - self.assertEqual(len(services), min(num, DESIRED_NUM_DNSSRP_UNCIAST)) + self.assertEqual(len(services), min(num, DESIRED_NUM_DNSSRP_UNICAST)) + self.verify_unicast_services(services) + + #--------------------------------------------------------------------------------- + # DNS/SRP server data unicast vs service data unicast + + leader.netdata_publish_dnssrp_unicast(DNSSRP_ADDRESS, DNSSRP_PORT) + self.simulator.go(WAIT_TIME) + services = leader.get_services() + self.assertEqual(len(services), 1) + self.verify_unicast_services(services) + + # Removing the service data unicast entry will cause the lower + # priority server data unicast entries to be added again. + + leader.netdata_unpublish_dnssrp() + self.simulator.go(WAIT_TIME) + + services = leader.get_services() + self.assertEqual(len(services), min(num, DESIRED_NUM_DNSSRP_UNICAST)) self.verify_unicast_services(services) for node in routers: