diff --git a/src/resolve/resolved-dns-browse-services.c b/src/resolve/resolved-dns-browse-services.c index 8b5544c01a..19b773abf4 100644 --- a/src/resolve/resolved-dns-browse-services.c +++ b/src/resolve/resolved-dns-browse-services.c @@ -489,10 +489,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; @@ -515,11 +526,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; } 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