[BlueZ PATCH v4 0/3] Add Advertisement Monitor Device Found/Lost events

[BlueZ PATCH v4 0/3] Add Advertisement Monitor Device Found/Lost events

[Manish Mandlik]

Hello Bt-Maintainers,

Bluetooth Advertisement Monitor API was introduced to support background
scanning and proximity detection based on the application specified RSSI
thresholds and content filters on LE advertisement packets.

To optimize the power consumption, the API offloads the content
filtering and RSSI tracking to the controller if the controller
offloading support is available. However, this monitoring is not
completely offloaded as the bluetoothd also handles RSSI thresholds and
timeouts in order to fulfill high/low thresholds/timeouts filtering with
D-bus clients.

There is further room to achieve better power optimization by supporting
the controller event HCI_VS_MSFT_LE_Monitor_Device_Event to fulfill true
monitor offloading. This is currently not supported as it was originally
desired to minimize the changes to the MGMT interface and reuse the
existing MGMT event - MGMT_EV_DEVICE_FOUND to pass advertisements to
bluetoothd and let bluetoothd handle the RSSI thresholds and timeouts in
order to fulfill the D-bus API requirements for the client.

This patch series introduces MGMT event MGMT_EV_ADV_MONITOR_DEVICE_FOUND
and MGMT_EV_ADV_MONITOR_DEVICE_LOST to indicate that the controller has
started/stopped monitoring a particular device.

Please let me know what you think about this or if you have any further
questions.

Thanks,
Manish.

Changes in v4:
- Add Advertisement Monitor Device Found event, make Address_Type 0 as
  reserved.
- Add Advertisement Monitor Device Found event.
- Add Advertisement Monitor Device Found event.

Changes in v3:
- Discard changes to the Device Found event and notify bluetoothd only
  when the controller stops monitoring the device via new Device Lost
  event.
- Discard changes to the Device Found event and notify bluetoothd only
  when the controller stops monitoring the device via new Device Lost
  event.
- Fix indentation of the adv_monitor_device_lost_callback() name and
  it's arguments.

Changes in v2:
- Instead of creating a new 'Device Tracking' event, add a flag 'Device
  Tracked' in the existing 'Device Found' event and add a new 'Device
  Lost' event to indicate that the controller has stopped tracking that
  device.
- Instead of creating a new 'Device Tracking' event, add a flag 'Device
  Tracked' in the existing 'Device Found' event and add a new 'Device
  Lost' event to indicate that the controller has stopped tracking that
  device.
- Update function name adv_monitor_tracking_callback() to
  adv_monitor_device_lost_callback() as it will receive only Device Lost
  event.

Manish Mandlik (3):
  doc: Introduce the Adv Monitor Device Found/Lost events
  lib: Add definitions of the Adv Monitor Device Found/Lost events
  adv_monitor: Receive the Device Found/Lost events

 doc/mgmt-api.txt  | 50 ++++++++++++++++++++++++++++++++++++++++++++++-
 lib/mgmt.h        | 14 +++++++++++++
 src/adv_monitor.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 113 insertions(+), 1 deletion(-)

-- 
2.33.0.1079.g6e70778dc9-goog

[BlueZ PATCH v4 1/3] doc: Introduce the Adv Monitor Device Found/Lost events

[Manish Mandlik]

This patch introduces new MGMT events to indicate that the controller
has started/stopped tracking a particular device matching one of the
already added Advertisement Monitor.

---

Changes in v4:
- Add Advertisement Monitor Device Found event, make Address_Type 0 as
  reserved.

Changes in v3:
- Discard changes to the Device Found event and notify bluetoothd only
  when the controller stops monitoring the device via new Device Lost
  event.

Changes in v2:
- Instead of creating a new 'Device Tracking' event, add a flag 'Device
  Tracked' in the existing 'Device Found' event and add a new 'Device
  Lost' event to indicate that the controller has stopped tracking that
  device.

 doc/mgmt-api.txt | 50 +++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 49 insertions(+), 1 deletion(-)

diff --git a/doc/mgmt-api.txt b/doc/mgmt-api.txt
index 5355fedb0..32dd8c0f2 100644
--- a/doc/mgmt-api.txt
+++ b/doc/mgmt-api.txt
@@ -107,7 +107,8 @@ Configuration command, Default Runtime Configuration Changed event, Get
 Device Flags command, Set Device Flags command, Device Flags Changed event,
 Read Advertisement Monitor Features command, Add Advertisement Patterns
 Monitor command, Remove Advertisement Monitor command, Advertisement Monitor
