Skip to content

Commit

Permalink
Expose interfaceIndex from the DNSServiceResolve API, in order to all…
Browse files Browse the repository at this point in the history 
…ow all interfaces to be correctly queried, and in the DNS-SD API, merge multiple browse/resolve results if they have the same PTR/SRV/TXT records, in order to report all IP addresses (thanks to @rhastie, Rich Hastie, Mellanox, for help debugging this issue)

(cherry picked from commit 9d5b6d47d29a8db3aa71a03f249ccfc1fe2d35ad)
  • Loading branch information
@garethsb
garethsb committed Dec 16, 2019
1 parent 7bb1d03 commit 6a26bdb
Show file tree
Hide file tree
Showing 3 changed files with 82 additions and 40 deletions.
5 changes: 3 additions & 2 deletions Development/mdns/service_discovery.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ namespace mdns
struct browse_result
{
browse_result() : interface_id(0) {}
browse_result(const std::string& name, const std::string& type, const std::string& domain, std::uint32_t interface_id) : name(name), type(type), domain(domain), interface_id(interface_id) {}
browse_result(const std::string& name, const std::string& type, const std::string& domain, std::uint32_t interface_id = 0) : name(name), type(type), domain(domain), interface_id(interface_id) {}

std::string name;
std::string type;
Expand All @@ -40,11 +40,12 @@ namespace mdns
struct resolve_result
{
resolve_result() {}
resolve_result(const std::string& host_name, std::uint16_t port, const mdns::txt_records& txt_records) : host_name(host_name), port(port), txt_records(txt_records) {}
resolve_result(const std::string& host_name, std::uint16_t port, const mdns::txt_records& txt_records, std::uint32_t interface_id = 0) : host_name(host_name), port(port), txt_records(txt_records), interface_id(interface_id) {}

std::string host_name;
std::uint16_t port;
mdns::txt_records txt_records;
std::uint32_t interface_id;

std::vector<std::string> ip_addresses;
};
Expand Down
24 changes: 16 additions & 8 deletions Development/mdns/service_discovery_impl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,12 @@ namespace mdns_details
: more_coming ? (std::max)(earliest_timeout, latest_timeout) : latest_timeout;
}

// map kDNSServiceInterfaceIndexLocalOnly to kDNSServiceInterfaceIndexAny, to handle AVAHI_IF_UNSPEC escaping from the Avahi compatibility layer
static inline std::uint32_t make_interface_id(std::uint32_t interfaceIndex)
{
return kDNSServiceInterfaceIndexLocalOnly == interfaceIndex ? kDNSServiceInterfaceIndexAny : interfaceIndex;
}

