bpf: TSTAMP_COMPLETION_CB timestamping + enable it for Bluetooth

Emit timestamps also for BPF timestamping.

***

The tskey management here is not quite right: see cover letter.

---
  include/net/bluetooth/bluetooth.h |  1 +
  net/bluetooth/hci_conn.c          | 21 +++++++++++++++++++--
  2 files changed, 20 insertions(+), 2 deletions(-)

include/net/bluetooth/bluetooth.h +1 -0

index bbefde319f95..3b2e59cedd2d 100644
@@ -383,6 +383,7 @@ struct bt_sock {
  	struct list_head accept_q;
  	struct sock *parent;
  	unsigned long flags;
+	atomic_t bpf_tskey;
  	void (*skb_msg_name)(struct sk_buff *, void *, int *);
  	void (*skb_put_cmsg)(struct sk_buff *, struct msghdr *, struct sock *);
  };

net/bluetooth/hci_conn.c +2 -0

index 95972fd4c784..7430df1c5822 100644
@@ -28,6 +28,7 @@
  #include <linux/export.h>
  #include <linux/debugfs.h>
  #include <linux/errqueue.h>
+#include <linux/bpf-cgroup.h>
 
  #include <net/bluetooth/bluetooth.h>
  #include <net/bluetooth/hci_core.h>

