| Message ID | 20240220163531.2900-1-iulia.tanasescu@nxp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC] Bluetooth: ISO: Reassemble fragmented BASE | expand |
| Context | Check | Description |
|---|---|---|
| tedd_an/pre-ci_am | success | Success |
| tedd_an/CheckPatch | success | CheckPatch PASS |
| tedd_an/GitLint | success | Gitlint PASS |
| tedd_an/SubjectPrefix | success | Gitlint PASS |
| tedd_an/BuildKernel | success | BuildKernel PASS |
| tedd_an/CheckAllWarning | success | CheckAllWarning PASS |
| tedd_an/CheckSparse | success | CheckSparse PASS |
| tedd_an/CheckSmatch | success | CheckSparse PASS |
| tedd_an/BuildKernel32 | success | BuildKernel32 PASS |
| tedd_an/TestRunnerSetup | success | TestRunnerSetup PASS |
| tedd_an/TestRunner_l2cap-tester | success | TestRunner PASS |
| tedd_an/TestRunner_iso-tester | success | TestRunner PASS |
| tedd_an/TestRunner_bnep-tester | success | TestRunner PASS |
| tedd_an/TestRunner_mgmt-tester | fail | TestRunner_mgmt-tester: Total: 492, Passed: 489 (99.4%), Failed: 2, Not Run: 1 |
| tedd_an/TestRunner_rfcomm-tester | success | TestRunner PASS |
| tedd_an/TestRunner_sco-tester | success | TestRunner PASS |
| tedd_an/TestRunner_ioctl-tester | success | TestRunner PASS |
| tedd_an/TestRunner_mesh-tester | success | TestRunner PASS |
| tedd_an/TestRunner_smp-tester | success | TestRunner PASS |
| tedd_an/TestRunner_userchan-tester | success | TestRunner PASS |
| tedd_an/IncrementalBuild | success | Incremental Build PASS |
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=827921 ---Test result--- Test Summary: CheckPatch PASS 0.97 seconds GitLint PASS 0.32 seconds SubjectPrefix PASS 0.12 seconds BuildKernel PASS 28.34 seconds CheckAllWarning PASS 31.40 seconds CheckSparse PASS 36.48 seconds CheckSmatch PASS 99.30 seconds BuildKernel32 PASS 27.78 seconds TestRunnerSetup PASS 507.72 seconds TestRunner_l2cap-tester PASS 18.21 seconds TestRunner_iso-tester PASS 31.13 seconds TestRunner_bnep-tester PASS 4.87 seconds TestRunner_mgmt-tester FAIL 166.83 seconds TestRunner_rfcomm-tester PASS 7.45 seconds TestRunner_sco-tester PASS 15.06 seconds TestRunner_ioctl-tester PASS 7.92 seconds TestRunner_mesh-tester PASS 6.02 seconds TestRunner_smp-tester PASS 6.97 seconds TestRunner_userchan-tester PASS 5.21 seconds IncrementalBuild PASS 26.76 seconds Details ############################## Test: TestRunner_mgmt-tester - FAIL Desc: Run mgmt-tester with test-runner Output: Total: 492, Passed: 489 (99.4%), Failed: 2, Not Run: 1 Failed Test Cases LL Privacy - Add Device 5 (2 Devices to RL) Failed 0.123 seconds LL Privacy - Remove Device 2 (Remove from RL) Timed out 2.779 seconds --- Regards, Linux Bluetooth
Hi Iulia, On Tue, Feb 20, 2024 at 11:35 AM Iulia Tanasescu <iulia.tanasescu@nxp.com> wrote: > > For a Broadcast Sink, this adds support for reassembling fragmented PA > data. This is needed if the BASE comes fragmented in multiple PA reports. > After all PA fragments have been reassembled, the BASE can be retrieved > by the user via getsockopt. > > PA fragments could be reassembled inside hci_event.c and a complete PA > report could be indicated to iso.c, or like it is currently, if PA > fragments are indicated separately, reassembly would need to be done in > iso.c. > > For a Broadcast Source, the user sets the BASE via setsockopt. The full > PA data is built at bis bind, inside conn->le_per_adv_data. This array > would be fit to hold the reassembled PA data on the Sink side as well, > but a listening socket does not have an associated hcon...Currently for > the Broadcast Sink, a hcon is created only after the first BIGInfo report, > which arrives after the PA sync established event and all initial PA > reports. The hcon is notified to iso.c, a child socket is created and it > is notified to userspace, to mark the PA sync step as completed. > > I see 2 possibilities: > > 1. Keep a pa_data array inside the iso sock struct, use it to reassemble > PA fragments, and extract BASE from it when needed. > > 2. Instead of creating the hcon after the first BIGInfo report, the hcon > could be created when the PA sync established event is received. The > le_per_adv_data can be used to reassemble the PA fragments, and the hcon > will be notified to iso.c after the first BIGInfo report, when all > information necessary to create and populate the child socket is > available. > > In this patch I am showing the first idea, but I think the second one > may provide a design more in line with the broadcast source scenario, > when handling PA data and BASE. > > Are there any opinions about this? I was actually going to ask about why we don't have an hcon at the early stages of PA Sync, Im doing some optimizations so we scan based on the QoS PHY which should be faster than scanning in all support PHYs like we do today, but at that point there is no hcon and therefore the is no visible QoS since the socket info is not public. > Signed-off-by: Iulia Tanasescu <iulia.tanasescu@nxp.com> > --- > include/net/bluetooth/hci.h | 7 +++- > net/bluetooth/iso.c | 79 +++++++++++++++++++++++++++---------- > 2 files changed, 64 insertions(+), 22 deletions(-) > > diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h > index 08cb5cb249a4..a856e88719d2 100644 > --- a/include/net/bluetooth/hci.h > +++ b/include/net/bluetooth/hci.h > @@ -1,7 +1,7 @@ > /* > BlueZ - Bluetooth protocol stack for Linux > Copyright (C) 2000-2001 Qualcomm Incorporated > - Copyright 2023 NXP > + Copyright 2023-2024 NXP > > Written 2000,2001 by Maxim Krasnyansky <maxk@qualcomm.com> > > @@ -2037,6 +2037,7 @@ struct hci_cp_le_set_per_adv_params { > } __packed; > > #define HCI_MAX_PER_AD_LENGTH 252 > +#define HCI_MAX_PER_AD_TOT_LENGTH 1650 > > #define HCI_OP_LE_SET_PER_ADV_DATA 0x203f > struct hci_cp_le_set_per_adv_data { > @@ -2797,6 +2798,10 @@ struct hci_ev_le_per_adv_report { > __u8 data[]; > } __packed; > > +#define LE_PA_DATA_COMPLETE 0x00 > +#define LE_PA_DATA_MORE_TO_COME 0x01 > +#define LE_PA_DATA_TRUNCATED 0x02 > + > #define HCI_EV_LE_EXT_ADV_SET_TERM 0x12 > struct hci_evt_le_ext_adv_set_term { > __u8 status; > diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c > index 04f6572d35f1..e9a3b4f74e2f 100644 > --- a/net/bluetooth/iso.c > +++ b/net/bluetooth/iso.c > @@ -3,7 +3,7 @@ > * BlueZ - Bluetooth protocol stack for Linux > * > * Copyright (C) 2022 Intel Corporation > - * Copyright 2023 NXP > + * Copyright 2023-2024 NXP > */ > > #include <linux/module.h> > @@ -70,8 +70,15 @@ struct iso_pinfo { > unsigned long flags; > struct bt_iso_qos qos; > bool qos_user_set; > - __u8 base_len; > - __u8 base[BASE_MAX_LENGTH]; > + union { > + __u8 base_len; > + __u16 pa_data_len; > + }; > + union { > + __u8 base[BASE_MAX_LENGTH]; > + __u8 pa_data[HCI_MAX_PER_AD_TOT_LENGTH]; > + }; > + __u16 pa_data_offset; > struct iso_conn *conn; > }; > > @@ -1573,7 +1580,7 @@ static int iso_sock_getsockopt(struct socket *sock, int level, int optname, > struct sock *sk = sock->sk; > int len, err = 0; > struct bt_iso_qos *qos; > - u8 base_len; > + size_t base_len; > u8 *base; > > BT_DBG("sk %p", sk); > @@ -1612,13 +1619,20 @@ static int iso_sock_getsockopt(struct socket *sock, int level, int optname, > break; > > case BT_ISO_BASE: > - if (sk->sk_state == BT_CONNECTED && > - !bacmp(&iso_pi(sk)->dst, BDADDR_ANY)) { > - base_len = iso_pi(sk)->conn->hcon->le_per_adv_data_len; > - base = iso_pi(sk)->conn->hcon->le_per_adv_data; > - } else { > + if (!bacmp(&iso_pi(sk)->dst, BDADDR_ANY)) { > + /* For a Broadcast Source, the BASE was stored > + * in iso_pi(sk)->base. > + */ > base_len = iso_pi(sk)->base_len; > base = iso_pi(sk)->base; > + } else { > + /* For a Broadcast Sink, the complete data received in > + * PA reports is stored. Extract BASE from there. > + */ > + base = eir_get_service_data(iso_pi(sk)->pa_data, > + iso_pi(sk)->pa_data_len, > + EIR_BAA_SERVICE_UUID, > + &base_len); > } > > len = min_t(unsigned int, len, base_len); > @@ -1834,8 +1848,9 @@ static void iso_conn_ready(struct iso_conn *conn) > bacpy(&iso_pi(sk)->dst, &hcon->dst); > iso_pi(sk)->dst_type = hcon->dst_type; > iso_pi(sk)->sync_handle = iso_pi(parent)->sync_handle; > - memcpy(iso_pi(sk)->base, iso_pi(parent)->base, iso_pi(parent)->base_len); > - iso_pi(sk)->base_len = iso_pi(parent)->base_len; > + memcpy(iso_pi(sk)->pa_data, iso_pi(parent)->pa_data, > + iso_pi(parent)->pa_data_len); > + iso_pi(sk)->pa_data_len = iso_pi(parent)->pa_data_len; > > hci_conn_hold(hcon); > iso_chan_add(conn, sk, parent); > @@ -1904,8 +1919,8 @@ int iso_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr, __u8 *flags) > * a BIG Info it attempts to check if there any listening socket with > * the same sync_handle and if it does then attempt to create a sync. > * 3. HCI_EV_LE_PER_ADV_REPORT: When a PA report is received, it is stored > - * in iso_pi(sk)->base so it can be passed up to user, in the case of a > - * broadcast sink. > + * in iso_pi(sk)->pa_data so the BASE can later be passed up to user, in > + * the case of a broadcast sink. > */ > ev1 = hci_recv_event_data(hdev, HCI_EV_LE_PA_SYNC_ESTABLISHED); > if (ev1) { > @@ -1961,16 +1976,38 @@ int iso_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr, __u8 *flags) > > ev3 = hci_recv_event_data(hdev, HCI_EV_LE_PER_ADV_REPORT); > if (ev3) { > - size_t base_len = ev3->length; > - u8 *base; > - > sk = iso_get_sock_listen(&hdev->bdaddr, bdaddr, > iso_match_sync_handle_pa_report, ev3); > - base = eir_get_service_data(ev3->data, ev3->length, > - EIR_BAA_SERVICE_UUID, &base_len); > - if (base && sk && base_len <= sizeof(iso_pi(sk)->base)) { > - memcpy(iso_pi(sk)->base, base, base_len); > - iso_pi(sk)->base_len = base_len; > + > + if (!sk) > + goto done; > + > + if (ev3->data_status == LE_PA_DATA_TRUNCATED) { > + /* The controller was unable to retrieve PA data. */ > + memset(iso_pi(sk)->pa_data, 0, > + HCI_MAX_PER_AD_TOT_LENGTH); > + iso_pi(sk)->pa_data_len = 0; > + iso_pi(sk)->pa_data_offset = 0; > + return lm; > + } > + > + if (iso_pi(sk)->pa_data_offset + ev3->length > > + HCI_MAX_PER_AD_TOT_LENGTH) > + goto done; > + > + memcpy(iso_pi(sk)->pa_data + iso_pi(sk)->pa_data_offset, > + ev3->data, ev3->length); > + iso_pi(sk)->pa_data_offset += ev3->length; > + > + if (ev3->data_status == LE_PA_DATA_COMPLETE) { > + /* All PA data has been received. */ > + iso_pi(sk)->pa_data_len = iso_pi(sk)->pa_data_offset; > + iso_pi(sk)->pa_data_offset = 0; > + } else { > + /* This is a PA data fragment. Keep pa_data_len set to 0 > + * until all data has been reassembled. > + */ > + iso_pi(sk)->pa_data_len = 0; > } > } else { > sk = iso_get_sock_listen(&hdev->bdaddr, BDADDR_ANY, NULL, NULL); > -- > 2.39.2 >
Hi Luiz, > -----Original Message----- > From: Luiz Augusto von Dentz <luiz.dentz@gmail.com> > Sent: Wednesday, February 21, 2024 4:44 PM > To: Iulia Tanasescu <iulia.tanasescu@nxp.com> > Cc: linux-bluetooth@vger.kernel.org; Claudia Cristina Draghicescu > <claudia.rosu@nxp.com>; Mihai-Octavian Urzica <mihai- > octavian.urzica@nxp.com>; Silviu Florian Barbulescu > <silviu.barbulescu@nxp.com>; Vlad Pruteanu <vlad.pruteanu@nxp.com>; > Andrei Istodorescu <andrei.istodorescu@nxp.com> > Subject: Re: [RFC] Bluetooth: ISO: Reassemble fragmented BASE > > Hi Iulia, > > On Tue, Feb 20, 2024 at 11:35 AM Iulia Tanasescu > <iulia.tanasescu@nxp.com> wrote: > > > > For a Broadcast Sink, this adds support for reassembling fragmented PA > > data. This is needed if the BASE comes fragmented in multiple PA reports. > > After all PA fragments have been reassembled, the BASE can be retrieved > > by the user via getsockopt. > > > > PA fragments could be reassembled inside hci_event.c and a complete PA > > report could be indicated to iso.c, or like it is currently, if PA > > fragments are indicated separately, reassembly would need to be done in > > iso.c. > > > > For a Broadcast Source, the user sets the BASE via setsockopt. The full > > PA data is built at bis bind, inside conn->le_per_adv_data. This array > > would be fit to hold the reassembled PA data on the Sink side as well, > > but a listening socket does not have an associated hcon...Currently for > > the Broadcast Sink, a hcon is created only after the first BIGInfo report, > > which arrives after the PA sync established event and all initial PA > > reports. The hcon is notified to iso.c, a child socket is created and it > > is notified to userspace, to mark the PA sync step as completed. > > > > I see 2 possibilities: > > > > 1. Keep a pa_data array inside the iso sock struct, use it to reassemble > > PA fragments, and extract BASE from it when needed. > > > > 2. Instead of creating the hcon after the first BIGInfo report, the hcon > > could be created when the PA sync established event is received. The > > le_per_adv_data can be used to reassemble the PA fragments, and the > hcon > > will be notified to iso.c after the first BIGInfo report, when all > > information necessary to create and populate the child socket is > > available. > > > > In this patch I am showing the first idea, but I think the second one > > may provide a design more in line with the broadcast source scenario, > > when handling PA data and BASE. > > > > Are there any opinions about this? > > I was actually going to ask about why we don't have an hcon at the > early stages of PA Sync, Im doing some optimizations so we scan based > on the QoS PHY which should be faster than scanning in all support > PHYs like we do today, but at that point there is no hcon and > therefore the is no visible QoS since the socket info is not public. I agree, so similar to iso_connect_bis, I will create a hcon for the parent socket at iso_listen_bis, and I will use hcon->le_per_adv_data to reassemble PA reports. > > > Signed-off-by: Iulia Tanasescu <iulia.tanasescu@nxp.com> > > --- > > include/net/bluetooth/hci.h | 7 +++- > > net/bluetooth/iso.c | 79 +++++++++++++++++++++++++++---------- > > 2 files changed, 64 insertions(+), 22 deletions(-) > > > > diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h > > index 08cb5cb249a4..a856e88719d2 100644 > > --- a/include/net/bluetooth/hci.h > > +++ b/include/net/bluetooth/hci.h > > @@ -1,7 +1,7 @@ > > /* > > BlueZ - Bluetooth protocol stack for Linux > > Copyright (C) 2000-2001 Qualcomm Incorporated > > - Copyright 2023 NXP > > + Copyright 2023-2024 NXP > > > > Written 2000,2001 by Maxim Krasnyansky <maxk@qualcomm.com> > > > > @@ -2037,6 +2037,7 @@ struct hci_cp_le_set_per_adv_params { > > } __packed; > > > > #define HCI_MAX_PER_AD_LENGTH 252 > > +#define HCI_MAX_PER_AD_TOT_LENGTH 1650 > > > > #define HCI_OP_LE_SET_PER_ADV_DATA 0x203f > > struct hci_cp_le_set_per_adv_data { > > @@ -2797,6 +2798,10 @@ struct hci_ev_le_per_adv_report { > > __u8 data[]; > > } __packed; > > > > +#define LE_PA_DATA_COMPLETE 0x00 > > +#define LE_PA_DATA_MORE_TO_COME 0x01 > > +#define LE_PA_DATA_TRUNCATED 0x02 > > + > > #define HCI_EV_LE_EXT_ADV_SET_TERM 0x12 > > struct hci_evt_le_ext_adv_set_term { > > __u8 status; > > diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c > > index 04f6572d35f1..e9a3b4f74e2f 100644 > > --- a/net/bluetooth/iso.c > > +++ b/net/bluetooth/iso.c > > @@ -3,7 +3,7 @@ > > * BlueZ - Bluetooth protocol stack for Linux > > * > > * Copyright (C) 2022 Intel Corporation > > - * Copyright 2023 NXP > > + * Copyright 2023-2024 NXP > > */ > > > > #include <linux/module.h> > > @@ -70,8 +70,15 @@ struct iso_pinfo { > > unsigned long flags; > > struct bt_iso_qos qos; > > bool qos_user_set; > > - __u8 base_len; > > - __u8 base[BASE_MAX_LENGTH]; > > + union { > > + __u8 base_len; > > + __u16 pa_data_len; > > + }; > > + union { > > + __u8 base[BASE_MAX_LENGTH]; > > + __u8 pa_data[HCI_MAX_PER_AD_TOT_LENGTH]; > > + }; > > + __u16 pa_data_offset; > > struct iso_conn *conn; > > }; > > > > @@ -1573,7 +1580,7 @@ static int iso_sock_getsockopt(struct socket > *sock, int level, int optname, > > struct sock *sk = sock->sk; > > int len, err = 0; > > struct bt_iso_qos *qos; > > - u8 base_len; > > + size_t base_len; > > u8 *base; > > > > BT_DBG("sk %p", sk); > > @@ -1612,13 +1619,20 @@ static int iso_sock_getsockopt(struct socket > *sock, int level, int optname, > > break; > > > > case BT_ISO_BASE: > > - if (sk->sk_state == BT_CONNECTED && > > - !bacmp(&iso_pi(sk)->dst, BDADDR_ANY)) { > > - base_len = iso_pi(sk)->conn->hcon->le_per_adv_data_len; > > - base = iso_pi(sk)->conn->hcon->le_per_adv_data; > > - } else { > > + if (!bacmp(&iso_pi(sk)->dst, BDADDR_ANY)) { > > + /* For a Broadcast Source, the BASE was stored > > + * in iso_pi(sk)->base. > > + */ > > base_len = iso_pi(sk)->base_len; > > base = iso_pi(sk)->base; > > + } else { > > + /* For a Broadcast Sink, the complete data received in > > + * PA reports is stored. Extract BASE from there. > > + */ > > + base = eir_get_service_data(iso_pi(sk)->pa_data, > > + iso_pi(sk)->pa_data_len, > > + EIR_BAA_SERVICE_UUID, > > + &base_len); > > } > > > > len = min_t(unsigned int, len, base_len); > > @@ -1834,8 +1848,9 @@ static void iso_conn_ready(struct iso_conn > *conn) > > bacpy(&iso_pi(sk)->dst, &hcon->dst); > > iso_pi(sk)->dst_type = hcon->dst_type; > > iso_pi(sk)->sync_handle = iso_pi(parent)->sync_handle; > > - memcpy(iso_pi(sk)->base, iso_pi(parent)->base, iso_pi(parent)- > >base_len); > > - iso_pi(sk)->base_len = iso_pi(parent)->base_len; > > + memcpy(iso_pi(sk)->pa_data, iso_pi(parent)->pa_data, > > + iso_pi(parent)->pa_data_len); > > + iso_pi(sk)->pa_data_len = iso_pi(parent)->pa_data_len; > > > > hci_conn_hold(hcon); > > iso_chan_add(conn, sk, parent); > > @@ -1904,8 +1919,8 @@ int iso_connect_ind(struct hci_dev *hdev, > bdaddr_t *bdaddr, __u8 *flags) > > * a BIG Info it attempts to check if there any listening socket with > > * the same sync_handle and if it does then attempt to create a sync. > > * 3. HCI_EV_LE_PER_ADV_REPORT: When a PA report is received, it is > stored > > - * in iso_pi(sk)->base so it can be passed up to user, in the case of a > > - * broadcast sink. > > + * in iso_pi(sk)->pa_data so the BASE can later be passed up to user, in > > + * the case of a broadcast sink. > > */ > > ev1 = hci_recv_event_data(hdev, HCI_EV_LE_PA_SYNC_ESTABLISHED); > > if (ev1) { > > @@ -1961,16 +1976,38 @@ int iso_connect_ind(struct hci_dev *hdev, > bdaddr_t *bdaddr, __u8 *flags) > > > > ev3 = hci_recv_event_data(hdev, HCI_EV_LE_PER_ADV_REPORT); > > if (ev3) { > > - size_t base_len = ev3->length; > > - u8 *base; > > - > > sk = iso_get_sock_listen(&hdev->bdaddr, bdaddr, > > iso_match_sync_handle_pa_report, ev3); > > - base = eir_get_service_data(ev3->data, ev3->length, > > - EIR_BAA_SERVICE_UUID, &base_len); > > - if (base && sk && base_len <= sizeof(iso_pi(sk)->base)) { > > - memcpy(iso_pi(sk)->base, base, base_len); > > - iso_pi(sk)->base_len = base_len; > > + > > + if (!sk) > > + goto done; > > + > > + if (ev3->data_status == LE_PA_DATA_TRUNCATED) { > > + /* The controller was unable to retrieve PA data. */ > > + memset(iso_pi(sk)->pa_data, 0, > > + HCI_MAX_PER_AD_TOT_LENGTH); > > + iso_pi(sk)->pa_data_len = 0; > > + iso_pi(sk)->pa_data_offset = 0; > > + return lm; > > + } > > + > > + if (iso_pi(sk)->pa_data_offset + ev3->length > > > + HCI_MAX_PER_AD_TOT_LENGTH) > > + goto done; > > + > > + memcpy(iso_pi(sk)->pa_data + iso_pi(sk)->pa_data_offset, > > + ev3->data, ev3->length); > > + iso_pi(sk)->pa_data_offset += ev3->length; > > + > > + if (ev3->data_status == LE_PA_DATA_COMPLETE) { > > + /* All PA data has been received. */ > > + iso_pi(sk)->pa_data_len = iso_pi(sk)->pa_data_offset; > > + iso_pi(sk)->pa_data_offset = 0; > > + } else { > > + /* This is a PA data fragment. Keep pa_data_len set to 0 > > + * until all data has been reassembled. > > + */ > > + iso_pi(sk)->pa_data_len = 0; > > } > > } else { > > sk = iso_get_sock_listen(&hdev->bdaddr, BDADDR_ANY, NULL, > NULL); > > -- > > 2.39.2 > > > > > -- > Luiz Augusto von Dentz Regards, Iulia
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h index 08cb5cb249a4..a856e88719d2 100644 --- a/include/net/bluetooth/hci.h +++ b/include/net/bluetooth/hci.h @@ -1,7 +1,7 @@ /* BlueZ - Bluetooth protocol stack for Linux Copyright (C) 2000-2001 Qualcomm Incorporated - Copyright 2023 NXP + Copyright 2023-2024 NXP Written 2000,2001 by Maxim Krasnyansky <maxk@qualcomm.com> @@ -2037,6 +2037,7 @@ struct hci_cp_le_set_per_adv_params { } __packed; #define HCI_MAX_PER_AD_LENGTH 252 +#define HCI_MAX_PER_AD_TOT_LENGTH 1650 #define HCI_OP_LE_SET_PER_ADV_DATA 0x203f struct hci_cp_le_set_per_adv_data { @@ -2797,6 +2798,10 @@ struct hci_ev_le_per_adv_report { __u8 data[]; } __packed; +#define LE_PA_DATA_COMPLETE 0x00 +#define LE_PA_DATA_MORE_TO_COME 0x01 +#define LE_PA_DATA_TRUNCATED 0x02 + #define HCI_EV_LE_EXT_ADV_SET_TERM 0x12 struct hci_evt_le_ext_adv_set_term { __u8 status; diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c index 04f6572d35f1..e9a3b4f74e2f 100644 --- a/net/bluetooth/iso.c +++ b/net/bluetooth/iso.c @@ -3,7 +3,7 @@ * BlueZ - Bluetooth protocol stack for Linux * * Copyright (C) 2022 Intel Corporation - * Copyright 2023 NXP + * Copyright 2023-2024 NXP */ #include <linux/module.h> @@ -70,8 +70,15 @@ struct iso_pinfo { unsigned long flags; struct bt_iso_qos qos; bool qos_user_set; - __u8 base_len; - __u8 base[BASE_MAX_LENGTH]; + union { + __u8 base_len; + __u16 pa_data_len; + }; + union { + __u8 base[BASE_MAX_LENGTH]; + __u8 pa_data[HCI_MAX_PER_AD_TOT_LENGTH]; + }; + __u16 pa_data_offset; struct iso_conn *conn; }; @@ -1573,7 +1580,7 @@ static int iso_sock_getsockopt(struct socket *sock, int level, int optname, struct sock *sk = sock->sk; int len, err = 0; struct bt_iso_qos *qos; - u8 base_len; + size_t base_len; u8 *base; BT_DBG("sk %p", sk); @@ -1612,13 +1619,20 @@ static int iso_sock_getsockopt(struct socket *sock, int level, int optname, break; case BT_ISO_BASE: - if (sk->sk_state == BT_CONNECTED && - !bacmp(&iso_pi(sk)->dst, BDADDR_ANY)) { - base_len = iso_pi(sk)->conn->hcon->le_per_adv_data_len; - base = iso_pi(sk)->conn->hcon->le_per_adv_data; - } else { + if (!bacmp(&iso_pi(sk)->dst, BDADDR_ANY)) { + /* For a Broadcast Source, the BASE was stored + * in iso_pi(sk)->base. + */ base_len = iso_pi(sk)->base_len; base = iso_pi(sk)->base; + } else { + /* For a Broadcast Sink, the complete data received in + * PA reports is stored. Extract BASE from there. + */ + base = eir_get_service_data(iso_pi(sk)->pa_data, + iso_pi(sk)->pa_data_len, + EIR_BAA_SERVICE_UUID, + &base_len); } len = min_t(unsigned int, len, base_len); @@ -1834,8 +1848,9 @@ static void iso_conn_ready(struct iso_conn *conn) bacpy(&iso_pi(sk)->dst, &hcon->dst); iso_pi(sk)->dst_type = hcon->dst_type; iso_pi(sk)->sync_handle = iso_pi(parent)->sync_handle; - memcpy(iso_pi(sk)->base, iso_pi(parent)->base, iso_pi(parent)->base_len); - iso_pi(sk)->base_len = iso_pi(parent)->base_len; + memcpy(iso_pi(sk)->pa_data, iso_pi(parent)->pa_data, + iso_pi(parent)->pa_data_len); + iso_pi(sk)->pa_data_len = iso_pi(parent)->pa_data_len; hci_conn_hold(hcon); iso_chan_add(conn, sk, parent); @@ -1904,8 +1919,8 @@ int iso_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr, __u8 *flags) * a BIG Info it attempts to check if there any listening socket with * the same sync_handle and if it does then attempt to create a sync. * 3. HCI_EV_LE_PER_ADV_REPORT: When a PA report is received, it is stored - * in iso_pi(sk)->base so it can be passed up to user, in the case of a - * broadcast sink. + * in iso_pi(sk)->pa_data so the BASE can later be passed up to user, in + * the case of a broadcast sink. */ ev1 = hci_recv_event_data(hdev, HCI_EV_LE_PA_SYNC_ESTABLISHED); if (ev1) { @@ -1961,16 +1976,38 @@ int iso_connect_ind(struct hci_dev *hdev, bdaddr_t *bdaddr, __u8 *flags) ev3 = hci_recv_event_data(hdev, HCI_EV_LE_PER_ADV_REPORT); if (ev3) { - size_t base_len = ev3->length; - u8 *base; - sk = iso_get_sock_listen(&hdev->bdaddr, bdaddr, iso_match_sync_handle_pa_report, ev3); - base = eir_get_service_data(ev3->data, ev3->length, - EIR_BAA_SERVICE_UUID, &base_len); - if (base && sk && base_len <= sizeof(iso_pi(sk)->base)) { - memcpy(iso_pi(sk)->base, base, base_len); - iso_pi(sk)->base_len = base_len; + + if (!sk) + goto done; + + if (ev3->data_status == LE_PA_DATA_TRUNCATED) { + /* The controller was unable to retrieve PA data. */ + memset(iso_pi(sk)->pa_data, 0, + HCI_MAX_PER_AD_TOT_LENGTH); + iso_pi(sk)->pa_data_len = 0; + iso_pi(sk)->pa_data_offset = 0; + return lm; + } + + if (iso_pi(sk)->pa_data_offset + ev3->length > + HCI_MAX_PER_AD_TOT_LENGTH) + goto done; + + memcpy(iso_pi(sk)->pa_data + iso_pi(sk)->pa_data_offset, + ev3->data, ev3->length); + iso_pi(sk)->pa_data_offset += ev3->length; + + if (ev3->data_status == LE_PA_DATA_COMPLETE) { + /* All PA data has been received. */ + iso_pi(sk)->pa_data_len = iso_pi(sk)->pa_data_offset; + iso_pi(sk)->pa_data_offset = 0; + } else { + /* This is a PA data fragment. Keep pa_data_len set to 0 + * until all data has been reassembled. + */ + iso_pi(sk)->pa_data_len = 0; } } else { sk = iso_get_sock_listen(&hdev->bdaddr, BDADDR_ANY, NULL, NULL);
For a Broadcast Sink, this adds support for reassembling fragmented PA data. This is needed if the BASE comes fragmented in multiple PA reports. After all PA fragments have been reassembled, the BASE can be retrieved by the user via getsockopt. PA fragments could be reassembled inside hci_event.c and a complete PA report could be indicated to iso.c, or like it is currently, if PA fragments are indicated separately, reassembly would need to be done in iso.c. For a Broadcast Source, the user sets the BASE via setsockopt. The full PA data is built at bis bind, inside conn->le_per_adv_data. This array would be fit to hold the reassembled PA data on the Sink side as well, but a listening socket does not have an associated hcon...Currently for the Broadcast Sink, a hcon is created only after the first BIGInfo report, which arrives after the PA sync established event and all initial PA reports. The hcon is notified to iso.c, a child socket is created and it is notified to userspace, to mark the PA sync step as completed. I see 2 possibilities: 1. Keep a pa_data array inside the iso sock struct, use it to reassemble PA fragments, and extract BASE from it when needed. 2. Instead of creating the hcon after the first BIGInfo report, the hcon could be created when the PA sync established event is received. The le_per_adv_data can be used to reassemble the PA fragments, and the hcon will be notified to iso.c after the first BIGInfo report, when all information necessary to create and populate the child socket is available. In this patch I am showing the first idea, but I think the second one may provide a design more in line with the broadcast source scenario, when handling PA data and BASE. Are there any opinions about this? Signed-off-by: Iulia Tanasescu <iulia.tanasescu@nxp.com> --- include/net/bluetooth/hci.h | 7 +++- net/bluetooth/iso.c | 79 +++++++++++++++++++++++++++---------- 2 files changed, 64 insertions(+), 22 deletions(-)