From patchwork Tue Jan 2 18:59:28 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Jonas_Dre=C3=9Fler?= X-Patchwork-Id: 759453 Received: from mout-p-202.mailbox.org (mout-p-202.mailbox.org [80.241.56.172]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 88FAC15AD3; Tue, 2 Jan 2024 18:59:41 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=v0yd.nl Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=v0yd.nl Received: from smtp1.mailbox.org (smtp1.mailbox.org [IPv6:2001:67c:2050:b231:465::1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by mout-p-202.mailbox.org (Postfix) with ESMTPS id 4T4MbY5VtLz9sb4; Tue, 2 Jan 2024 19:59:37 +0100 (CET) From: =?utf-8?q?Jonas_Dre=C3=9Fler?= To: Marcel Holtmann , Johan Hedberg , Luiz Augusto von Dentz Cc: =?utf-8?q?Jonas_Dre=C3=9Fler?= , linux-bluetooth@vger.kernel.org, linux-kernel@vger.kernel.org, netdev@vger.kernel.org Subject: [PATCH 1/5] Bluetooth: Remove superfluous call to hci_conn_check_pending() Date: Tue, 2 Jan 2024 19:59:28 +0100 Message-ID: <20240102185933.64179-2-verdre@v0yd.nl> In-Reply-To: <20240102185933.64179-1-verdre@v0yd.nl> References: <20240102185933.64179-1-verdre@v0yd.nl> Precedence: bulk X-Mailing-List: linux-bluetooth@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Rspamd-Queue-Id: 4T4MbY5VtLz9sb4 The "pending connections" feature was originally introduced with commit 4c67bc74f016b0d360b8573e18969c0ff7926974 and 6bd57416127e92d35e6798925502c84e14a3a966 to handle controllers supporting only a single connection request at a time. Later things were extended to also cancel ongoing inquiries on connect() with commit 89e65975fea5c25706e8cc3a89f9f97b20fc45ad. With commit a9de9248064bfc8eb0a183a6a951a4e7b5ca10a4, hci_conn_check_pending() was introduced as a helper to consolidate a few places where we check for pending connections (indicated by the BT_CONNECT2 flag) and then try to connect. This refactoring commit also snuck in two more calls to hci_conn_check_pending(): - One is in the failure callback of hci_cs_inquiry(), this one probably makes sense: If we send an "HCI Inquiry" command and then immediately after a "Create Connection" command, the "Create Connection" command might fail before the "HCI Inquiry" command, and then we want to retry the "Create Connection" on failure of the "HCI Inquiry". - The other added call to hci_conn_check_pending() is in the event handler for the "Remote Name" event, this seems unrelated and is possibly a copy-paste error, so remove that one. Fixes: a9de9248064bfc8eb0a183a6a951a4e7b5ca10a4 Signed-off-by: Jonas Dreßler --- net/bluetooth/hci_event.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c index 31ca320ce..13396329f 100644 --- a/net/bluetooth/hci_event.c +++ b/net/bluetooth/hci_event.c @@ -3538,8 +3538,6 @@ static void hci_remote_name_evt(struct hci_dev *hdev, void *data, bt_dev_dbg(hdev, "status 0x%2.2x", ev->status); - hci_conn_check_pending(hdev); - hci_dev_lock(hdev); conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK, &ev->bdaddr); From patchwork Tue Jan 2 18:59:29 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Jonas_Dre=C3=9Fler?= X-Patchwork-Id: 759731 Received: from mout-p-201.mailbox.org (mout-p-201.mailbox.org [80.241.56.171]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 41C2D15E97; Tue, 2 Jan 2024 18:59:43 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=v0yd.nl Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=v0yd.nl Received: from smtp1.mailbox.org (smtp1.mailbox.org [10.196.197.1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by mout-p-201.mailbox.org (Postfix) with ESMTPS id 4T4Mbb1SQzz9sq5; Tue, 2 Jan 2024 19:59:39 +0100 (CET) From: =?utf-8?q?Jonas_Dre=C3=9Fler?= To: Marcel Holtmann , Johan Hedberg , Luiz Augusto von Dentz Cc: =?utf-8?q?Jonas_Dre=C3=9Fler?= , linux-bluetooth@vger.kernel.org, linux-kernel@vger.kernel.org, netdev@vger.kernel.org Subject: [PATCH 2/5] Bluetooth: hci_event: Use HCI error defines instead of magic values Date: Tue, 2 Jan 2024 19:59:29 +0100 Message-ID: <20240102185933.64179-3-verdre@v0yd.nl> In-Reply-To: <20240102185933.64179-1-verdre@v0yd.nl> References: <20240102185933.64179-1-verdre@v0yd.nl> Precedence: bulk X-Mailing-List: linux-bluetooth@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Signed-off-by: Jonas Dreßler --- include/net/bluetooth/hci.h | 2 ++ net/bluetooth/hci_event.c | 8 ++++---- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h index 111e8f8e5..fef723afd 100644 --- a/include/net/bluetooth/hci.h +++ b/include/net/bluetooth/hci.h @@ -641,6 +641,7 @@ enum { #define HCI_ERROR_PIN_OR_KEY_MISSING 0x06 #define HCI_ERROR_MEMORY_EXCEEDED 0x07 #define HCI_ERROR_CONNECTION_TIMEOUT 0x08 +#define HCI_ERROR_COMMAND_DISALLOWED 0x0c #define HCI_ERROR_REJ_LIMITED_RESOURCES 0x0d #define HCI_ERROR_REJ_BAD_ADDR 0x0f #define HCI_ERROR_INVALID_PARAMETERS 0x12 @@ -649,6 +650,7 @@ enum { #define HCI_ERROR_REMOTE_POWER_OFF 0x15 #define HCI_ERROR_LOCAL_HOST_TERM 0x16 #define HCI_ERROR_PAIRING_NOT_ALLOWED 0x18 +#define HCI_ERROR_UNSUPPORTED_REMOTE_FEATURE 0x1e #define HCI_ERROR_INVALID_LL_PARAMS 0x1e #define HCI_ERROR_UNSPECIFIED 0x1f #define HCI_ERROR_ADVERTISING_TIMEOUT 0x3c diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c index 13396329f..e8b4a0126 100644 --- a/net/bluetooth/hci_event.c +++ b/net/bluetooth/hci_event.c @@ -92,11 +92,11 @@ static u8 hci_cc_inquiry_cancel(struct hci_dev *hdev, void *data, /* It is possible that we receive Inquiry Complete event right * before we receive Inquiry Cancel Command Complete event, in * which case the latter event should have status of Command - * Disallowed (0x0c). This should not be treated as error, since + * Disallowed. This should not be treated as error, since * we actually achieve what Inquiry Cancel wants to achieve, * which is to end the last Inquiry session. */ - if (rp->status == 0x0c && !test_bit(HCI_INQUIRY, &hdev->flags)) { + if (rp->status == HCI_ERROR_COMMAND_DISALLOWED && !test_bit(HCI_INQUIRY, &hdev->flags)) { bt_dev_warn(hdev, "Ignoring error of Inquiry Cancel command"); rp->status = 0x00; } @@ -2323,7 +2323,7 @@ static void hci_cs_create_conn(struct hci_dev *hdev, __u8 status) if (status) { if (conn && conn->state == BT_CONNECT) { - if (status != 0x0c || conn->attempt > 2) { + if (status != HCI_ERROR_COMMAND_DISALLOWED || conn->attempt > 2) { conn->state = BT_CLOSED; hci_connect_cfm(conn, status); hci_conn_del(conn); @@ -6578,7 +6578,7 @@ static void hci_le_remote_feat_complete_evt(struct hci_dev *hdev, void *data, * transition into connected state and mark it as * successful. */ - if (!conn->out && ev->status == 0x1a && + if (!conn->out && ev->status == HCI_ERROR_UNSUPPORTED_REMOTE_FEATURE && (hdev->le_features[0] & HCI_LE_PERIPHERAL_FEATURES)) status = 0x00; else From patchwork Tue Jan 2 18:59:30 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Jonas_Dre=C3=9Fler?= X-Patchwork-Id: 759452 Received: from mout-p-101.mailbox.org (mout-p-101.mailbox.org [80.241.56.151]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 5A3F516415; Tue, 2 Jan 2024 18:59:44 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=v0yd.nl Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=v0yd.nl Received: from smtp1.mailbox.org (smtp1.mailbox.org [IPv6:2001:67c:2050:b231:465::1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by mout-p-101.mailbox.org (Postfix) with ESMTPS id 4T4Mbc4QNnz9sQb; Tue, 2 Jan 2024 19:59:40 +0100 (CET) From: =?utf-8?q?Jonas_Dre=C3=9Fler?= To: Marcel Holtmann , Johan Hedberg , Luiz Augusto von Dentz Cc: =?utf-8?q?Jonas_Dre=C3=9Fler?= , linux-bluetooth@vger.kernel.org, linux-kernel@vger.kernel.org, netdev@vger.kernel.org Subject: [PATCH 3/5] Bluetooth: hci_event: Remove limit of 2 reconnection attempts Date: Tue, 2 Jan 2024 19:59:30 +0100 Message-ID: <20240102185933.64179-4-verdre@v0yd.nl> In-Reply-To: <20240102185933.64179-1-verdre@v0yd.nl> References: <20240102185933.64179-1-verdre@v0yd.nl> Precedence: bulk X-Mailing-List: linux-bluetooth@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Rspamd-Queue-Id: 4T4Mbc4QNnz9sQb Since commit 4c67bc74f016b0d360b8573e18969c0ff7926974, we retry connecting later when we get a "Command Disallowed" error returned by "Create Connection". In this commit the intention was to retry only once, and give up if we see "Command Disallowed" again on the second try. This made sense back then when the retry was initiated *only* from the "Connect Complete" event. If we received that event, we knew that now the card now must have a "free slot" for a new connection request again. These days we call hci_conn_check_pending() from a few more places though, and in these places we can't really be sure that there's a "free slot" on the card, so the second try to "Create Connection" might fail again. Deal with this by being less strict about these retries and try again every time we get "Command Disallowed" errors, removing the limitation to only two attempts. Since this can potentially cause us to enter an endless cycle of reconnection attempts, we'll add some guarding against that with the next commit. Signed-off-by: Jonas Dreßler --- net/bluetooth/hci_event.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c index e8b4a0126..e1f5b6f90 100644 --- a/net/bluetooth/hci_event.c +++ b/net/bluetooth/hci_event.c @@ -2323,12 +2323,13 @@ static void hci_cs_create_conn(struct hci_dev *hdev, __u8 status) if (status) { if (conn && conn->state == BT_CONNECT) { - if (status != HCI_ERROR_COMMAND_DISALLOWED || conn->attempt > 2) { + if (status == HCI_ERROR_COMMAND_DISALLOWED) { + conn->state = BT_CONNECT2; + } else { conn->state = BT_CLOSED; hci_connect_cfm(conn, status); hci_conn_del(conn); - } else - conn->state = BT_CONNECT2; + } } } else { if (!conn) { From patchwork Tue Jan 2 18:59:31 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Jonas_Dre=C3=9Fler?= X-Patchwork-Id: 759730 Received: from mout-p-201.mailbox.org (mout-p-201.mailbox.org [80.241.56.171]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 7F0C616433; Tue, 2 Jan 2024 18:59:45 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=v0yd.nl Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=v0yd.nl Received: from smtp1.mailbox.org (smtp1.mailbox.org [10.196.197.1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by mout-p-201.mailbox.org (Postfix) with ESMTPS id 4T4Mbd6BVcz9smm; Tue, 2 Jan 2024 19:59:41 +0100 (CET) From: =?utf-8?q?Jonas_Dre=C3=9Fler?= To: Marcel Holtmann , Johan Hedberg , Luiz Augusto von Dentz Cc: =?utf-8?q?Jonas_Dre=C3=9Fler?= , linux-bluetooth@vger.kernel.org, linux-kernel@vger.kernel.org, netdev@vger.kernel.org Subject: [PATCH 4/5] Bluetooth: hci_event: Do sanity checks before retrying to connect Date: Tue, 2 Jan 2024 19:59:31 +0100 Message-ID: <20240102185933.64179-5-verdre@v0yd.nl> In-Reply-To: <20240102185933.64179-1-verdre@v0yd.nl> References: <20240102185933.64179-1-verdre@v0yd.nl> Precedence: bulk X-Mailing-List: linux-bluetooth@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 When we receive "Command Disallowed" response to HCI_CREATE_CONNECTION, we'll try to connect again later, assuming that the command failed either because there's already concurrent "Create Connection" requests on the card and all "slots" for new connections are exhausted, or the card is in the middle of doing an HCI Inquiry. Both of those conditions we should know about, so do some sanity checking to ensure one of them actually applies. If they don't, log an error and delete the connection. Signed-off-by: Jonas Dreßler --- net/bluetooth/hci_event.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c index e1f5b6f90..1376092c5 100644 --- a/net/bluetooth/hci_event.c +++ b/net/bluetooth/hci_event.c @@ -2323,8 +2323,28 @@ static void hci_cs_create_conn(struct hci_dev *hdev, __u8 status) if (status) { if (conn && conn->state == BT_CONNECT) { + /* If the request failed with "Command Disallowed", the + * card is either using all its available "slots" for + * attempting new connections, or it's currently + * doing an HCI Inquiry. In these cases we'll try to + * do the "Create Connection" request again later. + */ if (status == HCI_ERROR_COMMAND_DISALLOWED) { conn->state = BT_CONNECT2; + + if (!hci_conn_hash_lookup_state(hdev, ACL_LINK, BT_CONNECT) && + !test_bit(HCI_INQUIRY, &hdev->flags)) { + bt_dev_err(hdev, + "\"Create Connection\" returned error " + "(0x%2.2x) indicating to try again, but " + "there's no concurrent \"Create " + "Connection\" nor an ongoing inquiry", + status); + + conn->state = BT_CLOSED; + hci_connect_cfm(conn, status); + hci_conn_del(conn); + } } else { conn->state = BT_CLOSED; hci_connect_cfm(conn, status); From patchwork Tue Jan 2 18:59:32 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Jonas_Dre=C3=9Fler?= X-Patchwork-Id: 759451 Received: from mout-p-101.mailbox.org (mout-p-101.mailbox.org [80.241.56.151]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id DC09D168DF; Tue, 2 Jan 2024 18:59:46 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=v0yd.nl Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=v0yd.nl Received: from smtp1.mailbox.org (smtp1.mailbox.org [IPv6:2001:67c:2050:b231:465::1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by mout-p-101.mailbox.org (Postfix) with ESMTPS id 4T4Mbg1nPPz9sp4; Tue, 2 Jan 2024 19:59:43 +0100 (CET) From: =?utf-8?q?Jonas_Dre=C3=9Fler?= To: Marcel Holtmann , Johan Hedberg , Luiz Augusto von Dentz Cc: =?utf-8?q?Jonas_Dre=C3=9Fler?= , linux-bluetooth@vger.kernel.org, linux-kernel@vger.kernel.org, netdev@vger.kernel.org Subject: [PATCH 5/5] Bluetooth: hci_event: Try reconnecting on more kinds of errors Date: Tue, 2 Jan 2024 19:59:32 +0100 Message-ID: <20240102185933.64179-6-verdre@v0yd.nl> In-Reply-To: <20240102185933.64179-1-verdre@v0yd.nl> References: <20240102185933.64179-1-verdre@v0yd.nl> Precedence: bulk X-Mailing-List: linux-bluetooth@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Rspamd-Queue-Id: 4T4Mbg1nPPz9sp4 While some hardware seems to return "HCI Command Disallowed" errors when trying to connect to too many devices at once, other hardware (eg. the BCM4378 found in M1 macbooks) returns "HCI Hardware Failure" in this case. And the Marvell 88W8897 in various Microsoft Surface devices behaves different again: Here the "HCI Create Connection" succeeds, but later a "HCI Connection Complete" event with status "Rejected Limited Resources" comes in. Handle all these cases as expected by userspace and reuse the existing BT_CONNECT2 logic to try "HCI Create Connection" again after the ongoing connection attempts have been completed. Signed-off-by: Jonas Dreßler --- include/net/bluetooth/hci.h | 1 + net/bluetooth/hci_event.c | 26 +++++++++++++++++++++++--- 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h index fef723afd..23890f53e 100644 --- a/include/net/bluetooth/hci.h +++ b/include/net/bluetooth/hci.h @@ -637,6 +637,7 @@ enum { /* ---- HCI Error Codes ---- */ #define HCI_ERROR_UNKNOWN_CONN_ID 0x02 +#define HCI_ERROR_HARDWARE_FAILURE 0x03 #define HCI_ERROR_AUTH_FAILURE 0x05 #define HCI_ERROR_PIN_OR_KEY_MISSING 0x06 #define HCI_ERROR_MEMORY_EXCEEDED 0x07 diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c index 1376092c5..46b6d7e27 100644 --- a/net/bluetooth/hci_event.c +++ b/net/bluetooth/hci_event.c @@ -2323,13 +2323,14 @@ static void hci_cs_create_conn(struct hci_dev *hdev, __u8 status) if (status) { if (conn && conn->state == BT_CONNECT) { - /* If the request failed with "Command Disallowed", the + /* If the request failed with a certain status, the * card is either using all its available "slots" for * attempting new connections, or it's currently * doing an HCI Inquiry. In these cases we'll try to * do the "Create Connection" request again later. */ - if (status == HCI_ERROR_COMMAND_DISALLOWED) { + if (status == HCI_ERROR_COMMAND_DISALLOWED || + status == HCI_ERROR_HARDWARE_FAILURE) { conn->state = BT_CONNECT2; if (!hci_conn_hash_lookup_state(hdev, ACL_LINK, BT_CONNECT) && @@ -3254,7 +3255,26 @@ static void hci_conn_complete_evt(struct hci_dev *hdev, void *data, done: if (status) { - hci_conn_failed(conn, status); + if (status == HCI_ERROR_REJ_LIMITED_RESOURCES) { + conn->state = BT_CONNECT2; + + if (!hci_conn_hash_lookup_state(hdev, ACL_LINK, BT_CONNECT) && + !test_bit(HCI_INQUIRY, &hdev->flags)) { + bt_dev_err(hdev, + "\"Connect Complete\" event with error " + "(0x%2.2x) indicating to try again, but " + "there's no concurrent \"Create " + "Connection\" nor an ongoing inquiry", + status); + + hci_conn_failed(conn, status); + } + + hci_dev_unlock(hdev); + return; + } else { + hci_conn_failed(conn, status); + } } else if (ev->link_type == SCO_LINK) { switch (conn->setting & SCO_AIRMODE_MASK) { case SCO_AIRMODE_CVSD: