From 835297415805cbc93f3e11a4b941392fd5d130d1 Mon Sep 17 00:00:00 2001 From: Christian Glombek Date: Thu, 9 Jul 2026 01:03:27 +0200 Subject: [PATCH 1/2] 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. --- src/resolve/resolved-dns-browse-services.c | 34 ++++++++++++++++++---- 1 file changed, 29 insertions(+), 5 deletions(-) diff --git a/src/resolve/resolved-dns-browse-services.c b/src/resolve/resolved-dns-browse-services.c index 6a48089a59..8395d4efaf 100644 --- 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; } From 4235dea97ae36fd3d4a90fdc913468674ea5aa6b Mon Sep 17 00:00:00 2001 From: Christian Glombek Date: Thu, 9 Jul 2026 03:28:02 +0200 Subject: [PATCH 2/2] test: TEST-89: assert ifindex=0 browse does not flap Add a testcase that browses all interfaces (ifindex=0) and asserts zero 'removed' events arrive while every publisher stays up. Before the combined-answer reconciliation this flapped added/removed continuously for services present on another interface. --- test/units/TEST-89-RESOLVED-MDNS.sh | 72 +++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/test/units/TEST-89-RESOLVED-MDNS.sh b/test/units/TEST-89-RESOLVED-MDNS.sh index 8e8c4e38a9..3ebb95a42c 100755 --- a/test/units/TEST-89-RESOLVED-MDNS.sh +++ b/test/units/TEST-89-RESOLVED-MDNS.sh @@ -237,6 +237,78 @@ testcase_browse_all_interfaces_ifindex_zero() { run_and_check_services_with_ifindex 0 check_both 0 } +testcase_browse_ifindex_zero_no_flap() { + : "ifindex=0 browse must not emit spurious 'removed' events while publishers stay up" + resolvectl flush-caches + + local out_file unit_name service_type added removed + local dummy="ravc-noflap" + + out_file="$(mktemp)" + unit_name="varlinkctl-noflap-$SRANDOM.service" + service_type="_testService5._udp" + + # The flap only manifests when the browser reconciles >=2 same-family mDNS + # scopes: the pre-fix code diffed the browser's global service list against + # each scope's partial answer, spuriously removing services absent from that + # one scope. The host normally has only the container bridge as an mDNS + # interface, so add a service-less dummy link with mDNS enabled to guarantee a + # second (empty) scope that the ifindex=0 reconciliation must combine. This + # must succeed -- without the second scope the testcase asserts nothing. + # A previously interrupted run may have leaked the fixed-name link: RETURN traps + # do not fire when set -e aborts a function mid-flight, so clear it first to keep + # re-runs self-healing instead of tripping over EEXIST here. + ip link del "$dummy" 2>/dev/null || : + ip link add "$dummy" type dummy + # Arm the cleanup before anything else can fail, so the fixed-name link never + # leaks into later testcases. The browse unit may not exist yet, hence the + # best-effort stop. + # shellcheck disable=SC2064 + trap "trap - RETURN; systemctl stop $unit_name 2>/dev/null || :; ip link del $dummy 2>/dev/null || :" RETURN + ip link set "$dummy" up multicast on + ip address add 169.254.171.171/16 dev "$dummy" + resolvectl mdns "$dummy" yes + [[ "$(resolvectl mdns "$dummy")" =~ :\ yes$ ]] + sleep 2 # let resolved create the scope before we start browsing + + # Long-running browse across *all* interfaces (ifindex=0). With the + # combined-answer reconciliation there must be no 'removed' event as long as + # every publisher stays up. Use --timeout=infinity: the subscription goes idle + # once everything is discovered, and varlinkctl's default 45s idle timeout + # would sever it (and the assertion) mid-observation. + systemd-run --unit="$unit_name" --service-type=exec -p StandardOutput="file:$out_file" \ + varlinkctl call --more --timeout=infinity /run/systemd/resolve/io.systemd.Resolve io.systemd.Resolve.BrowseServices \ + "{ \"domain\": \"$service_type.local\", \"type\": \"\", \"ifindex\": 0, \"flags\": 16785432 }" + + # Wait until both containers' services (20 each, 40 total) have been + # discovered. Count occurrences, not lines: varlinkctl --more emits compact + # JSON-SEQ and one notify batches many entries onto a single line. + for _ in {0..14}; do + added="$( { grep -o '"updateFlag":"added"' "$out_file" || :; } | wc -l)" + [[ "$added" -ge 40 ]] && break + sleep 2 + done + if [[ "${added:-0}" -lt 40 ]]; then + echo >&2 "Did not discover the expected services on ifindex=0" + cat "$out_file" >&2 + return 1 + fi + + # Observe a further window during which several continuous-query revisits + # happen; a correct ifindex=0 browse emits zero 'removed' events while every + # publisher stays up. + sleep 12 + + removed="$( { grep -o '"updateFlag":"removed"' "$out_file" || :; } | wc -l)" + if [[ "${removed:-0}" -ne 0 ]]; then + echo >&2 "Got $removed spurious 'removed' event(s) on ifindex=0 while all publishers were up:" + grep -oE '"updateFlag":"removed"[^}]*"name":"[^"]*"' "$out_file" >&2 || : + return 1 + fi + + echo testcase_end +} + testcase_second_unreachable() { : "Test each service type while the second container is unreachable" systemd-run -M "$CONTAINER_2" --wait --pipe -- networkctl down host0