-
Notifications
You must be signed in to change notification settings - Fork 30
[RFC] Proof of concept: Wireshark as retis UI #533
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
base: main
Are you sure you want to change the base?
Conversation
Instead of returning a Boxed EventSection, make the factories insert sections in the event directly. Signed-off-by: Adrian Moreno <[email protected]>
Instead of a dynamically allocated hashmap of sections, make it a struct with optional fields. This has the following benefits: - cleaner internal usage (no more downcasts) - getting rid of another Id (SectionId) - cleaner python access "event.skb" - cleaner marshaling, Event now implementes {De,Se}rialize - cleaner event-section generation, no longer needed to internally bind an event section to a SectinId Signed-off-by: Adrian Moreno <[email protected]>
There is no need to treat EventSections as a common trait. For python handling, we have pyo3::Py and for json-related stuff we have serde::Serialize. Since Event is Serialize, Series is as well. So converting them to/from json can be done directly on the types without going through serde_json::Value. Signed-off-by: Adrian Moreno <[email protected]>
Now that Event directly implementes serde::Serialize, we can easily make Series do so as well. With that, there is no need to go through serde_json::Value as intermediate step to serialize and deserialize them. Signed-off-by: Adrian Moreno <[email protected]>
Using schemars, we can generate a JSON schema that would describe the Event struct gets serialized. Signed-off-by: Adrian Moreno <[email protected]>
DO NOT MERGE: It still depends on non-upstreamed pcap-file work. Add a custom block at the begining of the pcap file containing the json-schema of the Event and a custom option on each packet with the serialized Event data. Signed-off-by: Adrian Moreno <[email protected]>
The retis wireshark plugin reads the json schema from the first custom block and uses that information to dissect each packet and add the retis metadata to the protocol tree. Signed-off-by: Adrian Moreno <[email protected]>
Shamelessly doing so in a way that wireshark events look nice. Signed-off-by: Adrian Moreno <[email protected]>
Just adding the probe is redundant (already present in interface info). Signed-off-by: Adrian Moreno <[email protected]>
Signed-off-by: Adrian Moreno <[email protected]>
TEST. In a naive attempt to make Wireshark not display the same packet traversing different probes as a retransmit, use the probe as interface. Signed-off-by: Adrian Moreno <[email protected]>
I tested a bit this PR. Retis captured data integrates quite well with the Wirehark workflow and it's not only a way to see packets in Wireshark while having access to extra Retis data, but also to use that data (for filtering for example). It's really nice! This is an RFC so I'll only make two high level comments:
|
At the moment it's quite generic. If it remains generic after the proper review and rework of this feature I agree we could move it outside or even consider proposing it to upstream Wireshark.
Agree, some tweaking seems appropriate, e.g: remove the |
We all love Wireshark!
Here is a proof of concept of using Wireshark as UI for retis.
Code will surely need some refinement but sending it out soon for early feedback and experimentation.
In a nutshell, the PR does the following
schemars
to build a Json-schema that represets ourEvent
retis pcap
.The result is all the retis metadata inside Wireshark, the user can create columns and filters based on them.
Further experiments:
netif_receive_skb
we could adddev
andns
back (in addition to probe info) but for other probes (NFT, OVS) we are still going to see the packet multiple times. So maybe we just need a warning somewhere. Maybe taking the issue upstream (in Wireshark community).Dependencies: