Re: [PATCH net-next v5 13/15] net-timestamp: support tcp_sendmsg for bpf extension

[Martin KaFai Lau <martin.lau@linux.dev>]

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. ]