From fc2fcd10cf482023bee9b2b97ca74ed21dd09f8b Mon Sep 17 00:00:00 2001 From: Johan Hedberg Date: Mon, 17 Jun 2019 15:54:45 +0300 Subject: [PATCH] Bluetooth: Add dedicated pool for HCI_Num_Completed_Packets HCI event This event is a priority one, so it's not safe to have it use the RX buffer pool which may be depleted due to non-priority events (e.g. advertising events). Since the event is consumed synchronously it's safe to have a single-buffer pool for it. Also introduce a new bt_buf_get_evt() API for HCI drivers to simplify the driver-side code, this effectively also deprecates bt_buf_get_cmd_complete() which now has no in-tree HCI driver users anymore. Fixes #16864 Signed-off-by: Johan Hedberg --- drivers/bluetooth/hci/h4.c | 11 ++---- drivers/bluetooth/hci/h5.c | 11 +----- drivers/bluetooth/hci/ipm_stm32wb.c | 6 +--- drivers/bluetooth/hci/spi.c | 7 ++-- drivers/bluetooth/hci/userchan.c | 11 ++---- include/bluetooth/buf.h | 12 +++++++ subsys/bluetooth/controller/hci/hci.c | 4 +-- subsys/bluetooth/controller/hci/hci_driver.c | 3 +- subsys/bluetooth/host/hci_core.c | 35 ++++++++++++++++++++ subsys/bluetooth/host/hci_ecc.c | 2 +- subsys/bluetooth/host/hci_raw.c | 12 +++++++ tests/bluetooth/hci_prop_evt/src/main.c | 2 +- 12 files changed, 75 insertions(+), 41 deletions(-) diff --git a/drivers/bluetooth/hci/h4.c b/drivers/bluetooth/hci/h4.c index 5248c11e056..cb886846bfc 100644 --- a/drivers/bluetooth/hci/h4.c +++ b/drivers/bluetooth/hci/h4.c @@ -161,16 +161,11 @@ static struct net_buf *get_rx(int timeout) { BT_DBG("type 0x%02x, evt 0x%02x", rx.type, rx.evt.evt); - if (rx.type == H4_EVT && (rx.evt.evt == BT_HCI_EVT_CMD_COMPLETE || - rx.evt.evt == BT_HCI_EVT_CMD_STATUS)) { - return bt_buf_get_cmd_complete(timeout); + if (rx.type == H4_EVT) { + return bt_buf_get_evt(rx.evt.evt, timeout); } - if (rx.type == H4_ACL) { - return bt_buf_get_rx(BT_BUF_ACL_IN, timeout); - } else { - return bt_buf_get_rx(BT_BUF_EVT, timeout); - } + return bt_buf_get_rx(BT_BUF_ACL_IN, timeout); } static void rx_thread(void *p1, void *p2, void *p3) diff --git a/drivers/bluetooth/hci/h5.c b/drivers/bluetooth/hci/h5.c index a3c5b64bad0..52727f7ec63 100644 --- a/drivers/bluetooth/hci/h5.c +++ b/drivers/bluetooth/hci/h5.c @@ -408,16 +408,7 @@ static inline struct net_buf *get_evt_buf(u8_t evt) { struct net_buf *buf; - switch (evt) { - case BT_HCI_EVT_CMD_COMPLETE: - case BT_HCI_EVT_CMD_STATUS: - buf = bt_buf_get_cmd_complete(K_NO_WAIT); - break; - default: - buf = bt_buf_get_rx(BT_BUF_EVT, K_NO_WAIT); - break; - } - + buf = bt_buf_get_evt(evt, K_NO_WAIT); if (buf) { net_buf_add_u8(h5.rx_buf, evt); } diff --git a/drivers/bluetooth/hci/ipm_stm32wb.c b/drivers/bluetooth/hci/ipm_stm32wb.c index a4d14a1979f..9f2b95129f6 100644 --- a/drivers/bluetooth/hci/ipm_stm32wb.c +++ b/drivers/bluetooth/hci/ipm_stm32wb.c @@ -114,12 +114,8 @@ void TM_EvtReceivedCb(TL_EvtPacket_t *hcievt) BT_ERR("Unknown evtcode type 0x%02x", hcievt->evtserial.evt.evtcode); goto out; - case BT_HCI_EVT_CMD_COMPLETE: - case BT_HCI_EVT_CMD_STATUS: - buf = bt_buf_get_cmd_complete(K_FOREVER); - break; default: - buf = bt_buf_get_rx(BT_BUF_EVT, K_FOREVER); + buf = bt_buf_get_evt(evtserial.evt.evtcode, K_FOREVER); break; } net_buf_add_mem(buf, &hcievt->evtserial.evt, diff --git a/drivers/bluetooth/hci/spi.c b/drivers/bluetooth/hci/spi.c index 534fb860cc6..5749c355411 100644 --- a/drivers/bluetooth/hci/spi.c +++ b/drivers/bluetooth/hci/spi.c @@ -353,12 +353,9 @@ static void bt_spi_rx_thread(void) /* Vendor events are currently unsupported */ bt_spi_handle_vendor_evt(rxmsg); continue; - case BT_HCI_EVT_CMD_COMPLETE: - case BT_HCI_EVT_CMD_STATUS: - buf = bt_buf_get_cmd_complete(K_FOREVER); - break; default: - buf = bt_buf_get_rx(BT_BUF_EVT, K_FOREVER); + buf = bt_buf_get_evt(rxmsg[EVT_HEADER_EVENT], + K_FOREVER); break; } diff --git a/drivers/bluetooth/hci/userchan.c b/drivers/bluetooth/hci/userchan.c index 1b80dc19d7b..f2c8b3f14d1 100644 --- a/drivers/bluetooth/hci/userchan.c +++ b/drivers/bluetooth/hci/userchan.c @@ -57,16 +57,11 @@ static int bt_dev_index = -1; static struct net_buf *get_rx(const u8_t *buf) { - if (buf[0] == H4_EVT && (buf[1] == BT_HCI_EVT_CMD_COMPLETE || - buf[1] == BT_HCI_EVT_CMD_STATUS)) { - return bt_buf_get_cmd_complete(K_FOREVER); + if (buf[0] == H4_EVT) { + return bt_buf_get_evt(buf[1], K_FOREVER); } - if (buf[0] == H4_ACL) { - return bt_buf_get_rx(BT_BUF_ACL_IN, K_FOREVER); - } else { - return bt_buf_get_rx(BT_BUF_EVT, K_FOREVER); - } + return bt_buf_get_rx(BT_BUF_ACL_IN, K_FOREVER); } static bool uc_ready(void) diff --git a/include/bluetooth/buf.h b/include/bluetooth/buf.h index b0b26185e63..3110ba94ccc 100644 --- a/include/bluetooth/buf.h +++ b/include/bluetooth/buf.h @@ -64,6 +64,18 @@ struct net_buf *bt_buf_get_rx(enum bt_buf_type type, s32_t timeout); */ struct net_buf *bt_buf_get_cmd_complete(s32_t timeout); +/** Allocate a buffer for an HCI Event + * + * This will set the buffer type so bt_buf_set_type() does not need to + * be explicitly called before bt_recv_prio() or bt_recv(). + * + * @param evt HCI event code + * @param timeout Timeout in milliseconds, or one of the special values + * K_NO_WAIT and K_FOREVER. + * @return A new buffer. + */ +struct net_buf *bt_buf_get_evt(u8_t evt, s32_t timeout); + /** Set the buffer type * * @param buf Bluetooth buffer diff --git a/subsys/bluetooth/controller/hci/hci.c b/subsys/bluetooth/controller/hci/hci.c index 91153b119de..1b99bd48f4d 100644 --- a/subsys/bluetooth/controller/hci/hci.c +++ b/subsys/bluetooth/controller/hci/hci.c @@ -120,7 +120,7 @@ void *hci_cmd_complete(struct net_buf **buf, u8_t plen) { struct bt_hci_evt_cmd_complete *cc; - *buf = bt_buf_get_cmd_complete(K_FOREVER); + *buf = bt_buf_get_evt(BT_HCI_EVT_CMD_COMPLETE, K_FOREVER); hci_evt_create(*buf, BT_HCI_EVT_CMD_COMPLETE, sizeof(*cc) + plen); @@ -137,7 +137,7 @@ static struct net_buf *cmd_status(u8_t status) struct bt_hci_evt_cmd_status *cs; struct net_buf *buf; - buf = bt_buf_get_cmd_complete(K_FOREVER); + buf = bt_buf_get_evt(BT_HCI_EVT_CMD_STATUS, K_FOREVER); hci_evt_create(buf, BT_HCI_EVT_CMD_STATUS, sizeof(*cs)); cs = net_buf_add(buf, sizeof(*cs)); diff --git a/subsys/bluetooth/controller/hci/hci_driver.c b/subsys/bluetooth/controller/hci/hci_driver.c index 814a3d20fa9..1229c4625b8 100644 --- a/subsys/bluetooth/controller/hci/hci_driver.c +++ b/subsys/bluetooth/controller/hci/hci_driver.c @@ -90,7 +90,8 @@ static void prio_recv_thread(void *p1, void *p2, void *p3) #if defined(CONFIG_BT_CONN) struct net_buf *buf; - buf = bt_buf_get_rx(BT_BUF_EVT, K_FOREVER); + buf = bt_buf_get_evt(BT_HCI_EVT_NUM_COMPLETED_PACKETS, + K_FOREVER); hci_num_cmplt_encode(buf, handle, num_cmplt); BT_DBG("Num Complete: 0x%04x:%u", handle, num_cmplt); bt_recv_prio(buf); diff --git a/subsys/bluetooth/host/hci_core.c b/subsys/bluetooth/host/hci_core.c index 8b2fcb9bd2a..ba32c8970f5 100644 --- a/subsys/bluetooth/host/hci_core.c +++ b/subsys/bluetooth/host/hci_core.c @@ -138,6 +138,16 @@ NET_BUF_POOL_DEFINE(hci_cmd_pool, CONFIG_BT_HCI_CMD_COUNT, NET_BUF_POOL_DEFINE(hci_rx_pool, CONFIG_BT_RX_BUF_COUNT, BT_BUF_RX_SIZE, BT_BUF_USER_DATA_MIN, NULL); +#if defined(CONFIG_BT_CONN) +/* Dedicated pool for HCI_Number_of_Completed_Packets. This event is always + * consumed synchronously by bt_recv_prio() so a single buffer is enough. + * Having a dedicated pool for it ensures that exhaustion of the RX pool + * cannot block the delivery of this priority event. + */ +NET_BUF_POOL_DEFINE(num_complete_pool, 1, BT_BUF_RX_SIZE, + BT_BUF_USER_DATA_MIN, NULL); +#endif /* CONFIG_BT_CONN */ + struct event_handler { u8_t event; u8_t min_len; @@ -5662,6 +5672,31 @@ struct net_buf *bt_buf_get_cmd_complete(s32_t timeout) return bt_buf_get_rx(BT_BUF_EVT, timeout); } +struct net_buf *bt_buf_get_evt(u8_t evt, s32_t timeout) +{ + switch (evt) { +#if defined(CONFIG_BT_CONN) + case BT_HCI_EVT_NUM_COMPLETED_PACKETS: + { + struct net_buf *buf; + + buf = net_buf_alloc(&num_complete_pool, timeout); + if (buf) { + net_buf_reserve(buf, CONFIG_BT_HCI_RESERVE); + bt_buf_set_type(buf, BT_BUF_EVT); + } + + return buf; + } +#endif /* CONFIG_BT_CONN */ + case BT_HCI_EVT_CMD_COMPLETE: + case BT_HCI_EVT_CMD_STATUS: + return bt_buf_get_cmd_complete(timeout); + default: + return bt_buf_get_rx(BT_BUF_EVT, timeout); + } +} + #if defined(CONFIG_BT_BREDR) static int br_start_inquiry(const struct bt_br_discovery_param *param) { diff --git a/subsys/bluetooth/host/hci_ecc.c b/subsys/bluetooth/host/hci_ecc.c index a907583ec11..7d8ee3e82b4 100644 --- a/subsys/bluetooth/host/hci_ecc.c +++ b/subsys/bluetooth/host/hci_ecc.c @@ -84,7 +84,7 @@ static void send_cmd_status(u16_t opcode, u8_t status) BT_DBG("opcode %x status %x", opcode, status); - buf = bt_buf_get_cmd_complete(K_FOREVER); + buf = bt_buf_get_evt(BT_HCI_EVT_CMD_STATUS, K_FOREVER); bt_buf_set_type(buf, BT_BUF_EVT); hdr = net_buf_add(buf, sizeof(*hdr)); diff --git a/subsys/bluetooth/host/hci_raw.c b/subsys/bluetooth/host/hci_raw.c index f81d4e8ed3a..6694c766ebf 100644 --- a/subsys/bluetooth/host/hci_raw.c +++ b/subsys/bluetooth/host/hci_raw.c @@ -72,6 +72,18 @@ struct net_buf *bt_buf_get_cmd_complete(s32_t timeout) return buf; } +struct net_buf *bt_buf_get_evt(u8_t evt, s32_t timeout) +{ + struct net_buf *buf; + + buf = net_buf_alloc(&hci_rx_pool, timeout); + if (buf) { + bt_buf_set_type(buf, BT_BUF_EVT); + } + + return buf; +} + int bt_recv(struct net_buf *buf) { BT_DBG("buf %p len %u", buf, buf->len); diff --git a/tests/bluetooth/hci_prop_evt/src/main.c b/tests/bluetooth/hci_prop_evt/src/main.c index a455ef30735..75a50bc4daa 100644 --- a/tests/bluetooth/hci_prop_evt/src/main.c +++ b/tests/bluetooth/hci_prop_evt/src/main.c @@ -53,7 +53,7 @@ static void *cmd_complete(struct net_buf **buf, u8_t plen, u16_t opcode) { struct bt_hci_evt_cmd_complete *cc; - *buf = bt_buf_get_cmd_complete(K_FOREVER); + *buf = bt_buf_get_evt(BT_HCI_EVT_CMD_COMPLETE, K_FOREVER); evt_create(*buf, BT_HCI_EVT_CMD_COMPLETE, sizeof(*cc) + plen); cc = net_buf_add(*buf, sizeof(*cc)); cc->ncmd = 1U;