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 #33119

Merged
merged 3 commits into from
Apr 23, 2024
Merged
Changes from 1 commit
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
Next Next commit
MinMDNS - do not ask for IP addresses for every SRV record that is se…
…en (#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 <andreilitvin@google.com>
andy31415 and andreilitvin committed Apr 23, 2024
commit f307ffbd05da084d03ec608c1a863a0e13e63b35
28 changes: 26 additions & 2 deletions src/lib/dnssd/ActiveResolveAttempts.cpp
Original file line number Diff line number Diff line change
@@ -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)
6 changes: 6 additions & 0 deletions src/lib/dnssd/ActiveResolveAttempts.h
Original file line number Diff line number Diff line change
@@ -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;

7 changes: 7 additions & 0 deletions src/lib/dnssd/IncrementalResolve.h
Original file line number Diff line number Diff line change
@@ -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.
///
20 changes: 17 additions & 3 deletions src/lib/dnssd/Resolver_ImplMinimalMdns.cpp
Original file line number Diff line number Diff line change
@@ -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->IsActiveBrowseParse())
{
// 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;
}