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. 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)
*skb_hwtstamps(skb) = *hwtstamps;
But it looks no different actually.
Thanks,
Jason
