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

sashiko-bot
2026-04-20T00:22:42Z

Sashiko AI review found 2 potential issue(s):
- [Medium] The `KeyboardInterrupt` exception handler unintentionally restarts the cleanup phase, leading to duplicated data.
- [Medium] Drop locations are incorrectly sorted lexicographically as strings instead of numerically.
--

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

perf net_dropmonitor: Port net_dropmonitor to use python module

This patch ports net_dropmonitor to use the python module by encapsulating
state in a DropMonitor class. It replaces legacy global handlers with
perf.session for event processing and removes Python 2 compatibility
artifacts.

> diff --git a/tools/perf/python/net_dropmonitor.py b/tools/perf/python/net_dropmonitor.py
> new file mode 100755
> index 0000000000000..474b616725b8f
> --- /dev/null
> +++ b/tools/perf/python/net_dropmonitor.py
[ ... ]
> + def print_drop_table(self) -> None:
> + """Print aggregated results."""
> + print(f"{'LOCATION':>25} {'OFFSET':>25} {'COUNT':>25}")
> + for i in sorted(self.drop_log.keys()):

Since drop_log stores string keys, will sorted(self.drop_log.keys()) order
the memory locations lexicographically rather than numerically?

[ ... ]
> + def process_event(self, sample: perf.sample_event) -> None:
[ ... ]
> + slocation = str(location)
> + self.drop_log[slocation] += 1
[ ... ]
> + try:
> + print("Starting trace (Ctrl-C to dump results)")
> + session = perf.session(perf.data(args.input), sample=monitor.process_event)
> + session.process_events()
> +
> + print("Gathering kallsyms data")
> + monitor.get_kallsyms_table()
> + monitor.print_drop_table()
> + except KeyboardInterrupt:
> + print("\nGathering kallsyms data")
> + monitor.get_kallsyms_table()
> + monitor.print_drop_table()

If a KeyboardInterrupt occurs while gathering kallsyms data or printing the
table, will this exception handler execute the cleanup phase a second time?

Looking at get_kallsyms_table():

def get_kallsyms_table(self) -> None:
...
self.kallsyms.append((loc, name))

Because it appends to the kallsyms list, could restarting this phase lead
to duplicated entries?

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