From f9e37e59ffac434f8da1bfc39277eea6e1e31020 Mon Sep 17 00:00:00 2001 From: Aleksander Wasaznik Date: Mon, 7 Jul 2025 10:48:46 +0200 Subject: [PATCH 1/6] Revert "[nrf noup] bluetooth: conn: Skip buffer ref count check in send_buf" This reverts commit 827eb444b1947eb198d4dad28f40ab38e5d3f1eb. Signed-off-by: Aleksander Wasaznik --- subsys/bluetooth/host/conn.c | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/subsys/bluetooth/host/conn.c b/subsys/bluetooth/host/conn.c index 6d8939eca36..856f53846f3 100644 --- a/subsys/bluetooth/host/conn.c +++ b/subsys/bluetooth/host/conn.c @@ -698,17 +698,13 @@ static int send_buf(struct bt_conn *conn, struct net_buf *buf, uint16_t frag_len = MIN(conn_mtu(conn), len); - /* If ATT sent callback is delayed until data transmission is done by BLE controller, the - * transmitted buffer may have an additional reference. The reference is used to extend - * lifetime of the net buffer until the data transmission is confirmed by ACK of the remote. - * - * send_buf function can be called multiple times, if buffer has to be fragmented over HCI. - * In that case, the callback is provided as an argument only for the last transmitted - * fragment. The `buf->ref == 1` check is skipped because it's impossible to properly - * validate number of references for the sent fragments if buffers may have the additional - * reference. - */ - __ASSERT_NO_MSG(IS_ENABLED(CONFIG_BT_ATT_SENT_CB_AFTER_TX) || (buf->ref == 1)); + if (buf->ref > 1 + (cb ? 1 : 0)) { + /* Allow for an additional buffer reference if callback is provided. + * This can be used to extend lifetime of the net buffer until the + * data transmission is confirmed by ACK of the remote. + */ + __ASSERT_NO_MSG(false); + } if (buf->len > frag_len) { LOG_DBG("keep %p around", buf); From 5394bcc8e2fbd0764a35857ab0c6ef30f2f2b813 Mon Sep 17 00:00:00 2001 From: Aleksander Wasaznik Date: Mon, 7 Jul 2025 11:02:36 +0200 Subject: [PATCH 2/6] Revert "[nrf noup] bluetooth: conn: Allow for an extra ref in bt_l2cap_send_pdu" This reverts commit 685c53acebd503a6931b8c2811d8a1e74e0e4828. Signed-off-by: Aleksander Wasaznik --- subsys/bluetooth/host/conn.c | 8 +------- subsys/bluetooth/host/l2cap.c | 8 ++------ 2 files changed, 3 insertions(+), 13 deletions(-) diff --git a/subsys/bluetooth/host/conn.c b/subsys/bluetooth/host/conn.c index 856f53846f3..a7aca69fa9d 100644 --- a/subsys/bluetooth/host/conn.c +++ b/subsys/bluetooth/host/conn.c @@ -698,13 +698,7 @@ static int send_buf(struct bt_conn *conn, struct net_buf *buf, uint16_t frag_len = MIN(conn_mtu(conn), len); - if (buf->ref > 1 + (cb ? 1 : 0)) { - /* Allow for an additional buffer reference if callback is provided. - * This can be used to extend lifetime of the net buffer until the - * data transmission is confirmed by ACK of the remote. - */ - __ASSERT_NO_MSG(false); - } + __ASSERT_NO_MSG(buf->ref == 1); if (buf->len > frag_len) { LOG_DBG("keep %p around", buf); diff --git a/subsys/bluetooth/host/l2cap.c b/subsys/bluetooth/host/l2cap.c index 683aafdf561..433eb8faef0 100644 --- a/subsys/bluetooth/host/l2cap.c +++ b/subsys/bluetooth/host/l2cap.c @@ -735,17 +735,13 @@ int bt_l2cap_send_pdu(struct bt_l2cap_le_chan *le_chan, struct net_buf *pdu, return -ENOTCONN; } - /* Allow for an additional buffer reference if callback is provided. This can be used to - * extend lifetime of the net buffer until the data transmission is confirmed by ACK of the - * remote. - */ - if (pdu->ref > 1 + (cb ? 1 : 0)) { + if (pdu->ref != 1) { /* The host may alter the buf contents when fragmenting. Higher * layers cannot expect the buf contents to stay intact. Extra * refs suggests a silent data corruption would occur if not for * this error. */ - LOG_ERR("Expecting up to %d refs, got %d", cb ? 2 : 1, pdu->ref); + LOG_ERR("Expecting 1 ref, got %d", pdu->ref); return -EINVAL; } From 8622c2df7c444eda9b2f6e109190f385cdf7bf59 Mon Sep 17 00:00:00 2001 From: "Mike J. Chen" Date: Wed, 5 Mar 2025 16:39:00 -0800 Subject: [PATCH 3/6] [nrf fromtree] bluetooth: fix bug when destroying tx queue buffers on disconnect Channel tx_queue purging on disconnect was inconsistently handled by the different channels: iso, l2cap, l2cap_br. iso channels handled purging in the tx_data_pull hook. l2cap and l2cap_br did the purging in channel delete functions and did not expect tx_data_pull to be called for a disconnected channel. Their data_pull functions could return a ptr to a net_buf that was still on the tx_queue, which is problematic when the conn tx_processor unrefs the returned buffer resulting in multiple calls to the buf destroy function. To make things consistent and correct, remove the code that tries to purge tx_queues in the tx_processor and only do purging in the channels themselves when they are deleted/disconnected. Also refactor and clarify referencing of the net_buf returned by tx_data_pull. It was confusing who had a reference and when, which could vary depending on the length of the original buffer. There are three cases: the buffer length is less than the tx.mps, greater the mps but less than the mtu so requiring segementation but not fragmentation, or greater than both mps and mtu so requiring both segmentation and fragmentation. The conn layer would increase the refcnt if the length was greater than the mtu, but not have any awareness of whether the net_buf was still on the tx_queue or not. Now it is the tx_data_pull callbacks responsibitity to increment the reference count if it is returning a pointer to a net_buf that it is still keeping on the tx_queue for segmentation purposes. The conn layer will now always transfer that reference into a fragment view and not conditional it on the length relative to the mtu, and always decrement the reference to the parent when the fragment is destroyed. So there is no risk of decrementing a reference to a net buf that might still be on a tx_queue, which simplifies error handling in particular. Also add error handling paths for when asserts are not enabled. Signed-off-by: Mike J. Chen (cherry picked from commit a392c33c60286574b74fe708e1a182dd99666c8d) --- subsys/bluetooth/host/classic/l2cap_br.c | 16 +++- subsys/bluetooth/host/conn.c | 111 ++++++++--------------- subsys/bluetooth/host/conn_internal.h | 14 ++- subsys/bluetooth/host/iso.c | 31 ++++--- subsys/bluetooth/host/l2cap.c | 30 +++--- 5 files changed, 98 insertions(+), 104 deletions(-) diff --git a/subsys/bluetooth/host/classic/l2cap_br.c b/subsys/bluetooth/host/classic/l2cap_br.c index 66343aa512d..1d8616013e9 100644 --- a/subsys/bluetooth/host/classic/l2cap_br.c +++ b/subsys/bluetooth/host/classic/l2cap_br.c @@ -1574,6 +1574,8 @@ struct net_buf *l2cap_br_data_pull(struct bt_conn *conn, size_t amount, size_t * return NULL; } + __ASSERT_NO_MSG(conn->state == BT_CONN_CONNECTED); + struct bt_l2cap_br_chan *br_chan; br_chan = CONTAINER_OF(pdu_ready, struct bt_l2cap_br_chan, _pdu_ready); @@ -1591,13 +1593,15 @@ struct net_buf *l2cap_br_data_pull(struct bt_conn *conn, size_t amount, size_t * __ASSERT(tx_pdu, "signaled ready but no PDUs in the TX queue"); - struct net_buf *pdu = CONTAINER_OF(tx_pdu, struct net_buf, node); + struct net_buf *q_pdu = CONTAINER_OF(tx_pdu, struct net_buf, node); - if (bt_buf_has_view(pdu)) { - LOG_ERR("already have view on %p", pdu); + if (bt_buf_has_view(q_pdu)) { + LOG_ERR("already have view on %p", q_pdu); return NULL; } + struct net_buf *pdu = net_buf_ref(q_pdu); + /* We can't interleave ACL fragments from different channels for the * same ACL conn -> we have to wait until a full L2 PDU is transferred * before switching channels. @@ -1605,13 +1609,15 @@ struct net_buf *l2cap_br_data_pull(struct bt_conn *conn, size_t amount, size_t * bool last_frag = amount >= pdu->len; if (last_frag) { - LOG_DBG("last frag, removing %p", pdu); + LOG_DBG("last frag, removing %p", q_pdu); __maybe_unused bool found; - found = sys_slist_find_and_remove(&br_chan->_pdu_tx_queue, &pdu->node); + found = sys_slist_find_and_remove(&br_chan->_pdu_tx_queue, &q_pdu->node); __ASSERT_NO_MSG(found); + net_buf_unref(q_pdu); + LOG_DBG("chan %p done", br_chan); lower_data_ready(br_chan); diff --git a/subsys/bluetooth/host/conn.c b/subsys/bluetooth/host/conn.c index a7aca69fa9d..3ebc2f9ca3c 100644 --- a/subsys/bluetooth/host/conn.c +++ b/subsys/bluetooth/host/conn.c @@ -649,7 +649,7 @@ static bool is_acl_conn(struct bt_conn *conn) } static int send_buf(struct bt_conn *conn, struct net_buf *buf, - size_t len, void *cb, void *ud) + size_t len, bt_conn_tx_cb_t cb, void *ud) { struct net_buf *frag = NULL; struct bt_conn_tx *tx = NULL; @@ -659,13 +659,15 @@ static int send_buf(struct bt_conn *conn, struct net_buf *buf, if (buf->len == 0) { __ASSERT_NO_MSG(0); - return -EMSGSIZE; + err = -EMSGSIZE; + goto error_return; } if (bt_buf_has_view(buf)) { __ASSERT_NO_MSG(0); - return -EIO; + err = -EIO; + goto error_return; } LOG_DBG("conn %p buf %p len %zu buf->len %u cb %p ud %p", @@ -680,7 +682,8 @@ static int send_buf(struct bt_conn *conn, struct net_buf *buf, */ __ASSERT(0, "No controller bufs"); - return -ENOMEM; + err = -ENOMEM; + goto error_return; } /* Allocate and set the TX context */ @@ -689,8 +692,9 @@ static int send_buf(struct bt_conn *conn, struct net_buf *buf, /* See big comment above */ if (!tx) { __ASSERT(0, "No TX context"); - - return -ENOMEM; + k_sem_give(bt_conn_get_pkts(conn)); + err = -ENOMEM; + goto error_return; } tx->cb = cb; @@ -698,18 +702,17 @@ static int send_buf(struct bt_conn *conn, struct net_buf *buf, uint16_t frag_len = MIN(conn_mtu(conn), len); - __ASSERT_NO_MSG(buf->ref == 1); + /* Check that buf->ref is 1 or 2. It would be 1 if this + * was the only reference (e.g. buf was removed + * from the conn tx_queue). It would be 2 if the + * tx_data_pull kept it on the tx_queue for segmentation. + */ + __ASSERT_NO_MSG((buf->ref == 1) || (buf->ref == 2)); - if (buf->len > frag_len) { - LOG_DBG("keep %p around", buf); - frag = get_data_frag(net_buf_ref(buf), frag_len); - } else { - LOG_DBG("move %p ref in", buf); - /* Move the ref into `frag` for the last TX. That way `buf` will - * get destroyed when `frag` is destroyed. - */ - frag = get_data_frag(buf, frag_len); - } + /* The reference is always transferred to the frag, so when + * the frag is destroyed, the parent reference is decremented. + */ + frag = get_data_frag(buf, frag_len); /* Caller is supposed to check we have all resources to send */ __ASSERT_NO_MSG(frag != NULL); @@ -723,7 +726,7 @@ static int send_buf(struct bt_conn *conn, struct net_buf *buf, conn->next_is_frag = false; } - LOG_DBG("send frag: buf %p len %d", buf, frag_len); + LOG_DBG("send frag: buf %p len %d", frag, frag_len); /* At this point, the buffer is either a fragment or a full HCI packet. * The flags are also valid. @@ -766,15 +769,26 @@ static int send_buf(struct bt_conn *conn, struct net_buf *buf, */ net_buf_unref(frag); - /* `buf` might not get destroyed right away, and its `tx` - * pointer will still be reachable. Make sure that we don't try - * to use the destroyed context later. + /* `buf` might not get destroyed right away because it may + * still be on a conn tx_queue, and its `tx` pointer will still + * be reachable. Make sure that we don't try to use the + * destroyed context later. */ conn_tx_destroy(conn, tx); k_sem_give(bt_conn_get_pkts(conn)); /* Merge HCI driver errors */ return -EIO; + +error_return: + /* Runtime handling of fatal errors when ASSERTS are disabled. + * Unref the buf and invoke callback with the error. + */ + net_buf_unref(buf); + if (cb) { + cb(conn, ud, err); + } + return err; } static struct k_poll_signal conn_change = @@ -956,8 +970,8 @@ struct bt_conn *get_conn_ready(void) sys_slist_remove(&bt_dev.le.conn_ready, prev, &conn->_conn_ready); (void)atomic_set(&conn->_conn_ready_lock, 0); - /* Append connection to list if it still has data */ - if (conn->has_data(conn)) { + /* Append connection to list if it is connected and still has data */ + if (conn->has_data(conn) && (conn->state == BT_CONN_CONNECTED)) { LOG_DBG("appending %p to back of TX queue", conn); bt_conn_data_ready(conn); } @@ -985,30 +999,6 @@ static void acl_get_and_clear_cb(struct bt_conn *conn, struct net_buf *buf, } #endif /* defined(CONFIG_BT_CONN) */ -/* Acts as a "null-routed" bt_send(). This fn will decrease the refcount of - * `buf` and call the user callback with an error code. - */ -static void destroy_and_callback(struct bt_conn *conn, - struct net_buf *buf, - bt_conn_tx_cb_t cb, - void *ud) -{ - if (!cb) { - conn->get_and_clear_cb(conn, buf, &cb, &ud); - } - - LOG_DBG("pop: cb %p userdata %p", cb, ud); - - /* bt_send() would've done an unref. Do it here also, so the buffer is - * hopefully destroyed and the user callback can allocate a new one. - */ - net_buf_unref(buf); - - if (cb) { - cb(conn, ud, -ESHUTDOWN); - } -} - static volatile bool _suspend_tx; #if defined(CONFIG_BT_TESTING) @@ -1051,17 +1041,7 @@ void bt_conn_tx_processor(void) if (conn->state != BT_CONN_CONNECTED) { LOG_WRN("conn %p: not connected", conn); - - /* Call the user callbacks & destroy (final-unref) the buffers - * we were supposed to send. - */ - buf = conn->tx_data_pull(conn, SIZE_MAX, &buf_len); - while (buf) { - destroy_and_callback(conn, buf, cb, ud); - buf = conn->tx_data_pull(conn, SIZE_MAX, &buf_len); - } - - goto exit; + goto raise_and_exit; } /* now that we are guaranteed resources, we can pull data from the upper @@ -1095,25 +1075,12 @@ void bt_conn_tx_processor(void) int err = send_buf(conn, buf, buf_len, cb, ud); if (err) { - /* -EIO means `unrecoverable error`. It can be an assertion that - * failed or an error from the HCI driver. - * - * -ENOMEM means we thought we had all the resources to send the - * buf (ie. TX context + controller buffer) but one of them was - * not available. This is likely due to a failure of - * assumption, likely that we have been pre-empted somehow and - * that `tx_processor()` has been re-entered. - * - * In both cases, we destroy the buffer and mark the connection - * as dead. - */ LOG_ERR("Fatal error (%d). Disconnecting %p", err, conn); - destroy_and_callback(conn, buf, cb, ud); bt_conn_disconnect(conn, BT_HCI_ERR_REMOTE_USER_TERM_CONN); - goto exit; } +raise_and_exit: /* Always kick the TX work. It will self-suspend if it doesn't get * resources or there is nothing left to send. */ diff --git a/subsys/bluetooth/host/conn_internal.h b/subsys/bluetooth/host/conn_internal.h index 41f396a1aee..909c0ebd22e 100644 --- a/subsys/bluetooth/host/conn_internal.h +++ b/subsys/bluetooth/host/conn_internal.h @@ -288,10 +288,22 @@ struct bt_conn { #endif /* Callback into the higher-layers (L2CAP / ISO) to return a buffer for - * sending `amount` of bytes to HCI. + * sending `amount` of bytes to HCI. Will only be called when + * the state is connected. The higher-layer is responsible for purging + * the remaining buffers on disconnect. * * Scheduling from which channel to pull (e.g. for L2CAP) is done at the * upper layer's discretion. + * + * Details about the returned net_buf when it is not NULL: + * - If the net_buf->len <= *length, then the net_buf has been removed + * from the tx_queue of the connection and the caller is now the + * owner of the only reference to the net_buf. + * - Otherwise, the net_buf is still on the tx_queue of the connection, + * and the callback has incremented the reference count to account + * for it having a reference still. + * - The caller must consume *length bytes from the net_buf before + * calling this function again. */ struct net_buf * (*tx_data_pull)(struct bt_conn *conn, size_t amount, diff --git a/subsys/bluetooth/host/iso.c b/subsys/bluetooth/host/iso.c index ffdf3ec0736..f3e9b684f4c 100644 --- a/subsys/bluetooth/host/iso.c +++ b/subsys/bluetooth/host/iso.c @@ -454,10 +454,18 @@ void bt_iso_connected(struct bt_conn *iso) static void bt_iso_chan_disconnected(struct bt_iso_chan *chan, uint8_t reason) { const uint8_t conn_type = chan->iso->iso.info.type; + struct net_buf *buf; + LOG_DBG("%p, reason 0x%02x", chan, reason); __ASSERT(chan->iso != NULL, "NULL conn for iso chan %p", chan); + /* release buffers from tx_queue */ + while ((buf = k_fifo_get(&chan->iso->iso.txq, K_NO_WAIT))) { + __ASSERT_NO_MSG(!bt_buf_has_view(buf)); + net_buf_unref(buf); + } + bt_iso_chan_set_state(chan, BT_ISO_STATE_DISCONNECTED); bt_conn_set_state(chan->iso, BT_CONN_DISCONNECT_COMPLETE); @@ -775,7 +783,8 @@ void bt_iso_recv(struct bt_conn *iso, struct net_buf *buf, uint8_t flags) static bool iso_has_data(struct bt_conn *conn) { #if defined(CONFIG_BT_ISO_TX) - return !k_fifo_is_empty(&conn->iso.txq); + return ((conn->iso.chan->state == BT_ISO_STATE_CONNECTED) && + !k_fifo_is_empty(&conn->iso.txq)); #else /* !CONFIG_BT_ISO_TX */ return false; #endif /* CONFIG_BT_ISO_TX */ @@ -789,9 +798,9 @@ static struct net_buf *iso_data_pull(struct bt_conn *conn, size_t amount, size_t /* Leave the PDU buffer in the queue until we have sent all its * fragments. */ - struct net_buf *frag = k_fifo_peek_head(&conn->iso.txq); + struct net_buf *q_frag = k_fifo_peek_head(&conn->iso.txq); - if (!frag) { + if (!q_frag) { BT_ISO_DATA_DBG("signaled ready but no frag available"); /* Service other connections */ bt_tx_irq_raise(); @@ -799,13 +808,10 @@ static struct net_buf *iso_data_pull(struct bt_conn *conn, size_t amount, size_t return NULL; } - if (conn->iso.chan->state != BT_ISO_STATE_CONNECTED) { - __maybe_unused struct net_buf *b = k_fifo_get(&conn->iso.txq, K_NO_WAIT); + __ASSERT_NO_MSG(conn->state == BT_CONN_CONNECTED); + if (conn->iso.chan->state != BT_ISO_STATE_CONNECTED) { LOG_DBG("channel has been disconnected"); - __ASSERT_NO_MSG(b == frag); - - net_buf_unref(b); /* Service other connections */ bt_tx_irq_raise(); @@ -813,7 +819,7 @@ static struct net_buf *iso_data_pull(struct bt_conn *conn, size_t amount, size_t return NULL; } - if (bt_buf_has_view(frag)) { + if (bt_buf_has_view(q_frag)) { /* This should not happen. conn.c should wait until the view is * destroyed before requesting more data. */ @@ -821,13 +827,16 @@ static struct net_buf *iso_data_pull(struct bt_conn *conn, size_t amount, size_t return NULL; } + struct net_buf *frag = net_buf_ref(q_frag); bool last_frag = amount >= frag->len; if (last_frag) { - __maybe_unused struct net_buf *b = k_fifo_get(&conn->iso.txq, K_NO_WAIT); + q_frag = k_fifo_get(&conn->iso.txq, K_NO_WAIT); BT_ISO_DATA_DBG("last frag, pop buf"); - __ASSERT_NO_MSG(b == frag); + __ASSERT_NO_MSG(q_frag == frag); + + net_buf_unref(q_frag); } *length = frag->len; diff --git a/subsys/bluetooth/host/l2cap.c b/subsys/bluetooth/host/l2cap.c index 433eb8faef0..ac6fc60dbad 100644 --- a/subsys/bluetooth/host/l2cap.c +++ b/subsys/bluetooth/host/l2cap.c @@ -83,11 +83,6 @@ NET_BUF_POOL_FIXED_DEFINE(disc_pool, 1, #define l2cap_remove_ident(conn, ident) __l2cap_lookup_ident(conn, ident, true) static sys_slist_t servers = SYS_SLIST_STATIC_INIT(&servers); - -static void l2cap_tx_buf_destroy(struct bt_conn *conn, struct net_buf *buf, int err) -{ - net_buf_unref(buf); -} #endif /* CONFIG_BT_L2CAP_DYNAMIC_CHANNEL */ /* L2CAP signalling channel specific context */ @@ -257,6 +252,7 @@ void bt_l2cap_chan_del(struct bt_l2cap_chan *chan) { const struct bt_l2cap_chan_ops *ops = chan->ops; struct bt_l2cap_le_chan *le_chan = BT_L2CAP_LE_CHAN(chan); + struct net_buf *buf; LOG_DBG("conn %p chan %p", chan->conn, chan); @@ -269,9 +265,7 @@ void bt_l2cap_chan_del(struct bt_l2cap_chan *chan) /* Remove buffers on the PDU TX queue. We can't do that in * `l2cap_chan_destroy()` as it is not called for fixed channels. */ - while (chan_has_data(le_chan)) { - struct net_buf *buf = k_fifo_get(&le_chan->tx_queue, K_NO_WAIT); - + while ((buf = k_fifo_get(&le_chan->tx_queue, K_NO_WAIT))) { net_buf_unref(buf); } @@ -919,20 +913,22 @@ struct net_buf *l2cap_data_pull(struct bt_conn *conn, * For SDUs we do the same, we keep it in the queue until all the * segments have been sent, adding the PDU headers just-in-time. */ - struct net_buf *pdu = k_fifo_peek_head(&lechan->tx_queue); + struct net_buf *fifo_pdu = k_fifo_peek_head(&lechan->tx_queue); /* We don't have anything to send for the current channel. We could * however have something to send on another channel that is attached to * the same ACL connection. Re-trigger the TX processor: it will call us * again and this time we will select another channel to pull data from. */ - if (!pdu) { + if (!fifo_pdu) { bt_tx_irq_raise(); return NULL; } - if (bt_buf_has_view(pdu)) { - LOG_ERR("already have view on %p", pdu); + __ASSERT_NO_MSG(conn->state == BT_CONN_CONNECTED); + + if (bt_buf_has_view(fifo_pdu)) { + LOG_ERR("already have view on %p", fifo_pdu); return NULL; } @@ -946,6 +942,8 @@ struct net_buf *l2cap_data_pull(struct bt_conn *conn, return NULL; } + struct net_buf *pdu = net_buf_ref(fifo_pdu); + /* Add PDU header */ if (lechan->_pdu_remaining == 0) { struct bt_l2cap_hdr *hdr; @@ -975,9 +973,11 @@ struct net_buf *l2cap_data_pull(struct bt_conn *conn, if (last_frag && last_seg) { LOG_DBG("last frag of last seg, dequeuing %p", pdu); - __maybe_unused struct net_buf *b = k_fifo_get(&lechan->tx_queue, K_NO_WAIT); + fifo_pdu = k_fifo_get(&lechan->tx_queue, K_NO_WAIT); - __ASSERT_NO_MSG(b == pdu); + __ASSERT_NO_MSG(fifo_pdu == pdu); + + net_buf_unref(fifo_pdu); } if (last_frag && L2CAP_LE_CID_IS_DYN(lechan->tx.cid)) { @@ -2290,7 +2290,7 @@ static void l2cap_chan_shutdown(struct bt_l2cap_chan *chan) /* Remove buffers on the TX queue */ while ((buf = k_fifo_get(&le_chan->tx_queue, K_NO_WAIT))) { - l2cap_tx_buf_destroy(chan->conn, buf, -ESHUTDOWN); + net_buf_unref(buf); } /* Remove buffers on the RX queue */ From 2aabbe60c70ab4f3f1a65bc3e9e7764322db2df0 Mon Sep 17 00:00:00 2001 From: Marek Pieta Date: Fri, 14 Jun 2024 12:08:55 +0200 Subject: [PATCH 4/6] [nrf noup] bluetooth: conn: Allow for an extra ref in bt_l2cap_send_pdu Allow for an additional buffer reference if callback is provided. This can be used to extend lifetime of the net buffer until the data transmission is confirmed by ACK of the remote. Signed-off-by: Marek Pieta (cherry picked from commit d6539f49a567b8539781356ecd03c749702b28ba) (cherry picked from commit 685c53acebd503a6931b8c2811d8a1e74e0e4828) --- subsys/bluetooth/host/conn.c | 17 ++++++++++++----- subsys/bluetooth/host/l2cap.c | 8 ++++++-- 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/subsys/bluetooth/host/conn.c b/subsys/bluetooth/host/conn.c index 3ebc2f9ca3c..74a14648537 100644 --- a/subsys/bluetooth/host/conn.c +++ b/subsys/bluetooth/host/conn.c @@ -702,12 +702,19 @@ static int send_buf(struct bt_conn *conn, struct net_buf *buf, uint16_t frag_len = MIN(conn_mtu(conn), len); - /* Check that buf->ref is 1 or 2. It would be 1 if this - * was the only reference (e.g. buf was removed - * from the conn tx_queue). It would be 2 if the - * tx_data_pull kept it on the tx_queue for segmentation. + /* Check that buf->ref is 1 or 2. It would be 1 if this was + * the only reference (e.g. buf was removed from the conn + * tx_queue). It would be 2 if the tx_data_pull kept it on + * the tx_queue for segmentation. + * + * Allow for an additional buffer reference if callback is + * provided. This can be used to extend lifetime of the net + * buffer until the data transmission is confirmed by ACK of + * the remote. */ - __ASSERT_NO_MSG((buf->ref == 1) || (buf->ref == 2)); + if (buf->ref > 2 + (cb ? 1 : 0)) { + __ASSERT_NO_MSG(false); + } /* The reference is always transferred to the frag, so when * the frag is destroyed, the parent reference is decremented. diff --git a/subsys/bluetooth/host/l2cap.c b/subsys/bluetooth/host/l2cap.c index ac6fc60dbad..9fa5638d961 100644 --- a/subsys/bluetooth/host/l2cap.c +++ b/subsys/bluetooth/host/l2cap.c @@ -729,13 +729,17 @@ int bt_l2cap_send_pdu(struct bt_l2cap_le_chan *le_chan, struct net_buf *pdu, return -ENOTCONN; } - if (pdu->ref != 1) { + /* Allow for an additional buffer reference if callback is provided. This can be used to + * extend lifetime of the net buffer until the data transmission is confirmed by ACK of the + * remote. + */ + if (pdu->ref > 1 + (cb ? 1 : 0)) { /* The host may alter the buf contents when fragmenting. Higher * layers cannot expect the buf contents to stay intact. Extra * refs suggests a silent data corruption would occur if not for * this error. */ - LOG_ERR("Expecting 1 ref, got %d", pdu->ref); + LOG_ERR("Expecting up to %d refs, got %d", cb ? 2 : 1, pdu->ref); return -EINVAL; } From 3f5429916746cc270c43fe6a4c45b78d43fcf5f8 Mon Sep 17 00:00:00 2001 From: Marek Pieta Date: Wed, 7 Aug 2024 10:29:21 +0200 Subject: [PATCH 5/6] [nrf noup] bluetooth: conn: Skip buffer ref count check in send_buf If ATT sent callback is delayed until data transmission is done by BLE controller, the transmitted buffer may have an additional reference. The reference is used to extend lifetime of the net buffer until the data transmission is confirmed by ACK of the remote. send_buf function can be called multiple times, if buffer has to be fragmented over HCI. In that case, the callback is provided as an argument only for the last transmitted fragment. The `buf->ref == 1` check is skipped because it's impossible to properly validate number of references for the sent fragments if buffers may have the additional reference. Jira: NCSDK-28624 Signed-off-by: Marek Pieta (cherry picked from commit e589307758ca108bd1d3f12501418e0d1d892f5c) (cherry picked from commit 827eb444b1947eb198d4dad28f40ab38e5d3f1eb) --- subsys/bluetooth/host/conn.c | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/subsys/bluetooth/host/conn.c b/subsys/bluetooth/host/conn.c index 74a14648537..fb8d7b97e45 100644 --- a/subsys/bluetooth/host/conn.c +++ b/subsys/bluetooth/host/conn.c @@ -702,19 +702,27 @@ static int send_buf(struct bt_conn *conn, struct net_buf *buf, uint16_t frag_len = MIN(conn_mtu(conn), len); - /* Check that buf->ref is 1 or 2. It would be 1 if this was - * the only reference (e.g. buf was removed from the conn - * tx_queue). It would be 2 if the tx_data_pull kept it on - * the tx_queue for segmentation. + /* If ATT sent callback is delayed until data transmission + * is done by BLE controller, the transmitted buffer may + * have an additional reference. The reference is used to + * extend lifetime of the net buffer until the data + * transmission is confirmed by ACK of the remote. * - * Allow for an additional buffer reference if callback is - * provided. This can be used to extend lifetime of the net - * buffer until the data transmission is confirmed by ACK of - * the remote. + * send_buf function can be called multiple times, if buffer + * has to be fragmented over HCI. In that case, the callback + * is provided as an argument only for the last transmitted + * fragment. The `buf->ref == 1` (or 2) check is skipped + * because it's impossible to properly validate number of + * references for the sent fragments if buffers may have the + * additional reference. + * + * Otherwise, check that buf->ref is 1 or 2. It would be 1 + * if this was the only reference (e.g. buf was removed from + * the conn tx_queue). It would be 2 if the tx_data_pull + * kept it on the tx_queue for segmentation. */ - if (buf->ref > 2 + (cb ? 1 : 0)) { - __ASSERT_NO_MSG(false); - } + __ASSERT_NO_MSG(IS_ENABLED(CONFIG_BT_ATT_SENT_CB_AFTER_TX) || (buf->ref == 1) || + (buf->ref == 2)); /* The reference is always transferred to the frag, so when * the frag is destroyed, the parent reference is decremented. From d9f3c30032a900259701c468b8b4795a4a73f438 Mon Sep 17 00:00:00 2001 From: "Mike J. Chen" Date: Mon, 9 Jun 2025 16:10:44 -0700 Subject: [PATCH 6/6] [nrf fromtree] tests: bsim: add early disconnect tests for iso and l2cap Add early_disconnect test cases for iso/cis and l2cap/stress to test the bluetooth stack handling of disconnects while transmit is still in progress. Signed-off-by: Mike J. Chen (cherry picked from commit 93997dab8eaa4777351d6d00df49695a35d41c76) --- tests/bsim/bluetooth/host/iso/cis/prj.conf | 1 + .../bluetooth/host/iso/cis/src/cis_central.c | 18 +- .../host/iso/cis/src/cis_peripheral.c | 36 ++++ .../cis/tests_scripts/cis_early_disconnect.sh | 29 +++ .../stress/overlay-early-disconnect.conf | 1 + .../bluetooth/host/l2cap/stress/src/main.c | 203 +++++++++++++++++- .../bluetooth/host/l2cap/stress/testcase.yaml | 4 + .../tests_scripts/l2cap_early_disconnect.sh | 47 ++++ 8 files changed, 327 insertions(+), 12 deletions(-) create mode 100755 tests/bsim/bluetooth/host/iso/cis/tests_scripts/cis_early_disconnect.sh create mode 100644 tests/bsim/bluetooth/host/l2cap/stress/overlay-early-disconnect.conf create mode 100755 tests/bsim/bluetooth/host/l2cap/stress/tests_scripts/l2cap_early_disconnect.sh diff --git a/tests/bsim/bluetooth/host/iso/cis/prj.conf b/tests/bsim/bluetooth/host/iso/cis/prj.conf index 4ad0a0fd319..06f02823296 100644 --- a/tests/bsim/bluetooth/host/iso/cis/prj.conf +++ b/tests/bsim/bluetooth/host/iso/cis/prj.conf @@ -16,6 +16,7 @@ CONFIG_BT_ISO_TX_MTU=200 CONFIG_BT_ISO_RX_MTU=200 CONFIG_BT_ISO_LOG_LEVEL_DBG=y +CONFIG_NET_BUF_POOL_USAGE=y # Controller Connected ISO configs CONFIG_BT_CTLR_CENTRAL_ISO=y diff --git a/tests/bsim/bluetooth/host/iso/cis/src/cis_central.c b/tests/bsim/bluetooth/host/iso/cis/src/cis_central.c index dc7d46100a9..e3b6ee9284f 100644 --- a/tests/bsim/bluetooth/host/iso/cis/src/cis_central.c +++ b/tests/bsim/bluetooth/host/iso/cis/src/cis_central.c @@ -154,6 +154,11 @@ static void iso_disconnected(struct bt_iso_chan *chan, uint8_t reason) err = bt_iso_remove_data_path(chan, BT_HCI_DATAPATH_DIR_HOST_TO_CTLR); TEST_ASSERT(err == 0, "Failed to remove ISO data path: %d", err); + + if (seq_num < 100) { + printk("Channel disconnected early, bumping seq_num to 1000 to end test\n"); + seq_num = 1000; + } } static void sdu_sent_cb(struct bt_iso_chan *chan) @@ -462,9 +467,16 @@ static void test_main(void) k_sleep(K_USEC(interval_us)); } - disconnect_cis(); - disconnect_acl(); - terminate_cig(); + if (seq_num == 100) { + disconnect_cis(); + disconnect_acl(); + terminate_cig(); + } + + /* check that all buffers returned to pool */ + TEST_ASSERT(atomic_get(&tx_pool.avail_count) == ENQUEUE_COUNT, + "tx_pool has non returned buffers, should be %u but is %u", + ENQUEUE_COUNT, atomic_get(&tx_pool.avail_count)); TEST_PASS("Test passed"); } diff --git a/tests/bsim/bluetooth/host/iso/cis/src/cis_peripheral.c b/tests/bsim/bluetooth/host/iso/cis/src/cis_peripheral.c index 96b8865c1f4..e8b3ff37a9d 100644 --- a/tests/bsim/bluetooth/host/iso/cis/src/cis_peripheral.c +++ b/tests/bsim/bluetooth/host/iso/cis/src/cis_peripheral.c @@ -35,6 +35,9 @@ static const struct bt_data ad[] = { }; static struct bt_iso_chan iso_chan; +static size_t disconnect_after_recv_cnt; +static size_t iso_recv_cnt; + /** Print data as d_0 d_1 d_2 ... d_(n-2) d_(n-1) d_(n) to show the 3 first and 3 last octets * * Examples: @@ -72,14 +75,26 @@ static void iso_print_data(uint8_t *data, size_t data_len) printk("\t %s\n", data_str); } +static void disconnect_device(struct bt_conn *conn, void *data) +{ + int err = bt_conn_disconnect(conn, BT_HCI_ERR_REMOTE_USER_TERM_CONN); + + TEST_ASSERT(!err, "Failed to initate disconnect (err %d)", err); +} + static void iso_recv(struct bt_iso_chan *chan, const struct bt_iso_recv_info *info, struct net_buf *buf) { + iso_recv_cnt++; if (info->flags & BT_ISO_FLAGS_VALID) { printk("Incoming data channel %p len %u\n", chan, buf->len); iso_print_data(buf->data, buf->len); SET_FLAG(flag_data_received); } + if (disconnect_after_recv_cnt && (iso_recv_cnt >= disconnect_after_recv_cnt)) { + printk("Disconnecting\n"); + bt_conn_foreach(BT_CONN_TYPE_LE, disconnect_device, NULL); + } } static void iso_connected(struct bt_iso_chan *chan) @@ -189,12 +204,33 @@ static void test_main(void) } } +static void test_main_early_disconnect(void) +{ + init(); + + disconnect_after_recv_cnt = 10; + + while (true) { + adv_connect(); + bt_testlib_conn_wait_free(); + + if (IS_FLAG_SET(flag_data_received)) { + TEST_PASS("Test passed"); + } + } +} + static const struct bst_test_instance test_def[] = { { .test_id = "peripheral", .test_descr = "Peripheral", .test_main_f = test_main, }, + { + .test_id = "peripheral_early_disconnect", + .test_descr = "Peripheral that tests early disconnect", + .test_main_f = test_main_early_disconnect, + }, BSTEST_END_MARKER, }; diff --git a/tests/bsim/bluetooth/host/iso/cis/tests_scripts/cis_early_disconnect.sh b/tests/bsim/bluetooth/host/iso/cis/tests_scripts/cis_early_disconnect.sh new file mode 100755 index 00000000000..bac2c2e82a1 --- /dev/null +++ b/tests/bsim/bluetooth/host/iso/cis/tests_scripts/cis_early_disconnect.sh @@ -0,0 +1,29 @@ +#!/usr/bin/env bash +# Copyright (c) 2025 Google LLC +# SPDX-License-Identifier: Apache-2.0 + +source ${ZEPHYR_BASE}/tests/bsim/sh_common.source + +# Tests cleanup within the ble stack for ISO connections when +# peripheral disconnects early while the central still has +# buffers queued for sending. Using the base code, which +# has the central sending 100 ISO packets, the peripheral +# triggers a disconnect after receiving 10 packets. +# The central checks that all tx_pool buffers have been +# returned to the pool after the disconnect. +simulation_id="iso_cis_early_disconnect" +verbosity_level=2 +EXECUTE_TIMEOUT=120 + +cd ${BSIM_OUT_PATH}/bin + +Execute ./bs_${BOARD_TS}_tests_bsim_bluetooth_host_iso_cis_prj_conf \ + -v=${verbosity_level} -s=${simulation_id} -d=0 -testid=central + +Execute ./bs_${BOARD_TS}_tests_bsim_bluetooth_host_iso_cis_prj_conf \ + -v=${verbosity_level} -s=${simulation_id} -d=1 -testid=peripheral_early_disconnect + +Execute ./bs_2G4_phy_v1 -v=${verbosity_level} -s=${simulation_id} \ + -D=2 -sim_length=30e6 $@ + +wait_for_background_jobs diff --git a/tests/bsim/bluetooth/host/l2cap/stress/overlay-early-disconnect.conf b/tests/bsim/bluetooth/host/l2cap/stress/overlay-early-disconnect.conf new file mode 100644 index 00000000000..94dfb1ed30d --- /dev/null +++ b/tests/bsim/bluetooth/host/l2cap/stress/overlay-early-disconnect.conf @@ -0,0 +1 @@ +CONFIG_BT_L2CAP_SEG_RECV=y diff --git a/tests/bsim/bluetooth/host/l2cap/stress/src/main.c b/tests/bsim/bluetooth/host/l2cap/stress/src/main.c index d05f4e4f0a7..a411354bb79 100644 --- a/tests/bsim/bluetooth/host/l2cap/stress/src/main.c +++ b/tests/bsim/bluetooth/host/l2cap/stress/src/main.c @@ -17,6 +17,7 @@ #include #include "babblekit/testcase.h" #include "babblekit/flags.h" +#include "bsim_args_runner.h" #define LOG_MODULE_NAME main #include @@ -31,6 +32,18 @@ DEFINE_FLAG_STATIC(flag_l2cap_connected); #define SDU_LEN 3000 #define RESCHEDULE_DELAY K_MSEC(100) +/* The early_disconnect test has the peripheral disconnect at various + * times: + * + * Peripheral 1: disconnects after all 20 SDUs as before + * Peripheral 2: disconnects immediately before receiving anything + * Peripheral 3: disconnects after receiving first SDU + * Peripheral 4: disconnects after receiving first PDU in second SDU + * Peripheral 5: disconnects after receiving third PDU in third SDU + * Peripheral 6: disconnects atfer receiving tenth PDU in tenth SDU + */ +static unsigned int device_nbr; + static void sdu_destroy(struct net_buf *buf) { LOG_DBG("%p", buf); @@ -56,7 +69,7 @@ NET_BUF_POOL_DEFINE(sdu_rx_pool, 8, rx_destroy); static uint8_t tx_data[SDU_LEN]; -static uint16_t rx_cnt; +static uint16_t sdu_rx_cnt; static uint8_t disconnect_counter; struct test_ctx { @@ -138,10 +151,66 @@ void sent_cb(struct bt_l2cap_chan *chan) continue_sending(ctx); } +#ifdef CONFIG_BT_L2CAP_SEG_RECV +static void disconnect_device_no_wait(struct bt_conn *conn, void *data) +{ + int err; + + err = bt_conn_disconnect(conn, BT_HCI_ERR_REMOTE_USER_TERM_CONN); + TEST_ASSERT(!err, "Failed to initate disconnect (err %d)", err); + + UNSET_FLAG(is_connected); +} + +static void seg_recv_cb(struct bt_l2cap_chan *chan, size_t sdu_len, off_t seg_offset, + struct net_buf_simple *seg) +{ + static size_t pdu_rx_cnt; + + if ((seg_offset + seg->len) == sdu_len) { + /* last segment/PDU of a SDU */ + LOG_DBG("len %d", seg->len); + sdu_rx_cnt++; + pdu_rx_cnt = 0; + } else { + LOG_DBG("SDU %u, pdu %u at seg_offset %u, len %u", + sdu_rx_cnt, pdu_rx_cnt, seg_offset, seg->len); + pdu_rx_cnt++; + } + + /* Verify SDU data matches TX'd data. */ + int pos = memcmp(seg->data, &tx_data[seg_offset], seg->len); + + if (pos != 0) { + LOG_ERR("RX data doesn't match TX: pos %d", seg_offset); + LOG_HEXDUMP_ERR(seg->data, seg->len, "RX data"); + LOG_HEXDUMP_INF(tx_data, seg->len, "TX data"); + + for (uint16_t p = 0; p < seg->len; p++) { + __ASSERT(seg->data[p] == tx_data[p + seg_offset], + "Failed rx[%d]=%x != expect[%d]=%x", + p, seg->data[p], p, tx_data[p + seg_offset]); + } + } + + if (((device_nbr == 4) && (sdu_rx_cnt >= 1) && (pdu_rx_cnt == 1)) || + ((device_nbr == 5) && (sdu_rx_cnt >= 2) && (pdu_rx_cnt == 3)) || + ((device_nbr == 6) && (sdu_rx_cnt >= 9) && (pdu_rx_cnt == 10))) { + LOG_INF("disconnecting after receiving PDU %u of SDU %u", + pdu_rx_cnt - 1, sdu_rx_cnt); + bt_conn_foreach(BT_CONN_TYPE_LE, disconnect_device_no_wait, NULL); + return; + } + + if (is_connected) { + bt_l2cap_chan_give_credits(chan, 1); + } +} +#else /* CONFIG_BT_L2CAP_SEG_RECV */ int recv_cb(struct bt_l2cap_chan *chan, struct net_buf *buf) { LOG_DBG("len %d", buf->len); - rx_cnt++; + sdu_rx_cnt++; /* Verify SDU data matches TX'd data. */ int pos = memcmp(buf->data, tx_data, buf->len); @@ -160,6 +229,7 @@ int recv_cb(struct bt_l2cap_chan *chan, struct net_buf *buf) return 0; } +#endif /* CONFIG_BT_L2CAP_SEG_RECV */ void l2cap_chan_connected_cb(struct bt_l2cap_chan *l2cap_chan) { @@ -167,25 +237,41 @@ void l2cap_chan_connected_cb(struct bt_l2cap_chan *l2cap_chan) CONTAINER_OF(l2cap_chan, struct bt_l2cap_le_chan, chan); SET_FLAG(flag_l2cap_connected); - LOG_DBG("%x (tx mtu %d mps %d) (tx mtu %d mps %d)", + LOG_DBG("%x (tx mtu %d mps %d cr %ld) (tx mtu %d mps %d cr %ld)", l2cap_chan, chan->tx.mtu, chan->tx.mps, + atomic_get(&chan->tx.credits), chan->rx.mtu, - chan->rx.mps); + chan->rx.mps, + atomic_get(&chan->rx.credits)); } -void l2cap_chan_disconnected_cb(struct bt_l2cap_chan *chan) +void l2cap_chan_disconnected_cb(struct bt_l2cap_chan *l2cap_chan) { UNSET_FLAG(flag_l2cap_connected); - LOG_DBG("%p", chan); + LOG_DBG("%p", l2cap_chan); + for (int i = 0; i < L2CAP_CHANS; i++) { + if (&contexts[i].le_chan == CONTAINER_OF(l2cap_chan, + struct bt_l2cap_le_chan, chan)) { + if (contexts[i].tx_left > 0) { + LOG_INF("setting tx_left to 0 because of disconnect"); + contexts[i].tx_left = 0; + } + break; + } + } } static struct bt_l2cap_chan_ops ops = { .connected = l2cap_chan_connected_cb, .disconnected = l2cap_chan_disconnected_cb, .alloc_buf = alloc_buf_cb, +#ifdef CONFIG_BT_L2CAP_SEG_RECV + .seg_recv = seg_recv_cb, +#else .recv = recv_cb, +#endif .sent = sent_cb, }; @@ -234,6 +320,10 @@ int server_accept_cb(struct bt_conn *conn, struct bt_l2cap_server *server, memset(le_chan, 0, sizeof(*le_chan)); le_chan->chan.ops = &ops; le_chan->rx.mtu = SDU_LEN; +#ifdef CONFIG_BT_L2CAP_SEG_RECV + le_chan->rx.mps = BT_L2CAP_RX_MTU; + le_chan->rx.credits = CONFIG_BT_BUF_ACL_RX_COUNT_EXTRA; +#endif *chan = &le_chan->chan; return 0; @@ -335,15 +425,93 @@ static void test_peripheral_main(void) LOG_DBG("Registered server PSM %x", psm); LOG_DBG("Peripheral waiting for transfer completion"); - while (rx_cnt < SDU_NUM) { + while (sdu_rx_cnt < SDU_NUM) { + k_msleep(100); + } + + bt_conn_foreach(BT_CONN_TYPE_LE, disconnect_device, NULL); + + WAIT_FOR_FLAG_UNSET(is_connected); + LOG_INF("Total received: %d", sdu_rx_cnt); + + /* check that all buffers returned to pool */ + TEST_ASSERT(atomic_get(&sdu_tx_pool.avail_count) == CONFIG_BT_MAX_CONN, + "sdu_tx_pool has non returned buffers, should be %u but is %u", + CONFIG_BT_MAX_CONN, atomic_get(&sdu_tx_pool.avail_count)); + TEST_ASSERT(atomic_get(&sdu_rx_pool.avail_count) == CONFIG_BT_MAX_CONN, + "sdu_rx_pool has non returned buffers, should be %u but is %u", + CONFIG_BT_MAX_CONN, atomic_get(&sdu_rx_pool.avail_count)); + + TEST_PASS("L2CAP STRESS Peripheral passed"); +} + +static void test_peripheral_early_disconnect_main(void) +{ + device_nbr = bsim_args_get_global_device_nbr(); + LOG_DBG("*L2CAP STRESS EARLY DISCONNECT Peripheral started*"); + int err; + + /* Prepare tx_data */ + for (size_t i = 0; i < sizeof(tx_data); i++) { + tx_data[i] = (uint8_t)i; + } + + err = bt_enable(NULL); + if (err) { + TEST_FAIL("Can't enable Bluetooth (err %d)", err); + return; + } + + LOG_DBG("Peripheral Bluetooth initialized."); + LOG_DBG("Connectable advertising..."); + err = bt_le_adv_start(BT_LE_ADV_CONN_FAST_1, NULL, 0, NULL, 0); + if (err) { + TEST_FAIL("Advertising failed to start (err %d)", err); + return; + } + + LOG_DBG("Advertising started."); + LOG_DBG("Peripheral waiting for connection..."); + WAIT_FOR_FLAG(is_connected); + LOG_DBG("Peripheral Connected."); + + int psm = l2cap_server_register(BT_SECURITY_L1); + + LOG_DBG("Registered server PSM %x", psm); + + if (device_nbr == 2) { + LOG_INF("disconnecting before receiving any SDU"); + k_msleep(1000); + goto disconnect; + } + + LOG_DBG("Peripheral waiting for transfer completion"); + while (sdu_rx_cnt < SDU_NUM) { + if ((device_nbr == 3) && (sdu_rx_cnt >= 1)) { + LOG_INF("disconnecting after receiving SDU %u", sdu_rx_cnt); + break; + } + k_msleep(100); + if (!is_connected) { + goto done; + } } +disconnect: bt_conn_foreach(BT_CONN_TYPE_LE, disconnect_device, NULL); +done: + WAIT_FOR_FLAG_UNSET(is_connected); - LOG_INF("Total received: %d", rx_cnt); + LOG_INF("Total received: %d", sdu_rx_cnt); - TEST_ASSERT(rx_cnt == SDU_NUM, "Did not receive expected no of SDUs"); + /* check that all buffers returned to pool */ + TEST_ASSERT(atomic_get(&sdu_tx_pool.avail_count) == CONFIG_BT_MAX_CONN, + "sdu_tx_pool has non returned buffers, should be %u but is %u", + CONFIG_BT_MAX_CONN, atomic_get(&sdu_tx_pool.avail_count)); + TEST_ASSERT(atomic_get(&sdu_rx_pool.avail_count) == CONFIG_BT_MAX_CONN, + "sdu_rx_pool has non returned buffers, should be %u but is %u", + CONFIG_BT_MAX_CONN, atomic_get(&sdu_rx_pool.avail_count)); TEST_PASS("L2CAP STRESS Peripheral passed"); } @@ -405,6 +573,10 @@ static void connect_l2cap_channel(struct bt_conn *conn, void *data) le_chan->chan.ops = &ops; le_chan->rx.mtu = SDU_LEN; +#ifdef CONFIG_BT_L2CAP_SEG_RECV + le_chan->rx.mps = BT_L2CAP_RX_MTU; + le_chan->rx.credits = CONFIG_BT_BUF_ACL_RX_COUNT_EXTRA; +#endif UNSET_FLAG(flag_l2cap_connected); @@ -461,6 +633,14 @@ static void test_central_main(void) } LOG_DBG("All peripherals disconnected."); + /* check that all buffers returned to pool */ + TEST_ASSERT(atomic_get(&sdu_tx_pool.avail_count) == CONFIG_BT_MAX_CONN, + "sdu_tx_pool has non returned buffers, should be %u but is %u", + CONFIG_BT_MAX_CONN, atomic_get(&sdu_tx_pool.avail_count)); + TEST_ASSERT(atomic_get(&sdu_rx_pool.avail_count) == CONFIG_BT_MAX_CONN, + "sdu_rx_pool has non returned buffers, should be %u but is %u", + CONFIG_BT_MAX_CONN, atomic_get(&sdu_rx_pool.avail_count)); + TEST_PASS("L2CAP STRESS Central passed"); } @@ -470,6 +650,11 @@ static const struct bst_test_instance test_def[] = { .test_descr = "Peripheral L2CAP STRESS", .test_main_f = test_peripheral_main }, + { + .test_id = "peripheral_early_disconnect", + .test_descr = "Peripheral L2CAP STRESS EARLY DISCONNECT", + .test_main_f = test_peripheral_early_disconnect_main, + }, { .test_id = "central", .test_descr = "Central L2CAP STRESS", diff --git a/tests/bsim/bluetooth/host/l2cap/stress/testcase.yaml b/tests/bsim/bluetooth/host/l2cap/stress/testcase.yaml index f872f5873da..8fbe85a3b64 100644 --- a/tests/bsim/bluetooth/host/l2cap/stress/testcase.yaml +++ b/tests/bsim/bluetooth/host/l2cap/stress/testcase.yaml @@ -10,6 +10,10 @@ tests: bluetooth.host.l2cap.stress: harness_config: bsim_exe_name: tests_bsim_bluetooth_host_l2cap_stress_prj_conf + bluetooth.host.l2cap.stress_early_disconnect: + harness_config: + bsim_exe_name: tests_bsim_bluetooth_host_l2cap_stress_prj_conf_overlay-early-disc_conf + extra_args: EXTRA_CONF_FILE="overlay-early-disconnect.conf" bluetooth.host.l2cap.stress_nofrag: harness_config: bsim_exe_name: tests_bsim_bluetooth_host_l2cap_stress_prj_conf_overlay-nofrag_conf diff --git a/tests/bsim/bluetooth/host/l2cap/stress/tests_scripts/l2cap_early_disconnect.sh b/tests/bsim/bluetooth/host/l2cap/stress/tests_scripts/l2cap_early_disconnect.sh new file mode 100755 index 00000000000..8741839e762 --- /dev/null +++ b/tests/bsim/bluetooth/host/l2cap/stress/tests_scripts/l2cap_early_disconnect.sh @@ -0,0 +1,47 @@ +#!/usr/bin/env bash +# Copyright (c) 2025 Google LLC +# SPDX-License-Identifier: Apache-2.0 + +source ${ZEPHYR_BASE}/tests/bsim/sh_common.source + +# Tests cleanup within the ble stack for L2CAP connections when +# peripheral disconnects early while the central still has +# buffers queued for sending. Using the base code, which +# has the central sending 20 SDUs to 6 different peripherals, +# the test has the peripherals disconnect as follows: +# +# Peripheral 1: disconnects after all 20 SDUs as before +# Peripheral 2: disconnects immediately before receiving anything +# Peripheral 3: disconnects after receiving first SDU +# Peripheral 4: disconnects after receiving first PDU in second SDU +# Peripheral 5: disconnects after receiving third PDU in third SDU +# Peripheral 6: disconnects atfer receiving tenth PDU in tenth SDU +# +# The central and peripherals check that all tx_pool and rx_pool +# buffers have been returned after the disconnect. +simulation_id="l2cap_stress_early_disconnect" +verbosity_level=2 +EXECUTE_TIMEOUT=240 + +bsim_exe=./bs_${BOARD_TS}_tests_bsim_bluetooth_host_l2cap_stress_prj_conf_overlay-early-disc_conf + +cd ${BSIM_OUT_PATH}/bin + +Execute "${bsim_exe}" -v=${verbosity_level} -s=${simulation_id} -d=0 -testid=central -rs=43 + +Execute "${bsim_exe}" -v=${verbosity_level} -s=${simulation_id} -d=1 \ + -testid=peripheral_early_disconnect -rs=42 +Execute "${bsim_exe}" -v=${verbosity_level} -s=${simulation_id} -d=2 \ + -testid=peripheral_early_disconnect -rs=10 +Execute "${bsim_exe}" -v=${verbosity_level} -s=${simulation_id} -d=3 \ + -testid=peripheral_early_disconnect -rs=23 +Execute "${bsim_exe}" -v=${verbosity_level} -s=${simulation_id} -d=4 \ + -testid=peripheral_early_disconnect -rs=7884 +Execute "${bsim_exe}" -v=${verbosity_level} -s=${simulation_id} -d=5 \ + -testid=peripheral_early_disconnect -rs=230 +Execute "${bsim_exe}" -v=${verbosity_level} -s=${simulation_id} -d=6 \ + -testid=peripheral_early_disconnect -rs=9 + +Execute ./bs_2G4_phy_v1 -v=${verbosity_level} -s=${simulation_id} -D=7 -sim_length=400e6 $@ + +wait_for_background_jobs