Bluetooth: Host: Deprecate BT_BUF_ACL_RX_COUNT symbol

theob-pro · kartben · commit 66ff97e69bd1 · 2024-11-27T10:30:59.000+01:00
Because the number of ACL RX buffers must be at least the number of
maximum connections plus one, increasing `CONFIG_BT_MAX_CONN` could
inadvertently lead to a build failure if the number of ACL RX buffers is
not also increased. This dependency may not be obvious to users.

To address this issue, this commit deprecates the
`CONFIG_BT_BUF_RX_COUNT` Kconfig symbol and computes the value in
`buf.h` using the new `BT_BUF_RX_COUNT` define. Note that the default
value and the minimum range value have been changed to 0 to "disable"
the option.

Additionally, to allow users to increase the number of ACL RX buffers,
this commit introduces the new `CONFIG_BT_BUF_RX_COUNT_EXTRA` Kconfig
symbol. The value of this symbol will be added to the computed value of
`BT_BUF_RX_COUNT`.

The configurations of tests and samples have been updated to reflect
these changes.

Signed-off-by: Théo Battrel &lt;theo.battrel@nordicsemi.no&gt;
diff --git a/doc/releases/migration-guide-4.1.rst b/doc/releases/migration-guide-4.1.rst
@@ -153,6 +153,19 @@ Bluetooth Classic
 Bluetooth Host
 ==============
 
+* :kconfig:option:`CONFIG_BT_BUF_ACL_RX_COUNT` has been deprecated. The number of ACL RX buffers is
+  now computed internally and is equal to :kconfig:option:`CONFIG_BT_MAX_CONN` + 1. If an application
+  needs more buffers, it can use the new :kconfig:option:`CONFIG_BT_BUF_ACL_RX_COUNT_EXTRA` to add
+  additional ones.
+
+  e.g. if :kconfig:option:`CONFIG_BT_MAX_CONN` was ``3`` and
+  :kconfig:option:`CONFIG_BT_BUF_ACL_RX_COUNT` was ``7`` then
+  :kconfig:option:`CONFIG_BT_BUF_ACL_RX_COUNT_EXTRA` should be set to ``7 - (3 + 1) = 3``.
+
+  .. warning::
+
+   The default value of :kconfig:option:`CONFIG_BT_BUF_ACL_RX_COUNT` has been set to 0.
+
 Bluetooth Crypto
 ================
 
diff --git a/doc/releases/release-notes-4.1.rst b/doc/releases/release-notes-4.1.rst
@@ -77,6 +77,9 @@ Bluetooth
 
 * Host
 
+  * :kconfig:option:`CONFIG_BT_BUF_ACL_RX_COUNT` has been deprecated and
+    :kconfig:option:`CONFIG_BT_BUF_ACL_RX_COUNT_EXTRA` has been added.
+
 * HCI Drivers
 
 Boards & SoC Support
diff --git a/include/zephyr/bluetooth/buf.h b/include/zephyr/bluetooth/buf.h
@@ -85,13 +85,41 @@ struct bt_buf_data {
 #define BT_BUF_ISO_RX_COUNT 0
 #endif /* CONFIG_BT_ISO */
 
+/* see Core Spec v6.0 vol.4 part E 7.4.5 */
+#define BT_BUF_ACL_RX_COUNT_MAX 65535
+
+#if defined(CONFIG_BT_CONN) && defined(CONFIG_BT_HCI_HOST)
+ /* The host needs more ACL buffers than maximum ACL links. This is because of
+  * the way we re-assemble ACL packets into L2CAP PDUs.
+  *
+  * We keep around the first buffer (that comes from the driver) to do
+  * re-assembly into, and if all links are re-assembling, there will be no buffer
+  * available for the HCI driver to allocate from.
+  *
+  * TODO: When CONFIG_BT_BUF_ACL_RX_COUNT is removed,
+  *       remove the MAX and only keep (CONFIG_BT_MAX_CONN + 1)
+  */
+#define BT_BUF_ACL_RX_COUNT                                                                        \
+	(MAX(CONFIG_BT_BUF_ACL_RX_COUNT, (CONFIG_BT_MAX_CONN + 1)) +                               \
+	 CONFIG_BT_BUF_ACL_RX_COUNT_EXTRA)
+#else
+#define BT_BUF_ACL_RX_COUNT 0
+#endif /* CONFIG_BT_CONN && CONFIG_BT_HCI_HOST */
+
+#if defined(CONFIG_BT_BUF_ACL_RX_COUNT) && CONFIG_BT_BUF_ACL_RX_COUNT > 0
+#warning "CONFIG_BT_BUF_ACL_RX_COUNT is deprecated, see Zephyr 4.1 migration guide"
+#endif /* CONFIG_BT_BUF_ACL_RX_COUNT && CONFIG_BT_BUF_ACL_RX_COUNT > 0 */
+
+BUILD_ASSERT(BT_BUF_ACL_RX_COUNT <= BT_BUF_ACL_RX_COUNT_MAX,
+	     "Maximum number of ACL RX buffer is 65535, reduce CONFIG_BT_BUF_ACL_RX_COUNT_EXTRA");
+
 /** Data size needed for HCI ACL, HCI ISO or Event RX buffers */
 #define BT_BUF_RX_SIZE (MAX(MAX(BT_BUF_ACL_RX_SIZE, BT_BUF_EVT_RX_SIZE), \
 			    BT_BUF_ISO_RX_SIZE))
 
 /** Buffer count needed for HCI ACL, HCI ISO or Event RX buffers */
 #define BT_BUF_RX_COUNT (MAX(MAX(CONFIG_BT_BUF_EVT_RX_COUNT, \
