On Fri, Nov 1, 2024 at 7:50 AM Martin KaFai Lau <martin.lau@xxxxxxxxx> wrote: > > On 10/30/24 7:41 PM, Jason Xing wrote: > >> bpf prog cannot directly access the skops->skb now. It is because the sockops > >> prog sees the uapi "struct bpf_sock_ops" instead of "struct > >> bpf_sock_ops(_kern)". The conversion is done in sock_ops_convert_ctx_access. It > >> is an old way before BTF. I don't want to extend the uapi "struct bpf_sock_ops". > > > > Oh, so it seems we cannot use this way, right? > > No. don't extend the uapi "struct bpf_sock_ops". Use bpf_cast_to_kern_ctx() instead. Got it! > > > > > I also noticed a use case that allow users to get the information from one skb: > > "int BPF_PROG(trace_netif_receive_skb, struct sk_buff *skb)" in > > tools/testing/selftests/bpf/progs/netif_receive_skb.c > > But it requires us to add the tracepoint in __skb_tstamp_tx() first. > > Two months ago, I was planning to use a tracepoint for some people who > > find it difficult to deploy bpf. > > > It is a tracing prog instead of sockops prog. The verifier allows accessing > different things based on the program type. This patch set is using the sockops > bpf prog type which is not a tracing prog. Tracing can do a lot of read-only > things but here we need write (e.g. bpf_setsockopt), so tracing prog is not > suitable here. Thanks for the explaination. > > > > >> > >> Instead, use bpf_cast_to_kern_ctx((struct bpf_sock_ops *)skops_ctx) to get a > >> trusted "struct bpf_sock_ops(_kern) *skops" pointer. Then it can access the > >> skops->skb. > > > > Let me spend some time on it. Thanks. > > Take a look at the bpf_cast_to_kern_ctx() examples in selftests/bpf. I think > this can be directly used to get to (struct bpf_sock_ops_kern *)skops->skb. Ping > back if your selftest bpf prog cannot load. No problem :) > > > > >> afaik, the tcb->seq should be available already during sendmsg. it > >> should be able to get it from TCP_SKB_CB(skb)->seq with the bpf_core_cast. Take > >> a look at the existing examples of bpf_core_cast. > >> > >> The same goes for the skb->data. It can use the bpf_dynptr_from_skb(). It is not > >> available to skops program now but should be easy to expose. > > I wonder what the use of skb->data is here. > > You are right, not needed. I was thinking it may need to parse the tcp header > from the skb at the rx timestamping. It is not needed. The tcp stack should have > already parsed it and TCP_SKB_CB can be directly used as long as the sockops > prog can get to the skops->skb. Agreed. > > >> > >> In the bpf prog, when the SCHED/SND/ACK timestamp comes back, it has to find the > >> earlier sendmsg timestamp. One option is to store the earlier sendmsg timestamp > >> at the bpf map key-ed by seqno or the shinfo's tskey. Storing in a bpf map > >> key-ed by seqno/tskey is probably what the selftest should do. In the future, we > >> can consider allowing the rbtree in the bpf sk local storage for searching > >> seqno. There is shinfo's hwtstamp that can be used also if there is a need. > > > > Thanks for the information! Let me investigate how the bpf map works... > > > > I wonder that for the selftests could it be much simpler if we just > > record each timestamp stored in three variables and calculate them at > > last since we only send the small packet once instead of using bpf > > map. I mean, bpf map is really good as far as I know, but I'm a bit > > worried that implementing such a function could cause more extra work > > (implementation and review). > > Don't worry on the review side. imo, a closer to the real world selftest prog is > actually helping the review process. It needs to test the tskey anyway and it > needs to store somewhere. bpf map is pretty simple to use. I don't think it will > have much different in term of complexity also. Got it, will do it soon :) Thanks, Jason