struct browse_context
{
// browse in-flight state
Expand All @@ -44,8 +50,7 @@ namespace mdns_details
{
if (0 != (flags & kDNSServiceFlagsAdd))
{
// map kDNSServiceInterfaceIndexLocalOnly to kDNSServiceInterfaceIndexAny, to handle AVAHI_IF_UNSPEC escaping from the Avahi compatibility layer
const browse_result result{ serviceName, regtype, replyDomain, kDNSServiceInterfaceIndexLocalOnly == interfaceIndex ? kDNSServiceInterfaceIndexAny : interfaceIndex };
const browse_result result{ serviceName, regtype, replyDomain, make_interface_id(interfaceIndex) };

slog::log<slog::severities::more_info>(impl->gate, SLOG_FLF) << "After DNSServiceBrowse, DNSServiceBrowseReply got service: " << result.name << " for regtype: " << result.type << " domain: " << result.domain << " on interface: " << result.interface_id;

Expand Down Expand Up @@ -179,7 +184,7 @@ namespace mdns_details
if (errorCode == kDNSServiceErr_NoError)
{
{
const resolve_result result{ hosttarget, ntohs(port), parse_txt_records(txtRecord, txtLen) };
const resolve_result result{ hosttarget, ntohs(port), parse_txt_records(txtRecord, txtLen), make_interface_id(interfaceIndex) };

slog::log<slog::severities::more_info>(impl->gate, SLOG_FLF) << "After DNSServiceResolve, DNSServiceResolveReply got host: " << result.host_name << " port: " << (int)result.port;

Expand All @@ -197,11 +202,12 @@ namespace mdns_details
struct address_result
{
address_result() {}
address_result(const std::string& host_name, const std::string& ip_address, std::uint32_t ttl = 0) : host_name(host_name), ip_address(ip_address), ttl(ttl) {}
address_result(const std::string& host_name, const std::string& ip_address, std::uint32_t ttl = 0, std::uint32_t interface_id = 0) : host_name(host_name), ip_address(ip_address), ttl(ttl), interface_id(interface_id) {}

std::string host_name;
std::string ip_address;
std::uint32_t ttl;
std::uint32_t interface_id;
};

// return true from the address result callback if the operation should be ended before its specified timeout once no more results are "imminent"
Expand Down Expand Up @@ -263,7 +269,7 @@ namespace mdns_details
const auto ip_address = from_sockaddr(*address);
if (!ip_address.is_unspecified())
{
const address_result result{ hostname, ip_address.to_string(), ttl };
const address_result result{ hostname, ip_address.to_string(), ttl, make_interface_id(interfaceIndex) };
slog::log<slog::severities::more_info>(impl->gate, SLOG_FLF) << "After DNSServiceGetAddrInfo, DNSServiceGetAddrInfoReply got address: " << result.ip_address << " for host: " << result.host_name;

impl->had_enough = impl->handler(result);
Expand All @@ -281,7 +287,8 @@ namespace mdns_details

static bool resolve(const resolve_handler& handler, const std::string& name, const std::string& type, const std::string& domain, std::uint32_t interface_id, const std::chrono::steady_clock::duration& latest_timeout_, DNSServiceCancellationToken cancel, slog::base_gate& gate)
{
const auto earliest_timeout_ = std::chrono::seconds(0);
// apply a minimum timeout when the interface id isn't known e.g. from the result of a browse
const auto earliest_timeout_ = std::chrono::seconds(0 == interface_id ? 1 : 0);

bool had_enough = false;
bool more_coming = true;
Expand Down Expand Up @@ -339,7 +346,8 @@ namespace mdns_details

static bool getaddrinfo(const address_handler& handler, const std::string& host_name, std::uint32_t interface_id, const std::chrono::steady_clock::duration& latest_timeout_, DNSServiceCancellationToken cancel, slog::base_gate& gate)
{
const auto earliest_timeout_ = std::chrono::seconds(0);
// apply a minimum timeout when the interface id isn't known e.g. from the result of a browse
const auto earliest_timeout_ = std::chrono::seconds(0 == interface_id ? 1 : 0);

bool had_enough = false;
#ifdef HAVE_DNSSERVICEGETADDRINFO
Expand Down Expand Up @@ -501,7 +509,7 @@ namespace mdns
{
resolved.ip_addresses.push_back(address.ip_address);
return true;
}, resolved.host_name, interface_id, timeout, guard.target, *gate_);
}, resolved.host_name, resolved.interface_id, timeout, guard.target, *gate_);
return handler(resolved);
}, name, type, domain, interface_id, timeout, guard.target, *gate_);
// when this task is cancelled, make sure it doesn't just return an empty/partial result
Expand Down
93 changes: 63 additions & 30 deletions Development/nmos/mdns_api.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,40 +120,56 @@ namespace nmos
{
if (!browsed.empty())
{
std::shared_ptr<std::map<std::string, value>> results(new std::map<std::string, value>());

// add a continuation for each discovered service to resolve it
// but, to simplify inserting these into the priority map, the continuations are sequential
// rather than parallelised using pplx::when_all
auto resolve_all = pplx::task_from_result();
for (const auto& resolving : browsed)
typedef std::pair<std::string, std::vector<::mdns::resolve_result>> mdns_result;
std::vector<pplx::task<mdns_result>> tasks;
for (const auto& service : browsed)
{
resolve_all = resolve_all.then([discovery, results, resolving, gate]
const auto& serviceName = service.name;
tasks.push_back(discovery->resolve(service.name, service.type, service.domain, service.interface_id)
.then([serviceName, gate](std::vector<::mdns::resolve_result> resolved)
{
const auto& serviceName = resolving.name;
return mdns_result(serviceName, resolved);
}));
}
return pplx::when_all(tasks.begin(), tasks.end()).then([res, tasks](pplx::task<std::vector<mdns_result>> finally) mutable
{
// to ensure an exception from one doesn't leave other tasks' exceptions unobserved
for (auto& task : tasks)
{
try { task.wait(); } catch (...) {}
}

// check if we already have resolved this service
if (results->end() != results->find(serviceName)) return pplx::task_from_result();
// merge results that have the same host_target, port and txt records
// and only differ in the resolved addresses

// if not, resolve and add a result for it
return discovery->resolve(resolving.name, resolving.type, resolving.domain, resolving.interface_id)
.then([results, serviceName, gate](std::vector<::mdns::resolve_result> resolved)
std::map<value, std::set<std::string>> results;
for (auto& resolved : finally.get())
{
const auto& serviceName = resolved.first;

for (auto& service : resolved.second)
{
if (!resolved.empty())
{
(*results)[serviceName] = make_mdns_result(serviceName, resolved.front());
}
});
});
}
return resolve_all.then([res, results]() mutable
{
if (!results->empty())
auto ip_addresses = service.ip_addresses;
service.ip_addresses.clear();
auto instance = make_mdns_result(serviceName, service);
results[instance].insert(ip_addresses.begin(), ip_addresses.end());
}
}

if (!results.empty())
{
set_reply(res, status_codes::OK,
web::json::serialize(*results, [](const std::map<std::string, value>::value_type& kv) { return kv.second; }),
web::json::serialize(results, [](const std::map<value, std::set<std::string>>::value_type& result)
{
auto instance = result.first;
for (const auto& address : result.second)
{
web::json::push_back(instance[U("addresses")], web::json::value::string(utility::s2us(address)));
}
return instance;
}),
web::http::details::mime_types::application_json);
res.headers().add(U("X-Total-Count"), results->size());
res.headers().add(U("X-Total-Count"), results.size());
}
else
{
Expand Down Expand Up @@ -196,12 +212,29 @@ namespace nmos
return discovery->resolve(serviceName, serviceType, service_domain.empty() ? "local." : service_domain)
.then([res, serviceName, gate](std::vector<::mdns::resolve_result> resolved) mutable
{
if (!resolved.empty())
// merge results that have the same host_target, port and txt records
// and only differ in the resolved addresses

std::map<value, std::set<std::string>> results;
for (auto& service : resolved)
{
// for now, pick one result, even though there may be one per interface
value result = make_mdns_result(serviceName, resolved.front());
auto ip_addresses = service.ip_addresses;
service.ip_addresses.clear();
auto instance = make_mdns_result(serviceName, service);
results[instance].insert(ip_addresses.begin(), ip_addresses.end());
}

set_reply(res, status_codes::OK, result);
if (!results.empty())
{
// for now, pick one of the merged results, even though there conceivably may be one per interface
const auto& result = *results.begin();

auto instance = result.first;
for (const auto& address : result.second)
{
web::json::push_back(instance[U("addresses")], web::json::value::string(utility::s2us(address)));
}
set_reply(res, status_codes::OK, instance);
}
else
{
Expand Down

0 comments on commit 6a26bdb

Please sign in to comment.