Re: [PATCH v2 2/2] Bluetooth: Fix printing errors if LE Connection times out

Ahmad Fatoum
2023-03-29T09:00:04Z

Hello Luiz,

On 27.03.23 22:53, Luiz Augusto von Dentz wrote:
> From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
>
> This fixes errors like bellow when LE Connection times out since that
> is actually not a controller error:
>
> Bluetooth: hci0: Opcode 0x200d failed: -110
> Bluetooth: hci0: request failed to create LE connection: err -110
>
> Instead the code shall properly detect if -ETIMEDOUT is returned and
> send HCI_OP_LE_CREATE_CONN_CANCEL to give up on the connection.
>
> Link: https://github.com/bluez/bluez/issues/340

I assume the issue described in the Github PR to be the same issue
I had reported here:

https://lore.kernel.org/linux-bluetooth/a1ce1743-e450-6cdb-dfab-56a3e3eb9aed@pengutronix.de/

I gave these patches a test and all pairing attempts after the first pair/unpair
are still unsuccessful. Only visible change to me is that there's no -110 error
message printed anymore with default log level.

Cheers,
Ahmad

#regzbot monitor: https://lore.kernel.org/linux-bluetooth/a1ce1743-e450-6cdb-dfab-56a3e3eb9aed@pengutronix.de/

> Fixes: Fixes: 8e8b92ee60de ("Bluetooth: hci_sync: Add hci_le_create_conn_sync")
> Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
> ---
> include/net/bluetooth/hci_core.h | 1 +
> net/bluetooth/hci_conn.c | 7 +++++--
> net/bluetooth/hci_event.c | 16 ++++++----------
> net/bluetooth/hci_sync.c | 13 ++++++++++---
> 4 files changed, 22 insertions(+), 15 deletions(-)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 53d3328c2b8b..e22e45fbe8db 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -954,6 +954,7 @@ enum {
> HCI_CONN_STK_ENCRYPT,
> HCI_CONN_AUTH_INITIATOR,
> HCI_CONN_DROP,
> + HCI_CONN_CANCEL,
> HCI_CONN_PARAM_REMOVAL_PEND,
> HCI_CONN_NEW_LINK_KEY,
> HCI_CONN_SCANNING,
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index 5af3f6b011c9..e4aee5950c36 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -1233,6 +1233,8 @@ static void create_le_conn_complete(struct hci_dev *hdev, void *data, int err)
> {
> struct hci_conn *conn = data;
>
> + bt_dev_dbg(hdev, "err %d", err);
> +
> hci_dev_lock(hdev);
>
> if (!err) {
> @@ -1240,8 +1242,6 @@ static void create_le_conn_complete(struct hci_dev *hdev, void *data, int err)
> goto done;
> }
>
> - bt_dev_err(hdev, "request failed to create LE connection: err %d", err);
> -
> /* Check if connection is still pending */
> if (conn != hci_lookup_le_connect(hdev))
> goto done;
> @@ -2771,6 +2771,9 @@ int hci_abort_conn(struct hci_conn *conn, u8 reason)
> {
> int r = 0;
>
> + if (test_and_set_bit(HCI_CONN_CANCEL, &conn->flags))
> + return 0;
> +
> switch (conn->state) {
> case BT_CONNECTED:
> case BT_CONFIG:
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 8d8547fa9032..ff99e8c2750f 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -2886,16 +2886,6 @@ static void cs_le_create_conn(struct hci_dev *hdev, bdaddr_t *peer_addr,
>
> conn->resp_addr_type = peer_addr_type;
> bacpy(&conn->resp_addr, peer_addr);
> -
> - /* We don't want the connection attempt to stick around
> - * indefinitely since LE doesn't have a page timeout concept
> - * like BR/EDR. Set a timer for any connection that doesn't use
> - * the accept list for connecting.
> - */
> - if (filter_policy == HCI_LE_USE_PEER_ADDR)
> - queue_delayed_work(conn->hdev->workqueue,
> - &conn->le_conn_timeout,
> - conn->conn_timeout);
> }
>
> static void hci_cs_le_create_conn(struct hci_dev *hdev, u8 status)
> @@ -5907,6 +5897,12 @@ static void le_conn_complete_evt(struct hci_dev *hdev, u8 status,
> if (status)
> goto unlock;
>
> + /* Drop the connection if it has been aborted */
> + if (test_bit(HCI_CONN_CANCEL, &conn->flags)) {
> + hci_conn_drop(conn);
> + goto unlock;
> + }
> +
> if (conn->dst_type == ADDR_LE_DEV_PUBLIC)
> addr_type = BDADDR_LE_PUBLIC;
> else
> diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
> index 4376b6f06702..31231f0e4a28 100644
> --- a/net/bluetooth/hci_sync.c
> +++ b/net/bluetooth/hci_sync.c
> @@ -246,8 +246,9 @@ int __hci_cmd_sync_status_sk(struct hci_dev *hdev, u16 opcode, u32 plen,
>
> skb = __hci_cmd_sync_sk(hdev, opcode, plen, param, event, timeout, sk);
> if (IS_ERR(skb)) {
> - bt_dev_err(hdev, "Opcode 0x%4x failed: %ld", opcode,
> - PTR_ERR(skb));
> + if (!event)
> + bt_dev_err(hdev, "Opcode 0x%4x failed: %ld", opcode,
> + PTR_ERR(skb));
> return PTR_ERR(skb);
> }
>
> @@ -5128,8 +5129,11 @@ static int hci_le_connect_cancel_sync(struct hci_dev *hdev,
> if (test_bit(HCI_CONN_SCANNING, &conn->flags))
> return 0;
>
> + if (test_and_set_bit(HCI_CONN_CANCEL, &conn->flags))
> + return 0;
> +
> return __hci_cmd_sync_status(hdev, HCI_OP_LE_CREATE_CONN_CANCEL,
> - 6, &conn->dst, HCI_CMD_TIMEOUT);
> + 0, NULL, HCI_CMD_TIMEOUT);
> }
>
> static int hci_connect_cancel_sync(struct hci_dev *hdev, struct hci_conn *conn)
> @@ -6103,6 +6107,9 @@ int hci_le_create_conn_sync(struct hci_dev *hdev, struct hci_conn *conn)
> conn->conn_timeout, NULL);
>
> done:
> + if (err == -ETIMEDOUT)
> + hci_le_connect_cancel_sync(hdev, conn);
> +
> /* Re-enable advertising after the connection attempt is finished. */
> hci_resume_advertising_sync(hdev);
> return err;

--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |