netdev - Re: [PATCH 0/3] bpf: TSTAMP_COMPLETION_CB timestamping + enable it for Bluetooth

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAL+tcoAvFCm9xCOwCLAp18JpT-JBzXQ2yziTZvO8QvZdL5gRZw@mail.gmail.com>
Date: Mon, 31 Mar 2025 08:39:46 +0800
From: Jason Xing <kerneljasonxing@...il.com>
To: Pauli Virtanen <pav@....fi>
Cc: linux-bluetooth@...r.kernel.org, netdev@...r.kernel.org, 
	bpf@...r.kernel.org, willemdebruijn.kernel@...il.com, 
	Martin KaFai Lau <martin.lau@...ux.dev>
Subject: Re: [PATCH 0/3] bpf: TSTAMP_COMPLETION_CB timestamping + enable it
 for Bluetooth

Hi Pauli,

On Sun, Mar 30, 2025 at 8:23 PM Pauli Virtanen <pav@....fi> wrote:
>
> Add BPF_SOCK_OPS_TSTAMP_COMPLETION_CB and emit it on completion
> timestamps.
>
> Enable that for Bluetooth.

Thanks for working on this!

It would be better if you can cc Martin in the next revision since he
is one of co-authors of BPF timestamping. Using
./scripts/get_maintainer.pl will show you which group people you're
supposed to cc.

>
> Tests:
> https://lore.kernel.org/linux-bluetooth/a74e58b9cf12bc9c64a024d18e6e58999202f853.1743336056.git.pav@iki.fi/
>
> ***
>
> However, I don't quite see how to do the tskey management so
> that BPF and socket timestamping do not interfere with each other.
>
> The tskey counter here increments only for sendmsg() that have
> timestamping turned on. IIUC this works similarly as for UDP.  I
> understood the documentation so that stream sockets would do similarly,
> but apparently TCP increments also for non-timestamped packets.

TCP increments sequence number for every skb regardless of BPF
timestamping feature. BPF timetamping uses the last byte of the last
skb to generate the tskey in tcp_tx_timestamp(). So it means the tskey
comes with the sequence number of each to-be-traced skb. It works for
both socket and BPF timestamping features.

>
> If BPF needs tskey while socket timestamping is off, we can't increment
> sk_tskey, as that interferes with counting by user applications doing
> socket timestamps.

That is the reason why in TCP we chose to implement the tskey of BPF
timestamping in the socket timestamping area. Please take a look at
tcp_tx_timestamp(). As for UDP implementation, it is a leftover that I
will make work next month.

>
> Should the Bluetooth timestamping actually just increment the counters
> for any packet, timestamped or not?

It's supposed to be the same tskey shared with socket timestamping so
that people don't need to separately take care of a new tskey
management.That is to say, if the socket timestamping and BPF
timestamping are turned on, sharing the same tskey will be consistent.

Thanks,
Jason

>
> Pauli Virtanen (3):
>   bpf: Add BPF_SOCK_OPS_TSTAMP_COMPLETION_CB callback
>   [RFC] bpf: allow non-TCP skbs for bpf_sock_ops_enable_tx_tstamp
>   [RFC] Bluetooth: enable bpf TX timestamping
>
>  include/net/bluetooth/bluetooth.h |  1 +
>  include/uapi/linux/bpf.h          |  5 +++++
>  net/bluetooth/hci_conn.c          | 21 +++++++++++++++++++--
>  net/core/filter.c                 | 12 ++++++++++--
>  net/core/skbuff.c                 |  3 +++
>  5 files changed, 38 insertions(+), 4 deletions(-)
>
> --
> 2.49.0
>