* [PATCH v7 1/4] tracing: add __print_sym() to replace __print_symbolic()
2026-09-22 11:08 [PATCH v7 0/4] tracing: improve symbolic printing Johannes Berg
@ 2026-09-22 11:08 ` Johannes Berg
2026-09-22 11:08 ` [PATCH v7 2/4] tracing/timer: use __print_sym() Johannes Berg
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: Johannes Berg @ 2026-09-22 11:08 UTC (permalink / raw)
To: linux-trace-kernel, linux-kernel, netdev
Cc: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Ilya Maximets, Matthieu Baerts, Aaron Conole, Eelco Chaudron, dev,
Adrian Moreno, Antoine Tenart, Johannes Berg
From: Johannes Berg <johannes.berg@intel.com>
The way __print_symbolic() works is limited and inefficient
in multiple ways:
- you can only use it with a static list of symbols, but
e.g. the SKB dropreasons are now a dynamic list
- it builds the list in memory _three_ times, so it takes
a lot of memory:
- The print_fmt contains the list (since it's passed to
the macro there). This actually contains the names
_twice_, which is fixed up at runtime.
- TRACE_DEFINE_ENUM() puts a 24-byte struct trace_eval_map
for every entry, plus the string pointed to by it, which
cannot be deduplicated with the strings in the print_fmt
- The in-kernel symbolic printing creates yet another list
of struct trace_print_flags for trace_print_symbols_seq()
- it also requires runtime fixup during init, which is a lot
of string parsing due to the print_fmt fixup
Introduce __print_sym() to - over time - replace the old one.
We can easily extend this also to __print_flags later, but I
cared only about the SKB dropreasons for now, which has only
__print_symbolic().
This new __print_sym() requires only a single list of items,
created by TRACE_DEFINE_SYM_LIST(), or can even use another
already existing list by using TRACE_DEFINE_SYM_FNS() with
lookup and show methods.
Then, instead of doing an init-time fixup, just do this at the
time when userspace reads the print_fmt. This way, dynamically
updated lists are possible.
For userspace, nothing actually changes, because the print_fmt
is shown exactly the same way the old __print_symbolic() was.
This adds about 4k .text in my test builds, but that'll be
more than paid for by the actual conversions.
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
v7: address sashiko's alignment concern
v6: don't place data needed at runtime in .init sections,
fixing the crash Steven reported with v1:
https://lore.kernel.org/r/20240819182340.3bd23d67@gandalf.local.home/
(yes, it's been over 2 years ... my bad)
---
include/asm-generic/vmlinux.lds.h | 4 +
include/linux/module.h | 2 +
include/linux/trace_events.h | 7 ++
include/linux/tracepoint.h | 20 +++++
include/trace/stages/init.h | 55 ++++++++++++
include/trace/stages/stage2_data_offsets.h | 6 ++
include/trace/stages/stage3_trace_output.h | 9 ++
include/trace/stages/stage7_class_define.h | 3 +
kernel/module/main.c | 3 +
kernel/trace/trace_events.c | 99 +++++++++++++++++++++-
kernel/trace/trace_output.c | 45 ++++++++++
11 files changed, 251 insertions(+), 2 deletions(-)
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index b2988aa12f66..632eafd0f496 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -275,9 +275,12 @@
#define FTRACE_EVENTS() \
. = ALIGN(8); \
BOUNDED_SECTION(_ftrace_events) \
+ . = ALIGN(8); \
BOUNDED_SECTION_BY(_ftrace_eval_map, _ftrace_eval_maps)
+#define FTRACE_SYM_DEFS() BOUNDED_SECTION(_ftrace_sym_defs)
#else
#define FTRACE_EVENTS()
+#define FTRACE_SYM_DEFS()
#endif
#ifdef CONFIG_TRACING
@@ -391,6 +394,7 @@
TRACE_PRINTKS() \
BPF_RAW_TP() \
TRACEPOINT_STR() \
+ FTRACE_SYM_DEFS() \
KUNIT_TABLE()
/*
diff --git a/include/linux/module.h b/include/linux/module.h
index 96cc98568eea..a02833ac421b 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -516,6 +516,8 @@ struct module {
unsigned int num_trace_events;
struct trace_eval_map **trace_evals;
unsigned int num_trace_evals;
+ struct trace_sym_def **trace_sym_defs;
+ unsigned int num_trace_sym_defs;
#endif
#ifdef CONFIG_DYNAMIC_FTRACE
unsigned int num_ftrace_callsites;
diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
index 5cbd09c8be8d..80c34ef89feb 100644
--- a/include/linux/trace_events.h
+++ b/include/linux/trace_events.h
@@ -29,6 +29,13 @@ const char *trace_print_symbols_seq(struct trace_seq *p, unsigned long val,
const struct trace_print_flags *symbol_array,
size_t symbol_array_size);
+const char *trace_print_sym_seq(struct trace_seq *p, unsigned long long val,
+ const char *(*lookup)(unsigned long long val));
+const char *trace_sym_lookup(const struct trace_sym_entry *list,
+ size_t len, unsigned long long value);
+void trace_sym_show(struct seq_file *m,
+ const struct trace_sym_entry *list, size_t len);
+
#if BITS_PER_LONG == 32
const char *trace_print_flags_seq_u64(struct trace_seq *p, const char *delim,
unsigned long long flags,
diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
index e0d838c9ce93..05c45a72756c 100644
--- a/include/linux/tracepoint.h
+++ b/include/linux/tracepoint.h
@@ -32,6 +32,24 @@ struct trace_eval_map {
unsigned long eval_value;
};
+struct trace_sym_def {
+ const char *system;
+ const char *symbol_id;
+ /* may return NULL, called under rcu_read_lock() */
+ const char * (*lookup)(unsigned long long);
+ /*
+ * Must print the list: ', { val, "name"}, ...'
+ * with no trailing comma, but with the leading ', '
+ * to simplify things:
+ */
+ void (*show)(struct seq_file *);
+};
+
+struct trace_sym_entry {
+ unsigned long long value;
+ const char *name;
+};
+
#define TRACEPOINT_DEFAULT_PRIO 10
extern int
@@ -163,6 +181,8 @@ extern void syscall_unregfunc(void);
#define TRACE_DEFINE_ENUM(x)
#define TRACE_DEFINE_SIZEOF(x)
+#define TRACE_DEFINE_SYM_FNS(...)
+#define TRACE_DEFINE_SYM_LIST(...)
#ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS
static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
diff --git a/include/trace/stages/init.h b/include/trace/stages/init.h
index 000bcfc8dd2e..6285a03a52b2 100644
--- a/include/trace/stages/init.h
+++ b/include/trace/stages/init.h
@@ -23,6 +23,61 @@ TRACE_MAKE_SYSTEM_STR();
__section("_ftrace_eval_map") \
*TRACE_SYSTEM##_##a = &__##TRACE_SYSTEM##_##a
+/*
+ * Define a symbol for __print_sym by giving lookup and
+ * show functions. See &struct trace_sym_def.
+ */
+#undef TRACE_DEFINE_SYM_FNS
+#define TRACE_DEFINE_SYM_FNS(_symbol_id, _lookup, _show) \
+ _TRACE_DEFINE_SYM_FNS(TRACE_SYSTEM, _symbol_id, _lookup, _show)
+#define _TRACE_DEFINE_SYM_FNS(_system, _symbol_id, _lookup, _show) \
+ __TRACE_DEFINE_SYM_FNS(_system, _symbol_id, _lookup, _show)
+#define __TRACE_DEFINE_SYM_FNS(_system, _symbol_id, _lookup, _show) \
+ ___TRACE_DEFINE_SYM_FNS(_system ## _ ## _symbol_id, _symbol_id, \
+ _lookup, _show)
+#define ___TRACE_DEFINE_SYM_FNS(_name, _symbol_id, _lookup, _show) \
+ static struct trace_sym_def \
+ __trace_sym_def_ ## _name = { \
+ .system = TRACE_SYSTEM_STRING, \
+ .symbol_id = #_symbol_id, \
+ .lookup = _lookup, \
+ .show = _show, \
+ }; \
+ static struct trace_sym_def __used \
+ __section("_ftrace_sym_defs") \
+ *__trace_sym_def_p_ ## _name = &__trace_sym_def_ ## _name
+
+/*
+ * Define a symbol for __print_sym by giving lookup and
+ * show functions. See &struct trace_sym_def.
+ */
+#undef TRACE_DEFINE_SYM_LIST
+#define TRACE_DEFINE_SYM_LIST(_symbol_id, ...) \
+ _TRACE_DEFINE_SYM_LIST(TRACE_SYSTEM, _symbol_id, __VA_ARGS__)
+#define _TRACE_DEFINE_SYM_LIST(_system, _symbol_id, ...) \
+ __TRACE_DEFINE_SYM_LIST(_system, _symbol_id, __VA_ARGS__)
+#define __TRACE_DEFINE_SYM_LIST(_system, _symbol_id, ...) \
+ ___TRACE_DEFINE_SYM_LIST(_system ## _ ## _symbol_id, _symbol_id,\
+ __VA_ARGS__)
+#define ___TRACE_DEFINE_SYM_LIST(_name, _symbol_id, ...) \
+ static struct trace_sym_entry \
+ __trace_sym_list_ ## _name[] = { __VA_ARGS__ }; \
+ static const char * \
+ __trace_sym_lookup_ ## _name(unsigned long long value) \
+ { \
+ return trace_sym_lookup(__trace_sym_list_ ## _name, \
+ ARRAY_SIZE(__trace_sym_list_ ## _name), value); \
+ } \
+ static void \
+ __trace_sym_show_ ## _name(struct seq_file *m) \
+ { \
+ trace_sym_show(m, __trace_sym_list_ ## _name, \
+ ARRAY_SIZE(__trace_sym_list_ ## _name)); \
+ } \
+ ___TRACE_DEFINE_SYM_FNS(_name, _symbol_id, \
+ __trace_sym_lookup_ ## _name, \
+ __trace_sym_show_ ## _name)
+
#undef TRACE_DEFINE_SIZEOF
#define TRACE_DEFINE_SIZEOF(a) \
static struct trace_eval_map __used __initdata \
diff --git a/include/trace/stages/stage2_data_offsets.h b/include/trace/stages/stage2_data_offsets.h
index 8b0cff06d346..5afd9de7deb3 100644
--- a/include/trace/stages/stage2_data_offsets.h
+++ b/include/trace/stages/stage2_data_offsets.h
@@ -5,6 +5,12 @@
#undef TRACE_DEFINE_ENUM
#define TRACE_DEFINE_ENUM(a)
+#undef TRACE_DEFINE_SYM_FNS
+#define TRACE_DEFINE_SYM_FNS(_symbol_id, _lookup, _show)
+
+#undef TRACE_DEFINE_SYM_LIST
+#define TRACE_DEFINE_SYM_LIST(_symbol_id, ...)
+
#undef TRACE_DEFINE_SIZEOF
#define TRACE_DEFINE_SIZEOF(a)
diff --git a/include/trace/stages/stage3_trace_output.h b/include/trace/stages/stage3_trace_output.h
index 181b81335781..09304259a8c1 100644
--- a/include/trace/stages/stage3_trace_output.h
+++ b/include/trace/stages/stage3_trace_output.h
@@ -79,6 +79,15 @@
trace_print_symbols_seq(p, value, symbols, ARRAY_SIZE(symbols)); \
})
+#undef __print_sym
+#define __print_sym(value, symbol_id) \
+ ___print_sym(TRACE_SYSTEM, value, symbol_id)
+#define ___print_sym(sys, value, symbol_id) \
+ ____print_sym(sys, value, symbol_id)
+#define ____print_sym(sys, value, symbol_id) \
+ trace_print_sym_seq(p, value, \
+ __trace_sym_def_p_ ## sys ## _ ## symbol_id->lookup)
+
#undef __print_flags_u64
#undef __print_symbolic_u64
#if BITS_PER_LONG == 32
diff --git a/include/trace/stages/stage7_class_define.h b/include/trace/stages/stage7_class_define.h
index 47008897a795..936b9fa08cea 100644
--- a/include/trace/stages/stage7_class_define.h
+++ b/include/trace/stages/stage7_class_define.h
@@ -45,6 +45,9 @@
#define __event_in_softirq() (REC->common_flags & 0x10)
#define __event_in_irq() (REC->common_flags & 0x18)
+#undef __print_sym
+#define __print_sym(value, symbol_id) __print_sym(value:symbol_id)
+
/*
* The below is not executed in the kernel. It is only what is
* displayed in the print format for userspace to parse.
diff --git a/kernel/module/main.c b/kernel/module/main.c
index d0e1e0bd2ad0..8ab386a37376 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -2735,6 +2735,9 @@ static int find_module_sections(struct module *mod, struct load_info *info)
mod->trace_evals = section_objs(info, "_ftrace_eval_map",
sizeof(*mod->trace_evals),
&mod->num_trace_evals);
+ mod->trace_sym_defs = section_objs(info, "_ftrace_sym_defs",
+ sizeof(*mod->trace_sym_defs),
+ &mod->num_trace_sym_defs);
#endif
#ifdef CONFIG_TRACING
mod->trace_bprintk_fmt_start = section_objs(info, "__trace_printk_fmt",
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 30c0ddf90887..53849baae291 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -2111,6 +2111,102 @@ static void *f_next(struct seq_file *m, void *v, loff_t *pos)
return node;
}
+extern struct trace_sym_def *__start_ftrace_sym_defs[];
+extern struct trace_sym_def *__stop_ftrace_sym_defs[];
+
+/* note: @name is not NUL-terminated */
+static void show_sym_list(struct seq_file *m, struct trace_event_call *call,
+ const char *name, unsigned int name_len)
+{
+ struct trace_sym_def **sym_defs;
+ unsigned int n_sym_defs, i;
+
+ if ((call->flags & TRACE_EVENT_FL_DYNAMIC) || !call->module) {
+ sym_defs = __start_ftrace_sym_defs;
+ n_sym_defs = __stop_ftrace_sym_defs - __start_ftrace_sym_defs;
+ } else {
+#ifdef CONFIG_MODULES
+ struct module *mod = call->module;
+
+ sym_defs = mod->trace_sym_defs;
+ n_sym_defs = mod->num_trace_sym_defs;
+#else
+ return;
+#endif /* CONFIG_MODULES */
+ }
+
+ for (i = 0; i < n_sym_defs; i++) {
+ unsigned int sym_len;
+
+ if (!sym_defs[i])
+ continue;
+ if (sym_defs[i]->system != call->class->system)
+ continue;
+ sym_len = strlen(sym_defs[i]->symbol_id);
+ if (name_len != sym_len)
+ continue;
+ if (strncmp(sym_defs[i]->symbol_id, name, sym_len))
+ continue;
+ if (sym_defs[i]->show)
+ sym_defs[i]->show(m);
+ break;
+ }
+}
+
+static void show_print_fmt(struct seq_file *m, struct trace_event_call *call)
+{
+ char *ptr = call->print_fmt;
+ bool in_print_sym = false;
+ int quote = 0;
+
+ seq_puts(m, "\nprint fmt: ");
+ while (*ptr) {
+ if (*ptr == '\\') {
+ seq_putc(m, *ptr);
+ ptr++;
+ /* paranoid */
+ if (!*ptr)
+ break;
+ goto next;
+ }
+ if (*ptr == '"') {
+ quote ^= 1;
+ goto next;
+ }
+ if (quote)
+ goto next;
+
+ if (in_print_sym && *ptr != ':')
+ goto next;
+
+ if (in_print_sym && *ptr == ':') {
+ const char *name;
+
+ ptr++;
+ name = ptr;
+ /* skip the name */
+ while (*ptr && *ptr != ')')
+ ptr++;
+ /* and show the actual list inline now */
+ show_sym_list(m, call, name, ptr - name);
+ in_print_sym = false;
+ continue;
+ }
+
+ if (strncmp(ptr, "__print_sym(", 12) == 0) {
+ ptr += 12;
+ seq_puts(m, "__print_symbolic(");
+ in_print_sym = true;
+ continue;
+ }
+next:
+ seq_putc(m, *ptr);
+ ptr++;
+ }
+
+ seq_putc(m, '\n');
+}
+
static int f_show(struct seq_file *m, void *v)
{
struct trace_event_file *file = event_file_data(m->private);
@@ -2130,8 +2226,7 @@ static int f_show(struct seq_file *m, void *v)
return 0;
case FORMAT_PRINTFMT:
- seq_printf(m, "\nprint fmt: %s\n",
- call->print_fmt);
+ show_print_fmt(m, call);
return 0;
}
diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
index a5ad76175d10..0850b3ad5707 100644
--- a/kernel/trace/trace_output.c
+++ b/kernel/trace/trace_output.c
@@ -131,6 +131,51 @@ trace_print_symbols_seq(struct trace_seq *p, unsigned long val,
}
EXPORT_SYMBOL(trace_print_symbols_seq);
+const char *trace_sym_lookup(const struct trace_sym_entry *list,
+ size_t len, unsigned long long value)
+{
+ size_t i;
+
+ for (i = 0; i < len; i++) {
+ if (list[i].value == value)
+ return list[i].name;
+ }
+ return NULL;
+}
+EXPORT_SYMBOL(trace_sym_lookup);
+
+void trace_sym_show(struct seq_file *m,
+ const struct trace_sym_entry *list, size_t len)
+{
+ size_t i;
+
+ for (i = 0; i < len; i++)
+ seq_printf(m, ", { %lld, \"%s\" }",
+ list[i].value, list[i].name);
+}
+EXPORT_SYMBOL(trace_sym_show);
+
+const char *
+trace_print_sym_seq(struct trace_seq *p, unsigned long long val,
+ const char *(*lookup)(unsigned long long val))
+{
+ const char *ret = trace_seq_buffer_ptr(p);
+ const char *name;
+
+ rcu_read_lock();
+ name = lookup(val);
+ if (name)
+ trace_seq_puts(p, name);
+ else
+ trace_seq_printf(p, "0x%llx", val);
+ rcu_read_unlock();
+
+ trace_seq_putc(p, 0);
+
+ return ret;
+}
+EXPORT_SYMBOL(trace_print_sym_seq);
+
#if BITS_PER_LONG == 32
const char *
trace_print_flags_seq_u64(struct trace_seq *p, const char *delim,
--
2.55.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH v7 2/4] tracing/timer: use __print_sym()
2026-09-22 11:08 [PATCH v7 0/4] tracing: improve symbolic printing Johannes Berg
2026-09-22 11:08 ` [PATCH v7 1/4] tracing: add __print_sym() to replace __print_symbolic() Johannes Berg
@ 2026-09-22 11:08 ` Johannes Berg
2026-09-22 11:08 ` [PATCH v7 3/4] net: dropreason: use new __print_sym() in tracing Johannes Berg
2026-09-22 11:08 ` [PATCH v7 4/4] net: drop_monitor: use drop_reason_lookup() Johannes Berg
3 siblings, 0 replies; 7+ messages in thread
From: Johannes Berg @ 2026-09-22 11:08 UTC (permalink / raw)
To: linux-trace-kernel, linux-kernel, netdev
Cc: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Ilya Maximets, Matthieu Baerts, Aaron Conole, Eelco Chaudron, dev,
Adrian Moreno, Antoine Tenart, Johannes Berg
From: Johannes Berg <johannes.berg@intel.com>
Use the new __print_sym() in the timer tracing, just to show
how to convert something. This adds ~80 bytes of .text for a
saving of ~1.5K of data in my builds.
Note the format changes from
print fmt: "success=%d dependency=%s", REC->success, __print_symbolic(REC->dependency, { 0, "NONE" }, { (1 << 0), "POSIX_TIMER" }, { (1 << 1), "PERF_EVENTS" }, { (1 << 2), "SCHED" }, { (1 << 3), "CLOCK_UNSTABLE" }, { (1 << 4), "RCU" }, { (1 << 5), "RCU_EXP" })
to
print fmt: "success=%d dependency=%s", REC->success, __print_symbolic(REC->dependency, { 0, "NONE" }, { 1, "POSIX_TIMER" }, { 2, "PERF_EVENTS" }, { 4, "SCHED" }, { 8, "CLOCK_UNSTABLE" }, { 16, "RCU" }, { 32, "RCU_EXP" })
since the values are now just printed in the show function as
pure decimal values.
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
include/trace/events/timer.h | 18 +++++-------------
1 file changed, 5 insertions(+), 13 deletions(-)
diff --git a/include/trace/events/timer.h b/include/trace/events/timer.h
index 3aa0608c6361..d45259d96069 100644
--- a/include/trace/events/timer.h
+++ b/include/trace/events/timer.h
@@ -441,26 +441,18 @@ TRACE_EVENT(itimer_expire,
#undef tick_dep_mask_name
#undef tick_dep_name_end
-/* The MASK will convert to their bits and they need to be processed too */
-#define tick_dep_name(sdep) TRACE_DEFINE_ENUM(TICK_DEP_BIT_##sdep); \
- TRACE_DEFINE_ENUM(TICK_DEP_MASK_##sdep);
-#define tick_dep_name_end(sdep) TRACE_DEFINE_ENUM(TICK_DEP_BIT_##sdep); \
- TRACE_DEFINE_ENUM(TICK_DEP_MASK_##sdep);
-/* NONE only has a mask defined for it */
-#define tick_dep_mask_name(sdep) TRACE_DEFINE_ENUM(TICK_DEP_MASK_##sdep);
+#define tick_dep_name(sdep) { TICK_DEP_MASK_##sdep, #sdep },
+#define tick_dep_mask_name(sdep) { TICK_DEP_MASK_##sdep, #sdep },
+#define tick_dep_name_end(sdep) { TICK_DEP_MASK_##sdep, #sdep }
-TICK_DEP_NAMES
+TRACE_DEFINE_SYM_LIST(tick_dep_names, TICK_DEP_NAMES);
#undef tick_dep_name
#undef tick_dep_mask_name
#undef tick_dep_name_end
-#define tick_dep_name(sdep) { TICK_DEP_MASK_##sdep, #sdep },
-#define tick_dep_mask_name(sdep) { TICK_DEP_MASK_##sdep, #sdep },
-#define tick_dep_name_end(sdep) { TICK_DEP_MASK_##sdep, #sdep }
-
#define show_tick_dep_name(val) \
- __print_symbolic(val, TICK_DEP_NAMES)
+ __print_sym(val, tick_dep_names)
TRACE_EVENT(tick_stop,
--
2.55.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH v7 3/4] net: dropreason: use new __print_sym() in tracing
2026-09-22 11:08 [PATCH v7 0/4] tracing: improve symbolic printing Johannes Berg
2026-09-22 11:08 ` [PATCH v7 1/4] tracing: add __print_sym() to replace __print_symbolic() Johannes Berg
2026-09-22 11:08 ` [PATCH v7 2/4] tracing/timer: use __print_sym() Johannes Berg
@ 2026-09-22 11:08 ` Johannes Berg
2026-09-24 15:31 ` Ilya Maximets
2026-09-28 13:34 ` Aaron Conole
2026-09-22 11:08 ` [PATCH v7 4/4] net: drop_monitor: use drop_reason_lookup() Johannes Berg
3 siblings, 2 replies; 7+ messages in thread
From: Johannes Berg @ 2026-09-22 11:08 UTC (permalink / raw)
To: linux-trace-kernel, linux-kernel, netdev
Cc: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Ilya Maximets, Matthieu Baerts, Aaron Conole, Eelco Chaudron, dev,
Adrian Moreno, Antoine Tenart, Johannes Berg
From: Johannes Berg <johannes.berg@intel.com>
The __print_symbolic() could only ever print the core
drop reasons, since that's the way the infrastructure
works. Now that we have __print_sym() with all the
advantages mentioned in that commit, convert to that
and get all the drop reasons from all subsystems. As
we already have a list of them, that's really easy.
This is a little bit of .text (~100 bytes in my build)
and saves a lot of .data (~17k).
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
v7: update the selftest accordingly
---
include/net/dropreason.h | 5 +++
include/trace/events/skb.h | 13 ++----
net/core/skbuff.c | 43 +++++++++++++++++++
.../selftests/net/openvswitch/openvswitch.sh | 17 ++------
4 files changed, 55 insertions(+), 23 deletions(-)
diff --git a/include/net/dropreason.h b/include/net/dropreason.h
index 1df60645fb27..dc4a60130c09 100644
--- a/include/net/dropreason.h
+++ b/include/net/dropreason.h
@@ -42,6 +42,11 @@ struct drop_reason_list {
extern const struct drop_reason_list __rcu *
drop_reasons_by_subsys[SKB_DROP_REASON_SUBSYS_NUM];
+#ifdef CONFIG_TRACEPOINTS
+const char *drop_reason_lookup(unsigned long long value);
+void drop_reason_show(struct seq_file *m);
+#endif
+
void drop_reasons_register_subsys(enum skb_drop_reason_subsys subsys,
const struct drop_reason_list *list);
void drop_reasons_unregister_subsys(enum skb_drop_reason_subsys subsys);
diff --git a/include/trace/events/skb.h b/include/trace/events/skb.h
index 2945aa7fe9a7..991bf172a6ea 100644
--- a/include/trace/events/skb.h
+++ b/include/trace/events/skb.h
@@ -8,15 +8,9 @@
#include <linux/skbuff.h>
#include <linux/netdevice.h>
#include <linux/tracepoint.h>
+#include <net/dropreason.h>
-#undef FN
-#define FN(reason) TRACE_DEFINE_ENUM(SKB_DROP_REASON_##reason);
-DEFINE_DROP_REASON(FN, FN)
-
-#undef FN
-#undef FNe
-#define FN(reason) { SKB_DROP_REASON_##reason, #reason },
-#define FNe(reason) { SKB_DROP_REASON_##reason, #reason }
+TRACE_DEFINE_SYM_FNS(drop_reason, drop_reason_lookup, drop_reason_show);
/*
* Tracepoint for free an sk_buff:
@@ -47,8 +41,7 @@ TRACE_EVENT(kfree_skb,
TP_printk("skbaddr=%p rx_sk=%p protocol=%u location=%pS reason: %s",
__entry->skbaddr, __entry->rx_sk, __entry->protocol,
__entry->location,
- __print_symbolic(__entry->reason,
- DEFINE_DROP_REASON(FN, FNe)))
+ __print_sym(__entry->reason, drop_reason))
);
#undef FN
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index dbbe10277d51..8c8f7a05dea4 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -154,6 +154,49 @@ drop_reasons_by_subsys[SKB_DROP_REASON_SUBSYS_NUM] = {
};
EXPORT_SYMBOL(drop_reasons_by_subsys);
+#ifdef CONFIG_TRACEPOINTS
+const char *drop_reason_lookup(unsigned long long value)
+{
+ unsigned long long subsys_id = value >> SKB_DROP_REASON_SUBSYS_SHIFT;
+ u32 reason = value & ~SKB_DROP_REASON_SUBSYS_MASK;
+ const struct drop_reason_list *subsys;
+
+ if (subsys_id >= SKB_DROP_REASON_SUBSYS_NUM)
+ return NULL;
+
+ subsys = rcu_dereference(drop_reasons_by_subsys[subsys_id]);
+ if (!subsys)
+ return NULL;
+ if (reason >= subsys->n_reasons)
+ return NULL;
+ return subsys->reasons[reason];
+}
+
+void drop_reason_show(struct seq_file *m)
+{
+ u32 subsys_id;
+
+ rcu_read_lock();
+ for (subsys_id = 0; subsys_id < SKB_DROP_REASON_SUBSYS_NUM; subsys_id++) {
+ const struct drop_reason_list *subsys;
+ u32 i;
+
+ subsys = rcu_dereference(drop_reasons_by_subsys[subsys_id]);
+ if (!subsys)
+ continue;
+
+ for (i = 0; i < subsys->n_reasons; i++) {
+ if (!subsys->reasons[i])
+ continue;
+ seq_printf(m, ", { %u, \"%s\" }",
+ (subsys_id << SKB_DROP_REASON_SUBSYS_SHIFT) | i,
+ subsys->reasons[i]);
+ }
+ }
+ rcu_read_unlock();
+}
+#endif
+
/**
* drop_reasons_register_subsys - register another drop reason subsystem
* @subsys: the subsystem to register, must not be the core
diff --git a/tools/testing/selftests/net/openvswitch/openvswitch.sh b/tools/testing/selftests/net/openvswitch/openvswitch.sh
index a31f7fb6882d..9b8edfcd2d1a 100755
--- a/tools/testing/selftests/net/openvswitch/openvswitch.sh
+++ b/tools/testing/selftests/net/openvswitch/openvswitch.sh
@@ -234,7 +234,7 @@ ovs_drop_reason_count()
local reason=$1
local perf_output=`perf script -i ${ovs_dir}/perf.data -F trace:event,trace`
- local pattern="skb:kfree_skb:.*reason: $reason"
+ local pattern="skb:kfree_skb:.*reason: $reason$"
return `echo "$perf_output" | grep "$pattern" | wc -l`
}
@@ -790,15 +790,6 @@ test_psample() {
# - drop packets and verify the right drop reason is reported
test_drop_reason() {
which perf >/dev/null 2>&1 || return $ksft_skip
- which pahole >/dev/null 2>&1 || return $ksft_skip
-
- ovs_drop_subsys=$(pahole -C skb_drop_reason_subsys |
- awk '/OPENVSWITCH/ { print $3; }' |
- tr -d ,)
- if [ -z "$ovs_drop_subsys" ]; then
- info "failed to get OVS drop subsys ID"
- return $ksft_skip
- fi
sbx_add "test_drop_reason" || return $?
@@ -842,7 +833,7 @@ test_drop_reason() {
"in_port(2),eth(),eth_type(0x0800),ipv4(src=172.31.110.20,proto=1),icmp()" 'drop'
ovs_drop_record_and_run "test_drop_reason" ip netns exec client ping -c 2 172.31.110.20
- ovs_drop_reason_count 0x${ovs_drop_subsys}0001 # OVS_DROP_FLOW_ACTION
+ ovs_drop_reason_count OVS_DROP_LAST_ACTION
if [[ "$?" -ne "2" ]]; then
info "Did not detect expected drops: $?"
return 1
@@ -859,7 +850,7 @@ test_drop_reason() {
ovs_drop_record_and_run \
"test_drop_reason" ip netns exec client nc -i 1 -zuv 172.31.110.20 6000
- ovs_drop_reason_count 0x${ovs_drop_subsys}0004 # OVS_DROP_EXPLICIT_ACTION_ERROR
+ ovs_drop_reason_count OVS_DROP_EXPLICIT_WITH_ERROR
if [[ "$?" -ne "1" ]]; then
info "Did not detect expected explicit error drops: $?"
return 1
@@ -867,7 +858,7 @@ test_drop_reason() {
ovs_drop_record_and_run \
"test_drop_reason" ip netns exec client nc -i 1 -zuv 172.31.110.20 7000
- ovs_drop_reason_count 0x${ovs_drop_subsys}0003 # OVS_DROP_EXPLICIT_ACTION
+ ovs_drop_reason_count OVS_DROP_EXPLICIT
if [[ "$?" -ne "1" ]]; then
info "Did not detect expected explicit drops: $?"
return 1
--
2.55.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH v7 4/4] net: drop_monitor: use drop_reason_lookup()
2026-09-22 11:08 [PATCH v7 0/4] tracing: improve symbolic printing Johannes Berg
` (2 preceding siblings ...)
2026-09-22 11:08 ` [PATCH v7 3/4] net: dropreason: use new __print_sym() in tracing Johannes Berg
@ 2026-09-22 11:08 ` Johannes Berg
3 siblings, 0 replies; 7+ messages in thread
From: Johannes Berg @ 2026-09-22 11:08 UTC (permalink / raw)
To: linux-trace-kernel, linux-kernel, netdev
Cc: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Ilya Maximets, Matthieu Baerts, Aaron Conole, Eelco Chaudron, dev,
Adrian Moreno, Antoine Tenart, Johannes Berg
From: Johannes Berg <johannes.berg@intel.com>
Now that we have drop_reason_lookup(), we can just use it for
drop_monitor as well, rather than exporting the list itself.
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
v7: don't hide drop_reason_show() prototype (sashiko,
but it got it the wrong way around)
---
include/net/dropreason.h | 5 +----
net/core/drop_monitor.c | 20 +++++---------------
net/core/skbuff.c | 6 +++---
3 files changed, 9 insertions(+), 22 deletions(-)
diff --git a/include/net/dropreason.h b/include/net/dropreason.h
index dc4a60130c09..537428c72642 100644
--- a/include/net/dropreason.h
+++ b/include/net/dropreason.h
@@ -38,12 +38,9 @@ struct drop_reason_list {
size_t n_reasons;
};
-/* Note: due to dynamic registrations, access must be under RCU */
-extern const struct drop_reason_list __rcu *
-drop_reasons_by_subsys[SKB_DROP_REASON_SUBSYS_NUM];
+const char *drop_reason_lookup(unsigned long long value);
#ifdef CONFIG_TRACEPOINTS
-const char *drop_reason_lookup(unsigned long long value);
void drop_reason_show(struct seq_file *m);
#endif
diff --git a/net/core/drop_monitor.c b/net/core/drop_monitor.c
index edc660778408..5aa7692c5df2 100644
--- a/net/core/drop_monitor.c
+++ b/net/core/drop_monitor.c
@@ -611,9 +611,8 @@ static int net_dm_packet_report_fill(struct sk_buff *msg, struct sk_buff *skb,
size_t payload_len)
{
struct net_dm_skb_cb *cb = NET_DM_SKB_CB(skb);
- const struct drop_reason_list *list = NULL;
- unsigned int subsys, subsys_reason;
char buf[NET_DM_MAX_SYMBOL_LEN];
+ const char *reason_str;
struct nlattr *attr;
void *hdr;
int rc;
@@ -631,19 +630,10 @@ static int net_dm_packet_report_fill(struct sk_buff *msg, struct sk_buff *skb,
goto nla_put_failure;
rcu_read_lock();
- subsys = u32_get_bits(cb->reason, SKB_DROP_REASON_SUBSYS_MASK);
- if (subsys < SKB_DROP_REASON_SUBSYS_NUM)
- list = rcu_dereference(drop_reasons_by_subsys[subsys]);
- subsys_reason = cb->reason & ~SKB_DROP_REASON_SUBSYS_MASK;
- if (!list ||
- subsys_reason >= list->n_reasons ||
- !list->reasons[subsys_reason] ||
- strlen(list->reasons[subsys_reason]) > NET_DM_MAX_REASON_LEN) {
- list = rcu_dereference(drop_reasons_by_subsys[SKB_DROP_REASON_SUBSYS_CORE]);
- subsys_reason = SKB_DROP_REASON_NOT_SPECIFIED;
- }
- if (nla_put_string(msg, NET_DM_ATTR_REASON,
- list->reasons[subsys_reason])) {
+ reason_str = drop_reason_lookup(cb->reason);
+ if (unlikely(!reason_str))
+ reason_str = drop_reason_lookup(SKB_DROP_REASON_NOT_SPECIFIED);
+ if (nla_put_string(msg, NET_DM_ATTR_REASON, reason_str)) {
rcu_read_unlock();
goto nla_put_failure;
}
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 8c8f7a05dea4..c675d3330937 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -148,13 +148,11 @@ static const struct drop_reason_list drop_reasons_core = {
.n_reasons = ARRAY_SIZE(drop_reasons),
};
-const struct drop_reason_list __rcu *
+static const struct drop_reason_list __rcu *
drop_reasons_by_subsys[SKB_DROP_REASON_SUBSYS_NUM] = {
[SKB_DROP_REASON_SUBSYS_CORE] = RCU_INITIALIZER(&drop_reasons_core),
};
-EXPORT_SYMBOL(drop_reasons_by_subsys);
-#ifdef CONFIG_TRACEPOINTS
const char *drop_reason_lookup(unsigned long long value)
{
unsigned long long subsys_id = value >> SKB_DROP_REASON_SUBSYS_SHIFT;
@@ -171,7 +169,9 @@ const char *drop_reason_lookup(unsigned long long value)
return NULL;
return subsys->reasons[reason];
}
+EXPORT_SYMBOL(drop_reason_lookup);
+#ifdef CONFIG_TRACEPOINTS
void drop_reason_show(struct seq_file *m)
{
u32 subsys_id;
--
2.55.0
^ permalink raw reply related [flat|nested] 7+ messages in thread