-Added event and Advertisement Monitor Removed event.
+Added event, Advertisement Monitor Removed event, Advertisement Monitor Device
+Found event and Advertisement Monitor Device Lost event.
 
 
 Example
@@ -4910,3 +4911,50 @@ Controller Resume Event
 	Address_Type. Otherwise, Address and Address_Type will both be zero.
 
 	This event will be sent to all management sockets.
+
+
+Advertisement Monitor Device Found Event
+========================================
+
+	Event code:		0x002f
+	Controller Index:	<controller_id>
+	Event Parameters:	Monitor_Handle (2 Octets)
+				Address (6 Octets)
+				Address_Type (1 Octet)
+
+	This event indicates that the controller has started tracking a device
+	matching an Advertisement Monitor with handle Monitor_Handle.
+
+	The address of the device being tracked will be shared in Address and
+	Address_Type.
+
+	Possible values for the Address_Type parameter:
+		0	Reserved (not in use)
+		1	LE Public
+		2	LE Random
+
+	This event will be sent to all management sockets.
+
+
+Advertisement Monitor Device Lost Event
+=======================================
+
+	Event code:		0x0030
+	Controller Index:	<controller_id>
+	Event Parameters:	Monitor_Handle (2 Octets)
+				Address (6 Octets)
+				Address_Type (1 Octet)
+
+	This event indicates that the controller has stopped tracking a device
+	that was being tracked by an Advertisement Monitor with handle
+	Monitor_Handle.
+
+	The address of the device being tracked will be shared in Address and
+	Address_Type.
+
+	Possible values for the Address_Type parameter:
+		0	Reserved (not in use)
+		1	LE Public
+		2	LE Random
+
+	This event will be sent to all management sockets.
-- 
2.33.0.1079.g6e70778dc9-goog

[BlueZ PATCH v4 2/3] lib: Add definitions of the Adv Monitor Device Found/Lost events

[Manish Mandlik]

This patch adds definitions of the new Advertisement Monitor Device
Found and Device Lost events to indicate that the controller has
started/stopped tracking a particular device.

---

Changes in v4:
- Add Advertisement Monitor Device Found event.

Changes in v3:
- Discard changes to the Device Found event and notify bluetoothd only
  when the controller stops monitoring the device via new Device Lost
  event.

Changes in v2:
- Instead of creating a new 'Device Tracking' event, add a flag 'Device
  Tracked' in the existing 'Device Found' event and add a new 'Device
  Lost' event to indicate that the controller has stopped tracking that
  device.

 lib/mgmt.h | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/lib/mgmt.h b/lib/mgmt.h
index 0a6349321..50519c065 100644
--- a/lib/mgmt.h
+++ b/lib/mgmt.h
@@ -1014,6 +1014,18 @@ struct mgmt_ev_controller_resume {
 	uint8_t wake_reason;
 } __packed;
 
