Bluetooth: scan: Implement non-blocking ext-adv report reassembly

drivers/bluetooth/hci/h4.c

@@ -205,8 +205,12 @@ static struct net_buf *get_rx(struct h4_data *h4, k_timeout_t timeout)
 	LOG_DBG("type 0x%02x, evt 0x%02x", h4->rx.type, h4->rx.evt.evt);
 
 	switch (h4->rx.type) {
-	case BT_HCI_H4_EVT:
-		return bt_buf_get_evt(h4->rx.evt.evt, h4->rx.discardable, timeout);
+	case BT_HCI_H4_EVT: {
+		uint8_t evt = h4->rx.evt.evt;
+		uint8_t meta = h4->rx.hdr[3];
+
+		return bt_buf_get_evt_but_better(evt, meta, timeout);
+	}
 	case BT_HCI_H4_ACL:
 		return bt_buf_get_rx(BT_BUF_ACL_IN, timeout);
 	case BT_HCI_H4_ISO:

drivers/bluetooth/hci/ipc.c

@@ -30,45 +30,8 @@ struct ipc_data {
 	const struct device *ipc;
 };
 
-static bool is_hci_event_discardable(const uint8_t *evt_data)
-{
-	uint8_t evt_type = evt_data[0];
-
-	switch (evt_type) {
-#if defined(CONFIG_BT_CLASSIC)
-	case BT_HCI_EVT_INQUIRY_RESULT_WITH_RSSI:
-	case BT_HCI_EVT_EXTENDED_INQUIRY_RESULT:
-		return true;
-#endif
-	case BT_HCI_EVT_LE_META_EVENT: {
-		uint8_t subevt_type = evt_data[sizeof(struct bt_hci_evt_hdr)];
-
-		switch (subevt_type) {
-		case BT_HCI_EVT_LE_ADVERTISING_REPORT:
-			return true;
-#if defined(CONFIG_BT_EXT_ADV)
-		case BT_HCI_EVT_LE_EXT_ADVERTISING_REPORT:
-		{
-			const struct bt_hci_evt_le_ext_advertising_report *ext_adv =
-				(void *)&evt_data[3];
-
-			return (ext_adv->num_reports == 1) &&
-				   ((ext_adv->adv_info[0].evt_type &
-					 BT_HCI_LE_ADV_EVT_TYPE_LEGACY) != 0);
-		}
-#endif
-		default:
-			return false;
-		}
-	}
-	default:
-		return false;
-	}
-}
-
 static struct net_buf *bt_ipc_evt_recv(const uint8_t *data, size_t remaining)
 {
-	bool discardable;
 	struct bt_hci_evt_hdr hdr;
 	struct net_buf *buf;
 	size_t buf_tailroom;
@@ -78,8 +41,6 @@ static struct net_buf *bt_ipc_evt_recv(const uint8_t *data, size_t remaining)
 		return NULL;
 	}
 
-	discardable = is_hci_event_discardable(data);
-
 	memcpy((void *)&hdr, data, sizeof(hdr));
 	data += sizeof(hdr);
 	remaining -= sizeof(hdr);
@@ -88,18 +49,14 @@ static struct net_buf *bt_ipc_evt_recv(const uint8_t *data, size_t remaining)
 		LOG_ERR("Event payload length is not correct");
 		return NULL;
 	}
-	LOG_DBG("len %u", hdr.len);
-
-	do {
-		buf = bt_buf_get_evt(hdr.evt, discardable, discardable ? K_NO_WAIT : K_SECONDS(10));
-		if (!buf) {
-			if (discardable) {
-				LOG_DBG("Discardable buffer pool full, ignoring event");
-				return buf;
-			}
-			LOG_WRN("Couldn't allocate a buffer after waiting 10 seconds.");
-		}
-	} while (!buf);
+	LOG_DBG("evt 0x%2x len %u", hdr.evt, hdr.len);
+
+	buf = bt_buf_get_evt_but_better(hdr.evt, data[0], K_SECONDS(60));
+	if (!buf) {
+		/* If events are blocked for 60s, something has gone wrong. */
+		LOG_DBG("Buffer pool full, ignoring event");
+		return buf;
+	}
 
 	net_buf_add_mem(buf, &hdr, sizeof(hdr));
 

