Skip to content

Commit 4466dad

Browse files
lufenCopilot
andauthored
Fix Ping RoundtripTime returning 0 for TtlExpired replies (#124424)
When a Ping results in TtlExpired or TimeExceeded, the underlying ICMP_ECHO_REPLY struct contains a valid round-trip time from the intermediate router. Previously this was discarded and hardcoded to 0 for all non-Success statuses. This change reads the RTT from the reply for TtlExpired and TimeExceeded statuses in both IPv4 and IPv6 Windows code paths, matching the behavior of the raw socket implementation on Unix. Fixes #118150 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent f5a27eb commit 4466dad

File tree

2 files changed

+62
-6
lines changed

2 files changed

+62
-6
lines changed

src/libraries/System.Net.Ping/src/System/Net/NetworkInformation/Ping.Windows.cs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -344,21 +344,21 @@ private static PingReply CreatePingReplyFromIcmpEchoReply(in Interop.IpHlpApi.IC
344344
IPAddress address = new IPAddress(reply.address);
345345
IPStatus ipStatus = GetStatusFromCode((int)reply.status);
346346

347-
long rtt;
347+
// The ICMP_ECHO_REPLY RoundTripTime field is always populated by the OS
348+
// for any received reply, regardless of status (e.g. TTL expired, unreachable).
349+
long rtt = reply.roundTripTime;
348350
PingOptions? options;
349351
byte[] buffer;
350352

351353
if (ipStatus == IPStatus.Success)
352354
{
353355
// Only copy the data if we succeed w/ the ping operation.
354-
rtt = reply.roundTripTime;
355356
options = new PingOptions(reply.options.ttl, (reply.options.flags & DontFragmentFlag) > 0);
356357
buffer = new byte[reply.dataSize];
357358
Marshal.Copy(reply.data, buffer, 0, reply.dataSize);
358359
}
359360
else
360361
{
361-
rtt = 0;
362362
options = null;
363363
buffer = Array.Empty<byte>();
364364
}
@@ -371,19 +371,19 @@ private static PingReply CreatePingReplyFromIcmp6EchoReply(in Interop.IpHlpApi.I
371371
IPAddress address = new IPAddress(reply.Address.Address, reply.Address.ScopeID);
372372
IPStatus ipStatus = GetStatusFromCode((int)reply.Status);
373373

374-
long rtt;
374+
// The ICMPV6_ECHO_REPLY RoundTripTime field is always populated by the OS
375+
// for any received reply, regardless of status (e.g. TTL expired, unreachable).
376+
long rtt = reply.RoundTripTime;
375377
byte[] buffer;
376378

377379
if (ipStatus == IPStatus.Success)
378380
{
379381
// Only copy the data if we succeed w/ the ping operation.
380-
rtt = reply.RoundTripTime;
381382
buffer = new byte[sendSize];
382383
Marshal.Copy(dataPtr + 36, buffer, 0, sendSize);
383384
}
384385
else
385386
{
386-
rtt = 0;
387387
buffer = Array.Empty<byte>();
388388
}
389389

src/libraries/System.Net.Ping/tests/FunctionalTests/PingTest.cs

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -787,6 +787,62 @@ public async Task SendPingToExternalHostWithLowTtlTest()
787787
Assert.NotEqual(IPAddress.Any, pingReply.Address);
788788
}
789789

790+
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsMultithreadingSupported))]
791+
[OuterLoop] // Depends on external host
792+
public async Task SendPingWithLowTtl_RoundtripTimeIsNonZero()
793+
{
794+
// Regression test: non-Success replies (e.g. TtlExpired) should preserve the
795+
// round-trip time from the ICMP reply, not hardcode it to 0.
796+
if (UsesPingUtility)
797+
{
798+
throw new SkipTestException("Test is only applicable to the IcmpSendEcho code path.");
799+
}
800+
801+
string host = System.Net.Test.Common.Configuration.Ping.PingHost;
802+
PingOptions options = new PingOptions();
803+
byte[] payload = TestSettings.PayloadAsBytesShort;
804+
805+
using Ping ping = new Ping();
806+
807+
// Verify host is reachable first.
808+
bool reachable = false;
809+
for (int i = 0; i < s_pingcount; i++)
810+
{
811+
PingReply checkReply = await ping.SendPingAsync(host, TestSettings.PingTimeout, payload);
812+
if (checkReply.Status == IPStatus.Success)
813+
{
814+
reachable = true;
815+
break;
816+
}
817+
}
818+
if (!reachable)
819+
{
820+
throw new SkipTestException($"Host {host} is not reachable. Skipping test.");
821+
}
822+
823+
// RTT can legitimately be 0ms on very fast networks, so retry a few times
824+
// and assert that at least one reply reports a non-zero RTT.
825+
options.Ttl = 1;
826+
bool gotNonZeroRtt = false;
827+
for (int attempt = 0; attempt < 3; attempt++)
828+
{
829+
PingReply pingReply = await ping.SendPingAsync(host, TestSettings.PingTimeout, payload, options);
830+
831+
Assert.True(
832+
pingReply.Status == IPStatus.TimeExceeded || pingReply.Status == IPStatus.TtlExpired,
833+
$"pingReply.Status was {pingReply.Status} instead of TimeExceeded or TtlExpired");
834+
835+
if (pingReply.RoundtripTime > 0)
836+
{
837+
gotNonZeroRtt = true;
838+
break;
839+
}
840+
}
841+
842+
Assert.True(gotNonZeroRtt,
843+
"Expected at least one TtlExpired reply with non-zero RoundtripTime across 3 attempts");
844+
}
845+
790846
private async Task Ping_TimedOut_Core(Func<Ping, string, Task<PingReply>> sendPing)
791847
{
792848
Ping sender = new Ping();

0 commit comments

Comments
 (0)