2023-05-29 06:14:46

by Iulia Tanasescu

[permalink] [raw]
Subject: [PATCH v2 0/1] Bluetooth: ISO: Add support for connecting multiple BISes

This patch implements support for connecting multiple BISes as part
of the same BIG.

This proposes a flow similar to the one implemented for unicast:
DEFER_SETUP is used to bind a BIS to a BIG, without starting
periodic advertising or creating the BIG.

Connecting a socket without DEFER_SETUP will start periodic
advertising and will create the BIG, with a BIS for each bound
connection.

Iulia Tanasescu (1):
Bluetooth: ISO: Add support for connecting multiple BISes

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(-)


base-commit: a088d769ef3adfbc59ed86660d0de2abd86660e5
--
2.34.1



2023-05-29 06:14:46

by Iulia Tanasescu

[permalink] [raw]
Subject: [PATCH v2 1/1] Bluetooth: ISO: Add support for connecting multiple BISes

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 <[email protected]>
---
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));
} 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


2023-05-29 06:46:17

by bluez.test.bot

[permalink] [raw]
Subject: RE: Bluetooth: ISO: Add support for connecting multiple BISes

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=751739

---Test result---

Test Summary:
CheckPatch PASS 1.52 seconds
GitLint FAIL 0.60 seconds
SubjectPrefix PASS 0.10 seconds
BuildKernel PASS 37.61 seconds
CheckAllWarning PASS 40.56 seconds
CheckSparse WARNING 46.17 seconds
CheckSmatch WARNING 125.75 seconds
BuildKernel32 PASS 36.28 seconds
TestRunnerSetup PASS 514.38 seconds
TestRunner_l2cap-tester PASS 18.66 seconds
TestRunner_iso-tester PASS 25.30 seconds
TestRunner_bnep-tester PASS 6.57 seconds
TestRunner_mgmt-tester PASS 128.98 seconds
TestRunner_rfcomm-tester PASS 10.25 seconds
TestRunner_sco-tester PASS 9.34 seconds
TestRunner_ioctl-tester PASS 10.88 seconds
TestRunner_mesh-tester PASS 8.05 seconds
TestRunner_smp-tester PASS 9.30 seconds
TestRunner_userchan-tester PASS 6.83 seconds
IncrementalBuild PASS 34.26 seconds

Details
##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
[v2,1/1] Bluetooth: ISO: Add support for connecting multiple BISes

WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search
84: B2 Line has trailing whitespace: " Codec Configuration: "
100: B2 Line has trailing whitespace: " Codec Configuration: "
##############################
Test: CheckSparse - WARNING
Desc: Run sparse tool with linux kernel
Output:
net/bluetooth/hci_event.c: note: in included file (through include/net/bluetooth/hci_core.h):
##############################
Test: CheckSmatch - WARNING
Desc: Run smatch tool with source
Output:
net/bluetooth/hci_event.c: note: in included file (through include/net/bluetooth/hci_core.h):


---
Regards,
Linux Bluetooth

2023-05-29 20:05:49

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH v2 1/1] Bluetooth: ISO: Add support for connecting multiple BISes

Hi Iulia,

On Sun, May 28, 2023 at 11:14 PM Iulia Tanasescu
<[email protected]> 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 <[email protected]>
> ---
> 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

2023-05-30 14:49:45

by Iulia Tanasescu

[permalink] [raw]
Subject: Re: [PATCH v2 1/1] Bluetooth: ISO: Add support for connecting multiple BISes

Hi Luiz,

> -----Original Message-----
> From: Luiz Augusto von Dentz <[email protected]>
> Sent: Monday, May 29, 2023 10:59 PM
> To: Iulia Tanasescu <[email protected]>
> Cc: [email protected]
> Subject: Re: [PATCH v2 1/1] Bluetooth: ISO: Add support for connecting
> multiple BISes
>
> Hi Iulia,
>
> On Sun, May 28, 2023 at 11:14 PM Iulia Tanasescu <[email protected]>
> 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 <[email protected]>
> > ---
> > 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.
>

I needed to pass the flag because hci_le_terminate_big queues the commands to
remove the advertising set and to terminate the BIG. If I get past the initial
checks and I reach hci_le_terminate_big, I always want to remove the advertising
set, but I only want to terminate the BIG if it has been created, so I used
the flag to check if I need to also terminate the BIG or not. I submitted
another version of the patch, where I passed the conn object as parameter
and I'm checking everything internally.

> > } 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

Regards,
Iulia