From 99caccb845c1bcca3dd4c60b61e402dc33ddacf0 Mon Sep 17 00:00:00 2001 From: Szymon Janc Date: Tue, 29 Jul 2025 18:19:15 +0200 Subject: [PATCH] nimble/host: Rework locking in L2CAP Main goal here to cleanup how locking is being done in L2CAP code: - avoid double/nested locking - avoid lock-unlock-lock pattern due to public API calls from internal code - provide _nolock variants to be used internally - public API is locked wrapper around _nolock private API - avoid locking in FOO and unlocking in BAR function for code clarify --- nimble/host/src/ble_l2cap.c | 33 ++- nimble/host/src/ble_l2cap_coc.c | 2 +- nimble/host/src/ble_l2cap_sig.c | 374 +++++++++++++------------- nimble/host/src/ble_l2cap_sig_cmd.c | 16 +- nimble/host/src/ble_l2cap_sig_priv.h | 42 +-- nimble/host/test/src/ble_l2cap_test.c | 9 +- 6 files changed, 256 insertions(+), 220 deletions(-) diff --git a/nimble/host/src/ble_l2cap.c b/nimble/host/src/ble_l2cap.c index 5a4ff9ad4b..69e879918a 100644 --- a/nimble/host/src/ble_l2cap.c +++ b/nimble/host/src/ble_l2cap.c @@ -152,7 +152,13 @@ int ble_l2cap_connect(uint16_t conn_handle, uint16_t psm, uint16_t mtu, struct os_mbuf *sdu_rx, ble_l2cap_event_fn *cb, void *cb_arg) { - return ble_l2cap_sig_coc_connect(conn_handle, psm, mtu, sdu_rx, cb, cb_arg); + int rc; + + ble_hs_lock(); + rc = ble_l2cap_sig_connect_nolock(conn_handle, psm, mtu, sdu_rx, cb, cb_arg); + ble_hs_unlock(); + + return rc; } int @@ -183,8 +189,14 @@ ble_l2cap_enhanced_connect(uint16_t conn_handle, uint8_t num, struct os_mbuf *sdu_rx[], ble_l2cap_event_fn *cb, void *cb_arg) { - return ble_l2cap_sig_ecoc_connect(conn_handle, psm, mtu, - num, sdu_rx, cb, cb_arg); + int rc; + + ble_hs_lock(); + rc = ble_l2cap_sig_ecoc_connect_nolock(conn_handle, psm, mtu, num, sdu_rx, + cb, cb_arg); + ble_hs_unlock(); + + return rc; } int @@ -192,6 +204,7 @@ ble_l2cap_reconfig(struct ble_l2cap_chan *chans[], uint8_t num, uint16_t new_mtu { int i; uint16_t conn_handle; + int rc; if (num == 0 || !chans) { return BLE_HS_EINVAL; @@ -206,13 +219,23 @@ ble_l2cap_reconfig(struct ble_l2cap_chan *chans[], uint8_t num, uint16_t new_mtu } } - return ble_l2cap_sig_coc_reconfig(conn_handle, chans, num, new_mtu); + ble_hs_lock(); + rc = ble_l2cap_sig_coc_reconfig_nolock(conn_handle, chans, num, new_mtu); + ble_hs_unlock(); + + return rc; } int ble_l2cap_disconnect(struct ble_l2cap_chan *chan) { - return ble_l2cap_sig_disconnect(chan); + int rc; + + ble_hs_lock(); + rc = ble_l2cap_sig_disconnect_nolock(chan); + ble_hs_unlock(); + + return rc; } /** diff --git a/nimble/host/src/ble_l2cap_coc.c b/nimble/host/src/ble_l2cap_coc.c index cb74e7b70d..baed49eeb0 100644 --- a/nimble/host/src/ble_l2cap_coc.c +++ b/nimble/host/src/ble_l2cap_coc.c @@ -599,8 +599,8 @@ ble_l2cap_coc_le_credits_update(uint16_t conn_handle, uint16_t dcid, if (chan->coc_tx.credits + credits > 0xFFFF) { BLE_HS_LOG(INFO, "LE CoC credits overflow...disconnecting\n"); + ble_l2cap_sig_disconnect_nolock(chan); ble_hs_unlock(); - ble_l2cap_sig_disconnect(chan); return; } diff --git a/nimble/host/src/ble_l2cap_sig.c b/nimble/host/src/ble_l2cap_sig.c index 3b9e452e5b..bd53a91d8b 100644 --- a/nimble/host/src/ble_l2cap_sig.c +++ b/nimble/host/src/ble_l2cap_sig.c @@ -248,16 +248,6 @@ ble_l2cap_sig_proc_free(struct ble_l2cap_sig_proc *proc) } } -static void -ble_l2cap_sig_proc_insert(struct ble_l2cap_sig_proc *proc) -{ - ble_l2cap_sig_dbg_assert_proc_not_inserted(proc); - - ble_hs_lock(); - STAILQ_INSERT_HEAD(&ble_l2cap_sig_procs, proc, next); - ble_hs_unlock(); -} - /** * Tests if a proc entry fits the specified criteria. * @@ -345,14 +335,20 @@ ble_l2cap_sig_proc_set_timer(struct ble_l2cap_sig_proc *proc) } static void -ble_l2cap_sig_process_status(struct ble_l2cap_sig_proc *proc, int status) +ble_l2cap_sig_proc_insert(struct ble_l2cap_sig_proc *proc) { - if (status == 0) { - ble_l2cap_sig_proc_set_timer(proc); - ble_l2cap_sig_proc_insert(proc); - } else { - ble_l2cap_sig_proc_free(proc); - } + BLE_HS_DBG_ASSERT(ble_hs_locked_by_cur_task()); + + ble_l2cap_sig_dbg_assert_proc_not_inserted(proc); + + STAILQ_INSERT_HEAD(&ble_l2cap_sig_procs, proc, next); +} + +static void +ble_l2cap_sig_proc_start(struct ble_l2cap_sig_proc *proc) +{ + ble_l2cap_sig_proc_set_timer(proc); + ble_l2cap_sig_proc_insert(proc); } /***************************************************************************** @@ -564,9 +560,9 @@ ble_l2cap_sig_update_rsp_rx(uint16_t conn_handle, } int -ble_l2cap_sig_update(uint16_t conn_handle, - struct ble_l2cap_sig_update_params *params, - ble_l2cap_sig_update_fn *cb, void *cb_arg) +ble_l2cap_sig_update_nolock(uint16_t conn_handle, + struct ble_l2cap_sig_update_params *params, + ble_l2cap_sig_update_fn *cb, void *cb_arg) { struct os_mbuf *txom; struct ble_l2cap_sig_update_req *req; @@ -580,30 +576,25 @@ ble_l2cap_sig_update(uint16_t conn_handle, STATS_INC(ble_l2cap_stats, update_init); - ble_hs_lock(); rc = ble_hs_misc_conn_chan_find_reqd(conn_handle, BLE_L2CAP_CID_SIG, &conn, &chan); if (rc != 0) { - ble_hs_unlock(); - goto done; + return rc; } master = conn->bhc_flags & BLE_HS_CONN_F_MASTER; - ble_hs_unlock(); if (master) { /* Only the slave can initiate the L2CAP connection update * procedure. */ - rc = BLE_HS_EINVAL; - goto done; + return BLE_HS_EINVAL; } proc = ble_l2cap_sig_proc_alloc(); if (proc == NULL) { STATS_INC(ble_l2cap_stats, update_fail); - rc = BLE_HS_ENOMEM; - goto done; + return BLE_HS_ENOMEM; } proc->op = BLE_L2CAP_SIG_PROC_OP_UPDATE; @@ -616,8 +607,8 @@ ble_l2cap_sig_update(uint16_t conn_handle, sizeof(*req), &txom); if (!req) { STATS_INC(ble_l2cap_stats, update_fail); - rc = BLE_HS_ENOMEM; - goto done; + ble_l2cap_sig_proc_free(proc); + return BLE_HS_ENOMEM; } req->itvl_min = htole16(params->itvl_min); @@ -625,10 +616,27 @@ ble_l2cap_sig_update(uint16_t conn_handle, req->slave_latency = htole16(params->slave_latency); req->timeout_multiplier = htole16(params->timeout_multiplier); - rc = ble_l2cap_sig_tx(conn_handle, txom); + rc = ble_l2cap_sig_tx_nolock(conn_handle, txom); + if (rc) { + STATS_INC(ble_l2cap_stats, update_fail); + ble_l2cap_sig_proc_free(proc); + return rc; + } + + ble_l2cap_sig_proc_start(proc); + return 0; +} + +int +ble_l2cap_sig_update(uint16_t conn_handle, struct ble_l2cap_sig_update_params *params, + ble_l2cap_sig_update_fn *cb, void *cb_arg) +{ + int rc; + + ble_hs_lock(); + rc = ble_l2cap_sig_update_nolock(conn_handle, params, cb, cb_arg); + ble_hs_unlock(); -done: - ble_l2cap_sig_process_status(proc, rc); return rc; } @@ -858,6 +866,8 @@ ble_l2cap_sig_credit_base_reconfig_req_rx(uint16_t conn_handle, goto failed; } + ble_l2cap_sig_tx_nolock(conn_handle, txom); + ble_hs_unlock(); for (i = 0; i < cid_cnt; i++) { @@ -866,12 +876,11 @@ ble_l2cap_sig_credit_base_reconfig_req_rx(uint16_t conn_handle, ble_l2cap_event_coc_reconfigured(conn_handle, 0, chan[i], true); } - ble_l2cap_sig_tx(conn_handle, txom); return 0; failed: + ble_l2cap_sig_tx_nolock(conn_handle, txom); ble_hs_unlock(); - ble_l2cap_sig_tx(conn_handle, txom); return 0; } @@ -945,8 +954,8 @@ ble_l2cap_sig_credit_base_con_req_rx(uint16_t conn_handle, struct ble_hs_conn *conn; uint16_t scid; unsigned int num_of_scids; - uint8_t chan_created = 0; - int i; + unsigned int i; + unsigned int j; unsigned int len; rc = ble_l2cap_sig_mbuf_pullup_base(om, sizeof(*req), &num_of_scids); @@ -965,16 +974,9 @@ ble_l2cap_sig_credit_base_con_req_rx(uint16_t conn_handle, return 0; } - ble_hs_lock(); - memset(rsp, 0, len); - /* Initial dummy values in case of error, just to satisfy PTS */ - rsp->credits = htole16(1); - rsp->mps = htole16(BLE_L2CAP_ECOC_MIN_MTU); - rsp->mtu = htole16(BLE_L2CAP_ECOC_MIN_MTU); - - if (hdr->length <= sizeof(*req)) { + if (hdr->length <= sizeof(*req) || num_of_scids > ARRAY_SIZE(chans)) { rsp->result = htole16(BLE_L2CAP_COC_ERR_INVALID_PARAMETERS); goto failed; } @@ -986,8 +988,6 @@ ble_l2cap_sig_credit_base_con_req_rx(uint16_t conn_handle, goto failed; } - conn = ble_hs_conn_find_assert(conn_handle); - /* First verify that provided SCIDs are good */ for (i = 0; i < num_of_scids; i++) { scid = le16toh(req->scids[i]); @@ -997,16 +997,28 @@ ble_l2cap_sig_credit_base_con_req_rx(uint16_t conn_handle, } } - /* Let us try to connect channels */ + ble_hs_lock(); + /* we should not handle any disconnects in between unlock-lock so this can + * be asserted + */ + conn = ble_hs_conn_find_assert(conn_handle); + + /* Note: to simplify implementation stop processing further channels on + * error + */ + + /* verify for already used CIDs */ for (i = 0; i < num_of_scids; i++) { - /* Verify CID. Note, scid in the request is dcid for out local channel */ scid = le16toh(req->scids[i]); - chans[i] = ble_hs_conn_chan_find_by_dcid(conn, scid); - if (chans[i]) { + if (ble_hs_conn_chan_find_by_dcid(conn, scid)) { rsp->result = htole16(BLE_L2CAP_COC_ERR_SOURCE_CID_ALREADY_USED); - rsp->dcids[i] = htole16(chans[i]->scid); - continue; + ble_hs_unlock(); + goto failed; } + } + + for (i = 0; i < num_of_scids; i++) { + scid = le16toh(req->scids[i]); rc = ble_l2cap_coc_create_srv_chan(conn, le16toh(req->psm), &chans[i]); if (rc != 0) { @@ -1015,12 +1027,16 @@ ble_l2cap_sig_credit_base_con_req_rx(uint16_t conn_handle, * or we are out of resources. Just send a response now. */ rsp->result = htole16(ble_l2cap_sig_ble_hs_err2coc_err(rc)); + ble_hs_unlock(); goto failed; - } else { - /* We cannot create number of channels req by peer due to limited resources. */ - rsp->result = htole16(BLE_L2CAP_COC_ERR_NO_RESOURCES); - goto done; } + + /* We cannot process more due to limited resources + * If we limit further due to eg. authorization result will be + * overwritten later on + */ + rsp->result = htole16(BLE_L2CAP_COC_ERR_NO_RESOURCES); + break; } /* Fill up remote configuration. Note MPS is the L2CAP MTU*/ @@ -1030,64 +1046,69 @@ ble_l2cap_sig_credit_base_con_req_rx(uint16_t conn_handle, chans[i]->coc_tx.mtu = le16toh(req->mtu); ble_hs_conn_chan_insert(conn, chans[i]); - /* Sending event to the app. Unlock hs */ - ble_hs_unlock(); + } + + /* We need to set it once as there are same initial parameters for all + * the channels, regardless if were accepted or not + */ + rsp->credits = htole16(chans[0]->coc_rx.credits); + rsp->mps = htole16(chans[0]->my_mtu); + rsp->mtu = htole16(chans[0]->coc_rx.mtu); + + /* further process only channels we have resources for */ + num_of_scids = i; + /* Sending events to the app. Unlock hs */ + ble_hs_unlock(); + + for (i = 0; i < num_of_scids; i++) { rc = ble_l2cap_event_coc_accept(chans[i], le16toh(req->mtu)); - if (rc == 0) { - rsp->dcids[i] = htole16(chans[i]->scid); - chan_created++; - if (chan_created == 1) { - /* We need to set it once as there are same initial parameters - * for all the channels - */ - rsp->credits = htole16(chans[i]->coc_rx.credits); - rsp->mps = htole16(chans[i]->my_mtu); - rsp->mtu = htole16(chans[i]->coc_rx.mtu); - } - } else { - /* Make sure we do not send disconnect event when removing channel */ + if (rc != 0) { chans[i]->cb = NULL; - - ble_hs_lock(); - conn = ble_hs_conn_find_assert(conn_handle); - ble_hs_conn_delete_chan(conn, chans[i]); - chans[i] = NULL; rsp->result = htole16(ble_l2cap_sig_ble_hs_err2coc_err(rc)); - rc = 0; - ble_hs_unlock(); + break; } - ble_hs_lock(); - conn = ble_hs_conn_find_assert(conn_handle); + rsp->dcids[i] = htole16(chans[i]->scid); } -done: - ble_hs_unlock(); - rc = ble_l2cap_sig_tx(conn_handle, txom); + ble_hs_lock(); + conn = ble_hs_conn_find_assert(conn_handle); + + /* removed not accepted channels */ + for (j = i; j < num_of_scids; j++) { + /* Make sure we do not send disconnect event when removing channel */ + chans[j]->cb = NULL; + ble_hs_conn_delete_chan(conn, chans[j]); + } + + num_of_scids = i; + + /* send response */ + rc = ble_l2cap_sig_tx_nolock(conn_handle, txom); if (rc != 0) { - ble_hs_lock(); - conn = ble_hs_conn_find_assert(conn_handle); + /* cleanup if we failed to send response */ for (i = 0; i < num_of_scids; i++) { - if (chans[i]) { - ble_hs_conn_delete_chan(conn, chans[i]); - } + chans[i]->cb = NULL; + ble_hs_conn_delete_chan(conn, chans[i]); } - ble_hs_unlock(); - return 0; } + ble_hs_unlock(); + /* Notify user about connection status */ for (i = 0; i < num_of_scids; i++) { - if (chans[i]) { - ble_l2cap_event_coc_connected(chans[i], rc); - } + assert(chans[i]); + ble_l2cap_event_coc_connected(chans[i], 0); } return 0; failed: - ble_hs_unlock(); + /* dummy values in case of error, just to satisfy PTS */ + rsp->credits = htole16(1); + rsp->mps = htole16(BLE_L2CAP_ECOC_MIN_MTU); + rsp->mtu = htole16(BLE_L2CAP_ECOC_MIN_MTU); ble_l2cap_sig_tx(conn_handle, txom); return 0; } @@ -1169,19 +1190,16 @@ ble_l2cap_sig_credit_base_con_rsp_rx(uint16_t conn_handle, ble_hs_conn_chan_insert(conn, chan); } - ble_hs_unlock(); - -done: - for (i = 0; i < 5; i++){ - if (duplicated_cids[i] != 0){ - ble_hs_lock(); - conn = ble_hs_conn_find(conn_handle); + for (i = 0; i < ARRAY_SIZE(duplicated_cids); i++) { + if (duplicated_cids[i] != 0) { chan = ble_hs_conn_chan_find_by_dcid(conn, duplicated_cids[i]); - ble_hs_unlock(); - rc = ble_l2cap_sig_disconnect(chan); + rc |= ble_l2cap_sig_disconnect_nolock(chan); } } + ble_hs_unlock(); + +done: ble_l2cap_sig_coc_connect_cb(proc, rc); ble_l2cap_sig_proc_free(proc); @@ -1355,9 +1373,9 @@ ble_l2cap_sig_coc_rsp_rx(uint16_t conn_handle, struct ble_l2cap_sig_hdr *hdr, } int -ble_l2cap_sig_coc_connect(uint16_t conn_handle, uint16_t psm, uint16_t mtu, - struct os_mbuf *sdu_rx, - ble_l2cap_event_fn *cb, void *cb_arg) +ble_l2cap_sig_connect_nolock(uint16_t conn_handle, uint16_t psm, uint16_t mtu, + struct os_mbuf *sdu_rx, ble_l2cap_event_fn *cb, + void *cb_arg) { struct ble_hs_conn *conn; struct ble_l2cap_sig_proc *proc; @@ -1370,24 +1388,19 @@ ble_l2cap_sig_coc_connect(uint16_t conn_handle, uint16_t psm, uint16_t mtu, return BLE_HS_EINVAL; } - ble_hs_lock(); conn = ble_hs_conn_find(conn_handle); - if (!conn) { - ble_hs_unlock(); return BLE_HS_ENOTCONN; } chan = ble_l2cap_coc_chan_alloc(conn, psm, mtu, sdu_rx, cb, cb_arg); if (!chan) { - ble_hs_unlock(); return BLE_HS_ENOMEM; } proc = ble_l2cap_sig_proc_alloc(); if (!proc) { ble_l2cap_chan_free(conn, chan); - ble_hs_unlock(); return BLE_HS_ENOMEM; } @@ -1401,10 +1414,8 @@ ble_l2cap_sig_coc_connect(uint16_t conn_handle, uint16_t psm, uint16_t mtu, sizeof(*req), &txom); if (!req) { ble_l2cap_chan_free(conn, chan); - ble_hs_unlock(); - rc = BLE_HS_ENOMEM; - /* Goto done to clear proc */ - goto done; + ble_l2cap_sig_proc_free(proc); + return BLE_HS_ENOMEM; } req->psm = htole16(psm); @@ -1413,52 +1424,42 @@ ble_l2cap_sig_coc_connect(uint16_t conn_handle, uint16_t psm, uint16_t mtu, req->mps = htole16(chan->my_coc_mps); req->credits = htole16(chan->coc_rx.credits); - ble_hs_unlock(); - - rc = ble_l2cap_sig_tx(proc->conn_handle, txom); + rc = ble_l2cap_sig_tx_nolock(proc->conn_handle, txom); if (rc != 0) { - ble_hs_lock(); - conn = ble_hs_conn_find_assert(conn_handle); ble_l2cap_chan_free(conn, chan); - ble_hs_unlock(); + ble_l2cap_sig_proc_free(proc); + return rc; } -done: - ble_l2cap_sig_process_status(proc, rc); + ble_l2cap_sig_proc_start(proc); - return rc; + return 0; } #if MYNEWT_VAL(BLE_L2CAP_ENHANCED_COC) int -ble_l2cap_sig_ecoc_connect(uint16_t conn_handle, uint16_t psm, uint16_t mtu, - uint8_t num, struct os_mbuf *sdu_rx[], - ble_l2cap_event_fn *cb, void *cb_arg) +ble_l2cap_sig_ecoc_connect_nolock(uint16_t conn_handle, uint16_t psm, uint16_t mtu, + uint8_t num, struct os_mbuf *sdu_rx[], + ble_l2cap_event_fn *cb, void *cb_arg) { struct ble_hs_conn *conn; struct ble_l2cap_sig_proc *proc; - struct ble_l2cap_chan *chan = NULL; struct os_mbuf *txom; struct ble_l2cap_sig_credit_base_connect_req *req; int rc; int i; - int j; if (!sdu_rx || !cb) { return BLE_HS_EINVAL; } - ble_hs_lock(); conn = ble_hs_conn_find(conn_handle); - if (!conn) { - ble_hs_unlock(); return BLE_HS_ENOTCONN; } proc = ble_l2cap_sig_proc_alloc(); if (!proc) { - ble_hs_unlock(); return BLE_HS_ENOMEM; } @@ -1469,52 +1470,56 @@ ble_l2cap_sig_ecoc_connect(uint16_t conn_handle, uint16_t psm, uint16_t mtu, req = ble_l2cap_sig_cmd_get(BLE_L2CAP_SIG_OP_CREDIT_CONNECT_REQ, proc->id, sizeof(*req) + num * sizeof(uint16_t), &txom); if (!req) { - ble_hs_unlock(); - rc = BLE_HS_ENOMEM; - /* Goto done to clear proc */ - goto done; + ble_l2cap_sig_proc_free(proc); + return BLE_HS_ENOMEM; } for (i = 0; i < num; i++) { - chan = ble_l2cap_coc_chan_alloc(conn, psm, mtu, sdu_rx[i], cb, cb_arg); - if (!chan) { + proc->connect.chan[i] = + ble_l2cap_coc_chan_alloc(conn, psm, mtu, sdu_rx[i], cb, cb_arg); + if (!proc->connect.chan[i]) { /* Clear request buffer */ os_mbuf_free_chain(txom); - - for (j = 0; j < i; j++) { - /* Clear callback to make sure "Disconnected event" to the user */ - chan[j].cb = NULL; - ble_l2cap_chan_free(conn, proc->connect.chan[j]); - } - ble_hs_unlock(); rc = BLE_HS_ENOMEM; - goto done; + goto failed; } - proc->connect.chan[i] = chan; } proc->connect.chan_cnt = num; + /* all channels have same params */ req->psm = htole16(psm); - req->mtu = htole16(chan->coc_rx.mtu); - req->mps = htole16(chan->my_mtu); - req->credits = htole16(chan->coc_rx.credits); + req->mtu = htole16(proc->connect.chan[0]->coc_rx.mtu); + req->mps = htole16(proc->connect.chan[0]->my_mtu); + req->credits = htole16(proc->connect.chan[0]->coc_rx.credits); for (i = 0; i < num; i++) { req->scids[i] = htole16(proc->connect.chan[i]->scid); } - ble_hs_unlock(); + rc = ble_l2cap_sig_tx_nolock(proc->conn_handle, txom); + if (rc) { + rc = BLE_HS_ENOMEM; + goto failed; + } - rc = ble_l2cap_sig_tx(proc->conn_handle, txom); + ble_l2cap_sig_proc_start(proc); -done: - ble_l2cap_sig_process_status(proc, rc); + return 0; - return rc; +failed: + /* clean up on failure, ble_l2cap_chan_free() handles NULL as well */ + for (i = 0; i < num; i++) { + proc->connect.chan[i]->cb = NULL; + ble_l2cap_chan_free(conn, proc->connect.chan[i]); + } + + ble_l2cap_sig_proc_free(proc); + return BLE_HS_ENOMEM; } int -ble_l2cap_sig_coc_reconfig(uint16_t conn_handle, struct ble_l2cap_chan *chans[], - uint8_t num, uint16_t new_mtu) +ble_l2cap_sig_coc_reconfig_nolock(uint16_t conn_handle, + struct ble_l2cap_chan *chans[], uint8_t num, + uint16_t new_mtu) { struct ble_hs_conn *conn; struct ble_l2cap_sig_proc *proc; @@ -1523,17 +1528,13 @@ ble_l2cap_sig_coc_reconfig(uint16_t conn_handle, struct ble_l2cap_chan *chans[], int rc; int i; - ble_hs_lock(); conn = ble_hs_conn_find(conn_handle); - if (!conn) { - ble_hs_unlock(); return BLE_HS_ENOTCONN; } proc = ble_l2cap_sig_proc_alloc(); if (!proc) { - ble_hs_unlock(); return BLE_HS_ENOMEM; } @@ -1541,9 +1542,8 @@ ble_l2cap_sig_coc_reconfig(uint16_t conn_handle, struct ble_l2cap_chan *chans[], if (ble_hs_conn_chan_exist(conn, chans[i])) { proc->reconfig.cids[i] = chans[i]->scid; } else { - ble_hs_unlock(); - rc = BLE_HS_ENOMEM; - goto done; + ble_l2cap_sig_proc_free(proc); + return BLE_HS_ENOMEM; } } @@ -1557,9 +1557,8 @@ ble_l2cap_sig_coc_reconfig(uint16_t conn_handle, struct ble_l2cap_chan *chans[], req = ble_l2cap_sig_cmd_get(BLE_L2CAP_SIG_OP_CREDIT_RECONFIG_REQ, proc->id, sizeof(*req) + num * sizeof(uint16_t), &txom); if (!req) { - ble_hs_unlock(); - rc = BLE_HS_ENOMEM; - goto done; + ble_l2cap_sig_proc_free(proc); + return BLE_HS_ENOMEM; } /* For now we allow to change CoC MTU only.*/ @@ -1570,14 +1569,14 @@ ble_l2cap_sig_coc_reconfig(uint16_t conn_handle, struct ble_l2cap_chan *chans[], req->dcids[i] = htole16(proc->reconfig.cids[i]); } - ble_hs_unlock(); - - rc = ble_l2cap_sig_tx(proc->conn_handle, txom); - -done: - ble_l2cap_sig_process_status(proc, rc); + rc = ble_l2cap_sig_tx_nolock(proc->conn_handle, txom); + if (rc) { + ble_l2cap_sig_proc_free(proc); + return rc; + } - return rc; + ble_l2cap_sig_proc_start(proc); + return 0; } #endif @@ -1714,7 +1713,7 @@ ble_l2cap_sig_disc_rsp_rx(uint16_t conn_handle, struct ble_l2cap_sig_hdr *hdr, rsp = (struct ble_l2cap_sig_disc_rsp *)(*om)->om_data; if (chan->dcid != le16toh(rsp->dcid) || chan->scid != le16toh(rsp->scid)) { /* This response is incorrect, lets wait for timeout */ - ble_l2cap_sig_process_status(proc, 0); + ble_l2cap_sig_proc_start(proc); return 0; } @@ -1726,7 +1725,7 @@ ble_l2cap_sig_disc_rsp_rx(uint16_t conn_handle, struct ble_l2cap_sig_hdr *hdr, } int -ble_l2cap_sig_disconnect(struct ble_l2cap_chan *chan) +ble_l2cap_sig_disconnect_nolock(struct ble_l2cap_chan *chan) { struct os_mbuf *txom; struct ble_l2cap_sig_disc_req *req; @@ -1755,23 +1754,24 @@ ble_l2cap_sig_disconnect(struct ble_l2cap_chan *chan) req = ble_l2cap_sig_cmd_get(BLE_L2CAP_SIG_OP_DISCONN_REQ, proc->id, sizeof(*req), &txom); if (!req) { - rc = BLE_HS_ENOMEM; - goto done; + ble_l2cap_sig_proc_free(proc); + return BLE_HS_ENOMEM; } req->dcid = htole16(chan->dcid); req->scid = htole16(chan->scid); - rc = ble_l2cap_sig_tx(proc->conn_handle, txom); - /* Mark channel as disconnecting */ - if (rc == 0) { - chan->flags |= BLE_L2CAP_CHAN_F_DISCONNECTING; + rc = ble_l2cap_sig_tx_nolock(proc->conn_handle, txom); + if (rc) { + ble_l2cap_sig_proc_free(proc); + return rc; } -done: - ble_l2cap_sig_process_status(proc, rc); + /* Mark channel as disconnecting */ + chan->flags |= BLE_L2CAP_CHAN_F_DISCONNECTING; + ble_l2cap_sig_proc_start(proc); - return rc; + return 0; } static int diff --git a/nimble/host/src/ble_l2cap_sig_cmd.c b/nimble/host/src/ble_l2cap_sig_cmd.c index 48b35249ec..381e3a996a 100644 --- a/nimble/host/src/ble_l2cap_sig_cmd.c +++ b/nimble/host/src/ble_l2cap_sig_cmd.c @@ -23,13 +23,14 @@ #if NIMBLE_BLE_CONNECT /* this function consumes tx os_mbuf */ int -ble_l2cap_sig_tx(uint16_t conn_handle, struct os_mbuf *txom) +ble_l2cap_sig_tx_nolock(uint16_t conn_handle, struct os_mbuf *txom) { struct ble_l2cap_chan *chan; struct ble_hs_conn *conn; int rc; - ble_hs_lock(); + BLE_HS_DBG_ASSERT(ble_hs_locked_by_cur_task()); + rc = ble_hs_misc_conn_chan_find_reqd(conn_handle, BLE_L2CAP_CID_SIG, &conn, &chan); if (rc == 0) { @@ -37,6 +38,17 @@ ble_l2cap_sig_tx(uint16_t conn_handle, struct os_mbuf *txom) } else { os_mbuf_free_chain(txom); } + + return rc; +} + +int +ble_l2cap_sig_tx(uint16_t conn_handle, struct os_mbuf *txom) +{ + int rc; + + ble_hs_lock(); + rc = ble_l2cap_sig_tx_nolock(conn_handle, txom); ble_hs_unlock(); return rc; diff --git a/nimble/host/src/ble_l2cap_sig_priv.h b/nimble/host/src/ble_l2cap_sig_priv.h index a698cd0d8f..d6d04d3a60 100644 --- a/nimble/host/src/ble_l2cap_sig_priv.h +++ b/nimble/host/src/ble_l2cap_sig_priv.h @@ -123,50 +123,52 @@ int ble_l2cap_sig_reject_tx(uint16_t conn_handle, int ble_l2cap_sig_reject_invalid_cid_tx(uint16_t conn_handle, uint8_t id, uint16_t src_cid, uint16_t dst_cid); int ble_l2cap_sig_tx(uint16_t conn_handle, struct os_mbuf *txom); +int ble_l2cap_sig_tx_nolock(uint16_t conn_handle, struct os_mbuf *txom); void *ble_l2cap_sig_cmd_get(uint8_t opcode, uint8_t id, uint16_t len, struct os_mbuf **txom); #if MYNEWT_VAL(BLE_L2CAP_COC_MAX_NUM) != 0 -int ble_l2cap_sig_coc_connect(uint16_t conn_handle, uint16_t psm, uint16_t mtu, - struct os_mbuf *sdu_rx, - ble_l2cap_event_fn *cb, void *cb_arg); -int ble_l2cap_sig_disconnect(struct ble_l2cap_chan *chan); +int ble_l2cap_sig_connect_nolock(uint16_t conn_handle, uint16_t psm, + uint16_t mtu, struct os_mbuf *sdu_rx, + ble_l2cap_event_fn *cb, void *cb_arg); +int ble_l2cap_sig_disconnect_nolock(struct ble_l2cap_chan *chan); int ble_l2cap_sig_le_credits(uint16_t conn_handle, uint16_t scid, uint16_t credits); #else static inline int -ble_l2cap_sig_coc_connect(uint16_t conn_handle, uint16_t psm, uint16_t mtu, - struct os_mbuf *sdu_rx, - ble_l2cap_event_fn *cb, void *cb_arg) +ble_l2cap_sig_connect_nolock(uint16_t conn_handle, uint16_t psm, uint16_t mtu, + struct os_mbuf *sdu_rx, ble_l2cap_event_fn *cb, + void *cb_arg) { return BLE_HS_ENOTSUP; } static inline int -ble_l2cap_sig_disconnect(struct ble_l2cap_chan *chan) +ble_l2cap_sig_disconnect_nolock(struct ble_l2cap_chan *chan) { return BLE_HS_ENOTSUP; } #endif #if MYNEWT_VAL(BLE_L2CAP_ENHANCED_COC) -int ble_l2cap_sig_ecoc_connect(uint16_t conn_handle, - uint16_t psm, uint16_t mtu, - uint8_t num, struct os_mbuf *sdu_rx[], - ble_l2cap_event_fn *cb, void *cb_arg); -int ble_l2cap_sig_coc_reconfig(uint16_t conn_handle, struct ble_l2cap_chan *chans[], - uint8_t num, uint16_t new_mtu); +int ble_l2cap_sig_ecoc_connect_nolock(uint16_t conn_handle, uint16_t psm, uint16_t mtu, + uint8_t num, struct os_mbuf *sdu_rx[], + ble_l2cap_event_fn *cb, void *cb_arg); +int ble_l2cap_sig_coc_reconfig_nolock(uint16_t conn_handle, + struct ble_l2cap_chan *chans[], + uint8_t num, uint16_t new_mtu); #else static inline int -ble_l2cap_sig_ecoc_connect(uint16_t conn_handle, - uint16_t psm, uint16_t mtu, - uint8_t num, struct os_mbuf *sdu_rx[], - ble_l2cap_event_fn *cb, void *cb_arg) +ble_l2cap_sig_ecoc_connect_nolock(uint16_t conn_handle, uint16_t psm, uint16_t mtu, + uint8_t num, struct os_mbuf *sdu_rx[], + ble_l2cap_event_fn *cb, void *cb_arg) { return BLE_HS_ENOTSUP; } + static inline int -ble_l2cap_sig_coc_reconfig(uint16_t conn_handle, struct ble_l2cap_chan *chans[], - uint8_t num, uint16_t new_mtu) +ble_l2cap_sig_coc_reconfig_nolock(uint16_t conn_handle, + struct ble_l2cap_chan *chans[], uint8_t num, + uint16_t new_mtu) { return BLE_HS_ENOTSUP; } diff --git a/nimble/host/test/src/ble_l2cap_test.c b/nimble/host/test/src/ble_l2cap_test.c index 9ea7f8533a..5d7729a86a 100644 --- a/nimble/host/test/src/ble_l2cap_test.c +++ b/nimble/host/test/src/ble_l2cap_test.c @@ -786,8 +786,8 @@ ble_l2cap_test_coc_connect_multi(struct test_data *t) assert(sdu_rx[i] != NULL); } - rc = ble_l2cap_sig_ecoc_connect(2, t->psm, t->mtu, t->num, sdu_rx, - ble_l2cap_test_event, t); + rc = ble_l2cap_enhanced_connect(2, t->psm, t->mtu, t->num, sdu_rx, + ble_l2cap_test_event, t); TEST_ASSERT_FATAL(rc == ev->early_error); if (rc != 0) { @@ -851,8 +851,7 @@ ble_l2cap_test_coc_connect(struct test_data *t) sdu_rx = os_mbuf_get_pkthdr(&sdu_os_mbuf_pool, 0); assert(sdu_rx != NULL); - rc = ble_l2cap_sig_coc_connect(2, t->psm, t->mtu, sdu_rx, - ble_l2cap_test_event, t); + rc = ble_l2cap_connect(2, t->psm, t->mtu, sdu_rx, ble_l2cap_test_event, t); TEST_ASSERT_FATAL(rc == ev->early_error); if (rc != 0) { @@ -955,7 +954,7 @@ ble_l2cap_test_coc_disc(struct test_data *t) uint8_t id; int rc; - rc = ble_l2cap_sig_disconnect(t->chan[0]); + rc = ble_l2cap_disconnect(t->chan[0]); TEST_ASSERT_FATAL(rc == 0); req.dcid = htole16(t->chan[0]->dcid);