linux-trace-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] xdp: Remove unused events xdp_redirect_map and xdp_redirect_map_err
@ 2025-06-11 19:56 Steven Rostedt
  2025-06-12 10:30 ` Toke Høiland-Jørgensen
  2025-06-13  2:50 ` patchwork-bot+netdevbpf
  0 siblings, 2 replies; 6+ messages in thread
From: Steven Rostedt @ 2025-06-11 19:56 UTC (permalink / raw)
  To: LKML, Linux trace kernel, bpf
  Cc: Masami Hiramatsu, Mathieu Desnoyers, Alexei Starovoitov,
	Daniel Borkmann, David S. Miller, Jakub Kicinski,
	Jesper Dangaard Brouer, John Fastabend

From: Steven Rostedt <rostedt@goodmis.org>

Each TRACE_EVENT() defined can take up around 5K of text and meta data
regardless if they are used or not. New code is being developed that will
warn when a tracepoint is defined but not used.

The trace events xdp_redirect_map and xdp_redirect_map_err are defined but
not used, but there's also a comment that states these are kept around for
backward compatibility. Which is interesting because since they are not
used, any old BPF program that expects them to exist will get incorrect
data (no data) when they use them. It's worse than not working, it's
silently failing.

Remove them as they will soon cause warnings, or if they really need to
stick around, then code needs to be added to use them.

Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 include/trace/events/xdp.h | 19 -------------------
 1 file changed, 19 deletions(-)