-				 CONFIG_BT_BUF_ACL_RX_COUNT), \
+				 BT_BUF_ACL_RX_COUNT), \
 			     BT_BUF_ISO_RX_COUNT))
 
 /** Data size needed for HCI Command buffers. */
diff --git a/samples/bluetooth/central_hr/prj_minimal.conf b/samples/bluetooth/central_hr/prj_minimal.conf
@@ -94,7 +94,6 @@ CONFIG_BT_CTLR_PHY_2M=n
 # Reduce Bluetooth buffers
 CONFIG_BT_BUF_EVT_DISCARDABLE_COUNT=1
 CONFIG_BT_BUF_EVT_DISCARDABLE_SIZE=45
-CONFIG_BT_BUF_ACL_RX_COUNT=2
 CONFIG_BT_BUF_EVT_RX_COUNT=2
 
 CONFIG_BT_L2CAP_TX_BUF_COUNT=2
diff --git a/samples/bluetooth/central_multilink/prj.conf b/samples/bluetooth/central_multilink/prj.conf
@@ -4,7 +4,6 @@ CONFIG_BT_AUTO_PHY_UPDATE=n
 CONFIG_BT_PRIVACY=y
 
 CONFIG_BT_MAX_CONN=62
-CONFIG_BT_BUF_ACL_RX_COUNT=63
 
 # CONFIG_BT_GATT_CLIENT=y
 
diff --git a/samples/bluetooth/hci_uart_async/prj.conf b/samples/bluetooth/hci_uart_async/prj.conf
@@ -10,7 +10,6 @@ CONFIG_BT_HCI_RAW_H4_ENABLE=y
 
 # Controller configuration. Modify these for your application's needs.
 CONFIG_BT_MAX_CONN=16
-CONFIG_BT_BUF_ACL_RX_COUNT=17
 CONFIG_BT_BUF_ACL_RX_SIZE=255
 CONFIG_BT_BUF_CMD_TX_SIZE=255
 CONFIG_BT_BUF_EVT_DISCARDABLE_SIZE=255
diff --git a/samples/bluetooth/mesh/boards/bbc_microbit.conf b/samples/bluetooth/mesh/boards/bbc_microbit.conf
@@ -13,7 +13,7 @@ CONFIG_BT_PERIPHERAL=n
 CONFIG_BT_EXT_ADV=n
 CONFIG_BT_RX_STACK_SIZE=1100
 CONFIG_BT_BUF_EVT_RX_COUNT=3
-CONFIG_BT_BUF_ACL_RX_COUNT=3
+CONFIG_BT_BUF_ACL_RX_COUNT_EXTRA=1
 CONFIG_BT_BUF_EVT_DISCARDABLE_COUNT=3
 
 CONFIG_BT_CTLR_ADV_EXT=n
diff --git a/samples/bluetooth/peripheral_hr/prj_minimal.conf b/samples/bluetooth/peripheral_hr/prj_minimal.conf
@@ -100,7 +100,6 @@ CONFIG_BT_CTLR_PHY_2M=n
 # Reduce Bluetooth buffers
 CONFIG_BT_BUF_EVT_DISCARDABLE_COUNT=1
 CONFIG_BT_BUF_EVT_DISCARDABLE_SIZE=45
-CONFIG_BT_BUF_ACL_RX_COUNT=2
 CONFIG_BT_BUF_EVT_RX_COUNT=2
 
 CONFIG_BT_L2CAP_TX_BUF_COUNT=2
diff --git a/samples/bluetooth/peripheral_identity/prj.conf b/samples/bluetooth/peripheral_identity/prj.conf
@@ -8,7 +8,6 @@ CONFIG_BT_GAP_AUTO_UPDATE_CONN_PARAMS=n
 
 CONFIG_BT_MAX_CONN=62
 CONFIG_BT_ID_MAX=62
-CONFIG_BT_BUF_ACL_RX_COUNT=63
 
 # CONFIG_BT_SMP=y
 # CONFIG_BT_MAX_PAIRED=62
diff --git a/subsys/bluetooth/common/Kconfig b/subsys/bluetooth/common/Kconfig
@@ -77,10 +77,21 @@ config BT_BUF_ACL_RX_SIZE
 	  In a Controller only build this will determine the maximum ACL size
 	  that the Controller will send to the Host.
 
+config BT_BUF_ACL_RX_COUNT_EXTRA
+	int "Number of extra incoming ACL data buffers"
+	default 0
+	range 0 65535
+	help
+	  Number of incoming extra ACL data buffers sent from the Controller to
+	  the Host.
+
+	  By default, the number of incoming ACL data buffers is equal to
+	  CONFIG_BT_MAX_CONN + 1.
+
 config BT_BUF_ACL_RX_COUNT
-	int "Number of incoming ACL data buffers"
-	default 6
-	range 2 256
+	int "[DEPRECATED] Number of incoming ACL data buffers"
+	default 0
+	range 0 256
 	help
 	  Number or incoming ACL data buffers sent from the Controller to the
 	  Host.
diff --git a/subsys/bluetooth/common/dummy.c b/subsys/bluetooth/common/dummy.c
@@ -60,27 +60,3 @@ BUILD_ASSERT(!IS_ENABLED(CONFIG_LOG_MODE_IMMEDIATE), "Immediate logging "
 	     "on selected backend(s) not "
 	     "supported with the software Link Layer");
 #endif
-
-#if defined(CONFIG_BT_CONN) && defined(CONFIG_BT_HCI_HOST)
-/* The host needs more ACL buffers than maximum ACL links. This is because of
- * the way we re-assemble ACL packets into L2CAP PDUs.
- *
- * We keep around the first buffer (that comes from the driver) to do
- * re-assembly into, and if all links are re-assembling, there will be no buffer
- * available for the HCI driver to allocate from.
- *
- * Fixing it properly involves a re-design of the HCI driver interface.
- */
-#if defined(CONFIG_BT_HCI_ACL_FLOW_CONTROL)
-BUILD_ASSERT(CONFIG_BT_BUF_ACL_RX_COUNT > CONFIG_BT_MAX_CONN);
-
-#else /* !CONFIG_BT_HCI_ACL_FLOW_CONTROL */
-
-/* BT_BUF_RX_COUNT is defined in include/zephyr/bluetooth/buf.h */
-BUILD_ASSERT(BT_BUF_RX_COUNT > CONFIG_BT_MAX_CONN,
-	     "BT_BUF_RX_COUNT needs to be greater than CONFIG_BT_MAX_CONN. "
-	     "In order to do that, increase CONFIG_BT_BUF_ACL_RX_COUNT.");
-
-#endif /* CONFIG_BT_HCI_ACL_FLOW_CONTROL */
-
-#endif /* CONFIG_BT_CONN */
diff --git a/subsys/bluetooth/host/buf.c b/subsys/bluetooth/host/buf.c
@@ -40,7 +40,7 @@ NET_BUF_POOL_FIXED_DEFINE(discardable_pool, CONFIG_BT_BUF_EVT_DISCARDABLE_COUNT,
 			  sizeof(struct bt_buf_data), NULL);
 
 #if defined(CONFIG_BT_HCI_ACL_FLOW_CONTROL)
-NET_BUF_POOL_DEFINE(acl_in_pool, CONFIG_BT_BUF_ACL_RX_COUNT,
+NET_BUF_POOL_DEFINE(acl_in_pool, BT_BUF_ACL_RX_COUNT,
 		    BT_BUF_ACL_SIZE(CONFIG_BT_BUF_ACL_RX_SIZE),
 		    sizeof(struct acl_data), bt_hci_host_num_completed_packets);
 
diff --git a/subsys/bluetooth/host/classic/rfcomm.c b/subsys/bluetooth/host/classic/rfcomm.c
@@ -36,7 +36,7 @@ LOG_MODULE_REGISTER(bt_rfcomm);
 #define RFCOMM_MIN_MTU		BT_RFCOMM_SIG_MIN_MTU
 #define RFCOMM_DEFAULT_MTU	127
 
-#define RFCOMM_MAX_CREDITS		(CONFIG_BT_BUF_ACL_RX_COUNT - 1)
+#define RFCOMM_MAX_CREDITS		(BT_BUF_ACL_RX_COUNT - 1)
 #define RFCOMM_CREDITS_THRESHOLD	(RFCOMM_MAX_CREDITS / 2)
 #define RFCOMM_DEFAULT_CREDIT		RFCOMM_MAX_CREDITS
 
diff --git a/subsys/bluetooth/host/hci_core.c b/subsys/bluetooth/host/hci_core.c
@@ -1998,7 +1998,7 @@ static int set_flow_control(void)
 	hbs = net_buf_add(buf, sizeof(*hbs));
 	(void)memset(hbs, 0, sizeof(*hbs));
 	hbs->acl_mtu = sys_cpu_to_le16(CONFIG_BT_BUF_ACL_RX_SIZE);
-	hbs->acl_pkts = sys_cpu_to_le16(CONFIG_BT_BUF_ACL_RX_COUNT);
+	hbs->acl_pkts = sys_cpu_to_le16(BT_BUF_ACL_RX_COUNT);
 
 	err = bt_hci_cmd_send_sync(BT_HCI_OP_HOST_BUFFER_SIZE, buf, NULL);
 	if (err) {
diff --git a/subsys/bluetooth/host/l2cap.c b/subsys/bluetooth/host/l2cap.c
@@ -41,7 +41,7 @@ LOG_MODULE_REGISTER(bt_l2cap, CONFIG_BT_L2CAP_LOG_LEVEL);
 
 #define L2CAP_LE_MIN_MTU		23
 
-#define L2CAP_LE_MAX_CREDITS		(CONFIG_BT_BUF_ACL_RX_COUNT - 1)
+#define L2CAP_LE_MAX_CREDITS		(BT_BUF_ACL_RX_COUNT - 1)
 
 #define L2CAP_LE_CID_DYN_START	0x0040
 #define L2CAP_LE_CID_DYN_END	0x007f
diff --git a/tests/bluetooth/tester/boards/frdm_rw612.conf b/tests/bluetooth/tester/boards/frdm_rw612.conf
@@ -1,5 +1,4 @@
 CONFIG_BT_MAX_CONN=16
-CONFIG_BT_BUF_ACL_RX_COUNT=17
 
 # debug options
 # CONFIG_UART_CONSOLE=y
diff --git a/tests/bluetooth/tester/boards/rd_rw612_bga.conf b/tests/bluetooth/tester/boards/rd_rw612_bga.conf
@@ -1,5 +1,4 @@
 CONFIG_BT_MAX_CONN=16
-CONFIG_BT_BUF_ACL_RX_COUNT=17
 
 # debug options
 # CONFIG_UART_CONSOLE=y
diff --git a/tests/bsim/bluetooth/host/l2cap/einprogress/src/dut.c b/tests/bsim/bluetooth/host/l2cap/einprogress/src/dut.c
@@ -29,7 +29,7 @@ LOG_MODULE_REGISTER(dut, LOG_LEVEL_INF);
  *  application. This allows us to notice if the stack has freed
  *  references that were ours.
  */
-static atomic_t acl_pool_refs_held[CONFIG_BT_BUF_ACL_RX_COUNT];
+static atomic_t acl_pool_refs_held[BT_BUF_ACL_RX_COUNT];
 
 BUILD_ASSERT(IS_ENABLED(CONFIG_BT_TESTING));
 BUILD_ASSERT(IS_ENABLED(CONFIG_BT_HCI_ACL_FLOW_CONTROL));
@@ -46,7 +46,7 @@ static void acl_pool_refs_held_add(struct net_buf *buf)
 {
 	int buf_id = net_buf_id(buf);
 
-	__ASSERT_NO_MSG(0 <= buf_id && buf_id < CONFIG_BT_BUF_ACL_RX_COUNT);
+	__ASSERT_NO_MSG(0 <= buf_id && buf_id < BT_BUF_ACL_RX_COUNT);
 	atomic_inc(&acl_pool_refs_held[buf_id]);
 }
 
diff --git a/tests/bsim/bluetooth/host/l2cap/reassembly/dut/prj.conf b/tests/bsim/bluetooth/host/l2cap/reassembly/dut/prj.conf
@@ -28,5 +28,5 @@ CONFIG_BT_GAP_AUTO_UPDATE_CONN_PARAMS=n
 # RX buffer pool, it is a good idea to constrain said buffer
 # pool.
 CONFIG_BT_MAX_CONN=1
-CONFIG_BT_BUF_ACL_RX_COUNT=6
+CONFIG_BT_BUF_ACL_RX_COUNT_EXTRA=4
 CONFIG_BT_BUF_EVT_RX_COUNT=6
diff --git a/tests/bsim/bluetooth/host/l2cap/stress/prj.conf b/tests/bsim/bluetooth/host/l2cap/stress/prj.conf
@@ -42,7 +42,6 @@ CONFIG_BT_CTLR_DATA_LENGTH_MAX=27
 CONFIG_BT_CTLR_RX_BUFFERS=10
 
 CONFIG_BT_MAX_CONN=10
-CONFIG_BT_BUF_ACL_RX_COUNT=11
 
 CONFIG_LOG=y
 CONFIG_ASSERT=y
diff --git a/tests/bsim/bluetooth/host/l2cap/stress/prj_nofrag.conf b/tests/bsim/bluetooth/host/l2cap/stress/prj_nofrag.conf
@@ -34,7 +34,6 @@ CONFIG_BT_CTLR_DATA_LENGTH_MAX=81
 CONFIG_BT_CTLR_RX_BUFFERS=10
 
 CONFIG_BT_MAX_CONN=10
-CONFIG_BT_BUF_ACL_RX_COUNT=11
 
 CONFIG_LOG=y
 CONFIG_ASSERT=y
diff --git a/tests/bsim/bluetooth/host/l2cap/stress/prj_syswq.conf b/tests/bsim/bluetooth/host/l2cap/stress/prj_syswq.conf
@@ -42,7 +42,6 @@ CONFIG_BT_CTLR_DATA_LENGTH_MAX=27
 CONFIG_BT_CTLR_RX_BUFFERS=10
 
 CONFIG_BT_MAX_CONN=10
-CONFIG_BT_BUF_ACL_RX_COUNT=11
 
 CONFIG_LOG=y
 CONFIG_ASSERT=y
diff --git a/tests/bsim/bluetooth/host/misc/conn_stress/central/prj.conf b/tests/bsim/bluetooth/host/misc/conn_stress/central/prj.conf
@@ -2,7 +2,6 @@ CONFIG_BT=y
 CONFIG_LOG=y
 CONFIG_BT_CENTRAL=y
 CONFIG_BT_MAX_CONN=12
-CONFIG_BT_BUF_ACL_RX_COUNT=13
 CONFIG_BT_MAX_PAIRED=12
 CONFIG_BT_SMP=y
 CONFIG_BT_PRIVACY=y
diff --git a/tests/bsim/bluetooth/host/misc/hfc_multilink/dut/prj.conf b/tests/bsim/bluetooth/host/misc/hfc_multilink/dut/prj.conf
@@ -27,9 +27,5 @@ CONFIG_BT_AUTO_DATA_LEN_UPDATE=n
 CONFIG_BT_GAP_AUTO_UPDATE_CONN_PARAMS=n
 
 CONFIG_BT_MAX_CONN=3
-CONFIG_BT_BUF_ACL_RX_COUNT=4
-
-# This test will fail when CONFIG_BT_MAX_CONN == CONFIG_BT_BUF_ACL_RX_COUNT
-# CONFIG_BT_BUF_ACL_RX_COUNT=3
 
 CONFIG_BT_HCI_ACL_FLOW_CONTROL=y
diff --git a/tests/bsim/bluetooth/host/misc/hfc_multilink/dut/src/dut.c b/tests/bsim/bluetooth/host/misc/hfc_multilink/dut/src/dut.c
@@ -24,7 +24,7 @@ LOG_MODULE_REGISTER(dut, CONFIG_APP_LOG_LEVEL);
 #define NUM_TESTERS CONFIG_BT_MAX_CONN
 
 /* Build with the minimum possible amount of RX buffers */
-BUILD_ASSERT(CONFIG_BT_BUF_ACL_RX_COUNT == (CONFIG_BT_MAX_CONN + 1));
+BUILD_ASSERT(BT_BUF_ACL_RX_COUNT >= (CONFIG_BT_MAX_CONN + 1));
 
 struct tester {
 	size_t sdu_count;
diff --git a/tests/bsim/bluetooth/ll/multiple_id/prj.conf b/tests/bsim/bluetooth/ll/multiple_id/prj.conf
@@ -12,7 +12,6 @@ CONFIG_BT_AUTO_DATA_LEN_UPDATE=y
 
 CONFIG_BT_MAX_CONN=250
 CONFIG_BT_ID_MAX=250
-CONFIG_BT_BUF_ACL_RX_COUNT=251
 
 # L2CAP, ATT and SMP usage cause data transmission deadlock due to shortage
 # of buffers when transactions crossover amongst the connections in the same
