netdev - Re: [PATCH net-next v3 02/14] net-timestamp: allow two features to work parallelly

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

Message-ID: <d138a81d-f9f5-4d51-bedd-3916d377699d@linux.dev>
Date: Wed, 30 Oct 2024 23:27:13 -0700
From: Martin KaFai Lau <martin.lau@...ux.dev>
To: Jason Xing <kerneljasonxing@...il.com>,
 Willem de Bruijn <willemdebruijn.kernel@...il.com>
Cc: willemb@...gle.com, davem@...emloft.net, edumazet@...gle.com,
 kuba@...nel.org, pabeni@...hat.com, dsahern@...nel.org, 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, shuah@...nel.org,
 ykolal@...com, bpf@...r.kernel.org, netdev@...r.kernel.org,
 Jason Xing <kernelxing@...cent.com>
Subject: Re: [PATCH net-next v3 02/14] net-timestamp: allow two features to
 work parallelly

On 10/30/24 5:13 PM, Jason Xing wrote:
> I realized that we will have some new sock_opt flags like
> TS_SCHED_OPT_CB in patch 4, so we can control whether to print or
> not... For each sock_opt point, they will be called without caring if
> related flags in skb are set. Well, it's meaningless to add more
> control of skb tsflags at each TS_xx_OPT_CB point.
> 
> Am I understanding in a correct way? Now, I'm totally fine with this great idea!
Yes, I think so.

The sockops prog can choose to ignore any BPF_SOCK_OPS_TS_*_CB. The are only 3: 
SCHED, SND, and ACK. If the hwtstamp is available from a NIC, I think it would 
be quite wasteful to throw it away. ACK can be controlled by the 
TCP_SKB_CB(skb)->bpf_txstamp_ack.

Going back to my earlier bpf_setsockopt(SOL_SOCKET, BPF_TX_TIMESTAMPING) 
comment. I think it may as well go back to use the "u8 
bpf_sock_ops_cb_flags;" and use the bpf_sock_ops_cb_flags_set() helper to 
enable/disable the timestamp related callback hook. May be add one 
BPF_SOCK_OPS_TX_TIMESTAMPING_CB_FLAG.

For tx, one new hook should be at the sendmsg and should be around 
tcp_tx_timestamp (?) for tcp. Another hook is __skb_tstamp_tx() which should be 
similar to your patch. Add a new kfunc to set shinfo->tx_flags |= SKBTX_BPF 
and/or TCP_SKB_CB(skb)->bpf_txstamp_ack during sendmsg.

For rx, add one BPF_SOCK_OPS_RX_TIMESTAMPING_CB_FLAG. bpf_sock_ops_cb_flags 
needs to move from the tcp_sock to the sock because it will be used by UDP also. 
When enabling or disabling this flag, it needs to take care of the 
net_{enable,disable}_timestamp. The same for the __sk_destruct() also.