[PATCH net-next] udp: Add tracepoint for udp_sendmsg()

[PATCH net-next] udp: Add tracepoint for udp_sendmsg()

[Breno Leitao]

Add a lightweight tracepoint to monitor UDP send message operations,
similar to the recently introduced tcp_sendmsg_locked() trace event in
commit 0f08335ade712 ("trace: tcp: Add tracepoint for
tcp_sendmsg_locked()")

This implementation uses DECLARE_TRACE instead of TRACE_EVENT to avoid
creating extensive trace event infrastructure and exporting to tracefs,
keeping it minimal and efficient.

Since this patch creates a rawtracepoint, it can be accessed using
standard tracing tools like bpftrace:

    rawtracepoint:udp_sendmsg_tp {
        ...
    }

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 include/trace/events/udp.h | 5 +++++
 net/ipv4/udp.c             | 2 ++
 2 files changed, 7 insertions(+)

diff --git a/include/trace/events/udp.h b/include/trace/events/udp.h
index 6142be4068e29..38ab24053b6ff 100644
--- a/include/trace/events/udp.h
+++ b/include/trace/events/udp.h
@@ -46,6 +46,11 @@ TRACE_EVENT(udp_fail_queue_rcv_skb,
 		  __entry->saddr, __entry->daddr)
 );
 
+DECLARE_TRACE(udp_sendmsg_tp,
+	TP_PROTO(const struct sock *sk, const struct msghdr *msg),
+	TP_ARGS(sk, msg)
+);
+
 #endif /* _TRACE_UDP_H */
 
 /* This part must be outside protection */
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index f9f5b92cf4b61..8c2902504a399 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1345,6 +1345,8 @@ int udp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
 		connected = 1;
 	}
 
+	trace_udp_sendmsg_tp(sk, msg);
+
 	ipcm_init_sk(&ipc, inet);
 	ipc.gso_size = READ_ONCE(up->gso_size);
 

---
base-commit: 1d6f4861027b451e064896f34dd0beada8871bfe
change-id: 20250416-udp_sendmsg-084a32657a56

Best regards,
-- 
Breno Leitao <leitao@debian.org>

Re: [PATCH net-next] udp: Add tracepoint for udp_sendmsg()

[Willem de Bruijn]

