Add panic hook to disable BPF stats and restore terminal on panic #33
Conversation
| fs::write(PROCFS_BPF_STATS_ENABLED, b"0").context(format!( | ||
| "Failed to disable BPF stats via {}", | ||
| PROCFS_BPF_STATS_ENABLED | ||
| ))?; |
There was a problem hiding this comment.
style: Explicitly throw away the value here by mapping the result
| @@ -131,6 +145,14 @@ fn main() -> Result<()> { | |||
| Ok(()) | |||
| } | |||
|
|
|||
| fn disable_bpf_stats_procfs() -> Result<()> { | |||
There was a problem hiding this comment.
Probably impossible to manage this comprehensively, but, ... does this cripple state for other consumers of stats?
There was a problem hiding this comment.
Yes. Before 5.8 there was only this global toggle. In 5.8+ there is a ref-count model via the bpf syscall, so we'll only increment the count and decrement when starting/exiting bpftop. The kernel will only disable stats if the refcount it at zero, so other consumers of stats won't be impacted if they hold a ref.
What we can do is for 5.7 and below, verify first that the global procfs toggle is enabled. If it was already enabled when the bpftop runs, then we don't disable it on exit/panic. I can follow up with a second PR for this behavior.
Previously if there was a panic we would not disable BPF stats when using the procfs toggle (kernels older than 5.8). This adds a custom panic hook that disables BPF stats if needed, and then attempts to restore the terminal.