| Message ID | 20230821102810.Bluez.v1.1.Ib819b0964a5b8339305d94611acab85774a6c8ce@changeid |
|---|---|
| State | New |
| Headers | show |
| Series | [Bluez,v1] adapter: check wake_override when setting device privacy | expand |
Hi, On Mon, Aug 21, 2023 at 10:28 AM Zhengping Jiang <jiangzp@google.com> 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@intel.com> 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@google.com> > --- > > 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 >
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=777963 ---Test result--- Test Summary: CheckPatch PASS 0.62 seconds GitLint PASS 0.38 seconds BuildEll PASS 33.46 seconds BluezMake PASS 1188.33 seconds MakeCheck PASS 13.16 seconds MakeDistcheck PASS 194.36 seconds CheckValgrind PASS 317.52 seconds CheckSmatch PASS 440.99 seconds bluezmakeextell PASS 134.63 seconds IncrementalBuild PASS 1031.74 seconds ScanBuild PASS 1340.51 seconds --- Regards, Linux Bluetooth
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.
Thanks,
Zhengping
On Mon, Aug 21, 2023 at 11:00 AM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi,
>
> On Mon, Aug 21, 2023 at 10:28 AM Zhengping Jiang <jiangzp@google.com> 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@intel.com>
> 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@google.com>
> > ---
> >
> > 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
Hi, On Mon, Aug 21, 2023 at 12:51 PM Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote: > > Hi, > > On Mon, Aug 21, 2023 at 12:45 PM Zhengping Jiang <jiangzp@google.com> 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. Note you need LL Privacy on the scanner side, not on the peripheral that is advertising, so if you got this the other way around it will not work. RPA resolution is required to be able to resolve the advertisements using RPAs since the accept list can only be programmed with the identity address. > > > > > 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@iki.fi/ > > 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@gmail.com> wrote: > > > > > > Hi, > > > > > > On Mon, Aug 21, 2023 at 10:28 AM Zhengping Jiang <jiangzp@google.com> 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@intel.com> > > > 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@google.com> > > > > --- > > > > > > > > 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
Hi Luiz, A few questions on the patch. (1) I checked in the new_irk_callback, the patch calls device_set_rpa(device, true). This feels like the assumption is still having IRK means using RPA. If I understand correctly, this is not true. For example, the MX Master 3 mouse I am using. It is using an identity address, but still having IRK. This is from the pairing log, the RPA is empty in the callback. > src/adapter.c:new_irk_callback() hci0 new IRK for E3:C1:E3:A6:10:D9 RPA 00:00:00:00:00:00 I think we may need to check the type of the conn_bdaddr and make sure it is a RPA. (2) > Note you need LL Privacy on the scanner side, not on the peripheral > that is advertising, so if you got this the other way around it will > not work. RPA resolution is required to be able to resolve the > advertisements using RPAs since the accept list can only be programmed > with the identity address. Regarding this, I have enabled privacy in both scanner and peripheral. As shown in the log, after the adapter is powered on. The identity address is added to the accept list and the IRK is added to the resolving list. Then the address resolution is enabled. < HCI Command: LE Add Device To Resolving List (0x08|0x0027) plen 39 #582 [hci0] 16:06:32.070202 Address type: Public (0x00) Address: DC:A6:32:A4:09:DF (OUI DC-A6-32) Peer identity resolving key: b724b925ee4e955d723fa2247bba24a0 Local identity resolving key: 00000000000000000000000000000000 ... < HCI Command: LE Set Privacy Mode (0x08|0x004e) plen 8 #584 [hci0] 16:06:32.071113 Peer Identity address type: Public (0x00) Peer Identity address: DC:A6:32:A4:09:DF (OUI DC-A6-32) Privacy Mode: Use Device Privacy (0x01) ... < HCI Command: LE Add Device To Accept List (0x08|0x0011) plen 7 #586 [hci0] 16:06:32.072126 Address type: Public (0x00) Address: DC:A6:32:A4:09:DF (OUI DC-A6-32) ... < HCI Command: LE Set Address Resolution Enable (0x08|0x002d) plen 1 #588 [hci0] 16:06:32.073130 Address resolution: Enabled (0x01) > HCI Event: Command Complete (0x0e) plen 4 #589 [hci0] 16:06:32.073986 LE Set Address Resolution Enable (0x08|0x002d) ncmd 1 Status: Success (0x00) The scan policy is set to 0x01. < HCI Command: LE Set Extended Scan Parameters (0x08|0x0041) plen 8 #590 [hci0] 16:06:32.074074 Own address type: Public (0x00) Filter policy: Ignore not in accept list (0x01) PHYs: 0x01 Entry 0: LE 1M Type: Passive (0x00) Interval: 367.500 msec (0x024c) Window: 37.500 msec (0x003c) The advertisement also shows the address is resolved. > HCI Event: LE Meta Event (0x3e) plen 32 #604 [hci0] 16:07:05.292173 LE Extended Advertising Report (0x0d) ... Address type: Resolved Public (0x02) Address: DC:A6:32:A4:09:DF (OUI DC-A6-32) (3) Given comment (2), when LL privacy is enabled both on scanner and peripheral, the issue still exists in hog_probe. The connection is built with the RPA, so the connection cannot be found before adding the device is done. It will fail at hci_conn_params_lookup. As set_device_flags is always called with identity address. params = hci_conn_params_lookup(hdev, &cp->addr.bdaddr, le_addr_type(cp->addr.type)); During add_device, a new hci_conn_params will be added with hci_conn_params_set. Then the following set_device_flags will not fail. Zhengping Thanks, Zhengping On Mon, Aug 21, 2023 at 12:59 PM Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote: > > Hi, > > On Mon, Aug 21, 2023 at 12:51 PM Luiz Augusto von Dentz > <luiz.dentz@gmail.com> wrote: > > > > Hi, > > > > On Mon, Aug 21, 2023 at 12:45 PM Zhengping Jiang <jiangzp@google.com> 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. > > Note you need LL Privacy on the scanner side, not on the peripheral > that is advertising, so if you got this the other way around it will > not work. RPA resolution is required to be able to resolve the > advertisements using RPAs since the accept list can only be programmed > with the identity address. > > > > > > > > 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@iki.fi/ > > > > 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@gmail.com> wrote: > > > > > > > > Hi, > > > > > > > > On Mon, Aug 21, 2023 at 10:28 AM Zhengping Jiang <jiangzp@google.com> 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@intel.com> > > > > 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@google.com> > > > > > --- > > > > > > > > > > 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 > > > > -- > Luiz Augusto von Dentz
Hi, On Mon, Aug 21, 2023 at 9:10 PM Zhengping Jiang <jiangzp@google.com> wrote: > > Hi Luiz, > > A few questions on the patch. > (1) I checked in the new_irk_callback, the patch calls > device_set_rpa(device, true). This feels like the assumption is still > having IRK means using RPA. If I understand correctly, this is not > true. For example, the MX Master 3 mouse I am using. It is using an > identity address, but still having IRK. > > This is from the pairing log, the RPA is empty in the callback. > > src/adapter.c:new_irk_callback() hci0 new IRK for E3:C1:E3:A6:10:D9 RPA 00:00:00:00:00:00 > > I think we may need to check the type of the conn_bdaddr and make sure > it is a RPA. The presence of an IRK means the device _can_ use RPA so it doesn't really matter if the connection was done using an identity address or not, that said some devices have the logic to user itd identity address if they are nothing connects after some time so perhaps we can detect that somehow since that means we can use the allow list. > (2) > > Note you need LL Privacy on the scanner side, not on the peripheral > > that is advertising, so if you got this the other way around it will > > not work. RPA resolution is required to be able to resolve the > > advertisements using RPAs since the accept list can only be programmed > > with the identity address. > > Regarding this, I have enabled privacy in both scanner and peripheral. > As shown in the log, after the adapter is powered on. The identity > address is added to the accept list and the IRK is added to the > resolving list. Then the address resolution is enabled. > > < HCI Command: LE Add Device To Resolving List (0x08|0x0027) plen 39 > > #582 [hci0] 16:06:32.070202 > Address type: Public (0x00) > Address: DC:A6:32:A4:09:DF (OUI DC-A6-32) > Peer identity resolving key: b724b925ee4e955d723fa2247bba24a0 > Local identity resolving key: 00000000000000000000000000000000 > ... > < HCI Command: LE Set Privacy Mode (0x08|0x004e) plen 8 > > #584 [hci0] 16:06:32.071113 > Peer Identity address type: Public (0x00) > Peer Identity address: DC:A6:32:A4:09:DF (OUI DC-A6-32) > Privacy Mode: Use Device Privacy (0x01) > ... > < HCI Command: LE Add Device To Accept List (0x08|0x0011) plen 7 > > #586 [hci0] 16:06:32.072126 > Address type: Public (0x00) > Address: DC:A6:32:A4:09:DF (OUI DC-A6-32) > ... > < HCI Command: LE Set Address Resolution Enable (0x08|0x002d) plen 1 > > #588 [hci0] 16:06:32.073130 > Address resolution: Enabled (0x01) > > HCI Event: Command Complete (0x0e) plen 4 #589 [hci0] 16:06:32.073986 > LE Set Address Resolution Enable (0x08|0x002d) ncmd 1 > Status: Success (0x00) > > The scan policy is set to 0x01. > > < HCI Command: LE Set Extended Scan Parameters (0x08|0x0041) plen 8 > > #590 [hci0] 16:06:32.074074 > Own address type: Public (0x00) > Filter policy: Ignore not in accept list (0x01) > PHYs: 0x01 > Entry 0: LE 1M > Type: Passive (0x00) > Interval: 367.500 msec (0x024c) > Window: 37.500 msec (0x003c) > > The advertisement also shows the address is resolved. > > > HCI Event: LE Meta Event (0x3e) plen 32 #604 [hci0] 16:07:05.292173 > LE Extended Advertising Report (0x0d) > ... > Address type: Resolved Public (0x02) > Address: DC:A6:32:A4:09:DF (OUI DC-A6-32) > > (3) Given comment (2), when LL privacy is enabled both on scanner and > peripheral, the issue still exists in hog_probe. The connection is > built with the RPA, so the connection cannot be found before adding > the device is done. It will fail at hci_conn_params_lookup. As > set_device_flags is always called with identity address. Not really following, what is failing on hog_probe? Also please try with master since we have merged some changes wrt to how experimental is enabled. > params = hci_conn_params_lookup(hdev, &cp->addr.bdaddr, > le_addr_type(cp->addr.type)); > > During add_device, a new hci_conn_params will be added with > hci_conn_params_set. Then the following set_device_flags will not > fail. > > Zhengping > > > Thanks, > Zhengping > > > > > > On Mon, Aug 21, 2023 at 12:59 PM Luiz Augusto von Dentz > <luiz.dentz@gmail.com> wrote: > > > > Hi, > > > > On Mon, Aug 21, 2023 at 12:51 PM Luiz Augusto von Dentz > > <luiz.dentz@gmail.com> wrote: > > > > > > Hi, > > > > > > On Mon, Aug 21, 2023 at 12:45 PM Zhengping Jiang <jiangzp@google.com> 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. > > > > Note you need LL Privacy on the scanner side, not on the peripheral > > that is advertising, so if you got this the other way around it will > > not work. RPA resolution is required to be able to resolve the > > advertisements using RPAs since the accept list can only be programmed > > with the identity address. > > > > > > > > > > > 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@iki.fi/ > > > > > > 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@gmail.com> wrote: > > > > > > > > > > Hi, > > > > > > > > > > On Mon, Aug 21, 2023 at 10:28 AM Zhengping Jiang <jiangzp@google.com> 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@intel.com> > > > > > 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@google.com> > > > > > > --- > > > > > > > > > > > > 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 > > > > > > > > -- > > Luiz Augusto von Dentz
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; /* 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,
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. When setting the device privacy flag, the wakeup flag should be set according to the wake_override, instead of the current flags. Signed-off-by: Zhengping Jiang <jiangzp@google.com> --- 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(+)