[BlueZ PATCH v1] doc: Describe the new Advertisement Monitor support

[BlueZ PATCH v1] doc: Describe the new Advertisement Monitor support

[Miao-chen Chou]

This describes the following commands.
- Add Advertisement Patterns Monitor
- Remove Advertisement Monitor
- Remove All Advertisement Monitors
Note that the content of a monitor can differ based on its type. For now we
introduce only pattern-based monitor, so you may find that unlike commands
for removing monitor(s), the Add command is tied to a specific type.

Signed-off-by: Miao-chen Chou <mcchou@chromium.org>
---

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

diff --git a/doc/mgmt-api.txt b/doc/mgmt-api.txt
index 39f23c456..fcd281a35 100644
--- a/doc/mgmt-api.txt
+++ b/doc/mgmt-api.txt
@@ -3138,6 +3138,74 @@ Read Security Information Command
 				Invalid Index
 
 
+Add Advertisement Patterns Monitor Command
+=========================================
+
+	Command Code:		0x0049
+	Controller Index:	<controller id>
+	Command Parameters:	Pattern_count (1 Octets)
+				Pattern1 {
+					AD_Data_Type (1 Octet)
+					Index (1 Octet)
+					Length (1 Octet)
+					Value (variable length)
+				}
+				Pattern2 { }
+				...
+	Return Parameters:	Monitor_Index (8 Octets)
+
+	This command is used to add an advertisement monitor whose filtering
+	conditions are patterns. The kernel would track the number of registered
+	monitors to determine whether to perform LE scanning while there is
+	ongoing LE scanning for other intentions, such as auto-reconnection and
+	discovery session. If the controller supports Microsoft HCI extension,
+	the kernel would offload the content filtering to the controller in
+	order to reduce power consumption; otherwise the kernel ignore the
+	content of the monitor.
+
+	Possible errors:	Failed
+				Busy
+				Invalid Parameters
+
+
+Remove Advertisement Monitor Command
+====================================
+
+	Command Code:		0x004A
+	Controller Index:	<controller id>
+	Command Parameters:	Monitor_Index (8 Octets)
+	Return Parameters:	Monitor_Index (8 Octets)
+
+	This command is used to remove an advertisement monitor. The kernel
+	would remove the monitor with Monitor_Index and update the LE scanning.
+	If the controller supports Microsoft HCI extension and the monitor has
+	been offloaded, the kernel would cancel the offloading; otherwise the
+	kernel takes no further actions other than removing it from the list.
+
+	Possible errors:	Failed
+				Busy
+				Invalid Index
+
+
+Remove All Advertisement Monitors Command
+=========================================
+
+	Command Code:		0x004B
+	Controller Index:	<controller id>
+	Command Parameters:
+	Return Parameters:	Num_removed_Monitors (2 Octets)
+				Monitor_Index[i] (2 Octets)
+
+	This command is used to remove all advertisement monitors. The kernel
+	would remove all monitors and update the LE scanning if needed. If the
+	controller supports Microsoft HCI extension the monitors have been
+	offloaded, the kernel would cancel all offloadings; otherwise the kernel
+	takes no further actions other than removing all monitors from the list.
+
+	Possible errors:	Failed
+				Busy
+
+
 Command Complete Event
 ======================
 
-- 
2.24.1

Re: [BlueZ PATCH v1] doc: Describe the new Advertisement Monitor support

[Luiz Augusto von Dentz]

Hi Miao,

On Fri, Apr 17, 2020 at 8:13 PM Miao-chen Chou <mcchou@chromium.org> wrote:
>
> This describes the following commands.
> - Add Advertisement Patterns Monitor
> - Remove Advertisement Monitor
> - Remove All Advertisement Monitors
> Note that the content of a monitor can differ based on its type. For now we
> introduce only pattern-based monitor, so you may find that unlike commands
> for removing monitor(s), the Add command is tied to a specific type.
>
> Signed-off-by: Miao-chen Chou <mcchou@chromium.org>

For userspace we don't require signed-of-by line.

