[PATCH 0/1] MR296: nsiproxy.sys: Add static ARP entries which are always present on Windows.

[Paul Gofman (＠gofman)]

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=53175

Split off from MR https://gitlab.winehq.org/wine/wine/-/merge_requests/286.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/296

[PATCH 1/1] nsiproxy.sys: Add static ARP entries which are always present on Windows.

[Paul Gofman]

From: Paul Gofman <pgofman(a)codeweavers.com>

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=53175
Signed-off-by: Paul Gofman <pgofman(a)codeweavers.com>
---
 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 84d5143ba8e..6645326d845 100644
--- a/dlls/iphlpapi/tests/iphlpapi.c
+++ b/dlls/iphlpapi/tests/iphlpapi.c
@@ -369,7 +369,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;
 
@@ -406,6 +408,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/296

[Huw Davies (＠huw)]

Huw Davies (@huw) commented about dlls/iphlpapi/tests/iphlpapi.c:
> static void testGetIpNetTable(void)
>  {
> -    DWORD apiReturn;
> +    const DWORD static_addr1 = 0x160000e0, static_addr2 = 0xfaffffef;

Could you use a macro or something to generate these from decimal?  Also please use more meaningful variable names based on the addresses' uses (see https://en.wikipedia.org/wiki/Multicast_address#Notable_IPv4_multicast_addre...)

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/296#note_2523

[Huw Davies (＠huw)]

Huw Davies (@huw) commented about dlls/nsiproxy.sys/ip.c:
> return STATUS_NOT_IMPLEMENTED;
>  #endif
>
> +    if (!want_data || num <= *count)
> +    {
> +        iface_indices = if_nameindex();

Rather than using `if_nametoindex()` please use `nsi_enumerate_all()` to get the `NSI_NDIS_IFINFO_TABLE` table.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/296#note_2526

[Huw Davies (＠huw)]

Huw Davies (@huw) commented about dlls/nsiproxy.sys/ip.c:
> +                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++;
> Adding the second entry looks a lot like the first.  It should be possible to write this as a loop.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/296#note_2528

[Huw Davies (＠huw)]

Huw Davies (@huw) commented about dlls/nsi/tests/nsi.c:
> 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" );
> +            }

Please test these in the iphlpapi tests - perhaps by calling `GetIpNetTable2()`.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/296#note_2525

[Huw Davies (＠huw)]

Huw Davies (@huw) commented about dlls/nsiproxy.sys/ip.c:
> 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" );

Let's not use `inet_addr()`.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/296#note_2527

[Huw Davies (＠huw)]

Huw Davies (@huw) commented about dlls/iphlpapi/tests/iphlpapi.c:
>

Please add more of an explanation to the commit message, not just a bug url.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/296#note_2524

[Huw Davies (＠huw)]

Huw Davies (@huw) commented about dlls/nsiproxy.sys/ip.c:
> +            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;
> macOS already has `239.255.255.250` on interfaces that up, so this ends up duplicating that entry.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/296#note_2529

[Huw Davies (＠huw)]

On Thu Jun 23 06:36:08 2022 +0000, Huw Davies wrote:
> Rather than using `if_nametoindex()` please use `nsi_enumerate_all()` to
> get the `NSI_NDIS_IFINFO_TABLE` table.
> Also, let's add a comment above the start of this block explaining what we're doing.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/296#note_2530