From 0ce7dad5b2daa273d3f54483a7755b7ba8adbd34 Mon Sep 17 00:00:00 2001 From: Johan Hedberg Date: Mon, 17 Jun 2019 19:26:47 +0300 Subject: [PATCH] Bluetooth: Allow disconnected state connections to be reconnected Make it possible to initiate new connections from within the disconnect callback. This wasn't completely trivial since there was connection cleanup done through deferred action using the CONN_CLEANUP flag. This patch moves the disconnected callbacks to be run after all cleanup is done. We can't directly do this in the TX thread, since that's internal, so we instead take advantage of the deferred work support and do it using the update_work callback. Since the same cleanup is needed also for BR/EDR connections the work definition is moved from the LE-specific struct to the generic struct bt_conn. A valid bt_conn object in disconnected state is a likely indication of a connection reference leak, so there's a new BT_WARN() for this case in bt_conn_create_le(). Signed-off-by: Johan Hedberg --- subsys/bluetooth/host/conn.c | 50 ++++++++++++++++++--------- subsys/bluetooth/host/conn_internal.h | 6 ++-- subsys/bluetooth/host/hci_core.c | 2 +- 3 files changed, 37 insertions(+), 21 deletions(-) diff --git a/subsys/bluetooth/host/conn.c b/subsys/bluetooth/host/conn.c index 871e9a75e63..1e7fb93d117 100644 --- a/subsys/bluetooth/host/conn.c +++ b/subsys/bluetooth/host/conn.c @@ -236,15 +236,28 @@ static int send_conn_le_param_update(struct bt_conn *conn, return bt_l2cap_update_conn_param(conn, param); } -static void conn_le_update_timeout(struct k_work *work) +static void conn_update_timeout(struct k_work *work) { - struct bt_conn_le *le = CONTAINER_OF(work, struct bt_conn_le, - update_work); - struct bt_conn *conn = CONTAINER_OF(le, struct bt_conn, le); + struct bt_conn *conn = CONTAINER_OF(work, struct bt_conn, update_work); const struct bt_le_conn_param *param; BT_DBG("conn %p", conn); + if (conn->state == BT_CONN_DISCONNECTED) { + bt_l2cap_disconnected(conn); + notify_disconnected(conn); + + /* Release the reference we took for the very first + * state transition. + */ + bt_conn_unref(conn); + return; + } + + if (conn->type != BT_CONN_TYPE_LE) { + return; + } + if (IS_ENABLED(CONFIG_BT_CENTRAL) && conn->role == BT_CONN_ROLE_MASTER) { /* we don't call bt_conn_disconnect as it would also clear @@ -340,6 +353,7 @@ static struct bt_conn *conn_new(void) } (void)memset(conn, 0, sizeof(*conn)); + k_delayed_work_init(&conn->update_work, conn_update_timeout); atomic_set(&conn->ref, 1); @@ -1442,10 +1456,7 @@ static void conn_cleanup(struct bt_conn *conn) bt_conn_reset_rx_state(conn); - /* Release the reference we took for the very first - * state transition. - */ - bt_conn_unref(conn); + k_delayed_work_submit(&conn->update_work, K_NO_WAIT); } int bt_conn_prepare_events(struct k_poll_event events[]) @@ -1530,7 +1541,6 @@ struct bt_conn *bt_conn_add_le(const bt_addr_le_t *peer) conn->type = BT_CONN_TYPE_LE; conn->le.interval_min = BT_GAP_INIT_CONN_INT_MIN; conn->le.interval_max = BT_GAP_INIT_CONN_INT_MAX; - k_delayed_work_init(&conn->le.update_work, conn_le_update_timeout); return conn; } @@ -1582,7 +1592,7 @@ void bt_conn_set_state(struct bt_conn *conn, bt_conn_state_t state) case BT_CONN_CONNECT: if (IS_ENABLED(CONFIG_BT_CENTRAL) && conn->type == BT_CONN_TYPE_LE) { - k_delayed_work_cancel(&conn->le.update_work); + k_delayed_work_cancel(&conn->update_work); } break; default: @@ -1617,18 +1627,16 @@ void bt_conn_set_state(struct bt_conn *conn, bt_conn_state_t state) */ if (old_state == BT_CONN_CONNECTED || old_state == BT_CONN_DISCONNECT) { - bt_l2cap_disconnected(conn); - notify_disconnected(conn); process_unack_tx(conn); /* Cancel Connection Update if it is pending */ if (conn->type == BT_CONN_TYPE_LE) { - k_delayed_work_cancel(&conn->le.update_work); + k_delayed_work_cancel(&conn->update_work); } atomic_set_bit(conn->flags, BT_CONN_CLEANUP); k_poll_signal_raise(&conn_change, 0); - /* The last ref will be dropped by the tx_thread */ + /* The last ref will be dropped during cleanup */ } else if (old_state == BT_CONN_CONNECT) { /* conn->err will be set in this case */ notify_connected(conn); @@ -1664,8 +1672,7 @@ void bt_conn_set_state(struct bt_conn *conn, bt_conn_state_t state) */ if (IS_ENABLED(CONFIG_BT_CENTRAL) && conn->type == BT_CONN_TYPE_LE) { - k_delayed_work_submit(&conn->le.update_work, - CONN_TIMEOUT); + k_delayed_work_submit(&conn->update_work, CONN_TIMEOUT); } break; @@ -1980,7 +1987,7 @@ int bt_conn_disconnect(struct bt_conn *conn, u8_t reason) #endif /* CONFIG_BT_BREDR */ if (IS_ENABLED(CONFIG_BT_CENTRAL)) { - k_delayed_work_cancel(&conn->le.update_work); + k_delayed_work_cancel(&conn->update_work); return bt_hci_cmd_send(BT_HCI_OP_LE_CREATE_CONN_CANCEL, NULL); } @@ -2180,6 +2187,9 @@ struct bt_conn *bt_conn_create_le(const bt_addr_le_t *peer, case BT_CONN_CONNECT: case BT_CONN_CONNECTED: return conn; + case BT_CONN_DISCONNECTED: + BT_WARN("Found valid but disconnected conn object"); + goto start_scan; default: bt_conn_unref(conn); return NULL; @@ -2202,6 +2212,7 @@ struct bt_conn *bt_conn_create_le(const bt_addr_le_t *peer, /* Only default identity supported for now */ conn->id = BT_ID_DEFAULT; +start_scan: bt_conn_set_param_le(conn, param); bt_conn_set_state(conn, BT_CONN_CONNECT_SCAN); @@ -2295,6 +2306,9 @@ struct bt_conn *bt_conn_create_slave_le(const bt_addr_le_t *peer, case BT_CONN_CONNECT: case BT_CONN_CONNECTED: return conn; + case BT_CONN_DISCONNECTED: + BT_WARN("Found valid but disconnected conn object"); + goto start_adv; default: bt_conn_unref(conn); return NULL; @@ -2307,6 +2321,8 @@ struct bt_conn *bt_conn_create_slave_le(const bt_addr_le_t *peer, } conn->id = param->id; + +start_adv: bt_conn_set_state(conn, BT_CONN_CONNECT_DIR_ADV); err = bt_le_adv_start_internal(¶m_int, NULL, 0, NULL, 0, peer); diff --git a/subsys/bluetooth/host/conn_internal.h b/subsys/bluetooth/host/conn_internal.h index 563becd5525..167a0a4b3ad 100644 --- a/subsys/bluetooth/host/conn_internal.h +++ b/subsys/bluetooth/host/conn_internal.h @@ -53,9 +53,6 @@ struct bt_conn_le { u8_t features[8]; struct bt_keys *keys; - - /* Delayed work for connection update and timeout handling */ - struct k_delayed_work update_work; }; #if defined(CONFIG_BT_BREDR) @@ -131,6 +128,9 @@ struct bt_conn { atomic_t ref; + /* Delayed work for connection update and other deferred tasks */ + struct k_delayed_work update_work; + union { struct bt_conn_le le; #if defined(CONFIG_BT_BREDR) diff --git a/subsys/bluetooth/host/hci_core.c b/subsys/bluetooth/host/hci_core.c index eaf72f335a6..ba953781e3f 100644 --- a/subsys/bluetooth/host/hci_core.c +++ b/subsys/bluetooth/host/hci_core.c @@ -989,7 +989,7 @@ static void slave_update_conn_param(struct bt_conn *conn) * The Peripheral device should not perform a Connection Parameter * Update procedure within 5 s after establishing a connection. */ - k_delayed_work_submit(&conn->le.update_work, CONN_UPDATE_TIMEOUT); + k_delayed_work_submit(&conn->update_work, CONN_UPDATE_TIMEOUT); } #if defined(CONFIG_BT_SMP)