[PATCH 0/4] MR286: nsiproxy.sys: Add static ARP entries which are always present on Windows.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=53175 Commit 910d58520a6d75cada82d992757ca013099d9345 (iphlpapi: Return ERROR_NO_DATA from GetIpNetTable() if no entries are found.) introduced a regression in Roon application. It looks like the app doesn't tolerate an error return from GetIpNetTable() if there are active interfaces found. The blamed patch still looks correct to me per se, that is, if number of entries ends up being zero Windows returns ERROR_NO_DATA. However, number of entries is never zero on Windows if there is any network adapter. There are two things why Windows always has some arp entries and we do not: 1. We currently parse /proc/net/arp line on Linux a bit wrong and end up getting no entries from there (patch 1 fixes that); 2. With the patch patch 1 there Roon seems happy (at least as far as I could test it right at start), at least as long we have an entry in /proc/sys/net (which might not be necessarily the case if there is, e. g., no actual network connection). But that breaks SCP: Secret Laboratory again the same way as before 910d58520a6d75cada82d992757ca013099d9345. It turns out the latter game depends on GetIpNetTable() returning at least 4 entries when returning success (for some reason, I am not sure why but I think I saw that quite clearly). And both apps can actually work reliably on Windows as there are always at least 4 static arp entries if there is any network adaptere configured at all. There are "224.0.0.22" and "239.255.255.250" static arp multicast entries which are always there for any interface on Windows, including loopback (starting from Win10; earlier versions might be missing "239.255.255.250" but have "224.0.0.22"). So adding those like on Windows should hopefully fix all those cases. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/286
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/nsiproxy.sys/ip.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/dlls/nsiproxy.sys/ip.c b/dlls/nsiproxy.sys/ip.c index 27344756580..79f3bd80bfe 100644 --- a/dlls/nsiproxy.sys/ip.c +++ b/dlls/nsiproxy.sys/ip.c @@ -1014,7 +1014,7 @@ static NTSTATUS ipv4_neighbour_enumerate_all( void *key_data, UINT key_size, voi #ifdef __linux__ { - char buf[512], *ptr; + char buf[512], *ptr, *s; UINT atf_flags; FILE *fp; @@ -1053,6 +1053,9 @@ static NTSTATUS ipv4_neighbour_enumerate_all( void *key_data, UINT key_size, voi while (*ptr && !isspace( *ptr )) ptr++; /* mask (skip) */ while (*ptr && isspace( *ptr )) ptr++; + s = ptr; + while (*s && !isspace(*s)) s++; + *s = 0; if (!convert_unix_name_to_luid( ptr, &entry.luid )) continue; if (!convert_luid_to_index( &entry.luid, &entry.if_index )) continue; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/286
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/iphlpapi/iphlpapi_main.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dlls/iphlpapi/iphlpapi_main.c b/dlls/iphlpapi/iphlpapi_main.c index 33b12361d70..5d904d875fe 100644 --- a/dlls/iphlpapi/iphlpapi_main.c +++ b/dlls/iphlpapi/iphlpapi_main.c @@ -2344,7 +2344,8 @@ DWORD WINAPI GetIpNetTable( MIB_IPNETTABLE *table, ULONG *size, BOOL sort ) memset( row->bPhysAddr + row->dwPhysAddrLen, 0, sizeof(row->bPhysAddr) - row->dwPhysAddrLen ); row->dwAddr = keys[i].addr.s_addr; - switch (dyn->state) + + switch (dyn[i].state) { case NlnsUnreachable: case NlnsIncomplete: -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/286
From: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/iphlpapi/iphlpapi_main.c | 3 +++ dlls/iphlpapi/tests/iphlpapi.c | 15 ++++++++++++++- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/dlls/iphlpapi/iphlpapi_main.c b/dlls/iphlpapi/iphlpapi_main.c index 5d904d875fe..75024a249bb 100644 --- a/dlls/iphlpapi/iphlpapi_main.c +++ b/dlls/iphlpapi/iphlpapi_main.c @@ -2282,6 +2282,9 @@ err: static int ipnetrow_cmp( const void *a, const void *b ) { const MIB_IPNETROW *rowA = a, *rowB = b; + + if (rowA->dwIndex != rowB->dwIndex) return rowA->dwIndex < rowB->dwIndex ? -1 : 1; + return DWORD_cmp(RtlUlongByteSwap( rowA->dwAddr ), RtlUlongByteSwap( rowB->dwAddr )); } diff --git a/dlls/iphlpapi/tests/iphlpapi.c b/dlls/iphlpapi/tests/iphlpapi.c index 29256eb2774..af893aeae54 100644 --- a/dlls/iphlpapi/tests/iphlpapi.c +++ b/dlls/iphlpapi/tests/iphlpapi.c @@ -370,6 +370,7 @@ static void testGetIpNetTable(void) { DWORD apiReturn; ULONG dwSize = 0; + unsigned int i; apiReturn = GetIpNetTable(NULL, NULL, FALSE); if (apiReturn == ERROR_NOT_SUPPORTED) { @@ -389,11 +390,23 @@ static void testGetIpNetTable(void) PMIB_IPNETTABLE buf = HeapAlloc(GetProcessHeap(), 0, dwSize); memset(buf, 0xcc, dwSize); - apiReturn = GetIpNetTable(buf, &dwSize, FALSE); + apiReturn = GetIpNetTable(buf, &dwSize, TRUE); ok((apiReturn == NO_ERROR && buf->dwNumEntries) || (apiReturn == ERROR_NO_DATA && !buf->dwNumEntries), "got apiReturn %lu, dwSize %lu, buf->dwNumEntries %lu.\n", apiReturn, dwSize, buf->dwNumEntries); + if (apiReturn == NO_ERROR) + { + for (i = 0; i < buf->dwNumEntries - 1; ++i) + { + ok( buf->table[i].dwIndex <= buf->table[i + 1].dwIndex, + "Entries are not sorted by index, i %u.\n", i ); + if (buf->table[i].dwIndex == buf->table[i + 1].dwIndex) + ok(ntohl(buf->table[i].dwAddr) <= ntohl(buf->table[i + 1].dwAddr), + "Entries are not sorted by address, i %u.\n", i ); + } + } + if (apiReturn == NO_ERROR && winetest_debug > 1) { DWORD i, j; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/286
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 full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=117340 Your paranoid android. === w7u_2qxl (32 bit report) === iphlpapi: iphlpapi.c:353: Test failed: 0: got 37 vs 38 iphlpapi.c:353: Test failed: 1: got 33 vs 34 iphlpapi.c:353: Test failed: 2: got 33 vs 34 iphlpapi.c:353: Test failed: 3: got 33 vs 34 iphlpapi.c:353: Test failed: 4: got 39 vs 40 iphlpapi.c:353: Test failed: 5: got 39 vs 40 iphlpapi.c:353: Test failed: 6: got 39 vs 40 iphlpapi.c:353: Test failed: 7: got 39 vs 40 iphlpapi.c:353: Test failed: 8: got 37 vs 38 iphlpapi.c:353: Test failed: 9: got 39 vs 40 iphlpapi.c:353: Test failed: 10: got 37 vs 38
This looks unrelated to my patches, the changes do not touch anything in the tables concerned by the test. It is 1sec difference in dwForwardAge between GetIpForwardTable() and GetIpForwardTable2() called one after another and the test is probably a bit flaky: we should probably add a 1sec tolerance to comparison. On 6/20/22 21:25, Marvin wrote:
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 full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=117340
Your paranoid android.
=== w7u_2qxl (32 bit report) ===
iphlpapi: iphlpapi.c:353: Test failed: 0: got 37 vs 38 iphlpapi.c:353: Test failed: 1: got 33 vs 34 iphlpapi.c:353: Test failed: 2: got 33 vs 34 iphlpapi.c:353: Test failed: 3: got 33 vs 34 iphlpapi.c:353: Test failed: 4: got 39 vs 40 iphlpapi.c:353: Test failed: 5: got 39 vs 40 iphlpapi.c:353: Test failed: 6: got 39 vs 40 iphlpapi.c:353: Test failed: 7: got 39 vs 40 iphlpapi.c:353: Test failed: 8: got 37 vs 38 iphlpapi.c:353: Test failed: 9: got 39 vs 40 iphlpapi.c:353: Test failed: 10: got 37 vs 38
From: Paul Gofman <pgofman(a)codeweavers.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=53175 --- dlls/iphlpapi/tests/iphlpapi.c | 29 +++++++++++++++++++++++- dlls/nsi/tests/nsi.c | 6 +++++ dlls/nsiproxy.sys/ip.c | 40 ++++++++++++++++++++++++++++++++++ 3 files changed, 74 insertions(+), 1 deletion(-) diff --git a/dlls/iphlpapi/tests/iphlpapi.c b/dlls/iphlpapi/tests/iphlpapi.c index af893aeae54..92d9e0cc87a 100644 --- a/dlls/iphlpapi/tests/iphlpapi.c +++ b/dlls/iphlpapi/tests/iphlpapi.c @@ -368,7 +368,9 @@ static void testGetIpForwardTable(void) static void testGetIpNetTable(void) { - DWORD apiReturn; + const DWORD static_addr1 = 0x160000e0, static_addr2 = 0xfaffffef; + BOOL addr1_found, addr2_found; + DWORD apiReturn, prev_idx; ULONG dwSize = 0; unsigned int i; @@ -405,6 +407,31 @@ static void testGetIpNetTable(void) ok(ntohl(buf->table[i].dwAddr) <= ntohl(buf->table[i + 1].dwAddr), "Entries are not sorted by address, i %u.\n", i ); } + + addr1_found = addr2_found = FALSE; + prev_idx = ~0ul; + for (i = 0; i < buf->dwNumEntries; ++i) + { + if (buf->table[i].dwIndex != prev_idx) + { + if (prev_idx != ~0ul) + { + ok( addr1_found, "%s not found, iface index %lu.\n", ntoa( static_addr1 ), prev_idx); + ok( addr2_found || broken(!addr2_found) /* 239.255.255.250 is always present since Win10 */, + "%s not found.\n", ntoa( static_addr2 )); + } + prev_idx = buf->table[i].dwIndex; + addr1_found = addr2_found = FALSE; + } + if (buf->table[i].dwAddr == static_addr1) + addr1_found = TRUE; + else if (buf->table[i].dwAddr == static_addr2) + addr2_found = TRUE; + } + ok( addr1_found, "%s not found.\n", ntoa( static_addr1 )); + ok( addr2_found || broken(!addr2_found) /* 239.255.255.250 is always present since Win10 */, + "%s not found.\n", ntoa( static_addr2 )); + } if (apiReturn == NO_ERROR && winetest_debug > 1) diff --git a/dlls/nsi/tests/nsi.c b/dlls/nsi/tests/nsi.c index b6dd8f2416d..9ffdf9fba8d 100644 --- a/dlls/nsi/tests/nsi.c +++ b/dlls/nsi/tests/nsi.c @@ -663,6 +663,12 @@ static void test_ip_neighbour( int family ) ok( key4->luid.Value == row->InterfaceLuid.Value, "%s vs %s\n", wine_dbgstr_longlong( key4->luid.Value ), wine_dbgstr_longlong( row->InterfaceLuid.Value ) ); ok( key4->luid2.Value == row->InterfaceLuid.Value, "mismatch\n" ); + if (key4->addr.s_addr == 0x160000e0 || key4->addr.s_addr == 0xfaffffef) + { + ok( dyn->state == NlnsPermanent, "got state %d.\n", dyn->state ); + ok( !dyn->flags.is_router, "is_router is set.\n" ); + ok( !dyn->flags.is_unreachable, "is_unreachable is set.\n" ); + } } else if (family == AF_INET6) { diff --git a/dlls/nsiproxy.sys/ip.c b/dlls/nsiproxy.sys/ip.c index 79f3bd80bfe..59d9143e908 100644 --- a/dlls/nsiproxy.sys/ip.c +++ b/dlls/nsiproxy.sys/ip.c @@ -59,6 +59,10 @@ #include <netinet6/ip6_var.h> #endif +#ifdef HAVE_NET_IF_H +#include <net/if.h> +#endif + #ifdef __APPLE__ /* For reasons unknown, Mac OS doesn't export <netinet6/ip6_var.h> to user- * space. We'll have to define the needed struct ourselves. @@ -1004,6 +1008,7 @@ static NTSTATUS ipv4_neighbour_enumerate_all( void *key_data, UINT key_size, voi void *dynamic_data, UINT dynamic_size, void *static_data, UINT static_size, UINT_PTR *count ) { + struct if_nameindex *iface_indices, *iface; UINT num = 0; NTSTATUS status = STATUS_SUCCESS; BOOL want_data = key_size || rw_size || dynamic_size || static_size; @@ -1139,6 +1144,41 @@ static NTSTATUS ipv4_neighbour_enumerate_all( void *key_data, UINT key_size, voi return STATUS_NOT_IMPLEMENTED; #endif + if (!want_data || num <= *count) + { + iface_indices = if_nameindex(); + for (iface = iface_indices; iface->if_index; iface++) + { + if (num <= *count) + { + memset( &entry, 0, sizeof(entry) ); + entry.if_index = iface->if_index; + convert_index_to_luid( entry.if_index, &entry.luid ); + entry.state = NlnsPermanent; + entry.addr.s_addr = inet_addr( "224.0.0.22" ); + ipv4_neighbour_fill_entry( &entry, key_data, rw_data, dynamic_data, static_data ); + + if (key_data) key_data = (BYTE *)key_data + key_size; + if (rw_data) rw_data = (BYTE *)rw_data + rw_size; + if (dynamic_data) dynamic_data = (BYTE *)dynamic_data + dynamic_size; + if (static_data) static_data = (BYTE *)static_data + static_size; + } + num++; + if (num <= *count) + { + entry.addr.s_addr = inet_addr( "239.255.255.250" ); + ipv4_neighbour_fill_entry( &entry, key_data, rw_data, dynamic_data, static_data ); + + if (key_data) key_data = (BYTE *)key_data + key_size; + if (rw_data) rw_data = (BYTE *)rw_data + rw_size; + if (dynamic_data) dynamic_data = (BYTE *)dynamic_data + dynamic_size; + if (static_data) static_data = (BYTE *)static_data + static_size; + } + num++; + } + if_freenameindex( iface_indices ); + } + if (!want_data || num <= *count) *count = num; else status = STATUS_BUFFER_OVERFLOW; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/286
Huw Davies (@huw) commented about dlls/iphlpapi/iphlpapi_main.c:
static int ipnetrow_cmp( const void *a, const void *b ) { const MIB_IPNETROW *rowA = a, *rowB = b; + + if (rowA->dwIndex != rowB->dwIndex) return rowA->dwIndex < rowB->dwIndex ? -1 : 1; + Please use `DWORD_cmp()` here.
Could you push just the first three patches to this MR - the fourth will require a bit more thought so please create a new MR for that. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/286#note_2430
Huw Davies (@huw) commented about dlls/nsiproxy.sys/ip.c:
Commit msg typo: s/ndisproxy/nsiproxy/ -- https://gitlab.winehq.org/wine/wine/-/merge_requests/286#note_2429
participants (5)
-
Huw Davies (＠huw) -
Marvin -
Paul Gofman -
Paul Gofman
-
Paul Gofman (＠gofman)