Re: [RFC PATCH 0/3] Bluetooth: add transmission latency tracking for ISO & L2CAP

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Luiz,

ke, 2024-02-28 kello 15:31 -0500, Luiz Augusto von Dentz kirjoitti:
> Hi Pauli,
> 
> On Wed, Feb 28, 2024 at 3:11 PM Pauli Virtanen <pav@xxxxxx> wrote:
> > 
> > Add ISO/L2CAP socket ioctl() BTGETTXINFO which informs the user how long
> > it took the kernel and controller to complete their sendmsg(), and how
> > many sendmsg() are in socket and controller queues.
> > 
> > This currently provides information of the latest packet only, in
> > principle there could be a ringbuffer containing few latest packets, not
> > clear if that would be useful.
> > 
> > These patches allow fixing / working around controller(?) issue where
> > two ISO streams in same group get desynchronized.  Having accurate
> > knowledge of the packet queue lengths, user application can drop packets
> > if it detects the ISO streams are not in sync.
> > 
> > Pipewire side:
> > https://gitlab.freedesktop.org/pvir/pipewire/-/commits/iso-ts-test
> > 
> > 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, this can be changed to use LE Read ISO TX Sync, when the clock
> > and sequence number synchronization issues there are figured out, and a
> > quirk is added for controllers with nonfunctional implementation.
> > 
> > For the L2CAP latency, I'll need to think a bit more what is the audio
> > use case. Motivation was that AVDTP delay report values appear to be off
> > by ~0..40 ms compared to observed audio latency and this amount can vary
> > per connection and time, so not explained by unaccounted code
> > algorithmic delays etc. Currently it's not clear if there is relation to
> > TX side latency, so it may be down to receiver side implementation.
> > 
> > This needs a bit more work to figure out, but the L2CAP patch is anyway
> > here.  Due to the possible fragmentation in ISO sendmsg(), it seems we
> > anyway need the tx_info_queue thing and can't easily do it by counting
> > packets, and L2CAP required part is small addition on top of that.
> 
> That is not how it is normally done with sockets, normally this is
> done with use of SO_TIMESTAMPING which is then reported using the
> socket error queue:
> 
> https://www.kernel.org/doc/html/latest/networking/timestamping.html
> 
> Sorry to tell you just now, you might have lost a lot of time doing
> all of the changes, next time just drop and RFC early on with the
> general design so you don't spend too much time before getting any
> feedback.

Thanks, looks like I missed that was not just for RX timestamping.

We then need to hang on to the skbs to be timestamped in hci_core until
packet completion. Looks like skb_queue_* is used by drivers, so
probably skb_get and ringbuffer like in this patchset then.

> > TBD: iso-tester / l2cap-tester tests
> > 
> > Pauli Virtanen (3):
> >   Bluetooth: add transmission latency tracking for ISO and ACL
> >   Bluetooth: ISO: add new ioctl() for reading tx latency
> >   Bluetooth: L2CAP: add new ioctl() for reading tx latency
> > 
> >  include/net/bluetooth/bluetooth.h |  39 +++++++++++
> >  include/net/bluetooth/hci_core.h  |  30 ++++++++
> >  net/bluetooth/hci_conn.c          | 110 +++++++++++++++++++++++++++++-
> >  net/bluetooth/hci_core.c          |  14 ++++
> >  net/bluetooth/hci_event.c         |  66 ++++++++++++++++++
> >  net/bluetooth/iso.c               |  58 ++++++++++++++--
> >  net/bluetooth/l2cap_core.c        |  12 ++++
> >  net/bluetooth/l2cap_sock.c        |  50 +++++++++++++-
> >  8 files changed, 372 insertions(+), 7 deletions(-)
> > 
> > --
> > 2.44.0
> > 
> > 
> 
> 

-- 
Pauli Virtanen





[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux