On Sat, Jan 25, 2025 at 8:28 AM Jason Xing <kerneljasonxing@xxxxxxxxx> wrote: > > On Sat, Jan 25, 2025 at 7:41 AM Martin KaFai Lau <martin.lau@xxxxxxxxx> wrote: > > > > On 1/20/25 5:28 PM, Jason Xing wrote: > > > Applying the new member allow_tcp_access in the existing callbacks > > > where is_fullsock is set to 1 can help us stop UDP socket accessing > > > struct tcp_sock, or else it could be catastrophe leading to panic. > > > > > > For now, those existing callbacks are used only for TCP. I believe > > > in the short run, we will have timestamping UDP callbacks support. > > > > The commit message needs adjustment. UDP is not supported yet, so this change > > feels like it's unnecessary based on the commit message. However, even without > > UDP support, the new timestamping callbacks cannot directly write some fields > > because the sk lock is not held, so this change is needed for TCP timestamping > > Thanks and I will revise them. But I still want to say that the > timestamping callbacks after this series are all under the protection > of socket lock. For the record only, I was wrong about the understanding of socket lock like above because there remains cases where this kind of path, say, i40e_intr()->i40e_ptp_tx_hwtstamp()->skb_tstamp_tx()->__skb_tstamp_tx(), will not be protected under the socket lock. With that said, directly accessing tcp_sock is not safe even if the socket type is TCP. Thanks, Jason