Hi,
ti, 2025-04-01 kello 18:34 -0700, Martin KaFai Lau kirjoitti:
> On 3/30/25 5:23 AM, Pauli Virtanen wrote:
> > 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(-)
> >
> > diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
> > index bbefde319f95..3b2e59cedd2d 100644
> > --- a/include/net/bluetooth/bluetooth.h
> > +++ b/include/net/bluetooth/bluetooth.h
> > @@ -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 *);
> > };
> > diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> > index 95972fd4c784..7430df1c5822 100644
> > --- a/net/bluetooth/hci_conn.c
> > +++ b/net/bluetooth/hci_conn.c
> > @@ -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,
> >
> > 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.
>
> > +
> > + if (!have_tskey)
> > + skb_shinfo(skb)->tskey = key - 1;
> > +
> > + bpf_skops_tx_timestamping(sk, skb,
> > + BPF_SOCK_OPS_TSTAMP_SENDMSG_CB);
> > +
> > }
> > }
> >
>
>
--
Pauli Virtanen
