Add feature: support ICMP type 13/14 timestamps

[gsnw-sebast]

This is an extension to send ICMP type 13 and receive ICMP type 14 with fping (Issue: #265).
To use it, root rights are required on the system and it can be used as follows.

1. ./fping --icmp-timestamp 127.0.0.1

127.0.0.1 is alive (Timestamp Originate=67895434 Receive=67895434 Transmit=67895434)

2. ./fping --icmp-timestamp -c 2 127.0.0.1 (For the output, I first partly oriented myself on hping3)

27.0.0.1 : [0], 20 bytes, 0.072 ms (0.072 avg, 0% loss)
ICMP timestamp: Originate=67951889 Receive=67951889 Transmit=67951889
ICMP timestamp RTT tsrtt=71991
127.0.0.1 : [1], 20 bytes, 0.057 ms (0.064 avg, 0% loss)
ICMP timestamp: Originate=67952890 Receive=67952890 Transmit=67952890
ICMP timestamp RTT tsrtt=56783

127.0.0.1 : xmt/rcv/%loss = 2/2/0%, min/avg/max = 0.057/0.064/0.072

Unfortunately I could not test the function under macOS and the Github Action Test under macOS is not possible.

[coveralls]

coverage: 87.714% (-0.05%) from 87.762%
when pulling 3aa5894 on gsnw:icmp-13_and_14
into 6a7e7ad on schweikert:develop.

[auerswal]

@gsnw-sebast: Thanks for your work on this!

I have not yet found the time to do a thorough review, but the approach looks sensible. I'll try to review this pull request in a couple of weeks.

In your example output for --count=2, you compute a ICMP timestamp RTT with a higher resolution than that if the ICMP timestamps. This is based on existing fping RTT measurements, I think. IMHO this is a bit misleading, since it does not use only the ICMP timestamp data (which has a millisecond resolution). What do you think?

Then I have two high-level suggestions for consideration:

1. Should --icmp-timestamp automatically set --ipv4 since the ICMP timestamp and timestamp reply messages are only defined for IPv4?
2. Should the option combination --icmp-timestamp --size=N be rejected since the ICMP timestamp message has a fixed size?

[gsnw-sebast]

I'm not sure about the RTT issue myself.
I have also tried to orientate myself on hping3 here.

The formula should be TSRTT = (Transmittime - Originatetime) + (ICMP RTT - (Transmittime - Originatetime)
Here it could be that the ICMP RTT is not in this_reply line 2618

if (icmp_request_typ == 13) {
            if(ip_header_otime_ms != -1 && ip_header_rtime_ms != -1 && ip_header_ttime_ms != -1) {
                printf("ICMP timestamp: Originate=%lu Receive=%lu Transmit=%lu\n", ip_header_otime_ms, ip_header_rtime_ms, ip_header_ttime_ms);
                tsrtt = (ip_header_ttime_ms - ip_header_otime_ms) + (this_reply - (ip_header_rtime_ms - ip_header_otime_ms));
                printf("ICMP timestamp RTT tsrtt=%d\n", tsrtt);
            }
            else {
                printf("ICMP timestamp: unknown\n");
            }
        }

1. According to my IPv6 book and the IANA page [1], ICMP Timestamp does not exist under IPv6
2. I would have to check that, thanks for the feedback (Answer: With ICMP Timestamp there is no data area in the header. It therefore has a fixed size and format)

[1] https://www.iana.org/assignments/icmpv6-parameters/icmpv6-parameters.xhtml

[auerswal]

I would prefer fping not to compute an ICMP timestamp RTT. I would also prefer the ICMP timestamp values to always be in the same line as the other output for a received reply message. This could look as follows:

$ fping --icmp-timestamp -c 2 127.0.0.1
127.0.0.1 : [0], 20 bytes, 0.072 ms (0.072 avg, 0% loss), ICMP timestamp: Originate=67951889 Receive=67951889 Transmit=67951889
127.0.0.1 : [1], 20 bytes, 0.057 ms (0.064 avg, 0% loss), ICMP timestamp: Originate=67952890 Receive=67952890 Transmit=67952890

127.0.0.1 : xmt/rcv/%loss = 2/2/0%, min/avg/max = 0.057/0.064/0.072

[gsnw-sebast]

The format you suggested was also my first idea. I have therefore adapted it. Thanks for the feedback.

[gsnw-sebast]

I have changed the parameters so that --icmp-timestamp and --size may not be used at the same time.
Even if e.g. --icmp-timestamp works with e.g. --size in the appropriate size

I know the documentation is still missing

[auerswal]

All in all this looks promising, I think we just need to look at some of the details before merging. Thanks!

[schweikert]

This looks good, thank you!

Suggestion: could you also update CHANGELOG.md?

[auerswal]

Looks good, thanks!

[gsnw-sebast]

Suggestion: could you also update CHANGELOG.md?

Shouldn't the CHANGELOG.md be created again with release 5.3 and then the rule defined that every pull request must also contain the extension of the CHANGELOG.md?

[schweikert]

Suggestion: could you also update CHANGELOG.md?

Shouldn't the CHANGELOG.md be created again with release 5.3 and then the rule defined that every pull request must also contain the extension of the CHANGELOG.md?

Ideally yes, but you could already start with adding a line to a "Next" section maybe? We definitely need to get the habit of always also changing CHANGELOG.md instead of me guessing a good name for the change summary based on the commits.

[moeller0]

This is excellent! However one request, would it be possible to also add the missing 4th number, the time the type 14 packet was received in the same time format as Originate? The goal is to calculate the two OWDs:
Forward OWD: Receive - Originate
Return OWD: MISSINGLOCALRECEIVE - Transmit

and to do this MISSINGLOCALRECEIVE would be helpful, or in the hping case that can be constructed as:
Originate + tsrtt

I guess the same is possible for fping:

user@macbook src % sudo ./fping -c 1 --icmp-timestamp 9.9.9.9
Password:
9.9.9.9 : [0], 20 bytes, 40.9 ms (40.9 avg, 0% loss), ICMP timestamp: Originate=55736120 Receive=55736157 Transmit=55736157

MISSINGLOCALRECEIVE = Originate + 40.9

it is just parsing this would be easier if there was something like:
Originate=55736120 Receive=55736157 Transmit=55736157 Localreceive=55736160

[lynxthecat]

Sweet, so fping is about to support ICMP type 13/14?

[moeller0]

It does, at least the development version!