Message ID | 20240214012328.BlueZ.1.I10eda6f375bc8dfedf4eef3a8cf6572c65803afc@changeid |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel+bounces-63979-ouuuleilei=gmail.com@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:7300:bc8a:b0:106:860b:bbdd with SMTP id dn10csp690196dyb; Tue, 13 Feb 2024 09:26:24 -0800 (PST) X-Forwarded-Encrypted: i=3; AJvYcCXEr0ELJDn3GBLQTbqTaq0pHnhkiutrS3Ul8Buesz7sGoZQjIxLKeM+p7umYTcGTWMu30qvsYVXnbw0VsULmS14IIpiLg== X-Google-Smtp-Source: AGHT+IFHpDSHbnS31F3mFIUF7MczHDecoNZg5WQ81JYLP2iiiHgpV2fGBiXpA+JzpO3EUNfuxE20 X-Received: by 2002:a05:6a20:c51c:b0:19c:8d73:721b with SMTP id gm28-20020a056a20c51c00b0019c8d73721bmr169460pzb.57.1707845183640; Tue, 13 Feb 2024 09:26:23 -0800 (PST) ARC-Seal: i=2; a=rsa-sha256; t=1707845183; cv=pass; d=google.com; s=arc-20160816; b=jTwGdo9RtBH+Du4HDiLnzdQ93tN3jBJqEyICP/62dBLKeoJu1Qux43f1sqsfMpop5o +rxGmbtMGWdIVYbHoefMry0e1mDUV3GI+6ygOnuxzKNM23yH52D6hnBr5WN6yJUdv0Kl aL7emjsmMM75zEXeiY2bopgiFtc1YGnvEJbWkV1VW7x9xNrJtr47xUnTpBnT8kspGfmo 7UTq7d93ZpNmC0Jh/7P6++uDFWvSQ9R+DkHyzDk1yR65WAFsenjl38i6QIyek4ypHJ4T QlwtO8wjzZPcRJ/IW0VXAMyq807byJWy1ldxL71pgTAlRasojyRcgqqhV5GTgakb0zbg hl8A== ARC-Message-Signature: i=2; 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:message-id:date:subject:cc:to :from:dkim-signature; bh=8XLUuSB3OMz4OtDEw/wq9wIqlJoGF0PKwvDRzd4XmEc=; fh=s6CstFnn8twnyFKCksar8hO1lFXSPUbqqG3jUX+eW0s=; b=eHUh4YwX1UHBxoBp8U+qo6Z426u9bC4lYb/YAQgoX6TNHXmIqJjpgpCEA8GCXvqplA ZWGfmAG9oU5xKBTWQy78fah7/wnIOJa46/WBOdmpbMqIVB5bwRedJeCCdzZBfxNXdpoq zZgd7IinDom5mmwHGrIcSfbKZELnd45KPSCbf66BxtfK5iBg+Gv7OYQ0J8apUFQ7dEP0 b1f4OeaHOfM9kMOmHcEmDlllIm77r+lZlOPlS+FEtBlZBmT31kSYxsj7bUO2Tpw8GIow CrIRkz2H6wRkYiUs3+VKyPEqr3v3vtS+2GOcx38OUPUysoVBvW6qRrQkAmaSsf509jhI R1YQ==; dara=google.com ARC-Authentication-Results: i=2; mx.google.com; dkim=pass header.i=@chromium.org header.s=google header.b=jSyTHznY; arc=pass (i=1 spf=pass spfdomain=chromium.org dkim=pass dkdomain=chromium.org dmarc=pass fromdomain=chromium.org); spf=pass (google.com: domain of linux-kernel+bounces-63979-ouuuleilei=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) smtp.mailfrom="linux-kernel+bounces-63979-ouuuleilei=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=chromium.org X-Forwarded-Encrypted: i=2; AJvYcCVmSk0JwetavdnXpQxI/9GICUw/B+lirEqzqCqwjRnHhH3uWNfflDsCKmYiwTIQmD7NHo7g+IEq509iuwu+G+6Y7vJcfw== Received: from sv.mirrors.kernel.org (sv.mirrors.kernel.org. [2604:1380:45e3:2400::1]) by mx.google.com with ESMTPS id e20-20020a633714000000b005d8b7c3a019si2293408pga.856.2024.02.13.09.26.23 for <ouuuleilei@gmail.com> (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 13 Feb 2024 09:26:23 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel+bounces-63979-ouuuleilei=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1; Authentication-Results: mx.google.com; dkim=pass header.i=@chromium.org header.s=google header.b=jSyTHznY; arc=pass (i=1 spf=pass spfdomain=chromium.org dkim=pass dkdomain=chromium.org dmarc=pass fromdomain=chromium.org); spf=pass (google.com: domain of linux-kernel+bounces-63979-ouuuleilei=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) smtp.mailfrom="linux-kernel+bounces-63979-ouuuleilei=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=chromium.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 sv.mirrors.kernel.org (Postfix) with ESMTPS id 9D62B28B359 for <ouuuleilei@gmail.com>; Tue, 13 Feb 2024 17:25:01 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 2E0F56025D; Tue, 13 Feb 2024 17:24:31 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="jSyTHznY" Received: from mail-pf1-f171.google.com (mail-pf1-f171.google.com [209.85.210.171]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 1FD865FDD7 for <linux-kernel@vger.kernel.org>; Tue, 13 Feb 2024 17:24:26 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.210.171 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1707845068; cv=none; b=ef6NrFd226Ikpl6lvLHI9f46kGG0QD/cDAs+nuOb3euLukjYEIrz4eBof2ushdmwQjDJdGnd4s7FApDkkVUFmkFPtM6H/EqRdcnLK+e6OinRyj5GXrH9kDvVCmKGnj/HosZhpA23EttdqNpkzTFEX3cGXvnHKOwGuElT8/zkP4o= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1707845068; c=relaxed/simple; bh=dAcS//bN5EUl0rmpd6MTRSeVq9WzyY8e6NY4x+P70zw=; h=From:To:Cc:Subject:Date:Message-ID:MIME-Version; b=nzmCoHnX1qoWSfyL8H9hlmOfkU7OtIEHzuIk+q9pxx2H64Hug9yHzsRFsRxCx7ZOHC5laFNt3HCSP/ShRZyAGNkaPdwNnTM2qumaY+nrNBSE6JQgWfJr5ZVrkQ750VVb2JIUUTm1PnSKSuzzSqblm1KMYoPDsi2ek4fEKzSWqHU= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=chromium.org; spf=pass smtp.mailfrom=chromium.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b=jSyTHznY; arc=none smtp.client-ip=209.85.210.171 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=chromium.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=chromium.org Received: by mail-pf1-f171.google.com with SMTP id d2e1a72fcca58-6e104196e6eso404608b3a.0 for <linux-kernel@vger.kernel.org>; Tue, 13 Feb 2024 09:24:26 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1707845066; x=1708449866; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=8XLUuSB3OMz4OtDEw/wq9wIqlJoGF0PKwvDRzd4XmEc=; b=jSyTHznYTizsJJLkDBNnEn1tFnEEfimEe8x37UwmjW5wso19x10HZZY61wY78EdTrW 9DyOEA5wzzD5R34w2VihyXRZklBwkrVUFT593Rfr1S3C2wxXn9aiFM5ujSdmB6503qjx q59YdCMVaSulr24jKgyFXy9WDopCwJdtmwL04= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1707845066; x=1708449866; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=8XLUuSB3OMz4OtDEw/wq9wIqlJoGF0PKwvDRzd4XmEc=; b=miPKVJz0Uf7YEDAq7unrmU+tHCmxQ6vgZ6a/Z3obOl2i+SE5jI6eiHhZfq4Llue9tE yZ5Yuum0zMB+qpacxExSvNE/6uFVFhL5afomWXIFRUAHtPeIVznJxHsfTGGU+9Uev0K/ GWwCyI7HwoC0YTRHye0OFEPrfeNLQRt6tjso/+LiICLRBOLZcuBBVKRqgOpWczmVr3nZ u6m9mytOu+X/Gtk4gX+zxziTKl+8+DpiR/jgcIL/+dqKIuFzO/mpCq01oHpMFFoDe1Lx djWC5D23UtmTRznNNheizTBcW4/NmI2BYUMEaFgm96NWq1IHDXnn4XbNukxnp1zPWsF7 Uumw== X-Forwarded-Encrypted: i=1; AJvYcCUxCseYQYvlJ7eeEyi3g0E/HOOG4TWJGg1u0WgWhUR5++STIhlW1QE3e6vZYfLb95Yr5/jgmxoTUoTsDPW0wwSOeuOSaCpMha5ifb8d X-Gm-Message-State: AOJu0YxklRulNAtZ73/WL8zNt8nyLBHYphiJfxZPsNdXyYSapHswyiuc 4Ttki6vIE2snm1eYTWzmTuopjaGGLCNB5ZgPzSlre4edTNzk8YDcPGXZQDJhtg== X-Received: by 2002:a05:6a21:3115:b0:19e:a270:e766 with SMTP id yz21-20020a056a21311500b0019ea270e766mr335577pzb.5.1707845066416; Tue, 13 Feb 2024 09:24:26 -0800 (PST) X-Forwarded-Encrypted: i=1; AJvYcCV3gdAZrMiwHKmpifH8I6waLmgmEwpS3trwB7VFLzXrxjOa5uNlGnMgRca14Hzd+QYpdvWYn5n9Liwb5rPRx6lgYUkHDUn2+cbT1jg1pJ5Q0cr3NA51wno716+wCz+1Ky7mnxQeNPx+XU919MMCJJL8nnt5gANYBIT/TVmz9WT2lovM+AK+ftEDGREWJSau0qCrGqe62uHuDdpTb6Nd+c96QUpwVeMd Received: from localhost (253.78.80.34.bc.googleusercontent.com. [34.80.78.253]) by smtp.gmail.com with UTF8SMTPSA id o21-20020a637e55000000b005c2420fb198sm2672177pgn.37.2024.02.13.09.24.24 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 13 Feb 2024 09:24:26 -0800 (PST) From: Hsin-chen Chuang <chharry@chromium.org> To: linux-bluetooth@vger.kernel.org Cc: chromeos-bluetooth-upstreaming@chromium.org, Hsin-chen Chuang <chharry@chromium.org>, Johan Hedberg <johan.hedberg@gmail.com>, Luiz Augusto von Dentz <luiz.dentz@gmail.com>, Marcel Holtmann <marcel@holtmann.org>, linux-kernel@vger.kernel.org Subject: [BlueZ PATCH] Bluetooth: hci_core: Skip hci_cmd_work if hci_request is pending Date: Wed, 14 Feb 2024 01:23:29 +0800 Message-ID: <20240214012328.BlueZ.1.I10eda6f375bc8dfedf4eef3a8cf6572c65803afc@changeid> X-Mailer: git-send-email 2.43.0.687.g38aa6559b0-goog Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: <linux-kernel.vger.kernel.org> List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org> List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1790805471179160661 X-GMAIL-MSGID: 1790805471179160661 |
Series |
[BlueZ] Bluetooth: hci_core: Skip hci_cmd_work if hci_request is pending
|
|
Commit Message
Hsin-chen Chuang
Feb. 13, 2024, 5:23 p.m. UTC
hci_cmd_work overwrites the hdev->sent_cmd which contains the required
info for a hci_request to work. In the real world, it's observed that
a request from hci_le_ext_create_conn_sync could be interrupted by
the authentication (hci_conn_auth) caused by rfcomm_sock_connect. When
it happends, hci_le_ext_create_conn_sync hangs until timeout; If the
LE connection is triggered by MGMT, it freezes the whole MGMT interface.
Signed-off-by: Hsin-chen Chuang <chharry@chromium.org>
---
net/bluetooth/hci_core.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
Comments
Hi Hsin-chen, On Tue, Feb 13, 2024 at 12:24 PM Hsin-chen Chuang <chharry@chromiumorg> wrote: > > hci_cmd_work overwrites the hdev->sent_cmd which contains the required > info for a hci_request to work. In the real world, it's observed that > a request from hci_le_ext_create_conn_sync could be interrupted by > the authentication (hci_conn_auth) caused by rfcomm_sock_connect. When > it happends, hci_le_ext_create_conn_sync hangs until timeout; If the > LE connection is triggered by MGMT, it freezes the whole MGMT interface. > > Signed-off-by: Hsin-chen Chuang <chharry@chromium.org> > --- > > net/bluetooth/hci_core.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c > index 34c8dca2069f..e3706889976d 100644 > --- a/net/bluetooth/hci_core.c > +++ b/net/bluetooth/hci_core.c > @@ -4213,8 +4213,11 @@ static void hci_cmd_work(struct work_struct *work) > BT_DBG("%s cmd_cnt %d cmd queued %d", hdev->name, > atomic_read(&hdev->cmd_cnt), skb_queue_len(&hdev->cmd_q)); > > - /* Send queued commands */ > - if (atomic_read(&hdev->cmd_cnt)) { > + /* Send queued commands. Don't send the command when there is a pending > + * hci_request because the request callbacks would be overwritten. > + */ > + if (atomic_read(&hdev->cmd_cnt) && > + !hci_dev_test_flag(hdev, HCI_CMD_PENDING)) { > skb = skb_dequeue(&hdev->cmd_q); > if (!skb) > return; > -- > 2.43.0.687.g38aa6559b0-goog This seems to be causing some mgmt-tester failures: Pair Device - Sec Mode 3 Success 1 Timed out 22.753 seconds Pair Device - Sec Mode 3 Reject 1 Timed out 22.533 seconds Pair Device - Sec Mode 3 Reject 2 Timed out 22.526 seconds I think this is because we need to respond to an event with a command like: < HCI Command: Create Conn.. (0x01|0x0005) plen 13 #241 [hci0] 16:25:38.699066 Address: 00:AA:01:01:00:00 (Intel Corporation) Packet type: 0x0018 DM1 may be used DH1 may be used Page scan repetition mode: R2 (0x02) Page scan mode: Mandatory (0x00) Clock offset: 0x0000 Role switch: Allow peripheral (0x01) > HCI Event: Command Status (0x0f) plen 4 #242 [hci0] 16:25:38.701881 Create Connection (0x01|0x0005) ncmd 1 Status: Success (0x00) > HCI Event: Link Key Request (0x17) plen 6 #243 [hci0] 16:25:38.702375 Address: 00:AA:01:01:00:00 (Intel Corporation) But because Create Connection is pending we cannot respond to Link Key Request, so it is actually a design problem if we cannot send commands because something is pending so perhaps we need to redesign how we store cmd_sent so we can have multiple outstanding commands rather than just one.
Hi Hsin-chen, On Thu, Feb 15, 2024 at 11:21 PM Hsin-chen Chuang <chharry@google.com> wrote: > > +Some Googlers who would be interested in > > Hi Luiz, > > How about moving the hci_req-related data out from sent_cmd? This allows sending HCI commands while hci_req data would not be overwritten. I have something like the following in the works: https://gist.github.com/Vudentz/251275bb688fac32585f90ac0076c407 It is not stable yet, but I think we can get away with it since it just means we can keep the pending request stored in the req_skb, that said we might need to overhaul this design since it is not very clean in my opinion. > On Fri, Feb 16, 2024 at 5:37 AM Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote: >> >> Hi Hsin-chen, >> >> On Tue, Feb 13, 2024 at 12:24 PM Hsin-chen Chuang <chharry@chromium.org> wrote: >> > >> > hci_cmd_work overwrites the hdev->sent_cmd which contains the required >> > info for a hci_request to work. In the real world, it's observed that >> > a request from hci_le_ext_create_conn_sync could be interrupted by >> > the authentication (hci_conn_auth) caused by rfcomm_sock_connect. When >> > it happends, hci_le_ext_create_conn_sync hangs until timeout; If the >> > LE connection is triggered by MGMT, it freezes the whole MGMT interface. >> > >> > Signed-off-by: Hsin-chen Chuang <chharry@chromium.org> >> > --- >> > >> > net/bluetooth/hci_core.c | 7 +++++-- >> > 1 file changed, 5 insertions(+), 2 deletions(-) >> > >> > diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c >> > index 34c8dca2069f..e3706889976d 100644 >> > --- a/net/bluetooth/hci_core.c >> > +++ b/net/bluetooth/hci_core.c >> > @@ -4213,8 +4213,11 @@ static void hci_cmd_work(struct work_struct *work) >> > BT_DBG("%s cmd_cnt %d cmd queued %d", hdev->name, >> > atomic_read(&hdev->cmd_cnt), skb_queue_len(&hdev->cmd_q)); >> > >> > - /* Send queued commands */ >> > - if (atomic_read(&hdev->cmd_cnt)) { >> > + /* Send queued commands. Don't send the command when there is a pending >> > + * hci_request because the request callbacks would be overwritten. >> > + */ >> > + if (atomic_read(&hdev->cmd_cnt) && >> > + !hci_dev_test_flag(hdev, HCI_CMD_PENDING)) { >> > skb = skb_dequeue(&hdev->cmd_q); >> > if (!skb) >> > return; >> > -- >> > 2.43.0.687.g38aa6559b0-goog >> >> >> This seems to be causing some mgmt-tester failures: >> >> Pair Device - Sec Mode 3 Success 1 Timed out 22.753 seconds >> Pair Device - Sec Mode 3 Reject 1 Timed out 22.533 seconds >> Pair Device - Sec Mode 3 Reject 2 Timed out 22.526 seconds >> >> I think this is because we need to respond to an event with a command like: >> >> < HCI Command: Create Conn.. (0x01|0x0005) plen 13 #241 [hci0] 16:25:38699066 >> Address: 00:AA:01:01:00:00 (Intel Corporation) >> Packet type: 0x0018 >> DM1 may be used >> DH1 may be used >> Page scan repetition mode: R2 (0x02) >> Page scan mode: Mandatory (0x00) >> Clock offset: 0x0000 >> Role switch: Allow peripheral (0x01) >> > HCI Event: Command Status (0x0f) plen 4 #242 [hci0] 16:25:38701881 >> Create Connection (0x01|0x0005) ncmd 1 >> Status: Success (0x00) >> > HCI Event: Link Key Request (0x17) plen 6 #243 [hci0] 16:25:38702375 >> Address: 00:AA:01:01:00:00 (Intel Corporation) >> >> But because Create Connection is pending we cannot respond to Link Key >> Request, so it is actually a design problem if we cannot send commands >> because something is pending so perhaps we need to redesign how we >> store cmd_sent so we can have multiple outstanding commands rather >> than just one. >> >> -- >> Luiz Augusto von Dentz > > > > -- > Best Regards, > Hsin-chen
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c index 34c8dca2069f..e3706889976d 100644 --- a/net/bluetooth/hci_core.c +++ b/net/bluetooth/hci_core.c @@ -4213,8 +4213,11 @@ static void hci_cmd_work(struct work_struct *work) BT_DBG("%s cmd_cnt %d cmd queued %d", hdev->name, atomic_read(&hdev->cmd_cnt), skb_queue_len(&hdev->cmd_q)); - /* Send queued commands */ - if (atomic_read(&hdev->cmd_cnt)) { + /* Send queued commands. Don't send the command when there is a pending + * hci_request because the request callbacks would be overwritten. + */ + if (atomic_read(&hdev->cmd_cnt) && + !hci_dev_test_flag(hdev, HCI_CMD_PENDING)) { skb = skb_dequeue(&hdev->cmd_q); if (!skb) return;