T6456: Convert "monitor traffic" to modern op-mode wrapper #3601
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
talmakion
requested review from
dmbaturin,
sarthurdev,
zdc,
jestabro,
sever-sever and
c-po
The old "monitor traffic" definition had misaligned arguments under the verbose node and manually offered the same parameter keyword in multiple positions to emulate flexible parameters. I've wrapped tcpdump for op-mode and replicated the "varargs" style from mtr.py/mtr.xml.in to present a few more parameters in a more flexible manner. Changes to the Makefile were required for recursive varargs lookup.
|
👍 |
sever-sever
approved these changes
Jun 13, 2024
dmbaturin
approved these changes
Jun 13, 2024
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.
Change Summary
The old "monitor traffic" definition had misaligned arguments under the verbose node and manually offered the same parameter keyword in multiple positions to emulate flexible parameters.
I've wrapped tcpdump for op-mode Python and replicated the "varargs" style from mtr.py/mtr.xml.in to present a few more parameters in a more flexible manner.
Changes to the Makefile were required for recursive varargs lookup.
Types of changes
Related Task(s)
Related PR(s)
Component(s) name
Proposed changes
The old "monitor traffic" op-mode definition called tcpdump directly, arguments passed through under the uppermost "verbose" node were off by one, so tcpdump would not work properly with verbose + any other following param.
I've adapted the mtr.xml.in/mtr.py op_mode varargs style for monitor traffic, allowing flexible parameter ordering and adding a few more flags (such as hexdump) without an exponential increase in XML complexity.
The previous options available are still intact and will work the same way as before - verbose, save and filter, but they may also be used flexibly in any order. The current documentation for "monitor traffic" is still applicable.
How to test
Running through many variations of each flag, for example:
(Example shows some debug output I added while working on it: this is removed in the final PR)
Smoketest result
No smoketests appear to apply to individual op_mode changes.
Checklist: