From e9f6c00b05bcd3af318f10f85e83a7b7f736fcac Mon Sep 17 00:00:00 2001 From: Vinayak Kariappa Chettimada Date: Fri, 10 Jan 2025 06:19:47 +0100 Subject: [PATCH] Bluetooth: Controller: Fix HCI command buffer allocation failure Fix HCI command buffer allocation failure, that can cause loss of Host Number of Completed Packets command. Fail by rejecting the HCI Host Buffer Size command if the required number of HCI command buffers are not allocated in the Controller implementation. Relates to commit 81614307e9fe ("Bluetooth: Add workaround for no command buffer available")'. Signed-off-by: Vinayak Kariappa Chettimada --- subsys/bluetooth/controller/hci/hci.c | 46 ++++++++++++++++++++------- subsys/bluetooth/host/hci_common.c | 20 ++++++++++++ 2 files changed, 55 insertions(+), 11 deletions(-) diff --git a/subsys/bluetooth/controller/hci/hci.c b/subsys/bluetooth/controller/hci/hci.c index b952092c8857e6c..10666cd5ac0ba09 100644 --- a/subsys/bluetooth/controller/hci/hci.c +++ b/subsys/bluetooth/controller/hci/hci.c @@ -184,13 +184,13 @@ static uint8_t sf_curr; #endif #if defined(CONFIG_BT_HCI_ACL_FLOW_CONTROL) -int32_t hci_hbuf_total; -uint32_t hci_hbuf_sent; -uint32_t hci_hbuf_acked; -uint16_t hci_hbuf_pend[CONFIG_BT_MAX_CONN]; +int32_t hci_hbuf_total; +uint32_t hci_hbuf_sent; +uint32_t hci_hbuf_acked; +uint16_t hci_hbuf_pend[CONFIG_BT_MAX_CONN]; atomic_t hci_state_mask; static struct k_poll_signal *hbuf_signal; -#endif +#endif /* CONFIG_BT_HCI_ACL_FLOW_CONTROL */ #if defined(CONFIG_BT_CONN) static uint32_t conn_count; @@ -475,7 +475,7 @@ static void reset(struct net_buf *buf, struct net_buf **evt) atomic_set_bit(&hci_state_mask, HCI_STATE_BIT_RESET); k_poll_signal_raise(hbuf_signal, 0x0); } -#endif +#endif /* CONFIG_BT_HCI_ACL_FLOW_CONTROL */ hci_recv_fifo_reset(); } @@ -536,10 +536,33 @@ static void host_buffer_size(struct net_buf *buf, struct net_buf **evt) ccst->status = BT_HCI_ERR_CMD_DISALLOWED; return; } - /* fragmentation from controller to host not supported, require + + /* Fragmentation from Controller to Host not supported, require * ACL MTU to be at least the LL MTU */ if (acl_mtu < LL_LENGTH_OCTETS_RX_MAX) { + LOG_ERR("FC: Require Host ACL MTU (%u) >= LL Max Data Length (%u)", acl_mtu, + LL_LENGTH_OCTETS_RX_MAX); + ccst->status = BT_HCI_ERR_INVALID_PARAM; + return; + } + + /* Host Number of Completed Packets command does not follow normal flow + * control of HCI commands and the Controller side HCI drivers that + * allocates HCI command buffers with K_NO_WAIT can end up running out + * of command buffers. + * + * Host will generate up to acl_pkts number of Host Number of Completed + * Packets command plus a number of normal HCI commands. + * + * Normal HCI commands follow the HCI command flow control using + * Num_HCI_Command_Packets return in HCI command complete and status. + * + * Note: Zephyr Controller does not support Num_HCI_Command_Packets > 1. + */ + if (acl_pkts >= CONFIG_BT_BUF_CMD_TX_COUNT) { + LOG_ERR("FC: Require Host ACL packets (%u) < CONFIG_BT_BUF_CMD_TX_COUNT (%u)", + acl_pkts, CONFIG_BT_BUF_CMD_TX_COUNT); ccst->status = BT_HCI_ERR_INVALID_PARAM; return; } @@ -586,7 +609,7 @@ static void host_num_completed_packets(struct net_buf *buf, hci_hbuf_acked += count; k_poll_signal_raise(hbuf_signal, 0x0); } -#endif +#endif /* CONFIG_BT_HCI_ACL_FLOW_CONTROL */ #if defined(CONFIG_BT_CTLR_LE_PING) static void read_auth_payload_timeout(struct net_buf *buf, struct net_buf **evt) @@ -747,7 +770,7 @@ static int ctrl_bb_cmd_handle(uint16_t ocf, struct net_buf *cmd, case BT_OCF(BT_HCI_OP_HOST_NUM_COMPLETED_PACKETS): host_num_completed_packets(cmd, evt); break; -#endif +#endif /* CONFIG_BT_HCI_ACL_FLOW_CONTROL */ #if defined(CONFIG_BT_CTLR_LE_PING) case BT_OCF(BT_HCI_OP_READ_AUTH_PAYLOAD_TIMEOUT): @@ -9094,7 +9117,7 @@ void hci_acl_encode(struct node_rx_pdu *node_rx, struct net_buf *buf) LL_ASSERT(handle < ARRAY_SIZE(hci_hbuf_pend)); hci_hbuf_pend[handle]++; } -#endif +#endif /* CONFIG_BT_HCI_ACL_FLOW_CONTROL */ break; default: @@ -9300,6 +9323,7 @@ void hci_init(struct k_poll_signal *signal_host_buf) { #if defined(CONFIG_BT_HCI_ACL_FLOW_CONTROL) hbuf_signal = signal_host_buf; -#endif +#endif /* CONFIG_BT_HCI_ACL_FLOW_CONTROL */ + reset(NULL, NULL); } diff --git a/subsys/bluetooth/host/hci_common.c b/subsys/bluetooth/host/hci_common.c index 1f382075d05f07c..bae131e759db3ab 100644 --- a/subsys/bluetooth/host/hci_common.c +++ b/subsys/bluetooth/host/hci_common.c @@ -33,7 +33,17 @@ struct net_buf *bt_hci_cmd_complete_create(uint16_t op, uint8_t plen) buf = bt_hci_evt_create(BT_HCI_EVT_CMD_COMPLETE, sizeof(*cc) + plen); cc = net_buf_add(buf, sizeof(*cc)); + + /* The Num_HCI_Command_Packets parameter allows the Controller to + * indicate the number of HCI command packets the Host can send to the + * Controller. If the Controller requires the Host to stop sending + * commands, Num_HCI_Command_Packets will be set to zero. + * + * NOTE: Zephyr Controller (and may be other Controllers) do not support + * higher Number of HCI Command packets than 1. + */ cc->ncmd = 1U; + cc->opcode = sys_cpu_to_le16(op); return buf; @@ -48,7 +58,17 @@ struct net_buf *bt_hci_cmd_status_create(uint16_t op, uint8_t status) cs = net_buf_add(buf, sizeof(*cs)); cs->status = status; + + /* The Num_HCI_Command_Packets parameter allows the Controller to + * indicate the number of HCI command packets the Host can send to the + * Controller. If the Controller requires the Host to stop sending + * commands, Num_HCI_Command_Packets will be set to zero. + * + * NOTE: Zephyr Controller (and may be other Controllers) do not support + * higher Number of HCI Command packets than 1. + */ cs->ncmd = 1U; + cs->opcode = sys_cpu_to_le16(op); return buf;