Daemon crashes if HCI commands are issued over USB too fast

[GoogleCodeExporter]

What steps will reproduce the problem?
1. Use daemon with libusb transport
2. Send a command at the wrong time

What is the expected output? What do you see instead?
Daemon crashes with "Error submitting control transfer -6"

What version of the product are you using? On what operating system?
svn trunk on Linux

The error message comes from platforms/posix/src/hci_transport_h2_libusb.c in 
usb_send_cmd_packet.
It is informative to move the hci_dump_packet() call in this function to 
*before* the error/return; I can submit a patch if you like.

I note that the stack normally never issues two CMDs in a row without waiting 
for a response. In this particular example, the user packet handler seems to 
receive a PIN request before the connection create command is complete. There 
are a number of other ways to trigger the issue (eg. from 
HCI_EVENT_CONNECTION_COMPLETE event handling).

An example log in this case ends with:
[2014-10-16 12:20:20.038] LOG -- L2CAP_CREATE_CHANNEL_MTU addr 
00:0A:95:04:93:23 psm 0x13 mtu 150
[2014-10-16 12:20:20.038] LOG -- Create_connection to 00:0A:95:04:93:23
[2014-10-16 12:20:20.038] LOG -- create_connection_for_addr 00:0A:95:04:93:23
[2014-10-16 12:20:20.038] LOG -- conn state 0
[2014-10-16 12:20:20.038] CMD => 05 04 0D 23 93 04 95 0A 00 18 CC 00 00 00 00 
01 
[2014-10-16 12:20:20.038] CMD => 0D 04 17 23 93 04 95 0A 00 04 30 30 30 30 00 
6C 69 6E 6B 20 6B 65 79 3A 20 25 
[2014-10-16 12:20:20.039] LOG -- Error submitting control transfer -6

Original issue reported on code.google.com by soul.cak...@gmail.com on 16 Oct 2014 at 1:20

[GoogleCodeExporter]

I should note that this particular trigger was delivered by sending 
hci_pin_code_request_reply from the HCI_EVENT_COMMAND_COMPLETE handler by 
accident. Would a saner test case be helpful?

Original comment by soul.cak...@gmail.com on 16 Oct 2014 at 1:23

[GoogleCodeExporter]

OK, this happens because daemon.c calls hci_send_cmd_packet(), without first 
checking hci_can_send_command_packet_now().

The question is - what's the right thing to do? Can the command(s) be queued 
somehow and issued later? It would be nice not to have to push the issue into 
the client.

Original comment by soul.cak...@gmail.com on 16 Oct 2014 at 12:24

[GoogleCodeExporter]

hi

thanks for pointing that one out. I though I'd added checks for can_send_now 
everywhere.
In the embedded version, this has to be handled by the app itself, which is 
necessary to save RAM.
For the daemon, it cannot be handled by the client, as multiple clients 
potentially could send an HCI command at the very same time.

the daemon already has a dirty hack to "park" client connections if the current 
packet cannot be handled.

if you like to try, you can add
                if (!hci_can_send_command_packet_now) return BTSTACK_ACL_BUFFERS_FULL;
before the hci_send_cmd_packet() call and let me know if that helps.

Original comment by matthias.ringwald@gmail.com on 16 Oct 2014 at 1:14

[GoogleCodeExporter]

It certainly stops the crashes, of course! The client doesn't get terminally 
confused either, so it's not getting stuck in park. But it's also not clear 
that the original command is being run; I'll take another look tomorrow.

Original comment by soul.cak...@gmail.com on 16 Oct 2014 at 2:04

[GoogleCodeExporter]

This patch has been working well for me. Checks for parked connections when HCI 
commands complete, too.

Original comment by soul.cak...@gmail.com on 22 Oct 2014 at 12:00

Attachments:

• daemon-park-hci.patch

[GoogleCodeExporter]

Previous patch used return instead of break and prevented clients from 
receiving command complete events! Fixed here.

Original comment by soul.cak...@gmail.com on 22 Oct 2014 at 3:31

Attachments:

• daemon-park-hci_v2.patch