Mdns::Publisher signals resolved with the first address

[abtink]

We noticed this while investigating the code and some related issues. The background for this stems from the following PRs:

• [mdns-publisher] move link-local address filtering to DiscoveryProxy #2710
• [trel] require link-local for peer discovered by mDNS #2709

In the above PRs, we aimed to fix the issue where the Publisher is filtering link-local addresses, and move the filtering to DiscoveryProxy. This is done because link-local addresses should now be preferred and used for TREL peers, so we cannot have them filtered by the Publisher.

---

The Mdns::Publisher implementation (with mDNSResponder) when resolving a host or service seem to invoke the callback indicating "resolved" upon getting the first IPv6 address.

• PublisherMDnsSd::ServiceInstanceResolution::HandleGetAddrInfoResult() is the callback which is called, passing the addresses one by one (it has a single const struct sockaddr *aAddress input).
• The implementation keep the addresses in a list:

    if (isAdd)
    {
        mInstanceInfo.AddAddress(address);
    }
    else
    {
        mInstanceInfo.RemoveAddress(address);
    }
    mInstanceInfo.mTtl = aTtl;

• But then, at the end of this method, if the list is not empty (meaning as soon as we find one address), it will do:

    if (!mInstanceInfo.mAddresses.empty() || aErrorCode != kDNSServiceErr_NoError)
    {
        FinishResolution();
    }

• where from FinishResolution, OnServiceResolved() is called.

I think we should aim to improve and fix this behavior.

I think the implemenation with Avahi is also behaving in the same way.

FYI. @superwhd @librasungirl @bukepo @wgtdkp

[abtink]

Reading mDNSResponder source code and API documentation, there is the following flag which can indicate whether more result is coming (callback will be called again immediately).

    kDNSServiceFlagsMoreComing          = 0x1,
    /* MoreComing indicates to a callback that at least one more result is
     * queued and will be delivered following immediately after this one.
     * When the MoreComing flag is set, applications should not immediately
     * update their UI, because this can result in a great deal of ugly flickering
     * on the screen, and can waste a great deal of CPU time repeatedly updating
     * the screen with content that is then immediately erased, over and over.
     * Applications should wait until MoreComing is not set, and then
     * update their UI when no more changes are imminent.
     * When MoreComing is not set, that doesn't mean there will be no more
     * answers EVER, just that there are no more answers immediately
     * available right now at this instant. If more answers become available
     * in the future they will be delivered as usual.
     */

This is also mentioned in the documentation of the callback (to resolve/retrive the addresses)

/**
 ...
 *  @param flags
 *                  Possible values are kDNSServiceFlagsMoreComing and
 *                  kDNSServiceFlagsAdd.
  ...
*/
typedef void (DNSSD_API *DNSServiceGetAddrInfoReply)
(
    DNSServiceRef sdRef,
    DNSServiceFlags flags,
    uint32_t interfaceIndex,
    DNSServiceErrorType errorCode,
    const char *hostname,
    const struct sockaddr *address,
    uint32_t ttl,
    void *context
);

We should use this flag.

[abtink]

Avahi seems to have a different way for this. Instead of a flag, it uses Event (which is passed as input in callbacks).

/** Type of callback event when browsing */
typedef enum {
    AVAHI_BROWSER_NEW,               /**< The object is new on the network */
    AVAHI_BROWSER_REMOVE,            /**< The object has been removed from the network */
    AVAHI_BROWSER_CACHE_EXHAUSTED,   /**< One-time event, to notify the user that all entries from the caches have been sent */
    AVAHI_BROWSER_ALL_FOR_NOW,       /**< One-time event, to notify the user that more records will probably not show up in the near future, i.e. all cache entries have been read and all static servers been queried */
    AVAHI_BROWSER_FAILURE            /**< Browsing failed due to some reason which can be retrieved using avahi_server_errno()/avahi_client_errno() */
} AvahiBrowserEvent;

The AVAHI_BROWSER_ALL_FOR_NOW and AVAHI_BROWSER_CACHE_EXHAUSTED are the related ones.

[abtink]

Copying the comment from the #2712

It seems that, unfortunately, some existing tests actually assumed that the "resolve" outcome would include only one address, and they are force-checking this.

So, ironically, fixing/improving the Publisher implementation to return all addresses is now causing test failures.

As far as I can tell, test_dnssd_server_multi_border_routers and test_dnssd_server are among those tests.

The challenge here is that these tests are added in the main openthread repo, which makes it harder to update them (along with this change).