include/zephyr/bluetooth/buf.h

@@ -161,6 +161,8 @@ static inline enum bt_buf_type bt_buf_get_type(struct net_buf *buf)
 		->type;
 }
 
+struct net_buf *bt_buf_get_evt_but_better(uint8_t evt, uint8_t meta, k_timeout_t timeout);
+
 /**
  * @}
  */

subsys/bluetooth/controller/hci/hci_driver.c

@@ -431,6 +431,29 @@ static void prio_recv_thread(void *p1, void *p2, void *p3)
 	}
 }
 
+static bool node_to_evt(struct node_rx_pdu *node_rx, uint8_t *evt, uint8_t *meta)
+{
+#if defined(CONFIG_BT_OBSERVER) && defined(CONFIG_BT_EXT_ADV)
+	switch (node_rx->hdr.type) {
+	case NODE_RX_TYPE_EXT_1M_REPORT:
+	case NODE_RX_TYPE_EXT_2M_REPORT:
+	case NODE_RX_TYPE_EXT_CODED_REPORT:
+		*evt = BT_HCI_EVT_LE_META_EVENT;
+		*meta = BT_HCI_EVT_LE_EXT_ADVERTISING_REPORT;
+		return true;
+	case NODE_RX_TYPE_EXT_SCAN_TERMINATE:
+		*evt = BT_HCI_EVT_LE_META_EVENT;
+		*meta = BT_HCI_EVT_LE_SCAN_TIMEOUT;
+		return true;
+	default:
+		return false;
+	}
+
+#else
+	return false;
+#endif
+}
+
 static inline struct net_buf *encode_node(struct node_rx_pdu *node_rx,
 					  int8_t class)
 {
@@ -443,10 +466,17 @@ static inline struct net_buf *encode_node(struct node_rx_pdu *node_rx,
 	case HCI_CLASS_EVT_CONNECTION:
 	case HCI_CLASS_EVT_LLCP:
 		if (class == HCI_CLASS_EVT_DISCARDABLE) {
-			buf = bt_buf_get_evt(BT_HCI_EVT_UNKNOWN, true,
-					     K_NO_WAIT);
+			buf = bt_buf_get_evt(BT_HCI_EVT_UNKNOWN, true, K_NO_WAIT);
 		} else {
-			buf = bt_buf_get_rx(BT_BUF_EVT, K_FOREVER);
+			uint8_t evt = 0;
+			uint8_t meta = 0;
+			bool recognized = node_to_evt(node_rx, &evt, &meta);
+
+			if (recognized) {
+				buf = bt_buf_get_evt_but_better(evt, meta, K_FOREVER);
+			} else {
+				buf = bt_buf_get_rx(BT_BUF_EVT, K_FOREVER);
+			}
 		}
 		if (buf) {
 			hci_evt_encode(node_rx, buf);

subsys/bluetooth/host/Kconfig

@@ -779,6 +779,19 @@ config BT_EXT_SCAN_BUF_SIZE
 	  provided by the controller is larger than this buffer size,
 	  the remaining data will be discarded.
 
+config BT_EXT_SCAN_BUF_NUM
+	int "Maximum number of report reassembly buffers"
+	range 1 65535
+	default 2
+	help
+	  Advertising reports can be fragmented when transmitted over HCI to the
+	  Host. They are re-assembled into a dedicated buffer pool and the
+	  application is notified asynchronously. This number governs the amount
+	  of pending reassembled advertising reports.
+
+	  Note that this option only uses up resources if extended advertising
+	  is enabled.
+
 endif # BT_OBSERVER
 
 config BT_SCAN_WITH_IDENTITY
@@ -1035,6 +1048,12 @@ config BT_TESTING
 	  This option enables custom Bluetooth testing interface.
 	  Shall only be used for testing purposes.
 
+config BT_HTOP
+	bool "Bluetooth Statistics"
+	help
+	  Enable printing of Bluetooth performance statistics.
+	  Sort of like THREAD_ANALYZER.
+
 config BT_CONN_DISABLE_SECURITY
 	bool "Disable security"
 	depends on BT_TESTING

subsys/bluetooth/host/buf.c

@@ -82,6 +82,101 @@ struct net_buf *bt_buf_get_rx(enum bt_buf_type type, k_timeout_t timeout)
 	return buf;
 }
 
+static bool is_hci_event_discardable(const uint8_t evt_type, const uint8_t subevt_type)
+{
+	switch (evt_type) {
+#if defined(CONFIG_BT_CLASSIC)
+	case BT_HCI_EVT_INQUIRY_RESULT_WITH_RSSI:
+	case BT_HCI_EVT_EXTENDED_INQUIRY_RESULT:
+		return true;
+#endif
+	case BT_HCI_EVT_LE_META_EVENT: {
+		switch (subevt_type) {
+		case BT_HCI_EVT_LE_ADVERTISING_REPORT:
+			return true;
+		default:
+			return false;
+		}
+	}
+	default:
+		return false;
+	}
+}
+
+NET_BUF_POOL_FIXED_DEFINE(sync_adv_pool, 2, SYNC_EVT_SIZE, sizeof(struct bt_buf_data), NULL);
+
+/* This is a host-private function */
+bool bt_buf_can_steal_ext_adv_report_buf(struct net_buf *buf)
+{
+	if (net_buf_pool_get(buf->pool_id) != &sync_adv_pool) {
+		/* The driver did not use `bt_buf_get_evt_but_better` */
+		return false;
+	}
+
+	/* Host can steal the buffer if we have at least one available.
+	 * This is the case when:
+	 * - There are buffers in the `free` LIFO
+	 * - There are un-initialized buffers
+	 */
+	return (!k_fifo_is_empty(&sync_adv_pool.free)) || (sync_adv_pool.uninit_count != 0);
+}
+
+struct net_buf *bt_buf_get_evt_but_better(uint8_t evt, uint8_t meta, k_timeout_t timeout)
+{
+	/* Always allocs so-called "discardable" events with K_NO_WAIT. Most of
+	 * the drivers will call from ISR so they should not block at all.
+	 *
+	 * For non-meta events, the `meta` param is just the next byte.
+	 */
+	struct net_buf *buf = NULL;
+
+	switch (evt) {
+	case BT_HCI_EVT_NUM_COMPLETED_PACKETS:
+	case BT_HCI_EVT_CMD_STATUS:
+	case BT_HCI_EVT_CMD_COMPLETE:
+	case BT_HCI_EVT_LE_META_EVENT: {
+		bool is_ext_adv = meta == BT_HCI_EVT_LE_EXT_ADVERTISING_REPORT;
+		bool is_meta = evt == BT_HCI_EVT_LE_META_EVENT;
+
+		if (is_ext_adv) {
+			buf = net_buf_alloc(&sync_adv_pool, timeout);
+			/* 60% of the time we get a buffer every time */
+			__ASSERT_NO_MSG(buf);
+			break;
+		}
+
+		if (is_meta && !is_ext_adv) {
+			/* For now we use the sync pool only for ext adv
+			 * reports. They cannot be marked as discardable thanks
+			 * to the geniuses in SIG that decided to save space and
+			 * only use two bits for reassembly status.
+			 */
+			break;
+		}
+
+		buf = net_buf_alloc(&sync_evt_pool, timeout);
+		break;
+	}
+	default:
+		break;
+	}
+
+	if (!buf) {
+		if (is_hci_event_discardable(evt, meta)) {
+			buf = net_buf_alloc(&discardable_pool, K_NO_WAIT);
+		} else {
+			return bt_buf_get_rx(BT_BUF_EVT, timeout);
+		}
+	}
+
+	if (buf) {
+		net_buf_reserve(buf, BT_BUF_RESERVE);
+		bt_buf_set_type(buf, BT_BUF_EVT);
+	}
+
+	return buf;
+}
+
 struct net_buf *bt_buf_get_evt(uint8_t evt, bool discardable,
 			       k_timeout_t timeout)
 {

subsys/bluetooth/host/hci_common.c

@@ -14,7 +14,7 @@ struct net_buf *bt_hci_evt_create(uint8_t evt, uint8_t len)
 	struct bt_hci_evt_hdr *hdr;
 	struct net_buf *buf;
 
-	buf = bt_buf_get_evt(evt, false, K_FOREVER);
+	buf = bt_buf_get_evt_but_better(evt, 0, K_FOREVER);
 
 	BT_ASSERT(buf);