Skip to content

Attempting to bind specific interface using interfaces option doesn't work #193

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
kdart opened this issue Oct 22, 2019 · 7 comments
Closed

Attempting to bind specific interface using interfaces option doesn't work #193

kdart opened this issue Oct 22, 2019 · 7 comments

Comments

@kdart
Copy link

kdart commented Oct 22, 2019

The documentation implies that Zeroconf(interfaces=[<localIP>]) should restrict listening to one port. But it did not work because the actual socket is bound to all ports.

The following patch fixes it.

1747c1747
< def new_socket(port: int = _MDNS_PORT) -> socket.socket:
---
> def new_socket(port: int = _MDNS_PORT, address: str = "") -> socket.socket:
1778c1778
<     s.bind(('', port))
---
>     s.bind((address, port))
1840c1840
<                 respond_socket = new_socket()
---
>                 respond_socket = new_socket(address=i)
1843c1843
<                 respond_socket = new_socket(port=0)
---
>                 respond_socket = new_socket(port=0, address=i)
@garethsb garethsb mentioned this issue Oct 28, 2019
Open
@garethsb
Copy link

garethsb commented Oct 29, 2019 •
edited
Loading

Thanks, @kdart!

@jstasiak, I've tested this patch on v0.23.0 (on Windows) and it works for me too (although #180 indicates there could be more to a full solution). I started to turn it into a PR, but I'm not quite sure what to do about the interaction with the new IPv6 support on master. socket.bind for IPv6 sockets still needs host and port (at least), and we only have interface index currently?

@schmittner schmittner mentioned this issue Dec 17, 2019
Merged
@garethsb
Copy link

@jstasiak Do you have any thoughts on whether this approach is good, and could be integrated with master? It would be very useful to either be able to restrict browsing to one port or to ensure all interfaces' advertisements are discovered and reported. At the moment it seems a bit of a lottery. Thanks.

@kdart
Copy link
Author

kdart commented Mar 18, 2020

Honestly, I think this is a bit of a quick fix, but could be better. I'll see if I can look at this again.

@jstasiak
Copy link
Collaborator

Hey @kdart and @garethsb-sony, thanks for the path and the information, I totally forgot about this – I'll try to have a closer look at it soon.

@jstasiak
Copy link
Collaborator

jstasiak commented Jun 2, 2020

@garethsb-sony @kdart as you mentioned it was a bit more involved to integrate this with IPv6 support, but I managed to merge your suggestion with @estyrke's patch from #188 and I ended up with #270 with all tests passing. Do you mind testing the version of the code from that pull request (branch rebase-188) and letting me know if it addresses the problem for you?

@garethsb
Copy link

garethsb commented Jun 4, 2020

Thank you, @jstasiak. I will make some time to evaluate (I need to refresh my memory on the set up I had with DNS-SD services being advertised/discovered on multiple interfaces).

@jstasiak
Copy link
Collaborator

jstasiak commented Jul 7, 2020

I just merged #270 and released version 0.28.0, if effectively does what you propose in the original comment here, @kdart. I'll close this issue now as I consider it handled, please reopen it though (or create a new one) if you still have issues with this.

@jstasiak jstasiak closed this as completed Jul 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@kdart @jstasiak @garethsb