Make switch_tcpdump more robust and user-friendly #192
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ideaship
force-pushed
the
rl_switch_tcpdump_sighandler
branch
from
8df9563
to
48e2112
Compare
KanjiMonster
requested changes
May 7, 2024
ideaship
force-pushed
the
rl_switch_tcpdump_sighandler
branch
from
48e2112
to
d896fde
Compare
ideaship
force-pushed
the
rl_switch_tcpdump_sighandler
branch
from
d896fde
to
52046cb
Compare
ideaship
force-pushed
the
rl_switch_tcpdump_sighandler
branch
from
52046cb
to
50c764f
Compare
The tcpdump program uses exit status 1 (which is then passed back to the user via switch_tcpdump) to signal most common errors, and it does not use exit status 2. Have switch_tcpdump use exit status 2 to make it easier to identify the source of an error. Signed-off-by: Roger Luethi <roger.luethi@bisdn.de>
Make sure switch_tcpdump messages printed prior to starting tcpdump are flushed before the tcpdump output. Otherwise, the console output may look quite confusing. Signed-off-by: Roger Luethi <roger.luethi@bisdn.de>
Allow setting --maxSize to 0 to have unlimited file size. This lets the user skip all the system calls associated with checking the file size frequently. Signed-off-by: Roger Luethi <roger.luethi@bisdn.de>
There are two error messages from /usr/sbin/ofdpa_acl_flow_cli.py that a switch_tcpdump user is likely to encounter at some point. When trying to add the ACL rule which already exists: Error from ofdpaFlowAdd(). rc = -25 When trying to remove the ACL rule when it does not exist: Error from ofdpaFlowDelete(). rc = -30 These error message provide little useful information to the user, and they cannot be prevented entirely (consider, for instance, running switch_tcpdump after terminating it with signal 9). Catch them and show only messages for unexpected errors. Also turn the returncode values (e.g., 231) into signed int (e.g., -25). The function caller can still choose to handle the rc values as they see fit. Signed-off-by: Roger Luethi <roger.luethi@bisdn.de>
When shutting down, ignore the error returned by rule_delete if the ACL rule is already gone. It is not a problem. When starting up and receiving an error from rule_insert because a colliding ACL rule is already present (presumably the same one), just remove the rule (to be sure) and insert it again (instead of aborting with an error). Both changes should make switch_tcpdump more user-friendly. Signed-off-by: Roger Luethi <roger.luethi@bisdn.de>
Add a custom signal handler that deletes the ACL rule immediately. Attach the handler to signals INT and TERM (to have it called when the user hits Ctrl-C or the program ends any other way). At the end of the handler, raise KeyboardInterrupt to ensure the tcpdump process is terminated (if it hasn't been already). Note that we can disable signals in rule_delete because they get re-enabled when the custom signal handler is installed (and we don't care to receive further signals when rule_delete is called during process termination). Signed-off-by: Roger Luethi <roger.luethi@bisdn.de>
Eliminate misleading "ingress packets" which wrongly suggests that only ingress packets are visible, even though packets originating from the switch (and therefore being seen by the Linux kernel) are also captured when sent through the --inPort interface. Tweak first example to demonstrate the default 100 MB filesize limit, and add a second example to show how to remove the filesize limit. Change final usage example to include additional, typical extra arguments that are passed to tcpdump. Signed-off-by: Roger Luethi <roger.luethi@bisdn.de>
Add a man page for switch_tcpdump which provides information that does not easily fit into the usage message. Formatting for the new man page is based on the tcpdump(8) man page. Mention the man page in the program help text to help users find it. Signed-off-by: Roger Luethi <roger.luethi@bisdn.de>
ideaship
force-pushed
the
rl_switch_tcpdump_sighandler
branch
from
50c764f
to
251d382
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR contains a number of changes for switch_tcpdump that should make the script more robust and more user-friendly.
In particular, a custom signal handler increases the chance that we manage to remove the ACL rule correctly even when getting SIGINT or SIGTERM in quick succession. The script can now also deal automatically with an existing ACL rule (e.g., left by a previous switch_tcpdump terminated by SIGKILL) instead of passing on the error message from ofda_acl_flow_cli.py.
The documentation has also been updated and extended.