Re: [PATCH net-next] net: tcp: add tracepoint skb_latency for latency monitor

[Menglong Dong]

On Wed, Oct 9, 2024 at 11:53 PM Eric Dumazet <edumazet@xxxxxxxxxx> wrote:
>
> On Wed, Oct 9, 2024 at 2:17 PM Menglong Dong <menglong8.dong@xxxxxxxxx> wrote:
> >
> > In this commit, we introduce a new tracepoint "skb_latency", which is
> > used to trace the latency on sending or receiving packet. For now, only
> > TCP is supported. Maybe we should call it "tcp_latency"?
> >
> > There are 6 stages are introduced in this commit to trace the networking
> > latency.
> >
> > The existing SO_TIMESTAMPING and MSG_TSTAMP_* can obtain the timestamping
> > of sending and receiving packet, but it's not convenient.
> >
> > First, most applications didn't use this function when implement, and we
> > can't make them implement it right now when networking latency happens.
> >
> > Second, it's inefficient, as it need to get the timestamping from the
> > error queue with syscalls.
> >
> > Third, the timestamping it offers is not enough to analyse the latency
> > on sending or receiving packet.
> >
> > As for me, the main usage of this tracepoint is to be hooked by my BPF
> > program, and do some filter, and capture the latency that I interested
> > in.
> >
> > Signed-off-by: Menglong Dong <dongml2@xxxxxxxxxxxxxxx>
> > ---
>
> Big NACK from my side.
>
> Adding tcp_mstamp_refresh() all over the place is not what I call 'tracing'.

The tcp_mstamp_refresh() is optional, as we can update skb->tstamp with
tcp_clock_ns() directly instead (maybe with a control of the static branch).

As @Vadim says, @Jason Xing already has a similar series on the
networking latency monitor before, which is based on BPF.

(what a coincidence :/)

So let's focus on Jason's series.

Thanks!
Menglong Dong