Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MinMDNS - do not ask for IP addresses for every SRV record that is seen #33118

Merged
merged 7 commits into from
Apr 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions scripts/setup/constraints.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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

28 changes: 26 additions & 2 deletions src/lib/dnssd/ActiveResolveAttempts.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@

#include <lib/support/logging/CHIPLogging.h>

#include <algorithm>

using namespace chip;

namespace mdns {
Expand Down Expand Up @@ -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)
Expand Down
6 changes: 6 additions & 0 deletions src/lib/dnssd/ActiveResolveAttempts.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
6 changes: 6 additions & 0 deletions src/lib/dnssd/IncrementalResolve.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
///
Expand Down
20 changes: 17 additions & 3 deletions src/lib/dnssd/Resolver_ImplMinimalMdns.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@

#include "Resolver.h"

#include <limits>
andy31415 marked this conversation as resolved.
Show resolved Hide resolved

#include <lib/core/CHIPConfig.h>
#include <lib/dnssd/ActiveResolveAttempts.h>
#include <lib/dnssd/IncrementalResolve.h>
Expand Down Expand Up @@ -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;
}

Expand Down
Loading