Skip to content
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.

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

If socket allocations fails try again without IPv6 #245

Merged
merged 1 commit into from May 13, 2024

Conversation

Sean-Der
Copy link
Contributor

Resolves #244

@paullouisageneau
Copy link
Owner

Thank you for looking into the issue. While I'm open to a mitigation, this looks like a bug with NordVPN split tunnelling on Windows, because binding on IN6ADDR_ANY_INIT should always work provided the host supports IPv6 (when no interface has a valid IPv6 address it should effectively listen on localhost for IPv6).

The way you implemented the retry logic introduces unwanted behaviors. For instance, when IPv4 only is available, if it fails at a some point it will retry with prefer_ipv6 == false which will behave exactly the same, probably leading to twice the same error. Also, in introduces a flag which I'd prefer to avoid.

IMHO you should do the reverse, socket binding should simply be moved in the address family loop with socket creation. This way, if a failure happens, it will retry with the next address family. I pushed a suggestion, does it work for you?

@Sean-Der
Copy link
Contributor Author

@paullouisageneau Just tested and it works great!

Any way you want to solve the problem I am in support of (you are the one who has to maintain the code)

@Sean-Der
Copy link
Contributor Author

It is possible to get a new patch release of libdatachannel with this?

No worries if not, but would be nice to go into next major OBS release with a clean slate :) this is the last issue I am aware of

…ilure

Resolves paullouisageneau#244

Co-authored-by: Paul-Louis Ageneau <paul-louis@ageneau.org>
Co-authored-by: Sean DuBois <sean@siobud.com>
@paullouisageneau
Copy link
Owner

@Sean-Der Sure, I'll include it in a new patch release shortly!

@paullouisageneau paullouisageneau merged commit 9d35168 into paullouisageneau:master May 13, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Attempting to bind a IPv6 socket fails with NordVPN Installed
2 participants