[Bluez PATCH] doc: Add Suspend and Resume events

[Bluez PATCH] doc: Add Suspend and Resume events

[Abhishek Pandit-Subedi]

Add Controller Suspend Event and Controller Resume Event to identify
suspend or resume of the Bluetooth stack has occurred.

Also update Device Disconnected Event to indicate a new disconnect
reason: "Connection terminated by local host for suspend"

Reviewed-by: Alain Michaud <alainm@chromium.org>
Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
---

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

diff --git a/doc/mgmt-api.txt b/doc/mgmt-api.txt
index ca0d38469..f79c0222c 100644
--- a/doc/mgmt-api.txt
+++ b/doc/mgmt-api.txt
@@ -3834,6 +3834,7 @@ Device Disconnected Event
 		2	Connection terminated by local host
 		3	Connection terminated by remote host
 		4	Connection terminated due to authentication failure
+		5	Connection terminated by local host for suspend
 
 	Note that the local/remote distinction just determines which side
 	terminated the low-level connection, regardless of the
@@ -4577,3 +4578,55 @@ Advertisement Monitor Removed Event
 
 	The event will only be sent to management sockets other than the
 	one through which the command was sent.
+
+
+Controller Suspend Event
+========================
+
+	Event code:		0x002d
+	Controller Index:	<controller_id>
+	Event Parameters:	Suspend_State (1 octet)
+
+	This event indicates that the controller is suspended for host suspend.
+
+	Possible values for the Suspend_State parameter:
+		0	Running (not disconnected)
+		1	Disconnected and not scanning
+		2	Page scanning and/or passive scanning.
+
+	The value 0 is used for the running state and may be sent if the
+	controller could not be configured to suspend properly.
+
+	This event will be sent to all management sockets.
+
+
+Controller Resume Event
+=======================
+
+	Event code:		0x002e
+	Controller Index:	<controller_id>
+	Event Parameters:	Address (6 octets)
+				Address_Type (1 octet)
+				Wake_Reason (1 octet)
+
+	This event indicates that the controller has resumed from suspend.
+
+	Possible values for the Wake_Reason parameter:
+		0	Unexpected Event
+		1	Resume from non-Bluetooth wake source
+		2	Connection Request (BR/EDR)
+		3	Connection Complete (BR/EDR)
+		4	LE Advertisement
+		5	LE Direct Advertisement
+		6	LE Extended Advertisement
+
+	We expect that only peer reconnections should wake us from the suspended
+	state. Any other events that cause a wakeup will be marked as Unexpected
+	Event.
+
+	If the Wake_Reason was any of the expected wake reasons (values 2-6),
+	the address of the peer device that caused the event will be shared in
+	Address and Address_Type. Otherwise, Address and Address_Type will both
+	be zero.
+
+	This event will be sent to all management sockets.
-- 
2.27.0.383.g050319c2ae-goog

Re: [Bluez PATCH] doc: Add Suspend and Resume events

[Marcel Holtmann]

Hi Abhishek,

> Add Controller Suspend Event and Controller Resume Event to identify
> suspend or resume of the Bluetooth stack has occurred.
> 
> Also update Device Disconnected Event to indicate a new disconnect
> reason: "Connection terminated by local host for suspend"
> 
> Reviewed-by: Alain Michaud <alainm@chromium.org>
> Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
> ---
> 
> doc/mgmt-api.txt | 53 ++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 53 insertions(+)
> 
> diff --git a/doc/mgmt-api.txt b/doc/mgmt-api.txt
> index ca0d38469..f79c0222c 100644
> --- a/doc/mgmt-api.txt
> +++ b/doc/mgmt-api.txt
> @@ -3834,6 +3834,7 @@ Device Disconnected Event
> 		2	Connection terminated by local host
> 		3	Connection terminated by remote host
> 		4	Connection terminated due to authentication failure
> +		5	Connection terminated by local host for suspend
> 
> 	Note that the local/remote distinction just determines which side
> 	terminated the low-level connection, regardless of the
> @@ -4577,3 +4578,55 @@ Advertisement Monitor Removed Event
> 
> 	The event will only be sent to management sockets other than the
> 	one through which the command was sent.
> +
> +
> +Controller Suspend Event
> +========================
> +
> +	Event code:		0x002d
> +	Controller Index:	<controller_id>
> +	Event Parameters:	Suspend_State (1 octet)
> +
> +	This event indicates that the controller is suspended for host suspend.
> +
> +	Possible values for the Suspend_State parameter:
> +		0	Running (not disconnected)
> +		1	Disconnected and not scanning
> +		2	Page scanning and/or passive scanning.
> +
> +	The value 0 is used for the running state and may be sent if the
> +	controller could not be configured to suspend properly.

does it make sense to send a suspend event with state suspend not succeeded. That doesn’t really fit well.

> +
> +	This event will be sent to all management sockets.
> +
> +
> +Controller Resume Event
> +=======================
> +
> +	Event code:		0x002e
> +	Controller Index:	<controller_id>
> +	Event Parameters:	Address (6 octets)
> +				Address_Type (1 octet)
> +				Wake_Reason (1 octet)
> +
> +	This event indicates that the controller has resumed from suspend.
> +
> +	Possible values for the Wake_Reason parameter:
> +		0	Unexpected Event
> +		1	Resume from non-Bluetooth wake source
> +		2	Connection Request (BR/EDR)
> +		3	Connection Complete (BR/EDR)
> +		4	LE Advertisement
> +		5	LE Direct Advertisement
> +		6	LE Extended Advertisement
> +
> +	We expect that only peer reconnections should wake us from the suspended
> +	state. Any other events that cause a wakeup will be marked as Unexpected
> +	Event.
> +
> +	If the Wake_Reason was any of the expected wake reasons (values 2-6),
> +	the address of the peer device that caused the event will be shared in
> +	Address and Address_Type. Otherwise, Address and Address_Type will both
> +	be zero.

