resolved: fix spurious BrowseServices add/remove flapping with ifindex=0

When a BrowseServices subscription uses ifindex=0 ("all interfaces"),
mdns_browser_revisit_cache() looked up each mDNS scope's cache separately
and called mdns_manage_services_answer() once per scope. That function
derives "removed" events by diffing the browser's *global* service list
(all interfaces, filtered only by owner family) against the single answer
it is handed. So with two or more mDNS-relevant interfaces of the same
family, a service present on interface A is not contained in interface B's
answer and is therefore spuriously removed (emitting "removed") while B is
reconciled, then re-added (emitting "added") on the next revisit tick.

The result is a continuous added/removed/added/removed flap, within a
second, for a service that is still present and without any goodbye packet
involved -- as reported for the mDNS/DNS-SD browsing API.

Fix it by accumulating the pruned cache answers from every matching mDNS
scope into a single combined answer and reconciling exactly once, so the
removed pass diffs the global service list against the union across all
interfaces. Each scope's cache is still pruned before lookup, so genuinely
expired records still drop out of the union and real removals still fire.

The items are merged with dns_answer_add_full() rather than
dns_answer_extend(), so each item's ifindex, flags, rrsig and cache-expiry
"until" are preserved. dns_answer_extend()/dns_answer_merge() are not
expiry-aware and do not carry the source items' "until" through: it
defaults to USEC_INFINITY (the "no expiry" sentinel), which would skew the
RFC 6762 section 5.2 TTL maintenance schedule that
mdns_manage_services_answer() derives from item->until.

The single-interface (ifindex>0) path is unchanged.

diff --git a/src/resolve/resolved-dns-browse-services.c b/src/resolve/resolved-dns-browse-services.c
index 6a48089a594af3c57a3536058f33ce25094e9789..8395d4efafe22ebfdb8c9a6109ffa0914ce0db9b 100644 (file)
--- a/src/resolve/resolved-dns-browse-services.c
+++ b/src/resolve/resolved-dns-browse-services.c
@@ -487,10 +487,21 @@ int mdns_browser_revisit_cache(DnsServiceBrowser *sb, int owner_family) {
         assert(sb);
         assert(sb->manager);
 
-        /* ifindex=0 means "all interfaces" */
+        /* ifindex=0 means "all interfaces". Collect the cached answers from
+         * every matching mDNS scope into a single combined answer and reconcile
+         * once. Reconciling per-scope would be wrong: mdns_manage_services_answer()
+         * derives removals by diffing the browser's global service list against
+         * the answer it is handed, so a single scope's answer would spuriously
+         * "remove" (and then, on the next scope/pass, re-"add") services that are
+         * still present on other interfaces — resulting in a continuous
+         * added/removed event flap for services seen on more than the current
+         * scope. */
         if (sb->ifindex == 0) {
+                _cleanup_(dns_answer_unrefp) DnsAnswer *combined = NULL;
+
                 LIST_FOREACH(scopes, scope, sb->manager->dns_scopes) {
                         _cleanup_(dns_answer_unrefp) DnsAnswer *answer = NULL;
+                        DnsAnswerItem *item;
 
                         if (scope->protocol != DNS_PROTOCOL_MDNS)
                                 continue;
@@ -513,11 +524,24 @@ int mdns_browser_revisit_cache(DnsServiceBrowser *sb, int owner_family) {
                                 return log_error_errno(r, "Failed to look up DNS cache for service browser key on scope %s: %m",
                                                        dns_scope_ifname(scope) ?: "global");
 
-                        r = mdns_manage_services_answer(sb, answer, owner_family);
-                        if (r < 0)
-                                return log_error_errno(r, "Failed to manage mDNS services after cache lookup on scope %s: %m",
-                                                       dns_scope_ifname(scope) ?: "global");
+                        /* Merge preserving each item's ifindex, flags, rrsig and
+                         * cache-expiry 'until'. (dns_answer_extend()/merge() would
+                         * reset 'until' to USEC_INFINITY, which would skew the RFC
+                         * 6762 §5.2 TTL-maintenance schedule that
+                         * mdns_manage_services_answer() derives from item->until.) */
+                        DNS_ANSWER_FOREACH_ITEM(item, answer) {
+                                r = dns_answer_add_extend_full(&combined, item->rr, item->ifindex,
+                                                               item->flags, item->rrsig, item->until);
+                                if (r < 0)
+                                        return log_error_errno(r, "Failed to merge mDNS cache answer from scope %s: %m",
+                                                               dns_scope_ifname(scope) ?: "global");
+                        }
                 }
+
+                r = mdns_manage_services_answer(sb, combined, owner_family);
+                if (r < 0)
+                        return log_error_errno(r, "Failed to manage mDNS services after cache lookup for all interfaces: %m");
+
                 return 0;
         }