fix: don't suppress a scoped copy of a packet as a duplicate of an unscoped one - #1926

Thanks for contributing! A quick note: this project is in the process of moving from LGPL-2.1 to Apache 2.0, and may keep LGPL-2.1-or-later as an OR option so GPL-2.0-only projects can keep using the library, see #1835.

By continuing with this pull request you agree that this new contribution is offered under both Apache 2.0 and LGPL-2.1-or-later, so it is covered under either outcome.

This notice does not affect your previous contributions. If you would like to consent to relicensing those as well, please post the consent statement on #1835; silence is never treated as consent for past work.

If offering this contribution under these licenses does not work for you, no hard feelings; reply with the exact phrase I do not agree to the Apache 2.0 license or simply close this pull request.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.87%. Comparing base (bc25ee1) to head (ffe6cfd).
⚠️ Report is 1 commits behind head on master.

@@           Coverage Diff           @@
##           master    #1926   +/-   ##
=======================================
  Coverage   99.87%   99.87%           
=======================================
  Files          33       33           
  Lines        3913     3941   +28     
  Branches      568      573    +5     
=======================================
+ Hits         3908     3936   +28     
  Misses          3        3           
  Partials        2        2           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

Previous review — superseded by a newer review below.

Merging this PR will not alter performance

✅ 24 untouched benchmarks

_{Comparing akiernan:fix/dedup-per-sender-scope (ffe6cfd) with master (bc25ee1)}

Previous review — superseded by a newer review below.

Tested 73aa8cf (a334cb0 plus a master merge) against master 7c989c9 on a live home network. Both builds were Cython-compiled and ran side by side for 120 s on macOS with IPVersion.All, browsing all 36 service types on the link. On the 98 services both runs saw, scoping and query handling are unchanged. The PR adds one extra Updated event per service on a host with two interfaces on the same link.

Findings

• On this network the dual-stack bug doesn't change the outcome: these responders' IPv6 copies already arrive first, so master scopes the same 72 services. The 22 unscoped-only services answer over IPv4 only, the case the PR description lists as out of scope.
• Regression: this host has two interfaces on the same link, so every IPv6 copy arrives twice with different scopes. Master suppresses the second copy. The PR admits both, so each fe80:: AAAA is cached under both scopes. A cache-flush answer from one copy then marks the other scope's variant (if older than 1 s) to expire in 1 s, and the next copy re-adds it as a new record. That fires Updated again: 63 of the 98 services got more events than on master, 4 got fewer. This is the cache-flush point from the last review, made worse on a multihomed host.
• Query dedup works: the responder sent the same number of multicast responses on both builds.
• Fix: async_mark_unique_records_older_than_1s_to_expire should not expire an AAAA when the packet carries the same address under another scope. Coming next, then a rerun.

Still need some more work, lots of update event churn.

We don't really have good support for multiple interfaces or separate caches per interface, so this gets messy quickly. We probably need to scope it to single record and update the scope instead

@bdraco: Still need some more work, lots of update event churn.

668e745 stops flush re-add loop. First-seen variants still fire Updated: browser.py:634 sees old=None. Fire only for new address or gained scope. Also check goodbyes: _has_scope_variant ignores TTL 0.

Previous review — superseded by a newer review below.

Retested 3fab5cb against master 7c989c9 on the same network and host as the first run: both builds Cython-compiled, run side by side for 120 s with IPVersion.All. The extra Updated events are gone. The cache now holds each link-local address once, and on the 84 services both runs saw, the PR fires fewer Updated events than master.

What changed since the first run

• One AAAA per address in the cache. RecordManager looks up the cached AAAA for the address under any scope (new DNSCache.async_get_address). A scoped copy replaces an unscoped one. Any other copy takes the cached scope and refreshes that record. A goodbye under any scope removes the cached copy. This replaces three narrower patches: the known-answer collapse, the cache-flush exemption in 668e745 (reverted), and a browser-side Updated filter that was never pushed.
• A scoped copy suppresses later copies from any sender. Each duplicate-suppression entry records whether the processed copy was scoped, instead of keying on (payload, scope). Only an unscoped-first packet lets its scoped copy through, so a dual-stack or multihomed host parses each announcement once more at most, not on every repeat.
• Cost on the merge path: an unscoped copy of an already-scoped AAAA now costs +74 ns per packet over an exact cache hit (812 vs 738 ns through async_updates_from_response), down from +165 ns. The duplicate path and AAAA parsing are unchanged.
• New tests: one AAAA per address for each scope order, an unscoped refresh of a scoped record, goodbyes across scopes, a scoped copy suppressing later copies, and no Updated for a scope copy. The full suite passes with the Cython build and in pure Python (533 passed).

Previous review — superseded by a newer review below.

Retested bf4f5b4 (scoped goodbyes now report a RecordUpdate, and the replaced unscoped copy is removed right away) against master 7c989c9 on the same network and host, both Cython-compiled and running side by side: one 120 s run and two 60 s runs. The PR finds exactly the same services as master. In one run it scoped 22 link-local addresses that master left unscoped, and every extra Updated event comes from those upgrades.

Findings

• In run 2 the Home Assistant host's IPv6 copy arrived after its IPv4 copy. Master dropped it as a duplicate and kept those 22 services unscoped. The PR scoped them, and each upgrade fired one Updated, since the address became usable.
• One service was scoped to the host's other interface (%15 instead of %31). Both are valid on this two-interface host; the cache keeps the first scope it sees.
• Review follow-ups in bf4f5b4:
  • A scoped goodbye for an unscoped cached AAAA now takes the normal goodbye path and reports a RecordUpdate. test_aaaa_goodbye_under_another_scope_removes_cached asserts that for every scope order, and its (0, 2) case fails on 3fab5cb.
  • The replaced unscoped copy is removed immediately rather than staged, so a full cache that evicts it first cannot raise a KeyError in the remove step.
• Not changed: first scope wins on purpose, with one cache for all interfaces. Letting a later scope win would make a multihomed host flap between interfaces. A stale scope after an interface change lasts until the record expires, which the description now states. The async_get_address bucket scan runs only on an AAAA cache miss and walks one hostname's records.
• Full suite: 536 passed with the Cython build and in pure Python.

I think this is the best we can do without massively expanding scope to have a per interface cache. That's definitely outside the scope of this PR

PR Review — fix: don't suppress a scoped copy of a packet as a duplicate of an unscoped one

Merge-ready: all earlier findings are resolved or were dismissed by a maintainer, and one non-blocking suggestion remains.

The last round closed the remaining gaps. A scoped goodbye for an unscoped cached AAAA now takes the normal goodbye path and reports a RecordUpdate, and test_aaaa_goodbye_under_another_scope_removes_cached asserts that for each scope order. The replaced unscoped copy is removed immediately, so eviction from a full cache can't cause a KeyError in the remove step. The PR description now matches the code, including the (time, scoped) dedup entries, the query exemption and the cache merge. Each _recent_packets entry stays bounded, and _set_scope_id runs before answers_rrset is built, so cache flush and goodbyes match the cached record.

I verified this by reading the code. I did not run the tests or a Cython build, because this review is read-only.

• Own link-local AAAA records now miss the recently-multicast checks in both arrival orders, not only when the IPv6 copy arrives first

Not re-raised: first-scope-wins and a stale scope after an interface change. Dismissed per bdraco: "I think this is the best we can do without massively expanding scope to have a per interface cache." The limitation is documented in the PR description. The async_get_address scan is also not re-raised: the author answered that it runs only on an AAAA cache miss and walks one hostname's records.

✅ Resolved since last review (1)

Checklist

• No hardcoded secrets
• No injection or unsafe deserialization
• Collections remain bounded
• Cython declarations updated with the Python change
• Goodbye and cache-flush semantics preserved (RFC 6762 section 10.2)
• Listener notification consistent across goodbye paths
• Rate limiting of own records preserved (RFC 6762 sections 5.4 and 14)
• New behaviour covered by tests, including edge paths
• PR description matches the code

Silent Failure Analysis

if record.scope_id and not cached.scope_id and record.ttl:
    self.cache.async_remove_records((cached,))
    return None
record._set_scope_id(cached.scope_id)
return cached

Automated review by Kōan (Claude) HEAD=ffe6cfd 2 min 13s

@akiernan I think this is ready to go, and it's the best we can do with the current design. Please retest and make sure the cleanups didn't break your use case.

Retested ffe6cfd on the device where I first saw this: Linux 6.12, Python 3.14, pure Python, one interface, IPVersion.All. The control was the PR's merge base on master (bc25ee1). The two ran side by side for 300 s, browsing every service type on the link. The cleanups didn't break our use case.

On Linux the IPv4 copy almost always reaches the :: socket first, from a v4-mapped sender. So master loses the scoped IPv6 copy on most services, rather than only when arrival order goes the wrong way. Both builds found the same 125 services. No name had its address cached under two scopes.

The 4 still unscoped are python-zeroconf responders: a Home Assistant host and three of our own devices. They do answer over IPv6, but from a global address, where mDNSResponder answers from fe80::, so their AAAA records arrive with scope 0. Another responder on the HA host answers from fe80:: and was scoped on its first answer. Scoping these needs the receiving interface (#1568 or IP_PKTINFO), the limitation in the description.

Thanks for taking this on.

Written by Claude, posted on Alex's behalf.