An interval timer (itimer) allows a process to do repeated work at a given frequency. If an -i or -f option is specified, the FreeBSD ping6(8) will use this value to call setitimer(2) to register the timer, then the signal handler is called at every expiry of the timer, sending an ICMPv6 packet each time. However, the KAME version of ping6 utility immediately exits the loop after knowing that it is the last packet to send, which causes unread replies from a remote host to be ignored. This is a bug and can be fixed by setting itimer to zero as soon as the last packet is sent, then continue the loop to see if any replies are received. Fix: Apply the attached patch to src/sbin/ping6/ping6.c and the problem will be fixed. Tested against CVS revision 1.39 but this is supposed to work on older versions as well. Patch attached with submission follows: How-To-Repeat: Suppose there is an average RTT seconds round-trip delay between v6host1 and v6host2. INT is the interval (seconds) between packets. CNT is the total number of packets to send. Run as root on v6host1: v6host1# ping6 -i $INT -c $CNT -q v6host2 Then the reply might look like this (0.01 interval, 245.816 ms delay, 100 packets): --- ipv6.sjtu.edu.cn ping6 statistics --- 100 packets transmitted, 76 packets received, 24.0% packet loss round-trip min/avg/max/std-dev = 244.446/245.816/247.826/0.705 ms After re-adjusting $INT and switching to another destination host with a different RTT, one would simply conclude with the following formula: estimated-packets-lost = RTT / INT
MFC reminder.
A commit references this bug: Author: delphij Date: Mon Jul 28 08:22:08 UTC 2014 New revision: 269180 URL: http://svnweb.freebsd.org/changeset/base/269180 Log: When interval is set to very small value with limited amount of packets, ping6(8) would quit before the remote side gets a chance to respond. Solve this by resetting the itimer when we have reached the maximum packet number have reached, but let the other handling to continue. PR: bin/151023 Submitted by: tjmao at tjmao.net MFC after: 2 weeks Changes: head/sbin/ping6/ping6.c
A commit references this bug: Author: delphij Date: Mon Aug 11 06:54:07 UTC 2014 New revision: 269800 URL: http://svnweb.freebsd.org/changeset/base/269800 Log: MFC r269180: When interval is set to very small value with limited amount of packets, ping6(8) would quit before the remote side gets a chance to respond. Solve this by resetting the itimer when we have reached the maximum packet number have reached, but let the other handling to continue. PR: bin/151023 Submitted by: tjmao at tjmao.net Changes: _U stable/10/ stable/10/sbin/ping6/ping6.c
A commit references this bug: Author: delphij Date: Mon Aug 11 07:00:57 UTC 2014 New revision: 269802 URL: http://svnweb.freebsd.org/changeset/base/269802 Log: MFC r269180: When interval is set to very small value with limited amount of packets, ping6(8) would quit before the remote side gets a chance to respond. Solve this by resetting the itimer when we have reached the maximum packet number have reached, but let the other handling to continue. PR: bin/151023 Submitted by: tjmao at tjmao.net Changes: _U stable/9/sbin/ping6/ stable/9/sbin/ping6/ping6.c
A commit references this bug: Author: delphij Date: Mon Aug 11 07:01:29 UTC 2014 New revision: 269803 URL: http://svnweb.freebsd.org/changeset/base/269803 Log: MFC r269180: When interval is set to very small value with limited amount of packets, ping6(8) would quit before the remote side gets a chance to respond. Solve this by resetting the itimer when we have reached the maximum packet number have reached, but let the other handling to continue. PR: bin/151023 Submitted by: tjmao at tjmao.net Changes: _U stable/8/sbin/ping6/ stable/8/sbin/ping6/ping6.c
Mark as resolved. I don't have much time to work on this further right now, Bruce Evans have some recommendations that would be great to get addressed: http://lists.freebsd.org/pipermail/svn-src-head/2014-July/060928.html
The committed patch in r269180 is wrong because ping6 can enter an infinite loop when -c is specified and the last packet is not received. I will revert the change for now.
A commit references this bug: Author: hrs Date: Sat Sep 20 18:48:51 UTC 2014 New revision: 271909 URL: http://svnweb.freebsd.org/changeset/base/271909 Log: Revert changes in r269180. It could cause -c N option to enter an infinite loop if no reply packet is received. PR: 151023 Changes: head/sbin/ping6/ping6.c
A commit references this bug: Author: hrs Date: Sat Sep 20 19:54:19 UTC 2014 New revision: 271910 URL: http://svnweb.freebsd.org/changeset/base/271910 Log: Fix a problem that reply packets are not received when -i T option is set and (T < RTT). - Use select(2) for timeout instead of interval timer. Remove poll(2) support. - Use sigaction(2) instead of signal(3). - Exit in SIGINT handler when two signals are received and doing reverse DNS lookup as ping(8) does. - Remove redundant variables used for getaddrinfo(3). PR: 151023 Changes: head/sbin/ping6/Makefile head/sbin/ping6/ping6.c
I committed a fix in r271910. Please test it and let me know if the problem still persists or not.
r271910 does not hang when no reply packets are received, but it does wait a lot longer then the old pre r269180 code did, new code first: root@zibbi:/home/jhay/ping6 # /usr/bin/time ./ping6 -c 1 2001:4200:7000:1::90 PING6(56=40+8+8 bytes) 2001:4200:7000:1:d6ae:52ff:fed1:9981 --> 2001:4200:7000:1::90 --- 2001:4200:7000:1::90 ping6 statistics --- 1 packets transmitted, 0 packets received, 100.0% packet loss 11.05 real 0.00 user 0.00 sys root@zibbi:/home/jhay/ping6 # /usr/bin/time /sbin/ping6 -c 1 2001:4200:7000:1::90 PING6(56=40+8+8 bytes) 2001:4200:7000:1:d6ae:52ff:fed1:9981 --> 2001:4200:7000:1::90 --- 2001:4200:7000:1::90 ping6 statistics --- 1 packets transmitted, 0 packets received, 100.0% packet loss 1.00 real 0.00 user 0.00 sys root@zibbi:/home/jhay/ping6 # The new code waits 11 seconds for a ping6 -c 1, while the old code only waited 1 second. Adding -w 1 makes it even shorter: root@zibbi:/home/jhay/ping6 # /usr/bin/time ./ping6 -c 1 -w 1 2001:4200:7000:1::90 ping6: hostname nor servname provided, or not known 0.01 real 0.00 user 0.01 sys root@zibbi:/home/jhay/ping6 # root@zibbi:/home/jhay/ping6 # fgrep '$FreeBSD:' ping6.c __FBSDID("$FreeBSD: head/sbin/ping6/ping6.c 271910 2014-09-20 19:54:19Z hrs $"); root@zibbi:/home/jhay/ping6 #
(In reply to John Hay from comment #11) > r271910 does not hang when no reply packets are received, but it does wait a > lot longer then the old pre r269180 code did, new code first: The original code is intended to wait for 10 seconds if no reply packet is received. It waits for 2 * RTT if at least one reply is received. It is compatible with ping(8); you can check it by "ping -c N". The old behavior of ping6(8) did not work as intended due to a bug. > The new code waits 11 seconds for a ping6 -c 1, while the old code only > waited > 1 second. Adding -w 1 makes it even shorter: In ping(8), 10 seconds wait can be shorten by using waittime (-w) option. However, -w of ping6(8) is not an option for waittime. Actually ping6(8) does not have such an option. Is it better to implement an waittime option in ping6(8)?
Created attachment 147528 [details] A patch to add timeout and waittime options which ping(8) supports. This is a patch to implement timeout and waittime options. In ping(8) they are "-t timeout" and "-W waittime", but in ping6(8) both are already used for other purposes. This patch uses "-x waittime" and "-X timeout".
I have tried your patch and -X is working like I expect: root@zibbi:/home/jhay/ping6 # /usr/bin/time ./ping6 -c 1 -X 2 2001:4200:7000:1::90 PING6(56=40+8+8 bytes) 2001:4200:7000:1:d6ae:52ff:fed1:9981 --> 2001:4200:7000:1::90 --- 2001:4200:7000:1::90 ping6 statistics --- 1 packets transmitted, 0 packets received, 100.0% packet loss 2.02 real 0.00 user 0.00 sys root@zibbi:/home/jhay/ping6 # But -x seems to add 1 second to the -x millisecond value or do I misunderstand it? root@zibbi:/home/jhay/ping6 # /usr/bin/time ./ping6 -c 1 -x 500 2001:4200:7000:1::90 PING6(56=40+8+8 bytes) 2001:4200:7000:1:d6ae:52ff:fed1:9981 --> 2001:4200:7000:1::90 --- 2001:4200:7000:1::90 ping6 statistics --- 1 packets transmitted, 0 packets received, 100.0% packet loss 1.53 real 0.00 user 0.00 sys root@zibbi:/home/jhay/ping6 # /usr/bin/time ./ping6 -c 1 -x 1500 2001:4200:7000:1::90 PING6(56=40+8+8 bytes) 2001:4200:7000:1:d6ae:52ff:fed1:9981 --> 2001:4200:7000:1::90 --- 2001:4200:7000:1::90 ping6 statistics --- 1 packets transmitted, 0 packets received, 100.0% packet loss 2.54 real 0.00 user 0.00 sys root@zibbi:/home/jhay/ping6 #
(In reply to John Hay from comment #14) > But -x seems to add 1 second to the -x millisecond value or do I > misunderstand it? Correct. ping(8) uses second for -i and millisecond for -W, so I used the same units for -x and -X.
Hi, Note that r269180 still not reverted in stable/* branches.
To originators/assignees of this PR: A commit to the tree references this PR, however the PR is still in a non-closed state. Please review this PR and close as appropriate, or if closing the PR requires a merge to stable/10, please let re@ know as soon as possible. Thank you. Glen
(In reply to Andrey V. Elsukov from comment #16) Yes, it's reverted on stable/10, but not stable/8 and stable/9 :/..: ------------------------------------------------------------------------ r272871 | hrs | 2014-10-09 16:48:20 -0700 (Thu, 09 Oct 2014) | 4 lines MFC r271909: Revert changes in r269180. It could cause -c N option to enter an infinite loop if no reply packet is received.
batch change of PRs untouched in 2018 marked "in progress" back to open.
Can this PR be closed? If not, what work remains? It seems the proposed patches have landed in the following commits: head base r273211, base r271910 stable/11 base r273211, base r271910 stable/10 base r285820 Regarding comment #18 and comment #16, above: the broken patch that was committed in base r269180 has been reverted in the following commits. head base r271909 stable/11 base r271909 stable/10 base r272871 stable/9 base r298843 As far as I can tell, stable/8 was the only stable branch where the broken patch was merged (in base r269803), but never reverted.