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> <CAO271m=geLEUjtPhFETfqsYTsxDQoe+=c3WeYPbDuNjwc+4idA@mail.gmail.com>
In-Reply-To: <CAO271m=geLEUjtPhFETfqsYTsxDQoe+=c3WeYPbDuNjwc+4idA@mail.gmail.com>
From:   Sonny Sasaka <sonnysasaka@chromium.org>
Date:   Wed, 2 Dec 2020 15:24:58 -0800
Message-ID: <CAO271m=E1=+JYzYmOugMGGxByTs+k-EhuCLwsoc6n67mEhpbmg@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,

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
> >