> ---
>
>  doc/mgmt-api.txt | 68 ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 68 insertions(+)
>
> diff --git a/doc/mgmt-api.txt b/doc/mgmt-api.txt
> index 39f23c456..fcd281a35 100644
> --- a/doc/mgmt-api.txt
> +++ b/doc/mgmt-api.txt
> @@ -3138,6 +3138,74 @@ Read Security Information Command
>                                 Invalid Index
>
>
> +Add Advertisement Patterns Monitor Command
> +=========================================
> +
> +       Command Code:           0x0049
> +       Controller Index:       <controller id>
> +       Command Parameters:     Pattern_count (1 Octets)
> +                               Pattern1 {
> +                                       AD_Data_Type (1 Octet)
> +                                       Index (1 Octet)

What is this index for, it is not very clear why would we need it here
since we are going to return a index already.

> +                                       Length (1 Octet)
> +                                       Value (variable length)
> +                               }
> +                               Pattern2 { }
> +                               ...
> +       Return Parameters:      Monitor_Index (8 Octets)

I guess we could call this a handle if the is assigned by the
controller not the host stack, also it might be more a good idea to
put some example on how to build the parameter list, etc, to make it
easier to visualize how these parameters are put together.

> +
> +       This command is used to add an advertisement monitor whose filtering
> +       conditions are patterns. The kernel would track the number of registered
> +       monitors to determine whether to perform LE scanning while there is
> +       ongoing LE scanning for other intentions, such as auto-reconnection and
> +       discovery session. If the controller supports Microsoft HCI extension,
> +       the kernel would offload the content filtering to the controller in
> +       order to reduce power consumption; otherwise the kernel ignore the
> +       content of the monitor.
> +
> +       Possible errors:        Failed
> +                               Busy
> +                               Invalid Parameters
> +
> +
> +Remove Advertisement Monitor Command
> +====================================
> +
> +       Command Code:           0x004A
> +       Controller Index:       <controller id>
> +       Command Parameters:     Monitor_Index (8 Octets)
> +       Return Parameters:      Monitor_Index (8 Octets)
> +
> +       This command is used to remove an advertisement monitor. The kernel
> +       would remove the monitor with Monitor_Index and update the LE scanning.
> +       If the controller supports Microsoft HCI extension and the monitor has
> +       been offloaded, the kernel would cancel the offloading; otherwise the
> +       kernel takes no further actions other than removing it from the list.
> +
> +       Possible errors:        Failed
> +                               Busy
> +                               Invalid Index
> +
> +
> +Remove All Advertisement Monitors Command
> +=========================================
> +
> +       Command Code:           0x004B
> +       Controller Index:       <controller id>
> +       Command Parameters:
> +       Return Parameters:      Num_removed_Monitors (2 Octets)
> +                               Monitor_Index[i] (2 Octets)
> +
> +       This command is used to remove all advertisement monitors. The kernel
> +       would remove all monitors and update the LE scanning if needed. If the
> +       controller supports Microsoft HCI extension the monitors have been
> +       offloaded, the kernel would cancel all offloadings; otherwise the kernel
> +       takes no further actions other than removing all monitors from the list.
> +
> +       Possible errors:        Failed
> +                               Busy

I wonder if we could keep this simple and just have a special value
passed to Remove that would be consider remove all e.g.
0x0000000000000000, that way we don't have to allocate yet another
opcode for removing all, though Im not sure how the values are managed
in that namespace so it may not be possible allocate one for use with
remote all logic, the other option would be to have a num_monitors and
then in case none is provided do the remove all, that actually might
be useful since if there are multiple application/monitors we can
remove all monitors from one application without afting the others
leaving remove all logic just for when bluetoothd is restarted which
is basically a cleanup.

> +
>  Command Complete Event
>  ======================
>
> --
> 2.24.1
>


-- 
Luiz Augusto von Dentz

Re: [BlueZ PATCH v1] doc: Describe the new Advertisement Monitor support

[Miao-chen Chou]

On Mon, Apr 20, 2020 at 3:53 PM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi Miao,
>
> On Fri, Apr 17, 2020 at 8:13 PM Miao-chen Chou <mcchou@chromium.org> wrote:
> >
> > This describes the following commands.
> > - Add Advertisement Patterns Monitor
> > - Remove Advertisement Monitor
> > - Remove All Advertisement Monitors
> > Note that the content of a monitor can differ based on its type. For now we
> > introduce only pattern-based monitor, so you may find that unlike commands
> > for removing monitor(s), the Add command is tied to a specific type.
> >
> > Signed-off-by: Miao-chen Chou <mcchou@chromium.org>
>
> For userspace we don't require signed-of-by line.
Ack, it was added by patman tool, I will remove it before sending.
>
> > ---
> >
> >  doc/mgmt-api.txt | 68 ++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 68 insertions(+)
> >
> > diff --git a/doc/mgmt-api.txt b/doc/mgmt-api.txt
> > index 39f23c456..fcd281a35 100644
> > --- a/doc/mgmt-api.txt
> > +++ b/doc/mgmt-api.txt
> > @@ -3138,6 +3138,74 @@ Read Security Information Command
> >                                 Invalid Index
> >
> >
> > +Add Advertisement Patterns Monitor Command
> > +=========================================
> > +
> > +       Command Code:           0x0049
> > +       Controller Index:       <controller id>
> > +       Command Parameters:     Pattern_count (1 Octets)
> > +                               Pattern1 {
> > +                                       AD_Data_Type (1 Octet)
> > +                                       Index (1 Octet)
>
> What is this index for, it is not very clear why would we need it here
> since we are going to return a index already.
This is the starting position of the given pattern. For instance, if
we'd like to look for the pattern within a Manufacturer Data field
with value {0x01, 0x02, 0x03} starting from index 2. Given the
Manufacturer Data field is {0x00, 0xFF, 0x01, 0x02, 0x03, 0x04}, the
matching would start from octet 0x01, and the result would be a match.
Perhaps I should rename "Index" to "Start_Position" or rename
"Monitor_index" to "Monitor_handle" to avoid confusion.
>
> > +                                       Length (1 Octet)
> > +                                       Value (variable length)
> > +                               }
> > +                               Pattern2 { }
> > +                               ...
> > +       Return Parameters:      Monitor_Index (8 Octets)
>
> I guess we could call this a handle if the is assigned by the
> controller not the host stack, also it might be more a good idea to
> put some example on how to build the parameter list, etc, to make it
> easier to visualize how these parameters are put together.
"Monitor_Handle" sounds good to me. I will add some examples.
>
> > +
> > +       This command is used to add an advertisement monitor whose filtering
> > +       conditions are patterns. The kernel would track the number of registered
> > +       monitors to determine whether to perform LE scanning while there is
> > +       ongoing LE scanning for other intentions, such as auto-reconnection and
> > +       discovery session. If the controller supports Microsoft HCI extension,
> > +       the kernel would offload the content filtering to the controller in
> > +       order to reduce power consumption; otherwise the kernel ignore the
> > +       content of the monitor.
> > +
> > +       Possible errors:        Failed
> > +                               Busy
> > +                               Invalid Parameters
> > +
> > +
> > +Remove Advertisement Monitor Command
> > +====================================
> > +
> > +       Command Code:           0x004A
> > +       Controller Index:       <controller id>
> > +       Command Parameters:     Monitor_Index (8 Octets)
> > +       Return Parameters:      Monitor_Index (8 Octets)
> > +
> > +       This command is used to remove an advertisement monitor. The kernel
> > +       would remove the monitor with Monitor_Index and update the LE scanning.
> > +       If the controller supports Microsoft HCI extension and the monitor has
> > +       been offloaded, the kernel would cancel the offloading; otherwise the
> > +       kernel takes no further actions other than removing it from the list.
> > +
> > +       Possible errors:        Failed
> > +                               Busy
> > +                               Invalid Index
> > +
> > +
> > +Remove All Advertisement Monitors Command
> > +=========================================
> > +
> > +       Command Code:           0x004B
> > +       Controller Index:       <controller id>
> > +       Command Parameters:
> > +       Return Parameters:      Num_removed_Monitors (2 Octets)
> > +                               Monitor_Index[i] (2 Octets)
> > +
> > +       This command is used to remove all advertisement monitors. The kernel
> > +       would remove all monitors and update the LE scanning if needed. If the
> > +       controller supports Microsoft HCI extension the monitors have been
> > +       offloaded, the kernel would cancel all offloadings; otherwise the kernel
> > +       takes no further actions other than removing all monitors from the list.
> > +
> > +       Possible errors:        Failed
> > +                               Busy
>
> I wonder if we could keep this simple and just have a special value
> passed to Remove that would be consider remove all e.g.
> 0x0000000000000000, that way we don't have to allocate yet another
> opcode for removing all, though Im not sure how the values are managed
> in that namespace so it may not be possible allocate one for use with
> remote all logic, the other option would be to have a num_monitors and
> then in case none is provided do the remove all, that actually might
> be useful since if there are multiple application/monitors we can
> remove all monitors from one application without afting the others
> leaving remove all logic just for when bluetoothd is restarted which
> is basically a cleanup.
The intention of this command is indeed to handle the restart of
bluetoothd. It's a good point that we may want to remove all monitors
of an application. Let me try to combine remove and remove all
commands in my next patch.

Thanks,
Miao