| Message ID | 20240206110816.74995-1-verdre@v0yd.nl (mailing list archive) |
|---|---|
| Headers | show |
| Series | Bluetooth: Improve retrying of connection attempts | expand |
Hi Jonas, On Tue, Feb 6, 2024 at 6:08 AM Jonas Dreßler <verdre@v0yd.nl> wrote: > > Since commit 4c67bc74f016 ("[Bluetooth] Support concurrent connect > requests"), the kernel supports trying to connect again in case the > bluetooth card is busy and fails to connect. > > The logic that should handle this became a bit spotty over time, and also > cards these days appear to fail with more errors than just "Command > Disallowed". > > This series refactores the handling of concurrent connection requests > by serializing all "Create Connection" commands for ACL connections > similar to how we do it for LE connections. > > --- > > v1: https://lore.kernel.org/linux-bluetooth/20240102185933.64179-1-verdre@v0yd.nl/ > v2: https://lore.kernel.org/linux-bluetooth/20240108183938.468426-1-verdre@v0yd.nl/ > v3: https://lore.kernel.org/linux-bluetooth/20240108224614.56900-1-verdre@v0yd.nl/ > v4: > - Removed first two commits since they are already applied > - Removed a BT_DBG() message in the acl_create_connection() function > while moving to hci_sync because it seemed out of place in hci_sync > - Added a mention of the test failure in mgmt-tester to commit message > > Jonas Dreßler (2): > Bluetooth: hci_conn: Only do ACL connections sequentially > Bluetooth: Remove pending ACL connection attempts > > include/net/bluetooth/hci.h | 1 + > include/net/bluetooth/hci_core.h | 1 - > include/net/bluetooth/hci_sync.h | 3 ++ > net/bluetooth/hci_conn.c | 83 +++----------------------------- > net/bluetooth/hci_event.c | 21 ++------ > net/bluetooth/hci_sync.c | 70 +++++++++++++++++++++++++++ > 6 files changed, 86 insertions(+), 93 deletions(-) > > -- > 2.43.0 Doesn't seem to work with the new test: Sequential connect - setup complete Sequential connect - run Create connection finished Connect failed for Pair Device Create connection finished Sequential connect - test timed out Sequential connect - teardown
Hi Luiz, On 06.02.24 23:22, Luiz Augusto von Dentz wrote: > Hi Jonas, > > On Tue, Feb 6, 2024 at 6:08 AM Jonas Dreßler <verdre@v0yd.nl> wrote: >> >> Since commit 4c67bc74f016 ("[Bluetooth] Support concurrent connect >> requests"), the kernel supports trying to connect again in case the >> bluetooth card is busy and fails to connect. >> >> The logic that should handle this became a bit spotty over time, and also >> cards these days appear to fail with more errors than just "Command >> Disallowed". >> >> This series refactores the handling of concurrent connection requests >> by serializing all "Create Connection" commands for ACL connections >> similar to how we do it for LE connections. >> >> --- >> >> v1: https://lore.kernel.org/linux-bluetooth/20240102185933.64179-1-verdre@v0yd.nl/ >> v2: https://lore.kernel.org/linux-bluetooth/20240108183938.468426-1-verdre@v0yd.nl/ >> v3: https://lore.kernel.org/linux-bluetooth/20240108224614.56900-1-verdre@v0yd.nl/ >> v4: >> - Removed first two commits since they are already applied >> - Removed a BT_DBG() message in the acl_create_connection() function >> while moving to hci_sync because it seemed out of place in hci_sync >> - Added a mention of the test failure in mgmt-tester to commit message >> >> Jonas Dreßler (2): >> Bluetooth: hci_conn: Only do ACL connections sequentially >> Bluetooth: Remove pending ACL connection attempts >> >> include/net/bluetooth/hci.h | 1 + >> include/net/bluetooth/hci_core.h | 1 - >> include/net/bluetooth/hci_sync.h | 3 ++ >> net/bluetooth/hci_conn.c | 83 +++----------------------------- >> net/bluetooth/hci_event.c | 21 ++------ >> net/bluetooth/hci_sync.c | 70 +++++++++++++++++++++++++++ >> 6 files changed, 86 insertions(+), 93 deletions(-) >> >> -- >> 2.43.0 > > > Doesn't seem to work with the new test: > > Sequential connect - setup complete > Sequential connect - run > Create connection finished > Connect failed for Pair Device > Create connection finished > Sequential connect - test timed out > Sequential connect - teardown > Oh you're right, it's because of the increased delay to 5.12 seconds for sending the page timeout. We're actually waiting for connections to fail, so the test will now take at least 5.12s * 3 instead of 2s * 3, which means test timeout needs to be increased from 7 to more like 16 seconds. Sorry for not catching this one, should've run the test before submitting the patch... Cheers, Jonas
Hello: This series was applied to bluetooth/bluetooth-next.git (master) by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>: On Tue, 6 Feb 2024 12:08:12 +0100 you wrote: > Since commit 4c67bc74f016 ("[Bluetooth] Support concurrent connect > requests"), the kernel supports trying to connect again in case the > bluetooth card is busy and fails to connect. > > The logic that should handle this became a bit spotty over time, and also > cards these days appear to fail with more errors than just "Command > Disallowed". > > [...] Here is the summary with links: - [v4,1/2] Bluetooth: hci_conn: Only do ACL connections sequentially https://git.kernel.org/bluetooth/bluetooth-next/c/456561ba8e49 - [v4,2/2] Bluetooth: Remove pending ACL connection attempts https://git.kernel.org/bluetooth/bluetooth-next/c/8e14d581d125 You are awesome, thank you!
Hi Jonas, On Wed, Feb 7, 2024 at 11:10 AM <patchwork-bot+bluetooth@kernel.org> wrote: > > Hello: > > This series was applied to bluetooth/bluetooth-next.git (master) > by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>: > > On Tue, 6 Feb 2024 12:08:12 +0100 you wrote: > > Since commit 4c67bc74f016 ("[Bluetooth] Support concurrent connect > > requests"), the kernel supports trying to connect again in case the > > bluetooth card is busy and fails to connect. > > > > The logic that should handle this became a bit spotty over time, and also > > cards these days appear to fail with more errors than just "Command > > Disallowed". > > > > [...] > > Here is the summary with links: > - [v4,1/2] Bluetooth: hci_conn: Only do ACL connections sequentially > https://git.kernel.org/bluetooth/bluetooth-next/c/456561ba8e49 > - [v4,2/2] Bluetooth: Remove pending ACL connection attempts > https://git.kernel.org/bluetooth/bluetooth-next/c/8e14d581d125 > > You are awesome, thank you! > -- > Deet-doot-dot, I am a bot. > https://korg.docs.kernel.org/patchwork/pwbot.html Something is not quite right with these changes: L2CAP BR/EDR Client - Timeout - test passed L2CAP BR/EDR Client - Timeout - teardown Bluetooth: hci0: command 0x0408 tx timeout Index Removed callback Index: 0x0000 L2CAP BR/EDR Client - Timeout - teardown complete L2CAP BR/EDR Client - Timeout - done L2CAP BR/EDR Client - Timeout Passed 22.329 seconds The test seems to be blocked on teardown then the command times out, 0x0408 is HCI_OP_CREATE_CONN_CANCEL so I wonder why it would be timing out. Anyway the test uses SO_SNDTIMEO to set a timeout at the socket level but it doesn't seem it is used while creating the connection so it probably needs to be passed down or something.
Since commit 4c67bc74f016 ("[Bluetooth] Support concurrent connect requests"), the kernel supports trying to connect again in case the bluetooth card is busy and fails to connect. The logic that should handle this became a bit spotty over time, and also cards these days appear to fail with more errors than just "Command Disallowed". This series refactores the handling of concurrent connection requests by serializing all "Create Connection" commands for ACL connections similar to how we do it for LE connections. --- v1: https://lore.kernel.org/linux-bluetooth/20240102185933.64179-1-verdre@v0yd.nl/ v2: https://lore.kernel.org/linux-bluetooth/20240108183938.468426-1-verdre@v0yd.nl/ v3: https://lore.kernel.org/linux-bluetooth/20240108224614.56900-1-verdre@v0yd.nl/ v4: - Removed first two commits since they are already applied - Removed a BT_DBG() message in the acl_create_connection() function while moving to hci_sync because it seemed out of place in hci_sync - Added a mention of the test failure in mgmt-tester to commit message Jonas Dreßler (2): Bluetooth: hci_conn: Only do ACL connections sequentially Bluetooth: Remove pending ACL connection attempts include/net/bluetooth/hci.h | 1 + include/net/bluetooth/hci_core.h | 1 - include/net/bluetooth/hci_sync.h | 3 ++ net/bluetooth/hci_conn.c | 83 +++----------------------------- net/bluetooth/hci_event.c | 21 ++------ net/bluetooth/hci_sync.c | 70 +++++++++++++++++++++++++++ 6 files changed, 86 insertions(+), 93 deletions(-)