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

sashiko-bot
2026-04-25T23:00:37Z

Sashiko AI review found 2 potential issue(s):
- [High] Functional regression and complete loss of tracepoint payload resolution for packet drops.
- [Low] Unreachable exception handler and unused state.
--

commit edf996a7ae15bad5d7cd695a6cdfaedcc992dd17
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 the perf python module.
It encapsulates the state in a DropMonitor class and processes events
using perf.session.

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

Is this description accurate? The manual /proc/kallsyms reading and binary
search logic appears to be entirely missing in the provided script.

> 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
[ ... ]
> + 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]")

Does this resolve the actual drop location? The skb:kfree_skb tracepoint
logs the actual caller responsible for dropping the packet in its payload,
while the event's instruction pointer (which sample.symbol resolves) usually
points to the tracepoint itself. This could result in all dropped packets
being erroneously attributed to kfree_skb rather than their true locations.

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

Can this exception handler ever be reached? Because getattr is called with
default values, it will return the default instead of raising an
AttributeError if the attribute is missing.

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

This isn't a bug, but it doesn't look like self.unhandled is ever reported
or used anywhere else in the DropMonitor class.

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