Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(dumpcap): Handle dumpcap invocation for unix transports #489

Merged
merged 1 commit into from
Jan 2, 2024

Conversation

rumpelsepp
Copy link
Member

Currently there is a flaw in the plugin interface. Plugins cannot
specify how dumpcap should be invoked to capture traffic. The default
is ethernet traffic. There are Unix transports available which are
neither CAN traffic nor ethernet traffic. In this case the wrong
method _eth_cmd() is called which cannot create a valid CLI for
dumpcap.

Currently there is a flaw in the plugin interface. Plugins cannot
specify how dumpcap should be invoked to capture traffic. The default
is ethernet traffic. There are Unix transports available which are
neither CAN traffic nor ethernet traffic. In this case the wrong
method `_eth_cmd()` is called which cannot create a valid CLI for
dumpcap.
@rumpelsepp rumpelsepp requested a review from peckto as a code owner January 2, 2024 12:34
@ferdinandjarisch
Copy link
Contributor

Fixes the unix lines problem, but still has a pretty broad else branch...

Anyhow, looks good to me, but haven't tested it.

@rumpelsepp
Copy link
Member Author

but still has a pretty broad else branch...

I added a point here: #479 under "Open for Discussion". We must default to ethernet for now, since our internal transports are ethernet. There is no API where transports could specify a command/something to capture traffic.

@rumpelsepp rumpelsepp merged commit 7b34f3b into master Jan 2, 2024
11 checks passed
@rumpelsepp rumpelsepp deleted the dumpcap branch January 2, 2024 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants