On 11/4/24 10:22 PM, Jason Xing wrote:
On Tue, Nov 5, 2024 at 10:09 AM Martin KaFai Lau <martin.lau@xxxxxxxxx> wrote:
On 11/1/24 6:32 AM, Willem de Bruijn wrote:
In udp/raw/..., I don't know how likely is the user space having "cork->tx_flags
& SKBTX_ANY_TSTAMP" set but has neither "READ_ONCE(sk->sk_tsflags) &
SOF_TIMESTAMPING_OPT_ID" nor "cork->flags & IPCORK_TS_OPT_ID" set.
This is not something to rely on. OPT_ID was added relatively recently.
Older applications, or any that just use the most straightforward API,
will not set this.
Good point that the OPT_ID per cmsg is very new.
The datagram support on SOF_TIMESTAMPING_OPT_ID in sk->sk_tsflags had
been there for quite some time now. Is it a safe assumption that
most applications doing udp tx timestamping should have
the SOF_TIMESTAMPING_OPT_ID set to be useful?
If it is
unlikely, may be we can just disallow bpf prog from directly setting
skb_shinfo(skb)->tskey for this particular skb.
For all other cases, in __ip[6]_append_data, directly call a bpf prog and also
pass the kernel decided tskey to the bpf prog.
The kernel passed tskey could be 0 (meaning the user space has not used it). The
bpf prog can give one for the kernel to use. The bpf prog can store the
sk_tskey_bpf in the bpf_sk_storage now. Meaning no need to add one to the struct
sock. The bpf prog does not have to start from 0 (e.g. start from U32_MAX
instead) if it helps.
If the kernel passed tskey is not 0, the bpf prog can just use that one
(assuming the user space is doing something sane, like the value in
SCM_TS_OPT_ID won't be jumping back and front between 0 to U32_MAX). I hope this
is very unlikely also (?) but the bpf prog can probably detect this and choose
to ignore this sk.
If an applications uses OPT_ID, it is unlikely that they will toggle
the feature on and off on a per-packet basis. So in the common case
the program could use the user-set counter or use its own if userspace
does not enable the feature. In the rare case that an application does
intermittently set an OPT_ID, the numbering would be erratic. This
does mean that an actively malicious application could mess with admin
measurements.
All make sense. Given it is reasonable to assume the user space should either
has SOF_TIMESTAMPING_OPT_ID always on or always off. When it is off, the bpf
prog can directly provide its own tskey to be used in shinfo->tskey. The bpf
prog can generate the id itself without using the sk->sk_tskey, e.g. store an
atomic int in the bpf_sk_storage.
I wonder, how can we correlate the key with each skb in the bpf
program for non-TCP type without implementing a bpf extension for
SCM_TS_OPT_ID? Every time the timestamp is reported, we cannot know
which sendmsg() the skb belongs to for non-TCP cases.
SCM_TS_OPT_ID is eventually setting the shinfo->tskey.
If the shinfo->tskey is not set by the user space, the bpf prog can directly set
the shinfo->tskey. There is no need to use the sk->sk_tskey as the ID generator
also. The bpf prog can have its own id generator.
If the user space has already set the shinfo->tskey (either by sk->sk_tskey or
SCM_TS_OPT_ID), the bpf prog can just use the user space one.
If there is a weird application that flips flops between OPT_ID on/off, the bpf
prog will get confused which is fine. The bpf prog can detect this and choose to
ignore measuring this sk/skb. The bpf prog can also choose to be on the very
safe side and ignore all skb with SKBTX_ANY_TSTAMP set in txflags but with no
OPT_ID. The bpf prog can look into the details of the sk and skb to decide what
makes the most sense for its deployment.
I don't know whether it makes more sense to call the bpf prog to decide the
shinfo->{tx_flags,tskey} just before the "while (length > 0)" in
__ip[6]_append_data or it is better to call the bpf prog in ip[6]_setup_cork.
I admittedly less familiar with this code path than the tcp one.