So I would be using Wake_Reason as first argument. However do you need all this distinction. For example the Device Connected event could gain an extra Flags bit for wakeup. I would especially not make any distinction between the advertising types.

I am in principal fine telling bluetoothd when suspend / resume happened, but letting HCI event specifics bleed into the mgmt API is not really helpful.

Regards

Marcel

Re: [Bluez PATCH] doc: Add Suspend and Resume events

[Abhishek Pandit-Subedi]

Hi Marcel,

On Fri, Jul 10, 2020 at 10:20 AM Marcel Holtmann <marcel@holtmann.org> wrote:
>
> Hi Abhishek,
>
> > Add Controller Suspend Event and Controller Resume Event to identify
> > suspend or resume of the Bluetooth stack has occurred.
> >
> > Also update Device Disconnected Event to indicate a new disconnect
> > reason: "Connection terminated by local host for suspend"
> >
> > Reviewed-by: Alain Michaud <alainm@chromium.org>
> > Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
> > ---
> >
> > doc/mgmt-api.txt | 53 ++++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 53 insertions(+)
> >
> > diff --git a/doc/mgmt-api.txt b/doc/mgmt-api.txt
> > index ca0d38469..f79c0222c 100644
> > --- a/doc/mgmt-api.txt
> > +++ b/doc/mgmt-api.txt
> > @@ -3834,6 +3834,7 @@ Device Disconnected Event
> >               2       Connection terminated by local host
> >               3       Connection terminated by remote host
> >               4       Connection terminated due to authentication failure
> > +             5       Connection terminated by local host for suspend
> >
> >       Note that the local/remote distinction just determines which side
> >       terminated the low-level connection, regardless of the
> > @@ -4577,3 +4578,55 @@ Advertisement Monitor Removed Event
> >
> >       The event will only be sent to management sockets other than the
> >       one through which the command was sent.
> > +
> > +
> > +Controller Suspend Event
> > +========================
> > +
> > +     Event code:             0x002d
> > +     Controller Index:       <controller_id>
> > +     Event Parameters:       Suspend_State (1 octet)
> > +
> > +     This event indicates that the controller is suspended for host suspend.
> > +
> > +     Possible values for the Suspend_State parameter:
> > +             0       Running (not disconnected)
> > +             1       Disconnected and not scanning
> > +             2       Page scanning and/or passive scanning.
> > +
> > +     The value 0 is used for the running state and may be sent if the
> > +     controller could not be configured to suspend properly.
>
> does it make sense to send a suspend event with state suspend not succeeded. That doesn’t really fit well.

We don't block suspend if preparing for suspend fails so it's useful
to know when a suspend was attempted. Also, having the suspend event
emitted acts as an anchor that lets us search through the HCI trace
quickly and observe what happened around it to cause an unexpected
state.

>
> > +
> > +     This event will be sent to all management sockets.
> > +
> > +
> > +Controller Resume Event
> > +=======================
> > +
> > +     Event code:             0x002e
> > +     Controller Index:       <controller_id>
> > +     Event Parameters:       Address (6 octets)
> > +                             Address_Type (1 octet)
> > +                             Wake_Reason (1 octet)
> > +
> > +     This event indicates that the controller has resumed from suspend.
> > +
> > +     Possible values for the Wake_Reason parameter:
> > +             0       Unexpected Event
> > +             1       Resume from non-Bluetooth wake source
> > +             2       Connection Request (BR/EDR)
> > +             3       Connection Complete (BR/EDR)
> > +             4       LE Advertisement
> > +             5       LE Direct Advertisement
> > +             6       LE Extended Advertisement
> > +
> > +     We expect that only peer reconnections should wake us from the suspended
> > +     state. Any other events that cause a wakeup will be marked as Unexpected
> > +     Event.
> > +
> > +     If the Wake_Reason was any of the expected wake reasons (values 2-6),
> > +     the address of the peer device that caused the event will be shared in
> > +     Address and Address_Type. Otherwise, Address and Address_Type will both
> > +     be zero.
>
> So I would be using Wake_Reason as first argument. However do you need all this distinction. For example the Device Connected event could gain an extra Flags bit for wakeup. I would especially not make any distinction between the advertising types.
>
> I am in principal fine telling bluetoothd when suspend / resume happened, but letting HCI event specifics bleed into the mgmt API is not really helpful.

Sure, we can reduce the wake_reason to 0 = Unexpected, 1 = Non
bluetooth source, 2 = Device Connection.

I think a distinct event is preferable to adding another bit to Device
Connected.
a) It acts as an anchor point for searching an HCI trace for suspend occurring.
b) It is resilient to failures in any of the connection events (since
the Device Connected event sometimes requires subsequent calls to the
controller before the connection completes).
c) Properly captures wake from unexpected events. This is really nice
to understand what events came BEFORE the PM_SUSPEND_DONE event was
sent by the kernel and would help identify suspend bugs and
regressions.

>
> Regards
>
> Marcel
>