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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <d9601490-26f6-4aaf-80f0-0c92464e0c42@linux.dev>
Date: Wed, 15 Jan 2025 17:18:16 -0800
From: Martin KaFai Lau <martin.lau@...ux.dev>
To: Jason Xing <kerneljasonxing@...il.com>
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 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.

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.

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.

[ I will followup other replies later. ]