Bluetooth: add TX timestamping for ISO/SCO/L2CAP

Add socket option to disable POLLERR on non-empty socket error queue.

Applications can use this for sleeping socket error POLLERR waits, also when using TX timestamping. This is useful when multiple layers of the stack are using the same socket.

Signed-off-by Pauli Virtanen <[email protected]>

---
 include/net/bluetooth/bluetooth.h |  9 +++-
 net/bluetooth/af_bluetooth.c      | 72 ++++++++++++++++++++++++++++++-
 net/bluetooth/iso.c               |  8 ++--
 net/bluetooth/l2cap_sock.c        |  8 ++--
 net/bluetooth/sco.c               |  8 ++--
 5 files changed, 91 insertions(+), 14 deletions(-)

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

index 9280e72093ee..dcee5384d715 100644
@@ -240,6 +240,8 @@ struct bt_codecs {
 
 #define BT_ISO_BASE            20
 
+#define BT_NO_ERRQUEUE_POLL    21
+
 __printf(1, 2)
 void bt_info(const char *fmt, ...);
 __printf(1, 2)

@@ -387,7 +389,8 @@ struct bt_sock {
 enum {
        BT_SK_DEFER_SETUP,
        BT_SK_SUSPEND,
-       BT_SK_PKT_STATUS
+       BT_SK_PKT_STATUS,
+       BT_SK_NO_ERRQUEUE_POLL
 };
 
 struct bt_sock_list {

@@ -410,6 +413,10 @@ int  bt_sock_stream_recvmsg(struct socket *sock, struct msghdr *msg,
                            size_t len, int flags);
 __poll_t bt_sock_poll(struct file *file, struct socket *sock, poll_table *wait);
 int  bt_sock_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg);
+int bt_sock_setsockopt(struct socket *sock, int level, int optname,
+                      sockptr_t optval, unsigned int optlen);
+int bt_sock_getsockopt(struct socket *sock, int level, int optname,
+                      char __user *optval, int __user *optlen);
 int  bt_sock_wait_state(struct sock *sk, int state, unsigned long timeo);
 int  bt_sock_wait_ready(struct sock *sk, unsigned int msg_flags);
 

net/bluetooth/af_bluetooth.c +71 -1

index 67604ccec2f4..997197aa7b48 100644
@@ -500,6 +500,12 @@ static inline __poll_t bt_accept_poll(struct sock *parent)
        return 0;
 }
 
+static bool bt_sock_error_queue_poll(struct sock *sk)
+{
+       return !test_bit(BT_SK_NO_ERRQUEUE_POLL, &bt_sk(sk)->flags) &&
+               !skb_queue_empty_lockless(&sk->sk_error_queue);
+}
+
 __poll_t bt_sock_poll(struct file *file, struct socket *sock,
                      poll_table *wait)
 {

@@ -511,7 +517,7 @@ __poll_t bt_sock_poll(struct file *file, struct socket *sock,
        if (sk->sk_state == BT_LISTEN)
                return bt_accept_poll(sk);
 
-       if (sk->sk_err || !skb_queue_empty_lockless(&sk->sk_error_queue))
+       if (sk->sk_err || bt_sock_error_queue_poll(sk))
                mask |= EPOLLERR |
                        (sock_flag(sk, SOCK_SELECT_ERR_QUEUE) ? EPOLLPRI : 0);
 

@@ -582,6 +588,70 @@ int bt_sock_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
 }
 EXPORT_SYMBOL(bt_sock_ioctl);
 
+int bt_sock_setsockopt(struct socket *sock, int level, int optname,
+                      sockptr_t optval, unsigned int optlen)
+{
+       struct sock *sk = sock->sk;
+       int err = 0;
+       u32 opt;
+
+       if (level != SOL_BLUETOOTH)
+               return -ENOPROTOOPT;
+
+       lock_sock(sk);
+
+       switch (optname) {
+       case BT_NO_ERRQUEUE_POLL:
+               if (copy_from_sockptr(&opt, optval, sizeof(opt))) {
+                       err = -EFAULT;
+                       break;
+               }
+
+               if (opt)
+                       set_bit(BT_SK_NO_ERRQUEUE_POLL, &bt_sk(sk)->flags);
+               else
+                       clear_bit(BT_SK_NO_ERRQUEUE_POLL, &bt_sk(sk)->flags);
+               break;
+
+       default:
+               err = -ENOPROTOOPT;
+               break;
+       }
+
+       release_sock(sk);
+       return err;
+}
+EXPORT_SYMBOL(bt_sock_setsockopt);
+
+int bt_sock_getsockopt(struct socket *sock, int level, int optname,
+                      char __user *optval, int __user *optlen)
+{
+       struct sock *sk = sock->sk;
+       int err = 0;
+       u32 opt;
+
+       if (level != SOL_BLUETOOTH)
+               return -ENOPROTOOPT;
+
+       lock_sock(sk);
+
+       switch (optname) {
+       case BT_NO_ERRQUEUE_POLL:
+               opt = test_bit(BT_SK_NO_ERRQUEUE_POLL, &bt_sk(sk)->flags);
+               if (put_user(opt, (u32 __user *)optval))
+                       err = -EFAULT;
+               break;
+
+       default:
+               err = -ENOPROTOOPT;
+               break;
+       }
+
+       release_sock(sk);
+       return err;
+}
+EXPORT_SYMBOL(bt_sock_getsockopt);
+
 /* This function expects the sk lock to be held when called */
 int bt_sock_wait_state(struct sock *sk, int state, unsigned long timeo)
 {

net/bluetooth/iso.c +4 -4

index a77ab9df7994..6f5af549a8cc 100644
@@ -1596,8 +1596,8 @@ static int iso_sock_setsockopt(struct socket *sock, int level, int optname,
                break;
 
        default:
-               err = -ENOPROTOOPT;
-               break;
+               release_sock(sk);
+               return bt_sock_setsockopt(sock, level, optname, optval, optlen);
        }
 
        release_sock(sk);

@@ -1667,8 +1667,8 @@ static int iso_sock_getsockopt(struct socket *sock, int level, int optname,
                break;
 
        default:
-               err = -ENOPROTOOPT;
-               break;
+               release_sock(sk);
+               return bt_sock_getsockopt(sock, level, optname, optval, optlen);
        }
 
        release_sock(sk);

net/bluetooth/l2cap_sock.c +4 -4

index 9a9f933a748b..06277ce1fd6b 100644
@@ -697,8 +697,8 @@ static int l2cap_sock_getsockopt(struct socket *sock, int level, int optname,
                break;
 
        default:
-               err = -ENOPROTOOPT;
-               break;
+               release_sock(sk);
+               return bt_sock_getsockopt(sock, level, optname, optval, optlen);
        }
 
        release_sock(sk);

@@ -1102,8 +1102,8 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname,
                break;
 
        default:
-               err = -ENOPROTOOPT;
-               break;
+               release_sock(sk);
+               return bt_sock_setsockopt(sock, level, optname, optval, optlen);
        }
 
        release_sock(sk);

net/bluetooth/sco.c +4 -5

index b3c2af7c7d67..b8b1b314aed4 100644
@@ -968,8 +968,8 @@ static int sco_sock_setsockopt(struct socket *sock, int level, int optname,
                break;
 
        default:
-               err = -ENOPROTOOPT;
-               break;
+               release_sock(sk);
+               return bt_sock_setsockopt(sock, level, optname, optval, optlen);
        }
 
        release_sock(sk);

@@ -1211,8 +1211,8 @@ static int sco_sock_getsockopt(struct socket *sock, int level, int optname,
                break;
 
        default:
-               err = -ENOPROTOOPT;
-               break;
+               release_sock(sk);
+               return bt_sock_getsockopt(sock, level, optname, optval, optlen);
        }
 
        release_sock(sk);
--

2.44.0

Hi Pauli,

Quoted 6 lines · Pauli Virtanen

Add socket option to disable POLLERR on non-empty socket error queue.

Applications can use this for sleeping socket error POLLERR waits, also when using TX timestamping. This is useful when multiple layers of the stack are using the same socket.

So the idea here is that bluetoothd would set BT_NO_ERRQUEUE_POLL to not be awakened by timestamps? I wonder why this works on per fd/socket level while SO_TIMESTAMPING does apply to all sockets though, or perhaps that is not really related to SO_TIMESTAMPING but errqueue being the same to all cloned sockets? I'm a little hesitant to start introducing new socket options if the whole point was to avoid doing so with the use of SO_TIMESTAMPING.

Quoted 224 lines

Signed-off-by Pauli Virtanen <[email protected]>

---
 include/net/bluetooth/bluetooth.h |  9 +++-
 net/bluetooth/af_bluetooth.c      | 72 ++++++++++++++++++++++++++++++-
 net/bluetooth/iso.c               |  8 ++--
 net/bluetooth/l2cap_sock.c        |  8 ++--
 net/bluetooth/sco.c               |  8 ++--
 5 files changed, 91 insertions(+), 14 deletions(-)

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

index 9280e72093ee..dcee5384d715 100644
@@ -240,6 +240,8 @@ struct bt_codecs {
 
 #define BT_ISO_BASE            20
 
+#define BT_NO_ERRQUEUE_POLL    21
+
 __printf(1, 2)
 void bt_info(const char *fmt, ...);
 __printf(1, 2)

@@ -387,7 +389,8 @@ struct bt_sock {
 enum {
        BT_SK_DEFER_SETUP,
        BT_SK_SUSPEND,
-       BT_SK_PKT_STATUS
+       BT_SK_PKT_STATUS,
+       BT_SK_NO_ERRQUEUE_POLL
 };
 
 struct bt_sock_list {

@@ -410,6 +413,10 @@ int  bt_sock_stream_recvmsg(struct socket *sock, struct msghdr *msg,
                            size_t len, int flags);
 __poll_t bt_sock_poll(struct file *file, struct socket *sock, poll_table *wait);
 int  bt_sock_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg);
+int bt_sock_setsockopt(struct socket *sock, int level, int optname,
+                      sockptr_t optval, unsigned int optlen);
+int bt_sock_getsockopt(struct socket *sock, int level, int optname,
+                      char __user *optval, int __user *optlen);
 int  bt_sock_wait_state(struct sock *sk, int state, unsigned long timeo);
 int  bt_sock_wait_ready(struct sock *sk, unsigned int msg_flags);
 

net/bluetooth/af_bluetooth.c +71 -1

index 67604ccec2f4..997197aa7b48 100644
@@ -500,6 +500,12 @@ static inline __poll_t bt_accept_poll(struct sock *parent)
        return 0;
 }
 
+static bool bt_sock_error_queue_poll(struct sock *sk)
+{
+       return !test_bit(BT_SK_NO_ERRQUEUE_POLL, &bt_sk(sk)->flags) &&
+               !skb_queue_empty_lockless(&sk->sk_error_queue);
+}
+
 __poll_t bt_sock_poll(struct file *file, struct socket *sock,
                      poll_table *wait)
 {

@@ -511,7 +517,7 @@ __poll_t bt_sock_poll(struct file *file, struct socket *sock,
        if (sk->sk_state == BT_LISTEN)
                return bt_accept_poll(sk);
 
-       if (sk->sk_err || !skb_queue_empty_lockless(&sk->sk_error_queue))
+       if (sk->sk_err || bt_sock_error_queue_poll(sk))
                mask |= EPOLLERR |
                        (sock_flag(sk, SOCK_SELECT_ERR_QUEUE) ? EPOLLPRI : 0);
 

@@ -582,6 +588,70 @@ int bt_sock_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
 }
 EXPORT_SYMBOL(bt_sock_ioctl);
 
+int bt_sock_setsockopt(struct socket *sock, int level, int optname,
+                      sockptr_t optval, unsigned int optlen)
+{
+       struct sock *sk = sock->sk;
+       int err = 0;
+       u32 opt;
+
+       if (level != SOL_BLUETOOTH)
+               return -ENOPROTOOPT;
+
+       lock_sock(sk);
+
+       switch (optname) {
+       case BT_NO_ERRQUEUE_POLL:
+               if (copy_from_sockptr(&opt, optval, sizeof(opt))) {
+                       err = -EFAULT;
+                       break;
+               }
+
+               if (opt)
+                       set_bit(BT_SK_NO_ERRQUEUE_POLL, &bt_sk(sk)->flags);
+               else
+                       clear_bit(BT_SK_NO_ERRQUEUE_POLL, &bt_sk(sk)->flags);
+               break;
+
+       default:
+               err = -ENOPROTOOPT;
+               break;
+       }
+
+       release_sock(sk);
+       return err;
+}
+EXPORT_SYMBOL(bt_sock_setsockopt);
+
+int bt_sock_getsockopt(struct socket *sock, int level, int optname,
+                      char __user *optval, int __user *optlen)
+{
+       struct sock *sk = sock->sk;
+       int err = 0;
+       u32 opt;
+
+       if (level != SOL_BLUETOOTH)
+               return -ENOPROTOOPT;
+
+       lock_sock(sk);
+
+       switch (optname) {
+       case BT_NO_ERRQUEUE_POLL:
+               opt = test_bit(BT_SK_NO_ERRQUEUE_POLL, &bt_sk(sk)->flags);
+               if (put_user(opt, (u32 __user *)optval))
+                       err = -EFAULT;
+               break;
+
+       default:
+               err = -ENOPROTOOPT;
+               break;
+       }
+
+       release_sock(sk);
+       return err;
+}
+EXPORT_SYMBOL(bt_sock_getsockopt);
+
 /* This function expects the sk lock to be held when called */
 int bt_sock_wait_state(struct sock *sk, int state, unsigned long timeo)
 {

net/bluetooth/iso.c +4 -4

index a77ab9df7994..6f5af549a8cc 100644
@@ -1596,8 +1596,8 @@ static int iso_sock_setsockopt(struct socket *sock, int level, int optname,
                break;
 
        default:
-               err = -ENOPROTOOPT;
-               break;
+               release_sock(sk);
+               return bt_sock_setsockopt(sock, level, optname, optval, optlen);
        }
 
        release_sock(sk);

@@ -1667,8 +1667,8 @@ static int iso_sock_getsockopt(struct socket *sock, int level, int optname,
                break;
 
        default:
-               err = -ENOPROTOOPT;
-               break;
+               release_sock(sk);
+               return bt_sock_getsockopt(sock, level, optname, optval, optlen);
        }
 
        release_sock(sk);

net/bluetooth/l2cap_sock.c +4 -4

index 9a9f933a748b..06277ce1fd6b 100644
@@ -697,8 +697,8 @@ static int l2cap_sock_getsockopt(struct socket *sock, int level, int optname,
                break;
 
        default:
-               err = -ENOPROTOOPT;
-               break;
+               release_sock(sk);
+               return bt_sock_getsockopt(sock, level, optname, optval, optlen);
        }
 
        release_sock(sk);

@@ -1102,8 +1102,8 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname,
                break;
 
        default:
-               err = -ENOPROTOOPT;
-               break;
+               release_sock(sk);
+               return bt_sock_setsockopt(sock, level, optname, optval, optlen);
        }
 
        release_sock(sk);

net/bluetooth/sco.c +4 -5

index b3c2af7c7d67..b8b1b314aed4 100644
@@ -968,8 +968,8 @@ static int sco_sock_setsockopt(struct socket *sock, int level, int optname,
                break;
 
        default:
-               err = -ENOPROTOOPT;
-               break;
+               release_sock(sk);
+               return bt_sock_setsockopt(sock, level, optname, optval, optlen);
        }
 
        release_sock(sk);

@@ -1211,8 +1211,8 @@ static int sco_sock_getsockopt(struct socket *sock, int level, int optname,
                break;
 
        default:
-               err = -ENOPROTOOPT;
-               break;
+               release_sock(sk);
+               return bt_sock_getsockopt(sock, level, optname, optval, optlen);
        }
 
        release_sock(sk);
--

2.44.0

Add support for TX timestamping in ISO/SCO/L2CAP sockets.

These patches allow fixing / working around controller(?) issue where two ISO streams in same group get desynchronized. It also gives user applications the best latency information as available to kernel.

Also add sockopt BT_NO_ERRQUEUE_POLL to optionally disable POLLERR wakeup on TX timestamp arrival, which is mainly a nuisance in the use case here. The alternative to this seems be to deal with the POLLERR wakeups in BlueZ side, but this seems hard as it's always enabled in poll() flags so not clear if anything else than polling at regular intervals can be done there.

Pipewire side:
https://gitlab.freedesktop.org/pvir/pipewire/-/commits/iso-ts-test2

With this change, https://github.com/bluez/bluez/issues/515 is more or less fixed, and the sound server can figure out the total latency to audio rendering (tx latency + transport latency + presentation delay).

For ISO, we can later use LE Read ISO TX Sync to provide hardware timestamps, but this requires figuring out the sequence number synchronization first.

v2:
- Rename *tx_comp* -> *tx*
- Add hci_send_conn_frame() and handle all link types
- Add SCO timestamping. Deal with no flow control -> no Num_Comp_* events
- Handle HCI_FLOW_CTL_MODE_BLOCK_BASED
- Add BT_NO_ERRQUEUE_POLL

Pauli Virtanen (5):
Bluetooth: add support for skb TX timestamping
Bluetooth: ISO: add TX timestamping
Bluetooth: L2CAP: add TX timestamping
Bluetooth: SCO: add TX timestamping
Bluetooth: add BT_NO_ERRQUEUE_POLL socket option

 include/net/bluetooth/bluetooth.h |  10 ++-
 include/net/bluetooth/hci_core.h  |  12 ++++
 include/net/bluetooth/l2cap.h     |   3 +-
 net/bluetooth/6lowpan.c           |   2 +-
 net/bluetooth/af_bluetooth.c      |  72 ++++++++++++++++++-
 net/bluetooth/hci_conn.c          | 111 ++++++++++++++++++++++++++++++
 net/bluetooth/hci_core.c          |  19 +++--
 net/bluetooth/hci_event.c         |  11 ++-
 net/bluetooth/iso.c               |  32 ++++++---
 net/bluetooth/l2cap_core.c        |  11 ++-
 net/bluetooth/l2cap_sock.c        |  23 +++++--
 net/bluetooth/sco.c               |  27 ++++++--
 net/bluetooth/smp.c               |   2 +-
 13 files changed, 303 insertions(+), 32 deletions(-)

Hello:

This series was applied to bluetooth/bluetooth-next.git (master) by Luiz Augusto von Dentz <[email protected]>:

Quoted 6 lines · On Thu, 14 Mar 2024 20:20:16 +0200 you wrote:

Add support for TX timestamping in ISO/SCO/L2CAP sockets.

These patches allow fixing / working around controller(?) issue where two ISO streams in same group get desynchronized. It also gives user applications the best latency information as available to kernel.

Quoted 8 lines

Also add sockopt BT_NO_ERRQUEUE_POLL to optionally disable POLLERR wakeup on TX timestamp arrival, which is mainly a nuisance in the use case here. The alternative to this seems be to deal with the POLLERR wakeups in BlueZ side, but this seems hard as it's always enabled in poll() flags so not clear if anything else than polling at regular intervals can be done there.

[...]

Here is the summary with links:
- [v2,1/5] Bluetooth: add support for skb TX timestamping

    https://git.kernel.org/bluetooth/bluetooth-next/c/0368e609d090
  - [v2,2/5] Bluetooth: ISO: add TX timestamping
    https://git.kernel.org/bluetooth/bluetooth-next/c/3067d73e114f
  - [v2,3/5] Bluetooth: L2CAP: add TX timestamping
    https://git.kernel.org/bluetooth/bluetooth-next/c/e22f35ed23a7

- [v2,4/5] Bluetooth: SCO: add TX timestamping
https://git.kernel.org/bluetooth/bluetooth-next/c/b7adccd0e8b6
- [v2,5/5] Bluetooth: add BT_NO_ERRQUEUE_POLL socket option (no matching commit)

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Hi Pauli,

Quoted 29 lines · On Mon, Apr 1, 2024 at 4:00 PM <[email protected]> wrote:

Hello:

This series was applied to bluetooth/bluetooth-next.git (master) by Luiz Augusto von Dentz <[email protected]>:

Quoted 6 lines · On Thu, 14 Mar 2024 20:20:16 +0200 you wrote:

Add support for TX timestamping in ISO/SCO/L2CAP sockets.

These patches allow fixing / working around controller(?) issue where two ISO streams in same group get desynchronized. It also gives user applications the best latency information as available to kernel.

Quoted 8 lines

Also add sockopt BT_NO_ERRQUEUE_POLL to optionally disable POLLERR wakeup on TX timestamp arrival, which is mainly a nuisance in the use case here. The alternative to this seems be to deal with the POLLERR wakeups in BlueZ side, but this seems hard as it's always enabled in poll() flags so not clear if anything else than polling at regular intervals can be done there.

[...]

Here is the summary with links:
- [v2,1/5] Bluetooth: add support for skb TX timestamping

    https://git.kernel.org/bluetooth/bluetooth-next/c/0368e609d090
  - [v2,2/5] Bluetooth: ISO: add TX timestamping
    https://git.kernel.org/bluetooth/bluetooth-next/c/3067d73e114f
  - [v2,3/5] Bluetooth: L2CAP: add TX timestamping
    https://git.kernel.org/bluetooth/bluetooth-next/c/e22f35ed23a7

Quoted 4 lines

- [v2,4/5] Bluetooth: SCO: add TX timestamping
https://git.kernel.org/bluetooth/bluetooth-next/c/b7adccd0e8b6
- [v2,5/5] Bluetooth: add BT_NO_ERRQUEUE_POLL socket option (no matching commit)

Ive left 5/5 out on purpose, let's have it behind an experimental flag so we can later rework it if necessary, I know it is a little annoying to have to do extra setup in order to test it but we are not supposed to introduce something like this without some safeguards that we can later rework if necessary.