Skip to content

Suppress additionals when answer is suppressed#690

Merged
bdraco merged 15 commits into
python-zeroconf:masterfrom
bdraco:additionals_linked_to_answers
Jun 16, 2021
Merged

Suppress additionals when answer is suppressed#690
bdraco merged 15 commits into
python-zeroconf:masterfrom
bdraco:additionals_linked_to_answers

Conversation

@bdraco

@bdraco bdraco commented Jun 16, 2021

Copy link
Copy Markdown
Member

https://datatracker.ietf.org/doc/html/rfc6762#section-5.4

When receiving a question with the unicast-response bit set, a
responder SHOULD usually respond with a unicast packet directed back
to the querier. However, if the responder has not multicast that
record recently (within one quarter of its TTL), then the responder
SHOULD instead multicast the response so as to keep all the peer
caches up to date, and to permit passive conflict detection. In the
case of answering a probe question (Section 8.1) with the unicast-
response bit set, the responder should always generate the requested
unicast response, but it may also send a multicast announcement if
the time since the last multicast announcement of that record is more
than a quarter of its TTL.

Unicast replies are subject to all the same packet generation rules
as multicast replies, including the cache-flush bit (Section 10.2)
and (except when defending a unique name against a probe from another
host) randomized delays to reduce network collisions (Section 6).

This means additionals are properties of the answers and we only multicast them if we are multicasting the answer as well.

Apple's mDNSResponder behaves this way in testing so lets do that
fixes #689

@codecov-commenter

codecov-commenter commented Jun 16, 2021

Copy link
Copy Markdown

Codecov Report

Merging #690 (3430f61) into master (993a82e) will increase coverage by 0.03%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #690      +/-   ##
==========================================
+ Coverage   98.16%   98.20%   +0.03%     
==========================================
  Files          17       17              
  Lines        2130     2119      -11     
  Branches      374      374              
==========================================
- Hits         2091     2081      -10     
  Misses         23       23              
+ Partials       16       15       -1     
Impacted Files Coverage Δ
zeroconf/_handlers.py 100.00% <100.00%> (ø)
zeroconf/_utils/aio.py 100.00% <0.00%> (+3.57%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 993a82e...3430f61. Read the comment docs.

@bdraco

bdraco commented Jun 16, 2021

Copy link
Copy Markdown
Member Author

grep 'Sending to (None, 5353)' -B 10 -A 10 home-assistant.log looks much better now

@bdraco

bdraco commented Jun 16, 2021

Copy link
Copy Markdown
Member Author

I think we need to suppress known answers earlier

Screen Shot 2021-06-16 at 11 48 03 AM

@bdraco

bdraco commented Jun 16, 2021

Copy link
Copy Markdown
Member Author

Screen Shot 2021-06-16 at 12 06 09 PM

@bdraco

bdraco commented Jun 16, 2021

Copy link
Copy Markdown
Member Author

Screen Shot 2021-06-16 at 12 48 04 PM

Looks good now

@bdraco bdraco merged commit 0cdba98 into python-zeroconf:master Jun 16, 2021
@bdraco bdraco deleted the additionals_linked_to_answers branch June 16, 2021 22:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix fate sharing of additionals

2 participants