Hi Luiz,
[ EXTERNAL EMAIL ]
Hi,
On Tue, Jul 15, 2025 at 9:37 AM Luiz Augusto von Dentz
<luiz.dentz@xxxxxxxxx> wrote:
Hi Pauli,
On Tue, Jul 15, 2025 at 9:30 AM Pauli Virtanen <pav@xxxxxx> wrote:
Hi Yang,
ma, 2025-07-07 kello 10:38 +0800, Yang Li via B4 Relay kirjoitti:
From: Yang Li <yang.li@xxxxxxxxxxx>
User-space applications (e.g. PipeWire) depend on
ISO-formatted timestamps for precise audio sync.
The ISO ts is based on the controller’s clock domain,
so hardware timestamping (hwtimestamp) must be used.
Ref: Documentation/networking/timestamping.rst,
section 3.1 Hardware Timestamping.
Signed-off-by: Yang Li <yang.li@xxxxxxxxxxx>
---
Changes in v4:
- Optimizing the code
- Link to v3: https://lore.kernel.org/r/20250704-iso_ts-v3-1-2328bc602961@xxxxxxxxxxx
Changes in v3:
- Change to use hwtimestamp
- Link to v2: https://lore.kernel.org/r/20250702-iso_ts-v2-1-723d199c8068@xxxxxxxxxxx
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@xxxxxxxxxxx
---
net/bluetooth/iso.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c
index fc22782cbeeb..677144bb6b94 100644
--- a/net/bluetooth/iso.c
+++ b/net/bluetooth/iso.c
@@ -2278,6 +2278,7 @@ static void iso_disconn_cfm(struct hci_conn *hcon, __u8 reason)
void iso_recv(struct hci_conn *hcon, struct sk_buff *skb, u16 flags)
{
struct iso_conn *conn = hcon->iso_data;
+ struct skb_shared_hwtstamps *hwts;
__u16 pb, ts, len;
if (!conn)
@@ -2301,13 +2302,16 @@ void iso_recv(struct hci_conn *hcon, struct sk_buff *skb, u16 flags)
if (ts) {
struct hci_iso_ts_data_hdr *hdr;
- /* TODO: add timestamp to the packet? */
hdr = skb_pull_data(skb, HCI_ISO_TS_DATA_HDR_SIZE);
if (!hdr) {
BT_ERR("Frame is too short (len %d)", skb->len);
goto drop;
}
+ /* Record the timestamp to skb*/
+ hwts = skb_hwtstamps(skb);
+ hwts->hwtstamp = us_to_ktime(le32_to_cpu(hdr->ts));
Several lines below there is
conn->rx_skb = bt_skb_alloc(len, GFP_KERNEL);
skb_copy_from_linear_data(skb, skb_put(conn->rx_skb, skb-
len),
skb->len);
so timestamp should be copied explicitly also into conn->rx_skb,
otherwise it gets lost when you have ACL-fragmented ISO packets.
Yep, it is not that the code is completely wrong but it is operating
on the original skb not in the rx_skb as you said, that said is only
the first fragment that contains the ts header so we only have to do
it once in case that was not clear.
I might just do a fixup myself, something like the following:
diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c
index 0a951c6514af..f48fb62e640d 100644
--- a/net/bluetooth/iso.c
+++ b/net/bluetooth/iso.c
@@ -2374,6 +2374,13 @@ void iso_recv(struct hci_conn *hcon, struct
sk_buff *skb, u16 flags)
skb_copy_from_linear_data(skb, skb_put(conn->rx_skb, skb->len),
skb->len);
conn->rx_len = len - skb->len;
+
+ /* Copy timestamp from skb to rx_skb if present */
+ if (ts) {
+ hwts = skb_hwtstamps(conn->rx_skb);
+ hwts->hwtstamp = skb_hwtstamps(skb)->hwtstamp;
+ }
+
break;
case ISO_CONT:
Well, that's great!
Thanks so much for your help.
It could also be useful to write a simple test case that extracts the
timestamp from CMSG, see for example how it was done for BT_PKT_SEQNUM:
https://lore.kernel.org/linux-bluetooth/b98b7691e4ba06550bb8f275cad0635bc9e4e8d2.1752511478.git.pav@xxxxxx/
bthost_send_iso() can take ts=true and some timestamp value.
+
len = __le16_to_cpu(hdr->slen);
} else {
struct hci_iso_data_hdr *hdr;
---
base-commit: b8db3a9d4daeb7ff6a56c605ad6eca24e4da78ed
change-id: 20250421-iso_ts-c82a300ae784
Best regards,
--
Pauli Virtanen
--
Luiz Augusto von Dentz
--
Luiz Augusto von Dentz
