On Sun, Oct 13, 2024 at 11:28 AM Jason Xing <kerneljasonxing@xxxxxxxxx> wrote: > > On Sun, Oct 13, 2024 at 1:48 AM Willem de Bruijn > <willemdebruijn.kernel@xxxxxxxxx> wrote: > > > > Jason Xing wrote: > > > From: Jason Xing <kernelxing@xxxxxxxxxxx> > > > > > > A few weeks ago, I planned to extend SO_TIMESTMAMPING feature by using > > > tracepoint to print information (say, tstamp) so that we can > > > transparently equip applications with this feature and require no > > > modification in user side. > > > > > > Later, we discussed at netconf and agreed that we can use bpf for better > > > extension, which is mainly suggested by John Fastabend and Willem de > > > Bruijn. Many thanks here! So I post this series to see if we have a > > > better solution to extend. My feeling is BPF is a good place to provide > > > a way to add timestamping by administrators, without having to rebuild > > > applications. > > > > > > This approach mostly relies on existing SO_TIMESTAMPING feature, users > > > only needs to pass certain flags through bpf_setsocktop() to a separate > > > tsflags. For TX timestamps, they will be printed during generation > > > phase. For RX timestamps, we will wait for the moment when recvmsg() is > > > called. > > > > > > After this series, we could step by step implement more advanced > > > functions/flags already in SO_TIMESTAMPING feature for bpf extension. > > > > > > In this series, I only support TCP protocol which is widely used in > > > SO_TIMESTAMPING feature. > > > > > > --- > > > V2 > > > Link: https://lore.kernel.org/all/20241008095109.99918-1-kerneljasonxing@xxxxxxxxx/ > > > 1. Introduce tsflag requestors so that we are able to extend more in the > > > future. Besides, it enables TX flags for bpf extension feature separately > > > without breaking users. It is suggested by Vadim Fedorenko. > > > 2. introduce a static key to control the whole feature. (Willem) > > > 3. Open the gate of bpf_setsockopt for the SO_TIMESTAMPING feature in > > > some TX/RX cases, not all the cases. > > > > > > Note: > > > The main concern we've discussion in V1 thread is how to deal with the > > > applications using SO_TIMESTAMPING feature? In this series, I allow both > > > cases to happen at the same time, which indicates that even one > > > applications setting SO_TIMESTAMPING can still be traced through BPF > > > program. Please see patch [04/12]. > > > > This revision does not address the main concern. > > > > An administrator installed BPF program can affect results of a process > > using SO_TIMESTAMPING in ways that break it. > > Sorry, I didn't get it. How the following code snippet would break users? > > void __skb_tstamp_tx(struct sk_buff *orig_skb, > const struct sk_buff *ack_skb, > struct skb_shared_hwtstamps *hwtstamps, > struct sock *sk, int tstype) > { > if (!sk) > return; > > if (static_branch_unlikely(&bpf_tstamp_control)) > bpf_skb_tstamp_tx_output(sk, orig_skb, tstype, hwtstamps); > > skb_tstamp_tx_output(orig_skb, ack_skb, hwtstamps, sk, > tstype); > } > > You can see, the application shipped with SO_TIMESTAMPING still prints > timestamps even when the application stays in the attached cgroup > directory. I tested this by running "./txtimestamp -4 -L 127.0.0.1 -l 1000 -c 5" in the bpf attached directory and it can correctly print the timestamp. So it would not break users. And surprisingly I found the key is not that right (ERROR: key 1000, expected 999). I will investigate and fix it. Thanks, Jason
