Skip to content

Send unicast replies on the same socket the query was received#952

Merged
bdraco merged 8 commits into
python-zeroconf:masterfrom
bdraco:unicast_respond
Aug 13, 2021
Merged

Send unicast replies on the same socket the query was received#952
bdraco merged 8 commits into
python-zeroconf:masterfrom
bdraco:unicast_respond

Conversation

@bdraco
Copy link
Copy Markdown
Member

@bdraco bdraco commented Aug 12, 2021 •
edited
Loading

When replying to a QU question, we do not know if the sending host is reachable
from all of the sending sockets. We now avoid this problem by replying via
the receiving socket. This was the existing behavior when InterfaceChoice.Default
is set.

This change extends the unicast relay behavior to used with InterfaceChoice.Default
to apply when InterfaceChoice.All or interfaces are explicitly passed when
instantiating a Zeroconf instance.

Fixes #951

Reference home-assistant/core#54531
Reference home-assistant/core#54434
Reference home-assistant/core#54487

@bdraco
Copy link
Copy Markdown
Member Author

bdraco commented Aug 12, 2021

I don't think this is going to work since on the non-default case we end up with a socket list like:

2021-08-12 15:36:40 DEBUG (MainThread) [zeroconf] Listen socket <socket.socket fd=21, family=AddressFamily.AF_INET6, type=SocketKind.SOCK_DGRAM, proto=0, laddr=('::', 5353, 0, 0)>, respond sockets [<socket.socket fd=22, family=AddressFamily.AF_INET, type=SocketKind.SOCK_DGRAM, proto=0, laddr=('192.168.213.157', 5353)>, <socket.socket fd=23, family=AddressFamily.AF_INET6, type=SocketKind.SOCK_DGRAM, proto=0, laddr=('fda4:6b56:11f8:1:b3c0:811e:aff0:d60', 5353, 0, 0)>]

@bdraco
Copy link
Copy Markdown
Member Author

bdraco commented Aug 12, 2021

Maybe pass the transport instead

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Aug 12, 2021 •
edited
Loading

Codecov Report

Merging #952 (1c2357d) into master (ebc23ee) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #952   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           22        22           
  Lines         2428      2433    +5     
  Branches       401       401           
=========================================
+ Hits          2428      2433    +5
Impacted Files Coverage Δ
zeroconf/_core.py 100.00% <100.00%> (ø)

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 ebc23ee...1c2357d. Read the comment docs.

@bdraco
Copy link
Copy Markdown
Member Author

bdraco commented Aug 13, 2021

Seems to be working well.

I'd like to add some more tests though

@bdraco
Copy link
Copy Markdown
Member Author

bdraco commented Aug 13, 2021 •
edited
Loading

Received confirmation that this fixes an affected system, looking for additional confirmation.

This was referenced Aug 13, 2021
Closed
Closed
@bdraco bdraco marked this pull request as ready for review August 13, 2021 13:25
@bdraco bdraco merged commit 5fb3e20 into python-zeroconf:master Aug 13, 2021
@bdraco bdraco deleted the unicast_respond branch August 13, 2021 13:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Only respond to unicast queries on the receiving interface

2 participants

@bdraco @codecov-commenter