netdev - Re: [PATCH net-next v4 07/11] net-timestamp: support hwtstamp print for bpf extension

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: <eba3a270-7d0d-424d-91be-296224d51539@linux.dev>
Date: Thu, 12 Dec 2024 22:28:41 -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, bpf@...r.kernel.org,
 netdev@...r.kernel.org, Jason Xing <kernelxing@...cent.com>
Subject: Re: [PATCH net-next v4 07/11] net-timestamp: support hwtstamp print
 for bpf extension

On 12/12/24 3:25 PM, Martin KaFai Lau wrote:
> A more subtle thing for the hwtstamps case is, afaik the bpf prog will not be 
> called. All drivers are still only testing SKBTX_HW_TSTAMP instead of testing
> (SKBTX_HW_TSTAMP | SKBTX_BPF).
> 
> There are a lot of drivers to change though. A quick thought is to rename the 
> existing SKBTX_HW_TSTAMP (e.g. __SKBTX_HW_TSTAMP = 1 << 0) and define 
> SKBTX_HW_TSTAMP like:
> 
> #define SKBTX_HW_TSTAMP (__SKBTX_HW_TSTAMP | SKBTX_BPF)
> 
> Then change some of the existing skb_shinfo(skb)->tx_flags "setting" site to use 
> __SKBTX_HW_TSTAMP instead. e.g. in __sock_tx_timestamp(). Not very pretty but 
> may be still better than changing many drivers. May be there is a better way...
> 
> While talking about where to test the SKBTX_BPF bit, I wonder if the new 
> skb_tstamp_is_set() is needed. For the non SKBTX_HW_TSTAMP case, the number of 
> tx_flags testing sites should be limited, so should be easy to add the SKBTX_BPF 
> bit test. e.g. at the __dev_queue_xmit, test "if (unlikely(skb_shinfo(skb)- 
>  >tx_flags & (SKBTX_SCHED_TSTAMP | SKBTX_BPF)))". Patch 6 has also tested the 
> bpf specific bit at tcp_ack_tstamp() before calling the __skb_tstamp_tx().
> 
> At the beginning of __skb_tstamp_tx(), do something like this:
> 
> void __skb_tstamp_tx(struct sk_buff *orig_skb,
>               const struct sk_buff *ack_skb,
>               struct skb_shared_hwtstamps *hwtstamps,
>               struct sock *sk, int tstype)
> {
>      if (cgroup_bpf_enabled(CGROUP_SOCK_OPS) &&
>          unlikely(skb_shinfo(skb)->tx_flags & SKBTX_BPF))
>          __skb_tstamp_tx_bpf(sk, orig_skb, hwtstamps, tstype);
> 
>      if (unlikely(!(skb_shinfo(skb)->tx_flags & ~SKBTX_BPF)))

This is not enough. I was wrong here. The test in skb_tstamp_is_set() is needed 
when SKBTX_BPF is not set.

>          return;