Make IPv4 and IPv6 use separate sockets#3774
Conversation
Instead of blocking in recvfrom(), use poll() to wait until socket is ready to read, then receive all pending packets until it would block.
softins
added
bug
| continue; | ||
| } | ||
|
|
||
| // Server Bind IPv6 -------------------------------------------------- |
There was a problem hiding this comment.
Can we add --serverbindip4 as an alias for --serverbindip, too, please. (Once full Directory support is available, we can add --noipv4, as well...)
| @@ -50,8 +50,15 @@ | |||
| #ifdef _WIN32 | |||
There was a problem hiding this comment.
This all looks a bit hacky. Worth a comment.
There was a problem hiding this comment.
This is still WIP, and I agree it can be improved
| const bool bDisableIPv4 = false; // for future use | ||
|
|
||
| // first, close sockets if they are open - in case of re-init | ||
| if ( UdpSocket4 == INVALID_SOCKET ) |
There was a problem hiding this comment.
Are these meant to be !=? At the moment it says "if it's in valid, close it and set it to invalid", which sounds strange and redundant.
There was a problem hiding this comment.
Yes, you're right. I'll fix it in the next push. Same for 6 a few lines down.
|
I hadn't realised quite how strange Windows SDK sockets were... |
| UdpSocket4 = socket ( AF_INET, SOCK_DGRAM, 0 ); | ||
| if ( UdpSocket4 == INVALID_SOCKET ) | ||
| { | ||
| // IPv4 requested but not available, throw error (should never happen, but check anyway) |
There was a problem hiding this comment.
I believe that this could indeed happen on cheap ipv6 only VPS...
|
Crashes with exit code 141 on macOS for me. Edit: this might also be related to something else. |
Built from source or installed from the PR CI build? In any case, it's still a work in progress, with more to be done when I get home from holiday.
Ah, ok :) |
|
I think it's due to a missing ini it somehow cannot recreate as it occurs even on the official style release. |
|
Actually no. It happens only if I am connected via ethernet (on latest stable). Maybe something up with DNS (1.1.1.1)? Edit: no even switching to the standard DNS server it crashes as soon as I hit connect. Very odd. |
|
Ok. It seems to be related to the macOS firewall. If I disable it everything works fine. If I don't jamulus exits with sigpipe. I assume that this could be related. The hang I reported before could definitely be due to a non existent route. |
Just to clarify, that happens also on main and 3.12.2, not just this PR? |
|
Yes. #3776 |
3 participants
Short description of changes
Instead of using a single IP socket as dual-stack for IPv4 and IPv6, change to a separate socket for each protocol, but both contained within a single
CSocket. This has the following advantages:The sockets are set to non-blocking. In
OnDataReceived(), usepoll()to wait for either or both sockets to become ready, then read and process all available data before returning.When sending a packet, the correct socket is selected depending on the protocol of the destination address.
CHANGELOG: Client/Server: Improve handling of IPv6 by using separate sockets
Context: Fixes an issue?
May fix #3758
Does this change need documentation? What needs to be documented and how?
New option
--serverbindip6will need including in relevant documentation.Status of this Pull Request
Work in progress
What is missing until this pull request can be merged?
Cross-platform compilation needs to be tidied up and tested, particularly in
socket.cpp. Testing on all supported platforms. So far, only tested on a Mac with a variable-quality internet connection!Checklist