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

…en (#33119) * 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.3 compatibility * Undo submodule update --------- Co-authored-by: Andrei Litvin <[email protected]>
project-chip · Apr 23, 2024 · 530dec2 · 530dec2
1 parent 4431f18
commit 530dec2
Show file tree

Hide file tree

Showing 4 changed files with 56 additions and 5 deletions.
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
@@ -17,6 +17,7 @@
 #pragma once
 
 #include <lib/dnssd/Resolver.h>
+#include <lib/dnssd/Types.h>
 #include <lib/dnssd/minimal_mdns/Parser.h>
 #include <lib/dnssd/minimal_mdns/RecordData.h>
 #include <lib/dnssd/minimal_mdns/core/QName.h>
@@ -104,6 +105,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>
@@ -373,7 +371,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;
         }