MinMdnsResolver and DiscoveryImplPlatform resolve by instance name differently, could lead to behavior differences

[chrisdecenzo]

This is a followup to #17214

Problem

• When using the resolver to search by instance name, DiscoveryImplPlatform short-circuits to perform a simple resolve while the MinMdnsResolver does a browse. The DiscoveryImplPlatform behavior was added in 17214 because the full browse by instance name was not working on Mac. MinMdnsResolver should probably be updated to do a direct resolve too.
• When adding the fix 17214, the same short-circuit logic was added to commissioner discovery by instance name. I'm not aware of any code that uses this combination of features but Boris noted that the MinMdnsResolver looks to not handle commissioner discovery by instance name correctly.

Exact comments on 17214 from @bzbarsky-apple:

1. It looks to me like MinMdnsResolver::SendPendingBrowseQueries handles kInstanceName only when DiscoveryType::kCommissionableNode, and in particular would not handle it for the equivalent of FindCommissioners here. Is that a bug that needs a followup?
2. It looks to me like minimal mdns handles this by browsing, but just using a different qname that does not treat the instance name as a subtype. It would be best if we handled this similarly in the two dns-sd impls as much as possible... Would it make sense to keep doing the browse here but fix MakeServiceTypeName to handle the kInstanceName case correctly (just like it already handles the kNone case)?

Proposed Solution

• Bring MinMdnsResolver and DiscoveryImplPlatform into closer alignment

[bzbarsky-apple]

@vivien-apple @andreilitvin What's the best way to converge here?

[andy31415]

Could you give a description of the issue with an example? I.e. what browse/discovery is being done (what qname and type if relevant) and what the differences are?

The issue contains a lot of description and yet I still do not understand what we are looking for and what the differences are. From a MinMDNS perspective there is no "browse" - DNSSD will issue a mdns query packet and it will receive mdns response packets. There is no difference between "resolve" and "browse" (or maybe this is reffering to packet type?)

[andy31415]

I am also unclear what "Short circuit" means - minmdns always sens out a packet and parses received packet - no path is shorter or longer. Not sure if that is the problem or the requested change.

[bzbarsky-apple]

There is no difference between "resolve" and "browse" (or maybe this is reffering to packet type?)

In minmdns there's a BrowseNodes function. This calls SendPendingBrowseQueries.

What sort of data do those get back?

As far as I can tell for platform mdns the process is that you start with something like "_L3840._sub._matterc._udp", you use this to get an instance name (or list of them), then for each instance name maybe you have an IP already as part of the data that came back, and maybe not and then you have to look up the IP(s) for the instance name (which involves looking up the hostname and so on).

What does that process look like with minmdns?

[andy31415]

Not familiar with BrowseNodes however from a dnssd perspective, the browse is sending out the QNAME as a suffix and everyone that matches the suffix will reply.

MinMDNS as its core only has the logic of 'send query, report back responses' and I believe our code often relies that all responses contain IP addresses as extra data. As far as I have seen before, this used to always be the case however it is not guaranteed by the spec.

It would make sense for a request of 'minmdns resolver should follow up with resolving IP addresses if they are not part of the original reply'. I would expect this to not be as simple as 'resolve all IPs' since when we browse we likely only need the IP of one of the replies not all of them.

Is this what the bug is about?

[bzbarsky-apple]

The bug so far is about the apparent difference in overall structure between "platform" mdns, which has a two-step bit where it mas the provided PTR QNAME to an instance name and then maps the instance name to actual IP/port data, and hence has to figure out whether the thing it's being asked to look up is likely to be a PTR key or a SRV/TXT key and "minimal" mdns which ... well, I am still not quite clear what it's doing in this case. If it does a query for a string that matches PTR entries, does it get back the SRV/TXT records in the responses?

Anyway, there is one definite behavior difference: minmdns does not really support the kInstanceName filter type for commissioner discovery and platform mdns now does. Either we need this support and we should support it both places, or we don't and we should remove it from platform mdns.

There rest was just a question about whether there is a potential behavior difference....

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.