On Thu, Feb 20, 2025 at 2:15 AM Pauli Virtanen <pav@xxxxxx> wrote: > > Add support for TX timestamping in Bluetooth ISO/L2CAP/SCO sockets. > > Add new COMPLETION timestamp type, to report a software timestamp when > the hardware reports a packet completed. (Cc netdev for this) > > Previous discussions: > https://lore.kernel.org/linux-bluetooth/cover.1739097311.git.pav@xxxxxx/ > https://lore.kernel.org/all/6642c7f3427b5_20539c2949a@xxxxxxxxxxxxxxxxxxxxxx.notmuch/ > https://lore.kernel.org/all/cover.1710440392.git.pav@xxxxxx/ > > Changes > ======= > v4: > - Change meaning of SOF_TIMESTAMPING_TX_COMPLETION, to save a bit in > skb_shared_info.tx_flags: > > It now enables COMPLETION only for packets that also have software SND > enabled. The flag can now be enabled only via a socket option, but > coupling with SND allows user to still choose for which packets > SND+COMPLETION should be generated. This choice maybe is OK for > applications which can skip SND if they're not interested. > > However, this would make the timestamping API not uniform, as the > other TX flags can be enabled via CMSG. > > IIUC, sizeof skb_shared_info cannot be easily changed and I'm not sure > there is somewhere else in general skb info, where one could safely > put the extra separate flag bit for COMPLETION. So here's alternative > suggestion. > > - Better name in sof_timestamping_names > > - I decided to keep using sockcm_init(), to avoid open coding READ_ONCE > and since it's passed to sock_cmsg_send() which anyway also may init > such fields. Please don't do this since the current sockcm_init() initializes more than you need. Please take a look at what the new sockcm_init looks like and what this series has done[1] which just got merged in recent days :) [1]: https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=aefd232de5eb2e7 Thanks, Jason > > v3: > - Add new COMPLETION timestamp type, and emit it in HCI completion. > - Emit SND instead of SCHED, when sending to driver. > - Do not emit SCHED timestamps. > - Don't safeguard tx_q length explicitly. Now that hci_sched_acl_blk() > is no more, the scheduler flow control is guaranteed to keep it > bounded. > - Fix L2CAP stream sockets to use the bytestream timestamp conventions. > > Overview > ======== > > The packet flow in Bluetooth is the following. Timestamps added here > indicated: > > user sendmsg() generates skbs > | > * skb waits in net/bluetooth queue for a free HW packet slot > | > * orphan skb, send to driver -> TSTAMP_SND > | > * driver: send packet data to transport (eg. USB) > | > * wait for transport completion > | > * driver: transport tx completion, free skb (some do this immediately) > | > * packet waits in HW side queue > | > * HCI report for packet completion -> TSTAMP_COMPLETION (for non-SCO) > > In addition, we may want to do the following in future (but not > implemented in this series as we don't have ISO sequence number > synchronization yet which is needed first, moreover e.g. Intel > controllers return only zeros in timestamps): > > * if packet is ISO, send HCI LE Read ISO TX Sync > | > * HCI response -> hardware TSTAMP_SND for the packet the response > corresponds to if it was waiting for one, might not be possible > to get a tstamp for every packet > > Bluetooth does not have tx timestamps in the completion reports from > hardware, and only for ISO packets there are HCI commands in > specification for querying timestamps afterward. > > The drivers do not provide ways to get timestamps either, I'm also not > aware if some devices would have vendor-specific commands to get them. > > Driver-side timestamps > ====================== > > Generating SND on driver side may be slightly more accurate, but that > requires changing the BT driver API to not orphan skbs first. In theory > this probably won't cause problems, but it is not done in this patchset. > > For some of the drivers it won't gain much. E.g. btusb immediately > submits the URB, so if one would emit SND just before submit (as > drivers/net/usb/usbnet.c does), it is essentially identical to emitting > before sending to driver. btintel_pcie looks like it does synchronous > send, so looks the same. hci_serdev has internal queue, iiuc flushing > as fast as data can be transferred, but it shouldn't be waiting for > hardware slots due to HCI flow control. > > Unless HW buffers are full, packets mostly wait on the HW side. E.g. > with btusb (non-SCO) median time from sendmsg() to URB generation is > ~0.1 ms, to USB completion ~0.5 ms, and HCI completion report at ~5 ms. > > The exception is SCO, for which HCI flow control is disabled, so they do > not get completion events so it's possible to build up queues inside the > driver. For SCO, COMPLETION needs to be generated from driver side, eg. > for btusb maybe at URB completion. This could be useful for SCO PCM > modes (but which are more or less obsolete nowadays), where USB isoc > data rate matches audio data rate, so queues on USB side may build up. > > Use cases > ========= > > In audio use cases we want to track and avoid queues building up, to > control latency, especially in cases like ISO where the controller has a > fixed schedule that the sending application must match. E.g. > application can aim to keep 1 packet in HW queue, so it has 2*7.5ms of > slack for being woken up too late. > > Applications can use SND & COMPLETION timestamps to track in-kernel and > in-HW packet queues separately. This can matter for ISO, where the > specification allows HW to use the timings when it gets packets to > determine what packets are synchronized together. Applications can use > SND to track that. > > Tests > ===== > > See > https://lore.kernel.org/linux-bluetooth/cover.1739026302.git.pav@xxxxxx/ > > Pauli Virtanen (5): > net-timestamp: COMPLETION timestamp on packet tx completion > Bluetooth: add support for skb TX SND/COMPLETION timestamping > Bluetooth: ISO: add TX timestamping > Bluetooth: L2CAP: add TX timestamping > Bluetooth: SCO: add TX timestamping socket-level mechanism > > Documentation/networking/timestamping.rst | 9 ++ > include/net/bluetooth/bluetooth.h | 1 + > include/net/bluetooth/hci_core.h | 13 +++ > include/net/bluetooth/l2cap.h | 3 +- > include/uapi/linux/errqueue.h | 1 + > include/uapi/linux/net_tstamp.h | 6 +- > net/bluetooth/6lowpan.c | 2 +- > net/bluetooth/hci_conn.c | 118 ++++++++++++++++++++++ > net/bluetooth/hci_core.c | 17 +++- > net/bluetooth/hci_event.c | 4 + > net/bluetooth/iso.c | 24 ++++- > net/bluetooth/l2cap_core.c | 41 +++++++- > net/bluetooth/l2cap_sock.c | 15 ++- > net/bluetooth/sco.c | 19 +++- > net/bluetooth/smp.c | 2 +- > net/core/sock.c | 2 + > net/ethtool/common.c | 1 + > 17 files changed, 258 insertions(+), 20 deletions(-) > > -- > 2.48.1 > >