diff mbox series

[BlueZ,v1] doc:adding definitions for load default params mgmt op

Message ID 20200504142625.213143-1-alainm@chromium.org (mailing list archive)
State Changes Requested
Delegated to: Marcel Holtmann
Headers show
Series [BlueZ,v1] doc:adding definitions for load default params mgmt op | expand

Commit Message

Alain Michaud May 4, 2020, 2:26 p.m. UTC
This change adds the definition for the load default parameter command.
In particular, this command is used to load default parameters for
various operations in the kernel. This mechanism is also expandable to
future knobs that may be necessary.

This will allow bluetoothd to load parameters from a conf file that may
be customized for the specific requirements of each platforms.

---

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

Comments

Marcel Holtmann May 6, 2020, 11:22 a.m. UTC | #1
Hi Alain,

> This change adds the definition for the load default parameter command.
> In particular, this command is used to load default parameters for
> various operations in the kernel. This mechanism is also expandable to
> future knobs that may be necessary.
> 
> This will allow bluetoothd to load parameters from a conf file that may
> be customized for the specific requirements of each platforms.
> 
> ---
> 
> doc/mgmt-api.txt | 59 ++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 59 insertions(+)
> 
> diff --git a/doc/mgmt-api.txt b/doc/mgmt-api.txt
> index 39f23c456..71d528706 100644
> --- a/doc/mgmt-api.txt
> +++ b/doc/mgmt-api.txt
> @@ -3137,6 +3137,65 @@ Read Security Information Command
> 	Possible errors:	Invalid Parameters
> 				Invalid Index
> 
> +Load Default Parameter Command
> +=============================

I think that Load Controller Default Parameters is a better name.

> +
> +	Command Code:		0x0049
> +	Controller Index:	<controller id>
> +	Command Parameters:	Parameter_Count (2 Octets)
> +				Parameter1 {
> +					Parameter_Type (2 Octet)
> +					Value_Length (1 Octet)
> +					Value (0-255 Octets)
> +				}
> +				Parameter2 { }
> +				...
> +	Return Parameters:
> +
> +	This command is used to feed the kernel a list of default parameters.
> +
> +	Currently defined Parameter_Type values are:
> +
> +			0x0000	BR/EDR Page Scan Type
> +			0x0001	BR/EDR Page Scan Interval
> +			0x0002	BR/EDR Page Scan Window
> +			0x0003	BR/EDR Inquiry Scan Type
> +			0x0004	BR/EDR Inquiry Scan Interval
> +			0x0005	BR/EDR Inquiry Scan Window
> +			0x0006	BR/EDR Link Supervision Timeout
> +			0x0007	BR/EDR Page Timeout
> +			0x0008	BR/EDR Min Sniff Interval
> +			0x0009	BR/EDR Max Sniff Interval
> +			0x0080	LE Advertisement Min Interval
> +			0x0081	LE Advertisement Max Interval
> +			0x0082	LE Multi Advertisement Rotation Interval
> +			0x0083	LE Scanning Interval for auto connect
> +			0x0084	LE Scanning Window for auto connect
> +			0x0085	LE Scanning Interval for HID only
> +			0x0086	LE Scanning Window for HID only
> +			0x0087	LE Scanning Interval for wake scenarios
> +			0x0088	LE Scanning Window for wake scenarios
> +			0x0089	LE Scanning Interval for discovery
> +			0x008a	LE Scanning Window for discovery
> +			0x008b	LE Scanning Interval for adv monitoring
> +			0x008c	LE Scanning Window for adv monitoring
> +			0x008d	LE Scanning Interval for connect
> +			0x008e	LE Scanning Window for connect
> +			0x008f	LE Min Connection Interval
> +			0x0090	LE Max Connection Interval
> +			0x0091	LE Connection Connection Latency
> +			0x0092	LE Connection Supervision Timeout

I would just enumerate these and not try to split between 0x000 and 0x008 for BR/EDR and LE. Also I would just start with a few values that we currently want to change.

> +
> +	This command can be used any time, but will not take effect until the
> +	next activity requiring the parameters.  In the case the parameters are
> +	used during initialization of the adapter, it is expected that the
> +	parameters be set before the adapter is powered.

Hmmm. This one, I might want to limit this controllers powered down. Otherwise you get into the conflict case that we would expect parameters taking affect and I think that is just too complicated.

> +
> +	This command generates a Command Complete event on success or
> +	a Command Status event on failure.
> +
> +	Possible errors:	Invalid Parameters
> +				Invalid Index
> 

Regards

Marcel
Alain Michaud May 6, 2020, 2:06 p.m. UTC | #2
Hi Marcel,

