This is an automated email from the ASF dual-hosted git repository.
janc pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mynewt-nimble.git
The following commit(s) were added to refs/heads/master by this push:
new ec622cee5 nimble/host: Rework locking in L2CAP
ec622cee5 is described below
commit ec622cee58ed6c19b524ecff74a4a868b8b710c4
Author: Szymon Janc <[email protected]>
AuthorDate: Tue Jul 29 18:19:15 2025 +0200
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 | 8 +-
nimble/host/src/ble_l2cap_sig.c | 388 ++++++++++++++++++----------------
nimble/host/src/ble_l2cap_sig_cmd.c | 16 +-
nimble/host/src/ble_l2cap_sig_priv.h | 44 ++--
nimble/host/test/src/ble_l2cap_test.c | 9 +-
6 files changed, 274 insertions(+), 224 deletions(-)
diff --git a/nimble/host/src/ble_l2cap.c b/nimble/host/src/ble_l2cap.c
index 5a4ff9ad4..69e879918 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 cb74e7b70..0c38f6951 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;
}
@@ -648,10 +648,8 @@ ble_l2cap_coc_recv_ready(struct ble_l2cap_chan *chan,
struct os_mbuf *sdu_rx)
* to be able to send complete SDU.
*/
if (chan->coc_rx.credits < c->initial_credits) {
- ble_hs_unlock();
- ble_l2cap_sig_le_credits(chan->conn_handle, chan->scid,
- c->initial_credits - chan->coc_rx.credits);
- ble_hs_lock();
+ ble_l2cap_sig_le_credits_nolock(chan->conn_handle, chan->scid,
+ c->initial_credits -
chan->coc_rx.credits);
chan->coc_rx.credits = c->initial_credits;
}
diff --git a/nimble/host/src/ble_l2cap_sig.c b/nimble/host/src/ble_l2cap_sig.c
index 3b9e452e5..2325909be 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 @@ done:
}
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,9 +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 len;
+ unsigned int i;
rc = ble_l2cap_sig_mbuf_pullup_base(om, sizeof(*req), &num_of_scids);
if (rc != 0) {
@@ -965,16 +973,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 +987,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,30 +996,40 @@ 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
+ */
+
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]) {
+
+ /* verify for already used CIDs */
+ 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;
}
rc = ble_l2cap_coc_create_srv_chan(conn, le16toh(req->psm), &chans[i]);
if (rc != 0) {
- if (i == 0) {
- /* In case it is very first channel we cannot create it means
PSM is incorrect
- * or we are out of resources. Just send a response now.
- */
+ if (rc == BLE_HS_ENOTSUP) {
+ /* PSM not supported */
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 +1039,78 @@ 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();
- 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);
+ /* We need to set it once as there are same initial parameters for all
+ * the channels, regardless if were accepted or not
+ */
+ if (rsp->mtu == 0) {
+ rsp->mtu = htole16(chans[i]->coc_rx.mtu);
+ rsp->mps = htole16(chans[i]->my_mtu);
+ rsp->credits = htole16(chans[i]->coc_rx.credits);
+ }
+ }
+
+ /* 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++) {
+ if (chans[i]) {
+ rc = ble_l2cap_event_coc_accept(chans[i], le16toh(req->mtu));
+ if (rc != 0) {
+ chans[i]->cb = NULL;
+ rsp->result = htole16(ble_l2cap_sig_ble_hs_err2coc_err(rc));
+ continue;
}
- } else {
- /* Make sure we do not send disconnect event when removing channel
*/
- chans[i]->cb = NULL;
- ble_hs_lock();
- conn = ble_hs_conn_find_assert(conn_handle);
+ rsp->dcids[i] = htole16(chans[i]->scid);
+ }
+ }
+
+ ble_hs_lock();
+ conn = ble_hs_conn_find_assert(conn_handle);
+
+ /* removed not accepted channels */
+ for (i = 0; i < num_of_scids; i++) {
+ if (chans[i] && chans[i]->cb == NULL) {
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();
}
-
- ble_hs_lock();
- conn = ble_hs_conn_find_assert(conn_handle);
}
-done:
- ble_hs_unlock();
- rc = ble_l2cap_sig_tx(conn_handle, txom);
+ /* 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
+ * TODO should we notify app on failure?
+ */
for (i = 0; i < num_of_scids; i++) {
if (chans[i]) {
+ chans[i]->cb = NULL;
ble_hs_conn_delete_chan(conn, chans[i]);
+ chans[i] = NULL;
}
}
- 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);
+ 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 +1192,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 +1375,9 @@ done:
}
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 +1390,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 +1416,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 +1426,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 +1472,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 +1530,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 +1544,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 +1559,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 +1571,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 +1715,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 +1727,7 @@ done:
}
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 +1756,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
@@ -1800,7 +1802,7 @@ ble_l2cap_sig_le_credits_rx(uint16_t conn_handle, struct
ble_l2cap_sig_hdr *hdr,
}
int
-ble_l2cap_sig_le_credits(uint16_t conn_handle, uint16_t scid, uint16_t credits)
+ble_l2cap_sig_le_credits_nolock(uint16_t conn_handle, uint16_t scid, uint16_t
credits)
{
struct ble_l2cap_sig_le_credits *cmd;
struct os_mbuf *txom;
@@ -1815,7 +1817,19 @@ ble_l2cap_sig_le_credits(uint16_t conn_handle, uint16_t
scid, uint16_t credits)
cmd->scid = htole16(scid);
cmd->credits = htole16(credits);
- return ble_l2cap_sig_tx(conn_handle, txom);
+ return ble_l2cap_sig_tx_nolock(conn_handle, txom);
+}
+
+int
+ble_l2cap_sig_le_credits(uint16_t conn_handle, uint16_t scid, uint16_t credits)
+{
+ int rc;
+
+ ble_hs_lock();
+ rc = ble_l2cap_sig_le_credits_nolock(conn_handle, scid, credits);
+ ble_hs_unlock();
+
+ return rc;
}
#endif
diff --git a/nimble/host/src/ble_l2cap_sig_cmd.c
b/nimble/host/src/ble_l2cap_sig_cmd.c
index 48b35249e..381e3a996 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 a698cd0d8..c7e7b5b46 100644
--- a/nimble/host/src/ble_l2cap_sig_priv.h
+++ b/nimble/host/src/ble_l2cap_sig_priv.h
@@ -123,50 +123,54 @@ 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_nolock(uint16_t conn_handle, uint16_t scid,
+ uint16_t credits);
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 9ea7f8533..5d7729a86 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);