Breno Leitao wrote:
> Add a lightweight tracepoint to monitor UDP send message operations,
> similar to the recently introduced tcp_sendmsg_locked() trace event in
> commit 0f08335ade712 ("trace: tcp: Add tracepoint for
> tcp_sendmsg_locked()")
> 
> This implementation uses DECLARE_TRACE instead of TRACE_EVENT to avoid
> creating extensive trace event infrastructure and exporting to tracefs,
> keeping it minimal and efficient.
> 
> Since this patch creates a rawtracepoint, it can be accessed using
> standard tracing tools like bpftrace:
> 
>     rawtracepoint:udp_sendmsg_tp {
>         ...
>     }

What does this enable beyond kfunc:udp_sendmsg?

The arguments are the same, unlike the TCP tracepoint.

Re: [PATCH net-next] udp: Add tracepoint for udp_sendmsg()

[Breno Leitao]

Hello Willem,

On Wed, Apr 16, 2025 at 03:34:38PM -0400, Willem de Bruijn wrote:
> Breno Leitao wrote:
> > Add a lightweight tracepoint to monitor UDP send message operations,
> > similar to the recently introduced tcp_sendmsg_locked() trace event in
> > commit 0f08335ade712 ("trace: tcp: Add tracepoint for
> > tcp_sendmsg_locked()")
> > 
> > This implementation uses DECLARE_TRACE instead of TRACE_EVENT to avoid
> > creating extensive trace event infrastructure and exporting to tracefs,
> > keeping it minimal and efficient.
> > 
> > Since this patch creates a rawtracepoint, it can be accessed using
> > standard tracing tools like bpftrace:
> > 
> >     rawtracepoint:udp_sendmsg_tp {
> >         ...
> >     }
> 
> What does this enable beyond kfunc:udp_sendmsg?

A few things come to mind when evaluating the use of tracepoints.

One significant advantage is that tracepoints provide a stable API where
programs can hook into, making it easier for users to interact with key
functions.

However, kfunc/kprobes has some notable disadvantages. For instance,
partial or total inlining can cause hooks to fail, which is not ideal
and can lead to issues (mainly when we have partial inlines, and the
hook works _sometimes_).

In contrast, tracepoints create a more stable API for users to hook
into, eliminating the need to patch the kernel with noinline function
attributes. This solution may be ugly, but it is a common practice.
(and this is my main goal with it, remove `noinline` from our internal
kernel)

While tracepoints are not officially considered stable APIs, they tend
to be "more stable" in practice due to their deliberate and strategic
placement. As a result, they are less likely to get renamed or changed
frequently.

Additionally, tracepoints offer performance benefits, being faster than
both kfunc and kprobes. 

For further discussion on this topic, please refer to same discussion in
VFS:

https://lore.kernel.org/bpf/20250118033723.GV1977892@ZenIV/T/#m4c2fb2d904e839b34800daf8578dff0b9abd69a0

Thanks
--breno

Re: [PATCH net-next] udp: Add tracepoint for udp_sendmsg()

[Willem de Bruijn]

Breno Leitao wrote:
> Hello Willem,
> 
> On Wed, Apr 16, 2025 at 03:34:38PM -0400, Willem de Bruijn wrote:
> > Breno Leitao wrote:
> > > Add a lightweight tracepoint to monitor UDP send message operations,
> > > similar to the recently introduced tcp_sendmsg_locked() trace event in
> > > commit 0f08335ade712 ("trace: tcp: Add tracepoint for
> > > tcp_sendmsg_locked()")
> > > 
> > > This implementation uses DECLARE_TRACE instead of TRACE_EVENT to avoid
> > > creating extensive trace event infrastructure and exporting to tracefs,
> > > keeping it minimal and efficient.
> > > 
> > > Since this patch creates a rawtracepoint, it can be accessed using
> > > standard tracing tools like bpftrace:
> > > 
> > >     rawtracepoint:udp_sendmsg_tp {
> > >         ...
> > >     }
> > 
> > What does this enable beyond kfunc:udp_sendmsg?
> 
> A few things come to mind when evaluating the use of tracepoints.
> 
> One significant advantage is that tracepoints provide a stable API where
> programs can hook into, making it easier for users to interact with key
> functions.
> 
> However, kfunc/kprobes has some notable disadvantages. For instance,
> partial or total inlining can cause hooks to fail, which is not ideal
> and can lead to issues (mainly when we have partial inlines, and the
> hook works _sometimes_).

As Paolo explained, that is unlikely to happen in this case, as this
is a protocol specific callback function.
 
> In contrast, tracepoints create a more stable API for users to hook
> into, eliminating the need to patch the kernel with noinline function
> attributes. This solution may be ugly, but it is a common practice.
> (and this is my main goal with it, remove `noinline` from our internal
> kernel)
> 
> While tracepoints are not officially considered stable APIs, they tend
> to be "more stable" in practice due to their deliberate and strategic
> placement. As a result, they are less likely to get renamed or changed
> frequently.
> 
> Additionally, tracepoints offer performance benefits, being faster than
> both kfunc and kprobes. 

The performance argument is fair.

Perhaps we want to think this through more broadly for networking
tracepoints vs more flexible kprobes/kfuncs, rather than on a case
by case basis:

Where do we think the performance or functionality (if exposing
additional info, as for tcp_sendmsg) warrants the tracepoint?

I suspect that the use is predominantly for on-demand debugging,
where the performance cost (and latency impact) of measurement is
minor.
 
> For further discussion on this topic, please refer to same discussion in
> VFS:
> 
> https://lore.kernel.org/bpf/20250118033723.GV1977892@ZenIV/T/#m4c2fb2d904e839b34800daf8578dff0b9abd69a0
> 
> Thanks
> --breno

Re: [PATCH net-next] udp: Add tracepoint for udp_sendmsg()

[David Ahern]

On 4/16/25 1:23 PM, Breno Leitao wrote:
> diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
> index f9f5b92cf4b61..8c2902504a399 100644
> --- a/net/ipv4/udp.c
> +++ b/net/ipv4/udp.c
> @@ -1345,6 +1345,8 @@ int udp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
>  		connected = 1;
>  	}
>  
> +	trace_udp_sendmsg_tp(sk, msg);

why `_tp` suffix? the usual naming is trace_${func}

Re: [PATCH net-next] udp: Add tracepoint for udp_sendmsg()

[Breno Leitao]

Hello David,

On Wed, Apr 16, 2025 at 04:16:26PM -0700, David Ahern wrote:
> On 4/16/25 1:23 PM, Breno Leitao wrote:
> > diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
> > index f9f5b92cf4b61..8c2902504a399 100644
> > --- a/net/ipv4/udp.c
> > +++ b/net/ipv4/udp.c
> > @@ -1345,6 +1345,8 @@ int udp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
> >  		connected = 1;
> >  	}
> >  
> > +	trace_udp_sendmsg_tp(sk, msg);
> 
> why `_tp` suffix? the usual naming is trace_${func}

I got the impression that DECLARE_TRACE() raw tracepoints names end up
in _tp():

	include/trace/events/tcp.h
	DECLARE_TRACE(tcp_cwnd_reduction_tp,

	include/trace/events/sched.h
	DECLARE_TRACE(pelt_cfs_tp,
	DECLARE_TRACE(pelt_rt_tp,
	DECLARE_TRACE(pelt_dl_tp,
	DECLARE_TRACE(pelt_hw_tp,
	DECLARE_TRACE(pelt_irq_tp,
	DECLARE_TRACE(pelt_se_tp,
	DECLARE_TRACE(sched_cpu_capacity_tp,
	DECLARE_TRACE(sched_overutilized_tp,
	DECLARE_TRACE(sched_util_est_cfs_tp,
	DECLARE_TRACE(sched_util_est_se_tp,
	DECLARE_TRACE(sched_update_nr_running_tp,
	DECLARE_TRACE(sched_compute_energy_tp,
	DECLARE_TRACE(sched_entry_tp,
	DECLARE_TRACE(sched_exit_tp,

But, I am happy to remove _tp if that is not correct.

Thanks,
--breno

Re: [PATCH net-next] udp: Add tracepoint for udp_sendmsg()

[David Ahern]

On 4/17/25 5:42 AM, Breno Leitao wrote:
> Hello David,
> 
> On Wed, Apr 16, 2025 at 04:16:26PM -0700, David Ahern wrote:
>> On 4/16/25 1:23 PM, Breno Leitao wrote:
>>> diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
>>> index f9f5b92cf4b61..8c2902504a399 100644
>>> --- a/net/ipv4/udp.c
>>> +++ b/net/ipv4/udp.c
>>> @@ -1345,6 +1345,8 @@ int udp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
>>>  		connected = 1;
>>>  	}
>>>  
>>> +	trace_udp_sendmsg_tp(sk, msg);
>>
>> why `_tp` suffix? the usual naming is trace_${func}
> 
> I got the impression that DECLARE_TRACE() raw tracepoints names end up
> in _tp():
> 
> 	include/trace/events/tcp.h
> 	DECLARE_TRACE(tcp_cwnd_reduction_tp,

that is the only networking one:

$ git grep trace_ net drivers/net | grep _tp
net/bpf/test_run.c:	trace_bpf_trigger_tp(nonce);
net/ipv4/tcp_input.c:	trace_tcp_cwnd_reduction_tp(sk,
newly_acked_sacked, newly_lost, flag);

The rest follow do not have the _tp suffix:

$ git grep trace_ net drivers/net | wc -l
    2727

2725 without; 2 with

Re: [PATCH net-next] udp: Add tracepoint for udp_sendmsg()

[Breno Leitao]

On Thu, Apr 17, 2025 at 08:55:27AM -0700, David Ahern wrote:
> On 4/17/25 5:42 AM, Breno Leitao wrote:
> > Hello David,
> > 
> > On Wed, Apr 16, 2025 at 04:16:26PM -0700, David Ahern wrote:
> >> On 4/16/25 1:23 PM, Breno Leitao wrote:
> >>> diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
> >>> index f9f5b92cf4b61..8c2902504a399 100644
> >>> --- a/net/ipv4/udp.c
> >>> +++ b/net/ipv4/udp.c
> >>> @@ -1345,6 +1345,8 @@ int udp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
> >>>  		connected = 1;
> >>>  	}
> >>>  
> >>> +	trace_udp_sendmsg_tp(sk, msg);
> >>
> >> why `_tp` suffix? the usual naming is trace_${func}
> > 
> > I got the impression that DECLARE_TRACE() raw tracepoints names end up
> > in _tp():
> > 
> > 	include/trace/events/tcp.h
> > 	DECLARE_TRACE(tcp_cwnd_reduction_tp,
> 
> that is the only networking one:
> 
> $ git grep trace_ net drivers/net | grep _tp
> net/bpf/test_run.c:	trace_bpf_trigger_tp(nonce);
> net/ipv4/tcp_input.c:	trace_tcp_cwnd_reduction_tp(sk,
> newly_acked_sacked, newly_lost, flag);

Do we want to rename them and remove the _tp? I suppose it is OK given
that tracepoints are not expected to be stable?

Also, if we have consensus about this patch, I will remove the _tp from
it.

Thanks!
--breno

Re: [PATCH net-next] udp: Add tracepoint for udp_sendmsg()

[David Ahern]

On 4/17/25 10:00 AM, Breno Leitao wrote:
>> $ git grep trace_ net drivers/net | grep _tp
>> net/bpf/test_run.c:	trace_bpf_trigger_tp(nonce);
>> net/ipv4/tcp_input.c:	trace_tcp_cwnd_reduction_tp(sk,
>> newly_acked_sacked, newly_lost, flag);
> 
> Do we want to rename them and remove the _tp? I suppose it is OK given
> that tracepoints are not expected to be stable?
> 
> Also, if we have consensus about this patch, I will remove the _tp from
> it.
> 

I am only asking for consistency. Based on existing networking
instances, consistency is no _tp suffix.

Re: [PATCH net-next] udp: Add tracepoint for udp_sendmsg()

[Steven Rostedt]

On Thu, 17 Apr 2025 21:57:56 -0700
David Ahern <dsahern@kernel.org> wrote:

> On 4/17/25 10:00 AM, Breno Leitao wrote:
> >> $ git grep trace_ net drivers/net | grep _tp
> >> net/bpf/test_run.c:	trace_bpf_trigger_tp(nonce);
> >> net/ipv4/tcp_input.c:	trace_tcp_cwnd_reduction_tp(sk,
> >> newly_acked_sacked, newly_lost, flag);  
> > 
> > Do we want to rename them and remove the _tp? I suppose it is OK given
> > that tracepoints are not expected to be stable?
> > 
> > Also, if we have consensus about this patch, I will remove the _tp from
> > it.
> >   
> 
> I am only asking for consistency. Based on existing networking
> instances, consistency is no _tp suffix.

I was looking at what other tracepoints have "_tp" and found a few. What it
appears to be is that the "_tp" tracepoints are defined by:

  DECLARE_TRACEPOINT()

and have no corresponding trace event in tracefs (/sys/kernel/tracing/events).

I like that distinction because it lets the developer know that this
tracepoint is in kernel only, and not exposed to user space.

Perhaps it should stay as "_tp()" if it's not exposed via tracefs.

In fact, if there is a clean up, it should be adding "_tp" to all
tracepoints that do not have a corresponding trace event attached to them.
As they are in kernel only, that change should not cause any ABI breakage.

-- Steve

Re: [PATCH net-next] udp: Add tracepoint for udp_sendmsg()

[Steven Rostedt]

On Fri, 18 Apr 2025 08:33:51 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> In fact, if there is a clean up, it should be adding "_tp" to all
> tracepoints that do not have a corresponding trace event attached to them.
> As they are in kernel only, that change should not cause any ABI breakage.

Actually, I think I'll make it where tracepoints created via
DECLARE_TRACE() will automatically get the "_tp" ending. That would help
enforce this API.

-- Steve

Re: [PATCH net-next] udp: Add tracepoint for udp_sendmsg()

[Paolo Abeni]

On 4/16/25 9:23 PM, Breno Leitao wrote:
> Add a lightweight tracepoint to monitor UDP send message operations,
> similar to the recently introduced tcp_sendmsg_locked() trace event in
> commit 0f08335ade712 ("trace: tcp: Add tracepoint for
> tcp_sendmsg_locked()")

Why is it needed? what would add on top of a plain perf probe, which
will be always available for such function with such argument, as the
function can't be inlined?

Thanks,

Paolo

Re: [PATCH net-next] udp: Add tracepoint for udp_sendmsg()

[Breno Leitao]

Hello Paolo,

On Thu, Apr 17, 2025 at 08:57:24AM +0200, Paolo Abeni wrote:
> On 4/16/25 9:23 PM, Breno Leitao wrote:
> > Add a lightweight tracepoint to monitor UDP send message operations,
> > similar to the recently introduced tcp_sendmsg_locked() trace event in
> > commit 0f08335ade712 ("trace: tcp: Add tracepoint for
> > tcp_sendmsg_locked()")
> 
> Why is it needed? what would add on top of a plain perf probe, which
> will be always available for such function with such argument, as the
> function can't be inlined?

Why this function can't be inlined? I got the impression that this
funciton could be, at least, partially inlined. Mainly when generating
ultra optimized kernels (i.e, kernels compiled with PGO and LTO features
enabled).

Thanks,
--breno

Re: [PATCH net-next] udp: Add tracepoint for udp_sendmsg()

[Paolo Abeni]

On 4/17/25 1:34 PM, Breno Leitao wrote:
> On Thu, Apr 17, 2025 at 08:57:24AM +0200, Paolo Abeni wrote:
>> On 4/16/25 9:23 PM, Breno Leitao wrote:
>>> Add a lightweight tracepoint to monitor UDP send message operations,
>>> similar to the recently introduced tcp_sendmsg_locked() trace event in
>>> commit 0f08335ade712 ("trace: tcp: Add tracepoint for
>>> tcp_sendmsg_locked()")
>>
>> Why is it needed? what would add on top of a plain perf probe, which
>> will be always available for such function with such argument, as the
>> function can't be inlined?
> 
> Why this function can't be inlined? 

Because the kernel need to be able find a pointer to it:

	.sendmsg		= udp_sendmsg,

I'll be really curious to learn how the compiler could inline that.

/P

Re: [PATCH net-next] udp: Add tracepoint for udp_sendmsg()

[Song Liu]

Hi Paolo, 

> On Apr 17, 2025, at 6:17 AM, Paolo Abeni <pabeni@redhat.com> wrote:
> 
> On 4/17/25 1:34 PM, Breno Leitao wrote:
>> On Thu, Apr 17, 2025 at 08:57:24AM +0200, Paolo Abeni wrote:
>>> On 4/16/25 9:23 PM, Breno Leitao wrote:
>>>> Add a lightweight tracepoint to monitor UDP send message operations,
>>>> similar to the recently introduced tcp_sendmsg_locked() trace event in
>>>> commit 0f08335ade712 ("trace: tcp: Add tracepoint for
>>>> tcp_sendmsg_locked()")
>>> 
>>> Why is it needed? what would add on top of a plain perf probe, which
>>> will be always available for such function with such argument, as the
>>> function can't be inlined?
>> 
>> Why this function can't be inlined?
> 
> Because the kernel need to be able find a pointer to it:
> 
> .sendmsg = udp_sendmsg,
> 
> I'll be really curious to learn how the compiler could inline that.

It is true that functions that are only used via function pointers
will not be inlined by compilers (at least for those we have tested).
For this reason, we do not worry about functions in various
tcp_congestion_ops. However, udp_sendmsg is also called directly
by udpv6_sendmsg, so it can still get inlined by LTO. 

Thanks,
Song

Re: [PATCH net-next] udp: Add tracepoint for udp_sendmsg()

[Willem de Bruijn]

Song Liu wrote:
> Hi Paolo, 
> 
> > On Apr 17, 2025, at 6:17 AM, Paolo Abeni <pabeni@redhat.com> wrote:
> > 
> > On 4/17/25 1:34 PM, Breno Leitao wrote:
> >> On Thu, Apr 17, 2025 at 08:57:24AM +0200, Paolo Abeni wrote:
> >>> On 4/16/25 9:23 PM, Breno Leitao wrote:
> >>>> Add a lightweight tracepoint to monitor UDP send message operations,
> >>>> similar to the recently introduced tcp_sendmsg_locked() trace event in
> >>>> commit 0f08335ade712 ("trace: tcp: Add tracepoint for
> >>>> tcp_sendmsg_locked()")
> >>> 
> >>> Why is it needed? what would add on top of a plain perf probe, which
> >>> will be always available for such function with such argument, as the
> >>> function can't be inlined?
> >> 
> >> Why this function can't be inlined?
> > 
> > Because the kernel need to be able find a pointer to it:
> > 
> > .sendmsg = udp_sendmsg,
> > 
> > I'll be really curious to learn how the compiler could inline that.
> 
> It is true that functions that are only used via function pointers
> will not be inlined by compilers (at least for those we have tested).
> For this reason, we do not worry about functions in various
> tcp_congestion_ops. However, udp_sendmsg is also called directly
> by udpv6_sendmsg, so it can still get inlined by LTO. 
> 
> Thanks,
> Song
> 

I would think that hitting this tracepoint for ipv6_addr_v4mapped
addresses is unintentional and surprising, as those would already
hit udpv6_sendmsg.

On which note, any IPv4 change to UDP needs an equivalent IPv6 one.

Re: [PATCH net-next] udp: Add tracepoint for udp_sendmsg()

[Song Liu]

> On Apr 17, 2025, at 8:48 AM, Willem de Bruijn <willemdebruijn.kernel@gmail.com> wrote:
> 
> Song Liu wrote:
>> Hi Paolo, 
>> 
>>> On Apr 17, 2025, at 6:17 AM, Paolo Abeni <pabeni@redhat.com> wrote:
>>> 
>>> On 4/17/25 1:34 PM, Breno Leitao wrote:
>>>> On Thu, Apr 17, 2025 at 08:57:24AM +0200, Paolo Abeni wrote:
>>>>> On 4/16/25 9:23 PM, Breno Leitao wrote:
>>>>>> Add a lightweight tracepoint to monitor UDP send message operations,
>>>>>> similar to the recently introduced tcp_sendmsg_locked() trace event in
>>>>>> commit 0f08335ade712 ("trace: tcp: Add tracepoint for
>>>>>> tcp_sendmsg_locked()")
>>>>> 
>>>>> Why is it needed? what would add on top of a plain perf probe, which
>>>>> will be always available for such function with such argument, as the
>>>>> function can't be inlined?
>>>> 
>>>> Why this function can't be inlined?
>>> 
>>> Because the kernel need to be able find a pointer to it:
>>> 
>>> .sendmsg = udp_sendmsg,
>>> 
>>> I'll be really curious to learn how the compiler could inline that.
>> 
>> It is true that functions that are only used via function pointers
>> will not be inlined by compilers (at least for those we have tested).
>> For this reason, we do not worry about functions in various
>> tcp_congestion_ops. However, udp_sendmsg is also called directly
>> by udpv6_sendmsg, so it can still get inlined by LTO. 
>> 
>> Thanks,
>> Song
>> 
> 
> I would think that hitting this tracepoint for ipv6_addr_v4mapped
> addresses is unintentional and surprising, as those would already
> hit udpv6_sendmsg.

It is up to the user to decide how these tracepoints should be 
used. For example, the user may only be interested in 
udpv6_sendmsg => udp_sendmsg case. Without a tracepoint, the user
has to understand whether the compiler inlined this function. 

> 
> On which note, any IPv4 change to UDP needs an equivalent IPv6 one.

Do you mean we need to also add tracepoints for udpv6_sendmsg?

Thanks,
Song

Re: [PATCH net-next] udp: Add tracepoint for udp_sendmsg()

[Willem de Bruijn]

Song Liu wrote:
> 
> 
> > On Apr 17, 2025, at 8:48 AM, Willem de Bruijn <willemdebruijn.kernel@gmail.com> wrote:
> > 
> > Song Liu wrote:
> >> Hi Paolo, 
> >> 
> >>> On Apr 17, 2025, at 6:17 AM, Paolo Abeni <pabeni@redhat.com> wrote:
> >>> 
> >>> On 4/17/25 1:34 PM, Breno Leitao wrote:
> >>>> On Thu, Apr 17, 2025 at 08:57:24AM +0200, Paolo Abeni wrote:
> >>>>> On 4/16/25 9:23 PM, Breno Leitao wrote:
> >>>>>> Add a lightweight tracepoint to monitor UDP send message operations,
> >>>>>> similar to the recently introduced tcp_sendmsg_locked() trace event in
> >>>>>> commit 0f08335ade712 ("trace: tcp: Add tracepoint for
> >>>>>> tcp_sendmsg_locked()")
> >>>>> 
> >>>>> Why is it needed? what would add on top of a plain perf probe, which
> >>>>> will be always available for such function with such argument, as the
> >>>>> function can't be inlined?
> >>>> 
> >>>> Why this function can't be inlined?
> >>> 
> >>> Because the kernel need to be able find a pointer to it:
> >>> 
> >>> .sendmsg = udp_sendmsg,
> >>> 
> >>> I'll be really curious to learn how the compiler could inline that.
> >> 
> >> It is true that functions that are only used via function pointers
> >> will not be inlined by compilers (at least for those we have tested).
> >> For this reason, we do not worry about functions in various
> >> tcp_congestion_ops. However, udp_sendmsg is also called directly
> >> by udpv6_sendmsg, so it can still get inlined by LTO. 
> >> 
> >> Thanks,
> >> Song
> >> 
> > 
> > I would think that hitting this tracepoint for ipv6_addr_v4mapped
> > addresses is unintentional and surprising, as those would already
> > hit udpv6_sendmsg.
> 
> It is up to the user to decide how these tracepoints should be 
> used. For example, the user may only be interested in 
> udpv6_sendmsg => udp_sendmsg case. Without a tracepoint, the user
> has to understand whether the compiler inlined this function. 
> 
> > 
> > On which note, any IPv4 change to UDP needs an equivalent IPv6 one.
> 
> Do you mean we need to also add tracepoints for udpv6_sendmsg?

If there is consensus that a tracepoint at this point is valuable,
then it should be supported equally for IPv4 and IPv6.

That holds true for all such hooks. No IPv4 only.

Re: [PATCH net-next] udp: Add tracepoint for udp_sendmsg()

[Breno Leitao]

Hello,

On Fri, Apr 18, 2025 at 10:49:22AM -0400, Willem de Bruijn wrote:
> Song Liu wrote:
> > Do you mean we need to also add tracepoints for udpv6_sendmsg?
> 
> If there is consensus that a tracepoint at this point is valuable,
> then it should be supported equally for IPv4 and IPv6.
> 
> That holds true for all such hooks. No IPv4 only.

Revamping this thread to make sure we have a conclusion.

Any objection for not having the tracepoint in UDP sendmsg, both IPv4
and IPv6?

Thanks
--breno