Skip to content

Networking: support capturing all packets to a pcap file #30637

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

tridge
Copy link
Contributor

@tridge tridge commented Jul 14, 2025

This allows capturing of all packets (ethernet or PPP) to a pcap file which can be read by wireshark

this is very useful for debugging tricky networking issues

tested on CubeOrange with BotbloxDroneNet and on Pixhawk6X

depends on: ArduPilot/lwip#4

this also includes a new set of buffered write operations for logging in AP_Filesystem, based on stdio-like API calls

@tridge tridge force-pushed the pr-net-capture branch 3 times, most recently from 60ff2da to 9a5660c Compare July 14, 2025 05:11
@tridge tridge force-pushed the pr-net-capture branch 3 times, most recently from 36d41f1 to a6e3bdc Compare July 14, 2025 22:19
@tridge tridge added WIP and removed DevCallTopic labels Jul 14, 2025
@tridge tridge added DevCallEU and removed WIP labels Jul 15, 2025
@tpwrules
Copy link
Contributor

tpwrules commented Jul 16, 2025

Regarding the AP_Filesystem buffering, I did some testing and instrumentation with Lua and FATFS does have a sector buffer per file and it does work. I wrote a Lua script that writes one byte every 10ms and in the SD card interface layer in fatfs_diskio.c I see one sector written every five seconds and no reads (except on cluster boundaries). Certainly not multiple transactions per byte as we had previously thought. I don't think there is justification to be scared of misaligned writes, FATFS handles them fine. It's easy to misread the code at first and believe it does not though, I certainly did too!

Timing using the Lua script stats on Cube Orange suggests writing one byte takes about 10 microseconds which does not blow my socks off but is fine. Writing ten bytes also takes about 10 microseconds which matches my expectations of most of the time being locking and call overhead. Writing a sector takes about 4 milliseconds though, from the Lua thread at least, which is pretty bad. I merged this PR in to benchmark it and it seems to have shaved a couple microseconds off each call, but has had no effect on the sector write time, despite the fact that it's now writing four sectors at a time. Which is good as that means 4x the bandwidth.

The FATFS docs suggest a buffer of at least one cluster, which is usually 32K bytes IIRC. But this is a lot of RAM to sacrifice for each file particularly on F4. Your API could be used to amortize the sector write time but we might have to make the buffer size a processor-specific define. I think 8K is too often to sync, it should be once per cluster. I wanted to modify the logger code to sync after the cluster size and also after a fixed time, but haven't got around to it.

Ultimately I think it would be good to centralize the logic for streaming writing files (maybe with a new open code) but as it is doesn't quite feel right. Maybe I just need to sit with it more or see it as its own PR. I do have re-imaginings for AP_Filesystem in my head too which are doubtless conflicting... but we can always change the code again, there are not that many users.

@peterbarker
Copy link
Contributor

Off by default; 1.2kB on CubeOrangePlus Plane (etc.)

@tridge
Copy link
Contributor Author

tridge commented Jul 16, 2025

@tpwrules @peterbarker I've broken out the filesystem changes into #30671 for separate discussion

@tridge tridge requested a review from peterbarker July 29, 2025 00:47
Copy link
Contributor

@peterbarker peterbarker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Double-shuffle done?

@tridge
Copy link
Contributor Author

tridge commented Aug 3, 2025

Double-shuffle done?

yes. We use the 2.2 branch, here:
https://github.com/ArduPilot/lwip/commits/ArduPilot_LWIP_2_2/

@tridge tridge requested a review from peterbarker August 3, 2025 00:11
tridge added 3 commits August 8, 2025 08:03
allows for capture of ethernet or PPP traffic for debugging
enabled by default, but some users may want to save flash space
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants