Skip to content

Commit 2bc7129

Browse files
pvgregkh
authored andcommitted
Bluetooth: ISO: do not emit new LE Create CIS if previous is pending
[ Upstream commit 7f74563 ] LE Create CIS command shall not be sent before all CIS Established events from its previous invocation have been processed. Currently it is sent via hci_sync but that only waits for the first event, but there can be multiple. Make it wait for all events, and simplify the CIS creation as follows: Add new flag HCI_CONN_CREATE_CIS, which is set if Create CIS has been sent for the connection but it is not yet completed. Make BT_CONNECT state to mean the connection wants Create CIS. On events after which new Create CIS may need to be sent, send it if possible and some connections need it. These events are: hci_connect_cis, iso_connect_cfm, hci_cs_le_create_cis, hci_le_cis_estabilished_evt. The Create CIS status/completion events shall queue new Create CIS only if at least one of the connections transitions away from BT_CONNECT, so that we don't loop if controller is sending bogus events. This fixes sending multiple CIS Create for the same CIS in the "ISO AC 6(i) - Success" BlueZ test case: < HCI Command: LE Create Co.. (0x08|0x0064) plen 9 #129 [hci0] Number of CIS: 2 CIS Handle: 257 ACL Handle: 42 CIS Handle: 258 ACL Handle: 42 > HCI Event: Command Status (0x0f) plen 4 #130 [hci0] LE Create Connected Isochronous Stream (0x08|0x0064) ncmd 1 Status: Success (0x00) > HCI Event: LE Meta Event (0x3e) plen 29 #131 [hci0] LE Connected Isochronous Stream Established (0x19) Status: Success (0x00) Connection Handle: 257 ... < HCI Command: LE Setup Is.. (0x08|0x006e) plen 13 #132 [hci0] ... > HCI Event: Command Complete (0x0e) plen 6 #133 [hci0] LE Setup Isochronous Data Path (0x08|0x006e) ncmd 1 ... < HCI Command: LE Create Co.. (0x08|0x0064) plen 5 #134 [hci0] Number of CIS: 1 CIS Handle: 258 ACL Handle: 42 > HCI Event: Command Status (0x0f) plen 4 #135 [hci0] LE Create Connected Isochronous Stream (0x08|0x0064) ncmd 1 Status: ACL Connection Already Exists (0x0b) > HCI Event: LE Meta Event (0x3e) plen 29 #136 [hci0] LE Connected Isochronous Stream Established (0x19) Status: Success (0x00) Connection Handle: 258 ... Fixes: c09b80b ("Bluetooth: hci_conn: Fix not waiting for HCI_EVT_LE_CIS_ESTABLISHED") Signed-off-by: Pauli Virtanen <[email protected]> Signed-off-by: Luiz Augusto von Dentz <[email protected]> Signed-off-by: Sasha Levin <[email protected]>
1 parent b475c11 commit 2bc7129

File tree

6 files changed

+119
-78
lines changed

6 files changed

+119
-78
lines changed

include/net/bluetooth/hci_core.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -977,6 +977,7 @@ enum {
977977
HCI_CONN_AUTH_FAILURE,
978978
HCI_CONN_PER_ADV,
979979
HCI_CONN_BIG_CREATED,
980+
HCI_CONN_CREATE_CIS,
980981
};
981982

982983
static inline bool hci_conn_ssp_enabled(struct hci_conn *conn)
@@ -1353,7 +1354,8 @@ int hci_disconnect(struct hci_conn *conn, __u8 reason);
13531354
bool hci_setup_sync(struct hci_conn *conn, __u16 handle);
13541355
void hci_sco_setup(struct hci_conn *conn, __u8 status);
13551356
bool hci_iso_setup_path(struct hci_conn *conn);
1356-
int hci_le_create_cis(struct hci_conn *conn);
1357+
int hci_le_create_cis_pending(struct hci_dev *hdev);
1358+
int hci_conn_check_create_cis(struct hci_conn *conn);
13571359

13581360
struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst,
13591361
u8 role);

include/net/bluetooth/hci_sync.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ int hci_abort_conn_sync(struct hci_dev *hdev, struct hci_conn *conn, u8 reason);
124124

125125
int hci_le_create_conn_sync(struct hci_dev *hdev, struct hci_conn *conn);
126126

127-
int hci_le_create_cis_sync(struct hci_dev *hdev, struct hci_conn *conn);
127+
int hci_le_create_cis_sync(struct hci_dev *hdev);
128128

129129
int hci_le_remove_cig_sync(struct hci_dev *hdev, u8 handle);
130130

net/bluetooth/hci_conn.c

