mbox series

[v2,0/5] Bluetooth: add TX timestamping for ISO/SCO/L2CAP

Message ID cover.1710440392.git.pav@iki.fi (mailing list archive)
Headers show
Series Bluetooth: add TX timestamping for ISO/SCO/L2CAP | expand

Message

Pauli Virtanen March 14, 2024, 6:20 p.m. UTC
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(-)

Comments

Pauli Virtanen March 22, 2024, 4:56 p.m. UTC | #1
Hi Luiz,

to, 2024-03-14 kello 20:20 +0200, Pauli Virtanen kirjoitti:
> 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.

Any suggestions what the plan here should be?

The suggestions so far:

1. Socket TX timestamping & deal with POLLERR in BlueZ

2. Socket TX timestamping & disable POLLERR via setsockopt

3. Some custom latency reporting mechanism


> 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(-)
>
patchwork-bot+bluetooth@kernel.org April 1, 2024, 8 p.m. UTC | #2
Hello:

This series was applied to bluetooth/bluetooth-next.git (master)
by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:

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.
> 
> 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!
Luiz Augusto von Dentz April 1, 2024, 8:05 p.m. UTC | #3
Hi Pauli,

On Mon, Apr 1, 2024 at 4:00 PM <patchwork-bot+bluetooth@kernel.org> wrote:
>
> Hello:
>
> This series was applied to bluetooth/bluetooth-next.git (master)
> by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:
>
> 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.
> >
> > 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)

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.


> You are awesome, thank you!
> --
> Deet-doot-dot, I am a bot.
> https://korg.docs.kernel.org/patchwork/pwbot.html
>
>
>
Pauli Virtanen April 1, 2024, 8:39 p.m. UTC | #4
Hi Luiz,

ma, 2024-04-01 kello 16:05 -0400, Luiz Augusto von Dentz kirjoitti:
> Hi Pauli,
> 
> On Mon, Apr 1, 2024 at 4:00 PM <patchwork-bot+bluetooth@kernel.org> wrote:
> > 
> > Hello:
> > 
> > This series was applied to bluetooth/bluetooth-next.git (master)
> > by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:
> > 
> > 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.
> > > 
> > > 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)
> 
> 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.

Thanks, I'll add the experimental flag to 5/5 and resend.