On Sat, Jan 18, 2025 at 10:17 AM Martin KaFai Lau <martin.lau@xxxxxxxxx> wrote: > > On 1/17/25 5:58 PM, Jason Xing wrote: > >> On 1/15/25 5:12 PM, Jason Xing wrote: > >>>>> Also, I need to set allow_direct_access to one as long as there is > >>>>> "sock_ops.is_fullsock = 1;" in the existing callbacks. > >>>> Only set allow_direct_access when the sk is fullsock in the "existing" sockops > >>>> callback. > >>> Only "existing"? Then how can the bpf program access those members of > >>> the tcp socket structure in the current/new timestamping callbacks? > >> There is at least one sk write: > >> > >> case offsetof(struct bpf_sock_ops, sk_txhash): > >> SOCK_OPS_GET_OR_SET_FIELD(sk_txhash, sk_txhash, > >> struct sock, type); > >> > >> afaict, the kernel always writes sk->sk_txhash with the sk lock held. The new > >> timestamping callbacks cannot write because it does not hold the lock. > > Surely, I will handle the sk_txhash case as you suggested 🙂 > > to be clear, not setting the allow_tcp_access in the new timestamping cb should do. Right, I will only apply to the existing callbacks. I think your last email is pretty clear to me and dispelled my concern. Prior to this, I was worried about not being allowed to access struct tcp_sock in timestamping cb. Thanks for your guidance. Thanks, Jason
