From patchwork Mon Jan 8 22:46:08 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: =?utf-8?q?Jonas_Dre=C3=9Fler?= X-Patchwork-Id: 186150 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:7300:37c1:b0:101:2151:f287 with SMTP id y1csp1315614dyq; Mon, 8 Jan 2024 14:48:25 -0800 (PST) X-Google-Smtp-Source: AGHT+IGfD6NYDfYiFAr6jYw/+bGbeGLkXY+5kskI1dtPMsJYlg+I/M8hYchn10Up/JNq7V2/3C55 X-Received: by 2002:a50:c34d:0:b0:556:ece8:559d with SMTP id q13-20020a50c34d000000b00556ece8559dmr2296765edb.80.1704754105692; Mon, 08 Jan 2024 14:48:25 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1704754105; cv=none; d=google.com; s=arc-20160816; b=aS5mlQjBjOjVMbNwGmkn4xQ9CTdswrkhTFXiMyqcfOf0F9vM2PZpqHCUJsS8QxLgSz 7giCZI9KYzojdhvBKZkB9o5xh7vpvu6cIvJ5S9p4aGTKb7PIqlP8qZ6DlV+A5ckAOkjT loRdqoVUgJXUhu1sYEzV+0T3/Q2q9dVixwL/oc1/2jGJQLQ5yC3XufAV4XgQpbXPt4xU Tz44QxRG4/VPtV3BLpygUu5H5nxknkLHHH5uikmpd5b9Z8JpkJkIe8islY31T7sIwnxL Ngb0oHrF5cylWEpE/5I5ThBsrHDeBs+l6malBAsuCt4twrmGfHNESLh7hk9jawDBSu3C z/8Q== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=content-transfer-encoding:mime-version:list-unsubscribe :list-subscribe:list-id:precedence:references:in-reply-to:message-id :date:subject:cc:to:from; bh=ooMGCXfrzyxpbxCIG2NI7yxpsa3YD/VoicU333fxNVA=; fh=3AqDzvZy8BuFcn015+g5AB3sGQnZc/edyFwniXfpO+U=; b=cfI2ZFaEUS5KTN9w5XZbgD565gC41WOWI/fubXdtFFvWtbaHMOIJr7JaCJetgN7Mzp F0s73YMV2mJ1ebgI1af22V1JNDV0uTz8aD73x+hIsoQZrQ6HOJCSfV6EyXs7MxK8ZwyU WmjajI6T0I0NszC55ZU0M7rGnhzwiEhwhngI2iSfGhBFXC5L6sv4cFixbwlwNcEho1mO ZiEAr0jEEfDxggxDqi36DKMEC68Zy1QZHFv3w+hnahmo328lMDgYjGVjWA6bsZOAuFXA LdAfzmVeGaxzRICVirgOoJA1B6XE2hDhqNSv9NTsjMxK/dDdCAqXQJNa6vhXLf0MBb+/ WO+A== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel+bounces-20161-ouuuleilei=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) smtp.mailfrom="linux-kernel+bounces-20161-ouuuleilei=gmail.com@vger.kernel.org" Received: from am.mirrors.kernel.org (am.mirrors.kernel.org. [2604:1380:4601:e00::3]) by mx.google.com with ESMTPS id z100-20020a509e6d000000b00554c01b4588si258711ede.257.2024.01.08.14.48.25 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 08 Jan 2024 14:48:25 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel+bounces-20161-ouuuleilei=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel+bounces-20161-ouuuleilei=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) smtp.mailfrom="linux-kernel+bounces-20161-ouuuleilei=gmail.com@vger.kernel.org" Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by am.mirrors.kernel.org (Postfix) with ESMTPS id 21ABF1F2356C for ; Mon, 8 Jan 2024 22:48:25 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id AF2225732B; Mon, 8 Jan 2024 22:46:36 +0000 (UTC) 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 9CF0C56778; Mon, 8 Jan 2024 22:46:32 +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 smtp102.mailbox.org (smtp102.mailbox.org [10.196.197.102]) (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 4T88LX5Vlmz9scc; Mon, 8 Jan 2024 23:46:28 +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 v3 3/4] Bluetooth: hci_conn: Only do ACL connections sequentially Date: Mon, 8 Jan 2024 23:46:08 +0100 Message-ID: <20240108224614.56900-4-verdre@v0yd.nl> In-Reply-To: <20240108224614.56900-1-verdre@v0yd.nl> References: <20240108224614.56900-1-verdre@v0yd.nl> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1787564241080893570 X-GMAIL-MSGID: 1787564241080893570 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 + include/net/bluetooth/hci_sync.h | 3 ++ net/bluetooth/hci_conn.c | 69 ++++-------------------------- net/bluetooth/hci_sync.c | 72 ++++++++++++++++++++++++++++++++ 4 files changed, 85 insertions(+), 60 deletions(-) diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h index 63f84e185..a84102ad5 100644 --- a/include/net/bluetooth/hci.h +++ b/include/net/bluetooth/hci.h @@ -437,6 +437,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/include/net/bluetooth/hci_sync.h b/include/net/bluetooth/hci_sync.h index 57eeb07ae..2bc3235f3 100644 --- a/include/net/bluetooth/hci_sync.h +++ b/include/net/bluetooth/hci_sync.h @@ -136,3 +136,6 @@ int hci_le_terminate_big_sync(struct hci_dev *hdev, u8 handle, u8 reason); int hci_le_big_terminate_sync(struct hci_dev *hdev, u8 handle); int hci_le_pa_terminate_sync(struct hci_dev *hdev, u16 handle); + +int hci_acl_create_connection_sync(struct hci_dev *hdev, + struct hci_conn *conn); diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c index 73470cc35..c9a5734fc 100644 --- a/net/bluetooth/hci_conn.c +++ b/net/bluetooth/hci_conn.c @@ -178,64 +178,6 @@ static void hci_conn_cleanup(struct hci_conn *conn) hci_conn_put(conn); } -static void hci_acl_create_connection(struct hci_conn *conn) -{ - struct hci_dev *hdev = conn->hdev; - struct inquiry_entry *ie; - struct hci_cp_create_conn cp; - - BT_DBG("hcon %p", conn); - - /* Many controllers disallow HCI Create Connection while it is doing - * HCI Inquiry. So we cancel the Inquiry first before issuing HCI Create - * Connection. This may cause the MGMT discovering state to become false - * without user space's request but it is okay since the MGMT Discovery - * APIs do not promise that discovery should be done forever. Instead, - * the user space monitors the status of MGMT discovering and it may - * 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; - } - - conn->state = BT_CONNECT; - conn->out = true; - conn->role = HCI_ROLE_MASTER; - - conn->attempt++; - - conn->link_policy = hdev->link_policy; - - memset(&cp, 0, sizeof(cp)); - bacpy(&cp.bdaddr, &conn->dst); - cp.pscan_rep_mode = 0x02; - - ie = hci_inquiry_cache_lookup(hdev, &conn->dst); - if (ie) { - if (inquiry_entry_age(ie) <= INQUIRY_ENTRY_AGE_MAX) { - cp.pscan_rep_mode = ie->data.pscan_rep_mode; - cp.pscan_mode = ie->data.pscan_mode; - cp.clock_offset = ie->data.clock_offset | - cpu_to_le16(0x8000); - } - - memcpy(conn->dev_class, ie->data.dev_class, 3); - } - - cp.pkt_type = cpu_to_le16(conn->pkt_type); - if (lmp_rswitch_capable(hdev) && !(hdev->link_mode & HCI_LM_MASTER)) - cp.role_switch = 0x01; - else - cp.role_switch = 0x00; - - hci_send_cmd(hdev, HCI_OP_CREATE_CONN, sizeof(cp), &cp); -} - int hci_disconnect(struct hci_conn *conn, __u8 reason) { BT_DBG("hcon %p", conn); @@ -1647,10 +1589,17 @@ 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_acl_create_connection_sync(hdev, acl); + if (err) { + hci_conn_del(acl); + return ERR_PTR(err); + } } return acl; @@ -2580,7 +2529,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_acl_create_connection_sync(hdev, conn); hci_dev_unlock(hdev); } diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c index a15ab0b87..067d44570 100644 --- a/net/bluetooth/hci_sync.c +++ b/net/bluetooth/hci_sync.c @@ -6565,3 +6565,75 @@ int hci_update_adv_data(struct hci_dev *hdev, u8 instance) return hci_cmd_sync_queue(hdev, _update_adv_data_sync, UINT_PTR(instance), NULL); } + +static int __hci_acl_create_connection_sync(struct hci_dev *hdev, void *data) +{ + struct hci_conn *conn = data; + struct inquiry_entry *ie; + struct hci_cp_create_conn cp; + int err; + + BT_DBG("hcon %p", conn); + + /* Many controllers disallow HCI Create Connection while it is doing + * HCI Inquiry. So we cancel the Inquiry first before issuing HCI Create + * Connection. This may cause the MGMT discovering state to become false + * without user space's request but it is okay since the MGMT Discovery + * APIs do not promise that discovery should be done forever. Instead, + * the user space monitors the status of MGMT discovering and it may + * request for discovery again when this flag becomes false. + */ + if (test_bit(HCI_INQUIRY, &hdev->flags)) { + 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; + conn->out = true; + conn->role = HCI_ROLE_MASTER; + + conn->attempt++; + + conn->link_policy = hdev->link_policy; + + memset(&cp, 0, sizeof(cp)); + bacpy(&cp.bdaddr, &conn->dst); + cp.pscan_rep_mode = 0x02; + + ie = hci_inquiry_cache_lookup(hdev, &conn->dst); + if (ie) { + if (inquiry_entry_age(ie) <= INQUIRY_ENTRY_AGE_MAX) { + cp.pscan_rep_mode = ie->data.pscan_rep_mode; + cp.pscan_mode = ie->data.pscan_mode; + cp.clock_offset = ie->data.clock_offset | + cpu_to_le16(0x8000); + } + + memcpy(conn->dev_class, ie->data.dev_class, 3); + } + + cp.pkt_type = cpu_to_le16(conn->pkt_type); + if (lmp_rswitch_capable(hdev) && !(hdev->link_mode & HCI_LM_MASTER)) + cp.role_switch = 0x01; + else + cp.role_switch = 0x00; + + 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_sync(hdev, conn, HCI_ERROR_LOCAL_HOST_TERM); + + return err; +} + +int hci_acl_create_connection_sync(struct hci_dev *hdev, + struct hci_conn *conn) +{ + return hci_cmd_sync_queue(hdev, __hci_acl_create_connection_sync, + conn, NULL); +}