Re: [PATCH v5 40/58] perf net_dropmonitor: Port net_dropmonitor to use python module

sashiko-bot
2026-04-24T17:03:01Z

Sashiko AI review found 2 potential issue(s):
- [High] Functional regression: script incorrectly aggregates drops by the sample instruction pointer instead of the drop location, and missing kallsyms parsing contradicts the commit message.
- [Medium] Dead code in `process_event` due to catching `AttributeError` from a `getattr` call that provides a default value.
--

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

perf net_dropmonitor: Port net_dropmonitor to use python module

This commit ports the net_dropmonitor script to use a Python module,
refactoring it to use a class structure and perf.session for event
processing.

> - 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 is maintained. Is this description accurate? Looking at
the script, this logic appears to have been removed entirely.

> diff --git a/tools/perf/python/net_dropmonitor.py b/tools/perf/python/net_dropmonitor.py
> new file mode 100755
> index 00000000000000..25ea2a66ed3c89
> --- /dev/null
> +++ b/tools/perf/python/net_dropmonitor.py

[ ... ]

> + 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)

Does this correctly identify the drop locations? By using sample.symbol and
sample.symoff, it appears the script resolves the instruction pointer where
the sample was taken (which will likely always point to kfree_skb itself).

Should this be extracting the raw drop address from the event's "location"
payload field instead, to accurately report where the packets were dropped?

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

Can this exception handler ever be reached? Because getattr() is called above
with default values ("[unknown]" and 0), it will return the default instead of
raising an AttributeError when the attribute is missing.

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

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