From aabd8ac95689dbd97dfb474f0f06ab87710f5bd7 Mon Sep 17 00:00:00 2001 From: Pavel Vasilyev Date: Tue, 29 Apr 2025 12:36:03 +0200 Subject: [PATCH 1/2] [nrf fromtree] bluetooth: host: Add a check for num of bt_conn_tx and ACL/ISO bufs After https://github.com/zephyrproject-rtos/zephyr/pull/72090, each packet to be sent (wether ACL or ISO data) has a corresponding `bt_conn_tx` object, regardless of whether a callback is used. This means that number of packets Host can send to Controller is limited by the smaller of two values: ACL/ISO packets Controller can receive, and the number of `bt_conn_tx` objects allocated by Host. A mismatch between these numbers may lead to inefficient resource usage on either Host or Controller side. If Host allocates fewer `bt_conn_tx` objects than the number of buffers available on Controller for a given data type, some Controller buffers may go unused. Conversely, if Host allocates more `bt_conn_tx` objects than Controller can consume, the excess objects remain unused. This commit adds a check and issues a warning if the number of `bt_conn_tx` objects is not aligned with the number of ACL/ISO buffers reported by Controller via the LE Read Buffer Size v1 or v2 command. Signed-off-by: Pavel Vasilyev (cherry picked from commit ddeeecd0b4fce18bf7d01362e4a569fc473d7688) --- subsys/bluetooth/host/hci_core.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/subsys/bluetooth/host/hci_core.c b/subsys/bluetooth/host/hci_core.c index 0c52fa437f5..8ef0adcb2da 100644 --- a/subsys/bluetooth/host/hci_core.c +++ b/subsys/bluetooth/host/hci_core.c @@ -92,6 +92,26 @@ BUILD_ASSERT(IS_ENABLED(CONFIG_ZTEST), "Missing DT chosen property for HCI"); #define BT_HCI_QUIRKS 0 #endif +/* These checks are added to warn if the number of ACL or ISO packets in Controller is not equal to + * the number of bt_conn_tx contexts allocated by Host. The inequality of these two values can lead + * to inefficient resources usage either on Host's or Controller's side. + */ +#define CHECK_NUM_OF_ACL_PKTS(_num) \ + do { \ + if (CONFIG_BT_BUF_ACL_TX_COUNT != (_num)) { \ + LOG_WRN("Num of Controller's ACL packets != ACL bt_conn_tx contexts" \ + " (%u != %u)", (_num), CONFIG_BT_BUF_ACL_TX_COUNT); \ + } \ + } while (0) + +#define CHECK_NUM_OF_ISO_PKTS(_num) \ + do { \ + if (CONFIG_BT_ISO_TX_BUF_COUNT != (_num)) { \ + LOG_WRN("Num of Controller's ISO packets != ISO bt_conn_tx contexts" \ + "(%u != %u)", (_num), CONFIG_BT_ISO_TX_BUF_COUNT); \ + } \ + } while (0) + void bt_tx_irq_raise(void); #define HCI_CMD_TIMEOUT K_SECONDS(10) @@ -3159,6 +3179,8 @@ static void le_read_buffer_size_complete(struct net_buf *buf) LOG_DBG("ACL LE buffers: pkts %u mtu %u", rp->le_max_num, bt_dev.le.acl_mtu); + CHECK_NUM_OF_ACL_PKTS(rp->le_max_num); + k_sem_init(&bt_dev.le.acl_pkts, rp->le_max_num, rp->le_max_num); #endif /* CONFIG_BT_CONN */ } @@ -3178,6 +3200,8 @@ static void read_buffer_size_v2_complete(struct net_buf *buf) LOG_DBG("ACL LE buffers: pkts %u mtu %u", rp->acl_max_num, bt_dev.le.acl_mtu); k_sem_init(&bt_dev.le.acl_pkts, rp->acl_max_num, rp->acl_max_num); + + CHECK_NUM_OF_ACL_PKTS(rp->acl_max_num); } #endif /* CONFIG_BT_CONN */ @@ -3194,6 +3218,8 @@ static void read_buffer_size_v2_complete(struct net_buf *buf) k_sem_init(&bt_dev.le.iso_pkts, rp->iso_max_num, rp->iso_max_num); bt_dev.le.iso_limit = rp->iso_max_num; + + CHECK_NUM_OF_ISO_PKTS(rp->iso_max_num); #endif /* CONFIG_BT_ISO */ } From 8b3ea7815e6896350938e61b359ed1be0987a896 Mon Sep 17 00:00:00 2001 From: Pavel Vasilyev Date: Tue, 29 Apr 2025 13:41:03 +0200 Subject: [PATCH 2/2] [nrf fromtree] bluetooth: host: Deprecated BT_CONN_TX_MAX After https://github.com/zephyrproject-rtos/zephyr/pull/72090, `conn_tx_alloc` no longer blocks, and each buffer always has a corresponding `bt_conn_tx` object. This eliminates the need to configure the number of `bt_conn_tx` objects via `CONFIG_BT_CONN_TX_MAX`, since every buffer now carries its own context even when no callback is used. This commit deprecates `CONFIG_BT_CONN_TX_MAX` as it is no longer necessary. Instead, `CONFIG_BT_BUF_ACL_TX_COUNT` is used to allocate `bt_conn_tx` objects for outgoing ACL data. ZLL already uses `CONFIG_BT_BUF_ACL_TX_COUNT` to configure the number of outgoing ACL packets. With this change, modifying the packet count will automatically adjust the number of corresponding contexts, preventing both context starvatoin and underutilization. This approach also aligns with ISO, where the number of `bt_conn_tx` objects for outgoing ISOdata matches `CONFIG_BT_ISO_TX_BUF_COUNT`. Signed-off-by: Pavel Vasilyev (cherry picked from commit 14b4e30cdf62b8dda157986e4b2299e673125c72) --- doc/releases/release-notes-4.2.rst | 4 ++++ include/zephyr/bluetooth/gatt.h | 4 ---- subsys/bluetooth/host/Kconfig | 2 +- subsys/bluetooth/host/classic/Kconfig | 4 ++-- subsys/bluetooth/host/conn.c | 2 +- tests/bluetooth/host/conn/prj.conf | 1 - tests/bsim/bluetooth/host/l2cap/stress/prj.conf | 2 +- tests/bsim/bluetooth/host/misc/acl_tx_frag/prj.conf | 4 ---- 8 files changed, 9 insertions(+), 14 deletions(-) diff --git a/doc/releases/release-notes-4.2.rst b/doc/releases/release-notes-4.2.rst index bbd2bfda50b..ba95e155cd5 100644 --- a/doc/releases/release-notes-4.2.rst +++ b/doc/releases/release-notes-4.2.rst @@ -103,6 +103,10 @@ Deprecated APIs and options deprecated, because support for anonymous authentication had been removed from the hawkBit server in version 0.8.0. +* The :kconfig:option:`CONFIG_BT_CONN_TX_MAX` Kconfig option has been deprecated. The number of + pending TX buffers is now aligned with the :kconfig:option:`CONFIG_BT_BUF_ACL_TX_COUNT` Kconfig + option. + New APIs and options ==================== diff --git a/include/zephyr/bluetooth/gatt.h b/include/zephyr/bluetooth/gatt.h index e149ea577c7..3597c7c5fdb 100644 --- a/include/zephyr/bluetooth/gatt.h +++ b/include/zephyr/bluetooth/gatt.h @@ -1383,8 +1383,6 @@ struct bt_gatt_notify_params { * The callback is run from System Workqueue context. * When called from the System Workqueue context this API will not wait for * resources for the callback but instead return an error. - * The number of pending callbacks can be increased with the - * @kconfig{CONFIG_BT_CONN_TX_MAX} option. * * Alternatively it is possible to notify by UUID by setting it on the * parameters, when using this method the attribute if provided is used as the @@ -2078,8 +2076,6 @@ int bt_gatt_write(struct bt_conn *conn, struct bt_gatt_write_params *params); * The callback is run from System Workqueue context. * When called from the System Workqueue context this API will not wait for * resources for the callback but instead return an error. - * The number of pending callbacks can be increased with the - * @kconfig{CONFIG_BT_CONN_TX_MAX} option. * * @param conn Connection object. * @param handle Attribute handle. diff --git a/subsys/bluetooth/host/Kconfig b/subsys/bluetooth/host/Kconfig index 58c65bd909d..5d583b9fc71 100644 --- a/subsys/bluetooth/host/Kconfig +++ b/subsys/bluetooth/host/Kconfig @@ -326,7 +326,7 @@ config BT_CONN_FRAG_COUNT if BT_CONN config BT_CONN_TX_MAX - int "Maximum number of pending TX buffers with a callback" + int "Maximum number of pending TX buffers with a callback [DEPRECATED]" default BT_BUF_ACL_TX_COUNT range BT_BUF_ACL_TX_COUNT $(UINT8_MAX) help diff --git a/subsys/bluetooth/host/classic/Kconfig b/subsys/bluetooth/host/classic/Kconfig index 1fbf45f9099..3c76cb7f590 100644 --- a/subsys/bluetooth/host/classic/Kconfig +++ b/subsys/bluetooth/host/classic/Kconfig @@ -140,8 +140,8 @@ config BT_RFCOMM_L2CAP_MTU config BT_RFCOMM_TX_MAX int "Maximum number of pending TX buffers for RFCOMM" - default BT_CONN_TX_MAX - range BT_CONN_TX_MAX $(UINT8_MAX) + default BT_BUF_ACL_TX_COUNT + range BT_BUF_ACL_TX_COUNT $(UINT8_MAX) help Maximum number of pending TX buffers that have an associated sending buf. Normally this can be left to the default value, which diff --git a/subsys/bluetooth/host/conn.c b/subsys/bluetooth/host/conn.c index 03c5e1073a0..cd1a15f5115 100644 --- a/subsys/bluetooth/host/conn.c +++ b/subsys/bluetooth/host/conn.c @@ -120,7 +120,7 @@ sys_slist_t bt_auth_info_cbs = SYS_SLIST_STATIC_INIT(&bt_auth_info_cbs); static sys_slist_t conn_cbs = SYS_SLIST_STATIC_INIT(&conn_cbs); -static struct bt_conn_tx conn_tx[CONFIG_BT_CONN_TX_MAX]; +static struct bt_conn_tx conn_tx[CONFIG_BT_BUF_ACL_TX_COUNT]; #if defined(CONFIG_BT_CLASSIC) static struct bt_conn sco_conns[CONFIG_BT_MAX_SCO_CONN]; diff --git a/tests/bluetooth/host/conn/prj.conf b/tests/bluetooth/host/conn/prj.conf index f63ccd0a939..eb8d5a0fdbf 100644 --- a/tests/bluetooth/host/conn/prj.conf +++ b/tests/bluetooth/host/conn/prj.conf @@ -12,7 +12,6 @@ CONFIG_BT_PER_ADV_SYNC=y CONFIG_BT_MAX_CONN=1 CONFIG_BT_L2CAP_TX_MTU=23 -CONFIG_BT_CONN_TX_MAX=3 CONFIG_BT_CONN_PARAM_UPDATE_TIMEOUT=5000 CONFIG_BT_L2CAP_TX_BUF_COUNT=3 CONFIG_BT_ID_MAX=1 diff --git a/tests/bsim/bluetooth/host/l2cap/stress/prj.conf b/tests/bsim/bluetooth/host/l2cap/stress/prj.conf index fecd8ba08d1..f984d840e3b 100644 --- a/tests/bsim/bluetooth/host/l2cap/stress/prj.conf +++ b/tests/bsim/bluetooth/host/l2cap/stress/prj.conf @@ -32,7 +32,7 @@ CONFIG_BT_BUF_ACL_TX_COUNT=4 # L2AP MPS + L2CAP header (4) CONFIG_BT_BUF_ACL_RX_SIZE=81 -# Governs BT_CONN_TX_MAX, and so must be >= than the max number of +# Governs CONFIG_BT_BUF_ACL_TX_COUNT, and so must be >= than the max number of # peers, since we attempt to send one SDU per peer. The test execution # is a bit slowed down by having this at the very minimum, but we want # to keep it that way as to stress the stack as much as possible. diff --git a/tests/bsim/bluetooth/host/misc/acl_tx_frag/prj.conf b/tests/bsim/bluetooth/host/misc/acl_tx_frag/prj.conf index 6bd02c8b645..a520656fcf4 100644 --- a/tests/bsim/bluetooth/host/misc/acl_tx_frag/prj.conf +++ b/tests/bsim/bluetooth/host/misc/acl_tx_frag/prj.conf @@ -26,10 +26,6 @@ CONFIG_BT_GAP_AUTO_UPDATE_CONN_PARAMS=n CONFIG_BT_MAX_CONN=1 -# We don't want to be constrained on the number of TX -# contexts, rather on the number of LL TX buffers. -CONFIG_BT_CONN_TX_MAX=10 - # Outgoing ATT buffers CONFIG_BT_ATT_TX_COUNT=1