On Tue, Jan 21, 2025 at 9:15 AM Jason Xing <[email protected]> wrote: > > 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.
Before BPF gets widely used, changing function names will not hurt applications. Things are a little bit different now, changing names or inlined function will lead the monitor application adjusting codes through versions, because they are not kabi functions. Thanks, Jason
