Re: [PATCH v5 41/58] perf netdev-times: Port netdev-times to use python module

sashiko-bot
2026-04-24T17:18:18Z

Sashiko AI review found 2 potential issue(s):
- [Medium] The script uses an invalid 'skblen' field name for the skb_copy_datagram_iovec tracepoint.
- [Medium] A KeyError will crash the script if str(sample.evsel) does not start with 'evsel('.
--

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

perf netdev-times: Port netdev-times to use python module

This patch refactors the netdev-times python script to use a class structure
to encapsulate state. It updates the script to use perf.session for event
collection, extracting tracepoint fields directly from sample attributes and
processing them in time order.

> diff --git a/tools/perf/python/netdev-times.py b/tools/perf/python/netdev-times.py
> new file mode 100755
> index 00000000000000..568986e2d49240
> --- /dev/null
> +++ b/tools/perf/python/netdev-times.py

[ ... ]

> + def process_event(self, sample: perf.sample_event) -> None:
> + """Process events directly on-the-fly."""
> + name = str(sample.evsel)
> + pid = sample.sample_pid
> + if hasattr(self, 'session') and self.session:
> + comm = self.session.process(pid).comm()
> + else:
> + comm = "Unknown"
> + event_data = {
> + 'name': name[6:-1] if name.startswith("evsel(") else name,
> + 'time': sample.sample_time,
> + 'cpu': sample.sample_cpu,
> + 'pid': pid,
> + 'comm': comm,
> + }
> +
> + # Extract specific fields based on event type
> + if name.startswith("evsel(irq:softirq_"):

[ ... ]

> + elif name == "evsel(irq:irq_handler_entry)":
> + event_data['irq'] = getattr(sample, "irq", -1)
> + event_data['irq_name'] = getattr(sample, "name", "[unknown]")

If name does not start with "evsel(", event_data['name'] is normalized
to the raw name, but the field extraction checks below use the un-normalized
name variable. Will this cause the if/elif chain to be skipped entirely?

If the if/elif chain is skipped, handlers like handle_irq_handler_entry()
will still be called by handle_single_event(event_data) but will try to
access unpopulated fields like event['irq'], causing a KeyError.

Could the field extraction logic use the normalized event_data['name']
instead?

[ ... ]

> + elif name == "evsel(skb:skb_copy_datagram_iovec)":
> + event_data['skbaddr'] = getattr(sample, "skbaddr", 0)
> + event_data['skblen'] = getattr(sample, "skblen", 0)

Does the skb_copy_datagram_iovec tracepoint use skblen as the field name?

Looking at include/trace/events/skb.h, the field is defined as len. Will
getattr(sample, "skblen", 0) silently return 0 for every event, dropping
the length statistics?

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