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

sashiko-bot
2026-04-23T04:07:42Z

Sashiko AI review found 3 potential issue(s):
- [High] Functional regression: script incorrectly aggregates by tracepoint IP instead of the packet drop `location` payload.
- [Medium] Severe mismatch between the commit message and the implemented code regarding `/proc/kallsyms`.
- [Low] Dead code: Unreachable `except AttributeError:` block in `process_event`.
--

commit dd6738be8f63dfa72dba012e8f9cf02a2cba183b
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 class structure
and the perf Python module for event processing.

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

Is this description accurate?

The script doesn't appear to contain any logic to open /proc/kallsyms or
perform a binary search, instead relying directly on the symbol attributes
from the perf sample.

> diff --git a/tools/perf/python/net_dropmonitor.py b/tools/perf/python/net_dropmonitor.py
> new file mode 100755
> index 0000000000000..25ea2a66ed3c8
> --- /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 read the instruction pointer where the perf sample was taken
instead of the packet drop location?

The core purpose of net_dropmonitor is to track where packets are dropped
by inspecting the location payload field of the skb:kfree_skb tracepoint.

Since the tracepoint resides inside kfree_skb_reason() or kfree_skb(),
will using the sample's instruction pointer attribute all dropped packets
to the tracepoint hook itself rather than the caller that actually dropped
the skb?

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

Is this exception block reachable?

Since getattr() is provided with default values ("[unknown]" and 0), it
suppresses the AttributeError for missing attributes and directly returns
the default.

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