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

Darwin browse+resolve does not properly collect multiple IP addresses #13274

Closed
msandstedt opened this issue Dec 29, 2021 · 1 comment · Fixed by #15239
Closed

Darwin browse+resolve does not properly collect multiple IP addresses #13274

msandstedt opened this issue Dec 29, 2021 · 1 comment · Fixed by #15239

Comments

@msandstedt
Copy link
Contributor

msandstedt commented Dec 29, 2021

Problem

Observed while talking to the Linux all-clusters-app from chip-tool on both Linux and Darwin platforms.

On Linux, chip-tool discovery (browse+resolve) and node ID resolution produce a nodeData record with all IP addresses populated as expected. But on Darwin, discovered and resolved IP addresses are split across multiple nodeData records. For discovery, this means that code like the following can only retain a single IP address, as each new address overwrites the record:

https://github.com/project-chip/connectedhomeip/blob/a04c6d916d26785891751cd804b4b[…]4fc8cc4934f/src/controller/AbstractDnssdDiscoveryController.cpp

And for resolve, we only ever watch for the first received record from the host, so we end up with whichever address happens to arrive first.

Proposed Solution

Fix Darwin mDNS implementation to match the expected behavior and present discovery (browse) and resolve records with all IP address populated.

@msandstedt msandstedt changed the title Minimal mdns sends each address in a separate response Darwin browse+resolve does not properly collect multiple IP addresses Dec 30, 2021
@bzbarsky-apple
Copy link
Contributor

I believe this is a problem, at least for resolve, for everything using src/lib/dnssd/Discovery_ImplPlatform.cpp. That is, OpenThread SRP, Darwin, Linux using Avahi (instead of minimal mdns), etc. Specifically, for resolve the callback that is used is HandleNodeResolve. It gets handed a DnssdService (which contains a single IP address) and immediately creates a DiscoveredNodeData on the stack, populates it with the one address, and calls OnNodeDiscoveryComplete.

Similar for HandleNodeIdResolve.

Browse, on the other hand, should be producing all the IPs for Darwin based on what I see in the implementation, and the corresponding API in Discovery_ImplPlatform allows passing an array of DnssdService, which Darwin does.

That said, I just checked our GenericThreadStackManagerImpl_OpenThread<ImplClass>::DispatchBrowse implementation and that one always dispatches a single DnssdService.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants