netdev - Re: [PATCH net-next v5 13/15] net-timestamp: support tcp

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] [day] [month] [year] [list]

Message-ID: <CAL+tcoBu=5Ub_5E3HNK6uub4MiHEOpRCgtWMRQX3heKObM9rHA@mail.gmail.com>
Date: Thu, 16 Jan 2025 09:22:33 +0800
From: Jason Xing <kerneljasonxing@...il.com>
To: Martin KaFai Lau <martin.lau@...ux.dev>
Cc: davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org, 
	pabeni@...hat.com, dsahern@...nel.org, willemdebruijn.kernel@...il.com, 
	willemb@...gle.com, ast@...nel.org, daniel@...earbox.net, andrii@...nel.org, 
	eddyz87@...il.com, song@...nel.org, yonghong.song@...ux.dev, 
	john.fastabend@...il.com, kpsingh@...nel.org, sdf@...ichev.me, 
	haoluo@...gle.com, jolsa@...nel.org, horms@...nel.org, bpf@...r.kernel.org, 
	netdev@...r.kernel.org
Subject: Re: [PATCH net-next v5 13/15] net-timestamp: support tcp_sendmsg for
 bpf extension

On Thu, Jan 16, 2025 at 9:18 AM Martin KaFai Lau <martin.lau@...ux.dev> wrote:
>
> On 1/15/25 4:41 PM, Jason Xing wrote:
> >>> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> >>> index a0aff1b4eb61..87420c0f2235 100644
> >>> --- a/include/uapi/linux/bpf.h
> >>> +++ b/include/uapi/linux/bpf.h
> >>> @@ -7037,6 +7037,9 @@ enum {
> >>>                                         * feature is on. It indicates the
> >>>                                         * recorded timestamp.
> >>>                                         */
> >>> +     BPF_SOCK_OPS_TS_TCP_SND_CB,     /* Called when every tcp_sendmsg
> >>> +                                      * syscall is triggered
> >>> +                                      */
> >>
> >> UDP will need this also?
> >
> > Yep.
>
> Then the TCP naming will need to be adjusted.

Right, right!

>
> While on UDP, how the UDP bpf callback will look like during sendmsg?
>
> >>> @@ -1067,10 +1068,15 @@ int tcp_sendmsg_locked(struct sock *sk, struct msghdr *msg, size_t size)
> >>>        int flags, err, copied = 0;
> >>>        int mss_now = 0, size_goal, copied_syn = 0;
> >>>        int process_backlog = 0;
> >>> +     u32 first_write_seq = 0;
> >>>        int zc = 0;
> >>>        long timeo;
> >>>
> >>>        flags = msg->msg_flags;
> >>> +     if (SK_BPF_CB_FLAG_TEST(sk, SK_BPF_CB_TX_TIMESTAMPING)) {
> >>> +             first_write_seq = tp->write_seq;
> >>> +             bpf_skops_tx_timestamping(sk, NULL, BPF_SOCK_OPS_TS_TCP_SND_CB);
> >>
> >> My preference is to skip this bpf callout for now and depends on a bpf trace
> >> program if it is really needed.
> >
> > I have no idea if the bpf program wants to record the timestamp here
> > without the above three lines? Please enlighten me more. Thanks in
> > advance.
> >
> > I guess there is one way which I don't know yet to monitor at the
> > beginning of tcp_sendmsg_locked().
>
> The tracing bpf program (fentry in particular here). Give the one-liner bpftrace
> script a try.
>
> Take a look at trace_tcp_connect in test_sk_storage_tracing.c. It uses fentry
> and also bpf_sk_storage_get.

Thanks for the reference!

>
> If tcp_sendmsg_locked is inline-d, it can go up to the tcp_sendmsg(). It would
> be nice to have a stable bpf callback if it is really useful but I suspect this
> can be revisited later with the UDP support.

Got it!

>
> [ I will followup other replies later. ]
>

Thank you!