linux-kernel - Re: [PATCH v2] Bluetooth: ISO: Support SOCK

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <0CE441B2-0CB2-467D-A2AE-0BD37EDACEEA@iki.fi>
Date: Thu, 03 Jul 2025 17:46:46 +0000
From: Pauli Virtanen <pauli.virtanen@....fi>
To: Yang Li <yang.li@...ogic.com>, Marcel Holtmann <marcel@...tmann.org>,
 Johan Hedberg <johan.hedberg@...il.com>,
 Luiz Augusto von Dentz <luiz.dentz@...il.com>,
 "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
 Simon Horman <horms@...nel.org>
CC: linux-bluetooth@...r.kernel.org, netdev@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] Bluetooth: ISO: Support SOCK_RCVTSTAMP via CMSG for ISO sockets

Hi,

3. heinäkuuta 2025 10.34.46 UTC Yang Li <yang.li@...ogic.com> kirjoitti:
>Hi,
>> [ EXTERNAL EMAIL ]
>> 
>> Hi,
>> 
>> ke, 2025-07-02 kello 19:35 +0800, Yang Li via B4 Relay kirjoitti:
>>> From: Yang Li <yang.li@...ogic.com>
>>> 
>>> User-space applications (e.g., PipeWire) depend on
>>> ISO-formatted timestamps for precise audio sync.
>>> 
>>> Signed-off-by: Yang Li <yang.li@...ogic.com>
>>> ---
>>> Changes in v2:
>>> - Support SOCK_RCVTSTAMPNS via CMSG for ISO sockets
>>> - Link to v1: https://lore.kernel.org/r/20250429-iso_ts-v1-1-e586f30de6cb@amlogic.com
>>> ---
>>>   net/bluetooth/iso.c | 3 +++
>>>   1 file changed, 3 insertions(+)
>>> 
>>> diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c
>>> index fc22782cbeeb..6927c593a1d6 100644
>>> --- a/net/bluetooth/iso.c
>>> +++ b/net/bluetooth/iso.c
>>> @@ -2308,6 +2308,9 @@ void iso_recv(struct hci_conn *hcon, struct sk_buff *skb, u16 flags)
>>>                                goto drop;
>>>                        }
>>> 
>>> +                     /* Record the timestamp to skb*/
>>> +                     skb->skb_mstamp_ns = le32_to_cpu(hdr->ts);
>> Hardware timestamps are supposed to go in
>> 
>>          skb_hwtstamps(skb)->hwtstamp
>> 
>> See Documentation/networking/timestamping.rst
>> "3.1 Hardware Timestamping Implementation: Device Drivers" and how it
>> is done in drivers/net/
>> 
>> This documentation also explains how user applications can obtain the
>> hardware timestamps.
>> 
>> AFAIK, skb->tstamp (skb->skb_mstamp_ns is union for it) must be in
>> system clock. The hdr->ts is in some unsynchronized controller clock,
>> so they should go to HW timestamps.
>
>
>Following your suggestion, I switched to hwtstamp but kept SO_TIMESTAMPNS on the PipeWire side.
>
>+                       struct skb_shared_hwtstamps *hwts = skb_hwtstamps(skb);
>+                       if (hwts)
>+                               hwts->hwtstamp = us_to_ktime(le32_to_cpu(hdr->ts));
>+
>
>The value I get is unexpectedly large and not the same as the timestamp in the ISO data.

The value given by SO_TIMESTAMPNS is the system clock time when kernel received the packet.

It's not the same as the ISO timestamp, as the ISO TS is in controller clock. This is normal. Applications generally need timestamps in system clock.

The two clocks are not synchronized. There is an unknown offset between system clock and ISO TS times. AFAIK there is no specified way to find what it is precisely (= synchronize clocks) using HCI. This appears implementation-defined. So I think kernel should report both timestamps, and leave it to applications to try to correlate them.

Note that master branch of Pipewire already uses the kernel-provided RX timestamps. It can use also the HW timestamps after they are added. They likely improve CIS synchronization, as it's then unambiguous which packets belong together and we can correlate all CIS using same clock matching.

>
>read_data: received timestamp: 880608.479272966
>read_data: received timestamp: 880608.479438633
>read_data: received timestamp: 880608.489259466
>read_data: received timestamp: 880608.489434550
>read_data: received timestamp: 880608.499289258
>read_data: received timestamp: 880608.499464550
>read_data: received timestamp: 880608.509278008
>read_data: received timestamp: 880608.509451425
>read_data: received timestamp: 880608.519261175
>read_data: received timestamp: 880608.519438633
>read_data: received timestamp: 880608.529385008
>read_data: received timestamp: 880608.529462133
>read_data: received timestamp: 880608.539273758
>read_data: received timestamp: 880608.539452758
>read_data: received timestamp: 880608.549271258
>read_data: received timestamp: 880608.549450008
>read_data: received timestamp: 880608.559263466
>read_data: received timestamp: 880608.559443216
>read_data: received timestamp: 880608.569257466
>
>
>Is there any special processing in the application code?
>
>> 
>>> +
>>>                        len = __le16_to_cpu(hdr->slen);
>>>                } else {
>>>                        struct hci_iso_data_hdr *hdr;
>>> 
>>> ---
>>> base-commit: 3bc46213b81278f3a9df0324768e152de71eb9fe
>>> change-id: 20250421-iso_ts-c82a300ae784
>>> 
>>> Best regards,
>> --
>> Pauli Virtanen