[PATCH 0/2] MR6335: winsock: Support IPV4 UDP broadcast on connected socket without SO_BROADCAST.
From: Paul Gofman pgofman@codeweavers.com
--- dlls/ws2_32/tests/sock.c | 49 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+)
diff --git a/dlls/ws2_32/tests/sock.c b/dlls/ws2_32/tests/sock.c index e8c618cc0af..33e99e7df83 100644 --- a/dlls/ws2_32/tests/sock.c +++ b/dlls/ws2_32/tests/sock.c @@ -14194,6 +14194,54 @@ static void test_select_after_WSAEventSelect(void) closesocket(client); }
+static void test_broadcast(void) +{ + struct sockaddr_in bcast = {.sin_family = AF_INET, .sin_port = htons(12345), .sin_addr.s_addr = htonl(INADDR_BROADCAST)}; + struct sockaddr_in6 mcast6; + int val, ret, len; + SOCKET s; + + s = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP); + ok(s != INVALID_SOCKET, "got error %u.\n", WSAGetLastError()); + ret = sendto(s, "test", 4, 0, (struct sockaddr *)&bcast, sizeof(bcast)); + ok(ret == -1, "got %d, error %u.\n", ret, WSAGetLastError()); + val = 1; + ret = setsockopt(s, SOL_SOCKET, SO_BROADCAST, (char*)&val, sizeof(val)); + ok(!ret, "got %d, error %u.\n", ret, WSAGetLastError()); + ret = sendto(s, "test", 4, 0, (struct sockaddr *)&bcast, sizeof(bcast)); + ok(ret == 4, "got %d, error %u.\n", ret, WSAGetLastError()); + + val = 0; + ret = setsockopt(s, SOL_SOCKET, SO_BROADCAST, (char*)&val, sizeof(val)); + ok(!ret, "got %d, error %u.\n", ret, WSAGetLastError()); + ret = sendto(s, "test", 4, 0, (struct sockaddr *)&bcast, sizeof(bcast)); + ok(ret == -1, "got %d, error %u.\n", ret, WSAGetLastError()); + + ret = connect(s, (struct sockaddr *)&bcast, sizeof(bcast)); + todo_wine ok(!ret, "got error %u.\n", WSAGetLastError()); + val = 1; + len = sizeof(val); + ret = getsockopt(s, SOL_SOCKET, SO_BROADCAST, (char*)&val, &len); + ok(!ret, "got %d, error %u.\n", ret, WSAGetLastError()); + ok(!val, "got %d.\n", val); + ret = sendto(s, "test", 4, 0, (struct sockaddr *)&bcast, sizeof(bcast)); + ok(ret == -1, "got %d, error %u.\n", ret, WSAGetLastError()); + ret = send(s, "test", 4, 0); + todo_wine ok(ret == 4, "got %d, error %u.\n", ret, WSAGetLastError()); + closesocket(s); + + s = socket(AF_INET6, SOCK_DGRAM, IPPROTO_UDP); + ok(s != INVALID_SOCKET, "got error %u.\n", WSAGetLastError()); + memset(&mcast6, 0, sizeof(mcast6)); + ret = inet_pton(AF_INET6, "ff01::1", &mcast6.sin6_addr); + ok(ret, "got error %u.\n", WSAGetLastError()); + mcast6.sin6_family = AF_INET6; + mcast6.sin6_port = htons(12345); + ret = sendto(s, "test", 4, 0, (struct sockaddr *)&mcast6, sizeof(mcast6)); + ok(ret == 4, "got %d, error %u.\n", ret, WSAGetLastError()); + closesocket(s); +} + START_TEST( sock ) { int i; @@ -14276,6 +14324,7 @@ START_TEST( sock ) test_icmp(); test_connect_udp(); test_tcp_sendto_recvfrom(); + test_broadcast();
/* There is apparently an obscure interaction between this test and * test_WSAGetOverlappedResult().
From: Paul Gofman pgofman@codeweavers.com
--- dlls/ws2_32/tests/sock.c | 4 ++-- server/sock.c | 20 ++++++++++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-)
diff --git a/dlls/ws2_32/tests/sock.c b/dlls/ws2_32/tests/sock.c index 33e99e7df83..0015aae67a5 100644 --- a/dlls/ws2_32/tests/sock.c +++ b/dlls/ws2_32/tests/sock.c @@ -14218,7 +14218,7 @@ static void test_broadcast(void) ok(ret == -1, "got %d, error %u.\n", ret, WSAGetLastError());
ret = connect(s, (struct sockaddr *)&bcast, sizeof(bcast)); - todo_wine ok(!ret, "got error %u.\n", WSAGetLastError()); + ok(!ret, "got error %u.\n", WSAGetLastError()); val = 1; len = sizeof(val); ret = getsockopt(s, SOL_SOCKET, SO_BROADCAST, (char*)&val, &len); @@ -14227,7 +14227,7 @@ static void test_broadcast(void) ret = sendto(s, "test", 4, 0, (struct sockaddr *)&bcast, sizeof(bcast)); ok(ret == -1, "got %d, error %u.\n", ret, WSAGetLastError()); ret = send(s, "test", 4, 0); - todo_wine ok(ret == 4, "got %d, error %u.\n", ret, WSAGetLastError()); + ok(ret == 4, "got %d, error %u.\n", ret, WSAGetLastError()); closesocket(s);
s = socket(AF_INET6, SOCK_DGRAM, IPPROTO_UDP); diff --git a/server/sock.c b/server/sock.c index 06ffd1b81f8..ae70fb7fb24 100644 --- a/server/sock.c +++ b/server/sock.c @@ -2668,6 +2668,26 @@ static void sock_ioctl( struct fd *fd, ioctl_code_t code, struct async *async ) ret = connect( unix_fd, &unix_addr.addr, unix_len ); }
+ if (ret < 0 && errno == EACCES && sock->state == SOCK_CONNECTIONLESS && unix_addr.addr.sa_family == AF_INET + && unix_addr.in.sin_addr.s_addr == htonl( INADDR_BROADCAST )) + { + int broadcast, saved_errno; + socklen_t len = sizeof(broadcast); + + broadcast = 1; + getsockopt( unix_fd, SOL_SOCKET, SO_BROADCAST, &broadcast, &len ); + if (!broadcast) + { + broadcast = 1; + setsockopt( unix_fd, SOL_SOCKET, SO_BROADCAST, &broadcast, sizeof(broadcast) ); + ret = connect( unix_fd, &unix_addr.addr, unix_len ); + saved_errno = errno; + broadcast = 0; + setsockopt( unix_fd, SOL_SOCKET, SO_BROADCAST, &broadcast, sizeof(broadcast) ); + errno = saved_errno; + } + } + if (ret < 0 && errno != EINPROGRESS) { set_error( sock_get_ntstatus( errno ) );
Hi,
It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated.
The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details:
The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=147920
Your paranoid android.
=== w1064_2qxl (64 bit report) ===
ws2_32: sock.c:13177: Test failed: wait timed out sock.c:13193: Test failed: wait timed out sock.c:13206: Test failed: got size 5 sock.c:13207: Test failed: got "datad" sock.c:13040: Test failed: got error 996 sock.c:13041: Test failed: got size 0 sock.c:13042: Test failed: got "" sock.c:13040: Test failed: got error 996 sock.c:13041: Test failed: got size 0 sock.c:13042: Test failed: got ""
Motivated by the issue description here: https://github.com/ValveSoftware/Proton/issues/3189#issuecomment-2303407967 .
I didn't check anything with the game, but provided example shows the difference between Windows and Wine behaviour. I don't see Win11 specifics though, the tests I added so far seem to work on all the Testbot versions (while locally I am testing on Win11). The interesting bit however is that sendto() with given address is not allowed without SO_BROADCAST on Windows too, this "SO_BROADCAST" bypass works only when connect() is used on udp socket to attach to broadcast address and then send() without address. It works in a somewhat similar way on Linux. Unlike Windows, SO_BROADCAST is required to be set for connect to succeed. But when connect is performed send() without address will succeed even with later disabled SO_BROADCAST. I verified locally with Wireshark that it is not just success status, the broadcasts are actually sent over network when running included tests both on Windows and Wine with the patch.
David Gow (@sulix) commented about server/sock.c:
ret = connect( unix_fd, &unix_addr.addr, unix_len ); }
if (ret < 0 && errno == EACCES && sock->state == SOCK_CONNECTIONLESS && unix_addr.addr.sa_family == AF_INET&& unix_addr.in.sin_addr.s_addr == htonl( INADDR_BROADCAST ))
Does this need to also handle other broadcast subnets? e.g., for the 192.168.0.0/24 subnet, 192.168.255.255?
This doesn't affect Age of Empires / BattleServer (which always uses INADDR_BROADCAST/255.255.255.255), but could potentially be an issue on other applications. It is a lot more work, though…
Thanks very much! This seems to be working for Age of Empires 1 & 2 — at the very least, it resolves the issues with getting into the game lobby, and (in Age of Empires 2) starting a LAN game. Promising-looking packets are flowing. I haven't got a second copy of the game here to test an actual LAN multiplayer game fully at the moment, but this is definitely an improvement.
Indeed, this works better than the [patch here](https://github.com/ValveSoftware/Proton/issues/3189#issuecomment-1823909045) did when I tested it, though that may be due to other wine or game updates since: the lobby no longer disconnects on AOE2.
One minor thought about the implementation: does it need to support other broadcast addresses, not just 255.255.255.255? It's fine as-is for Age of Empires, but maybe something else cares.
-
David Gow (＠sulix) -
Marvin -
Paul Gofman
-
Paul Gofman (＠gofman)