On Wed, May 6, 2020 at 7:22 AM Marcel Holtmann <marcel@holtmann.org> wrote:
>
> Hi Alain,
>
> > This change adds the definition for the load default parameter command.
> > In particular, this command is used to load default parameters for
> > various operations in the kernel. This mechanism is also expandable to
> > future knobs that may be necessary.
> >
> > This will allow bluetoothd to load parameters from a conf file that may
> > be customized for the specific requirements of each platforms.
> >
> > ---
> >
> > doc/mgmt-api.txt | 59 ++++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 59 insertions(+)
> >
> > diff --git a/doc/mgmt-api.txt b/doc/mgmt-api.txt
> > index 39f23c456..71d528706 100644
> > --- a/doc/mgmt-api.txt
> > +++ b/doc/mgmt-api.txt
> > @@ -3137,6 +3137,65 @@ Read Security Information Command
> >       Possible errors:        Invalid Parameters
> >                               Invalid Index
> >
> > +Load Default Parameter Command
> > +=============================
>
> I think that Load Controller Default Parameters is a better name.
I wanted to avoid using "controller" in the name as some of these are
not really "controller" parameters but instead just general
parameters.

>
> > +
> > +     Command Code:           0x0049
> > +     Controller Index:       <controller id>
> > +     Command Parameters:     Parameter_Count (2 Octets)
> > +                             Parameter1 {
> > +                                     Parameter_Type (2 Octet)
> > +                                     Value_Length (1 Octet)
> > +                                     Value (0-255 Octets)
> > +                             }
> > +                             Parameter2 { }
> > +                             ...
> > +     Return Parameters:
> > +
> > +     This command is used to feed the kernel a list of default parameters.
> > +
> > +     Currently defined Parameter_Type values are:
> > +
> > +                     0x0000  BR/EDR Page Scan Type
> > +                     0x0001  BR/EDR Page Scan Interval
> > +                     0x0002  BR/EDR Page Scan Window
> > +                     0x0003  BR/EDR Inquiry Scan Type
> > +                     0x0004  BR/EDR Inquiry Scan Interval
> > +                     0x0005  BR/EDR Inquiry Scan Window
> > +                     0x0006  BR/EDR Link Supervision Timeout
> > +                     0x0007  BR/EDR Page Timeout
> > +                     0x0008  BR/EDR Min Sniff Interval
> > +                     0x0009  BR/EDR Max Sniff Interval
> > +                     0x0080  LE Advertisement Min Interval
> > +                     0x0081  LE Advertisement Max Interval
> > +                     0x0082  LE Multi Advertisement Rotation Interval
> > +                     0x0083  LE Scanning Interval for auto connect
> > +                     0x0084  LE Scanning Window for auto connect
> > +                     0x0085  LE Scanning Interval for HID only
> > +                     0x0086  LE Scanning Window for HID only
> > +                     0x0087  LE Scanning Interval for wake scenarios
> > +                     0x0088  LE Scanning Window for wake scenarios
> > +                     0x0089  LE Scanning Interval for discovery
> > +                     0x008a  LE Scanning Window for discovery
> > +                     0x008b  LE Scanning Interval for adv monitoring
> > +                     0x008c  LE Scanning Window for adv monitoring
> > +                     0x008d  LE Scanning Interval for connect
> > +                     0x008e  LE Scanning Window for connect
> > +                     0x008f  LE Min Connection Interval
> > +                     0x0090  LE Max Connection Interval
> > +                     0x0091  LE Connection Connection Latency
> > +                     0x0092  LE Connection Supervision Timeout
>
> I would just enumerate these and not try to split between 0x000 and 0x008 for BR/EDR and LE. Also I would just start with a few values that we currently want to change.
The goal was to try to keep BR and LE parameters together, but if you
prefer, I don't mind collapsing them.  Note that we intend to use all
of these, so I'd prefer not break this into a smaller subset.

>
> > +
> > +     This command can be used any time, but will not take effect until the
> > +     next activity requiring the parameters.  In the case the parameters are
> > +     used during initialization of the adapter, it is expected that the
> > +     parameters be set before the adapter is powered.
>
> Hmmm. This one, I might want to limit this controllers powered down. Otherwise you get into the conflict case that we would expect parameters taking affect and I think that is just too complicated.
Things like connection intervals and scan defaults actually apply the
next time the activity is started.  We need to be able to change these
without completely powering down the controller and tearing down all
connections etc.  Things that are obviously only used during
initialization, then I would expect they would require a power cycles.

