net/bluetooth/iso.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
From: Yang Li <yang.li@amlogic.com>
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@amlogic.com>
---
Changes in v4:
- Optimizing the code
- Link to v3: https://lore.kernel.org/r/20250704-iso_ts-v3-1-2328bc602961@amlogic.com
Changes in v3:
- Change to use hwtimestamp
- Link to v2: https://lore.kernel.org/r/20250702-iso_ts-v2-1-723d199c8068@amlogic.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 | 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));
+
len = __le16_to_cpu(hdr->slen);
} else {
struct hci_iso_data_hdr *hdr;
---
base-commit: b8db3a9d4daeb7ff6a56c605ad6eca24e4da78ed
change-id: 20250421-iso_ts-c82a300ae784
Best regards,
--
Yang Li <yang.li@amlogic.com>
Hi Yang,
ma, 2025-07-07 kello 10:38 +0800, Yang Li via B4 Relay kirjoitti:
> From: Yang Li <yang.li@amlogic.com>
>
> 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@amlogic.com>
> ---
> Changes in v4:
> - Optimizing the code
> - Link to v3: https://lore.kernel.org/r/20250704-iso_ts-v3-1-2328bc602961@amlogic.com
>
> Changes in v3:
> - Change to use hwtimestamp
> - Link to v2: https://lore.kernel.org/r/20250702-iso_ts-v2-1-723d199c8068@amlogic.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 | 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.
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@iki.fi/
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
Hi Pauli,
On Tue, Jul 15, 2025 at 9:30 AM Pauli Virtanen <pav@iki.fi> wrote:
>
> Hi Yang,
>
> ma, 2025-07-07 kello 10:38 +0800, Yang Li via B4 Relay kirjoitti:
> > From: Yang Li <yang.li@amlogic.com>
> >
> > 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@amlogic.com>
> > ---
> > Changes in v4:
> > - Optimizing the code
> > - Link to v3: https://lore.kernel.org/r/20250704-iso_ts-v3-1-2328bc602961@amlogic.com
> >
> > Changes in v3:
> > - Change to use hwtimestamp
> > - Link to v2: https://lore.kernel.org/r/20250702-iso_ts-v2-1-723d199c8068@amlogic.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 | 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.
> 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@iki.fi/
> 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
Hi,
On Tue, Jul 15, 2025 at 9:37 AM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi Pauli,
>
> On Tue, Jul 15, 2025 at 9:30 AM Pauli Virtanen <pav@iki.fi> wrote:
> >
> > Hi Yang,
> >
> > ma, 2025-07-07 kello 10:38 +0800, Yang Li via B4 Relay kirjoitti:
> > > From: Yang Li <yang.li@amlogic.com>
> > >
> > > 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@amlogic.com>
> > > ---
> > > Changes in v4:
> > > - Optimizing the code
> > > - Link to v3: https://lore.kernel.org/r/20250704-iso_ts-v3-1-2328bc602961@amlogic.com
> > >
> > > Changes in v3:
> > > - Change to use hwtimestamp
> > > - Link to v2: https://lore.kernel.org/r/20250702-iso_ts-v2-1-723d199c8068@amlogic.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 | 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:
> > 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@iki.fi/
> > 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
Hi Luiz,
> [ EXTERNAL EMAIL ]
>
> Hi,
>
> On Tue, Jul 15, 2025 at 9:37 AM Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
>> Hi Pauli,
>>
>> On Tue, Jul 15, 2025 at 9:30 AM Pauli Virtanen <pav@iki.fi> wrote:
>>> Hi Yang,
>>>
>>> ma, 2025-07-07 kello 10:38 +0800, Yang Li via B4 Relay kirjoitti:
>>>> From: Yang Li <yang.li@amlogic.com>
>>>>
>>>> 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@amlogic.com>
>>>> ---
>>>> Changes in v4:
>>>> - Optimizing the code
>>>> - Link to v3: https://lore.kernel.org/r/20250704-iso_ts-v3-1-2328bc602961@amlogic.com
>>>>
>>>> Changes in v3:
>>>> - Change to use hwtimestamp
>>>> - Link to v2: https://lore.kernel.org/r/20250702-iso_ts-v2-1-723d199c8068@amlogic.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 | 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@iki.fi/
>>> 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
Hi，
Just a gentle ping regarding this patch.
Best regards,
Yang
> [ EXTERNAL EMAIL ]
>
> From: Yang Li <yang.li@amlogic.com>
>
> 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@amlogic.com>
> ---
> Changes in v4:
> - Optimizing the code
> - Link to v3: https://lore.kernel.org/r/20250704-iso_ts-v3-1-2328bc602961@amlogic.com
>
> Changes in v3:
> - Change to use hwtimestamp
> - Link to v2: https://lore.kernel.org/r/20250702-iso_ts-v2-1-723d199c8068@amlogic.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 | 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));
> +
> len = __le16_to_cpu(hdr->slen);
> } else {
> struct hci_iso_data_hdr *hdr;
>
> ---
> base-commit: b8db3a9d4daeb7ff6a56c605ad6eca24e4da78ed
> change-id: 20250421-iso_ts-c82a300ae784
>
> Best regards,
> --
> Yang Li <yang.li@amlogic.com>
>
>
© 2016 - 2025 Red Hat, Inc.