Lines changed: 30 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -1992,59 +1992,47 @@ bool hci_iso_setup_path(struct hci_conn *conn)
19921992
return true;
19931993
}
19941994

1995-
static int hci_create_cis_sync(struct hci_dev *hdev, void *data)
1995+
int hci_conn_check_create_cis(struct hci_conn *conn)
19961996
{
1997-
return hci_le_create_cis_sync(hdev, data);
1998-
}
1997+
if (conn->type != ISO_LINK || !bacmp(&conn->dst, BDADDR_ANY))
1998+
return -EINVAL;
19991999

2000-
int hci_le_create_cis(struct hci_conn *conn)
2001-
{
2002-
struct hci_conn *cis;
2003-
struct hci_link *link, *t;
2004-
struct hci_dev *hdev = conn->hdev;
2005-
int err;
2000+
if (!conn->parent || conn->parent->state != BT_CONNECTED ||
2001+
conn->state != BT_CONNECT || conn->handle == HCI_CONN_HANDLE_UNSET)
2002+
return 1;
20062003

2007-
bt_dev_dbg(hdev, "hcon %p", conn);
2004+
return 0;
2005+
}
20082006

2009-
switch (conn->type) {
2010-
case LE_LINK:
2011-
if (conn->state != BT_CONNECTED || list_empty(&conn->link_list))
2012-
return -EINVAL;
2007+
static int hci_create_cis_sync(struct hci_dev *hdev, void *data)
2008+
{
2009+
return hci_le_create_cis_sync(hdev);
2010+
}
20132011

2014-
cis = NULL;
2012+
int hci_le_create_cis_pending(struct hci_dev *hdev)
2013+
{
2014+
struct hci_conn *conn;
2015+
bool pending = false;
20152016

2016-
/* hci_conn_link uses list_add_tail_rcu so the list is in
2017-
* the same order as the connections are requested.
2018-
*/
2019-
list_for_each_entry_safe(link, t, &conn->link_list, list) {
2020-
if (link->conn->state == BT_BOUND) {
2021-
err = hci_le_create_cis(link->conn);
2022-
if (err)
2023-
return err;
2017+
rcu_read_lock();
20242018

2025-
cis = link->conn;
2026-
}
2019+
list_for_each_entry_rcu(conn, &hdev->conn_hash.list, list) {
2020+
if (test_bit(HCI_CONN_CREATE_CIS, &conn->flags)) {
2021+
rcu_read_unlock();
2022+
return -EBUSY;
20272023
}
20282024

2029-
return cis ? 0 : -EINVAL;
2030-
case ISO_LINK:
2031-
cis = conn;
2032-
break;
2033-
default:
2034-
return -EINVAL;
2025+
if (!hci_conn_check_create_cis(conn))
2026+
pending = true;
20352027
}
20362028

2037-
if (cis->state == BT_CONNECT)
2029+
rcu_read_unlock();
2030+
2031+
if (!pending)
20382032
return 0;
20392033

20402034
/* Queue Create CIS */
2041-
err = hci_cmd_sync_queue(hdev, hci_create_cis_sync, cis, NULL);
2042-
if (err)
2043-
return err;
2044-
2045-
cis->state = BT_CONNECT;
2046-
2047-
return 0;
2035+
return hci_cmd_sync_queue(hdev, hci_create_cis_sync, NULL, NULL);
20482036
}
20492037

20502038
static void hci_iso_qos_setup(struct hci_dev *hdev, struct hci_conn *conn,
@@ -2319,11 +2307,9 @@ struct hci_conn *hci_connect_cis(struct hci_dev *hdev, bdaddr_t *dst,
23192307
return ERR_PTR(-ENOLINK);
23202308
}
23212309

2322-
/* If LE is already connected and CIS handle is already set proceed to
2323-
* Create CIS immediately.
2324-
*/
2325-
if (le->state == BT_CONNECTED && cis->handle != HCI_CONN_HANDLE_UNSET)
2326-
hci_le_create_cis(cis);
2310+
cis->state = BT_CONNECT;
2311+
2312+
hci_le_create_cis_pending(hdev);
23272313

23282314
return cis;
23292315
}

net/bluetooth/hci_event.c

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3810,6 +3810,7 @@ static u8 hci_cc_le_set_cig_params(struct hci_dev *hdev, void *data,
38103810
struct hci_cp_le_set_cig_params *cp;
38113811
struct hci_conn *conn;
38123812
u8 status = rp->status;
3813+
bool pending = false;
38133814
int i;
38143815

38153816
bt_dev_dbg(hdev, "status 0x%2.2x", rp->status);
@@ -3851,13 +3852,15 @@ static u8 hci_cc_le_set_cig_params(struct hci_dev *hdev, void *data,
38513852

38523853
bt_dev_dbg(hdev, "%p handle 0x%4.4x parent %p", conn,
38533854
conn->handle, conn->parent);
3854-
3855-
/* Create CIS if LE is already connected */
3856-
if (conn->parent && conn->parent->state == BT_CONNECTED)
3857-
hci_le_create_cis(conn);
3855+
3856+
if (conn->state == BT_CONNECT)
3857+
pending = true;
38583858
}
38593859

38603860
unlock:
3861+
if (pending)
3862+
hci_le_create_cis_pending(hdev);
3863+
38613864
hci_dev_unlock(hdev);
38623865

38633866
return rp->status;
@@ -4223,6 +4226,7 @@ static void hci_cmd_complete_evt(struct hci_dev *hdev, void *data,
42234226
static void hci_cs_le_create_cis(struct hci_dev *hdev, u8 status)
42244227
{
42254228
struct hci_cp_le_create_cis *cp;
4229+
bool pending = false;
42264230
int i;
42274231

42284232
bt_dev_dbg(hdev, "status 0x%2.2x", status);
@@ -4245,12 +4249,18 @@ static void hci_cs_le_create_cis(struct hci_dev *hdev, u8 status)
42454249

42464250
conn = hci_conn_hash_lookup_handle(hdev, handle);
42474251
if (conn) {
4252+
if (test_and_clear_bit(HCI_CONN_CREATE_CIS,
4253+
&conn->flags))
4254+
pending = true;
42484255
conn->state = BT_CLOSED;
42494256
hci_connect_cfm(conn, status);
42504257
hci_conn_del(conn);
42514258
}
42524259
}
42534260

4261+
if (pending)
4262+
hci_le_create_cis_pending(hdev);
4263+
42544264
hci_dev_unlock(hdev);
42554265
}
42564266

@@ -6789,6 +6799,7 @@ static void hci_le_cis_estabilished_evt(struct hci_dev *hdev, void *data,
67896799
struct hci_evt_le_cis_established *ev = data;
67906800
struct hci_conn *conn;
67916801
struct bt_iso_qos *qos;
6802+
bool pending = false;
67926803
u16 handle = __le16_to_cpu(ev->handle);
67936804

67946805
bt_dev_dbg(hdev, "status 0x%2.2x", ev->status);
@@ -6812,6 +6823,8 @@ static void hci_le_cis_estabilished_evt(struct hci_dev *hdev, void *data,
68126823

68136824
qos = &conn->iso_qos;
68146825

6826+
pending = test_and_clear_bit(HCI_CONN_CREATE_CIS, &conn->flags);
6827+
68156828
/* Convert ISO Interval (1.25 ms slots) to SDU Interval (us) */
68166829
qos->ucast.in.interval = le16_to_cpu(ev->interval) * 1250;
68176830
qos->ucast.out.interval = qos->ucast.in.interval;
@@ -6853,10 +6866,14 @@ static void hci_le_cis_estabilished_evt(struct hci_dev *hdev, void *data,
68536866
goto unlock;
68546867
}
68556868

6869+
conn->state = BT_CLOSED;
68566870
hci_connect_cfm(conn, ev->status);
68576871
hci_conn_del(conn);
68586872

68596873
unlock:
6874+
if (pending)
6875+
hci_le_create_cis_pending(hdev);
6876+
68606877
hci_dev_unlock(hdev);
68616878
}
68626879

net/bluetooth/hci_sync.c

Lines changed: 63 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -6262,56 +6262,92 @@ int hci_le_create_conn_sync(struct hci_dev *hdev, struct hci_conn *conn)
62626262
return err;
62636263
}
62646264

6265-
int hci_le_create_cis_sync(struct hci_dev *hdev, struct hci_conn *conn)
6265+
int hci_le_create_cis_sync(struct hci_dev *hdev)
62666266
{
62676267
struct {
62686268
struct hci_cp_le_create_cis cp;
62696269
struct hci_cis cis[0x1f];
62706270
} cmd;
6271-
u8 cig;
6272-
struct hci_conn *hcon = conn;
6271+
struct hci_conn *conn;
6272+
u8 cig = BT_ISO_QOS_CIG_UNSET;
6273+
6274+
/* The spec allows only one pending LE Create CIS command at a time. If
6275+
* the command is pending now, don't do anything. We check for pending
6276+
* connections after each CIS Established event.
6277+
*
6278+
* BLUETOOTH CORE SPECIFICATION Version 5.3 | Vol 4, Part E
6279+
* page 2566:
6280+
*
6281+
* If the Host issues this command before all the
6282+
* HCI_LE_CIS_Established events from the previous use of the
6283+
* command have been generated, the Controller shall return the
6284+
* error code Command Disallowed (0x0C).
6285+
*
6286+
* BLUETOOTH CORE SPECIFICATION Version 5.3 | Vol 4, Part E
6287+
* page 2567:
6288+
*
6289+
* When the Controller receives the HCI_LE_Create_CIS command, the
6290+
* Controller sends the HCI_Command_Status event to the Host. An
6291+
* HCI_LE_CIS_Established event will be generated for each CIS when it
6292+
* is established or if it is disconnected or considered lost before
6293+
* being established; until all the events are generated, the command
6294+
* remains pending.
6295+
*/
62736296

62746297
memset(&cmd, 0, sizeof(cmd));
6275-
cmd.cis[0].acl_handle = cpu_to_le16(conn->parent->handle);
6276-
cmd.cis[0].cis_handle = cpu_to_le16(conn->handle);
6277-
cmd.cp.num_cis++;
6278-
cig = conn->iso_qos.ucast.cig;
62796298

62806299
hci_dev_lock(hdev);
62816300

62826301
rcu_read_lock();
62836302

6303+
/* Wait until previous Create CIS has completed */
62846304
list_for_each_entry_rcu(conn, &hdev->conn_hash.list, list) {
6285-
struct hci_cis *cis = &cmd.cis[cmd.cp.num_cis];
6305+
if (test_bit(HCI_CONN_CREATE_CIS, &conn->flags))
6306+
goto done;
6307+
}
62866308

6287-
if (conn == hcon || conn->type != ISO_LINK ||
6288-
conn->state == BT_CONNECTED ||
6289-
conn->iso_qos.ucast.cig != cig)
6309+
/* Find CIG with all CIS ready */
6310+
list_for_each_entry_rcu(conn, &hdev->conn_hash.list, list) {
6311+
struct hci_conn *link;
6312+
6313+
if (hci_conn_check_create_cis(conn))
62906314
continue;
62916315

6292-
/* Check if all CIS(s) belonging to a CIG are ready */
6293-
if (!conn->parent || conn->parent->state != BT_CONNECTED ||
6294-
conn->state != BT_CONNECT) {
6295-
cmd.cp.num_cis = 0;
6296-
break;
6316+
cig = conn->iso_qos.ucast.cig;
6317+
6318+
list_for_each_entry_rcu(link, &hdev->conn_hash.list, list) {
6319+
if (hci_conn_check_create_cis(link) > 0 &&
6320+
link->iso_qos.ucast.cig == cig &&
6321+
link->state != BT_CONNECTED) {
6322+
cig = BT_ISO_QOS_CIG_UNSET;
6323+
break;
6324+
}
62976325
}
62986326

6299-
/* Group all CIS with state BT_CONNECT since the spec don't
6300-
* allow to send them individually:
6301-
*
6302-
* BLUETOOTH CORE SPECIFICATION Version 5.3 | Vol 4, Part E
6303-
* page 2566:
6304-
*
6305-
* If the Host issues this command before all the
6306-
* HCI_LE_CIS_Established events from the previous use of the
6307-
* command have been generated, the Controller shall return the
6308-
* error code Command Disallowed (0x0C).
6309-
*/
6327+
if (cig != BT_ISO_QOS_CIG_UNSET)
6328+
break;
6329+
}
6330+
6331+
if (cig == BT_ISO_QOS_CIG_UNSET)
6332+
goto done;
6333+
6334+
list_for_each_entry_rcu(conn, &hdev->conn_hash.list, list) {
6335+
struct hci_cis *cis = &cmd.cis[cmd.cp.num_cis];
6336+
6337+
if (hci_conn_check_create_cis(conn) ||
6338+
conn->iso_qos.ucast.cig != cig)
6339+
continue;
6340+
6341+
set_bit(HCI_CONN_CREATE_CIS, &conn->flags);
63106342
cis->acl_handle = cpu_to_le16(conn->parent->handle);
63116343
cis->cis_handle = cpu_to_le16(conn->handle);
63126344
cmd.cp.num_cis++;
6345+
6346+
if (cmd.cp.num_cis >= ARRAY_SIZE(cmd.cis))
6347+
break;
63136348
}
63146349

6350+
done:
63156351
rcu_read_unlock();
63166352

63176353
hci_dev_unlock(hdev);

net/bluetooth/iso.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1690,7 +1690,7 @@ static void iso_connect_cfm(struct hci_conn *hcon, __u8 status)
16901690
}
16911691

16921692
/* Create CIS if pending */
1693-
hci_le_create_cis(hcon);
1693+
hci_le_create_cis_pending(hcon->hdev);
16941694
return;
16951695
}
16961696

0 commit comments

Comments
 (0)