linux-bluetooth.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Jonas Dreßler" <verdre@v0yd.nl>
To: Marcel Holtmann <marcel@holtmann.org>,
	Johan Hedberg <johan.hedberg@gmail.com>,
	Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Cc: "Jonas Dreßler" <verdre@v0yd.nl>,
	linux-bluetooth@vger.kernel.org, linux-kernel@vger.kernel.org,
	netdev@vger.kernel.org
Subject: [PATCH v2 3/4] Bluetooth: hci_conn: Only do ACL connections sequentially
Date: Mon,  8 Jan 2024 19:39:35 +0100	[thread overview]
Message-ID: <20240108183938.468426-4-verdre@v0yd.nl> (raw)
In-Reply-To: <20240108183938.468426-1-verdre@v0yd.nl>

Pretty much all bluetooth chipsets only support paging a single device at
a time, and if they don't reject a secondary "Create Connection" request
while another is still ongoing, they'll most likely serialize those
requests in the firware.

With commit 4c67bc74f016 ("[Bluetooth] Support concurrent connect
requests") we started adding some serialization of our own in case the
adapter returns "Command Disallowed" HCI error.

This commit was using the BT_CONNECT2 state for the serialization, this
state is also used for a few more things (most notably to indicate we're
waiting for an inquiry to cancel) and therefore a bit unreliable. Also
not all BT firwares would respond with "Command Disallowed" on too many
connection requests, some will also respond with "Hardware Failure"
(BCM4378), and others will error out later and send a "Connect Complete"
event with error "Rejected Limited Resources" (Marvell 88W8897).

We can clean things up a bit and also make the serialization more reliable
by using our hci_sync machinery to always do "Create Connection" requests
in a sequential manner.

This is very similar to what we're already doing for establishing LE
connections, and it works well there.
---
 include/net/bluetooth/hci.h |  1 +
 net/bluetooth/hci_conn.c    | 37 ++++++++++++++++++++++++++-----------
 2 files changed, 27 insertions(+), 11 deletions(-)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index fef723afd..f2bbc0a14 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -427,6 +427,7 @@ enum {
 #define HCI_ACL_TX_TIMEOUT	msecs_to_jiffies(45000)	/* 45 seconds */
 #define HCI_AUTO_OFF_TIMEOUT	msecs_to_jiffies(2000)	/* 2 seconds */
 #define HCI_POWER_OFF_TIMEOUT	msecs_to_jiffies(5000)	/* 5 seconds */
+#define HCI_ACL_CONN_TIMEOUT	msecs_to_jiffies(20000)	/* 20 seconds */
 #define HCI_LE_CONN_TIMEOUT	msecs_to_jiffies(20000)	/* 20 seconds */
 #define HCI_LE_AUTOCONN_TIMEOUT	msecs_to_jiffies(4000)	/* 4 seconds */
 
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 76222565e..541d55301 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -229,11 +229,12 @@ static void hci_connect_le_scan_remove(struct hci_conn *conn)
 	schedule_work(&conn->le_scan_cleanup);
 }
 
-static void hci_acl_create_connection(struct hci_conn *conn)
+static int hci_acl_create_connection_sync(struct hci_dev *hdev, void *data)
 {
-	struct hci_dev *hdev = conn->hdev;
+	struct hci_conn *conn = data;
 	struct inquiry_entry *ie;
 	struct hci_cp_create_conn cp;
+	int err;
 
 	BT_DBG("hcon %p", conn);
 
@@ -246,12 +247,10 @@ static void hci_acl_create_connection(struct hci_conn *conn)
 	 * request for discovery again when this flag becomes false.
 	 */
 	if (test_bit(HCI_INQUIRY, &hdev->flags)) {
-		/* Put this connection to "pending" state so that it will be
-		 * executed after the inquiry cancel command complete event.
-		 */
-		conn->state = BT_CONNECT2;
-		hci_send_cmd(hdev, HCI_OP_INQUIRY_CANCEL, 0, NULL);
-		return;
+		err = __hci_cmd_sync_status(hdev, HCI_OP_INQUIRY_CANCEL, 0,
+					    NULL, HCI_CMD_TIMEOUT);
+		if (err)
+			bt_dev_warn(hdev, "Failed to cancel inquiry %d", err);
 	}
 
 	conn->state = BT_CONNECT;
@@ -284,7 +283,15 @@ static void hci_acl_create_connection(struct hci_conn *conn)
 	else
 		cp.role_switch = 0x00;
 
-	hci_send_cmd(hdev, HCI_OP_CREATE_CONN, sizeof(cp), &cp);
+	err = __hci_cmd_sync_status_sk(hdev, HCI_OP_CREATE_CONN,
+				       sizeof(cp), &cp,
+				       HCI_EV_CONN_COMPLETE,
+				       HCI_ACL_CONN_TIMEOUT, NULL);
+
+	if (err == -ETIMEDOUT)
+		hci_abort_conn(conn, HCI_ERROR_LOCAL_HOST_TERM);
+
+	return err;
 }
 
 int hci_disconnect(struct hci_conn *conn, __u8 reason)
@@ -1622,10 +1629,18 @@ struct hci_conn *hci_connect_acl(struct hci_dev *hdev, bdaddr_t *dst,
 
 	acl->conn_reason = conn_reason;
 	if (acl->state == BT_OPEN || acl->state == BT_CLOSED) {
+		int err;
+
 		acl->sec_level = BT_SECURITY_LOW;
 		acl->pending_sec_level = sec_level;
 		acl->auth_type = auth_type;
-		hci_acl_create_connection(acl);
+
+		err = hci_cmd_sync_queue(hdev, hci_acl_create_connection_sync,
+					 acl, NULL);
+		if (err) {
+			hci_conn_del(acl);
+			return ERR_PTR(err);
+		}
 	}
 
 	return acl;
@@ -2530,7 +2545,7 @@ void hci_conn_check_pending(struct hci_dev *hdev)
 
 	conn = hci_conn_hash_lookup_state(hdev, ACL_LINK, BT_CONNECT2);
 	if (conn)
-		hci_acl_create_connection(conn);
+		hci_cmd_sync_queue(hdev, hci_acl_create_connection_sync, conn, NULL);
 
 	hci_dev_unlock(hdev);
 }
-- 
2.43.0


  parent reply	other threads:[~2024-01-08 18:39 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-08 18:39 [PATCH v2 0/4] Bluetooth: Improve retrying of connection attempts Jonas Dreßler
2024-01-08 18:39 ` [PATCH v2 1/4] Bluetooth: Remove superfluous call to hci_conn_check_pending() Jonas Dreßler
2024-01-08 19:11   ` Bluetooth: Improve retrying of connection attempts bluez.test.bot
2024-01-08 18:39 ` [PATCH v2 2/4] Bluetooth: hci_event: Use HCI error defines instead of magic values Jonas Dreßler
2024-01-08 18:39 ` Jonas Dreßler [this message]
2024-01-08 19:11   ` [PATCH v2 3/4] Bluetooth: hci_conn: Only do ACL connections sequentially Luiz Augusto von Dentz
2024-01-08 18:39 ` [PATCH v2 4/4] Bluetooth: Remove pending ACL connection attempts Jonas Dreßler
2024-01-08 18:44   ` Jonas Dreßler
2024-01-08 18:55     ` Jonas Dreßler
2024-01-08 19:14       ` Luiz Augusto von Dentz
2024-01-08 19:29         ` Jonas Dreßler
2024-01-08 19:41           ` Luiz Augusto von Dentz
2024-01-08 20:26             ` Jonas Dreßler
2024-01-08 20:46               ` Luiz Augusto von Dentz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240108183938.468426-4-verdre@v0yd.nl \
    --to=verdre@v0yd.nl \
    --cc=johan.hedberg@gmail.com \
    --cc=linux-bluetooth@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luiz.dentz@gmail.com \
    --cc=marcel@holtmann.org \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).