netdev - Re: [PATCH RFC net-next] trace: tcp: Add tracepoint for tcp_cwnd

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAL+tcoC+hVZU2rx6vVQO_BjAAfjLXvqCOVDHkDoBHYuykqS1dg@mail.gmail.com>
Date: Tue, 21 Jan 2025 09:15:35 +0800
From: Jason Xing <kerneljasonxing@...il.com>
To: Breno Leitao <leitao@...ian.org>
Cc: Eric Dumazet <edumazet@...gle.com>, Steven Rostedt <rostedt@...dmis.org>, 
	Masami Hiramatsu <mhiramat@...nel.org>, Mathieu Desnoyers <mathieu.desnoyers@...icios.com>, 
	"David S. Miller" <davem@...emloft.net>, David Ahern <dsahern@...nel.org>, 
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Simon Horman <horms@...nel.org>, 
	netdev@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-trace-kernel@...r.kernel.org, kernel-team@...a.com, 
	Yonghong Song <yonghong.song@...ux.dev>, Song Liu <song@...nel.org>, 
	Martin KaFai Lau <martin.lau@...nel.org>
Subject: Re: [PATCH RFC net-next] trace: tcp: Add tracepoint for tcp_cwnd_reduction()

On Mon, Jan 20, 2025 at 9:20 PM Breno Leitao <leitao@...ian.org> wrote:
>
>
> On Mon, Jan 20, 2025 at 09:06:43PM +0800, Jason Xing wrote:
> > On Mon, Jan 20, 2025 at 9:02 PM Breno Leitao <leitao@...ian.org> wrote:
> > > On Mon, Jan 20, 2025 at 08:08:52PM +0800, Jason Xing wrote:
> > > > On Mon, Jan 20, 2025 at 8:03 PM Breno Leitao <leitao@...ian.org> wrote:
> > > > > diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> > > > > index 4811727b8a02258ec6fa1fd129beecf7cbb0f90e..fc88c511e81bc12ec57e8dc3e9185a920d1bd079 100644
> > > > > --- a/net/ipv4/tcp_input.c
> > > > > +++ b/net/ipv4/tcp_input.c
> > > > > @@ -2710,6 +2710,8 @@ void tcp_cwnd_reduction(struct sock *sk, int newly_acked_sacked, int newly_lost,
> > > > >         if (newly_acked_sacked <= 0 || WARN_ON_ONCE(!tp->prior_cwnd))
> > > > >                 return;
> > > > >
> > > > > +       trace_tcp_cwnd_reduction(sk, newly_acked_sacked, newly_lost, flag);
> > > > > +
> > > >
> > > > Are there any other reasons why introducing a new tracepoint here?
> > > > AFAIK, it can be easily replaced by a bpf related program or script to
> > > > monitor in the above position.
> > >
> > > In which position exactly?
> >
> > I meant, in the position where you insert a one-line tracepoint, which
> > should be easily replaced with a bpf program (kprobe
> > tcp_cwnd_reduction with two checks like in the earlier if-statement).
> > It doesn't mean that I object to this new tracepoint, just curious if
> > you have other motivations.
>
> This is exactly the current implementation we have at Meta, as it relies on
> hooking into this specific function. This approach is unstable, as
> compiler optimizations like inlining can break the functionality.
>
> This patch enhances the API's stability by introducing a guaranteed hook
> point, allowing the compiler to make changes without disrupting the
> BPF program's functionality.

Surely it does :) The reason why I asked is that perhaps one year ago
I'm trying to add many tracepoints so that the user space monitor can
take advantage of these various stable hook points. I believe that
there are many other places which might be inlined because of gcc,
receiving a few similar reports in recent months, but there is no
guarantee to not touch some functions which obviously break some
monitor applications.

I'm wondering that except for this new transepoint, if we can design a
common usage for the monitor program, so that people who are
interested will work on this together? Profiling is becoming more and
more important nowadays, from the point of my view.

Thanks,
Jason