[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAL+tcoBf+kQ3_kc9x62KnHx9O+6c==_DN+6EheL82UKQ3xQN1A@mail.gmail.com>
Date: Tue, 5 Nov 2024 14:22:06 +0800
From: Jason Xing <kerneljasonxing@...il.com>
To: Martin KaFai Lau <martin.lau@...ux.dev>
Cc: Willem de Bruijn <willemdebruijn.kernel@...il.com>, willemb@...gle.com,
davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com,
dsahern@...nel.org, ast@...nel.org, daniel@...earbox.net, andrii@...nel.org,
eddyz87@...il.com, song@...nel.org, yonghong.song@...ux.dev,
john.fastabend@...il.com, kpsingh@...nel.org, sdf@...ichev.me,
haoluo@...gle.com, jolsa@...nel.org, shuah@...nel.org, ykolal@...com,
bpf@...r.kernel.org, netdev@...r.kernel.org,
Jason Xing <kernelxing@...cent.com>
Subject: Re: [PATCH net-next v3 02/14] net-timestamp: allow two features to
work parallelly
On Tue, Nov 5, 2024 at 10:09 AM Martin KaFai Lau <martin.lau@...ux.dev> 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.
Thanks,
Jason
Powered by blists - more mailing lists