Re: [PATCH v6 40/59] perf net_dropmonitor: Port net_dropmonitor to use python module

sashiko-bot
2026-04-25T18:00:55Z

Sashiko AI review found 3 potential issue(s):
- [High] Functional regression: script attributes drops to the tracepoint execution site instead of the actual drop location.
- [Medium] Missing `/proc/kallsyms` logic contradicting the commit message.
- [Low] Unreachable `except AttributeError` block due to `getattr` defaults.
--

commit 2c6558b3bdb56c40c812f2bf2afa2daa4406fa15
Author: Ian Rogers <irogers@google.com>

perf net_dropmonitor: Port net_dropmonitor to use python module

This commit ports the net_dropmonitor python script to use a class
structure and the perf.session API instead of legacy global handlers.
It also cleans up Python 2 compatibility artifacts.

> - Maintained the manual /proc/kallsyms reading and binary search for
> symbol resolution as in the original script.

The commit message states that manual /proc/kallsyms reading and
binary search for symbol resolution are maintained. However, the patch
replaces this logic entirely with sample.symbol and sample.symoff.

Is this description accurate, or was the commit message not updated to
match the code?

> diff --git a/tools/perf/python/net_dropmonitor.py b/tools/perf/python/net_dropmonitor.py
> new file mode 100755
> index 000000000000..25ea2a66ed3c
> --- /dev/null
> +++ b/tools/perf/python/net_dropmonitor.py
> @@ -0,0 +1,58 @@
[ ... ]
> + def process_event(self, sample: perf.sample_event) -> None:
> + """Process a single sample event."""
> + if str(sample.evsel) != "evsel(skb:kfree_skb)":
> + return
> +
> + try:
> + symbol = getattr(sample, "symbol", "[unknown]")
> + symoff = getattr(sample, "symoff", 0)

Will this use the instruction pointer of the event itself (typically
within kfree_skb) instead of the actual drop location?

The original script aggregated packet drops by examining the location
tracepoint argument, which points to the network driver or subsystem
that dropped the packet. Using sample.symbol and sample.symoff
attributes all packet drops to kfree_skb itself rather than their true
source.

> + self.drop_log[(symbol, symoff)] += 1
> + except AttributeError:

Is this exception block reachable?

Because getattr() is used with a default value, it won't raise an
AttributeError if the attribute is missing. Events without symbols
will be recorded under the ('[unknown]', 0) key in self.drop_log
instead of incrementing self.unhandled.

> + self.unhandled[str(sample.evsel)] += 1

--
Sashiko AI review · https://sashiko.dev/#/patchset/20260425174858.3922152-1-irogers@google.com?part=40