michaelh/zephyr
1
0
Fork 0
Code Releases Activity

Bluetooth: ISO: Fixes ISO central disconnect and cleanup issues

Browse source 
When an ISO channel is disconnect on the central, it is
not deallocated, but merely disconnected. This is because,
as per the HCI spec, the CIS handle lives on in the CIG.

Instead of unref'ing the bt_conn to 0, we simply put the
channel and connection in the disconnected state.

This also fixes a few missing returns for
terminating a CIG.

Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
This commit is contained in:
Emil Gydesen 2021-08-25 18:25:46 +02:00 • committed by Anas Nashif
commit 5d14cddaa8
3 changed files with 21 additions and 28 deletions
Download patch file Download diff file Expand all files Collapse all files

2
include/bluetooth/iso.h
View file

@ -83,8 +83,6 @@ struct bt_iso_chan;
enum { enum {
/** Channel disconnected */ /** Channel disconnected */
BT_ISO_DISCONNECTED, BT_ISO_DISCONNECTED,
/** Channel bound to a connection */
BT_ISO_BOUND,
/** Channel in connecting state */ /** Channel in connecting state */
BT_ISO_CONNECT, BT_ISO_CONNECT,
/** Channel ready for upper layer traffic on it */ /** Channel ready for upper layer traffic on it */

9
subsys/bluetooth/host/conn.c
View file

@ -1066,15 +1066,6 @@ void bt_conn_unref(struct bt_conn *conn)
{ {
atomic_val_t old; atomic_val_t old;
/* Cleanup ISO before releasing the last reference to prevent other
* threads reallocating the same connection while cleanup is ongoing.
*/
if (IS_ENABLED(CONFIG_BT_ISO_UNICAST) &&
conn->type == BT_CONN_TYPE_ISO &&
atomic_get(&conn->ref) == 1) {
bt_iso_cleanup(conn);
}
old = atomic_dec(&conn->ref); old = atomic_dec(&conn->ref);
BT_DBG("handle %u ref %d -> %d", conn->handle, old, BT_DBG("handle %u ref %d -> %d", conn->handle, old,

38
subsys/bluetooth/host/iso.c
View file

@ -401,14 +401,22 @@ static void bt_iso_chan_disconnected(struct bt_iso_chan *chan, uint8_t reason)
bt_iso_chan_set_state(chan, BT_ISO_DISCONNECTED); bt_iso_chan_set_state(chan, BT_ISO_DISCONNECTED);
bt_iso_cleanup(chan->iso);
/* The peripheral does not have the concept of a CIG, so once a CIS /* The peripheral does not have the concept of a CIG, so once a CIS
* disconnects it is completely freed by unref'ing it * disconnects it is completely freed by unref'ing it
*/ */
if (IS_ENABLED(CONFIG_BT_ISO_UNICAST) && if (IS_ENABLED(CONFIG_BT_ISO_UNICAST) && !chan->iso->iso.is_bis) {
chan->iso->role == BT_HCI_ROLE_SLAVE) { if (chan->iso->role == BT_HCI_ROLE_SLAVE) {
chan->iso->iso.chan = NULL; bt_conn_unref(chan->iso);
bt_conn_unref(chan->iso); chan->iso = NULL;
chan->iso = NULL; } else {
/* ISO data paths are automatically removed when the
* peripheral/slave disconnects, so we only need to
* move it for the central
*/
bt_iso_remove_data_path(chan->iso);
}
} }
if (chan->ops->disconnected) { if (chan->ops->disconnected) {
@ -443,8 +451,6 @@ const char *bt_iso_chan_state_str(uint8_t state)
switch (state) { switch (state) {
case BT_ISO_DISCONNECTED: case BT_ISO_DISCONNECTED:
return "disconnected"; return "disconnected";
case BT_ISO_BOUND:
return "bound";
case BT_ISO_CONNECT: case BT_ISO_CONNECT:
return "connect"; return "connect";
case BT_ISO_CONNECTED: case BT_ISO_CONNECTED:
@ -466,17 +472,15 @@ void bt_iso_chan_set_state_debug(struct bt_iso_chan *chan, uint8_t state,
/* check transitions validness */ /* check transitions validness */
switch (state) { switch (state) {
case BT_ISO_DISCONNECTED: case BT_ISO_DISCONNECTED:
case BT_ISO_BOUND: /* regardless of old state always allows this states */
/* regardless of old state always allows these states */
break; break;
case BT_ISO_CONNECT: case BT_ISO_CONNECT:
if (chan->state != BT_ISO_BOUND) { if (chan->state != BT_ISO_DISCONNECTED) {
BT_WARN("%s()%d: invalid transition", func, line); BT_WARN("%s()%d: invalid transition", func, line);
} }
break; break;
case BT_ISO_CONNECTED: case BT_ISO_CONNECTED:
if (chan->state != BT_ISO_BOUND && if (chan->state != BT_ISO_CONNECT) {
chan->state != BT_ISO_CONNECT) {
BT_WARN("%s()%d: invalid transition", func, line); BT_WARN("%s()%d: invalid transition", func, line);
} }
break; break;
@ -1163,7 +1167,6 @@ int bt_iso_cig_create(const struct bt_iso_cig_create_param *param,
/* Assign the connection handle */ /* Assign the connection handle */
chan->iso->handle = sys_le16_to_cpu(cig_rsp->handle[i]); chan->iso->handle = sys_le16_to_cpu(cig_rsp->handle[i]);
bt_iso_chan_set_state(chan, BT_ISO_BOUND);
} }
net_buf_unref(rsp); net_buf_unref(rsp);
@ -1185,12 +1188,14 @@ int bt_iso_cig_terminate(struct bt_iso_cig *cig)
for (int i = 0; i < cig->num_cis; i++) { for (int i = 0; i < cig->num_cis; i++) {
if (cig->cis[i]->state != BT_ISO_DISCONNECTED) { if (cig->cis[i]->state != BT_ISO_DISCONNECTED) {
BT_DBG("[%d]: Channel is not disconnected", i); BT_DBG("[%d]: Channel is not disconnected", i);
return -EINVAL;
} }
} }
err = hci_le_remove_cig(cig->id); err = hci_le_remove_cig(cig->id);
if (err != 0) { if (err != 0) {
BT_DBG("Failed to terminate CIG: %d", err); BT_DBG("Failed to terminate CIG: %d", err);
return err;
} }
cleanup_cig(cig); cleanup_cig(cig);
@ -1254,7 +1259,7 @@ int bt_iso_accept(struct bt_conn *acl, struct bt_conn *iso)
} }
bt_iso_chan_add(iso, chan); bt_iso_chan_add(iso, chan);
bt_iso_chan_set_state(chan, BT_ISO_BOUND); bt_iso_chan_set_state(chan, BT_ISO_CONNECT);
return 0; return 0;
} }
@ -1301,8 +1306,8 @@ int bt_iso_chan_connect(const struct bt_iso_connect_param *param, size_t count)
return -EINVAL; return -EINVAL;
} }
if (param[i].iso_chan->state != BT_ISO_BOUND) { if (param[i].iso_chan->state != BT_ISO_DISCONNECTED) {
BT_DBG("[%d]: ISO is not in the BT_ISO_BOUND state: %u", BT_DBG("[%d]: ISO is not in the BT_ISO_DISCONNECTED state: %u",
i, param[i].iso_chan->state); i, param[i].iso_chan->state);
return -EINVAL; return -EINVAL;
} }
@ -1480,7 +1485,6 @@ static int big_init_bis(struct bt_iso_big *big, bool broadcaster)
bis->iso->iso.bis_id = bt_conn_index(bis->iso); bis->iso->iso.bis_id = bt_conn_index(bis->iso);
bt_iso_chan_add(bis->iso, bis); bt_iso_chan_add(bis->iso, bis);
bt_iso_chan_set_state(bis, BT_ISO_BOUND);
} }
return 0; return 0;