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 6d8939eca36..fb8d7b97e45 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,28 +702,32 @@ 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. + /* 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` (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. * - * 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. + * 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. */ - __ASSERT_NO_MSG(IS_ENABLED(CONFIG_BT_ATT_SENT_CB_AFTER_TX) || (buf->ref == 1)); + __ASSERT_NO_MSG(IS_ENABLED(CONFIG_BT_ATT_SENT_CB_AFTER_TX) || (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); @@ -733,7 +741,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. @@ -776,15 +784,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 = @@ -966,8 +985,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); } @@ -995,30 +1014,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) @@ -1061,17 +1056,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 @@ -1105,25 +1090,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 683aafdf561..9fa5638d961 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); } @@ -923,20 +917,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; } @@ -950,6 +946,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; @@ -979,9 +977,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)) { @@ -2294,7 +2294,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 */ 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