Hi Iulia,
On Sun, May 28, 2023 at 11:14 PM Iulia Tanasescu
<iulia.tanasescu@xxxxxxx> wrote:
>
> It is required for some configurations to have multiple BISes as part
> of the same BIG.
>
> Similar to the flow implemented for unicast, DEFER_SETUP will also be
> used to bind multiple BISes for the same BIG, before starting Periodic
> Advertising and creating the BIG.
>
> The user will have to open a new socket for each BIS. By setting the
> BT_DEFER_SETUP socket option and calling connect, a new connection
> will be added for the BIG and advertising handle set by the socket
> QoS parameters. Since all BISes will be bound for the same BIG and
> advertising handle, the socket QoS options and base parameters should
> match for all connections.
>
> By calling connect on a socket that does not have the BT_DEFER_SETUP
> option set, periodic advertising will be started and the BIG will
> be created, with a BIS for each previously bound connection. Since
> a BIG cannot be reconfigured with additional BISes after creation,
> no more connections can be bound for the BIG after the start periodic
> advertising and create BIG commands have been queued.
>
> The bis_cleanup function has also been updated, so that the advertising
> set and the BIG will not be terminated unless there are no more
> bound or connected BISes.
>
> The HCI_CONN_BIG_CREATED connection flag has been added to indicate
> that the BIG has been successfully created. This flag is checked at
> bis_cleanup, so that the BIG is only terminated if the
> HCI_LE_Create_BIG_Complete has been received.
>
> This implementation has been tested on hardware, using the "isotest"
> tool with an additional command line option, to specify the number of
> BISes to create as part of the desired BIG:
>
> tools/isotest -i hci0 -s 00:00:00:00:00:00 -N 2 -G 1 -T 1
>
> The btmon log shows that a BIG containing 2 BISes has been created:
>
> < HCI Command: LE Create Broadcast Isochronous Group (0x08|0x0068) plen 31
> Handle: 0x01
> Advertising Handle: 0x01
> Number of BIS: 2
> SDU Interval: 10000 us (0x002710)
> Maximum SDU size: 40
> Maximum Latency: 10 ms (0x000a)
> RTN: 0x02
> PHY: LE 2M (0x02)
> Packing: Sequential (0x00)
> Framing: Unframed (0x00)
> Encryption: 0x00
> Broadcast Code: 00000000000000000000000000000000
>
> > HCI Event: Command Status (0x0f) plen 4
> LE Create Broadcast Isochronous Group (0x08|0x0068) ncmd 1
> Status: Success (0x00)
>
> > HCI Event: LE Meta Event (0x3e) plen 23
> LE Broadcast Isochronous Group Complete (0x1b)
> Status: Success (0x00)
> Handle: 0x01
> BIG Synchronization Delay: 1974 us (0x0007b6)
> Transport Latency: 1974 us (0x0007b6)
> PHY: LE 2M (0x02)
> NSE: 3
> BN: 1
> PTO: 1
> IRC: 3
> Maximum PDU: 40
> ISO Interval: 10.00 msec (0x0008)
> Connection Handle #0: 10
> Connection Handle #1: 11
>
> < HCI Command: LE Setup Isochronous Data Path (0x08|0x006e) plen 13
> Handle: 10
> Data Path Direction: Input (Host to Controller) (0x00)
> Data Path: HCI (0x00)
> Coding Format: Transparent (0x03)
> Company Codec ID: Ericsson Technology Licensing (0)
> Vendor Codec ID: 0
> Controller Delay: 0 us (0x000000)
> Codec Configuration Length: 0
> Codec Configuration:
>
> > HCI Event: Command Complete (0x0e) plen 6
> LE Setup Isochronous Data Path (0x08|0x006e) ncmd 1
> Status: Success (0x00)
> Handle: 10
>
> < HCI Command: LE Setup Isochronous Data Path (0x08|0x006e) plen 13
> Handle: 11
> Data Path Direction: Input (Host to Controller) (0x00)
> Data Path: HCI (0x00)
> Coding Format: Transparent (0x03)
> Company Codec ID: Ericsson Technology Licensing (0)
> Vendor Codec ID: 0
> Controller Delay: 0 us (0x000000)
> Codec Configuration Length: 0
> Codec Configuration:
>
> > HCI Event: Command Complete (0x0e) plen 6
> LE Setup Isochronous Data Path (0x08|0x006e) ncmd 1
> Status: Success (0x00)
> Handle: 11
>
> < ISO Data TX: Handle 10 flags 0x02 dlen 44
>
> < ISO Data TX: Handle 11 flags 0x02 dlen 44
>
> > HCI Event: Number of Completed Packets (0x13) plen 5
> Num handles: 1
> Handle: 10
> Count: 1
>
> > HCI Event: Number of Completed Packets (0x13) plen 5
> Num handles: 1
> Handle: 11
> Count: 1
>
> Signed-off-by: Iulia Tanasescu <iulia.tanasescu@xxxxxxx>
> ---
> include/net/bluetooth/hci_core.h | 30 +++++++
> net/bluetooth/hci_conn.c | 147 ++++++++++++++++++++++---------
> net/bluetooth/hci_event.c | 50 ++++++-----
> net/bluetooth/iso.c | 28 ++++--
> 4 files changed, 186 insertions(+), 69 deletions(-)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 9f37326c1c05..74ec1f40ab6b 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -975,6 +975,7 @@ enum {
> HCI_CONN_SCANNING,
> HCI_CONN_AUTH_FAILURE,
> HCI_CONN_PER_ADV,
> + HCI_CONN_BIG_CREATED,
> };
>
> static inline bool hci_conn_ssp_enabled(struct hci_conn *conn)
> @@ -1116,6 +1117,32 @@ static inline struct hci_conn *hci_conn_hash_lookup_bis(struct hci_dev *hdev,
> return NULL;
> }
>
> +static inline struct hci_conn *
> +hci_conn_hash_lookup_per_adv_bis(struct hci_dev *hdev,
> + bdaddr_t *ba,
> + __u8 big, __u8 bis)
> +{
> + struct hci_conn_hash *h = &hdev->conn_hash;
> + struct hci_conn *c;
> +
> + rcu_read_lock();
> +
> + list_for_each_entry_rcu(c, &h->list, list) {
> + if (bacmp(&c->dst, ba) || c->type != ISO_LINK ||
> + !test_bit(HCI_CONN_PER_ADV, &c->flags))
> + continue;
> +
> + if (c->iso_qos.bcast.big == big &&
> + c->iso_qos.bcast.bis == bis) {
> + rcu_read_unlock();
> + return c;
> + }
> + }
> + rcu_read_unlock();
> +
> + return NULL;
> +}
> +
> static inline struct hci_conn *hci_conn_hash_lookup_handle(struct hci_dev *hdev,
> __u16 handle)
> {
> @@ -1351,6 +1378,9 @@ struct hci_conn *hci_connect_sco(struct hci_dev *hdev, int type, bdaddr_t *dst,
> __u16 setting, struct bt_codec *codec);
> struct hci_conn *hci_bind_cis(struct hci_dev *hdev, bdaddr_t *dst,
> __u8 dst_type, struct bt_iso_qos *qos);
> +struct hci_conn *hci_bind_bis(struct hci_dev *hdev, bdaddr_t *dst,
> + struct bt_iso_qos *qos,
> + __u8 base_len, __u8 *base);
> struct hci_conn *hci_connect_cis(struct hci_dev *hdev, bdaddr_t *dst,
> __u8 dst_type, struct bt_iso_qos *qos);
> struct hci_conn *hci_connect_bis(struct hci_dev *hdev, bdaddr_t *dst,
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index 1f906f8508bc..0838c0256a7e 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -790,6 +790,7 @@ struct iso_list_data {
> struct hci_cp_le_set_cig_params cp;
> struct hci_cis_params cis[0x11];
> } pdu;
> + bool big_term;
> };
>
> static void bis_list(struct hci_conn *conn, void *data)
> @@ -826,11 +827,8 @@ static int terminate_big_sync(struct hci_dev *hdev, void *data)
>
> hci_remove_ext_adv_instance_sync(hdev, d->bis, NULL);
>
> - /* Check if ISO connection is a BIS and terminate BIG if there are
> - * no other connections using it.
> - */
> - hci_conn_hash_list_state(hdev, find_bis, ISO_LINK, BT_CONNECTED, d);
> - if (d->count)
> + /* Only terminate BIG if it has been created */
> + if (!d->big_term)
> return 0;
>
> return hci_le_terminate_big_sync(hdev, d->big,
> @@ -842,7 +840,8 @@ static void terminate_big_destroy(struct hci_dev *hdev, void *data, int err)
> kfree(data);
> }
>
> -static int hci_le_terminate_big(struct hci_dev *hdev, u8 big, u8 bis)
> +static int hci_le_terminate_big(struct hci_dev *hdev, u8 big, u8 bis,
> + bool big_term)
> {
> struct iso_list_data *d;
> int ret;
> @@ -855,6 +854,7 @@ static int hci_le_terminate_big(struct hci_dev *hdev, u8 big, u8 bis)
>
> d->big = big;
> d->bis = bis;
> + d->big_term = big_term;
>
> ret = hci_cmd_sync_queue(hdev, terminate_big_sync, d,
> terminate_big_destroy);
> @@ -914,6 +914,7 @@ static int hci_le_big_terminate(struct hci_dev *hdev, u8 big, u16 sync_handle)
> static void bis_cleanup(struct hci_conn *conn)
> {
> struct hci_dev *hdev = conn->hdev;
> + struct hci_conn *bis;
>
> bt_dev_dbg(hdev, "conn %p", conn);
>
> @@ -921,8 +922,19 @@ static void bis_cleanup(struct hci_conn *conn)
> if (!test_and_clear_bit(HCI_CONN_PER_ADV, &conn->flags))
> return;
>
> + /* Check if ISO connection is a BIS and terminate advertising
> + * set and BIG if there are no other connections using it.
> + */
> + bis = hci_conn_hash_lookup_bis(hdev, BDADDR_ANY,
> + conn->iso_qos.bcast.big,
> + conn->iso_qos.bcast.bis);
> + if (bis)
> + return;
> +
> hci_le_terminate_big(hdev, conn->iso_qos.bcast.big,
> - conn->iso_qos.bcast.bis);
> + conn->iso_qos.bcast.bis,
> + test_and_clear_bit(HCI_CONN_BIG_CREATED,
> + &conn->flags));
Im not really following why you decided to pass a flag if the big has
to be terminated or not, I mean if it has not been created the this
should be a nop so instead you could do just:
if (test_and_clear_bit(HCI_CONN_BIG_CREATED,...))
hci_le_terminate_big(...)
Or better yet just incorporate the code above into
hci_le_terminate_big and pass the conn to it so it checks the flags,
etc.
> } else {
> hci_le_big_terminate(hdev, conn->iso_qos.bcast.big,
> conn->sync_handle);
> @@ -1491,10 +1503,10 @@ static int qos_set_bis(struct hci_dev *hdev, struct bt_iso_qos *qos)
>
> /* This function requires the caller holds hdev->lock */
> static struct hci_conn *hci_add_bis(struct hci_dev *hdev, bdaddr_t *dst,
> - struct bt_iso_qos *qos)
> + struct bt_iso_qos *qos, __u8 base_len,
> + __u8 *base)
> {
> struct hci_conn *conn;
> - struct iso_list_data data;
> int err;
>
> /* Let's make sure that le is enabled.*/
> @@ -1512,24 +1524,27 @@ static struct hci_conn *hci_add_bis(struct hci_dev *hdev, bdaddr_t *dst,
> if (err)
> return ERR_PTR(err);
>
> - data.big = qos->bcast.big;
> - data.bis = qos->bcast.bis;
> - data.count = 0;
> -
> - /* Check if there is already a matching BIG/BIS */
> - hci_conn_hash_list_state(hdev, bis_list, ISO_LINK, BT_BOUND, &data);
> - if (data.count)
> + /* Check if the LE Create BIG command has already been sent */
> + conn = hci_conn_hash_lookup_per_adv_bis(hdev, dst, qos->bcast.big,
> + qos->bcast.big);
> + if (conn)
> return ERR_PTR(-EADDRINUSE);
>
> - conn = hci_conn_hash_lookup_bis(hdev, dst, qos->bcast.big, qos->bcast.bis);
> - if (conn)
> + /* Check BIS settings against other bound BISes, since all
> + * BISes in a BIG must have the same value for all parameters
> + */
> + conn = hci_conn_hash_lookup_bis(hdev, dst, qos->bcast.big,
> + qos->bcast.bis);
> +
> + if (conn && (memcmp(qos, &conn->iso_qos, sizeof(*qos)) ||
> + base_len != conn->le_per_adv_data_len ||
> + memcmp(conn->le_per_adv_data, base, base_len)))
> return ERR_PTR(-EADDRINUSE);
>
> conn = hci_conn_add(hdev, ISO_LINK, dst, HCI_ROLE_MASTER);
> if (!conn)
> return ERR_PTR(-ENOMEM);
>
> - set_bit(HCI_CONN_PER_ADV, &conn->flags);
> conn->state = BT_CONNECT;
>
> hci_conn_hold(conn);
> @@ -1743,12 +1758,21 @@ static int hci_le_create_big(struct hci_conn *conn, struct bt_iso_qos *qos)
> {
> struct hci_dev *hdev = conn->hdev;
> struct hci_cp_le_create_big cp;
> + struct iso_list_data data;
>
> memset(&cp, 0, sizeof(cp));
>
> + data.big = qos->bcast.big;
> + data.bis = qos->bcast.bis;
> + data.count = 0;
> +
> + /* Create a BIS for each bound connection */
> + hci_conn_hash_list_state(hdev, bis_list, ISO_LINK,
> + BT_BOUND, &data);
> +
> cp.handle = qos->bcast.big;
> cp.adv_handle = qos->bcast.bis;
> - cp.num_bis = 0x01;
> + cp.num_bis = data.count;
> hci_cpu_to_le24(qos->bcast.out.interval, cp.bis.sdu_interval);
> cp.bis.sdu = cpu_to_le16(qos->bcast.out.sdu);
> cp.bis.latency = cpu_to_le16(qos->bcast.out.latency);
> @@ -2020,16 +2044,6 @@ static void hci_iso_qos_setup(struct hci_dev *hdev, struct hci_conn *conn,
> qos->latency = conn->le_conn_latency;
> }
>
> -static void hci_bind_bis(struct hci_conn *conn,
> - struct bt_iso_qos *qos)
> -{
> - /* Update LINK PHYs according to QoS preference */
> - conn->le_tx_phy = qos->bcast.out.phy;
> - conn->le_tx_phy = qos->bcast.out.phy;
> - conn->iso_qos = *qos;
> - conn->state = BT_BOUND;
> -}
> -
> static int create_big_sync(struct hci_dev *hdev, void *data)
> {
> struct hci_conn *conn = data;
> @@ -2152,27 +2166,80 @@ static void create_big_complete(struct hci_dev *hdev, void *data, int err)
> }
> }
>
> -struct hci_conn *hci_connect_bis(struct hci_dev *hdev, bdaddr_t *dst,
> - __u8 dst_type, struct bt_iso_qos *qos,
> - __u8 base_len, __u8 *base)
> +struct hci_conn *hci_bind_bis(struct hci_dev *hdev, bdaddr_t *dst,
> + struct bt_iso_qos *qos,
> + __u8 base_len, __u8 *base)
> {
> struct hci_conn *conn;
> - int err;
> + __u8 eir[HCI_MAX_PER_AD_LENGTH];
> +
> + if (base_len && base)
> + base_len = eir_append_service_data(eir, 0, 0x1851,
> + base, base_len);
>
> /* We need hci_conn object using the BDADDR_ANY as dst */
> - conn = hci_add_bis(hdev, dst, qos);
> + conn = hci_add_bis(hdev, dst, qos, base_len, eir);
> if (IS_ERR(conn))
> return conn;
>
> - hci_bind_bis(conn, qos);
> + /* Update LINK PHYs according to QoS preference */
> + conn->le_tx_phy = qos->bcast.out.phy;
> + conn->le_tx_phy = qos->bcast.out.phy;
>
> /* Add Basic Announcement into Peridic Adv Data if BASE is set */
> if (base_len && base) {
> - base_len = eir_append_service_data(conn->le_per_adv_data, 0,
> - 0x1851, base, base_len);
> + memcpy(conn->le_per_adv_data, eir, sizeof(eir));
> conn->le_per_adv_data_len = base_len;
> }
>
> + hci_iso_qos_setup(hdev, conn, &qos->bcast.out,
> + conn->le_tx_phy ? conn->le_tx_phy :
> + hdev->le_tx_def_phys);
> +
> + conn->iso_qos = *qos;
> + conn->state = BT_BOUND;
> +
> + return conn;
> +}
> +
> +static void bis_mark_per_adv(struct hci_conn *conn, void *data)
> +{
> + struct iso_list_data *d = data;
> +
> + /* Skip if not broadcast/ANY address */
> + if (bacmp(&conn->dst, BDADDR_ANY))
> + return;
> +
> + if (d->big != conn->iso_qos.bcast.big ||
> + d->bis == BT_ISO_QOS_BIS_UNSET ||
> + d->bis != conn->iso_qos.bcast.bis)
> + return;
> +
> + set_bit(HCI_CONN_PER_ADV, &conn->flags);
> +}
> +
> +struct hci_conn *hci_connect_bis(struct hci_dev *hdev, bdaddr_t *dst,
> + __u8 dst_type, struct bt_iso_qos *qos,
> + __u8 base_len, __u8 *base)
> +{
> + struct hci_conn *conn;
> + int err;
> + struct iso_list_data data;
> +
> + conn = hci_bind_bis(hdev, dst, qos, base_len, base);
> + if (IS_ERR(conn))
> + return conn;
> +
> + data.big = qos->bcast.big;
> + data.bis = qos->bcast.bis;
> +
> + /* Set HCI_CONN_PER_ADV for all bound connections, to mark that
> + * the start periodic advertising and create BIG commands have
> + * been queued
> + */
> + hci_conn_hash_list_state(hdev, bis_mark_per_adv, ISO_LINK,
> + BT_BOUND, &data);
> +
> /* Queue start periodic advertising and create BIG */
> err = hci_cmd_sync_queue(hdev, create_big_sync, conn,
> create_big_complete);
> @@ -2181,10 +2248,6 @@ struct hci_conn *hci_connect_bis(struct hci_dev *hdev, bdaddr_t *dst,
> return ERR_PTR(err);
> }
>
> - hci_iso_qos_setup(hdev, conn, &qos->bcast.out,
> - conn->le_tx_phy ? conn->le_tx_phy :
> - hdev->le_tx_def_phys);
> -
> return conn;
> }
>
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index d00ef6e3fc45..c24958af525a 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -6910,6 +6910,7 @@ static void hci_le_create_big_complete_evt(struct hci_dev *hdev, void *data,
> {
> struct hci_evt_le_create_big_complete *ev = data;
> struct hci_conn *conn;
> + __u8 bis_idx = 0;
>
> BT_DBG("%s status 0x%2.2x", hdev->name, ev->status);
>
> @@ -6918,33 +6919,42 @@ static void hci_le_create_big_complete_evt(struct hci_dev *hdev, void *data,
> return;
>
> hci_dev_lock(hdev);
> + rcu_read_lock();
>
> - conn = hci_conn_hash_lookup_big(hdev, ev->handle);
> - if (!conn)
> - goto unlock;
> + /* Connect all BISes that are bound to the BIG */
> + list_for_each_entry_rcu(conn, &hdev->conn_hash.list, list) {
> + if (bacmp(&conn->dst, BDADDR_ANY) ||
> + conn->type != ISO_LINK ||
> + conn->iso_qos.bcast.big != ev->handle)
> + continue;
>
> - if (conn->type != ISO_LINK) {
> - bt_dev_err(hdev,
> - "Invalid connection link type handle 0x%2.2x",
> - ev->handle);
> - goto unlock;
> - }
> + conn->handle = __le16_to_cpu(ev->bis_handle[bis_idx++]);
>
> - if (ev->num_bis)
> - conn->handle = __le16_to_cpu(ev->bis_handle[0]);
> + if (!ev->status) {
> + conn->state = BT_CONNECTED;
> + set_bit(HCI_CONN_BIG_CREATED, &conn->flags);
> + hci_debugfs_create_conn(conn);
> + hci_conn_add_sysfs(conn);
> + hci_iso_setup_path(conn);
> + continue;
> + }
>
> - if (!ev->status) {
> - conn->state = BT_CONNECTED;
> - hci_debugfs_create_conn(conn);
> - hci_conn_add_sysfs(conn);
> - hci_iso_setup_path(conn);
> - goto unlock;
> + hci_connect_cfm(conn, ev->status);
> + rcu_read_unlock();
> + hci_conn_del(conn);
> + rcu_read_lock();
> }
>
> - hci_connect_cfm(conn, ev->status);
> - hci_conn_del(conn);
> + if (!ev->status && !bis_idx)
> + /* If no BISes have been connected for the BIG,
> + * terminate. This is in case all bound connections
> + * have been closed before the BIG creation
> + * has completed.
> + */
> + hci_le_terminate_big_sync(hdev, ev->handle,
> + HCI_ERROR_LOCAL_HOST_TERM);
>
> -unlock:
> + rcu_read_unlock();
> hci_dev_unlock(hdev);
> }
>
> diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c
> index 34d55a85d8f6..485348fcc030 100644
> --- a/net/bluetooth/iso.c
> +++ b/net/bluetooth/iso.c
> @@ -284,13 +284,24 @@ static int iso_connect_bis(struct sock *sk)
> goto unlock;
> }
>
> - hcon = hci_connect_bis(hdev, &iso_pi(sk)->dst,
> - le_addr_type(iso_pi(sk)->dst_type),
> - &iso_pi(sk)->qos, iso_pi(sk)->base_len,
> - iso_pi(sk)->base);
> - if (IS_ERR(hcon)) {
> - err = PTR_ERR(hcon);
> - goto unlock;
> + /* Just bind if DEFER_SETUP has been set */
> + if (test_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags)) {
> + hcon = hci_bind_bis(hdev, &iso_pi(sk)->dst,
> + &iso_pi(sk)->qos, iso_pi(sk)->base_len,
> + iso_pi(sk)->base);
> + if (IS_ERR(hcon)) {
> + err = PTR_ERR(hcon);
> + goto unlock;
> + }
> + } else {
> + hcon = hci_connect_bis(hdev, &iso_pi(sk)->dst,
> + le_addr_type(iso_pi(sk)->dst_type),
> + &iso_pi(sk)->qos, iso_pi(sk)->base_len,
> + iso_pi(sk)->base);
> + if (IS_ERR(hcon)) {
> + err = PTR_ERR(hcon);
> + goto unlock;
> + }
> }
>
> conn = iso_conn_add(hcon);
> @@ -315,6 +326,9 @@ static int iso_connect_bis(struct sock *sk)
> if (hcon->state == BT_CONNECTED) {
> iso_sock_clear_timer(sk);
> sk->sk_state = BT_CONNECTED;
> + } else if (test_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags)) {
> + iso_sock_clear_timer(sk);
> + sk->sk_state = BT_CONNECT;
> } else {
> sk->sk_state = BT_CONNECT;
> iso_sock_set_timer(sk, sk->sk_sndtimeo);
> --
> 2.34.1
>
--
Luiz Augusto von Dentz