>
> > +
> > +     This command generates a Command Complete event on success or
> > +     a Command Status event on failure.
> > +
> > +     Possible errors:        Invalid Parameters
> > +                             Invalid Index
> >
>
> Regards
>
> Marcel
>
Marcel Holtmann May 13, 2020, 7:48 a.m. UTC | #3
Hi Alain,

>>> This change adds the definition for the load default parameter command.
>>> In particular, this command is used to load default parameters for
>>> various operations in the kernel. This mechanism is also expandable to
>>> future knobs that may be necessary.
>>> 
>>> This will allow bluetoothd to load parameters from a conf file that may
>>> be customized for the specific requirements of each platforms.
>>> 
>>> ---
>>> 
>>> doc/mgmt-api.txt | 59 ++++++++++++++++++++++++++++++++++++++++++++++++
>>> 1 file changed, 59 insertions(+)
>>> 
>>> diff --git a/doc/mgmt-api.txt b/doc/mgmt-api.txt
>>> index 39f23c456..71d528706 100644
>>> --- a/doc/mgmt-api.txt
>>> +++ b/doc/mgmt-api.txt
>>> @@ -3137,6 +3137,65 @@ Read Security Information Command
>>>      Possible errors:        Invalid Parameters
>>>                              Invalid Index
>>> 
>>> +Load Default Parameter Command
>>> +=============================
>> 
>> I think that Load Controller Default Parameters is a better name.
> I wanted to avoid using "controller" in the name as some of these are
> not really "controller" parameters but instead just general
> parameters.

fair enough. Now the question is if we might want to split between controller defaults and host defaults.

>>> +
>>> +     Command Code:           0x0049
>>> +     Controller Index:       <controller id>
>>> +     Command Parameters:     Parameter_Count (2 Octets)
>>> +                             Parameter1 {
>>> +                                     Parameter_Type (2 Octet)
>>> +                                     Value_Length (1 Octet)
>>> +                                     Value (0-255 Octets)
>>> +                             }
>>> +                             Parameter2 { }
>>> +                             ...
>>> +     Return Parameters:
>>> +
>>> +     This command is used to feed the kernel a list of default parameters.
>>> +
>>> +     Currently defined Parameter_Type values are:
>>> +
>>> +                     0x0000  BR/EDR Page Scan Type
>>> +                     0x0001  BR/EDR Page Scan Interval
>>> +                     0x0002  BR/EDR Page Scan Window
>>> +                     0x0003  BR/EDR Inquiry Scan Type
>>> +                     0x0004  BR/EDR Inquiry Scan Interval
>>> +                     0x0005  BR/EDR Inquiry Scan Window
>>> +                     0x0006  BR/EDR Link Supervision Timeout
>>> +                     0x0007  BR/EDR Page Timeout
>>> +                     0x0008  BR/EDR Min Sniff Interval
>>> +                     0x0009  BR/EDR Max Sniff Interval
>>> +                     0x0080  LE Advertisement Min Interval
>>> +                     0x0081  LE Advertisement Max Interval
>>> +                     0x0082  LE Multi Advertisement Rotation Interval
>>> +                     0x0083  LE Scanning Interval for auto connect
>>> +                     0x0084  LE Scanning Window for auto connect
>>> +                     0x0085  LE Scanning Interval for HID only
>>> +                     0x0086  LE Scanning Window for HID only
>>> +                     0x0087  LE Scanning Interval for wake scenarios
>>> +                     0x0088  LE Scanning Window for wake scenarios
>>> +                     0x0089  LE Scanning Interval for discovery
>>> +                     0x008a  LE Scanning Window for discovery
>>> +                     0x008b  LE Scanning Interval for adv monitoring
>>> +                     0x008c  LE Scanning Window for adv monitoring
>>> +                     0x008d  LE Scanning Interval for connect
>>> +                     0x008e  LE Scanning Window for connect
>>> +                     0x008f  LE Min Connection Interval
>>> +                     0x0090  LE Max Connection Interval
>>> +                     0x0091  LE Connection Connection Latency
>>> +                     0x0092  LE Connection Supervision Timeout
>> 
>> I would just enumerate these and not try to split between 0x000 and 0x008 for BR/EDR and LE. Also I would just start with a few values that we currently want to change.
> The goal was to try to keep BR and LE parameters together, but if you
> prefer, I don't mind collapsing them.  Note that we intend to use all
> of these, so I'd prefer not break this into a smaller subset.

My OCD wants to do that as well, but my experience has been that this never actually works out. And we will have parameters that apply to both transports. Like for example the minimum encryption key size.

