Inconsistent semantics of -r and -s with one and many targets

[patrakov]

The code contains some args->targets_len > 1 ifs that make no sense from the consistency viewpoint and have already tripped up the main user (cake-autorate). For example:

tsping -D -m -t -r 500 -s 0 127.0.0.1 127.0.0.2  # creates nicely half-second spaced pings to two targets
tsping -D -m -t -r 500 -s 0 127.0.0.1 127.0.0.2  127.0.0.3  # creates nicely half-second spaced pings to three targets

But, instead of creating nicely half-second spaced pings to one target, this creates an almost infinite-speed flood:

tsping -D -m -t -r 500 -s 0 127.0.0.1

Note that cake-autorate uses --sleep-time "0" --target-spacing "${ping_response_interval_ms}" as hard-coded tsping arguments, and will fall into this trap if the user sets no_pingers to 1.

[lynxthecat]

@patrakov what mechanism do you favour? I liked fping's approach with setting spacing between sends to a specific target and spacing between sends to any target. But @moeller0 prefers this:

[patrakov]

I have no preference. In both cases, you need validation: for fping-inspired syntax, you need to ensure that period >= interval * (targets - 1), while for that syntax, you need to validate that sleep >= 0.

[Lochnair]

I went with the current mechanism out of perceived flexibility (and simplicity), but I'm open to something similar to fping.
I do however not quite understand how fping's approach is supposed to work. Perhaps you can shed some light @lynxthecat?

For example, if I do this:

# fping -D -p 1000 -l 9.9.9.9 9.9.9.10 9.9.9.11
[1678662021.12888] 9.9.9.9  : [0], 64 bytes, 30.0 ms (30.0 avg, 0% loss)
[1678662021.13548] 9.9.9.10 : [0], 64 bytes, 26.5 ms (26.5 avg, 0% loss)
[1678662021.14669] 9.9.9.11 : [0], 64 bytes, 27.7 ms (27.7 avg, 0% loss)
[1678662022.12872] 9.9.9.9  : [1], 64 bytes, 29.5 ms (29.8 avg, 0% loss)
[1678662022.13558] 9.9.9.10 : [1], 64 bytes, 26.3 ms (26.4 avg, 0% loss)
[1678662022.14667] 9.9.9.11 : [1], 64 bytes, 27.3 ms (27.5 avg, 0% loss)

So far so good. And an interval also does what I expect:

# fping -D -i 1000 -l 9.9.9.9 9.9.9.10 9.9.9.11
[1678662097.89469] 9.9.9.9  : [0], 64 bytes, 28.2 ms (28.2 avg, 0% loss)
[1678662098.89077] 9.9.9.10 : [0], 64 bytes, 23.3 ms (23.3 avg, 0% loss)
[1678662099.89625] 9.9.9.11 : [0], 64 bytes, 27.8 ms (27.8 avg, 0% loss)
[1678662100.89827] 9.9.9.9  : [1], 64 bytes, 29.1 ms (28.7 avg, 0% loss)
[1678662101.89428] 9.9.9.10 : [1], 64 bytes, 24.1 ms (23.7 avg, 0% loss)

However, if I try to combine them:

# fping -D -i 1000 -p 2000 -l 9.9.9.9 9.9.9.10 9.9.9.11
[1678662129.65014] 9.9.9.9  : [0], 64 bytes, 33.3 ms (33.3 avg, 0% loss)
[1678662130.64809] 9.9.9.10 : [0], 64 bytes, 30.3 ms (30.3 avg, 0% loss)
[1678662131.64456] 9.9.9.11 : [0], 64 bytes, 25.7 ms (25.7 avg, 0% loss)
[1678662132.65013] 9.9.9.9  : [1], 64 bytes, 31.0 ms (32.1 avg, 0% loss)
[1678662133.64640] 9.9.9.10 : [1], 64 bytes, 26.3 ms (28.3 avg, 0% loss)
[1678662134.64499] 9.9.9.11 : [1], 64 bytes, 23.9 ms (24.8 avg, 0% loss)

Only the interval seems to have any effect, which seems wrong to me

[moeller0]

This is because fping does not really sanitize its inputs or at least does not reject paradoxical requests.
You asked for:

-i, --interval=MSEC
            The minimum amount of time (in milliseconds) between sending a ping packet to any target (default is 10, minimum is 1).

So 1000 ms between 3 reflectors for a total aggregate duration of 3000ms, but also requested to repeat this pattern every 2000 milliseconds. So either fping is going to get an ever increasing number of probes in fliight (but honoring both values) or it ignores one of them. And that is what it seems to do, albeit silently. This is why I argued that I prefer your method where you would need ot specify a negative sleep number to request something similarly paradoxical. But as @patrakov mentioned even such nonsensical negative numbers should be checked and reported as invalid inputs and at that point you might as well sanitize a -i and -p option... (even though sleep can be independently sanitized from target as all it needs to be >= 0, while with interval and period you always need to look at both plus the number of reflectors N as -I * N <= -p if I understand correctly)

[moeller0]

Technically @patrakov's sanitation rule would be OK, but unlikely to be what users want. What is the usecase for having the last interval in a set be shorter?

[lynxthecat]

For now I've pushed: lynxthecat/cake-autorate@a689b36 to prevent ping flooding with only one pinger set in cake-autorate.