pfSense

Since I think "ms" is much more widely used and easier to comprehend, how about using

echo ">>> Gateway alarm: ${GW} (Addr:${alarm_addr} Alarm:${alarm_flag} RTT:$( expr ${alarm_rtt} / 1000 )ms RTTsd:$( expr ${alarm_rttsd} / 1000 )ms Loss:${alarm_loss}%)" | /usr/bin/logger -p daemon.info -i -t rc.gateway_alarm

instead?

System log now shows:

Jul 22 19:49:56     rc.gateway_alarm     12028     >>> Gateway alarm: WAN_DHCP (Addr:172.21.16.1 Alarm:1 RTT:196us RTTsd:17us Loss:22%) 

And gateway log shows:

Jul 22 19:49:56     dpinger         WAN_DHCP 172.21.16.1: Alarm latency 196us stddev 17us loss 22% 

So consistent there. But the initial output and config is inb milliseconds:

Jul 22 17:53:02     dpinger         send_interval 500ms loss_interval 2000ms time_period 60000ms report_interval 0ms data_len 0 alert_interval 1000ms latency_alarm 500ms loss_alarm 20% dest_addr 172.21.16.1 bind_addr 172.21.16.82 identifier "WAN_DHCP " 

Milliseconds is the expected unit there IMO.

Steve:
For the first case (rc.gateway_alarm), how about the change I suggested over on the github commit?
https://github.com/pfsense/pfsense/commit/040a1579e474290abfc7a4a445e310ce8e462847#commitcomment-29773284

For the other 2 (dpinger) we need to update the way dpinger.c outputs to the log, to similarly show in ms if ms > 1, otherwise show in usec
https://github.com/dennypage/dpinger/blob/master/dpinger.c#L660
https://github.com/dennypage/dpinger/blob/master/dpinger.c#L1412

It should be pretty easy to add a quick function to dpinger.c and use it when logging. I could try to make it myself, but Denny could probably do it in his sleep

Personally I can live with seeing '0.196ms' there. Let's see what Stephen suggests when he gets gets the feedback.
However I can see the reasoning behind using µs everywhere.

I'll work on making it milliseconds for everything to be consistent. I added the code you suggested Luke for the gateway alarm I am however using the format 0.xxxms instead of us just to try and keep it as much ms as possible.

print_rtt() {
  usec=$1
  if [ "$usec" -lt "1000" ]; then
    retval="0.${usec}ms" 
  else
    retval="$( expr ${usec} / 1000 )ms" 
  fi
  echo $retval
}

Change made here 37be6b260f8b90393d994c40e2db34925acaa451. I'll have to look into the best way to get that into dpinger as well.

Stephen, That change looks problematic.
I made a comment on the commit but adding one here for completeness.

Example, if the rtt is 10usec, it will print as 0.10ms which actually == 100usec.
If you want everything in ms explicitly, you don't need the if anymore, just use something like

print_rtt() {
  usec_to_ms="$( expr $1 / 1000 )ms" 
  echo $usec_to_ms
}

or maybe just

print_rtt() {
  echo "$( expr $1 / 1000 )ms" 
}

Good catch. I'll change that. Do we care about decimal precision? anything under 1 ms will round to 0. I don't know if this matters or not. I can use bc to get some decimal precision if needs be. I haven't actually used these alarms so I don't know what is expected here.

Good point about expr only doing integer math. For floats we could use bc

print_rtt() {
  usec=$1
  usec_to_ms=$( echo "scale=3; ${usec}/1000" | bc -l )
  echo "${usec_to_ms}ms" 
}

Awesome, Thanks for your help. I made the change you have above to the code here. 21daa13ee2642a5f4821382a46be2dfc71ba2b5a

I made a rough PR for dpinger.c that also replicates this "usec to ms" format change. Minimal testing, but it works on my end. Submitted a PR here:
https://github.com/dennypage/dpinger/pull/32

Before this gets closed out, can the PR 32 for the dpinger.c update get merged as well so everything is consistent (ms)?