Message-ID: <CAB4PzUoj2=QNH0SqrSe8LbT74Z7DZr-K6Qw=b71k20a=1aLuSg@mail.gmail.com>
Date:   Tue, 14 Feb 2023 17:19:54 -0800
From:   Zhengping Jiang <jiangzp@...gle.com>
To:     Luiz Augusto von Dentz <luiz.dentz@...il.com>
Cc:     linux-bluetooth@...r.kernel.org, marcel@...tmann.org,
        chromeos-bluetooth-upstreaming@...omium.org,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Johan Hedberg <johan.hedberg@...il.com>,
        Paolo Abeni <pabeni@...hat.com>, linux-kernel@...r.kernel.org,
        netdev@...r.kernel.org
Subject: Re: [kernel PATCH v1] Bluetooth: hci_sync: Resume adv with no RPA
 when active scan

Hi Luiz,

> Where is that mentioned?
It is just below the command on "7.8.44 LE Set Address Resolution
Enable command".

On "4.7 RESOLVING LIST", there is another note:
> Note: The Controller may generate Resolvable Private Addresses even when address resolution is disabled.

If this is the case, then the comment in the kernel
(hci_active_scan_sync) is not accurate.
> /* Pause advertising since active scanning disables address resolution
> * which advertising depend on in order to generate its RPAs.
> */

> I think it may be related to the fact that it only affects the addr
> resolution of remote devices, that said if you are active scanning
> that probably means the user wants to setup a new device thus why we
> don't enable any filtering like accept list, etc, so it is not really
> useful to keep address resolution active either way.
That makes sense. When the local privacy is enabled, I assume the host
RPA will change
when advertising. I haven't tested that scenario, but if RPA
generation is not related to disable/enable
address resolution, why should the advertising be paused when active scan?

Thanks,
Zhengping

>
> > Thanks,
> > Zhengping
> >
> > On Tue, Feb 14, 2023 at 4:09 PM Luiz Augusto von Dentz
> > <luiz.dentz@...il.com> wrote:
> > >
> > > Hi Zhengping,
> > >
> > > On Tue, Feb 14, 2023 at 2:56 PM Zhengping Jiang <jiangzp@...gle.com> wrote:
> > > >
> > > > The address resolution should be disabled during the active scan,
> > > > so all the advertisements can reach the host. The advertising
> > > > has to be paused before disabling the address resolution,
> > > > because the advertising will prevent any changes to the resolving
> > > > list and the address resolution status. Skipping this will cause
> > > > the hci error and the discovery failure.
> > >
> > > It is probably a good idea to quote the spec saying:
> > >
> > > 7.8.44 LE Set Address Resolution Enable command
> > >
> > > This command shall not be used when:
> > > • Advertising (other than periodic advertising) is enabled,
> > >
> > > > If the host is using RPA, the controller needs to generate RPA for
> > > > the advertising, so the advertising must remain paused during the
> > > > active scan.
> > > >
> > > > If the host is not using RPA, the advertising can be resumed after
> > > > disabling the address resolution.
> > > >
> > > > Fixes: 9afc675edeeb ("Bluetooth: hci_sync: allow advertise when scan without RPA")
> > > > Signed-off-by: Zhengping Jiang <jiangzp@...gle.com>
> > > > ---
> > > >
> > > > Changes in v1:
> > > > - Always pause advertising when active scan, but resume the advertising if the host is not using RPA
> > > >
> > > >  net/bluetooth/hci_sync.c | 8 ++++++--
> > > >  1 file changed, 6 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
> > > > index 117eedb6f709..edbf9faf7fa1 100644
> > > > --- a/net/bluetooth/hci_sync.c
> > > > +++ b/net/bluetooth/hci_sync.c
> > > > @@ -2402,7 +2402,7 @@ static u8 hci_update_accept_list_sync(struct hci_dev *hdev)
> > > >         u8 filter_policy;
> > > >         int err;
> > > >
> > > > -       /* Pause advertising if resolving list can be used as controllers are
> > > > +       /* Pause advertising if resolving list can be used as controllers
> > > >          * cannot accept resolving list modifications while advertising.
> > > >          */
> > > >         if (use_ll_privacy(hdev)) {
> > > > @@ -5397,7 +5397,7 @@ static int hci_active_scan_sync(struct hci_dev *hdev, uint16_t interval)
> > > >         /* Pause advertising since active scanning disables address resolution
> > > >          * which advertising depend on in order to generate its RPAs.
> > > >          */
> > > > -       if (use_ll_privacy(hdev) && hci_dev_test_flag(hdev, HCI_PRIVACY)) {
> > > > +       if (use_ll_privacy(hdev)) {
> > > >                 err = hci_pause_advertising_sync(hdev);
> > > >                 if (err) {
> > > >                         bt_dev_err(hdev, "pause advertising failed: %d", err);
> > > > @@ -5416,6 +5416,10 @@ static int hci_active_scan_sync(struct hci_dev *hdev, uint16_t interval)
> > > >                 goto failed;
> > > >         }
> > > >
> > > > +       // Resume paused advertising if the host is not using RPA
> > > > +       if (use_ll_privacy(hdev) && !hci_dev_test_flag(hdev, HCI_PRIVACY))
> > > > +               hci_resume_advertising_sync(hdev);
> > > > +
> > > >         /* All active scans will be done with either a resolvable private
> > > >          * address (when privacy feature has been enabled) or non-resolvable
> > > >          * private address.
> > > > --
> > > > 2.39.1.581.gbfd45094c4-goog
> > >
> > > I think it is better that we add something like
> > > hci_pause_addr_resolution so we can make it check all the conditions,
> > > such as pausing advertising and resuming if needed. Btw, we do seem to
> > > have proper checks for these conditions on the emulator:
> > >
> > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/emulator/btdev.c#n4090
> > >
> > > But perhaps there is no test which attempts to enable LL Privacy
> > > without enabling Local Privacy, so it would be great if you could
> > > update mgmt-tester adding a test that emulates such behavior.
> > >
> > > --
> > > Luiz Augusto von Dentz
>
>
>
> --
> Luiz Augusto von Dentz

Powered by blists - more mailing lists