IDEA: -x option to define exit conditions for scripting

[ggs67]

IDEA: -x option to define exit conditions for scripting

[ggs67]

FYI: I did implemented a proof of concept version in a separate code module and added with minimal impact to the original code:

• opt_quiet was extended to 2 bits to implment n and N options
• if(opt_quiet < 2) was added to quiescence the remaining messages after -q
• check_exit_condition call was added to the main_loop exit checks
• exit conditions including sequences are implemented by using existing counters only
• -x flag was added to the main command line parser by calling parse_exit_cond()
• opt_exit_cond pointer to condition definition stucture was added to ping_rts (NULL if no -x option was given)
• x and m exit condition options are fully parsed but not yet implemented

[pevik]

Feel free to share this code. But the original spec is not simple, which means ping code would be complicated even further. It's very easy to bring a regression or change behavior others are depending on.

[ggs67]

I agree. I have been product consultant for many years mostly suggesting new features or algorithms. In this activity I have developed a multi-axis approach where one axis is to reduce impact of the feature to a maximum.

This is what I did here, I did minimal changes in the original code. Basically only opt_quiet has been extended to allow a value of 2 for this option. So the original -q quiesced code also works with this value as it only checks boolean. Only the remaining statistics where "iffed-out" by if(rts->opt_quiet < 2).

Finally the main_loop got an additional exit test check_exit_condition() in its head. This function handles all the exit condition handling including the map by just using existing statistics (eg. nerrors and nreceived). For detecting sequences if required I just keep track of the progress of these counters.

My idea is that, if I got something wrong on how ping is internally working, this should only affect the exit condition feature to be buggy, not change any behavior of the original ping.

I am quite finished with the map now as well (no change in original code required).

The only change that I still need to do to original code is the exit code handling, for the exit-condition option "x". My current intend is to implement it via atexit() calling _exit() with the status code if requested. This only requires this to be registered first in the list.

The thing that makes me a bit reluctant toward this solution is that global_rts is intended to be obsolete (while still in used in the code) and subject to removal. This suggests that rts would be gone at the time of atexit() handler calls. I would have to keep exit_cond structure either in a separate global variable or just maintain the current exit condition status in such (tending to the latter).

Once I have verified the map to work I will implement this last part and push code to my fork for review.

I'll keep you posted here.

[ggs67]

I have pushed a first version to my fork :

I have tested this against all options (mostly with ipv4) and a sever simulating intermittent drops (especially for sequence and map tests)

The behavior and options have slightly changed from the initial proposal (see below)

All changes in the original files have been marked with a /GGS/ tag

syntax: -x <exit-cond>

<exit-cond> : <count><loc>[:<opt>]
<count>     : <n>   - exit after receiving <n> (integer > 0) successful  ECHO_RESPONSEs
              -<n>  - exit after <n> unsuccessful pings
<loc>       : if not specified, exit whenever <n> (un)successful pings have been seen, as requested
              s - only exit if <count> was seen in sequence
<opt>       : <opt>* (options can be combined)
              x     - ping exit code reflects if exit-condition was met, rather than normal ping status. Unsuccessful status
                      could be seen if -c or -w are reached before the condition is met
              c     - report exit condition status 'T' (true, eg. met) or 'F' (false, eg. not met at exit)
              [-+]n - ping is silent only outputting a single integer '<m>' at exit, specifying the number of non matching
                      pings (by default). This is most useful with the 'x' option, so on success exit code we know that
                      the condition was met and thus <n> (un)successful pings where seen. This option outputs the number of
                      pings of opposite status than specified in the condition that have been seen. '-' (unsuccessful) or
                      '+' (successfull) may be specified to explicitly request the given count to be output instead

                      Example: -x 3:xn exits with status 0 (condition met (x option)) and prints out 1, this means that we
                               did see 3 successful pings as requested and 1 unsuccessful

                      Example: Waiting for a host to come online I may use -x 10s:x+n (or N instead of +n), if <success> is
                               greater than 10 I know that there have been a few failures after the host was first seen
                               (failures are expected as long as host is down)

              N     - ping is silent only outputting counts <success>/<failures>

              m(<n>[:<s><f>]) - ping is silent only outputting the ping map. Ping map is a sequence of characters, one per
                                ping which show if the corresponding ping was successful (+) or unsuccessful (-)
                                Parenthesis are only needed if options are passed:
                                  <n>   : only the <n> last pings are reported (default=all)
                                  <s><f>: two characters (optional) overriding successful <s> and unsuccessful <f> ping
                                          characters in the map
              q     - exit condition quisence all output from ping also the output generated after the -q ping option
                      This flag only has effect if any reporting has been reuestes (n,N or m options)
              NOTES:  - n and N are mutually exclusive
                      - 'n' or 'N' can be combined with 'm' and/or 'c', for any combination the order is always c/n1/n2/m
                        where n2 is only output if 'N' was used and then n1=sucess, n2=failures, with 'n' only one count is
                        output as reuested (see 'n' option). Any missing option will also not be output including its separator.
                      - -x can of course be combined with any other ping option, for instance combinations with -c allow
                        for some additional explicit conditions not having to be covered via -x. Like "-c 10 -x 10:x"
                        making sure that we had 10 only successful pings, or if we want reverse status for same condition
                        "-c 10 -x -1:x" (status=0 if ever we see an unsuccessful ping)
                      - the default map size is EXIT_COND_DEFAULT_MAP_SIZE (100)
                      - for any map size requested, the max8imum initial size will be EXIT_COND_MAX_INITIAL_MAP_SIZE (512)
                        extending each time by EXIT_COND_MAP_EXTENSION (512) without exceeding the requested maximum
                      - when the map is full it wraps around overwriting the oldest entries
                      - if the 'q' option is used only the report is output on a single line, if not, it is additional
                        labeled by some text to be able to localize and parse it

[pevik]

Tiny details before you send PR (I can fix most of them if the implementation actually works):

1. Changes should be atomic (there is no point to update comments in separate commits - git rebase -i is your friend).
2. Please rebase to the current master (also we have related code: #530).
3. Add your Signed-off-by: (see format of other commits).
4. Non-related .gitignore changes should be in separate commit.
5. Whitespace matters (mixing tabs and spaces it's not good), but I can solve it before merge).
6. typo s/statiustics/statistics/