ping: use CLOCK_REALTIME for ICMP Originate Timestamp
ClosedPublic
Actions

Authored by maxim on Fri, May 1, 5:57 PM.

Details

Reviewers

asomers
glebius

Commits

rG069681afd58a: ping: use CLOCK_REALTIME for ICMP Originate Timestamp

Summary

ping: use CLOCK_REALTIME for ICMP Originate Timestamp

RFC 792 defines the ICMP Originate Timestamp field as milliseconds
since midnight UTC.  However, ping(8) currently derives this value
from CLOCK_MONOTONIC, which represents time since an unspecified
starting point and is not related to UTC.

The issue was introduced by commit 1ad76f1b6047, which replaced
gettimeofday(2) with clock_gettime(CLOCK_MONOTONIC) for timekeeping
in ping(8).

Fix this by using CLOCK_REALTIME when generating the ICMP originate
timestamp.

Before:

$ ping -Mt -c1 127.0.0.1
ICMP_TSTAMP
PING 127.0.0.1 (127.0.0.1): 56 data bytes
<...> time=0.061 ms tso=16:50:31 tsr=17:38:28 tst=17:38:28

(note the tso is off)

After:

$ ping -Mt -c1 127.0.0.1
ICMP_TSTAMP
PING 127.0.0.1 (127.0.0.1): 56 data bytes
<...> time=0.038 ms tso=17:42:09 tsr=17:42:09 tst=17:42:09

Reviewed by:            XXX
Fixes:                  1ad76f1b6047
MFC after:              1 month
Differential Revision:  https://reviews.freebsd.org/D###

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

maxim requested review of this revision.Fri, May 1, 5:57 PM

maxim created this revision.

maxim added reviewers: asomers, glebius.Fri, May 1, 6:00 PM

Thanks for catching this. Would it be possible to add an ATF test to catch the problem?

glebius accepted this revision.Fri, May 1, 9:43 PM

This revision is now accepted and ready to land.Fri, May 1, 9:43 PM

In D56759#1300158, @asomers wrote:

Thanks for catching this. Would it be possible to add an ATF test to catch the problem?

Yeah, I was thinking about that. Let me dig into them.

kyua test ping_test

ping_test:inject_opts -> passed [2.470s]
ping_test:inject_pip -> passed [3.431s]
ping_test:inject_reply -> passed [2.411s]
ping_test:ping4_nohost -> passed [0.029s]
ping_test:ping6_4 -> passed [0.046s]
ping_test:ping6_c1_s8_t1 -> passed [0.080s]
ping_test:ping6_c1t4 -> passed [0.040s]
ping_test:ping6_nohost -> passed [0.034s]
ping_test:ping_46 -> passed [0.043s]
ping_test:ping_64 -> passed [0.040s]
ping_test:ping_6_c1_s8_t1 -> passed [0.056s]
ping_test:ping_c1_s56_t1 -> passed [0.053s]
ping_test:ping_c1_s56_t1_S127 -> passed [0.050s]
ping_test:ping_c1_s8_t1_S1 -> passed [0.047s]
ping_test:ping_c1t6 -> passed [0.043s]
ping_test:ping_nohost -> passed [0.024s]
ping_test:timestamp_origin -> passed [0.039s]

Results file id is usr_tests_sbin_ping.20260501-235651-527128
Results saved to /root/.kyua/store/results.usr_tests_sbin_ping.20260501-235651-527128.db

17/17 passed (0 broken, 0 failed, 0 skipped)

This revision now requires review to proceed.Fri, May 1, 11:57 PM

asomers added inline comments.Sat, May 2, 2:19 AM

sbin/ping/tests/ping_test.sh
274–282	How about this to be a little bit simpler? tso_s=`date -jf %H:%M:%S $tso` tsr_s=`date -jf %H:%M:%S $tsr`

maxim updated this revision to Diff 177033.Sat, May 2, 3:02 AM

Thanks for fixing this. You should ask re@ if they want it in 15.1.

This revision is now accepted and ready to land.Sat, May 2, 3:06 AM

maxim added inline comments.Sat, May 2, 3:11 AM

sbin/ping/tests/ping_test.sh
274–282	Oh yeah, I completely forgot about this feature of our date(1).

In D56759#1300316, @asomers wrote:

Thanks for fixing this. You should ask re@ if they want it in 15.1.

I think this is a quite minor issue and doubt that a lot of people know about and use icmp timestamp these days. That was pretty much a fun exercise.

Thank you for the review, Alan!

Closed by commit rG069681afd58a: ping: use CLOCK_REALTIME for ICMP Originate Timestamp (authored by maxim). · Explain WhySat, May 2, 3:18 AM

This revision was automatically updated to reflect the committed changes.

maxim added a commit: rG069681afd58a: ping: use CLOCK_REALTIME for ICMP Originate Timestamp.

Herald added a subscriber: imp. · View Herald TranscriptSat, May 2, 3:18 AM

markj added a subscriber: markj.Sun, May 3, 6:30 PM

markj added inline comments.

sbin/ping/tests/ping_test.sh
274–282	This doesn't work: $ ping -Mt -c1 127.0.0.1 \| sed -n 's/.tsr=$[0-9:]$.*/\1/p' 18:29:42 $ date -jf %H:%M:%S 18:29:42 Sun May 3 18:29:42 UTC 2026 So `tso_s` and `tsr_s` are not integers and can't be subtracted, and the test is failing. Could you please take a look?