Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20201124010906.340433-1-sonnysasaka@chromium.org> <92C0EFAB-EDF1-4EDB-ADE7-FF734928C8AE@holtmann.org>
In-Reply-To: <92C0EFAB-EDF1-4EDB-ADE7-FF734928C8AE@holtmann.org>
From:   Sonny Sasaka <sonnysasaka@chromium.org>
Date:   Wed, 25 Nov 2020 16:00:07 -0800
Message-ID: <CAO271m=geLEUjtPhFETfqsYTsxDQoe+=c3WeYPbDuNjwc+4idA@mail.gmail.com>
Subject: Re: [PATCH] Bluetooth: Cancel Inquiry before Create Connection
To:     Marcel Holtmann <marcel@holtmann.org>
Cc:     BlueZ development <linux-bluetooth@vger.kernel.org>,
        Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

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
>