MinMDNS - do not ask for IP addresses for every SRV record that is se…

…en (#33118) * MinMDNS - do not ask for IP addresses for every SRV record that is seen (#33095) * Initial version of a test app - ability to just send a packet into the world * Update constants to match a real advertisement * Only resolve IP addresses if we are interested in this path * Fix up logic: this is per peer id * Remove unused include * Undo debug code * Undo extra header add * Add header back, but with full path * Fix up some more headers * Remove unhelpful comment * Move tester program out (will be part of another PR) --------- Co-authored-by: Andrei Litvin <[email protected]> * Fix for 1.2 compatibility * Undo submodule update * Bump prompt-toolkit requirement to fix ptpython version update (#30987) * Remove include that is not valid for 1.2 --------- Co-authored-by: Andrei Litvin <[email protected]>
project-chip · Apr 23, 2024 · b35ef93 · b35ef93
1 parent 9897d2e
commit b35ef93
Show file tree

Hide file tree

Showing 5 changed files with 64 additions and 7 deletions.
diff --git a/scripts/setup/constraints.txt b/scripts/setup/constraints.txt
@@ -98,7 +98,7 @@ ghapi==1.0.3
     # via -r requirements.memory.txt
 humanfriendly==10.0
     # via coloredlogs
-idf-component-manager==1.2.2
+idf-component-manager==1.5.2
     # via -r requirements.esp32.txt
 idna==3.4
     # via requests
@@ -163,7 +163,7 @@ portpicker==1.5.2
     # via
     #   -r requirements.all.txt
     #   mobly
-prompt-toolkit==3.0.38
+prompt-toolkit==3.0.43
     # via ipython
 protobuf==4.24.4
     # via
@@ -290,3 +290,10 @@ setuptools==68.0.0
     # via
     #   pip-tools
     #   west
+
+# Manual edits:
+
+# Higher versions depend on proto-plus, which break
+# nanopb code generation (due to name conflict of the 'proto' module)
+google-api-core==2.17.0
+
diff --git a/src/lib/dnssd/ActiveResolveAttempts.cpp b/src/lib/dnssd/ActiveResolveAttempts.cpp
@@ -19,8 +19,6 @@
 
 #include <lib/support/logging/CHIPLogging.h>
 
-#include <algorithm>
-
 using namespace chip;
 
 namespace mdns {
@@ -284,6 +282,32 @@ Optional<ActiveResolveAttempts::ScheduledAttempt> ActiveResolveAttempts::NextSch
     return Optional<ScheduledAttempt>::Missing();
 }
 
+bool ActiveResolveAttempts::ShouldResolveIpAddress(PeerId peerId) const
+{
+    for (auto & item : mRetryQueue)
+    {
+        if (item.attempt.IsEmpty())
+        {
+            continue;
+        }
+        if (item.attempt.IsBrowse())
+        {
+            return true;
+        }
+
+        if (item.attempt.IsResolve())
+        {
+            auto & data = item.attempt.ResolveData();
+            if (data.peerId == peerId)
+            {
+                return true;
+            }
+        }
+    }
+
+    return false;
+}
+
 bool ActiveResolveAttempts::IsWaitingForIpResolutionFor(SerializedQNameIterator hostName) const
 {
     for (auto & entry : mRetryQueue)

diff --git a/src/lib/dnssd/ActiveResolveAttempts.h b/src/lib/dnssd/ActiveResolveAttempts.h
@@ -288,6 +288,12 @@ class ActiveResolveAttempts
     /// IP resolution.
     bool IsWaitingForIpResolutionFor(SerializedQNameIterator hostName) const;
 
+    /// Determines if address resolution for the given peer ID is required
+    ///
+    /// IP Addresses are required for active operational discovery of specific peers
+    /// or if an active browse is being performed.
+    bool ShouldResolveIpAddress(chip::PeerId peerId) const;
+
     /// Check if a browse operation is active for the given discovery type
     bool HasBrowseFor(chip::Dnssd::DiscoveryType type) const;
 

diff --git a/src/lib/dnssd/IncrementalResolve.h b/src/lib/dnssd/IncrementalResolve.h
@@ -104,6 +104,12 @@ class IncrementalResolver
 
     ServiceNameType GetCurrentType() const { return mServiceNameType; }
 
+    PeerId OperationalParsePeerId() const
+    {
+        VerifyOrReturnValue(IsActiveOperationalParse(), PeerId());
+        return mSpecificResolutionData.Get<OperationalNodeData>().peerId;
+    }
+
     /// Start parsing a new record. SRV records are the records we are mainly
     /// interested on, after which TXT and A/AAAA are looked for.
     ///

diff --git a/src/lib/dnssd/Resolver_ImplMinimalMdns.cpp b/src/lib/dnssd/Resolver_ImplMinimalMdns.cpp
@@ -17,8 +17,6 @@
 
 #include "Resolver.h"
 
-#include <limits>
-
 #include <lib/core/CHIPConfig.h>
 #include <lib/dnssd/ActiveResolveAttempts.h>
 #include <lib/dnssd/IncrementalResolve.h>
@@ -358,7 +356,23 @@ void MinMdnsResolver::AdvancePendingResolverStates()
 
         if (missing.Has(IncrementalResolver::RequiredInformationBitFlags::kIpAddress))
         {
-            ScheduleIpAddressResolve(resolver->GetTargetHostName());
+            if (resolver->IsActiveCommissionParse())
+            {
+                // Browse wants IP addresses
+                ScheduleIpAddressResolve(resolver->GetTargetHostName());
+            }
+            else if (mActiveResolves.ShouldResolveIpAddress(resolver->OperationalParsePeerId()))
+            {
+                // Keep searching for IP addresses if an active resolve needs these IP addresses
+                // otherwise ignore the data (received a SRV record without IP address, however we do not
+                // seem interested in it. Probably just a device that came online).
+                ScheduleIpAddressResolve(resolver->GetTargetHostName());
+            }
+            else
+            {
+                // This IP address is not interesting enough to run another discovery
+                resolver->ResetToInactive();
+            }
             continue;
         }