Hi,
On Mon, Aug 21, 2023 at 12:45 PM Zhengping Jiang <jiangzp@xxxxxxxxxx> wrote:
>
> Hi Luiz,
>
> Thanks for the reply. I applied the above patch "device: Fix enabling
> wake support without RPA Resolution" and tried again. I can see there
> are still some things missing.
> I have attached a btsnoop log for reference. Here I am using a rasp pi
> configured as a LE mouse which is in privacy mode.
>
> The ll privacy feature is enabled at the beginning.
> @ MGMT Command: Set Experimental Feature (0x004a) plen 17
> {0x0003} [hci0]
> 19:21:30.193783
> UUID: BlueZ Experimental LL privacy
> Action: Enabled (0x01)
>
> Later the LE advertisement with RPA is received.
>
> > HCI Event: LE Meta Event (0x3e) plen 32 #311 [hci0] 19:22:36.633724
> LE Extended Advertising Report (0x0d)
> Num reports: 1
> Entry 0
> Event type: 0x0013
> Props: 0x0013
> Connectable
> Scannable
> Use legacy advertising PDUs
> Data status: [0;32mComplete [0m
> Legacy PDU Type: ADV_IND (0x0013)
> Address type: Random (0x01)
> Address: 6F:CB:45:0C:AD:1F (Resolvable)
>
> But I still get the error for not being able to set wake_support. I
> think this is not right as the address resolution is already enabled.
> I haven't checked the details yet.
>
> = bluetoothd: Unable to set wake_support without RPA resolution
>
> 19:22:41.337325
>
> Then the wake up flag is not set as a result.
>
> @ MGMT Event: Device Flags Changed (0x002a) plen 15
> {0x0003} [hci0]
> 19:22:41.372969
> LE Address: DC:A6:32:A4:08:BF (OUI DC-A6-32)
> Supported Flags: 0x00000003
> Remote Wakeup
> Device Privacy Mode
> Current Flags: 0x00000002
> Device Privacy Mode
>
> The address resolution is enabled as shown later in the log, after the
> adapter reboot and the LE mouse can reconnect with RPA.
>
> < HCI Command: LE Set Address Resolution Enable (0x08|0x002d) plen 1
> #601 [hci0]
> 19:22:57.164690
> Address resolution: Disabled (0x00)
> > HCI Event: Command Complete (0x0e) plen 4 #602 [hci0] 19:22:57.165530
> LE Set Address Resolution Enable (0x08|0x002d) ncmd 1
> Status: Success (0x00)
> < HCI Command: LE Add Device To Resolving List (0x08|0x0027) plen 39
> #603 [hci0]
> 19:22:57.165663
> Address type: Public (0x00)
> Address: DC:A6:32:A4:08:BF (OUI DC-A6-32)
> Peer identity resolving key: 0bf179254f3b2eb51324711e416f22e8
> Local identity resolving key: 00000000000000000000000000000000
> ...
>
> Before applying the above patch, there will be a set device flag
> command in the place of the error, but the command will fail.
>
> @ MGMT Command: Set Device Flags (0x0050) plen 11
> {0x0003} [hci0]
> 00:42:59.247080
> LE Address: E4:5F:01:89:8F:85 (OUI E4-5F-01)
> Current Flags: 0x00000001
> Remote Wakeup
> @ MGMT Event: Command Complete (0x0001) plen 10
> {0x0003} [hci0]
> 00:42:59.247129
> Set Device Flags (0x0050) plen 7
> Status: Invalid Parameters (0x0d)
> LE Address: E4:5F:01:89:8F:85 (OUI E4-5F-01)
>
> Please let me know your opinion.
Perhaps we need the following patch in order to make sure the
experimental flags are enabled _before_ wake_support logic is
triggered:
https://patchwork.kernel.org/project/bluetooth/patch/00052eaeb78774fd7be365805203cb0c8b189243.1692531437.git.pav@xxxxxx/
I'm in the process of applying it since it we do have similar problem
with the likes of ISO sockets that needs to be enabled before loading
drivers, etc.
> Thanks,
> Zhengping
>
> On Mon, Aug 21, 2023 at 11:00 AM Luiz Augusto von Dentz
> <luiz.dentz@xxxxxxxxx> wrote:
> >
> > Hi,
> >
> > On Mon, Aug 21, 2023 at 10:28 AM Zhengping Jiang <jiangzp@xxxxxxxxxx> wrote:
> > >
> > > For the device using a RPA, hog_probe sets wake_override to true, but
> > > the command to set remote wakeup fails because the device has not been
> > > added to the kernel and the connection with the public address cannot be
> > > found.
> >
> > This is actually not true, hog_probe does call
> > device_set_wake_support, also note that I had a fix on how we handle
> > RPA with remote wakeup because that depends on LL Privacy/RPA
> > Resolution:
> >
> > commit 7a4b67f9caa6bdc004c910f3a52108744a8cab74
> > Author: Luiz Augusto von Dentz <luiz.von.dentz@xxxxxxxxx>
> > Date: Thu May 12 16:40:49 2022 -0700
> >
> > device: Fix enabling wake support without RPA Resolution
> >
> > If device uses RPA it shall only enable wakeup if RPA Resolution has
> > been enabled otherwise it cannot be programmed in the acceptlist which
> > can cause suspend to fail.
> >
> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=215768
> >
> >
> > > When setting the device privacy flag, the wakeup flag should be set
> > > according to the wake_override, instead of the current flags.
> >
> > Afaik wake_override is only set by the user via WakeAllowed, plugins
> > shall not be using it directly, so it really sounds like this doesn't
> > apply to current upstream.
> >
> > > Signed-off-by: Zhengping Jiang <jiangzp@xxxxxxxxxx>
> > > ---
> > >
> > > Changes in v1:
> > > - Add function to fetch wake_override value
> > > - Set the remote wakeup bit if privacy mode is used when setting flags
> > >
> > > src/adapter.c | 2 ++
> > > src/device.c | 5 +++++
> > > src/device.h | 1 +
> > > 3 files changed, 8 insertions(+)
> > >
> > > diff --git a/src/adapter.c b/src/adapter.c
> > > index 004062e7c..f63018495 100644
> > > --- a/src/adapter.c
> > > +++ b/src/adapter.c
> > > @@ -5520,6 +5520,8 @@ static void add_device_complete(uint8_t status, uint16_t length,
> > > if (btd_opts.device_privacy) {
> > > uint32_t flags = btd_device_get_current_flags(dev);
> > >
> > > + if (device_get_wake_override(dev))
> > > + flags |= DEVICE_FLAG_REMOTE_WAKEUP;
> >
> > We shall only use override if wake_support is set.
> >
> > > /* Set Device Privacy Mode has not set the flag yet. */
> > > if (!(flags & DEVICE_FLAG_DEVICE_PRIVACY)) {
> > > adapter_set_device_flags(adapter, dev, flags |
> > > diff --git a/src/device.c b/src/device.c
> > > index 9b58e0c4e..ae75f2fd1 100644
> > > --- a/src/device.c
> > > +++ b/src/device.c
> > > @@ -1545,6 +1545,11 @@ void device_set_wake_override(struct btd_device *device, bool wake_override)
> > > }
> > > }
> > >
> > > +bool device_get_wake_override(struct btd_device *device)
> > > +{
> > > + return device->wake_override;
> > > +}
> > > +
> > > static void device_set_wake_allowed_complete(struct btd_device *device)
> > > {
> > > if (device->wake_id != -1U) {
> > > diff --git a/src/device.h b/src/device.h
> > > index 3252e14ef..79377a13a 100644
> > > --- a/src/device.h
> > > +++ b/src/device.h
> > > @@ -141,6 +141,7 @@ void device_set_wake_support(struct btd_device *device, bool wake_support);
> > > void device_set_wake_override(struct btd_device *device, bool wake_override);
> > > void device_set_wake_allowed(struct btd_device *device, bool wake_allowed,
> > > guint32 id);
> > > +bool device_get_wake_override(struct btd_device *device);
> > > void device_set_refresh_discovery(struct btd_device *dev, bool refresh);
> > >
> > > typedef void (*disconnect_watch) (struct btd_device *device, gboolean removal,
> > > --
> > > 2.42.0.rc1.204.g551eb34607-goog
> > >
> >
> >
> > --
> > Luiz Augusto von Dentz
--
Luiz Augusto von Dentz