diff --git a/include/trace/events/xdp.h b/include/trace/events/xdp.h
index d3ef86c97ae3..0fe0893c2567 100644
--- a/include/trace/events/xdp.h
+++ b/include/trace/events/xdp.h
@@ -168,25 +168,6 @@ DEFINE_EVENT(xdp_redirect_template, xdp_redirect_err,
 #define _trace_xdp_redirect_map_err(dev, xdp, to, map_type, map_id, index, err) \
 	 trace_xdp_redirect_err(dev, xdp, to, err, map_type, map_id, index)
 
-/* not used anymore, but kept around so as not to break old programs */
-DEFINE_EVENT(xdp_redirect_template, xdp_redirect_map,
-	TP_PROTO(const struct net_device *dev,
-		 const struct bpf_prog *xdp,
-		 const void *tgt, int err,
-		 enum bpf_map_type map_type,
-		 u32 map_id, u32 index),
-	TP_ARGS(dev, xdp, tgt, err, map_type, map_id, index)
-);
-
-DEFINE_EVENT(xdp_redirect_template, xdp_redirect_map_err,
-	TP_PROTO(const struct net_device *dev,
-		 const struct bpf_prog *xdp,
-		 const void *tgt, int err,
-		 enum bpf_map_type map_type,
-		 u32 map_id, u32 index),
-	TP_ARGS(dev, xdp, tgt, err, map_type, map_id, index)
-);
-
 TRACE_EVENT(xdp_cpumap_kthread,
 
 	TP_PROTO(int map_id, unsigned int processed,  unsigned int drops,
-- 
2.47.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] xdp: Remove unused events xdp_redirect_map and xdp_redirect_map_err
  2025-06-11 19:56 [PATCH] xdp: Remove unused events xdp_redirect_map and xdp_redirect_map_err Steven Rostedt
@ 2025-06-12 10:30 ` Toke Høiland-Jørgensen
  2025-06-12 10:54   ` Jesper Dangaard Brouer
  2025-06-13  2:50 ` patchwork-bot+netdevbpf
  1 sibling, 1 reply; 6+ messages in thread
From: Toke Høiland-Jørgensen @ 2025-06-12 10:30 UTC (permalink / raw)
  To: Steven Rostedt, LKML, Linux trace kernel, bpf
  Cc: Masami Hiramatsu, Mathieu Desnoyers, Alexei Starovoitov,
	Daniel Borkmann, David S. Miller, Jakub Kicinski,
	Jesper Dangaard Brouer, John Fastabend

Steven Rostedt <rostedt@goodmis.org> writes:

> From: Steven Rostedt <rostedt@goodmis.org>
>
> Each TRACE_EVENT() defined can take up around 5K of text and meta data
> regardless if they are used or not. New code is being developed that will
> warn when a tracepoint is defined but not used.
>
> The trace events xdp_redirect_map and xdp_redirect_map_err are defined but
> not used, but there's also a comment that states these are kept around for
> backward compatibility. Which is interesting because since they are not
> used, any old BPF program that expects them to exist will get incorrect
> data (no data) when they use them. It's worse than not working, it's
> silently failing.
>
> Remove them as they will soon cause warnings, or if they really need to
> stick around, then code needs to be added to use them.
>
> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>

I guess that makes sense; I have no objections to getting rid of them.

Reviewed-by: Toke Høiland-Jørgensen <toke@kernel.org>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] xdp: Remove unused events xdp_redirect_map and xdp_redirect_map_err
  2025-06-12 10:30 ` Toke Høiland-Jørgensen
@ 2025-06-12 10:54   ` Jesper Dangaard Brouer
  2025-06-16 12:05     ` Jesper Dangaard Brouer
  0 siblings, 1 reply; 6+ messages in thread
From: Jesper Dangaard Brouer @ 2025-06-12 10:54 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen, Steven Rostedt, LKML,
	Linux trace kernel, bpf
  Cc: Masami Hiramatsu, Mathieu Desnoyers, Alexei Starovoitov,
	Daniel Borkmann, David S. Miller, Jakub Kicinski, John Fastabend



On 12/06/2025 12.30, Toke Høiland-Jørgensen wrote:
> Steven Rostedt <rostedt@goodmis.org> writes:
> 
>> From: Steven Rostedt <rostedt@goodmis.org>
>>
>> Each TRACE_EVENT() defined can take up around 5K of text and meta data
>> regardless if they are used or not. New code is being developed that will
>> warn when a tracepoint is defined but not used.
>>
>> The trace events xdp_redirect_map and xdp_redirect_map_err are defined but
>> not used, but there's also a comment that states these are kept around for
>> backward compatibility. Which is interesting because since they are not
>> used, any old BPF program that expects them to exist will get incorrect
>> data (no data) when they use them. It's worse than not working, it's
>> silently failing.
>>
>> Remove them as they will soon cause warnings, or if they really need to
>> stick around, then code needs to be added to use them.
>>
>> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
> 
> I guess that makes sense; I have no objections to getting rid of them.
> 
> Reviewed-by: Toke Høiland-Jørgensen <toke@kernel.org>

Make sense.


Toke we have to check how XDP-tools handle when these tracepoints 
disappears.

Acked-by: Jesper Dangaard Brouer <hawk@kernel.org>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] xdp: Remove unused events xdp_redirect_map and xdp_redirect_map_err
  2025-06-11 19:56 [PATCH] xdp: Remove unused events xdp_redirect_map and xdp_redirect_map_err Steven Rostedt
  2025-06-12 10:30 ` Toke Høiland-Jørgensen
@ 2025-06-13  2:50 ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 6+ messages in thread
From: patchwork-bot+netdevbpf @ 2025-06-13  2:50 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: linux-kernel, linux-trace-kernel, bpf, mhiramat,
	mathieu.desnoyers, ast, daniel, davem, kuba, hawk, john.fastabend

Hello:

This patch was applied to bpf/bpf-next.git (net)
by Alexei Starovoitov <ast@kernel.org>:

On Wed, 11 Jun 2025 15:56:15 -0400 you wrote:
> From: Steven Rostedt <rostedt@goodmis.org>
> 
> Each TRACE_EVENT() defined can take up around 5K of text and meta data
> regardless if they are used or not. New code is being developed that will
> warn when a tracepoint is defined but not used.
> 
> The trace events xdp_redirect_map and xdp_redirect_map_err are defined but
> not used, but there's also a comment that states these are kept around for
> backward compatibility. Which is interesting because since they are not
> used, any old BPF program that expects them to exist will get incorrect
> data (no data) when they use them. It's worse than not working, it's
> silently failing.
> 
> [...]

Here is the summary with links:
  - xdp: Remove unused events xdp_redirect_map and xdp_redirect_map_err
    https://git.kernel.org/bpf/bpf-next/c/a9a5f41b04dd

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] xdp: Remove unused events xdp_redirect_map and xdp_redirect_map_err
  2025-06-12 10:54   ` Jesper Dangaard Brouer
@ 2025-06-16 12:05     ` Jesper Dangaard Brouer
  2025-06-16 12:42       ` Toke Høiland-Jørgensen
  0 siblings, 1 reply; 6+ messages in thread
From: Jesper Dangaard Brouer @ 2025-06-16 12:05 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen, Steven Rostedt, LKML,
	Linux trace kernel, bpf
  Cc: Masami Hiramatsu, Mathieu Desnoyers, Alexei Starovoitov,
	Daniel Borkmann, David S. Miller, Jakub Kicinski, John Fastabend,
	Andrii Nakryiko



On 12/06/2025 12.54, Jesper Dangaard Brouer wrote:
> 
> 
> On 12/06/2025 12.30, Toke Høiland-Jørgensen wrote:
>> Steven Rostedt <rostedt@goodmis.org> writes:
>>
>>> From: Steven Rostedt <rostedt@goodmis.org>
>>>
>>> Each TRACE_EVENT() defined can take up around 5K of text and meta data
>>> regardless if they are used or not. New code is being developed that 
>>> will
>>> warn when a tracepoint is defined but not used.
>>>
>>> The trace events xdp_redirect_map and xdp_redirect_map_err are 
>>> defined but
>>> not used, but there's also a comment that states these are kept 
>>> around for
>>> backward compatibility. Which is interesting because since they are not
>>> used, any old BPF program that expects them to exist will get incorrect
>>> data (no data) when they use them. It's worse than not working, it's
>>> silently failing.
>>>
>>> Remove them as they will soon cause warnings, or if they really need to
>>> stick around, then code needs to be added to use them.
>>>
>>> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
>>
>> I guess that makes sense; I have no objections to getting rid of them.
>>
>> Reviewed-by: Toke Høiland-Jørgensen <toke@kernel.org>
> 
> Make sense.
> 
> 
> Toke we have to check how XDP-tools handle when these tracepoints 
> disappears.

To Toke, notice that userspace tools expect this tracepoint to be
available will fail as below (for kernel release v6.16):

  $ sudo ./xdp-bench redirect mlx5p1 veth41
   libbpf: prog 'tp_xdp_redirect_map_err': failed to find kernel BTF 
type ID of 'xdp_redirect_map_err': -3
   libbpf: prog 'tp_xdp_redirect_map_err': failed to prepare load 
attributes: -3
   libbpf: prog 'tp_xdp_redirect_map_err': failed to load: -3
   libbpf: failed to load object 'xdp_redirect_basic'
  Failed to attach XDP program: No such process

IMHO this is a userspace problem, that needs to be more flexible and
adapt to this change.

This was changed in kernel v5.6 (Jan 2020) commit 1d233886dd90 ("xdp:
Use bulking for non-map XDP_REDIRECT and consolidate code paths").
So, I'm thinking that xdp-tools could just remove monitoring for these
tracepoints?

--Jesper

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] xdp: Remove unused events xdp_redirect_map and xdp_redirect_map_err
  2025-06-16 12:05     ` Jesper Dangaard Brouer
@ 2025-06-16 12:42       ` Toke Høiland-Jørgensen
  0 siblings, 0 replies; 6+ messages in thread
From: Toke Høiland-Jørgensen @ 2025-06-16 12:42 UTC (permalink / raw)
  To: Jesper Dangaard Brouer, Steven Rostedt, LKML, Linux trace kernel,
	bpf
  Cc: Masami Hiramatsu, Mathieu Desnoyers, Alexei Starovoitov,
	Daniel Borkmann, David S. Miller, Jakub Kicinski, John Fastabend,
	Andrii Nakryiko

Jesper Dangaard Brouer <hawk@kernel.org> writes:

> On 12/06/2025 12.54, Jesper Dangaard Brouer wrote:
>> 
>> 
>> On 12/06/2025 12.30, Toke Høiland-Jørgensen wrote:
>>> Steven Rostedt <rostedt@goodmis.org> writes:
>>>
>>>> From: Steven Rostedt <rostedt@goodmis.org>
>>>>
>>>> Each TRACE_EVENT() defined can take up around 5K of text and meta data
>>>> regardless if they are used or not. New code is being developed that 
>>>> will
>>>> warn when a tracepoint is defined but not used.
>>>>
>>>> The trace events xdp_redirect_map and xdp_redirect_map_err are 
>>>> defined but
>>>> not used, but there's also a comment that states these are kept 
>>>> around for
>>>> backward compatibility. Which is interesting because since they are not
>>>> used, any old BPF program that expects them to exist will get incorrect
>>>> data (no data) when they use them. It's worse than not working, it's
>>>> silently failing.
>>>>
>>>> Remove them as they will soon cause warnings, or if they really need to
>>>> stick around, then code needs to be added to use them.
>>>>
>>>> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
>>>
>>> I guess that makes sense; I have no objections to getting rid of them.
>>>
>>> Reviewed-by: Toke Høiland-Jørgensen <toke@kernel.org>
>> 
>> Make sense.
>> 
>> 
>> Toke we have to check how XDP-tools handle when these tracepoints 
>> disappears.
>
> To Toke, notice that userspace tools expect this tracepoint to be
> available will fail as below (for kernel release v6.16):
>
>   $ sudo ./xdp-bench redirect mlx5p1 veth41
>    libbpf: prog 'tp_xdp_redirect_map_err': failed to find kernel BTF 
> type ID of 'xdp_redirect_map_err': -3
>    libbpf: prog 'tp_xdp_redirect_map_err': failed to prepare load 
> attributes: -3
>    libbpf: prog 'tp_xdp_redirect_map_err': failed to load: -3
>    libbpf: failed to load object 'xdp_redirect_basic'
>   Failed to attach XDP program: No such process
>
> IMHO this is a userspace problem, that needs to be more flexible and
> adapt to this change.
>
> This was changed in kernel v5.6 (Jan 2020) commit 1d233886dd90 ("xdp:
> Use bulking for non-map XDP_REDIRECT and consolidate code paths").
> So, I'm thinking that xdp-tools could just remove monitoring for these
> tracepoints?

Yeah, let's just get rid of them:
https://github.com/xdp-project/xdp-tools/pull/513

-Toke

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2025-06-16 12:43 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-11 19:56 [PATCH] xdp: Remove unused events xdp_redirect_map and xdp_redirect_map_err Steven Rostedt
2025-06-12 10:30 ` Toke Høiland-Jørgensen
2025-06-12 10:54   ` Jesper Dangaard Brouer
2025-06-16 12:05     ` Jesper Dangaard Brouer
2025-06-16 12:42       ` Toke Høiland-Jørgensen
2025-06-13  2:50 ` patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).