[PATCH v2 0/3] MR5195: ws2_32: Implement TCP_KEEPALIVE, TCP_KEEPCNT and TCP_KEEPINTVL socket options
Application "ZusiDisplay" wants to set these socket options. setsockopt() currently returns an error on wine, which prevents the application from working correctly.
I'm not really sure if I'm using the IOCTL_AFD_WINE_SET/GET_* correctly. Maybe the new ones should have different "function" numbers and be sorted below the one for TCP_NODELAY?
Also, not sure if anything needs to be done in unix/socket.c for portability. Are these TCP_KEEPIDLE etc. constants defined on all OSes supported by wine? If not, how should I handle this?
ZusiDisplay seems to work fine if the setsockopt() call is ignored in wine, without returning an error. So if this merge request is not that great, I'd be happy to send a merge request to ignore the call instead.
-- v2: ws2_32: Implement TCP_KEEP{ALIVE,CNT,INTVL} options.
From: Florian Will florian.will@gmail.com
--- include/ws2ipdef.h | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/include/ws2ipdef.h b/include/ws2ipdef.h index fcb1f56c005..72e2dad2fa5 100644 --- a/include/ws2ipdef.h +++ b/include/ws2ipdef.h @@ -313,6 +313,8 @@ typedef struct WS(in6_pktinfo) { #define TCP_OFFLOAD_PREFERENCE 11 #define TCP_CONGESTION_ALGORITHM 12 #define TCP_DELAY_FIN_ACK 13 +#define TCP_KEEPCNT 16 +#define TCP_KEEPINTVL 17 #else /* WS_TCP_NODELAY is defined elsewhere */ #define WS_TCP_EXPEDITED_1122 2 @@ -327,6 +329,8 @@ typedef struct WS(in6_pktinfo) { #define WS_TCP_OFFLOAD_PREFERENCE 11 #define WS_TCP_CONGESTION_ALGORITHM 12 #define WS_TCP_DELAY_FIN_ACK 13 +#define WS_TCP_KEEPCNT 16 +#define WS_TCP_KEEPINTVL 17 #endif /* USE_WS_PREFIX */
#define PROTECTION_LEVEL_UNRESTRICTED 10
From: Florian Will florian.will@gmail.com
--- dlls/ws2_32/tests/sock.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+)
diff --git a/dlls/ws2_32/tests/sock.c b/dlls/ws2_32/tests/sock.c index b0287fdd004..2563e8769f5 100644 --- a/dlls/ws2_32/tests/sock.c +++ b/dlls/ws2_32/tests/sock.c @@ -1453,6 +1453,34 @@ static void test_set_getsockopt(void) ok(!err, "getsockopt TCP_NODELAY failed\n"); ok(!value, "TCP_NODELAY should be 0\n");
+ size = sizeof(DWORD); + value = 3600; + err = setsockopt(s, IPPROTO_TCP, TCP_KEEPALIVE, (char*)&value, 4); + todo_wine ok(!err, "setsockopt TCP_KEEPALIVE failed\n"); + value = 0; + err = getsockopt(s, IPPROTO_TCP, TCP_KEEPALIVE, (char*)&value, &size); + todo_wine ok(!err, "getsockopt TCP_KEEPALIVE failed\n"); + todo_wine ok(value == 3600, "TCP_KEEPALIVE should be 3600, is %ld\n", value); + + /* TCP_KEEPCNT and TCP_KEEPINTVL are supported on win10 and later */ + value = 5; + err = setsockopt(s, IPPROTO_TCP, TCP_KEEPCNT, (char*)&value, 4); + if (!err || WSAGetLastError() != WSAENOPROTOOPT || winetest_platform_is_wine) + { + todo_wine ok(!err, "setsockopt TCP_KEEPCNT failed: %d\n", WSAGetLastError()); + value = 0; + err = getsockopt(s, IPPROTO_TCP, TCP_KEEPCNT, (char*)&value, &size); + todo_wine ok(!err, "getsockopt TCP_KEEPCNT failed\n"); + todo_wine ok(value == 5, "TCP_KEEPCNT should be 5, is %ld\n", value); + + err = setsockopt(s, IPPROTO_TCP, TCP_KEEPINTVL, (char*)&value, 4); + todo_wine ok(!err, "setsockopt TCP_KEEPINTVL failed\n"); + value = 0; + err = getsockopt(s, IPPROTO_TCP, TCP_KEEPINTVL, (char*)&value, &size); + todo_wine ok(!err, "getsockopt TCP_KEEPINTVL failed\n"); + todo_wine ok(value == 5, "TCP_KEEPINTVL should be 5, is %ld\n", value); + } + /* Test for erroneously passing a value instead of a pointer as optval */ size = sizeof(char); err = setsockopt(s, SOL_SOCKET, SO_DONTROUTE, (char *)1, size);
From: Florian Will florian.will@gmail.com
--- dlls/ntdll/unix/socket.c | 20 +++++++++++++++ dlls/ws2_32/socket.c | 54 ++++++++++++++++++++++++++++++++++++++++ dlls/ws2_32/tests/sock.c | 18 +++++++------- include/wine/afd.h | 6 +++++ 4 files changed, 89 insertions(+), 9 deletions(-)
diff --git a/dlls/ntdll/unix/socket.c b/dlls/ntdll/unix/socket.c index 4e6781df607..232336da221 100644 --- a/dlls/ntdll/unix/socket.c +++ b/dlls/ntdll/unix/socket.c @@ -2507,6 +2507,26 @@ NTSTATUS sock_ioctl( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, void *apc case IOCTL_AFD_WINE_SET_TCP_NODELAY: return do_setsockopt( handle, io, IPPROTO_TCP, TCP_NODELAY, in_buffer, in_size );
+ case IOCTL_AFD_WINE_GET_TCP_KEEPALIVE: + /* TCP_KEEPALIVE on Windows is called TCP_KEEPIDLE on Linux */ + return do_getsockopt( handle, io, IPPROTO_TCP, TCP_KEEPIDLE, out_buffer, out_size ); + + case IOCTL_AFD_WINE_SET_TCP_KEEPALIVE: + /* TCP_KEEPALIVE on Windows is called TCP_KEEPIDLE on Linux */ + return do_setsockopt( handle, io, IPPROTO_TCP, TCP_KEEPIDLE, in_buffer, in_size ); + + case IOCTL_AFD_WINE_GET_TCP_KEEPINTVL: + return do_getsockopt( handle, io, IPPROTO_TCP, TCP_KEEPINTVL, out_buffer, out_size ); + + case IOCTL_AFD_WINE_SET_TCP_KEEPINTVL: + return do_setsockopt( handle, io, IPPROTO_TCP, TCP_KEEPINTVL, in_buffer, in_size ); + + case IOCTL_AFD_WINE_GET_TCP_KEEPCNT: + return do_getsockopt( handle, io, IPPROTO_TCP, TCP_KEEPCNT, out_buffer, out_size ); + + case IOCTL_AFD_WINE_SET_TCP_KEEPCNT: + return do_setsockopt( handle, io, IPPROTO_TCP, TCP_KEEPCNT, in_buffer, in_size ); + default: { if ((code >> 16) == FILE_DEVICE_NETWORK) diff --git a/dlls/ws2_32/socket.c b/dlls/ws2_32/socket.c index 6aab249a1b8..49b380a6329 100644 --- a/dlls/ws2_32/socket.c +++ b/dlls/ws2_32/socket.c @@ -1931,6 +1931,33 @@ int WINAPI getsockopt( SOCKET s, int level, int optname, char *optval, int *optl *optlen = 1; return server_getsockopt( s, IOCTL_AFD_WINE_GET_TCP_NODELAY, optval, optlen );
+ case TCP_KEEPALIVE: + if (*optlen < sizeof(DWORD) || !optval) + { + SetLastError( WSAEFAULT ); + return SOCKET_ERROR; + } + *optlen = sizeof(DWORD); + return server_getsockopt( s, IOCTL_AFD_WINE_GET_TCP_KEEPALIVE, optval, optlen ); + + case TCP_KEEPCNT: + if (*optlen < sizeof(DWORD) || !optval) + { + SetLastError( WSAEFAULT ); + return SOCKET_ERROR; + } + *optlen = sizeof(DWORD); + return server_getsockopt( s, IOCTL_AFD_WINE_GET_TCP_KEEPCNT, optval, optlen ); + + case TCP_KEEPINTVL: + if (*optlen < sizeof(DWORD) || !optval) + { + SetLastError( WSAEFAULT ); + return SOCKET_ERROR; + } + *optlen = sizeof(DWORD); + return server_getsockopt( s, IOCTL_AFD_WINE_GET_TCP_KEEPINTVL, optval, optlen ); + default: FIXME( "unrecognized TCP option %#x\n", optname ); SetLastError( WSAENOPROTOOPT ); @@ -3336,6 +3363,33 @@ int WINAPI setsockopt( SOCKET s, int level, int optname, const char *optval, int value = *optval; return server_setsockopt( s, IOCTL_AFD_WINE_SET_TCP_NODELAY, (char*)&value, sizeof(value) );
+ case TCP_KEEPALIVE: + if (optlen < sizeof(DWORD) || !optval) + { + SetLastError( optlen && optval ? WSAENOBUFS : WSAEFAULT ); + return SOCKET_ERROR; + } + value = *(DWORD*)optval; + return server_setsockopt( s, IOCTL_AFD_WINE_SET_TCP_KEEPALIVE, (char*)&value, sizeof(value) ); + + case TCP_KEEPCNT: + if (optlen < sizeof(DWORD) || !optval) + { + SetLastError( optlen && optval ? WSAENOBUFS : WSAEFAULT ); + return SOCKET_ERROR; + } + value = *(DWORD*)optval; + return server_setsockopt( s, IOCTL_AFD_WINE_SET_TCP_KEEPCNT, (char*)&value, sizeof(value) ); + + case TCP_KEEPINTVL: + if (optlen < sizeof(DWORD) || !optval) + { + SetLastError( optlen && optval ? WSAENOBUFS : WSAEFAULT ); + return SOCKET_ERROR; + } + value = *(DWORD*)optval; + return server_setsockopt( s, IOCTL_AFD_WINE_SET_TCP_KEEPINTVL, (char*)&value, sizeof(value) ); + default: FIXME("Unknown IPPROTO_TCP optname 0x%08x\n", optname); SetLastError(WSAENOPROTOOPT); diff --git a/dlls/ws2_32/tests/sock.c b/dlls/ws2_32/tests/sock.c index 2563e8769f5..131499436cd 100644 --- a/dlls/ws2_32/tests/sock.c +++ b/dlls/ws2_32/tests/sock.c @@ -1456,29 +1456,29 @@ static void test_set_getsockopt(void) size = sizeof(DWORD); value = 3600; err = setsockopt(s, IPPROTO_TCP, TCP_KEEPALIVE, (char*)&value, 4); - todo_wine ok(!err, "setsockopt TCP_KEEPALIVE failed\n"); + ok(!err, "setsockopt TCP_KEEPALIVE failed\n"); value = 0; err = getsockopt(s, IPPROTO_TCP, TCP_KEEPALIVE, (char*)&value, &size); - todo_wine ok(!err, "getsockopt TCP_KEEPALIVE failed\n"); - todo_wine ok(value == 3600, "TCP_KEEPALIVE should be 3600, is %ld\n", value); + ok(!err, "getsockopt TCP_KEEPALIVE failed\n"); + ok(value == 3600, "TCP_KEEPALIVE should be 3600, is %ld\n", value);
/* TCP_KEEPCNT and TCP_KEEPINTVL are supported on win10 and later */ value = 5; err = setsockopt(s, IPPROTO_TCP, TCP_KEEPCNT, (char*)&value, 4); if (!err || WSAGetLastError() != WSAENOPROTOOPT || winetest_platform_is_wine) { - todo_wine ok(!err, "setsockopt TCP_KEEPCNT failed: %d\n", WSAGetLastError()); + ok(!err, "setsockopt TCP_KEEPCNT failed: %d\n", WSAGetLastError()); value = 0; err = getsockopt(s, IPPROTO_TCP, TCP_KEEPCNT, (char*)&value, &size); - todo_wine ok(!err, "getsockopt TCP_KEEPCNT failed\n"); - todo_wine ok(value == 5, "TCP_KEEPCNT should be 5, is %ld\n", value); + ok(!err, "getsockopt TCP_KEEPCNT failed\n"); + ok(value == 5, "TCP_KEEPCNT should be 5, is %ld\n", value);
err = setsockopt(s, IPPROTO_TCP, TCP_KEEPINTVL, (char*)&value, 4); - todo_wine ok(!err, "setsockopt TCP_KEEPINTVL failed\n"); + ok(!err, "setsockopt TCP_KEEPINTVL failed\n"); value = 0; err = getsockopt(s, IPPROTO_TCP, TCP_KEEPINTVL, (char*)&value, &size); - todo_wine ok(!err, "getsockopt TCP_KEEPINTVL failed\n"); - todo_wine ok(value == 5, "TCP_KEEPINTVL should be 5, is %ld\n", value); + ok(!err, "getsockopt TCP_KEEPINTVL failed\n"); + ok(value == 5, "TCP_KEEPINTVL should be 5, is %ld\n", value); }
/* Test for erroneously passing a value instead of a pointer as optval */ diff --git a/include/wine/afd.h b/include/wine/afd.h index 788adb4a495..aba559ebd8a 100644 --- a/include/wine/afd.h +++ b/include/wine/afd.h @@ -285,6 +285,12 @@ C_ASSERT( sizeof(struct afd_get_events_params) == 56 ); #define IOCTL_AFD_WINE_SET_IP_RECVTOS WINE_AFD_IOC(296) #define IOCTL_AFD_WINE_GET_SO_EXCLUSIVEADDRUSE WINE_AFD_IOC(297) #define IOCTL_AFD_WINE_SET_SO_EXCLUSIVEADDRUSE WINE_AFD_IOC(298) +#define IOCTL_AFD_WINE_GET_TCP_KEEPALIVE WINE_AFD_IOC(299) +#define IOCTL_AFD_WINE_SET_TCP_KEEPALIVE WINE_AFD_IOC(300) +#define IOCTL_AFD_WINE_GET_TCP_KEEPCNT WINE_AFD_IOC(301) +#define IOCTL_AFD_WINE_SET_TCP_KEEPCNT WINE_AFD_IOC(302) +#define IOCTL_AFD_WINE_GET_TCP_KEEPINTVL WINE_AFD_IOC(303) +#define IOCTL_AFD_WINE_SET_TCP_KEEPINTVL WINE_AFD_IOC(304)
struct afd_iovec {
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=143572
Your paranoid android.
=== w1064_2qxl (64 bit report) ===
ws2_32: sock.c:13136: Test failed: wait timed out
On Thu Feb 29 13:41:52 2024 +0000, Vijay Kiran Kamuju wrote:
Please fix the comments to C style /* .. */
Oh, sorry about that. Thank you for the reminder!
Also, not sure if anything needs to be done in unix/socket.c for portability. Are these TCP_KEEPIDLE etc. constants defined on all OSes supported by wine? If not, how should I handle this?
A brief search suggests that Solaris, FreeBSD, and NetBSD all have them. Mac has them as well, but it calls it TCP_KEEPALIVE like Windows, not TCP_KEEPIDLE, so we should check for that. If we do break compilation anywhere, I suspect it's fine to wait for a report and fix it after the fact.
We've had problems in the past with applications being very picky about how we do parameter validation, so I'd like to be proactive about getting that right, or at least not copy it from elsewhere without testing. The relevant tests are a bit tricky to read, so I've taken the liberty of adding them and adjusting the implementation appropriately; would you mind fixing up your patches accordingly?
[scratch.diff](/uploads/ecf33271bf2f18bce38f7ef6daf40aa7/scratch.diff)
``` + /* TCP_KEEPCNT and TCP_KEEPINTVL are supported on win10 and later */ + value = 5; + err = setsockopt(s, IPPROTO_TCP, TCP_KEEPCNT, (char*)&value, 4); + if (!err || WSAGetLastError() != WSAENOPROTOOPT || winetest_platform_is_wine) + { + todo_wine ok(!err, "setsockopt TCP_KEEPCNT failed: %d\n", WSAGetLastError()); + value = 0; + err = getsockopt(s, IPPROTO_TCP, TCP_KEEPCNT, (char*)&value, &size); + todo_wine ok(!err, "getsockopt TCP_KEEPCNT failed\n"); + todo_wine ok(value == 5, "TCP_KEEPCNT should be 5, is %ld\n", value); ```
I'd rather write this as:
``` + err = setsockopt(s, IPPROTO_TCP, TCP_KEEPCNT, (char*)&value, 4); + todo_wine ok(!err || broken(WSAGetLastError() != WSAENOPROTOOPT), "..."); + if (!err) + { + value = 0; + err = getsockopt(s, IPPROTO_TCP, TCP_KEEPCNT, (char*)&value, &size); + todo_wine ok(!err, "getsockopt TCP_KEEPCNT failed\n"); + todo_wine ok(value == 5, "TCP_KEEPCNT should be 5, is %ld\n", value); ```
-
Florian Will -
Florian Will (＠w-flo)
-
Marvin -
Zebediah Figura (＠zfigura)