>>> +
>>> +     This command can be used any time, but will not take effect until the
>>> +     next activity requiring the parameters.  In the case the parameters are
>>> +     used during initialization of the adapter, it is expected that the
>>> +     parameters be set before the adapter is powered.
>> 
>> Hmmm. This one, I might want to limit this controllers powered down. Otherwise you get into the conflict case that we would expect parameters taking affect and I think that is just too complicated.
> Things like connection intervals and scan defaults actually apply the
> next time the activity is started.  We need to be able to change these
> without completely powering down the controller and tearing down all
> connections etc.  Things that are obviously only used during
> initialization, then I would expect they would require a power cycles.

Now we might actually better split this into a command for system default parameters and one for runtime default parameters. I think it would make the code a lot simpler since we can have clear error codes. Thoughts?

Regards

Marcel
Alain Michaud May 13, 2020, 2:23 p.m. UTC | #4
Hi Marcel,

On Wed, May 13, 2020 at 3:49 AM Marcel Holtmann <marcel@holtmann.org> wrote:
>
> Hi Alain,
>
> >>> This change adds the definition for the load default parameter command.
> >>> In particular, this command is used to load default parameters for
> >>> various operations in the kernel. This mechanism is also expandable to
> >>> future knobs that may be necessary.
> >>>
> >>> This will allow bluetoothd to load parameters from a conf file that may
> >>> be customized for the specific requirements of each platforms.
> >>>
> >>> ---
> >>>
> >>> doc/mgmt-api.txt | 59 ++++++++++++++++++++++++++++++++++++++++++++++++
> >>> 1 file changed, 59 insertions(+)
> >>>
> >>> diff --git a/doc/mgmt-api.txt b/doc/mgmt-api.txt
> >>> index 39f23c456..71d528706 100644
> >>> --- a/doc/mgmt-api.txt
> >>> +++ b/doc/mgmt-api.txt
> >>> @@ -3137,6 +3137,65 @@ Read Security Information Command
> >>>      Possible errors:        Invalid Parameters
> >>>                              Invalid Index
> >>>
> >>> +Load Default Parameter Command
> >>> +=============================
> >>
> >> I think that Load Controller Default Parameters is a better name.
> > I wanted to avoid using "controller" in the name as some of these are
> > not really "controller" parameters but instead just general
> > parameters.
>
> fair enough. Now the question is if we might want to split between controller defaults and host defaults.
>
> >>> +
> >>> +     Command Code:           0x0049
> >>> +     Controller Index:       <controller id>
> >>> +     Command Parameters:     Parameter_Count (2 Octets)
> >>> +                             Parameter1 {
> >>> +                                     Parameter_Type (2 Octet)
> >>> +                                     Value_Length (1 Octet)
> >>> +                                     Value (0-255 Octets)
> >>> +                             }
> >>> +                             Parameter2 { }
> >>> +                             ...
> >>> +     Return Parameters:
> >>> +
> >>> +     This command is used to feed the kernel a list of default parameters.
> >>> +
> >>> +     Currently defined Parameter_Type values are:
> >>> +
> >>> +                     0x0000  BR/EDR Page Scan Type
> >>> +                     0x0001  BR/EDR Page Scan Interval
> >>> +                     0x0002  BR/EDR Page Scan Window
> >>> +                     0x0003  BR/EDR Inquiry Scan Type
> >>> +                     0x0004  BR/EDR Inquiry Scan Interval
> >>> +                     0x0005  BR/EDR Inquiry Scan Window
> >>> +                     0x0006  BR/EDR Link Supervision Timeout
> >>> +                     0x0007  BR/EDR Page Timeout
> >>> +                     0x0008  BR/EDR Min Sniff Interval
> >>> +                     0x0009  BR/EDR Max Sniff Interval
> >>> +                     0x0080  LE Advertisement Min Interval
> >>> +                     0x0081  LE Advertisement Max Interval
> >>> +                     0x0082  LE Multi Advertisement Rotation Interval
> >>> +                     0x0083  LE Scanning Interval for auto connect
> >>> +                     0x0084  LE Scanning Window for auto connect
> >>> +                     0x0085  LE Scanning Interval for HID only
> >>> +                     0x0086  LE Scanning Window for HID only
> >>> +                     0x0087  LE Scanning Interval for wake scenarios
> >>> +                     0x0088  LE Scanning Window for wake scenarios
> >>> +                     0x0089  LE Scanning Interval for discovery
> >>> +                     0x008a  LE Scanning Window for discovery
> >>> +                     0x008b  LE Scanning Interval for adv monitoring
> >>> +                     0x008c  LE Scanning Window for adv monitoring
> >>> +                     0x008d  LE Scanning Interval for connect
> >>> +                     0x008e  LE Scanning Window for connect
> >>> +                     0x008f  LE Min Connection Interval
> >>> +                     0x0090  LE Max Connection Interval
> >>> +                     0x0091  LE Connection Connection Latency
> >>> +                     0x0092  LE Connection Supervision Timeout
> >>
> >> I would just enumerate these and not try to split between 0x000 and 0x008 for BR/EDR and LE. Also I would just start with a few values that we currently want to change.
> > The goal was to try to keep BR and LE parameters together, but if you
> > prefer, I don't mind collapsing them.  Note that we intend to use all
> > of these, so I'd prefer not break this into a smaller subset.
>
> My OCD wants to do that as well, but my experience has been that this never actually works out. And we will have parameters that apply to both transports. Like for example the minimum encryption key size.
>
> >>> +
> >>> +     This command can be used any time, but will not take effect until the
> >>> +     next activity requiring the parameters.  In the case the parameters are
> >>> +     used during initialization of the adapter, it is expected that the
> >>> +     parameters be set before the adapter is powered.
> >>
> >> Hmmm. This one, I might want to limit this controllers powered down. Otherwise you get into the conflict case that we would expect parameters taking affect and I think that is just too complicated.
> > Things like connection intervals and scan defaults actually apply the
> > next time the activity is started.  We need to be able to change these
> > without completely powering down the controller and tearing down all
> > connections etc.  Things that are obviously only used during
> > initialization, then I would expect they would require a power cycles.
>
> Now we might actually better split this into a command for system default parameters and one for runtime default parameters. I think it would make the code a lot simpler since we can have clear error codes. Thoughts?