@@ -3072,6 +3073,7 @@ void hci_setup_tx_timestamp(struct sk_buff *skb, size_t key_offset,
  			    const struct sockcm_cookie *sockc)
  {
  	struct sock *sk = skb ? skb->sk : NULL;
+	bool have_tskey = false;
 
  	/* This shall be called on a single skb of those generated by user
  	 * sendmsg(), and only when the sendmsg() does not return error to

net/bluetooth/hci_conn.c +7 -0

+
+		if (!have_tskey)
+			skb_shinfo(skb)->tskey = key - 1;
+
+		bpf_skops_tx_timestamping(sk, skb,
+					  BPF_SOCK_OPS_TSTAMP_SENDMSG_CB);
+
  	}
  }
 

Quoted 44 lines

Emit timestamps also for BPF timestamping.

***

The tskey management here is not quite right: see cover letter.

---
  include/net/bluetooth/bluetooth.h |  1 +
  net/bluetooth/hci_conn.c          | 21 +++++++++++++++++++--
  2 files changed, 20 insertions(+), 2 deletions(-)

include/net/bluetooth/bluetooth.h +1 -0

index bbefde319f95..3b2e59cedd2d 100644
@@ -383,6 +383,7 @@ struct bt_sock {
  	struct list_head accept_q;
  	struct sock *parent;
  	unsigned long flags;
+	atomic_t bpf_tskey;
  	void (*skb_msg_name)(struct sk_buff *, void *, int *);
  	void (*skb_put_cmsg)(struct sk_buff *, struct msghdr *, struct sock *);
  };

net/bluetooth/hci_conn.c +2 -0

index 95972fd4c784..7430df1c5822 100644
@@ -28,6 +28,7 @@
  #include <linux/export.h>
  #include <linux/debugfs.h>
  #include <linux/errqueue.h>
+#include <linux/bpf-cgroup.h>
 
  #include <net/bluetooth/bluetooth.h>
  #include <net/bluetooth/hci_core.h>

@@ -3072,6 +3073,7 @@ void hci_setup_tx_timestamp(struct sk_buff *skb, size_t key_offset,
  			    const struct sockcm_cookie *sockc)
  {
  	struct sock *sk = skb ? skb->sk : NULL;
+	bool have_tskey = false;
 
  	/* This shall be called on a single skb of those generated by user
  	 * sendmsg(), and only when the sendmsg() does not return error to

@@ -3096,6 +3098,20 @@ void hci_setup_tx_timestamp(struct sk_buff *skb, size_t key_offset,
 

net/bluetooth/hci_conn.c void hci_setup_tx_timestamp(struct sk_buff *skb, size_t key_offset, · line 3107

Quoted 9 lines

net/bluetooth/hci_conn.c +7 -0

  			skb_shinfo(skb)->tskey = key - 1;
  		}
+		have_tskey = true;
+	}
+
+	if (cgroup_bpf_enabled(CGROUP_SOCK_OPS) &&
+	    SK_BPF_CB_FLAG_TEST(sk, SK_BPF_CB_TX_TIMESTAMPING)) {
+		struct bt_sock *bt_sk = container_of(sk, struct bt_sock, sk);
+		int key = atomic_inc_return(&bt_sk->bpf_tskey);

I don't think it needs to add "atomic_t bpf_tskey". Allow the bpf to decide what the skb_shinfo(skb)->tskey should be if it is not set by the userspace.

Quoted 11 lines

net/bluetooth/hci_conn.c +7 -0

+
+		if (!have_tskey)
+			skb_shinfo(skb)->tskey = key - 1;
+
+		bpf_skops_tx_timestamping(sk, skb,
+					  BPF_SOCK_OPS_TSTAMP_SENDMSG_CB);
+
  	}
  }
 

Hi,

ti, 2025-04-01 kello 18:34 -0700, Martin KaFai Lau kirjoitti:

Quoted 57 lines · Pauli Virtanen

Quoted 43 lines

Emit timestamps also for BPF timestamping.

***

The tskey management here is not quite right: see cover letter.

---
   include/net/bluetooth/bluetooth.h |  1 +
   net/bluetooth/hci_conn.c          | 21 +++++++++++++++++++--
   2 files changed, 20 insertions(+), 2 deletions(-)

include/net/bluetooth/bluetooth.h +1 -0

index bbefde319f95..3b2e59cedd2d 100644
@@ -383,6 +383,7 @@ struct bt_sock {
   	struct list_head accept_q;
   	struct sock *parent;
   	unsigned long flags;
+	atomic_t bpf_tskey;
   	void (*skb_msg_name)(struct sk_buff *, void *, int *);
   	void (*skb_put_cmsg)(struct sk_buff *, struct msghdr *, struct sock *);
   };

net/bluetooth/hci_conn.c +2 -0

index 95972fd4c784..7430df1c5822 100644
@@ -28,6 +28,7 @@
   #include <linux/export.h>
   #include <linux/debugfs.h>
   #include <linux/errqueue.h>
+#include <linux/bpf-cgroup.h>
 
   #include <net/bluetooth/bluetooth.h>
   #include <net/bluetooth/hci_core.h>

@@ -3072,6 +3073,7 @@ void hci_setup_tx_timestamp(struct sk_buff *skb, size_t key_offset,
   			    const struct sockcm_cookie *sockc)
   {
   	struct sock *sk = skb ? skb->sk : NULL;
+	bool have_tskey = false;
 
   	/* This shall be called on a single skb of those generated by user
   	 * sendmsg(), and only when the sendmsg() does not return error to

net/bluetooth/hci_conn.c void hci_setup_tx_timestamp(struct sk_buff *skb, size_t key_offset, · line 3107

Quoted 10 lines

net/bluetooth/hci_conn.c +7 -0

@@ -3096,6 +3098,20 @@ void hci_setup_tx_timestamp(struct sk_buff *skb, size_t key_offset,
 
   			skb_shinfo(skb)->tskey = key - 1;
   		}
+		have_tskey = true;
+	}
+
+	if (cgroup_bpf_enabled(CGROUP_SOCK_OPS) &&
+	    SK_BPF_CB_FLAG_TEST(sk, SK_BPF_CB_TX_TIMESTAMPING)) {
+		struct bt_sock *bt_sk = container_of(sk, struct bt_sock, sk);
+		int key = atomic_inc_return(&bt_sk->bpf_tskey);

I don't think it needs to add "atomic_t bpf_tskey". Allow the bpf to decide what the skb_shinfo(skb)->tskey should be if it is not set by the userspace.

Ok. So if I understand correctly, the plan is that for UDP and Bluetooth seqpacket sockets it works like this:

bpf_sock_ops_enable_tx_tstamp() does not set tskey.

Socket timestamping sets tskey the same way as previously.

So when both are in play, it shall work like:

* attach BPF timestamping
* setsockopt(SOF_TIMESTAMPING_SOFTWARE | SOF_TIMESTAMPING_OPT_ID)
* sendmsg() CMSG SO_TIMESTAMPING = 0
=> tskey 0 (unset)
* sendmsg() CMSG SO_TIMESTAMPING = SOF_TIMESTAMPING_TX_SOFTWARE => tskey 0
* sendmsg() CMSG SO_TIMESTAMPING = SOF_TIMESTAMPING_TX_SOFTWARE => tskey 1
* sendmsg() CMSG SO_TIMESTAMPING = 0
=> tskey 0 (unset)
* sendmsg() CMSG SO_TIMESTAMPING = 0
=> tskey 0 (unset)
* sendmsg() CMSG SO_TIMESTAMPING = 0
=> tskey 0 (unset)
* sendmsg() CMSG SO_TIMESTAMPING = SOF_TIMESTAMPING_TX_SOFTWARE => tskey 2

and BPF program has to handle the (unset) cases itself.

Quoted 13 lines

Quoted 11 lines

net/bluetooth/hci_conn.c +7 -0

+
+		if (!have_tskey)
+			skb_shinfo(skb)->tskey = key - 1;
+
+		bpf_skops_tx_timestamping(sk, skb,
+					  BPF_SOCK_OPS_TSTAMP_SENDMSG_CB);
+
   	}
   }
 

Add BPF_SOCK_OPS_TSTAMP_COMPLETION_CB and emit it on completion timestamps.

Enable that for Bluetooth.

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.

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.

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

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

Hi Pauli,

Quoted 5 lines · Pauli Virtanen

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.

Quoted 6 lines

Tests:
https://lore.kernel.org/linux-bluetooth/a74e58b9cf12bc9c64a024d18e6e58999202f853.1743336056.git.pav@iki.fi/

***

Quoted 7 lines

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.

Quoted 1 line

Quoted 3 lines

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.

Quoted 2 lines

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

Quoted 16 lines

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