Skip to content

Commit

Permalink
Bluetooth: Controller: Fix HCI command buffer allocation failure
Browse files Browse the repository at this point in the history 
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 8161430 ("Bluetooth: Add workaround
for no command buffer available")'.

Relates to commit 297f4f4 ("Bluetooth: Split HCI
command & event buffers to two pools").

Signed-off-by: Vinayak Kariappa Chettimada <[email protected]>
  • Loading branch information
@cvinayak
cvinayak committed Jan 10, 2025
1 parent bc6eded commit 3881e41
Show file tree
Hide file tree
Showing 3 changed files with 80 additions and 13 deletions.
27 changes: 25 additions & 2 deletions subsys/bluetooth/common/Kconfig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -184,11 +184,34 @@ config BT_BUF_CMD_TX_SIZE

config BT_BUF_CMD_TX_COUNT
int "Number of HCI command buffers"
default 2
range 2 64
default 2 if BT_HCI_ACL_FLOW_CONTROL
default 1
range 1 64
help
Number of buffers available for outgoing HCI commands from the Host.

HCI Controllers may not support Num_HCI_Command_Packets > 1, hence
default to 1 when not enabling Controller to Host Flow Control,
BT_HCI_ACL_FLOW_CONTROL.

Normal HCI commands follow the HCI command flow control using
Num_HCI_Command_Packets return in HCI command complete and status.

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.

When Controller to Host data flow control is enabled in the Host only
build, ensure that BT_BUF_CMD_TX_COUNT is atleast 2, to be able to

Check warning on line 206 in subsys/bluetooth/common/Kconfig

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

TYPO_SPELLING 

subsys/bluetooth/common/Kconfig:206  'atleast' may be misspelled - perhaps 'at least'?
send one normal HCI command and one Host Number of Completed Packets
command.

When Controller to Host data flow control is supported in the
Controller only build, ensure that BT_BUF_CMD_TX_COUNT is greater than
or equal to (BT_BUF_ACL_RX_COUNT + Ncmd), where Ncmd is supported
maximum Num_HCI_Command_Packets in the Controller implementation.

endmenu

# Workaround to have commas on function arguments
Expand Down
46 changes: 35 additions & 11 deletions subsys/bluetooth/controller/hci/hci.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
}
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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);
}
20 changes: 20 additions & 0 deletions subsys/bluetooth/host/hci_common.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down

0 comments on commit 3881e41

Please sign in to comment.