* [PATCH] Bluetooth: Cancel Inquiry before Create Connection
@ 2020-11-24 1:09 Sonny Sasaka
2020-11-24 1:35 ` bluez.test.bot
2020-11-25 14:43 ` [PATCH] " Marcel Holtmann
0 siblings, 2 replies; 6+ messages in thread
From: Sonny Sasaka @ 2020-11-24 1:09 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Sonny Sasaka, Abhishek Pandit-Subedi
Many controllers do not allow HCI Create Connection while it is doing
Inquiry. This patch adds Inquiry Cancel before Create Connection in this
case to allow the controller to do Create Connection. User space will be
aware of this Inquiry cancellation and they may issue another discovery
request afterwards.
Sample Command Disallowed response of HCI Create Connection:
< HCI Command: Inquiry (0x01|0x0001) plen 5
Access code: 0x9e8b33 (General Inquiry)
Length: 10.24s (0x08)
Num responses: 0
> HCI Event: Command Status (0x0f) plen 4
Inquiry (0x01|0x0001) ncmd 2
Status: Success (0x00)
< HCI Command: Create Connection (0x01|0x0005) plen 13
Address: XX:XX:XX:XX:XX:XX
Packet type: 0xcc18
Page scan repetition mode: R2 (0x02)
Page scan mode: Mandatory (0x00)
Clock offset: 0x0000
Role switch: Allow slave (0x01)
> HCI Event: Command Status (0x0f) plen 4
Create Connection (0x01|0x0005) ncmd 1
Status: Success (0x00)
> HCI Event: Connect Complete (0x03) plen 11
Status: Command Disallowed (0x0c)
Handle: 65535
Address: XX:XX:XX:XX:XX:XX
Link type: ACL (0x01)
Encryption: Disabled (0x00)
Reviewed-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
Signed-off-by: Sonny Sasaka <sonnysasaka@chromium.org>
---
net/bluetooth/hci_conn.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 4f1cd8063e720..b41ad08f8d411 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -233,6 +233,17 @@ static void hci_acl_create_connection(struct hci_conn *conn)
else
cp.role_switch = 0x00;
+ /* 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))
+ hci_send_cmd(hdev, HCI_OP_INQUIRY_CANCEL, 0, NULL);
+
hci_send_cmd(hdev, HCI_OP_CREATE_CONN, sizeof(cp), &cp);
}
--
2.26.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* RE: Bluetooth: Cancel Inquiry before Create Connection
2020-11-24 1:09 [PATCH] Bluetooth: Cancel Inquiry before Create Connection Sonny Sasaka
@ 2020-11-24 1:35 ` bluez.test.bot
2020-11-25 14:43 ` [PATCH] " Marcel Holtmann
1 sibling, 0 replies; 6+ messages in thread
From: bluez.test.bot @ 2020-11-24 1:35 UTC (permalink / raw)
To: linux-bluetooth, sonnysasaka
[-- Attachment #1: Type: text/plain, Size: 503 bytes --]
This is automated email and please do not reply to this email!
Dear submitter,
Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=389811
---Test result---
##############################
Test: CheckPatch - PASS
##############################
Test: CheckGitLint - PASS
##############################
Test: CheckBuildK - PASS
---
Regards,
Linux Bluetooth
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Bluetooth: Cancel Inquiry before Create Connection
2020-11-24 1:09 [PATCH] Bluetooth: Cancel Inquiry before Create Connection Sonny Sasaka
2020-11-24 1:35 ` bluez.test.bot
@ 2020-11-25 14:43 ` Marcel Holtmann
2020-11-26 0:00 ` Sonny Sasaka
1 sibling, 1 reply; 6+ messages in thread
From: Marcel Holtmann @ 2020-11-25 14:43 UTC (permalink / raw)
To: Sonny Sasaka; +Cc: BlueZ development, Abhishek Pandit-Subedi
Hi Sonny,
> Many controllers do not allow HCI Create Connection while it is doing
> Inquiry. This patch adds Inquiry Cancel before Create Connection in this
> case to allow the controller to do Create Connection. User space will be
> aware of this Inquiry cancellation and they may issue another discovery
> request afterwards.
>
> Sample Command Disallowed response of HCI Create Connection:
> < HCI Command: Inquiry (0x01|0x0001) plen 5
> Access code: 0x9e8b33 (General Inquiry)
> Length: 10.24s (0x08)
> Num responses: 0
>> HCI Event: Command Status (0x0f) plen 4
> Inquiry (0x01|0x0001) ncmd 2
> Status: Success (0x00)
> < HCI Command: Create Connection (0x01|0x0005) plen 13
> Address: XX:XX:XX:XX:XX:XX
> Packet type: 0xcc18
> Page scan repetition mode: R2 (0x02)
> Page scan mode: Mandatory (0x00)
> Clock offset: 0x0000
> Role switch: Allow slave (0x01)
>> HCI Event: Command Status (0x0f) plen 4
> Create Connection (0x01|0x0005) ncmd 1
> Status: Success (0x00)
>> HCI Event: Connect Complete (0x03) plen 11
> Status: Command Disallowed (0x0c)
> Handle: 65535
> Address: XX:XX:XX:XX:XX:XX
> Link type: ACL (0x01)
> Encryption: Disabled (0x00)
>
> Reviewed-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
> Signed-off-by: Sonny Sasaka <sonnysasaka@chromium.org>
>
> ---
> net/bluetooth/hci_conn.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index 4f1cd8063e720..b41ad08f8d411 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -233,6 +233,17 @@ static void hci_acl_create_connection(struct hci_conn *conn)
> else
> cp.role_switch = 0x00;
>
> + /* 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))
> + hci_send_cmd(hdev, HCI_OP_INQUIRY_CANCEL, 0, NULL);
> +
while this seems acceptable, what happens when we have interleaved discovery where we toggle between BR/EDR inquiry and LE scanning. Are you sure we not better cancel the mgmt discovery completely.
Regards
Marcel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Bluetooth: Cancel Inquiry before Create Connection
2020-11-25 14:43 ` [PATCH] " Marcel Holtmann
@ 2020-11-26 0:00 ` Sonny Sasaka
2020-12-02 23:24 ` Sonny Sasaka
2020-12-03 9:56 ` Marcel Holtmann
0 siblings, 2 replies; 6+ messages in thread
From: Sonny Sasaka @ 2020-11-26 0:00 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: BlueZ development, Abhishek Pandit-Subedi
Hi Marcel,
On Wed, Nov 25, 2020 at 6:43 AM Marcel Holtmann <marcel@holtmann.org> wrote:
>
> Hi Sonny,
>
> > Many controllers do not allow HCI Create Connection while it is doing
> > Inquiry. This patch adds Inquiry Cancel before Create Connection in this
> > case to allow the controller to do Create Connection. User space will be
> > aware of this Inquiry cancellation and they may issue another discovery
> > request afterwards.
> >
> > Sample Command Disallowed response of HCI Create Connection:
> > < HCI Command: Inquiry (0x01|0x0001) plen 5
> > Access code: 0x9e8b33 (General Inquiry)
> > Length: 10.24s (0x08)
> > Num responses: 0
> >> HCI Event: Command Status (0x0f) plen 4
> > Inquiry (0x01|0x0001) ncmd 2
> > Status: Success (0x00)
> > < HCI Command: Create Connection (0x01|0x0005) plen 13
> > Address: XX:XX:XX:XX:XX:XX
> > Packet type: 0xcc18
> > Page scan repetition mode: R2 (0x02)
> > Page scan mode: Mandatory (0x00)
> > Clock offset: 0x0000
> > Role switch: Allow slave (0x01)
> >> HCI Event: Command Status (0x0f) plen 4
> > Create Connection (0x01|0x0005) ncmd 1
> > Status: Success (0x00)
> >> HCI Event: Connect Complete (0x03) plen 11
> > Status: Command Disallowed (0x0c)
> > Handle: 65535
> > Address: XX:XX:XX:XX:XX:XX
> > Link type: ACL (0x01)
> > Encryption: Disabled (0x00)
> >
> > Reviewed-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
> > Signed-off-by: Sonny Sasaka <sonnysasaka@chromium.org>
> >
> > ---
> > net/bluetooth/hci_conn.c | 11 +++++++++++
> > 1 file changed, 11 insertions(+)
> >
> > diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> > index 4f1cd8063e720..b41ad08f8d411 100644
> > --- a/net/bluetooth/hci_conn.c
> > +++ b/net/bluetooth/hci_conn.c
> > @@ -233,6 +233,17 @@ static void hci_acl_create_connection(struct hci_conn *conn)
> > else
> > cp.role_switch = 0x00;
> >
> > + /* 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))
> > + hci_send_cmd(hdev, HCI_OP_INQUIRY_CANCEL, 0, NULL);
> > +
>
> while this seems acceptable, what happens when we have interleaved discovery where we toggle between BR/EDR inquiry and LE scanning. Are you sure we not better cancel the mgmt discovery completely.
Only Inquiry is preventing Create Connection, so we don't need to
overcomplicate it by stopping the mgmt discovery completely. In the
case of interleaved discovery, the LE discovery will linger for a
little bit before eventually being disabled and the situation restarts
at a good state. Not perfect but simple, clean, and fixes the issue.
>
> Regards
>
> Marcel
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Bluetooth: Cancel Inquiry before Create Connection
2020-11-26 0:00 ` Sonny Sasaka
@ 2020-12-02 23:24 ` Sonny Sasaka
2020-12-03 9:56 ` Marcel Holtmann
1 sibling, 0 replies; 6+ messages in thread
From: Sonny Sasaka @ 2020-12-02 23:24 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: BlueZ development, Abhishek Pandit-Subedi
Hi Marcel,
Friendly ping on this patch. Thanks.
On Wed, Nov 25, 2020 at 4:00 PM Sonny Sasaka <sonnysasaka@chromium.org> wrote:
>
> Hi Marcel,
>
>
> On Wed, Nov 25, 2020 at 6:43 AM Marcel Holtmann <marcel@holtmann.org> wrote:
> >
> > Hi Sonny,
> >
> > > Many controllers do not allow HCI Create Connection while it is doing
> > > Inquiry. This patch adds Inquiry Cancel before Create Connection in this
> > > case to allow the controller to do Create Connection. User space will be
> > > aware of this Inquiry cancellation and they may issue another discovery
> > > request afterwards.
> > >
> > > Sample Command Disallowed response of HCI Create Connection:
> > > < HCI Command: Inquiry (0x01|0x0001) plen 5
> > > Access code: 0x9e8b33 (General Inquiry)
> > > Length: 10.24s (0x08)
> > > Num responses: 0
> > >> HCI Event: Command Status (0x0f) plen 4
> > > Inquiry (0x01|0x0001) ncmd 2
> > > Status: Success (0x00)
> > > < HCI Command: Create Connection (0x01|0x0005) plen 13
> > > Address: XX:XX:XX:XX:XX:XX
> > > Packet type: 0xcc18
> > > Page scan repetition mode: R2 (0x02)
> > > Page scan mode: Mandatory (0x00)
> > > Clock offset: 0x0000
> > > Role switch: Allow slave (0x01)
> > >> HCI Event: Command Status (0x0f) plen 4
> > > Create Connection (0x01|0x0005) ncmd 1
> > > Status: Success (0x00)
> > >> HCI Event: Connect Complete (0x03) plen 11
> > > Status: Command Disallowed (0x0c)
> > > Handle: 65535
> > > Address: XX:XX:XX:XX:XX:XX
> > > Link type: ACL (0x01)
> > > Encryption: Disabled (0x00)
> > >
> > > Reviewed-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
> > > Signed-off-by: Sonny Sasaka <sonnysasaka@chromium.org>
> > >
> > > ---
> > > net/bluetooth/hci_conn.c | 11 +++++++++++
> > > 1 file changed, 11 insertions(+)
> > >
> > > diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> > > index 4f1cd8063e720..b41ad08f8d411 100644
> > > --- a/net/bluetooth/hci_conn.c
> > > +++ b/net/bluetooth/hci_conn.c
> > > @@ -233,6 +233,17 @@ static void hci_acl_create_connection(struct hci_conn *conn)
> > > else
> > > cp.role_switch = 0x00;
> > >
> > > + /* 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))
> > > + hci_send_cmd(hdev, HCI_OP_INQUIRY_CANCEL, 0, NULL);
> > > +
> >
> > while this seems acceptable, what happens when we have interleaved discovery where we toggle between BR/EDR inquiry and LE scanning. Are you sure we not better cancel the mgmt discovery completely.
> Only Inquiry is preventing Create Connection, so we don't need to
> overcomplicate it by stopping the mgmt discovery completely. In the
> case of interleaved discovery, the LE discovery will linger for a
> little bit before eventually being disabled and the situation restarts
> at a good state. Not perfect but simple, clean, and fixes the issue.
>
> >
> > Regards
> >
> > Marcel
> >
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Bluetooth: Cancel Inquiry before Create Connection
2020-11-26 0:00 ` Sonny Sasaka
2020-12-02 23:24 ` Sonny Sasaka
@ 2020-12-03 9:56 ` Marcel Holtmann
1 sibling, 0 replies; 6+ messages in thread
From: Marcel Holtmann @ 2020-12-03 9:56 UTC (permalink / raw)
To: Sonny Sasaka; +Cc: BlueZ development, Abhishek Pandit-Subedi
Hi Sonny,
>>> Many controllers do not allow HCI Create Connection while it is doing
>>> Inquiry. This patch adds Inquiry Cancel before Create Connection in this
>>> case to allow the controller to do Create Connection. User space will be
>>> aware of this Inquiry cancellation and they may issue another discovery
>>> request afterwards.
>>>
>>> Sample Command Disallowed response of HCI Create Connection:
>>> < HCI Command: Inquiry (0x01|0x0001) plen 5
>>> Access code: 0x9e8b33 (General Inquiry)
>>> Length: 10.24s (0x08)
>>> Num responses: 0
>>>> HCI Event: Command Status (0x0f) plen 4
>>> Inquiry (0x01|0x0001) ncmd 2
>>> Status: Success (0x00)
>>> < HCI Command: Create Connection (0x01|0x0005) plen 13
>>> Address: XX:XX:XX:XX:XX:XX
>>> Packet type: 0xcc18
>>> Page scan repetition mode: R2 (0x02)
>>> Page scan mode: Mandatory (0x00)
>>> Clock offset: 0x0000
>>> Role switch: Allow slave (0x01)
>>>> HCI Event: Command Status (0x0f) plen 4
>>> Create Connection (0x01|0x0005) ncmd 1
>>> Status: Success (0x00)
>>>> HCI Event: Connect Complete (0x03) plen 11
>>> Status: Command Disallowed (0x0c)
>>> Handle: 65535
>>> Address: XX:XX:XX:XX:XX:XX
>>> Link type: ACL (0x01)
>>> Encryption: Disabled (0x00)
>>>
>>> Reviewed-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
>>> Signed-off-by: Sonny Sasaka <sonnysasaka@chromium.org>
>>>
>>> ---
>>> net/bluetooth/hci_conn.c | 11 +++++++++++
>>> 1 file changed, 11 insertions(+)
>>>
>>> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
>>> index 4f1cd8063e720..b41ad08f8d411 100644
>>> --- a/net/bluetooth/hci_conn.c
>>> +++ b/net/bluetooth/hci_conn.c
>>> @@ -233,6 +233,17 @@ static void hci_acl_create_connection(struct hci_conn *conn)
>>> else
>>> cp.role_switch = 0x00;
>>>
>>> + /* 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))
>>> + hci_send_cmd(hdev, HCI_OP_INQUIRY_CANCEL, 0, NULL);
>>> +
>>
>> while this seems acceptable, what happens when we have interleaved discovery where we toggle between BR/EDR inquiry and LE scanning. Are you sure we not better cancel the mgmt discovery completely.
> Only Inquiry is preventing Create Connection, so we don't need to
> overcomplicate it by stopping the mgmt discovery completely. In the
> case of interleaved discovery, the LE discovery will linger for a
> little bit before eventually being disabled and the situation restarts
> at a good state. Not perfect but simple, clean, and fixes the issue.
can we get a mgmt-tester test case for this that also checks the various combinations of this.
And coming to think about it, should we mark controllers as “good” if they allow create connection and inquiry at the same time? So we do this conditionally based on what controller we have.
Eventually we have to get to the level to create a proper state machine for BR/EDR and LE so that we serialize all of these commands accordingly. That way we can easily continue where we left off. This should also come in handy for any kind of suspend / resume handling. Since at the end of the day it is all the same problem. We need to pause some radio activity and continue afterwards.
In case this wasn’t clear, I am not in favor of just sending a command and hope for the best. That got us in trouble with BlueZ 3.x and to large extend also with BlueZ 4.x since weird corner cases will bring us out of sync.
Regards
Marcel
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-12-03 9:57 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-11-24 1:09 [PATCH] Bluetooth: Cancel Inquiry before Create Connection Sonny Sasaka
2020-11-24 1:35 ` bluez.test.bot
2020-11-25 14:43 ` [PATCH] " Marcel Holtmann
2020-11-26 0:00 ` Sonny Sasaka
2020-12-02 23:24 ` Sonny Sasaka
2020-12-03 9:56 ` Marcel Holtmann
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox