On Sat, Jan 18, 2025 at 8:47 AM Martin KaFai Lau <martin.lau@xxxxxxxxx> wrote: > > On 1/15/25 3:56 PM, Jason Xing wrote: > > On Thu, Jan 16, 2025 at 6:48 AM Martin KaFai Lau <martin.lau@xxxxxxxxx> wrote: > >> > >> On 1/12/25 3:37 AM, Jason Xing wrote: > >>> Support SCM_TSTAMP_SND case. Then we will get the software > >>> timestamp when the driver is about to send the skb. Later, I > >>> will support the hardware timestamp. > >> > >>> diff --git a/net/core/skbuff.c b/net/core/skbuff.c > >>> index 169c6d03d698..0fb31df4ed95 100644 > >>> --- a/net/core/skbuff.c > >>> +++ b/net/core/skbuff.c > >>> @@ -5578,6 +5578,9 @@ static void __skb_tstamp_tx_bpf(struct sk_buff *skb, struct sock *sk, int tstype > >>> case SCM_TSTAMP_SCHED: > >>> op = BPF_SOCK_OPS_TS_SCHED_OPT_CB; > >>> break; > >>> + case SCM_TSTAMP_SND: > >>> + op = BPF_SOCK_OPS_TS_SW_OPT_CB; > >> > >> For the hwtstamps case, is skb_hwtstamps(skb) set? From looking at a few > >> drivers, it does not look like it. I don't see the hwtstamps support in patch 10 > >> either. What did I miss ? > > > > Sorry, I missed adding a new flag, namely, BPF_SOCK_OPS_TS_HW_OPT_CB. > > I can also skip adding that new one and rename > > BPF_SOCK_OPS_TS_SW_OPT_CB accordingly for sw and hw cases if we > > finally decide to use hwtstamps parameter to distinguish two different > > cases. > > I think having a separate BPF_SOCK_OPS_TS_HW_OPT_CB is better considering your > earlier hwtstamps may be NULL comment. I don't see the drivers I looked at > passing NULL though but the comment of skb_tstamp_tx did say it may be NULL :/ Yep, I was trying not to rely on or trust the hardware/driver's implementation, or else it will let the bpf program fetch the software timestamp instead of hardware timestamp which will cause unexpected behaviour. After re-reading this part, I reckon that using this SKBTX_IN_PROGRESS flag is enough to recognize if we are in the hardware timestamping generation period. I will try this one since it requires much less modification. > > Regardless, afaict, skb_hwtstamps(skb) is still not set to the hwtstamps passed > by the driver here. The bpf prog is supposed to directly get the hwtstamps from > the skops->skb pointer. Right. I will init the skb_shinfo(skb)->hwtstamps first in this hw callback. Thanks, Jason
