ping: Avoid reporting NaNs

[jlduran]

Summary

Avoid calculating the square root of negative zero, which can easily
happen on certain architectures when computing the population standard
deviation with a sample size of one, e.g.,
0.01 - (0.1 * 0.1) = -0.000000.

Avoid returning a NaN by capping the minimum possible variance value to
zero (positive).

In the future, maybe skip reporting statistics at all for a single
sample.

Reported by:    Jenkins

Test Plan

Some Jenkins tests are failing:

• https://ci.freebsd.org/job/FreeBSD-main-aarch64-test/1096/testReport/junit/sbin.ping/ping_test/ping6_c1_s8_t1/
• https://ci.freebsd.org/job/FreeBSD-main-aarch64-test/1096/testReport/junit/sbin.ping/ping_test/ping_c1_s56_t1_S127/
• https://ci.freebsd.org/job/FreeBSD-main-aarch64-test/1096/testReport/junit/sbin.ping/ping_test/ping_c1_s56_t1/

This failures are not present on amd64. In order to test it, aarch64 or powerpc64 must be used.

[asomers]

If we were computing with real numbers, then it should be impossible for vari to ever be negative. And in fact, all of the failing tests use -t 1, which means that n will be 1, so vari should always be exactly zero. So the fact that you're seeing these errors must mean that we're hitting some kind of floating point round-off error. Your patch will work, but it begs the question: why print min/avg/max/stddev when there is only one datapoint? I think that we should only print those things when n > 1. In fact, the standard deviation isn't really meaningful when n == 2, but it probably wouldn't hurt to print it, if that makes the code more readable.

[jlduran]

In D42114#960670, @asomers wrote:

If we were computing with real numbers, then it should be impossible for vari to ever be negative. And in fact, all of the failing tests use -t 1, which means that n will be 1, so vari should always be exactly zero. So the fact that you're seeing these errors must mean that we're hitting some kind of floating point round-off error.

Precisely! There must be something that is only present on aarch64 causing this error. This proposed fix just sweeps it under the carpet.

Your patch will work, but it begs the question: why print min/avg/max/stddev when there is only one datapoint? I think that we should only print those things when n > 1. In fact, the standard deviation isn't really meaningful when n == 2, but it probably wouldn't hurt to print it, if that makes the code more readable.

I'll jot this suggestion on my list of things, thank you!

[asomers]

Approved, but please note in the commit message that this condition can only occur due to FPU rounding error.

[jlduran]

I'll investigate a bit further. I am able to reproduce it on my mac:

#include <stdio.h>
#include <math.h>

int
main(void) {
	double tsumsq;
	double num = 1;
	double avg = 0.147;

	tsumsq = avg * avg;

	printf("0 = %f\n", tsumsq / num - avg * avg);
}

0 = -0.000000

[jlduran]

As you well pointed out, here is the fix from NetBSD:

https://github.com/NetBSD/src/commit/79b9ef47d29a719583422b3394753e47a7157443

I'll submit it with the unification of ping/ping6's statistics section (D39126).

[jlduran]

Updated commit message.

[markj]

In D42114#960701, @jlduran_gmail.com wrote:

I'll submit it with the unification of ping/ping6's statistics section (D39126).

Are some of your older ping patches ready to commit? Please point me at them if so.

[jlduran]

In D42114#961150, @markj wrote:

In D42114#960701, @jlduran_gmail.com wrote:

I'll submit it with the unification of ping/ping6's statistics section (D39126).

Are some of your older ping patches ready to commit? Please point me at them if so.

I have created: https://github.com/freebsd/freebsd-src/pull/863

Still missing the update to only display stats when n > 1. I'll submit it afterwards. Thank you!

[jlduran]

I would like to kindly request an MFC to stable/14 of at least this fix.
Reported by: Jenkins
https://ci.freebsd.org/view/Test/job/FreeBSD-stable-14-riscv64-test/lastCompletedBuild/testReport/sbin.ping/ping_test/ping6_c1_s8_t1/
(pasting RISC-V, as aarch64 images are failing)

[markj]

In D42114#969308, @jlduran_gmail.com wrote:

I would like to kindly request an MFC to stable/14 of at least this fix.
Reported by: Jenkins
https://ci.freebsd.org/view/Test/job/FreeBSD-stable-14-riscv64-test/lastCompletedBuild/testReport/sbin.ping/ping_test/ping6_c1_s8_t1/
(pasting RISC-V, as aarch64 images are failing)

Done, thanks for the reminder.

[jlduran]

In D42114#969408, @markj wrote:

In D42114#969308, @jlduran_gmail.com wrote:

I would like to kindly request an MFC to stable/14 of at least this fix.
Reported by: Jenkins
https://ci.freebsd.org/view/Test/job/FreeBSD-stable-14-riscv64-test/lastCompletedBuild/testReport/sbin.ping/ping_test/ping6_c1_s8_t1/
(pasting RISC-V, as aarch64 images are failing)

Done, thanks for the reminder.

Thank you!

[jlduran]

Sorry to bother again, would it be possible to MFC just this commit (4d348e83b738347f6aaf2b110459a01c5402d04e) to stable/13, the other commits in this series cannot be applied.
Reported by: Jenkins
https://ci.freebsd.org/view/Test/job/FreeBSD-stable-13-riscv64-test/936/testReport/sbin.ping/

[markj]

In D42114#970415, @jlduran_gmail.com wrote:

Sorry to bother again, would it be possible to MFC just this commit (4d348e83b738347f6aaf2b110459a01c5402d04e) to stable/13, the other commits in this series cannot be applied.
Reported by: Jenkins
https://ci.freebsd.org/view/Test/job/FreeBSD-stable-13-riscv64-test/936/testReport/sbin.ping/

No problem, done.