Lochnair / tsping

a simple application to send ICMP echo/timestamp requests
BSD 3-Clause "New" or "Revised" License
8 stars 2 forks source link

Excellent! #6

Open moeller0 opened 1 year ago

moeller0 commented 1 year ago

This is rather nifty! Excellent work. I have three questions though: 1) would it be hard to convert -m into -m DELIMITER to allow the usage of e.g. : instead of ',' as some locales unhelpfully use , as decimal separator?

2) would it be possible to also add a high resolution RTT field that exploits the fact that on the sender side we have higher timestamp resolution than 1ms for both the Originate and the Finished timestamps? (Reporting these with millisecond granularity is excellent, so all I want is essentially an addition RTT field) same for --icmp-echo... the high resolution data is already there so why not exploit it? Or would that require too much local state?

3) in --icmp-echo mode the -D, --print-timestamps Print UNIX timestamps for responses option seems to not work:

user@ubuntu:~/CODE/tsping/builddir$ sudo ./tsping -D -r 50  -s 0 9.9.9.9 51.75.49.241 145.239.2.3 51.91.81.112 79.137.19.131 62.109.121.1 --icmp-echo -D
Starting tsping 0.1 - pinging 6 targets
9.9.9.9         : [0], 9 ms
Lochnair commented 1 year ago
  1. That's a good point I didn't think of. And no not particularly difficult, I modified the option to optionally accept a different delimiter to use now, so that you can now do for example -m';' or with the long option --machine-readable=';'
  2. For timestamp pings this would require too much local state yes (read: any at all ;-) ). That is not to say I'm opposed to having local state though, I just don't trust myself to implement a sane data structure for it. For echo pings however it would be much simpler to do, as we're not restricted by 32-bits fields, so there we could do ns precision without much difficulty.
  3. facepalms I'd actually managed to forget to handle that option for echo's when I added it, should be fixed now
lynxthecat commented 1 year ago

I installed the new binary yesterday and the line buffering and that other odd issue brought to light seems fixed. Looking forward to testing thoroughly once I've got the bash wrapper properly finalized.

lynxthecat commented 1 year ago

@Lochnair I am pleased to report that overall tsping seems to be working just fine. Myself and at least one other have been using it 24/7 for quite a while now. I haven't tested your new handling of write completions on exit because I already accommodate for that in all ping binaries in cake-autorate now anyway by: 1) discarding buffer with any partial writes on exit; and; 2) appending the timestamp onto the end of each line and verifying that the timestamp matches the appended timestamp (belt and braces).

I think once the interval/sleep issue is looked into, it might be time to release this as an official OpenWrt package.