From 4be66bd33dec1edc82d35270c718f81443c42e8a Mon Sep 17 00:00:00 2001 From: Joakim Andersson Date: Wed, 3 Jun 2020 14:48:04 +0200 Subject: [PATCH] Bluetooth: Fix host RX thread deadlock Fix host RX thread being deadlocked. The deadlock occurs because the RX thread is stuck waiting in conn_tx_alloc with K_FOREVER but if the connection is disconnected only the RX thread can unblock it in the handling of the disconnect event. This commit fixes this deadlock by splitting the processing of the disconnected event into two parts. The part needed to unblock the RX is to release resources held by unack'ed TX packets and mark the connection state as not connected anymore. The RX thread waiting for free_tx fifo and the TX thread waiting for the bt_dev.le.pkts semaphore will both check the connected state after having acquired them and will abort if disconnected. The rest of the processing will be handled at normal RX thread priority like normal. Move the bt_recv_prio handling to the Bluetooth host when the host has defined its own RX thread (CONFIG_BT_RECV_IS_RX_THREAD=n). If the HCI driver has the RX thread (CONFIG_BT_RECV_IS_RX_THREAD=y), then the responsibility to call bt_recv and bt_recv_prio correctly falls to the HCI driver. The helper function bt_hci_evt_is_prio() is replaced with bt_hci_evt_get_flags() so that the HCI driver can do this correctly. This decision to replace was made so that existing HCI drivers maintained out-of-tree will fail at compile time with the new system. Signed-off-by: Joakim Andersson Signed-off-by: Carles Cufi Bluetooth: host: Move bt_recv_prio to host when RX thread is defined Signed-off-by: Joakim Andersson --- drivers/bluetooth/hci/h4.c | 12 +- drivers/bluetooth/hci/h5.c | 6 - drivers/bluetooth/hci/ipm_stm32wb.c | 8 +- drivers/bluetooth/hci/rpmsg.c | 15 +-- drivers/bluetooth/hci/spi.c | 8 +- drivers/bluetooth/hci/userchan.c | 6 +- include/drivers/bluetooth/hci_driver.h | 49 ++++---- subsys/bluetooth/controller/hci/hci.c | 20 ++-- subsys/bluetooth/controller/hci/hci_driver.c | 47 ++++++-- .../bluetooth/controller/hci/hci_internal.h | 3 + subsys/bluetooth/host/Kconfig | 9 +- subsys/bluetooth/host/conn.c | 20 ++-- subsys/bluetooth/host/conn_internal.h | 8 ++ subsys/bluetooth/host/hci_core.c | 109 +++++++++++++----- subsys/bluetooth/host/hci_ecc.c | 6 +- subsys/bluetooth/host/hci_raw.c | 11 ++ tests/bluetooth/hci_prop_evt/prj.conf | 1 + 17 files changed, 222 insertions(+), 116 deletions(-) diff --git a/drivers/bluetooth/hci/h4.c b/drivers/bluetooth/hci/h4.c index 3919a9adfdc..d0e00c309f3 100644 --- a/drivers/bluetooth/hci/h4.c +++ b/drivers/bluetooth/hci/h4.c @@ -229,7 +229,7 @@ static size_t h4_discard(struct device *uart, size_t len) static inline void read_payload(void) { struct net_buf *buf; - bool prio; + uint8_t evt_flags; int read; if (!rx.buf) { @@ -271,23 +271,25 @@ static inline void read_payload(void) return; } - prio = (rx.type == H4_EVT && bt_hci_evt_is_prio(rx.evt.evt)); - buf = rx.buf; rx.buf = NULL; if (rx.type == H4_EVT) { + evt_flags = bt_hci_evt_get_flags(rx.evt.evt); bt_buf_set_type(buf, BT_BUF_EVT); } else { + evt_flags = BT_HCI_EVT_FLAG_RECV; bt_buf_set_type(buf, BT_BUF_ACL_IN); } reset_rx(); - if (prio) { + if (evt_flags & BT_HCI_EVT_FLAG_RECV_PRIO) { BT_DBG("Calling bt_recv_prio(%p)", buf); bt_recv_prio(buf); - } else { + } + + if (evt_flags & BT_HCI_EVT_FLAG_RECV) { BT_DBG("Putting buf %p to rx fifo", buf); net_buf_put(&rx.fifo, buf); } diff --git a/drivers/bluetooth/hci/h5.c b/drivers/bluetooth/hci/h5.c index 77a000cf6a5..2d2194084dc 100644 --- a/drivers/bluetooth/hci/h5.c +++ b/drivers/bluetooth/hci/h5.c @@ -393,12 +393,6 @@ static void h5_process_complete_packet(uint8_t *hdr) net_buf_put(&h5.rx_queue, buf); break; case HCI_EVENT_PKT: - if (buf->len > sizeof(struct bt_hci_evt_hdr) && - bt_hci_evt_is_prio(((struct bt_hci_evt_hdr *)buf->data)->evt)) { - hexdump("=> ", buf->data, buf->len); - bt_recv_prio(buf); - break; - } case HCI_ACLDATA_PKT: hexdump("=> ", buf->data, buf->len); bt_recv(buf); diff --git a/drivers/bluetooth/hci/ipm_stm32wb.c b/drivers/bluetooth/hci/ipm_stm32wb.c index 680e27fdbc9..da7e600e4f0 100644 --- a/drivers/bluetooth/hci/ipm_stm32wb.c +++ b/drivers/bluetooth/hci/ipm_stm32wb.c @@ -199,13 +199,7 @@ static void bt_ipm_rx_thread(void) TL_MM_EvtDone(hcievt); - if (hcievt->evtserial.type == HCI_EVT && - bt_hci_evt_is_prio(hcievt->evtserial.evt.evtcode)) { - bt_recv_prio(buf); - } else { - bt_recv(buf); - } - + bt_recv(buf); end_loop: k_sem_give(&ipm_busy); } diff --git a/drivers/bluetooth/hci/rpmsg.c b/drivers/bluetooth/hci/rpmsg.c index c0ffce865cf..d1f70c83534 100644 --- a/drivers/bluetooth/hci/rpmsg.c +++ b/drivers/bluetooth/hci/rpmsg.c @@ -48,8 +48,7 @@ static bool is_hci_event_discardable(const uint8_t *evt_data) } } -static struct net_buf *bt_rpmsg_evt_recv(uint8_t *data, size_t remaining, - bool *prio) +static struct net_buf *bt_rpmsg_evt_recv(uint8_t *data, size_t remaining) { bool discardable; struct bt_hci_evt_hdr hdr; @@ -83,8 +82,6 @@ static struct net_buf *bt_rpmsg_evt_recv(uint8_t *data, size_t remaining, } net_buf_add_mem(buf, &hdr, sizeof(hdr)); - *prio = bt_hci_evt_is_prio(hdr.evt); - net_buf_add_mem(buf, data, remaining); return buf; @@ -127,7 +124,6 @@ static struct net_buf *bt_rpmsg_acl_recv(uint8_t *data, size_t remaining) void bt_rpmsg_rx(uint8_t *data, size_t len) { uint8_t pkt_indicator; - bool prio = false; struct net_buf *buf = NULL; size_t remaining = len; @@ -138,7 +134,7 @@ void bt_rpmsg_rx(uint8_t *data, size_t len) switch (pkt_indicator) { case RPMSG_EVT: - buf = bt_rpmsg_evt_recv(data, remaining, &prio); + buf = bt_rpmsg_evt_recv(data, remaining); break; case RPMSG_ACL: @@ -152,11 +148,8 @@ void bt_rpmsg_rx(uint8_t *data, size_t len) if (buf) { BT_DBG("Calling bt_recv(%p)", buf); - if (prio) { - bt_recv_prio(buf); - } else { - bt_recv(buf); - } + + bt_recv(buf); BT_HEXDUMP_DBG(buf->data, buf->len, "RX buf payload:"); } diff --git a/drivers/bluetooth/hci/spi.c b/drivers/bluetooth/hci/spi.c index 156e3da3b1c..6ce14fa0cb5 100644 --- a/drivers/bluetooth/hci/spi.c +++ b/drivers/bluetooth/hci/spi.c @@ -385,12 +385,8 @@ static void bt_spi_rx_thread(void) continue; } - if (rxmsg[PACKET_TYPE] == HCI_EVT && - bt_hci_evt_is_prio(rxmsg[EVT_HEADER_EVENT])) { - bt_recv_prio(buf); - } else { - bt_recv(buf); - } + bt_recv(buf); + /* On BlueNRG-MS, host is expected to read */ /* as long as IRQ pin is high */ } while (irq_pin_high()); diff --git a/drivers/bluetooth/hci/userchan.c b/drivers/bluetooth/hci/userchan.c index 657a726a62c..516203b3178 100644 --- a/drivers/bluetooth/hci/userchan.c +++ b/drivers/bluetooth/hci/userchan.c @@ -109,11 +109,7 @@ static void rx_thread(void *p1, void *p2, void *p3) BT_DBG("Calling bt_recv(%p)", buf); - if (frame[0] == H4_EVT && bt_hci_evt_is_prio(frame[1])) { - bt_recv_prio(buf); - } else { - bt_recv(buf); - } + bt_recv(buf); k_yield(); } diff --git a/include/drivers/bluetooth/hci_driver.h b/include/drivers/bluetooth/hci_driver.h index bc538443c0c..a24b779ea44 100644 --- a/include/drivers/bluetooth/hci_driver.h +++ b/include/drivers/bluetooth/hci_driver.h @@ -32,33 +32,40 @@ enum { BT_QUIRK_NO_RESET = BIT(0), }; -/** - * @brief Check if an HCI event is high priority or not. +/* @brief The HCI event shall be given to bt_recv_prio */ +#define BT_HCI_EVT_FLAG_RECV_PRIO BIT(0) +/* @brief The HCI event shall be given to bt_recv. */ +#define BT_HCI_EVT_FLAG_RECV BIT(1) + +/** @brief Get HCI event flags. * - * Helper for the HCI driver to know which events are ok to be passed - * through the RX thread and which must be given to bt_recv_prio() from - * another context (e.g. ISR). If this function returns true it's safe - * to pass the event through the RX thread, however if it returns false - * then this risks a deadlock. + * Helper for the HCI driver to get HCI event flags that describes rules that. + * must be followed. + * + * When CONFIG_BT_RECV_IS_RX_THREAD is enabled the flags + * BT_HCI_EVT_FLAG_RECV and BT_HCI_EVT_FLAG_RECV_PRIO indicates if the event + * should be given to bt_recv or bt_recv_prio. * * @param evt HCI event code. * - * @return true if the event can be processed in the RX thread, false - * if it cannot. + * @return HCI event flags for the specified event. */ -static inline bool bt_hci_evt_is_prio(uint8_t evt) +static inline uint8_t bt_hci_evt_get_flags(uint8_t evt) { switch (evt) { - case BT_HCI_EVT_CMD_COMPLETE: - case BT_HCI_EVT_CMD_STATUS: + case BT_HCI_EVT_DISCONN_COMPLETE: + return BT_HCI_EVT_FLAG_RECV | BT_HCI_EVT_FLAG_RECV_PRIO; /* fallthrough */ #if defined(CONFIG_BT_CONN) case BT_HCI_EVT_NUM_COMPLETED_PACKETS: case BT_HCI_EVT_DATA_BUF_OVERFLOW: -#endif - return true; + /* fallthrough */ +#endif /* defined(CONFIG_BT_CONN) */ + case BT_HCI_EVT_CMD_COMPLETE: + case BT_HCI_EVT_CMD_STATUS: + return BT_HCI_EVT_FLAG_RECV_PRIO; default: - return false; + return BT_HCI_EVT_FLAG_RECV; } } @@ -67,9 +74,11 @@ static inline bool bt_hci_evt_is_prio(uint8_t evt) * * This is the main function through which the HCI driver provides the * host with data from the controller. The buffer needs to have its type - * set with the help of bt_buf_set_type() before calling this API. This API - * should not be used for so-called high priority HCI events, which should - * instead be delivered to the host stack through bt_recv_prio(). + * set with the help of bt_buf_set_type() before calling this API. + * + * When CONFIG_BT_RECV_IS_RX_THREAD is defined then this API should not be used + * for so-called high priority HCI events, which should instead be delivered to + * the host stack through bt_recv_prio(). * * @param buf Network buffer containing data from the controller. * @@ -82,8 +91,8 @@ int bt_recv(struct net_buf *buf); * * This is the same as bt_recv(), except that it should be used for * so-called high priority HCI events. There's a separate - * bt_hci_evt_is_prio() helper that can be used to identify which events - * are high priority. + * bt_hci_evt_get_flags() helper that can be used to identify which events + * have the BT_HCI_EVT_FLAG_RECV_PRIO flag set. * * As with bt_recv(), the buffer needs to have its type set with the help of * bt_buf_set_type() before calling this API. The only exception is so called diff --git a/subsys/bluetooth/controller/hci/hci.c b/subsys/bluetooth/controller/hci/hci.c index 866b683f902..29e35d65654 100644 --- a/subsys/bluetooth/controller/hci/hci.c +++ b/subsys/bluetooth/controller/hci/hci.c @@ -292,12 +292,15 @@ static void reset(struct net_buf *buf, struct net_buf **evt) ccst->status = 0x00; } +#if defined(CONFIG_BT_CONN) + conn_count = 0U; +#endif + #if defined(CONFIG_BT_HCI_ACL_FLOW_CONTROL) hci_hbuf_total = 0; hci_hbuf_sent = 0U; hci_hbuf_acked = 0U; (void)memset(hci_hbuf_pend, 0, sizeof(hci_hbuf_pend)); - conn_count = 0U; if (buf) { atomic_set_bit(&hci_state_mask, HCI_STATE_BIT_RESET); k_poll_signal_raise(hbuf_signal, 0x0); @@ -3702,8 +3705,8 @@ static void le_conn_complete(struct pdu_data *pdu_data, uint16_t handle, lecc->clock_accuracy = node_rx->sca; } -static void disconn_complete(struct pdu_data *pdu_data, uint16_t handle, - struct net_buf *buf) +void hci_disconn_complete_encode(struct pdu_data *pdu_data, uint16_t handle, + struct net_buf *buf) { struct bt_hci_evt_disconn_complete *ep; @@ -3717,7 +3720,10 @@ static void disconn_complete(struct pdu_data *pdu_data, uint16_t handle, ep->status = 0x00; ep->handle = sys_cpu_to_le16(handle); ep->reason = *((uint8_t *)pdu_data); +} +void hci_disconn_complete_process(uint16_t handle) +{ #if defined(CONFIG_BT_HCI_ACL_FLOW_CONTROL) /* Clear any pending packets upon disconnection */ /* Note: This requires linear handle values starting from 0 */ @@ -3895,7 +3901,7 @@ static void encode_control(struct node_rx_pdu *node_rx, break; case NODE_RX_TYPE_TERMINATE: - disconn_complete(pdu_data, handle, buf); + hci_disconn_complete_encode(pdu_data, handle, buf); break; case NODE_RX_TYPE_CONN_UPDATE: @@ -4102,8 +4108,8 @@ static void le_data_len_change(struct pdu_data *pdu_data, uint16_t handle, #endif /* CONFIG_BT_CTLR_DATA_LENGTH */ #if defined(CONFIG_BT_REMOTE_VERSION) -void hci_remote_version_info_encode(struct pdu_data *pdu_data, uint16_t handle, - struct net_buf *buf) +static void remote_version_info_encode(struct pdu_data *pdu_data, + uint16_t handle, struct net_buf *buf) { struct pdu_data_llctrl_version_ind *ver_ind; struct bt_hci_evt_remote_version_info *ep; @@ -4143,7 +4149,7 @@ static void encode_data_ctrl(struct node_rx_pdu *node_rx, #if defined(CONFIG_BT_REMOTE_VERSION) case PDU_DATA_LLCTRL_TYPE_VERSION_IND: - hci_remote_version_info_encode(pdu_data, handle, buf); + remote_version_info_encode(pdu_data, handle, buf); break; #endif /* defined(CONFIG_BT_REMOTE_VERSION) */ diff --git a/subsys/bluetooth/controller/hci/hci_driver.c b/subsys/bluetooth/controller/hci/hci_driver.c index f0d71c9713a..67a0960cb12 100644 --- a/subsys/bluetooth/controller/hci/hci_driver.c +++ b/subsys/bluetooth/controller/hci/hci_driver.c @@ -69,9 +69,29 @@ static sys_slist_t hbuf_pend; static int32_t hbuf_count; #endif -static struct net_buf *process_prio_evt(struct node_rx_pdu *node_rx) +static struct net_buf *process_prio_evt(struct node_rx_pdu *node_rx, + uint8_t *evt_flags) { - /* Currently there are no events processed */ +#if defined(CONFIG_BT_CONN) + if (node_rx->hdr.user_meta == HCI_CLASS_EVT_CONNECTION) { + uint16_t handle; + struct pdu_data *pdu_data = PDU_DATA(node_rx); + + handle = node_rx->hdr.handle; + if (node_rx->hdr.type == NODE_RX_TYPE_TERMINATE) { + struct net_buf *buf; + + buf = bt_buf_get_evt(BT_HCI_EVT_DISCONN_COMPLETE, false, + K_FOREVER); + hci_disconn_complete_encode(pdu_data, handle, buf); + hci_disconn_complete_process(handle); + *evt_flags = BT_HCI_EVT_FLAG_RECV_PRIO | BT_HCI_EVT_FLAG_RECV; + return buf; + } + } +#endif /* CONFIG_BT_CONN */ + + *evt_flags = BT_HCI_EVT_FLAG_RECV; return NULL; } @@ -105,6 +125,8 @@ static void prio_recv_thread(void *p1, void *p2, void *p3) } if (node_rx) { + uint8_t evt_flags; + /* Until now we've only peeked, now we really do * the handover */ @@ -113,15 +135,24 @@ static void prio_recv_thread(void *p1, void *p2, void *p3) /* Find out and store the class for this node */ node_rx->hdr.user_meta = hci_get_class(node_rx); - buf = process_prio_evt(node_rx); + buf = process_prio_evt(node_rx, &evt_flags); if (buf) { - - node_rx->hdr.next = NULL; - ll_rx_mem_release((void **)&node_rx); - BT_DBG("Priority event"); + if (!(evt_flags & BT_HCI_EVT_FLAG_RECV)) { + node_rx->hdr.next = NULL; + ll_rx_mem_release((void **)&node_rx); + } + bt_recv_prio(buf); - } else { + /* bt_recv_prio would not release normal evt + * buf. + */ + if (evt_flags & BT_HCI_EVT_FLAG_RECV) { + net_buf_unref(buf); + } + } + + if (evt_flags & BT_HCI_EVT_FLAG_RECV) { /* Send the rx node up to Host thread, * recv_thread() */ diff --git a/subsys/bluetooth/controller/hci/hci_internal.h b/subsys/bluetooth/controller/hci/hci_internal.h index 8b6b0c9db6d..99864c81081 100644 --- a/subsys/bluetooth/controller/hci/hci_internal.h +++ b/subsys/bluetooth/controller/hci/hci_internal.h @@ -42,6 +42,9 @@ void hci_init(struct k_poll_signal *signal_host_buf); struct net_buf *hci_cmd_handle(struct net_buf *cmd, void **node_rx); void hci_evt_encode(struct node_rx_pdu *node_rx, struct net_buf *buf); uint8_t hci_get_class(struct node_rx_pdu *node_rx); +void hci_disconn_complete_encode(struct pdu_data *pdu_data, uint16_t handle, + struct net_buf *buf); +void hci_disconn_complete_process(uint16_t handle); #if defined(CONFIG_BT_CONN) int hci_acl_handle(struct net_buf *acl, struct net_buf **evt); void hci_acl_encode(struct node_rx_pdu *node_rx, struct net_buf *buf); diff --git a/subsys/bluetooth/host/Kconfig b/subsys/bluetooth/host/Kconfig index ac75c5c7fac..6a05bf2c23e 100644 --- a/subsys/bluetooth/host/Kconfig +++ b/subsys/bluetooth/host/Kconfig @@ -128,11 +128,12 @@ config BT_HCI_RESERVE config BT_RECV_IS_RX_THREAD # Hidden option set by the HCI driver to indicate that there's # no need for the host to have its own RX thread. - # It is then the responsibility of the HCI driver to call bt_recv_prio - # from a higher priority context than bt_recv in order to avoid deadlock. - # If the host has its own RX thread it is safe to call bt_recv and - # bt_recv_prio from the same priority context. + # If this option has been enabled it is then the responsibility of the + # HCI driver to call bt_recv_prio from a higher priority context than + # bt_recv in order to avoid deadlocks. + # If this option is disabled then only bt_recv should be called. bool + prompt "bt_recv is called from RX thread" if BT_NO_DRIVER config BT_RX_STACK_SIZE int "Size of the receiving thread stack" diff --git a/subsys/bluetooth/host/conn.c b/subsys/bluetooth/host/conn.c index 838d9b6d6c8..fade901e498 100644 --- a/subsys/bluetooth/host/conn.c +++ b/subsys/bluetooth/host/conn.c @@ -110,6 +110,8 @@ static inline const char *state2str(bt_conn_state_t state) switch (state) { case BT_CONN_DISCONNECTED: return "disconnected"; + case BT_CONN_DISCONNECT_COMPLETE: + return "disconnect-complete"; case BT_CONN_CONNECT_SCAN: return "connect-scan"; case BT_CONN_CONNECT_DIR_ADV: @@ -1725,9 +1727,7 @@ void bt_conn_set_state(struct bt_conn *conn, bt_conn_state_t state) * running. */ switch (old_state) { - case BT_CONN_CONNECTED: - case BT_CONN_DISCONNECT: - process_unack_tx(conn); + case BT_CONN_DISCONNECT_COMPLETE: tx_notify(conn); /* Cancel Connection Update if it is pending */ @@ -1783,8 +1783,11 @@ void bt_conn_set_state(struct bt_conn *conn, bt_conn_state_t state) */ bt_conn_unref(conn); break; + case BT_CONN_CONNECTED: + case BT_CONN_DISCONNECT: case BT_CONN_DISCONNECTED: - /* Cannot happen, no transition. */ + /* Cannot happen. */ + BT_WARN("Invalid (%u) old state", state); break; } break; @@ -1813,6 +1816,9 @@ void bt_conn_set_state(struct bt_conn *conn, bt_conn_state_t state) break; case BT_CONN_DISCONNECT: break; + case BT_CONN_DISCONNECT_COMPLETE: + process_unack_tx(conn); + break; default: BT_WARN("no valid (%u) state was set", state); @@ -1830,8 +1836,7 @@ struct bt_conn *bt_conn_lookup_handle(uint16_t handle) } /* We only care about connections with a valid handle */ - if (conns[i].state != BT_CONN_CONNECTED && - conns[i].state != BT_CONN_DISCONNECT) { + if (!bt_conn_is_handle_valid(&conns[i])) { continue; } @@ -1847,8 +1852,7 @@ struct bt_conn *bt_conn_lookup_handle(uint16_t handle) } /* We only care about connections with a valid handle */ - if (sco_conns[i].state != BT_CONN_CONNECTED && - sco_conns[i].state != BT_CONN_DISCONNECT) { + if (!bt_conn_is_handle_valid(&conns[i])) { continue; } diff --git a/subsys/bluetooth/host/conn_internal.h b/subsys/bluetooth/host/conn_internal.h index 34613c5d5c5..249027320ae 100644 --- a/subsys/bluetooth/host/conn_internal.h +++ b/subsys/bluetooth/host/conn_internal.h @@ -9,6 +9,7 @@ */ typedef enum __packed { BT_CONN_DISCONNECTED, + BT_CONN_DISCONNECT_COMPLETE, BT_CONN_CONNECT_SCAN, BT_CONN_CONNECT_AUTO, BT_CONN_CONNECT_ADV, @@ -212,6 +213,13 @@ void bt_conn_disconnect_all(uint8_t id); /* Look up an existing connection */ struct bt_conn *bt_conn_lookup_handle(uint16_t handle); +static inline bool bt_conn_is_handle_valid(struct bt_conn *conn) +{ + return conn->state == BT_CONN_CONNECTED || + conn->state == BT_CONN_DISCONNECT || + conn->state == BT_CONN_DISCONNECT_COMPLETE; +} + /* Check if the connection is with the given peer. */ bool bt_conn_is_peer_addr_le(const struct bt_conn *conn, uint8_t id, const bt_addr_le_t *peer); diff --git a/subsys/bluetooth/host/hci_core.c b/subsys/bluetooth/host/hci_core.c index 0ab3a593cac..5a5808f6ff8 100644 --- a/subsys/bluetooth/host/hci_core.c +++ b/subsys/bluetooth/host/hci_core.c @@ -251,8 +251,7 @@ static void report_completed_packet(struct net_buf *buf) return; } - if (conn->state != BT_CONN_CONNECTED && - conn->state != BT_CONN_DISCONNECT) { + if (!bt_conn_is_handle_valid(conn)) { BT_WARN("Not reporting packet for non-connected conn"); bt_conn_unref(conn); return; @@ -1634,6 +1633,29 @@ int bt_hci_disconnect(uint16_t handle, uint8_t reason) return bt_hci_cmd_send(BT_HCI_OP_DISCONNECT, buf); } +static void hci_disconn_complete_prio(struct net_buf *buf) +{ + struct bt_hci_evt_disconn_complete *evt = (void *)buf->data; + uint16_t handle = sys_le16_to_cpu(evt->handle); + struct bt_conn *conn; + + BT_DBG("status 0x%02x handle %u reason 0x%02x", evt->status, handle, + evt->reason); + + if (evt->status) { + return; + } + + conn = bt_conn_lookup_handle(handle); + if (!conn) { + BT_ERR("Unable to look up conn with handle %u", handle); + return; + } + + bt_conn_set_state(conn, BT_CONN_DISCONNECT_COMPLETE); + bt_conn_unref(conn); +} + static void hci_disconn_complete(struct net_buf *buf) { struct bt_hci_evt_disconn_complete *evt = (void *)buf->data; @@ -5250,7 +5272,7 @@ static void hci_event(struct net_buf *buf) hdr = net_buf_pull_mem(buf, sizeof(*hdr)); BT_DBG("event 0x%02x", hdr->evt); - BT_ASSERT(!bt_hci_evt_is_prio(hdr->evt)); + BT_ASSERT(bt_hci_evt_get_flags(hdr->evt) & BT_HCI_EVT_FLAG_RECV); handle_event(hdr->evt, buf, normal_events, ARRAY_SIZE(normal_events)); @@ -6352,6 +6374,47 @@ int bt_send(struct net_buf *buf) return bt_dev.drv->send(buf); } +static const struct event_handler prio_events[] = { + EVENT_HANDLER(BT_HCI_EVT_CMD_COMPLETE, hci_cmd_complete, + sizeof(struct bt_hci_evt_cmd_complete)), + EVENT_HANDLER(BT_HCI_EVT_CMD_STATUS, hci_cmd_status, + sizeof(struct bt_hci_evt_cmd_status)), +#if defined(CONFIG_BT_CONN) + EVENT_HANDLER(BT_HCI_EVT_DATA_BUF_OVERFLOW, + hci_data_buf_overflow, + sizeof(struct bt_hci_evt_data_buf_overflow)), + EVENT_HANDLER(BT_HCI_EVT_NUM_COMPLETED_PACKETS, + hci_num_completed_packets, + sizeof(struct bt_hci_evt_num_completed_packets)), + EVENT_HANDLER(BT_HCI_EVT_DISCONN_COMPLETE, hci_disconn_complete_prio, + sizeof(struct bt_hci_evt_disconn_complete)), + +#endif /* CONFIG_BT_CONN */ +}; + +void hci_event_prio(struct net_buf *buf) +{ + struct net_buf_simple_state state; + struct bt_hci_evt_hdr *hdr; + uint8_t evt_flags; + + net_buf_simple_save(&buf->b, &state); + + BT_ASSERT(buf->len >= sizeof(*hdr)); + + hdr = net_buf_pull_mem(buf, sizeof(*hdr)); + evt_flags = bt_hci_evt_get_flags(hdr->evt); + BT_ASSERT(evt_flags & BT_HCI_EVT_FLAG_RECV_PRIO); + + handle_event(hdr->evt, buf, prio_events, ARRAY_SIZE(prio_events)); + + if (evt_flags & BT_HCI_EVT_FLAG_RECV) { + net_buf_simple_restore(&buf->b, &state); + } else { + net_buf_unref(buf); + } +} + int bt_recv(struct net_buf *buf) { bt_monitor_send(bt_monitor_opcode(buf), buf->data, buf->len); @@ -6369,12 +6432,23 @@ int bt_recv(struct net_buf *buf) return 0; #endif /* BT_CONN */ case BT_BUF_EVT: + { #if defined(CONFIG_BT_RECV_IS_RX_THREAD) hci_event(buf); #else - net_buf_put(&bt_dev.rx_queue, buf); + struct bt_hci_evt_hdr *hdr = (void *)buf->data; + uint8_t evt_flags = bt_hci_evt_get_flags(hdr->evt); + + if (evt_flags & BT_HCI_EVT_FLAG_RECV_PRIO) { + hci_event_prio(buf); + } + + if (evt_flags & BT_HCI_EVT_FLAG_RECV) { + net_buf_put(&bt_dev.rx_queue, buf); + } #endif return 0; + } default: BT_ERR("Invalid buf type %u", bt_buf_get_type(buf)); net_buf_unref(buf); @@ -6382,39 +6456,18 @@ int bt_recv(struct net_buf *buf) } } -static const struct event_handler prio_events[] = { - EVENT_HANDLER(BT_HCI_EVT_CMD_COMPLETE, hci_cmd_complete, - sizeof(struct bt_hci_evt_cmd_complete)), - EVENT_HANDLER(BT_HCI_EVT_CMD_STATUS, hci_cmd_status, - sizeof(struct bt_hci_evt_cmd_status)), -#if defined(CONFIG_BT_CONN) - EVENT_HANDLER(BT_HCI_EVT_DATA_BUF_OVERFLOW, - hci_data_buf_overflow, - sizeof(struct bt_hci_evt_data_buf_overflow)), - EVENT_HANDLER(BT_HCI_EVT_NUM_COMPLETED_PACKETS, - hci_num_completed_packets, - sizeof(struct bt_hci_evt_num_completed_packets)), -#endif /* CONFIG_BT_CONN */ -}; - +#if defined(CONFIG_BT_RECV_IS_RX_THREAD) int bt_recv_prio(struct net_buf *buf) { - struct bt_hci_evt_hdr *hdr; - bt_monitor_send(bt_monitor_opcode(buf), buf->data, buf->len); BT_ASSERT(bt_buf_get_type(buf) == BT_BUF_EVT); - BT_ASSERT(buf->len >= sizeof(*hdr)); - hdr = net_buf_pull_mem(buf, sizeof(*hdr)); - BT_ASSERT(bt_hci_evt_is_prio(hdr->evt)); - - handle_event(hdr->evt, buf, prio_events, ARRAY_SIZE(prio_events)); - - net_buf_unref(buf); + hci_event_prio(buf); return 0; } +#endif /* defined(CONFIG_BT_RECV_IS_RX_THREAD) */ int bt_hci_driver_register(const struct bt_hci_driver *drv) { diff --git a/subsys/bluetooth/host/hci_ecc.c b/subsys/bluetooth/host/hci_ecc.c index 84b10bbc178..16f8da850b9 100644 --- a/subsys/bluetooth/host/hci_ecc.c +++ b/subsys/bluetooth/host/hci_ecc.c @@ -96,7 +96,11 @@ static void send_cmd_status(uint16_t opcode, uint8_t status) evt->opcode = sys_cpu_to_le16(opcode); evt->status = status; - bt_recv_prio(buf); + if (IS_ENABLED(CONFIG_BT_RECV_IS_RX_THREAD)) { + bt_recv_prio(buf); + } else { + bt_recv(buf); + } } static uint8_t generate_keys(void) diff --git a/subsys/bluetooth/host/hci_raw.c b/subsys/bluetooth/host/hci_raw.c index 2ef902deb0f..e3240ec05b9 100644 --- a/subsys/bluetooth/host/hci_raw.c +++ b/subsys/bluetooth/host/hci_raw.c @@ -185,6 +185,17 @@ int bt_recv(struct net_buf *buf) int bt_recv_prio(struct net_buf *buf) { + if (bt_buf_get_type(buf) == BT_BUF_EVT) { + struct bt_hci_evt_hdr *hdr = (void *)buf->data; + uint8_t evt_flags = bt_hci_evt_get_flags(hdr->evt); + + if ((evt_flags & BT_HCI_EVT_FLAG_RECV_PRIO) && + (evt_flags & BT_HCI_EVT_FLAG_RECV)) { + /* Avoid queuing the event twice */ + return 0; + } + } + return bt_recv(buf); } diff --git a/tests/bluetooth/hci_prop_evt/prj.conf b/tests/bluetooth/hci_prop_evt/prj.conf index 0f6d72f0e51..8b13eb66e23 100644 --- a/tests/bluetooth/hci_prop_evt/prj.conf +++ b/tests/bluetooth/hci_prop_evt/prj.conf @@ -4,6 +4,7 @@ CONFIG_ZTEST=y CONFIG_BT=y CONFIG_BT_CTLR=n CONFIG_BT_NO_DRIVER=y +CONFIG_BT_RECV_IS_RX_THREAD=y CONFIG_BT_HCI_VS_EVT_USER=y