[PATCH 0/1] MR4853: iphlpapi: Implement GetRTTAndHopCount().

[Marc-Aurel Zent (＠mzent)]

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

[PATCH 1/1] iphlpapi: Implement GetRTTAndHopCount().

[Marc-Aurel Zent]

From: Marc-Aurel Zent <mzent(a)codeweavers.com>

---
 dlls/iphlpapi/iphlpapi_main.c | 52 +++++++++++++++++++++++++++++------
 1 file changed, 44 insertions(+), 8 deletions(-)

diff --git a/dlls/iphlpapi/iphlpapi_main.c b/dlls/iphlpapi/iphlpapi_main.c
index 9c7582b71fb..1ee4942b57c 100644
--- a/dlls/iphlpapi/iphlpapi_main.c
+++ b/dlls/iphlpapi/iphlpapi_main.c
@@ -2781,15 +2781,51 @@ DWORD WINAPI GetPerAdapterInfo( ULONG index, IP_PER_ADAPTER_INFO *info, ULONG *s
  * RETURNS
  *  Success: TRUE
  *  Failure: FALSE
- *
- * FIXME
- *  Stub, returns FALSE.
  */
-BOOL WINAPI GetRTTAndHopCount(IPAddr DestIpAddress, PULONG HopCount, ULONG MaxHops, PULONG RTT)
-{
-  FIXME("(DestIpAddress 0x%08lx, HopCount %p, MaxHops %ld, RTT %p): stub\n",
-   DestIpAddress, HopCount, MaxHops, RTT);
-  return FALSE;
+BOOL WINAPI GetRTTAndHopCount( IPAddr DestIpAddress, PULONG HopCount, ULONG MaxHops, PULONG RTT )
+{    
+    char send_buffer[0x20] = { 0xDE, 0xAD, 0xBE, 0xEF };
+    char receive_buffer[sizeof(ICMP_ECHO_REPLY) + sizeof(send_buffer)];
+    const DWORD timeout = 5000;
+    DWORD replies;
+    IP_OPTION_INFORMATION send_options = { 0 };
+    PICMP_ECHO_REPLY reply;
+    HANDLE icmp_handle;
+
+    TRACE( "(DestIpAddress 0x%08lx, HopCount %p, MaxHops %ld, RTT %p)\n",
+        DestIpAddress, HopCount, MaxHops, RTT );
+
+    if (!HopCount || !RTT || DestIpAddress == INADDR_NONE)
+        return FALSE;
+
+    if ((icmp_handle = IcmpCreateFile()) == INVALID_HANDLE_VALUE)
+        return FALSE;
+
+    for (send_options.Ttl = 1; send_options.Ttl <= MaxHops; send_options.Ttl++) 
+    {
+        replies = IcmpSendEcho( icmp_handle, DestIpAddress, send_buffer, sizeof(send_buffer),
+                                &send_options, receive_buffer, sizeof(receive_buffer), timeout );
+
+        if (!replies) 
+        {
+            if (GetLastError() == IP_TTL_EXPIRED_TRANSIT) continue;
+            if (GetLastError() == IP_REQ_TIMED_OUT) continue;
+            break;
+        }
+
+        reply = (PICMP_ECHO_REPLY)receive_buffer;
+
+        if (reply->Status == IP_SUCCESS) 
+        {
+            *HopCount = send_options.Ttl;
+            *RTT = reply->RoundTripTime;
+            IcmpCloseHandle( icmp_handle );
+            return TRUE;
+        }
+    }
+
+    IcmpCloseHandle( icmp_handle );
+    return FALSE;
 }
 
 /******************************************************************
-- 
GitLab

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

[Marc-Aurel Zent (＠mzent)]

Did a bit more digging and the 32 byte payload on windows seems to be 

```
    0x61, 0x62, 0x63, 0x64, 0x65, 0x66, 0x67, 0x68, 0x69, 0x6a, 0x6b, 0x6c, 0x6d, 0x6e, 0x6f, 0x70,
    0x71, 0x72, 0x73, 0x74, 0x75, 0x76, 0x77, 0x61, 0x62, 0x63, 0x64, 0x65, 0x66, 0x67, 0x68, 0x69
```

On when a timeout doesn't happen, it also seems to send a random amount of ICMP packets per TTL step; not too sure what the mechanism is there exactly (and if any of this needs to be replicated, but am a bit hesitant a zero-length or zero-filled send buffer might produce inaccurate results with some routers/firewalls).

In any case I get similar-ish values of RTT and HopCount, and the time this function blocks on a timeout to what Windows returns on the same network with the current implementation.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4853#note_57870

[PATCH 0/1] MR4853: iphlpapi: Implement GetRTTAndHopCount(). - approved

[Huw Davies (＠huw)]

This merge request was approved by Huw Davies.
-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4853

[Huw Davies (＠huw)]

Huw Davies (@huw) commented about dlls/iphlpapi/iphlpapi_main.c:
> * RETURNS
>   *  Success: TRUE
>   *  Failure: FALSE
> - *
> - * FIXME
> - *  Stub, returns FALSE.
>   */
> -BOOL WINAPI GetRTTAndHopCount(IPAddr DestIpAddress, PULONG HopCount, ULONG MaxHops, PULONG RTT)
> -{
> -  FIXME("(DestIpAddress 0x%08lx, HopCount %p, MaxHops %ld, RTT %p): stub\n",
> -   DestIpAddress, HopCount, MaxHops, RTT);
> -  return FALSE;
> +BOOL WINAPI GetRTTAndHopCount( IPAddr DestIpAddress, PULONG HopCount, ULONG MaxHops, PULONG RTT )
> +{    
> Could we change the parameter names to be snake_case - e.g. `dest_addr`, `hop_count`? Also `PULONG` -> `ULONG *`.

There is also trailing whitespace after the opening brace.  There are other instances of trailing whitespace below.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4853#note_60977

[Huw Davies (＠huw)]

Huw Davies (@huw) commented about dlls/iphlpapi/iphlpapi_main.c:
> - * FIXME
> - *  Stub, returns FALSE.
>   */
> -BOOL WINAPI GetRTTAndHopCount(IPAddr DestIpAddress, PULONG HopCount, ULONG MaxHops, PULONG RTT)
> -{
> -  FIXME("(DestIpAddress 0x%08lx, HopCount %p, MaxHops %ld, RTT %p): stub\n",
> -   DestIpAddress, HopCount, MaxHops, RTT);
> -  return FALSE;
> +BOOL WINAPI GetRTTAndHopCount( IPAddr DestIpAddress, PULONG HopCount, ULONG MaxHops, PULONG RTT )
> +{    
> +    char send_buffer[0x20] = { 0xDE, 0xAD, 0xBE, 0xEF };
> +    char receive_buffer[sizeof(ICMP_ECHO_REPLY) + sizeof(send_buffer)];
> +    const DWORD timeout = 5000;
> +    DWORD replies;
> +    IP_OPTION_INFORMATION send_options = { 0 };
> +    PICMP_ECHO_REPLY reply;
> `PICMP_ECHO_REPLY` -> `ICMP_ECHO_REPLY *`.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/4853#note_60978