Received-SPF: pass (google.com: domain of linux-kernel+bounces-86234-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Precedence: bulk
MIME-Version: 1.0
References: <20240229052813.GA23899@didi-ThinkCentre-M920t-N000>
In-Reply-To: <20240229052813.GA23899@didi-ThinkCentre-M920t-N000>
From: Jason Xing <kerneljasonxing@gmail.com>
Date: Thu, 29 Feb 2024 15:29:56 +0800
Message-ID: <CAL+tcoAhvFhXdr1WQU8mv_6ZX5nOoNpbOLAB6=C+DB-qXQ11Ew@mail.gmail.com>
Subject: Re: [PATCH] tcp: Add skb addr and sock addr to arguments of
 tracepoint tcp_probe.
To: edumazet@google.com, rostedt@goodmis.org, mhiramat@kernel.org, 
	mathieu.desnoyers@efficios.com, netdev@vger.kernel.org, 
	linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org, 
	fuyuanli@didiglobal.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Thu, Feb 29, 2024 at 1:33=E2=80=AFPM fuyuanli <fuyuanli@didiglobal.com> =
wrote:
>
> It is useful to expose skb addr and sock addr to user in tracepoint
> tcp_probe, so that we can get more information while monitoring
> receiving of tcp data, by ebpf or other ways.
>
> For example, we need to identify a packet by seq and end_seq when
> calculate transmit latency between lay 2 and lay 4 by ebpf, but which is
> not available in tcp_probe, so we can only use kprobe hooking
> tcp_rcv_esatblised to get them. But we can use tcp_probe directly if skb
> addr and sock addr are available, which is more efficient.
>
> Signed-off-by: fuyuanli <fuyuanli@didiglobal.com>

Please target 'net-next' in the title of your v2 patch.

> ---
>  include/trace/events/tcp.h | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/include/trace/events/tcp.h b/include/trace/events/tcp.h
> index 7b1ddffa3dfc..096c15f64b92 100644
> --- a/include/trace/events/tcp.h
> +++ b/include/trace/events/tcp.h
> @@ -258,6 +258,8 @@ TRACE_EVENT(tcp_probe,
>                 __field(__u32, srtt)
>                 __field(__u32, rcv_wnd)
>                 __field(__u64, sock_cookie)
> +               __field(const void *, skbaddr)
> +               __field(const void *, skaddr)
>         ),
>
>         TP_fast_assign(
> @@ -285,6 +287,9 @@ TRACE_EVENT(tcp_probe,
>                 __entry->ssthresh =3D tcp_current_ssthresh(sk);
>                 __entry->srtt =3D tp->srtt_us >> 3;
>                 __entry->sock_cookie =3D sock_gen_cookie(sk);
> +
> +               __entry->skbaddr =3D skb;
> +               __entry->skaddr =3D sk;
>         ),
>
>         TP_printk("family=3D%s src=3D%pISpc dest=3D%pISpc mark=3D%#x data=
_len=3D%d snd_nxt=3D%#x snd_una=3D%#x snd_cwnd=3D%u ssthresh=3D%u snd_wnd=
=3D%u srtt=3D%u rcv_wnd=3D%u sock_cookie=3D%llx",

If they are useful, at least you should printk those two addresses
like what trace_kfree_skb() does.

May I ask how it could be useful if there is no more function printing
such information in the receive path?

Thanks,
Jason
> --
> 2.17.1
>
>