Hi Pauli,
On Mon, Feb 20, 2023 at 11:42 AM Pauli Virtanen <pav@xxxxxx> wrote:
>
> Use correct HCI ISO data packet header struct when the packet has
> timestamp. The timestamp, when present, goes before the other fields
> (Core v5.3 4E 5.4.5), so the structs are not compatible.
>
> Fixes: ccf74f2390d6 ("Bluetooth: Add BTPROTO_ISO socket type")
> Signed-off-by: Pauli Virtanen <pav@xxxxxx>
> ---
>
> Notes:
> My hardware doesn't seem to produce timestamped packets, so this is not
> properly tested, except to the extent that it doesn't break the
> non-timestamped code path.
>
> Regardless, the current state of things looks wrong, so sending this to
> the list in any case.
Perhaps we should first attempt to enable this in the emulator, Id use
BT_HCI_CMD_LE_READ_ISO_TX_SYNC as hint that the host is interested in
knowing the timestamp:
https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/emulator/btdev.c#n5752
> net/bluetooth/iso.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c
> index 2dabef488eaa..cb959e8eac18 100644
> --- a/net/bluetooth/iso.c
> +++ b/net/bluetooth/iso.c
> @@ -1621,7 +1621,6 @@ 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 hci_iso_data_hdr *hdr;
> __u16 pb, ts, len;
>
> if (!conn)
> @@ -1643,6 +1642,8 @@ 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? */
Perhaps we should use skb_set_delivery_time to set the timestamp as
received by the controller? That said I don't know if the unit would
be compatible.
> hdr = skb_pull_data(skb, HCI_ISO_TS_DATA_HDR_SIZE);
> if (!hdr) {
> @@ -1650,15 +1651,19 @@ void iso_recv(struct hci_conn *hcon, struct sk_buff *skb, u16 flags)
> goto drop;
> }
>
> + len = __le16_to_cpu(hdr->slen);
> } else {
> + struct hci_iso_data_hdr *hdr;
> +
> hdr = skb_pull_data(skb, HCI_ISO_DATA_HDR_SIZE);
> if (!hdr) {
> BT_ERR("Frame is too short (len %d)", skb->len);
> goto drop;
> }
> +
> + len = __le16_to_cpu(hdr->slen);
> }
>
> - len = __le16_to_cpu(hdr->slen);
> flags = hci_iso_data_flags(len);
> len = hci_iso_data_len(len);
>
> --
> 2.39.2
>
--
Luiz Augusto von Dentz
