Hi Pauli, On Wed, Feb 19, 2025 at 7:43 PM Jason Xing <kerneljasonxing@xxxxxxxxx> wrote: > > 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 > > > > We are sort of running out of time, if we want to be able to merge by the next merge window then it must be done this week. -- Luiz Augusto von Dentz