+#define MGMT_EV_ADV_MONITOR_DEVICE_FOUND	0x002f
+struct mgmt_ev_adv_monitor_device_found {
+	uint16_t monitor_handle;
+	struct mgmt_addr_info addr;
+} __packed;
+
+#define MGMT_EV_ADV_MONITOR_DEVICE_LOST		0x0030
+struct mgmt_ev_adv_monitor_device_lost {
+	uint16_t monitor_handle;
+	struct mgmt_addr_info addr;
+} __packed;
+
 static const char *mgmt_op[] = {
 	"<0x0000>",
 	"Read Version",
@@ -1152,6 +1164,8 @@ static const char *mgmt_ev[] = {
 	"Advertisement Monitor Removed",
 	"Controller Suspend",
 	"Controller Resume",
+	"Advertisement Monitor Device Found",		/* 0x002f */
+	"Advertisement Monitor Device Lost",
 };
 
 static const char *mgmt_status[] = {
-- 
2.33.0.1079.g6e70778dc9-goog

[BlueZ PATCH v4 3/3] adv_monitor: Receive the Device Found/Lost events

[Manish Mandlik]

This patch registers callback finctions to receive the Advertisement
Monitor Device Found and Device Lost events.

Test performed:
- verified by logs that the MSFT Monitor Device is received from the
  controller and the bluetoothd is notified whenever the controller
  starts/stops monitoring a device.

---

Changes in v4:
- Add Advertisement Monitor Device Found event.

Changes in v3:
- Fix indentation of the adv_monitor_device_lost_callback() name and
  it's arguments.

Changes in v2:
- Update function name adv_monitor_tracking_callback() to
  adv_monitor_device_lost_callback() as it will receive only Device Lost
  event.

 src/adv_monitor.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

diff --git a/src/adv_monitor.c b/src/adv_monitor.c
index 715ac5904..2aae5d372 100644
--- a/src/adv_monitor.c
+++ b/src/adv_monitor.c
@@ -1531,6 +1531,48 @@ static void adv_monitor_removed_callback(uint16_t index, uint16_t length,
 		ev->monitor_handle);
 }
 
+/* Processes Adv Monitor Device Found event from kernel */
+static void adv_monitor_device_found_callback(uint16_t index, uint16_t length,
+					const void *param, void *user_data)
+{
+	struct btd_adv_monitor_manager *manager = user_data;
+	const struct mgmt_ev_adv_monitor_device_found *ev = param;
+	uint16_t handle = le16_to_cpu(ev->monitor_handle);
+	const uint16_t adapter_id = manager->adapter_id;
+	char addr[18];
+
+	if (length < sizeof(*ev)) {
+		btd_error(adapter_id,
+				"Wrong size of Adv Monitor Device Found event");
+		return;
+	}
+
+	ba2str(&ev->addr.bdaddr, addr);
+	DBG("Adv Monitor with handle 0x%04x started tracking the device %s",
+			handle, addr);
+}
+
+/* Processes Adv Monitor Device Lost event from kernel */
+static void adv_monitor_device_lost_callback(uint16_t index, uint16_t length,
+					const void *param, void *user_data)
+{
+	struct btd_adv_monitor_manager *manager = user_data;
+	const struct mgmt_ev_adv_monitor_device_lost *ev = param;
+	uint16_t handle = le16_to_cpu(ev->monitor_handle);
+	const uint16_t adapter_id = manager->adapter_id;
+	char addr[18];
+
+	if (length < sizeof(*ev)) {
+		btd_error(adapter_id,
+				"Wrong size of Adv Monitor Device Lost event");
+		return;
+	}
+
+	ba2str(&ev->addr.bdaddr, addr);
+	DBG("Adv Monitor with handle 0x%04x stopped tracking the device %s",
+			handle, addr);
+}
+
 /* Allocates a manager object */
 static struct btd_adv_monitor_manager *manager_new(
 						struct btd_adapter *adapter,
@@ -1555,6 +1597,14 @@ static struct btd_adv_monitor_manager *manager_new(
 			manager->adapter_id, adv_monitor_removed_callback,
 			manager, NULL);
 
+	mgmt_register(manager->mgmt, MGMT_EV_ADV_MONITOR_DEVICE_FOUND,
+			manager->adapter_id, adv_monitor_device_found_callback,
+			manager, NULL);
+
+	mgmt_register(manager->mgmt, MGMT_EV_ADV_MONITOR_DEVICE_LOST,
+			manager->adapter_id, adv_monitor_device_lost_callback,
+			manager, NULL);
+
 	return manager;
 }
 
-- 
2.33.0.1079.g6e70778dc9-goog

RE: Add Advertisement Monitor Device Found/Lost events

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 884 bytes --]

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

---Test result---

Test Summary:
CheckPatch                    PASS      4.39 seconds
GitLint                       PASS      2.90 seconds
Prep - Setup ELL              PASS      52.11 seconds
Build - Prep                  PASS      0.54 seconds
Build - Configure             PASS      9.39 seconds
Build - Make                  PASS      225.67 seconds
Make Check                    PASS      9.84 seconds
Make Distcheck                PASS      266.48 seconds
Build w/ext ELL - Configure   PASS      9.65 seconds
Build w/ext ELL - Make        PASS      214.45 seconds



---
Regards,
Linux Bluetooth

Re: [BlueZ PATCH v4 1/3] doc: Introduce the Adv Monitor Device Found/Lost events

[Marcel Holtmann]

Hi Manish,

> This patch introduces new MGMT events to indicate that the controller
> has started/stopped tracking a particular device matching one of the
> already added Advertisement Monitor.
> 
> ---
> 
> Changes in v4:
> - Add Advertisement Monitor Device Found event, make Address_Type 0 as
>  reserved.
> 
> Changes in v3:
> - Discard changes to the Device Found event and notify bluetoothd only
>  when the controller stops monitoring the device via new Device Lost
>  event.
> 
> Changes in v2:
> - Instead of creating a new 'Device Tracking' event, add a flag 'Device
>  Tracked' in the existing 'Device Found' event and add a new 'Device
>  Lost' event to indicate that the controller has stopped tracking that
>  device.
> 
> doc/mgmt-api.txt | 50 +++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 49 insertions(+), 1 deletion(-)
> 
> diff --git a/doc/mgmt-api.txt b/doc/mgmt-api.txt
> index 5355fedb0..32dd8c0f2 100644
> --- a/doc/mgmt-api.txt
> +++ b/doc/mgmt-api.txt
> @@ -107,7 +107,8 @@ Configuration command, Default Runtime Configuration Changed event, Get
> Device Flags command, Set Device Flags command, Device Flags Changed event,
> Read Advertisement Monitor Features command, Add Advertisement Patterns
> Monitor command, Remove Advertisement Monitor command, Advertisement Monitor
> -Added event and Advertisement Monitor Removed event.
> +Added event, Advertisement Monitor Removed event, Advertisement Monitor Device
> +Found event and Advertisement Monitor Device Lost event.

we need to increase the mgmt version and add the new commands to that section. Leave this out of this patch and put it into a separate one.

> 
> Example
> @@ -4910,3 +4911,50 @@ Controller Resume Event
> 	Address_Type. Otherwise, Address and Address_Type will both be zero.
> 
> 	This event will be sent to all management sockets.
> +
> +
> +Advertisement Monitor Device Found Event
> +========================================
> +
> +	Event code:		0x002f
> +	Controller Index:	<controller_id>
> +	Event Parameters:	Monitor_Handle (2 Octets)
> +				Address (6 Octets)
> +				Address_Type (1 Octet)
> +
> +	This event indicates that the controller has started tracking a device
> +	matching an Advertisement Monitor with handle Monitor_Handle.
> +
> +	The address of the device being tracked will be shared in Address and
> +	Address_Type.
> +
> +	Possible values for the Address_Type parameter:
> +		0	Reserved (not in use)
> +		1	LE Public
> +		2	LE Random
> +
> +	This event will be sent to all management sockets.

My initial thought was actually to include all the fields from Device Found Event here as well. So that in case we are using advertising monitor, we just need to worry about this event only. Thoughts?

> +
> +
> +Advertisement Monitor Device Lost Event
> +=======================================
> +
> +	Event code:		0x0030
> +	Controller Index:	<controller_id>
> +	Event Parameters:	Monitor_Handle (2 Octets)
> +				Address (6 Octets)
> +				Address_Type (1 Octet)
> +
> +	This event indicates that the controller has stopped tracking a device
> +	that was being tracked by an Advertisement Monitor with handle
> +	Monitor_Handle.
> +
> +	The address of the device being tracked will be shared in Address and
> +	Address_Type.
> +
> +	Possible values for the Address_Type parameter:
> +		0	Reserved (not in use)
> +		1	LE Public
> +		2	LE Random
> +
> +	This event will be sent to all management sockets.

Regards

Marcel

Re: [BlueZ PATCH v4 1/3] doc: Introduce the Adv Monitor Device Found/Lost events

[Marcel Holtmann]

Hi Manish,

> > 
> > Example
> > @@ -4910,3 +4911,50 @@ Controller Resume Event
> >       Address_Type. Otherwise, Address and Address_Type will both be zero.
> > 
> >       This event will be sent to all management sockets.
> > +
> > +
> > +Advertisement Monitor Device Found Event
> > +========================================
> > +
> > +     Event code:             0x002f
> > +     Controller Index:       <controller_id>
> > +     Event Parameters:       Monitor_Handle (2 Octets)
> > +                             Address (6 Octets)
> > +                             Address_Type (1 Octet)
> > +
> > +     This event indicates that the controller has started tracking a device
> > +     matching an Advertisement Monitor with handle Monitor_Handle.
> > +
> > +     The address of the device being tracked will be shared in Address and
> > +     Address_Type.
> > +
> > +     Possible values for the Address_Type parameter:
> > +             0       Reserved (not in use)
> > +             1       LE Public
> > +             2       LE Random
> > +
> > +     This event will be sent to all management sockets.
> 
> My initial thought was actually to include all the fields from Device Found Event here as well. So that in case we are using advertising monitor, we just need to worry about this event only. Thoughts?
> The controller sends advertising reports in addition to the MSFT_Monitor_Device event. This event is reported only twice per bt-device (at start and at end of the monitoring); and it includes only the device addr and addr type [1]. To include other fields from the Device Found event, we need to buffer MSFT_Monitor_Device in the kernel and wait for the subsequent advertising report before we can send it to the bluetoothd. I feel this will unnecessarily complicate the logic in the kernel.
> 
> This event will be used to invoke DeviceFound/Lost on the interface only when we are completely offloading monitoring to the controller (i.e. when the Sampling_Period is set to 0xFF). When the Sampling_Period is set to 0xFF, the controller sends only one advertisement report per monitoring period [2]. So, we need to rely on the MSFT_Monitor_Device controller event for RSSI and timeouts tracking. In other cases, as the bluetoothd receives more than one advertisement report, it can perform RSSI and timeouts tracking.
> 
> So, I think it is better to pass on this event as it is to the bluetoothd and let the advertisement monitoring in the bluetoothd handle it. Let me know what you think about this?

the kernel has to buffer events related to advertising reports and inquiry results already anyway. That is just the cost of business and it is the job of the kernel to do exactly that.

I do not want to dumb down the kernel into a vendor extension passthrough since that is always going to come back and bite you. My current thinking is actually that unless Start Discovery triggers a discovery procedure, device that are monitored, should get its own report out path via mgmt.

So the kernel would send Advertising Monitor Device Found Event (most likely with a flag for tracking started) and subsequence events of the same type whenever it gets updated. I mean, the kernel should track the state when a device is tracked anyway. It has to keep track of these things. In case of power down or reset or anything, the kernel needs to generate the Device Lost event to keeps this all in sync. Otherwise userspace is left to figure out what happens. We can not have that. This is similar to when a process dies, the kernel cleans up all open file descriptors. That is the basic paradigm that we have enforced with mgmt. If bluetoothd dies or restarts, we can get back to a state where we now all the details without having to hard reset hardware.

One way to lock at this is always from the point of something that is not bluetoothd. Any other user of mgmt API needs to be sound and have a good API contract as well. Kernel APIs that are only designed with one daemon in mind are awful and cause major problems along the road.

That all said, we can do this as above, but I would explore the option of doing it with my proposal and really focus on completely offload of the monitoring the controller.

Regards

Marcel

Re: [BlueZ PATCH v4 1/3] doc: Introduce the Adv Monitor Device Found/Lost events

[Luiz Augusto von Dentz]

Hi Manish,

On Sun, Oct 31, 2021 at 10:53 PM Manish Mandlik <mmandlik@google.com> wrote:
>
> Hi Marcel,
>
>
> On Tue, Oct 26, 2021 at 4:33 AM Marcel Holtmann <marcel@holtmann.org> wrote:
>>
>> Hi Manish,
>>
>> > >
>> > > Example
>> > > @@ -4910,3 +4911,50 @@ Controller Resume Event
>> > >       Address_Type. Otherwise, Address and Address_Type will both be zero.
>> > >
>> > >       This event will be sent to all management sockets.
>> > > +
>> > > +
>> > > +Advertisement Monitor Device Found Event
>> > > +========================================
>> > > +
>> > > +     Event code:             0x002f
>> > > +     Controller Index:       <controller_id>
>> > > +     Event Parameters:       Monitor_Handle (2 Octets)
>> > > +                             Address (6 Octets)
>> > > +                             Address_Type (1 Octet)
>> > > +
>> > > +     This event indicates that the controller has started tracking a device
>> > > +     matching an Advertisement Monitor with handle Monitor_Handle.
>> > > +
>> > > +     The address of the device being tracked will be shared in Address and
>> > > +     Address_Type.
>> > > +
>> > > +     Possible values for the Address_Type parameter:
>> > > +             0       Reserved (not in use)
>> > > +             1       LE Public
>> > > +             2       LE Random
>> > > +
>> > > +     This event will be sent to all management sockets.
>> >
>> > My initial thought was actually to include all the fields from Device Found Event here as well. So that in case we are using advertising monitor, we just need to worry about this event only. Thoughts?
>> > The controller sends advertising reports in addition to the MSFT_Monitor_Device event. This event is reported only twice per bt-device (at start and at end of the monitoring); and it includes only the device addr and addr type [1]. To include other fields from the Device Found event, we need to buffer MSFT_Monitor_Device in the kernel and wait for the subsequent advertising report before we can send it to the bluetoothd. I feel this will unnecessarily complicate the logic in the kernel.
>> >
>> > This event will be used to invoke DeviceFound/Lost on the interface only when we are completely offloading monitoring to the controller (i.e. when the Sampling_Period is set to 0xFF). When the Sampling_Period is set to 0xFF, the controller sends only one advertisement report per monitoring period [2]. So, we need to rely on the MSFT_Monitor_Device controller event for RSSI and timeouts tracking. In other cases, as the bluetoothd receives more than one advertisement report, it can perform RSSI and timeouts tracking.
>> >
>> > So, I think it is better to pass on this event as it is to the bluetoothd and let the advertisement monitoring in the bluetoothd handle it. Let me know what you think about this?
>>
>> the kernel has to buffer events related to advertising reports and inquiry results already anyway. That is just the cost of business and it is the job of the kernel to do exactly that.
>>
>> I do not want to dumb down the kernel into a vendor extension passthrough since that is always going to come back and bite you. My current thinking is actually that unless Start Discovery triggers a discovery procedure, device that are monitored, should get its own report out path via mgmt.
>>
>> So the kernel would send Advertising Monitor Device Found Event (most likely with a flag for tracking started) and subsequence events of the same type whenever it gets updated. I mean, the kernel should track the state when a device is tracked anyway. It has to keep track of these things. In case of power down or reset or anything, the kernel needs to generate the Device Lost event to keeps this all in sync. Otherwise userspace is left to figure out what happens. We can not have that. This is similar to when a process dies, the kernel cleans up all open file descriptors. That is the basic paradigm that we have enforced with mgmt. If bluetoothd dies or restarts, we can get back to a state where we now all the details without having to hard reset hardware.
>>
>> One way to lock at this is always from the point of something that is not bluetoothd. Any other user of mgmt API needs to be sound and have a good API contract as well. Kernel APIs that are only designed with one daemon in mind are awful and cause major problems along the road.
>>
>> That all said, we can do this as above, but I would explore the option of doing it with my proposal and really focus on completely offload of the monitoring the controller.
>
>
> Thanks for the explanation. I see your point. Based on that I’d like to propose the following changes to the bluetooth kernel and user space to take all the advantages of the controller offloading whenever available. Let me know if it sounds good?
>
> Existing controller behavior (based on the MSFT extension specification):
> - Whenever the controller starts monitoring a device, it first sends the MSFT_Monitor_Device event with Monitor_State as 1 and provides the matched Monitor_Id and device address.
> - Based on the Sampling_Period configured, it then sends one or more LE Advertisement Reports for that device during the monitoring period.
> - Whenever the controller stops monitoring a device, it again sends the MSFT_Monitor_Device event but with Monitor_State as 0 and provides the Monitor_Id and device address of the device that was being tracked.
>
> Proposed kernel behavior:
> - Upon receipt of the MSFT_Monitor_Device event with Monitor_State 1, the kernel will buffer this event until it receives the subsequent LE Advertisement Report(s).
> - After receiving the first advertisement report for the monitored device, kernel will generate the “Adv Monitor Device Found” MGMT event which is identical to the “Device Found” MGMT event, but will have additional Monitor_Handle information for the matched monitor.
> - A separate Device_Tracked flag is not required since this event itself indicates that the device is being tracked.
> - If the active scanning is in progress, the existing “Device Found” event will also be generated before generating the “Adv Monitor Device Found” event.
> - For the subsequent advertisement reports (if any) for the monitored device, if the active scanning is in progress, only the existing "Device Found" event will be generated; else, only the "Adv Monitor Device Found" event will be generated.
> - Upon receipt of the MSFT_Monitor_Device event with Monitor_State 0, the kernel will generate the “Adv Monitor Device Lost” MGMT event with the Monitor_Handle and device address of the device that was being tracked.
> - Also, in case of power down or reset, the kernel will generate the "Adv Monitor Device Lost" event for the monitored devices.
>
> Proposed user space behavior:
> - Whenever the controller offloading is available, bluetoothd will only use the “Adv Monitor Device Found” and “Adv Monitor Device Lost” event for performing monitoring related functions and SW based filtering will be completely disabled as Monitor_Handle will be available from the kernel/controller and can be used to notify respective applications on DeviceFound/DeviceLost.
> - Whenever the controller offloading is NOT available, bluetoothd will use the existing “Device Found” event and perform SW based filtering as it is doing right now.
> - “Adv Monitor Device Found” event will also be used to create/update the found device when active scanning is NOT in progress.

Are you still working on this or are you waiting for some feedback on
the above, at first glance it looks proper. Btw, it would be great if
these changes are accompanied with mgmt-tester tests since vhci has
support for MSFT commands and we can emulate such code paths.


-- 
Luiz Augusto von Dentz

Re: [BlueZ PATCH v4 1/3] doc: Introduce the Adv Monitor Device Found/Lost events

[Luiz Augusto von Dentz]

Hi Manish,

On Mon, Nov 8, 2021 at 9:30 PM Manish Mandlik <mmandlik@google.com> wrote:
>
> Hi Luiz,
>
> On Mon, Nov 8, 2021 at 5:57 PM Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote:
>>
>> Hi Manish,
>>
>> On Sun, Oct 31, 2021 at 10:53 PM Manish Mandlik <mmandlik@google.com> wrote:
>> >
>> > Hi Marcel,
>> >
>> >
>> > On Tue, Oct 26, 2021 at 4:33 AM Marcel Holtmann <marcel@holtmann.org> wrote:
>> >>
>> >> Hi Manish,
>> >>
>> >> > >
>> >> > > Example
>> >> > > @@ -4910,3 +4911,50 @@ Controller Resume Event
>> >> > >       Address_Type. Otherwise, Address and Address_Type will both be zero.
>> >> > >
>> >> > >       This event will be sent to all management sockets.
>> >> > > +
>> >> > > +
>> >> > > +Advertisement Monitor Device Found Event
>> >> > > +========================================
>> >> > > +
>> >> > > +     Event code:             0x002f
>> >> > > +     Controller Index:       <controller_id>
>> >> > > +     Event Parameters:       Monitor_Handle (2 Octets)
>> >> > > +                             Address (6 Octets)
>> >> > > +                             Address_Type (1 Octet)
>> >> > > +
>> >> > > +     This event indicates that the controller has started tracking a device
>> >> > > +     matching an Advertisement Monitor with handle Monitor_Handle.
>> >> > > +
>> >> > > +     The address of the device being tracked will be shared in Address and
>> >> > > +     Address_Type.
>> >> > > +
>> >> > > +     Possible values for the Address_Type parameter:
>> >> > > +             0       Reserved (not in use)
>> >> > > +             1       LE Public
>> >> > > +             2       LE Random
>> >> > > +
>> >> > > +     This event will be sent to all management sockets.
>> >> >
>> >> > My initial thought was actually to include all the fields from Device Found Event here as well. So that in case we are using advertising monitor, we just need to worry about this event only. Thoughts?
>> >> > The controller sends advertising reports in addition to the MSFT_Monitor_Device event. This event is reported only twice per bt-device (at start and at end of the monitoring); and it includes only the device addr and addr type [1]. To include other fields from the Device Found event, we need to buffer MSFT_Monitor_Device in the kernel and wait for the subsequent advertising report before we can send it to the bluetoothd. I feel this will unnecessarily complicate the logic in the kernel.
>> >> >
>> >> > This event will be used to invoke DeviceFound/Lost on the interface only when we are completely offloading monitoring to the controller (i.e. when the Sampling_Period is set to 0xFF). When the Sampling_Period is set to 0xFF, the controller sends only one advertisement report per monitoring period [2]. So, we need to rely on the MSFT_Monitor_Device controller event for RSSI and timeouts tracking. In other cases, as the bluetoothd receives more than one advertisement report, it can perform RSSI and timeouts tracking.
>> >> >
>> >> > So, I think it is better to pass on this event as it is to the bluetoothd and let the advertisement monitoring in the bluetoothd handle it. Let me know what you think about this?
>> >>
>> >> the kernel has to buffer events related to advertising reports and inquiry results already anyway. That is just the cost of business and it is the job of the kernel to do exactly that.
>> >>
>> >> I do not want to dumb down the kernel into a vendor extension passthrough since that is always going to come back and bite you. My current thinking is actually that unless Start Discovery triggers a discovery procedure, device that are monitored, should get its own report out path via mgmt.
>> >>
>> >> So the kernel would send Advertising Monitor Device Found Event (most likely with a flag for tracking started) and subsequence events of the same type whenever it gets updated. I mean, the kernel should track the state when a device is tracked anyway. It has to keep track of these things. In case of power down or reset or anything, the kernel needs to generate the Device Lost event to keeps this all in sync. Otherwise userspace is left to figure out what happens. We can not have that. This is similar to when a process dies, the kernel cleans up all open file descriptors. That is the basic paradigm that we have enforced with mgmt. If bluetoothd dies or restarts, we can get back to a state where we now all the details without having to hard reset hardware.
>> >>
>> >> One way to lock at this is always from the point of something that is not bluetoothd. Any other user of mgmt API needs to be sound and have a good API contract as well. Kernel APIs that are only designed with one daemon in mind are awful and cause major problems along the road.
>> >>
>> >> That all said, we can do this as above, but I would explore the option of doing it with my proposal and really focus on completely offload of the monitoring the controller.
>> >
>> >
>> > Thanks for the explanation. I see your point. Based on that I’d like to propose the following changes to the bluetooth kernel and user space to take all the advantages of the controller offloading whenever available. Let me know if it sounds good?
>> >
>> > Existing controller behavior (based on the MSFT extension specification):
>> > - Whenever the controller starts monitoring a device, it first sends the MSFT_Monitor_Device event with Monitor_State as 1 and provides the matched Monitor_Id and device address.
>> > - Based on the Sampling_Period configured, it then sends one or more LE Advertisement Reports for that device during the monitoring period.
>> > - Whenever the controller stops monitoring a device, it again sends the MSFT_Monitor_Device event but with Monitor_State as 0 and provides the Monitor_Id and device address of the device that was being tracked.
>> >
>> > Proposed kernel behavior:
>> > - Upon receipt of the MSFT_Monitor_Device event with Monitor_State 1, the kernel will buffer this event until it receives the subsequent LE Advertisement Report(s).
>> > - After receiving the first advertisement report for the monitored device, kernel will generate the “Adv Monitor Device Found” MGMT event which is identical to the “Device Found” MGMT event, but will have additional Monitor_Handle information for the matched monitor.
>> > - A separate Device_Tracked flag is not required since this event itself indicates that the device is being tracked.
>> > - If the active scanning is in progress, the existing “Device Found” event will also be generated before generating the “Adv Monitor Device Found” event.
>> > - For the subsequent advertisement reports (if any) for the monitored device, if the active scanning is in progress, only the existing "Device Found" event will be generated; else, only the "Adv Monitor Device Found" event will be generated.
>> > - Upon receipt of the MSFT_Monitor_Device event with Monitor_State 0, the kernel will generate the “Adv Monitor Device Lost” MGMT event with the Monitor_Handle and device address of the device that was being tracked.
>> > - Also, in case of power down or reset, the kernel will generate the "Adv Monitor Device Lost" event for the monitored devices.
>> >
>> > Proposed user space behavior:
>> > - Whenever the controller offloading is available, bluetoothd will only use the “Adv Monitor Device Found” and “Adv Monitor Device Lost” event for performing monitoring related functions and SW based filtering will be completely disabled as Monitor_Handle will be available from the kernel/controller and can be used to notify respective applications on DeviceFound/DeviceLost.
>> > - Whenever the controller offloading is NOT available, bluetoothd will use the existing “Device Found” event and perform SW based filtering as it is doing right now.
>> > - “Adv Monitor Device Found” event will also be used to create/update the found device when active scanning is NOT in progress.
>>
>> Are you still working on this or are you waiting for some feedback on
>> the above, at first glance it looks proper. Btw, it would be great if
>> these changes are accompanied with mgmt-tester tests since vhci has
>> support for MSFT commands and we can emulate such code paths.
>
>
> Yes, I was waiting for the feedback. I wanted to run the above proposal by you and Marcel before making the code changes. Thanks for the feedback, I'll also include the mgmt-tester tests.

Great, next time just send a reminder that you are waiting for
feedback since we may have just assume you had started working
already, or hadn't been following the exact progress on the mailing
list. Another alternative is to change the status on patchwork to
something like Needs ACK so we can perhaps run some scripts that
generate email or something.

>>
>>
>> --
>> Luiz Augusto von Dentz
>
> Thanks,
> Manish.
>
>



-- 
Luiz Augusto von Dentz