Re: [PATCH] Bluetooth: Ignore cmd_timeout with HCI_USER_CHANNEL
From: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Date: 2022-08-08 20:32:03
Also in:
linux-bluetooth, lkml
Hi Abhishek, On Mon, Aug 8, 2022 at 11:04 AM Abhishek Pandit-Subedi [off-list ref] wrote:
From: Abhishek Pandit-Subedi <abhishekpandit@chromium.org> When using HCI_USER_CHANNEL, hci traffic is expected to be handled by userspace entirely. However, it is still possible (and sometimes desirable) to be able to send commands to the controller directly. In such cases, the kernel command timeout shouldn't do any error handling. Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org> --- This was tested by running a userchannel stack and sending commands via hcitool cmd on an Intel AX200 controller. Without this change, each command sent via hcitool would result in hci_cmd_timeout being called and after 5 commands, the controller would reset. Hcitool continues working here because it marks the socket as promiscuous and gets a copy of all traffic while the socket is open (and does filtering in userspace).
There is something not quite right here, if you have a controller using user_channel (addr.hci_channel = HCI_CHANNEL_USER) it probably shouldn't even accept to be opened again by the likes of hcitool which uses HCI_CHANNEL_RAW as it can cause conflicts. If you really need a test tool that does send the command while in HCI_CHANNEL_USER then it must be send on that mode but I wouldn't do it with hcitool anyway as that is deprecated and this exercise seem to revolve to a entire stack on top of HCI_USER_CHANNEL then you shall use tools of that stack and mix with BlueZ userspace tools.
quoted hunk ↗ jump to hunk
Tested on Chromebook with 5.4 kernel with patch (and applied cleanly on bluetooth-next). net/bluetooth/hci_core.c | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-)diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c index b3a5a3cc9372..c9a15f6633f7 100644 --- a/net/bluetooth/hci_core.c +++ b/net/bluetooth/hci_core.c@@ -1481,17 +1481,25 @@ static void hci_cmd_timeout(struct work_struct *work) struct hci_dev *hdev = container_of(work, struct hci_dev, cmd_timer.work); - if (hdev->sent_cmd) { - struct hci_command_hdr *sent = (void *) hdev->sent_cmd->data; - u16 opcode = __le16_to_cpu(sent->opcode); + /* Don't trigger the timeout behavior if it happens while we're in + * userchannel mode. Userspace is responsible for handling any command + * timeouts. + */ + if (!(hci_dev_test_flag(hdev, HCI_USER_CHANNEL) && + test_bit(HCI_UP, &hdev->flags))) { + if (hdev->sent_cmd) { + struct hci_command_hdr *sent = + (void *)hdev->sent_cmd->data; + u16 opcode = __le16_to_cpu(sent->opcode); - bt_dev_err(hdev, "command 0x%4.4x tx timeout", opcode); - } else { - bt_dev_err(hdev, "command tx timeout"); - } + bt_dev_err(hdev, "command 0x%4.4x tx timeout", opcode); + } else { + bt_dev_err(hdev, "command tx timeout"); + } - if (hdev->cmd_timeout) - hdev->cmd_timeout(hdev); + if (hdev->cmd_timeout) + hdev->cmd_timeout(hdev); + }
I wonder why hci_cmd_timeout is even active if the controller is in HCI_USER_CHANNEL mode, that sounds like a bug already.
atomic_set(&hdev->cmd_cnt, 1);
queue_work(hdev->workqueue, &hdev->cmd_work);
--
2.37.1.559.g78731f0fdb-goog-- Luiz Augusto von Dentz