On Sat, Jan 25, 2025 at 10:37 AM Martin KaFai Lau <martin.lau@xxxxxxxxx> wrote: > > On 1/24/25 5:35 PM, Jason Xing wrote: > > On Sat, Jan 25, 2025 at 9:30 AM Martin KaFai Lau <martin.lau@xxxxxxxxx> wrote: > >> > >> On 1/24/25 5:18 PM, Jason Xing wrote: > >>>>> @@ -5577,9 +5578,9 @@ static void skb_tstamp_tx_bpf(struct sk_buff *skb, struct sock *sk, > >>>>> op = BPF_SOCK_OPS_TS_SCHED_OPT_CB; > >>>>> break; > >>>>> case SCM_TSTAMP_SND: > >>>>> + op = sw ? BPF_SOCK_OPS_TS_SW_OPT_CB : BPF_SOCK_OPS_TS_HW_OPT_CB; > >>>>> if (!sw) > >>>>> - return; > >>>>> - op = BPF_SOCK_OPS_TS_SW_OPT_CB; > >>>>> + *skb_hwtstamps(skb) = *hwtstamps; > >>>> hwtstamps may still be NULL, no? > >>> Right, it can be zero if something wrong happens. > >> > >> Then it needs a NULL check, no? > > > > My original intention is passing whatever to the userspace, so the bpf > > program will be aware of what is happening in the kernel. > > This is fine. > > > Passing NULL to hwstamps is right which will not cause any problem, I think. > > > > Do you mean the default value of hwstamps itself is NULL so in this > > case we don't need to re-init it to NULL again? > > > > Like this: > > If (*hwtstamps) > if (hwtstamps) instead ? > > I don't know. If hwtstamps is NULL, doing *hwtstamps will be bad and oops.... > May be my brain doesn't work well at the end of Friday. Please check. Thanks for your effort, Martin! I will deliberately inject this error case and then see what will happen. Thanks, Jason
