Fix IPv6 socket for Windows.

[SigmundVik]

The IPV6_V6ONLY call to setsockopt() in net__socket_listen()
contained two mistakes for Windows.

The first mistake was that the type of "ss_opt" was wrong.
From the setsockopt() Win32 API documentation: "The optlen
parameter should be equal to sizeof(int) for Boolean options."
https://docs.microsoft.com/en-us/windows/win32/api/winsock/nf-winsock-setsockopt#remarks

The second mistake was that the IPV6_V6ONLY flag must be flipped
on Windows, see for instance how Boost.Asio does this:
https://github.com/boostorg/asio/blob/2d27fc712387c54b7eb45347fc4669be29a38d9e/include/boost/asio/detail/impl/socket_ops.ipp#L1806

This commit also changes the code to only set the IPV6_V6ONLY
flag for IPv6 (similarly to how Boost.Asio does it), because
that seems more correct.

Before this commit, starting the Mosquitto broker when the IPv6 port
already was in use would produce an output like this (and the broker
would be running):

1594385168: mosquitto version 1.6.9 starting
1594385168: Using default config.
1594385168: Opening ipv6 listen socket on port 31001.
1594385168: Opening ipv4 listen socket on port 31001.

With this commit, starting the Mosquitto broker when the IPv6 port
already is in use makes the broker correctly terminate with an exit
code of 1 and this output:

1594385193: mosquitto version 1.6.9 starting
1594385193: Using default config.
1594385193: Opening ipv6 listen socket on port 31001.
1594385193: Error: Only one usage of each socket address (protocol/network address/port) is normally permitted.

Signed-off-by: Sigmund Vik [email protected]

• Have you signed the Eclipse Contributor Agreement, using the same email address as you used in your commits?
• Do each of your commits have a "Signed-off-by" line, with the correct email address? Use "git commit -s" to generate this line for you.
• If you are contributing a new feature, is your work based off the develop branch?
• If you are contributing a bugfix, is your work based off the fixes branch?
• Have you added an explanation of what your changes do and why you'd like us to include them?
• Have you successfully run make test with your changes locally?

---

(Just like before this commit, I need to apply a hack to test/lib/02-subscribe-qos1.py in order to make all tests pass on my Ubuntu 18.04 machine.)

[ralight]

I agree that ss_opt should be an int for Windows, thanks for pointing that out.

The other part of what you are saying does not match the behaviour that I see. I have tested on Windows 10 only, but it works as expected by producing an error if trying to open an IPv6 socket for a port where one is already open. In fact if it was possible to open a socket with the same port but without setting SO_REUSEPORT then there would be a serious problem.

On Windows my understanding that the default behaviour is that IPV6_V6ONLY is set anyway. I don't think it is a problem to set this option on IPv4 sockets, and so there is no need to introduce extra complexity.

If you'd like to change the commit to just make the char to int change I'll happily accept it, otherwise if you'd prefer I can just make the change myself.

[SigmundVik]

Ah, when I run two programs that both set IPV6_V6ONLY to 1 (like two instances of the Mosquitto broker), then I do get the expected behavior (Error: Only one usage of each socket address (protocol/network address/port) is normally permitted.). However, if the Mosquitto broker sets IPV6_V6ONLY to 1 and another program sets IPV6_V6ONLY to 0, then that does not work. I agree that setting IPV6_V6ONLY to 1 the way Mosquitto does it is fine (I will update this PR to only include the ss_opt fix), but I am not sure how to deal with programs that set IPV6_V6ONLY to 0 (which is the default behavior for Boost.Asio)...

On Windows my understanding that the default behaviour is that IPV6_V6ONLY is set anyway. I don't think it is a problem to set this option on IPv4 sockets, and so there is no need to introduce extra complexity.

I agree that it is probably not a problem to set the IPV6_V6ONLY flag for IPPROTO_IPV4, it just looks a bit odd.

[ralight]

That's very interesting. I wonder if the case you describe works because they are effectively binding to different interfaces.

[SigmundVik]

The behavior in this regard seems to be different between Windows 10 and Ubuntu 18.04. On Ubuntu, if I create one version of the Mosquitto broker with IPV6_V6ONLY set to 1 (and no IPv4) and another version with IPV6_V6ONLY set to 0, then they will not both run using the same port. The one that is started last will produce the expected Address already in use error. This is unlike Windows, where unfortunately both brokers will start up without problems.

I did a test on Windows where the Mosquitto broker sets up two sockets when rp->ai_family == AF_INET6, one with IPV6_V6ONLY set to 1 and another with IPV6_V6ONLY set to 0. With this change the broker that is started last will produce the expected Only one usage of each socket address (protocol/network address/port) is normally permitted. error. Is this a solution we want to persuade further? If we are afraid that this change might have unforeseen consequences, then we could silently ignore if listening on the dual-stacked IPv6 socket fails (since the only purpose of doing this is to detect potential port conflicts).

If you'd like to change the commit to just make the char to int change I'll happily accept it, otherwise if you'd prefer I can just make the change myself.

Contrary to what I indicated earlier, it would be great if you could make that change. Then this PR can be used to discuss what to do about dual-stack sockets for IPv6 on Windows.

[SigmundVik]

Here is the commit with my suggested fix.

It also contains a fix that closes all opened sockets (instead of just the last one) when an error occurred. This should be separated out into its own commit.