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.