On Mon, Jan 20, 2025 at 9:20 PM Breno Leitao <[email protected]> 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 <[email protected]> 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 <[email protected]> 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