That's a good idea.  Any recommendation on naming for the runtime
ones?  I guess it would also need to carry the name "update" of some
sort.
>
>
> Regards
>
> Marcel
>
diff mbox series

Patch

diff --git a/doc/mgmt-api.txt b/doc/mgmt-api.txt
index 39f23c456..71d528706 100644
--- a/doc/mgmt-api.txt
+++ b/doc/mgmt-api.txt
@@ -3137,6 +3137,65 @@  Read Security Information Command
 	Possible errors:	Invalid Parameters
 				Invalid Index
 
+Load Default Parameter Command
+=============================
+
+	Command Code:		0x0049
+	Controller Index:	<controller id>
+	Command Parameters:	Parameter_Count (2 Octets)
+				Parameter1 {
+					Parameter_Type (2 Octet)
+					Value_Length (1 Octet)
+					Value (0-255 Octets)
+				}
+				Parameter2 { }
+				...
+	Return Parameters:
+
+	This command is used to feed the kernel a list of default parameters.
+
+	Currently defined Parameter_Type values are:
+
+			0x0000	BR/EDR Page Scan Type
+			0x0001	BR/EDR Page Scan Interval
+			0x0002	BR/EDR Page Scan Window
+			0x0003	BR/EDR Inquiry Scan Type
+			0x0004	BR/EDR Inquiry Scan Interval
+			0x0005	BR/EDR Inquiry Scan Window
+			0x0006	BR/EDR Link Supervision Timeout
+			0x0007	BR/EDR Page Timeout
+			0x0008	BR/EDR Min Sniff Interval
+			0x0009	BR/EDR Max Sniff Interval
+			0x0080	LE Advertisement Min Interval
+			0x0081	LE Advertisement Max Interval
+			0x0082	LE Multi Advertisement Rotation Interval
+			0x0083	LE Scanning Interval for auto connect
+			0x0084	LE Scanning Window for auto connect
+			0x0085	LE Scanning Interval for HID only
+			0x0086	LE Scanning Window for HID only
+			0x0087	LE Scanning Interval for wake scenarios
+			0x0088	LE Scanning Window for wake scenarios
+			0x0089	LE Scanning Interval for discovery
+			0x008a	LE Scanning Window for discovery
+			0x008b	LE Scanning Interval for adv monitoring
+			0x008c	LE Scanning Window for adv monitoring
+			0x008d	LE Scanning Interval for connect
+			0x008e	LE Scanning Window for connect
+			0x008f	LE Min Connection Interval
+			0x0090	LE Max Connection Interval
+			0x0091	LE Connection Connection Latency
+			0x0092	LE Connection Supervision Timeout
+
+	This command can be used any time, but will not take effect until the
+	next activity requiring the parameters.  In the case the parameters are
+	used during initialization of the adapter, it is expected that the
+	parameters be set before the adapter is powered.
+
+	This command generates a Command Complete event on success or
+	a Command Status event on failure.
+
+	Possible errors:	Invalid Parameters
+				Invalid Index
 
 Command Complete Event
 ======================