* [PATCH v4] Bluetooth: ISO: Support SCM_TIMESTAMPING for ISO TS
@ 2025-07-07 2:38 Yang Li via B4 Relay
2025-07-07 3:30 ` [v4] " bluez.test.bot
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Yang Li via B4 Relay @ 2025-07-07 2:38 UTC (permalink / raw)
To: Marcel Holtmann, Johan Hedberg, Luiz Augusto von Dentz,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman
Cc: linux-bluetooth, netdev, linux-kernel, Yang Li
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>
^ permalink raw reply related [flat|nested] 7+ messages in thread
* RE: [v4] Bluetooth: ISO: Support SCM_TIMESTAMPING for ISO TS
2025-07-07 2:38 [PATCH v4] Bluetooth: ISO: Support SCM_TIMESTAMPING for ISO TS Yang Li via B4 Relay
@ 2025-07-07 3:30 ` bluez.test.bot
2025-07-15 5:24 ` [PATCH v4] " Yang Li
2025-07-15 13:30 ` Pauli Virtanen
2 siblings, 0 replies; 7+ messages in thread
From: bluez.test.bot @ 2025-07-07 3:30 UTC (permalink / raw)
To: linux-bluetooth, yang.li
[-- Attachment #1: Type: text/plain, Size: 2456 bytes --]
This is automated email and please do not reply to this email!
Dear submitter,
Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=979522
---Test result---
Test Summary:
CheckPatch PENDING 0.27 seconds
GitLint PENDING 0.25 seconds
SubjectPrefix PASS 0.23 seconds
BuildKernel PASS 24.65 seconds
CheckAllWarning PASS 27.47 seconds
CheckSparse PASS 30.94 seconds
BuildKernel32 PASS 25.04 seconds
TestRunnerSetup PASS 475.90 seconds
TestRunner_l2cap-tester PASS 25.44 seconds
TestRunner_iso-tester PASS 40.61 seconds
TestRunner_bnep-tester PASS 6.01 seconds
TestRunner_mgmt-tester FAIL 132.02 seconds
TestRunner_rfcomm-tester PASS 9.62 seconds
TestRunner_sco-tester PASS 14.98 seconds
TestRunner_ioctl-tester PASS 10.20 seconds
TestRunner_mesh-tester FAIL 11.50 seconds
TestRunner_smp-tester PASS 8.72 seconds
TestRunner_userchan-tester PASS 6.41 seconds
IncrementalBuild PENDING 0.71 seconds
Details
##############################
Test: CheckPatch - PENDING
Desc: Run checkpatch.pl script
Output:
##############################
Test: GitLint - PENDING
Desc: Run gitlint
Output:
##############################
Test: TestRunner_mgmt-tester - FAIL
Desc: Run mgmt-tester with test-runner
Output:
Total: 490, Passed: 483 (98.6%), Failed: 3, Not Run: 4
Failed Test Cases
LL Privacy - Add Device 3 (AL is full) Failed 0.229 seconds
LL Privacy - Set Flags 1 (Add to RL) Failed 0.159 seconds
LL Privacy - Start Discovery 2 (Disable RL) Failed 0.203 seconds
##############################
Test: TestRunner_mesh-tester - FAIL
Desc: Run mesh-tester with test-runner
Output:
Total: 10, Passed: 8 (80.0%), Failed: 2, Not Run: 0
Failed Test Cases
Mesh - Send cancel - 1 Timed out 2.044 seconds
Mesh - Send cancel - 2 Timed out 1.998 seconds
##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series
Output:
---
Regards,
Linux Bluetooth
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v4] Bluetooth: ISO: Support SCM_TIMESTAMPING for ISO TS
2025-07-07 2:38 [PATCH v4] Bluetooth: ISO: Support SCM_TIMESTAMPING for ISO TS Yang Li via B4 Relay
2025-07-07 3:30 ` [v4] " bluez.test.bot
@ 2025-07-15 5:24 ` Yang Li
2025-07-15 13:30 ` Pauli Virtanen
2 siblings, 0 replies; 7+ messages in thread
From: Yang Li @ 2025-07-15 5:24 UTC (permalink / raw)
To: Marcel Holtmann, Johan Hedberg, Luiz Augusto von Dentz,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman
Cc: linux-bluetooth, netdev, linux-kernel
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>
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v4] Bluetooth: ISO: Support SCM_TIMESTAMPING for ISO TS
2025-07-07 2:38 [PATCH v4] Bluetooth: ISO: Support SCM_TIMESTAMPING for ISO TS Yang Li via B4 Relay
2025-07-07 3:30 ` [v4] " bluez.test.bot
2025-07-15 5:24 ` [PATCH v4] " Yang Li
@ 2025-07-15 13:30 ` Pauli Virtanen
2025-07-15 13:37 ` Luiz Augusto von Dentz
2 siblings, 1 reply; 7+ messages in thread
From: Pauli Virtanen @ 2025-07-15 13:30 UTC (permalink / raw)
To: yang.li, Marcel Holtmann, Johan Hedberg, Luiz Augusto von Dentz,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman
Cc: linux-bluetooth, netdev, linux-kernel
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v4] Bluetooth: ISO: Support SCM_TIMESTAMPING for ISO TS
2025-07-15 13:30 ` Pauli Virtanen
@ 2025-07-15 13:37 ` Luiz Augusto von Dentz
2025-07-15 13:57 ` Luiz Augusto von Dentz
0 siblings, 1 reply; 7+ messages in thread
From: Luiz Augusto von Dentz @ 2025-07-15 13:37 UTC (permalink / raw)
To: Pauli Virtanen
Cc: yang.li, Marcel Holtmann, Johan Hedberg, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
linux-bluetooth, netdev, linux-kernel
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v4] Bluetooth: ISO: Support SCM_TIMESTAMPING for ISO TS
2025-07-15 13:37 ` Luiz Augusto von Dentz
@ 2025-07-15 13:57 ` Luiz Augusto von Dentz
2025-07-16 1:45 ` Yang Li
0 siblings, 1 reply; 7+ messages in thread
From: Luiz Augusto von Dentz @ 2025-07-15 13:57 UTC (permalink / raw)
To: Pauli Virtanen
Cc: yang.li, Marcel Holtmann, Johan Hedberg, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
linux-bluetooth, netdev, linux-kernel
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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v4] Bluetooth: ISO: Support SCM_TIMESTAMPING for ISO TS
2025-07-15 13:57 ` Luiz Augusto von Dentz
@ 2025-07-16 1:45 ` Yang Li
0 siblings, 0 replies; 7+ messages in thread
From: Yang Li @ 2025-07-16 1:45 UTC (permalink / raw)
To: Luiz Augusto von Dentz, Pauli Virtanen
Cc: Marcel Holtmann, Johan Hedberg, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, linux-bluetooth,
netdev, linux-kernel
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
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-07-16 1:45 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-07 2:38 [PATCH v4] Bluetooth: ISO: Support SCM_TIMESTAMPING for ISO TS Yang Li via B4 Relay
2025-07-07 3:30 ` [v4] " bluez.test.bot
2025-07-15 5:24 ` [PATCH v4] " Yang Li
2025-07-15 13:30 ` Pauli Virtanen
2025-07-15 13:37 ` Luiz Augusto von Dentz
2025-07-15 13:57 ` Luiz Augusto von Dentz
2025-07-16 1:45 ` Yang Li
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox