xdp: Remove unused events xdp_redirect_map and xdp_redirect_map_err

[PATCH] xdp: Remove unused events xdp_redirect_map and xdp_redirect_map_err

Posted by Steven Rostedt 4 months ago

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

Re: [PATCH] xdp: Remove unused events xdp_redirect_map and xdp_redirect_map_err

Posted by Toke Høiland-Jørgensen 4 months ago

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>

Re: [PATCH] xdp: Remove unused events xdp_redirect_map and xdp_redirect_map_err

Posted by Jesper Dangaard Brouer 4 months ago


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>

Re: [PATCH] xdp: Remove unused events xdp_redirect_map and xdp_redirect_map_err

Posted by Jesper Dangaard Brouer 3 months, 3 weeks ago


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

Re: [PATCH] xdp: Remove unused events xdp_redirect_map and xdp_redirect_map_err

Posted by Toke Høiland-Jørgensen 3 months, 3 weeks ago

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